From 88a253927ede5fec2fe40d1e2b417f1f4a2db74f Mon Sep 17 00:00:00 2001 From: kushalmalani Date: Thu, 9 Jun 2022 23:55:32 -0700 Subject: [PATCH] fix for a non system admin user to create a deployment (#594) * fix for a non system admin user to create a deployment * Changing function arguments * fix unit tests & linting changes Co-authored-by: neel-astro --- astro-client/astro.go | 6 ++-- astro-client/astro_test.go | 4 +-- astro-client/mocks/Client.go | 46 ++++++++++++++--------------- cloud/deployment/deployment.go | 3 +- cloud/deployment/deployment_test.go | 10 +++---- cmd/cloud/deployment_test.go | 2 +- 6 files changed, 35 insertions(+), 36 deletions(-) diff --git a/astro-client/astro.go b/astro-client/astro.go index d9326da4b..ea3262eee 100644 --- a/astro-client/astro.go +++ b/astro-client/astro.go @@ -22,7 +22,7 @@ type Client interface { CreateImage(input ImageCreateInput) (*Image, error) DeployImage(input ImageDeployInput) (*Image, error) // Cluster - ListClusters(vars map[string]interface{}) ([]Cluster, error) + ListClusters(organizationID string) ([]Cluster, error) // RuntimeRelease ListInternalRuntimeReleases() ([]RuntimeRelease, error) ListPublicRuntimeReleases() ([]RuntimeRelease, error) @@ -174,10 +174,10 @@ func (c *HTTPClient) DeployImage(input ImageDeployInput) (*Image, error) { return resp.Data.DeployImage, nil } -func (c *HTTPClient) ListClusters(vars map[string]interface{}) ([]Cluster, error) { +func (c *HTTPClient) ListClusters(organizationID string) ([]Cluster, error) { req := Request{ Query: GetClusters, - Variables: map[string]interface{}{"input": vars}, + Variables: map[string]interface{}{"organizationId": organizationID}, } resp, err := req.DoWithClient(c) diff --git a/astro-client/astro_test.go b/astro-client/astro_test.go index 323e1ab8e..24201c493 100644 --- a/astro-client/astro_test.go +++ b/astro-client/astro_test.go @@ -592,7 +592,7 @@ func TestListClusters(t *testing.T) { }) astroClient := NewAstroClient(client) - resp, err := astroClient.ListClusters(map[string]interface{}{}) + resp, err := astroClient.ListClusters("test-org-id") assert.NoError(t, err) assert.Equal(t, resp, mockResponse.Data.GetClusters) }) @@ -607,7 +607,7 @@ func TestListClusters(t *testing.T) { }) astroClient := NewAstroClient(client) - _, err := astroClient.ListClusters(map[string]interface{}{}) + _, err := astroClient.ListClusters("test-org-id") assert.Contains(t, err.Error(), "Internal Server Error") }) } diff --git a/astro-client/mocks/Client.go b/astro-client/mocks/Client.go index 4dd0e6ab6..841a8624b 100644 --- a/astro-client/mocks/Client.go +++ b/astro-client/mocks/Client.go @@ -142,6 +142,29 @@ func (_m *Client) GetDeploymentHistory(vars map[string]interface{}) (astro.Deplo return r0, r1 } +// ListClusters provides a mock function with given fields: organizationId +func (_m *Client) ListClusters(organizationId string) ([]astro.Cluster, error) { + ret := _m.Called(organizationId) + + var r0 []astro.Cluster + if rf, ok := ret.Get(0).(func(string) []astro.Cluster); ok { + r0 = rf(organizationId) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).([]astro.Cluster) + } + } + + var r1 error + if rf, ok := ret.Get(1).(func(string) error); ok { + r1 = rf(organizationId) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + // ListDeployments provides a mock function with given fields: input func (_m *Client) ListDeployments(input astro.DeploymentsInput) ([]astro.Deployment, error) { ret := _m.Called(input) @@ -188,29 +211,6 @@ func (_m *Client) ListInternalRuntimeReleases() ([]astro.RuntimeRelease, error) return r0, r1 } -// ListClusters provides a mock function with given fields: vars -func (_m *Client) ListClusters(vars map[string]interface{}) ([]astro.Cluster, error) { - ret := _m.Called(vars) - - var r0 []astro.Cluster - if rf, ok := ret.Get(0).(func(map[string]interface{}) []astro.Cluster); ok { - r0 = rf(vars) - } else { - if ret.Get(0) != nil { - r0 = ret.Get(0).([]astro.Cluster) - } - } - - var r1 error - if rf, ok := ret.Get(1).(func(map[string]interface{}) error); ok { - r1 = rf(vars) - } else { - r1 = ret.Error(1) - } - - return r0, r1 -} - // ListPublicRuntimeReleases provides a mock function with given fields: func (_m *Client) ListPublicRuntimeReleases() ([]astro.RuntimeRelease, error) { ret := _m.Called() diff --git a/cloud/deployment/deployment.go b/cloud/deployment/deployment.go index ffa80837b..aab107259 100644 --- a/cloud/deployment/deployment.go +++ b/cloud/deployment/deployment.go @@ -288,8 +288,7 @@ func selectCluster(clusterID, organizationID string, client astro.Client) (newCl Header: []string{"#", "CLUSTER NAME", "CLOUD PROVIDER", "CLUSTER ID"}, } // cluster request - clusterInput := map[string]interface{}{"organizationId": organizationID} - cs, err := client.ListClusters(clusterInput) + cs, err := client.ListClusters(organizationID) if err != nil { return "", errors.Wrap(err, astro.AstronomerConnectionErrMsg) } diff --git a/cloud/deployment/deployment_test.go b/cloud/deployment/deployment_test.go index 6e5acb38d..22bce449b 100644 --- a/cloud/deployment/deployment_test.go +++ b/cloud/deployment/deployment_test.go @@ -177,7 +177,7 @@ func TestCreate(t *testing.T) { mockClient := new(astro_mocks.Client) mockClient.On("GetDeploymentConfig").Return(astro.DeploymentConfig{RuntimeReleases: []astro.RuntimeRelease{{Version: "4.2.5"}}}, nil).Once() mockClient.On("ListWorkspaces").Return([]astro.Workspace{{ID: ws, OrganizationID: "test-org-id"}}, nil).Once() - mockClient.On("ListClusters", map[string]interface{}{"organizationId": "test-org-id"}).Return([]astro.Cluster{{ID: csID}}, nil).Once() + mockClient.On("ListClusters", "test-org-id").Return([]astro.Cluster{{ID: csID}}, nil).Once() mockClient.On("CreateDeployment", mock.Anything).Return(astro.Deployment{ID: "test-id"}, nil).Once() // mock os.Stdin @@ -266,7 +266,7 @@ func TestSelectCluster(t *testing.T) { csID := "test-cluster-id" t.Run("list cluster failure", func(t *testing.T) { mockClient := new(astro_mocks.Client) - mockClient.On("ListClusters", map[string]interface{}{"organizationId": orgID}).Return([]astro.Cluster{}, errMock).Once() + mockClient.On("ListClusters", orgID).Return([]astro.Cluster{}, errMock).Once() _, err := selectCluster("", orgID, mockClient) assert.ErrorIs(t, err, errMock) @@ -275,7 +275,7 @@ func TestSelectCluster(t *testing.T) { t.Run("cluster id via selection", func(t *testing.T) { mockClient := new(astro_mocks.Client) - mockClient.On("ListClusters", map[string]interface{}{"organizationId": orgID}).Return([]astro.Cluster{{ID: csID}}, nil).Once() + mockClient.On("ListClusters", orgID).Return([]astro.Cluster{{ID: csID}}, nil).Once() // mock os.Stdin input := []byte("1") @@ -300,7 +300,7 @@ func TestSelectCluster(t *testing.T) { t.Run("cluster id invalid selection", func(t *testing.T) { mockClient := new(astro_mocks.Client) - mockClient.On("ListClusters", map[string]interface{}{"organizationId": orgID}).Return([]astro.Cluster{{ID: csID}}, nil).Once() + mockClient.On("ListClusters", orgID).Return([]astro.Cluster{{ID: csID}}, nil).Once() // mock os.Stdin input := []byte("4") @@ -324,7 +324,7 @@ func TestSelectCluster(t *testing.T) { t.Run("not able to find cluster", func(t *testing.T) { mockClient := new(astro_mocks.Client) - mockClient.On("ListClusters", map[string]interface{}{"organizationId": orgID}).Return([]astro.Cluster{{ID: csID}}, nil).Once() + mockClient.On("ListClusters", orgID).Return([]astro.Cluster{{ID: csID}}, nil).Once() _, err := selectCluster("test-invalid-id", orgID, mockClient) assert.Error(t, err) diff --git a/cmd/cloud/deployment_test.go b/cmd/cloud/deployment_test.go index 83dee8908..ca82fa9bc 100644 --- a/cmd/cloud/deployment_test.go +++ b/cmd/cloud/deployment_test.go @@ -83,7 +83,7 @@ func TestDeploymentCreate(t *testing.T) { mockClient := new(astro_mocks.Client) mockClient.On("GetDeploymentConfig").Return(astro.DeploymentConfig{RuntimeReleases: []astro.RuntimeRelease{{Version: "4.2.5"}}}, nil).Once() mockClient.On("ListWorkspaces").Return([]astro.Workspace{{ID: ws, OrganizationID: "test-org-id"}}, nil).Once() - mockClient.On("ListClusters", map[string]interface{}{"organizationId": "test-org-id"}).Return([]astro.Cluster{{ID: csID}}, nil).Once() + mockClient.On("ListClusters", "test-org-id").Return([]astro.Cluster{{ID: csID}}, nil).Once() mockClient.On("CreateDeployment", mock.Anything).Return(astro.Deployment{ID: "test-id"}, nil).Once() astroClient = mockClient