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.

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

Feat: add support for update credentials #709

Merged
merged 6 commits into from
Sep 18, 2023

Conversation

TomerHeber
Copy link
Collaborator

@TomerHeber TomerHeber commented Sep 12, 2023

Issue & Steps to Reproduce / Feature Request

Resolves #678

Solution

Part of this PR was upgrading to go 1.20 and replace gomock location (original project has been deprecated).
I have also updated github action versions. Therefore, "87" files are changed.

Some guidelines for reviewing this PR:

  1. client/cloud_credentials.go - added the new API for updating credentials.
  2. env0/resource_cost_credentials.go - was refactored, and the update operation was added.
  3. env0/resource_cost_credentials_test.go - acceptance tests were updated.
  4. The rest of the files are minor changes.

@@ -8,7 +8,7 @@ env:
ENV0_API_ENDPOINT: ${{ secrets.ENV0_API_ENDPOINT }}
ENV0_API_KEY: ${{ secrets.TF_PROVIDER_INTEGRATION_TEST_API_KEY }} # API Key for organization 'TF-provider-integration-tests' @ dev
ENV0_API_SECRET: ${{ secrets.TF_PROVIDER_INTEGRATION_TEST_API_SECRET }}
GO_VERSION: 1.19
GO_VERSION: "1.20"
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

had to add quotes. There's a bug/issue if this isn't done. (Will use go 1.2).

- name: Generate mocks
run: |
go install github.com/golang/mock/[email protected]
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 has been deprecated. Ownership moved to Uber.

@@ -143,10 +143,27 @@ func (client *ApiClient) CredentialsCreate(request CredentialCreatePayload) (Cre
request.SetOrganizationId(organizationId)

var result Credentials
err = client.http.Post("/credentials", request, &result)
if err := client.http.Post("/credentials", request, &result); err != nil {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

github diff mixup... this did not change other than this line.

return result, nil
}

func (client *ApiClient) CredentialsUpdate(id string, request CredentialCreatePayload) (Credentials, error) {
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 all new

@@ -115,6 +115,46 @@ var _ = Describe("CloudCredentials", func() {
})
})

Describe("AwsCredentialsUpdate", func() {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

unit test for the new update API.

@@ -2,7 +2,6 @@ package env0

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 code has been refactored. Easier to support the Update use-case without duplicating code.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

might be easier to just review the whole file.

@TomerHeber TomerHeber requested a review from yaronya September 13, 2023 16:30
go.mod Show resolved Hide resolved
@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 Sep 18, 2023
@TomerHeber TomerHeber merged commit 51748f7 into main Sep 18, 2023
14 checks passed
@TomerHeber TomerHeber deleted the feat-update-credentials-#678 branch September 18, 2023 15:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

env0_azure_cost_credentials Update Credentials Instead of Replace
2 participants