Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix: using git token_id on Templates always assumes GitLab #929

Merged
merged 1 commit into from
Aug 7, 2024
Merged

Fix: using git token_id on Templates always assumes GitLab #929

merged 1 commit into from
Aug 7, 2024

Conversation

TomerHeber
Copy link
Collaborator

Issue & Steps to Reproduce / Feature Request

fixes #545

Solution

  1. Added is_gitlab to the schema and updated the code.
  2. Updated the API struct.
  3. Updated the acceptance tests.
  4. As part of an effort to replace the linter, I'm slowly doing other small fixes.

@TomerHeber TomerHeber self-assigned this Aug 7, 2024
TomerHeber
TomerHeber commented Aug 7, 2024
View reviewed changes
.golangci.yaml
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is for a new linter... ignore unrelated changes... it's a lot of work so I'm slowly adding it to the PRS.

Copy link
Collaborator Author

@TomerHeber TomerHeber Aug 7, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

review only files related to the issue:

template.go
resource_template.go
resource_template_test.go

TomerHeber
TomerHeber commented Aug 7, 2024
View reviewed changes
env0/resource_template.go
@@ -385,14 +391,10 @@ func templateCreatePayloadFromParameters(prefix string, d *schema.ResourceData)

isNew := d.IsNewResource()

tokenIdKey := "token_id"
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not required anymore. Gitlab should be explicitly configured. This is a breaking change that was discussed in Slack.
Need to add to release notes.

TomerHeber
TomerHeber commented Aug 7, 2024
View reviewed changes
env0/resource_template_test.go
@@ -628,7 +633,7 @@ func TestUnitTemplateResource(t *testing.T) {
Description: templateUseCase.template.Description,
GithubInstallationId: templateUseCase.template.GithubInstallationId,
IsGitlabEnterprise: templateUseCase.template.IsGitlabEnterprise,
IsGitLab: templateUseCase.template.TokenId != "" && !templateUseCase.template.IsAzureDevOps,
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

now just check the field excplicitly.

@TomerHeber TomerHeber requested a review from yaronya August 7, 2024 13:50
Wassap124
Wassap124 approved these changes Aug 7, 2024
View reviewed changes
Copy link
Contributor

@Wassap124 Wassap124 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

well, it seems that you indeed do what's requested,
but i have a lot of questions

env0/cloud_configuration.go
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does this have anything to do with the issue?

env0/data_agent_values.go
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does this have anything to do with the issue?

env0/data_cloud_credentials.go
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does this have anything to do with the issue?

env0/data_configuration_variable.go
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does this have anything to do with the issue?

env0/data_custom_roles.go
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does this have anything to do with the issue?

env0/data_organization.go
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does this have anything to do with the issue?

env0/resource_environment.go
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does this have anything to do with the issue?

env0/resource_variable_set.go
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does this have anything to do with the issue?

env0/resource_variable_set_assignment.go
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does this have anything to do with the issue?

env0/data_project.go
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does this have anything to do with the issue?

@github-actions github-actions bot added ready to merge PR approved - can be merged once the PR owner is ready and removed pending final review labels Aug 7, 2024
@TomerHeber TomerHeber merged commit d93d1f0 into main Aug 7, 2024
12 checks passed
@TomerHeber TomerHeber deleted the fix-add-is_gitlab-to-schema-#545 branch August 7, 2024 16:40
@Wassap124
Copy link
Contributor

Can't create template using provider and git token

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Wassap124 Wassap124 Wassap124 approved these changes

@yaronya yaronya Awaiting requested review from yaronya

Assignees

@TomerHeber TomerHeber

Labels
api-client fix provider ready to merge PR approved - can be merged once the PR owner is ready
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Using git token_id on Templates always assumes GitLab
2 participants
@TomerHeber @Wassap124