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: support personal API keys #616

Merged
merged 2 commits into from
Mar 7, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 7 additions & 5 deletions client/api_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,9 @@ import (
)

type ApiClient struct {
http http.HttpClientInterface
cachedOrganizationId string
http http.HttpClientInterface
cachedOrganizationId string
defaultOrganizationId string
}

type ApiClientInterface interface {
Expand Down Expand Up @@ -117,9 +118,10 @@ type ApiClientInterface interface {
CustomFlowGetAssignments(assignments []CustomFlowAssignment) ([]CustomFlowAssignment, error)
}

func NewApiClient(client http.HttpClientInterface) ApiClientInterface {
func NewApiClient(client http.HttpClientInterface, defaultOrganizationId string) ApiClientInterface {
return &ApiClient{
http: client,
cachedOrganizationId: "",
http: client,
cachedOrganizationId: "",
defaultOrganizationId: defaultOrganizationId,
}
}
5 changes: 3 additions & 2 deletions client/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@ import (
// This file wraps the test suite for the entire client folder

const (
organizationId = "organization0"
organizationId = "organization0"
defaultOrganizationId = "organization1"
)

var (
Expand All @@ -31,7 +32,7 @@ var _ = BeforeSuite(func() {

var _ = BeforeEach(func() {
mockHttpClient = http.NewMockHttpClientInterface(ctrl)
apiClient = NewApiClient(mockHttpClient)
apiClient = NewApiClient(mockHttpClient, defaultOrganizationId)
})

var _ = AfterSuite(func() {
Expand Down
25 changes: 0 additions & 25 deletions client/notification_test.go
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
package client_test

import (
"errors"

. "github.com/env0/terraform-provider-env0/client"
gomock "github.com/golang/mock/gomock"
"github.com/jinzhu/copier"
Expand Down Expand Up @@ -87,29 +85,6 @@ var _ = Describe("Notification Client", func() {
Expect(*createdNotification).To(Equal(mockNotification))
})
})

// NotificationCreate calls "organizationId()""
// The test was "randomly" added here to test organizationId failure (when there are multiple organizations).
// Ideally should be tested in organization. Unfortunatly, organizationId is private and cannot be tested directly.
// (test package name varies from package name).
Describe("Failure - Multiple Organizations", 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.

This is no longer true.
This is tested in the organization test.

var err error

BeforeEach(func() {
organizationsResult := []Organization{{}, {}}
mockHttpClient.EXPECT().
Get("/organizations", nil, gomock.Any()).
Do(func(path string, request interface{}, response *[]Organization) {
*response = organizationsResult
})
_, err = apiClient.NotificationCreate(NotificationCreatePayload{})

})

It("Should return error", func() {
Expect(err).To(BeEquivalentTo(errors.New("server responded with too many organizations")))
})
})
})

Describe("NotificationDelete", func() {
Expand Down
13 changes: 12 additions & 1 deletion client/organization.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package client

import (
"errors"
"fmt"
)

type Organization struct {
Expand Down Expand Up @@ -36,7 +37,17 @@ func (client *ApiClient) Organization() (Organization, error) {
return Organization{}, err
}
if len(result) != 1 {
return Organization{}, errors.New("server responded with too many organizations")
if client.defaultOrganizationId != "" {
for _, organization := range result {
if organization.Id == client.defaultOrganizationId {
return organization, nil
}
}

return Organization{}, fmt.Errorf("the api key is not assigned to organization id: %s", client.defaultOrganizationId)
}

return Organization{}, errors.New("server responded with too many organizations (set a default organization id in the provider settings)")
}
return result[0], nil
}
Expand Down
29 changes: 28 additions & 1 deletion client/organization_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,11 @@ var _ = Describe("Organization", func() {
Name: "env0 🦄",
}

mockDefaultOrganization := Organization{
Id: defaultOrganizationId,
Name: "default",
}

Describe("Organization", func() {
var organization Organization
var err error
Expand All @@ -41,6 +46,28 @@ var _ = Describe("Organization", func() {
})
})

Describe("Default Organization", func() {
BeforeEach(func() {
organizationsResult := []Organization{mockOrganization, mockDefaultOrganization}
httpCall = mockHttpClient.EXPECT().
Get("/organizations", nil, gomock.Any()).
Do(func(path string, request interface{}, response *[]Organization) {
*response = organizationsResult
})

organization, err = apiClient.Organization()
})

It("Should send GET request once", func() {
httpCall.Times(1)
})

It("Should return organization", func() {
Expect(organization).Should(Equal(mockDefaultOrganization))
Expect(err).Should(BeNil())
})
})

Describe("Failure", func() {
It("On error from server return the error", func() {
expectedErr := errors.New("some error")
Expand All @@ -62,7 +89,7 @@ var _ = Describe("Organization", func() {

_, err = apiClient.Organization()
Expect(err).ShouldNot(BeNil())
Expect(err.Error()).Should(Equal("server responded with too many organizations"))
Expect(err.Error()).Should(Equal("the api key is not assigned to organization id: " + defaultOrganizationId))
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

the way unit tests are built... I could only test this use case.

})
})
})
Expand Down
7 changes: 6 additions & 1 deletion env0/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,11 @@ func Provider(version string) plugin.ProviderFunc {
Required: true,
Sensitive: true,
},
"organization_id": {
Type: schema.TypeString,
Description: "when the API key is associated with multiple organizations, this field is required. If an API key has one organization, this field is ignored.",
Optional: true,
},
},
DataSourcesMap: map[string]*schema.Resource{
"env0_organization": dataOrganization(),
Expand Down Expand Up @@ -174,7 +179,7 @@ func configureProvider(version string, p *schema.Provider) schema.ConfigureConte
return nil, diag.Diagnostics{diag.Diagnostic{Severity: diag.Error, Summary: err.Error()}}
}

apiClient := client.NewApiClient(httpClient)
apiClient := client.NewApiClient(httpClient, d.Get("organization_id").(string))

// organizations fetched to cache Auth0 API response.
if _, err := apiClient.OrganizationId(); err != nil {
Expand Down