From d1237df735a785d0434af93263dc2626d04250e6 Mon Sep 17 00:00:00 2001 From: yaellec Date: Fri, 4 Mar 2022 10:01:33 +0100 Subject: [PATCH 1/5] Updated endpoint to return list of all clusters --- pkg/redshift/api/api.go | 46 ++++++++---------- pkg/redshift/api/api_test.go | 84 +++++++++++++++++---------------- pkg/redshift/models/settings.go | 7 +-- 3 files changed, 67 insertions(+), 70 deletions(-) diff --git a/pkg/redshift/api/api.go b/pkg/redshift/api/api.go index 4144b848..38e0f0fe 100644 --- a/pkg/redshift/api/api.go +++ b/pkg/redshift/api/api.go @@ -23,10 +23,10 @@ import ( ) type API struct { - DataClient redshiftdataapiserviceiface.RedshiftDataAPIServiceAPI - SecretsClient secretsmanageriface.SecretsManagerAPI - ManagementClient redshiftiface.RedshiftAPI - settings *models.RedshiftDataSourceSettings + DataClient redshiftdataapiserviceiface.RedshiftDataAPIServiceAPI + SecretsClient secretsmanageriface.SecretsManagerAPI + ManagementClient redshiftiface.RedshiftAPI + settings *models.RedshiftDataSourceSettings } func New(sessionCache *awsds.SessionCache, settings awsModels.Settings) (api.AWSAPI, error) { @@ -54,10 +54,10 @@ func New(sessionCache *awsds.SessionCache, settings awsModels.Settings) (api.AWS } return &API{ - DataClient: redshiftdataapiservice.New(sess), - SecretsClient: secretsmanager.New(sess), + DataClient: redshiftdataapiservice.New(sess), + SecretsClient: secretsmanager.New(sess), ManagementClient: redshift.New(sess), - settings: redshiftSettings, + settings: redshiftSettings, }, nil } @@ -342,30 +342,24 @@ func (c *API) Secret(ctx aws.Context, options sqlds.Options) (*models.RedshiftSe return res, nil } -func (c *API) Cluster(options sqlds.Options) (*models.RedshiftCluster, error) { - clusterId := options["clusterIdentifier"] - input := &redshift.DescribeClustersInput{ - ClusterIdentifier: aws.String(clusterId), - } - out, err := c.ManagementClient.DescribeClusters(input) +func (c *API) Clusters() ([]models.RedshiftCluster, error) { + out, err := c.ManagementClient.DescribeClusters(&redshift.DescribeClustersInput{}) if err != nil { return nil, err } if out == nil { - return nil, fmt.Errorf("missing cluster content") + return nil, fmt.Errorf("missing clusters content") } - res := &models.RedshiftCluster{} - for _,r := range out.Clusters { - if (r != nil && r.ClusterIdentifier != nil && *r.ClusterIdentifier == clusterId && r.Endpoint != nil && r.Endpoint.Address != nil && r.Endpoint.Port != nil) { - res.Endpoint = models.RedshiftEndpoint{ + res := []models.RedshiftCluster{} + for _, r := range out.Clusters { + res = append(res, models.RedshiftCluster{ + ClusterIdentifier: *r.ClusterIdentifier, + Endpoint: models.RedshiftEndpoint{ Address: *r.Endpoint.Address, - Port: *r.Endpoint.Port, - } - if (r.DBName != nil) { - res.Database = *r.DBName - } - return res, nil - } + Port: *r.Endpoint.Port, + }, + Database: *r.DBName, + }) } - return nil, fmt.Errorf("ClusterId %s not found", clusterId) + return res, nil } diff --git a/pkg/redshift/api/api_test.go b/pkg/redshift/api/api_test.go index 5562f04b..89473663 100644 --- a/pkg/redshift/api/api_test.go +++ b/pkg/redshift/api/api_test.go @@ -65,8 +65,8 @@ func Test_apiInput(t *testing.T) { func Test_Execute(t *testing.T) { c := &API{ - settings: &models.RedshiftDataSourceSettings{}, - DataClient: &redshiftclientmock.MockRedshiftClient{ExecutionResult: &redshiftdataapiservice.ExecuteStatementOutput{Id: aws.String("foo")}}, + settings: &models.RedshiftDataSourceSettings{}, + DataClient: &redshiftclientmock.MockRedshiftClient{ExecutionResult: &redshiftdataapiservice.ExecuteStatementOutput{Id: aws.String("foo")}}, } res, err := c.Execute(context.TODO(), &api.ExecuteQueryInput{Query: "select * from foo"}) if err != nil { @@ -132,8 +132,8 @@ func Test_ListSchemas(t *testing.T) { } expectedResult := []string{"bar", "foo"} c := &API{ - settings: &models.RedshiftDataSourceSettings{}, - DataClient: &redshiftclientmock.MockRedshiftClient{Resources: resources}, + settings: &models.RedshiftDataSourceSettings{}, + DataClient: &redshiftclientmock.MockRedshiftClient{Resources: resources}, } res, err := c.Schemas(context.TODO(), sqlds.Options{}) if err != nil { @@ -156,8 +156,8 @@ func Test_ListTables(t *testing.T) { } expectedResult := []string{"foofoo"} c := &API{ - settings: &models.RedshiftDataSourceSettings{}, - DataClient: &redshiftclientmock.MockRedshiftClient{Resources: resources}, + settings: &models.RedshiftDataSourceSettings{}, + DataClient: &redshiftclientmock.MockRedshiftClient{Resources: resources}, } res, err := c.Tables(context.TODO(), sqlds.Options{"schema": "foo"}) if err != nil { @@ -182,8 +182,8 @@ func Test_ListColumns(t *testing.T) { } expectedResult := []string{"col1", "col2"} c := &API{ - settings: &models.RedshiftDataSourceSettings{}, - DataClient: &redshiftclientmock.MockRedshiftClient{Resources: resources}, + settings: &models.RedshiftDataSourceSettings{}, + DataClient: &redshiftclientmock.MockRedshiftClient{Resources: resources}, } res, err := c.Columns(context.TODO(), sqlds.Options{"schema": "public", "table": "foo"}) if err != nil { @@ -218,58 +218,60 @@ func Test_GetSecret(t *testing.T) { } } -func Test_GetCluster(t *testing.T) { - fooC := &API{ManagementClient: &redshiftclientmock.MockRedshiftClient{Clusters: []string{"foo"}}} - c := &API{ManagementClient: &redshiftclientmock.MockRedshiftClient{Clusters: []string{"foo"}}} +func Test_GetClusters(t *testing.T) { + c := &API{ManagementClient: &redshiftclientmock.MockRedshiftClient{Clusters: []string{"foo", "bar"}}} errC := &API{ManagementClient: &redshiftclientmock.MockRedshiftClientError{}} nilC := &API{ManagementClient: &redshiftclientmock.MockRedshiftClientNil{}} - expectedCluster := &models.RedshiftCluster{ - Endpoint: models.RedshiftEndpoint{ - Address: "foo", - Port: 123, - }, + expectedCluster1 := &models.RedshiftCluster{ + ClusterIdentifier: "foo", + Endpoint: models.RedshiftEndpoint{ + Address: "foo", + Port: 123, + }, Database: "foo", } + expectedCluster2 := &models.RedshiftCluster{ + ClusterIdentifier: "bar", + Endpoint: models.RedshiftEndpoint{ + Address: "bar", + Port: 123, + }, + Database: "bar", + } tests := []struct { - c *API - desc string - clusterId string - errMsg string - expectedCluster *models.RedshiftCluster + c *API + desc string + clusterId string + errMsg string + expectedClusters []models.RedshiftCluster }{ { - c: c, - desc: "Happy Path", - clusterId: "foo", - expectedCluster: expectedCluster, - }, - { - c: fooC, - desc: "Error cluster ID not found", - clusterId: "xyz", - errMsg: "ClusterId xyz not found", + c: c, + desc: "Happy Path", + clusterId: "foo", + expectedClusters: []models.RedshiftCluster{*expectedCluster1, *expectedCluster2}, }, { - c: errC, - desc: "Error with DescribeCluster", + c: errC, + desc: "Error with DescribeCluster", clusterId: "foo", - errMsg: "Boom!", + errMsg: "Boom!", }, { - c: nilC, - desc: "DescribeCluster returned nil", + c: nilC, + desc: "DescribeCluster returned nil", clusterId: "foo", - errMsg: "missing cluster content", + errMsg: "missing clusters content", }, } for _, tt := range tests { t.Run(tt.desc, func(t *testing.T) { - cluster, err := tt.c.Cluster(sqlds.Options{"clusterIdentifier": tt.clusterId}) - if (tt.errMsg == "") { + clusters, err := tt.c.Clusters() + if tt.errMsg == "" { assert.NoError(t, err) - assert.Equal(t, expectedCluster, cluster) + assert.Equal(t, tt.expectedClusters, clusters) } else { - assert.Nil(t, cluster) + assert.Nil(t, clusters) assert.EqualError(t, err, tt.errMsg) } }) diff --git a/pkg/redshift/models/settings.go b/pkg/redshift/models/settings.go index 6681b96e..4104297d 100644 --- a/pkg/redshift/models/settings.go +++ b/pkg/redshift/models/settings.go @@ -22,12 +22,13 @@ type RedshiftSecret struct { type RedshiftEndpoint struct { Address string `json:"address"` - Port int64 `json:"port"` + Port int64 `json:"port"` } type RedshiftCluster struct { - Endpoint RedshiftEndpoint `json:"endpoint"` - Database string `json:"database"` + ClusterIdentifier string `json:"clusterIdentifier"` + Endpoint RedshiftEndpoint `json:"endpoint"` + Database string `json:"database"` } type RedshiftDataSourceSettings struct { From 4ad151102c0ef715fb5f9052b6c7e521a06722a5 Mon Sep 17 00:00:00 2001 From: yaellec Date: Fri, 4 Mar 2022 10:13:57 +0100 Subject: [PATCH 2/5] Updated datasource and routes --- pkg/redshift/datasource.go | 6 +++--- pkg/redshift/fake/datasource.go | 6 +++--- pkg/redshift/routes/routes.go | 8 ++++---- pkg/redshift/routes/routes_test.go | 18 ++++++++++-------- 4 files changed, 20 insertions(+), 18 deletions(-) diff --git a/pkg/redshift/datasource.go b/pkg/redshift/datasource.go index ddfe4b65..66174a4d 100644 --- a/pkg/redshift/datasource.go +++ b/pkg/redshift/datasource.go @@ -25,7 +25,7 @@ type RedshiftDatasourceIface interface { Columns(ctx context.Context, options sqlds.Options) ([]string, error) Secrets(ctx context.Context, options sqlds.Options) ([]models.ManagedSecret, error) Secret(ctx context.Context, options sqlds.Options) (*models.RedshiftSecret, error) - Cluster(ctx context.Context, options sqlds.Options) (*models.RedshiftCluster, error) + Clusters(ctx context.Context, options sqlds.Options) ([]models.RedshiftCluster, error) } type RedshiftDatasource struct { @@ -141,10 +141,10 @@ func (s *RedshiftDatasource) Secret(ctx context.Context, options sqlds.Options) return api.Secret(ctx, options) } -func (s *RedshiftDatasource) Cluster(ctx context.Context, options sqlds.Options) (*models.RedshiftCluster, error) { +func (s *RedshiftDatasource) Clusters(ctx context.Context, options sqlds.Options) ([]models.RedshiftCluster, error) { api, err := s.getApi(ctx, options) if err != nil { return nil, err } - return api.Cluster(options) + return api.Clusters() } diff --git a/pkg/redshift/fake/datasource.go b/pkg/redshift/fake/datasource.go index bbb3ff3e..e7028b90 100644 --- a/pkg/redshift/fake/datasource.go +++ b/pkg/redshift/fake/datasource.go @@ -14,7 +14,7 @@ import ( type RedshiftFakeDatasource struct { SecretList []models.ManagedSecret RSecret models.RedshiftSecret - RCluster models.RedshiftCluster + RClusters []models.RedshiftCluster } func (s *RedshiftFakeDatasource) Settings(_ backend.DataSourceInstanceSettings) sqlds.DriverSettings { @@ -60,6 +60,6 @@ func (s *RedshiftFakeDatasource) Secrets(ctx context.Context, options sqlds.Opti func (s *RedshiftFakeDatasource) Secret(ctx context.Context, options sqlds.Options) (*models.RedshiftSecret, error) { return &s.RSecret, nil } -func (s *RedshiftFakeDatasource) Cluster(ctx context.Context, options sqlds.Options) (*models.RedshiftCluster, error) { - return &s.RCluster, nil +func (s *RedshiftFakeDatasource) Clusters(ctx context.Context, options sqlds.Options) ([]models.RedshiftCluster, error) { + return s.RClusters, nil } diff --git a/pkg/redshift/routes/routes.go b/pkg/redshift/routes/routes.go index b6deac26..9c575f48 100644 --- a/pkg/redshift/routes/routes.go +++ b/pkg/redshift/routes/routes.go @@ -33,21 +33,21 @@ func (r *RedshiftResourceHandler) secret(rw http.ResponseWriter, req *http.Reque routes.SendResources(rw, secret, err) } -func (r *RedshiftResourceHandler) cluster(rw http.ResponseWriter, req *http.Request) { +func (r *RedshiftResourceHandler) clusters(rw http.ResponseWriter, req *http.Request) { reqBody, err := routes.ParseBody(req.Body) if err != nil { rw.WriteHeader(http.StatusBadRequest) routes.Write(rw, []byte(err.Error())) return } - cluster, err := r.redshift.Cluster(req.Context(), reqBody) - routes.SendResources(rw, cluster, err) + clusters, err := r.redshift.Clusters(req.Context(), reqBody) + routes.SendResources(rw, clusters, err) } func (r *RedshiftResourceHandler) Routes() map[string]func(http.ResponseWriter, *http.Request) { routes := r.DefaultRoutes() routes["/secrets"] = r.secrets routes["/secret"] = r.secret - routes["/cluster"] = r.cluster + routes["/clusters"] = r.clusters return routes } diff --git a/pkg/redshift/routes/routes_test.go b/pkg/redshift/routes/routes_test.go index e32b7274..51cc0f34 100644 --- a/pkg/redshift/routes/routes_test.go +++ b/pkg/redshift/routes/routes_test.go @@ -18,13 +18,15 @@ var ds = &fake.RedshiftFakeDatasource{ {Name: "secret1", ARN: "arn:secret1"}, }, RSecret: models.RedshiftSecret{ClusterIdentifier: "clu", DBUser: "user"}, - RCluster: models.RedshiftCluster{ + RClusters: []models.RedshiftCluster{models.RedshiftCluster{ + ClusterIdentifier: "foo", Endpoint: models.RedshiftEndpoint{ Address: "foo.a.b.c", - Port: 123, + Port: 123, }, Database: "db-foo", }, + }, } func TestRoutes(t *testing.T) { @@ -47,10 +49,10 @@ func TestRoutes(t *testing.T) { expectedResult: `{"dbClusterIdentifier":"clu","username":"user"}`, }, { - description: "return cluster", - route: "cluster", + description: "return clusters", + route: "clusters", expectedCode: http.StatusOK, - expectedResult: `{"endpoint":{"address":"foo.a.b.c","port":123},"database":"db-foo"}`, + expectedResult: `[{"clusterIdentifier":"foo","endpoint":{"address":"foo.a.b.c","port":123},"database":"db-foo"}]`, }, } for _, tt := range tests { @@ -63,8 +65,8 @@ func TestRoutes(t *testing.T) { rh.secrets(rw, req) case "secret": rh.secret(rw, req) - case "cluster": - rh.cluster(rw, req) + case "clusters": + rh.clusters(rw, req) default: t.Fatalf("unexpected route %s", tt.route) } @@ -90,5 +92,5 @@ func Test_Routes(t *testing.T) { r := rh.Routes() assert.Contains(t, r, "/secrets") assert.Contains(t, r, "/secret") - assert.Contains(t, r, "/cluster") + assert.Contains(t, r, "/clusters") } From ef90cca0424798bad5758ca33b0b7f8e097bc84a Mon Sep 17 00:00:00 2001 From: yaellec Date: Fri, 4 Mar 2022 15:01:20 +0100 Subject: [PATCH 3/5] Added nil checks --- pkg/redshift/api/api.go | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/pkg/redshift/api/api.go b/pkg/redshift/api/api.go index 38e0f0fe..e957b81f 100644 --- a/pkg/redshift/api/api.go +++ b/pkg/redshift/api/api.go @@ -352,14 +352,16 @@ func (c *API) Clusters() ([]models.RedshiftCluster, error) { } res := []models.RedshiftCluster{} for _, r := range out.Clusters { - res = append(res, models.RedshiftCluster{ - ClusterIdentifier: *r.ClusterIdentifier, - Endpoint: models.RedshiftEndpoint{ - Address: *r.Endpoint.Address, - Port: *r.Endpoint.Port, - }, - Database: *r.DBName, - }) + if (r != nil && r.ClusterIdentifier != nil && r.Endpoint != nil && r.Endpoint.Address != nil && r.Endpoint.Port != nil && r.DBName != nil) { + res = append(res, models.RedshiftCluster{ + ClusterIdentifier: *r.ClusterIdentifier, + Endpoint: models.RedshiftEndpoint{ + Address: *r.Endpoint.Address, + Port: *r.Endpoint.Port, + }, + Database: *r.DBName, + }) + } } return res, nil } From 8da056fb50de912d26b26dcf3a688d7e4b25af6e Mon Sep 17 00:00:00 2001 From: yaellec Date: Fri, 4 Mar 2022 15:02:29 +0100 Subject: [PATCH 4/5] Removed clusterID in tests --- pkg/redshift/api/api_test.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/pkg/redshift/api/api_test.go b/pkg/redshift/api/api_test.go index 89473663..0560fd30 100644 --- a/pkg/redshift/api/api_test.go +++ b/pkg/redshift/api/api_test.go @@ -241,26 +241,22 @@ func Test_GetClusters(t *testing.T) { tests := []struct { c *API desc string - clusterId string errMsg string expectedClusters []models.RedshiftCluster }{ { c: c, desc: "Happy Path", - clusterId: "foo", expectedClusters: []models.RedshiftCluster{*expectedCluster1, *expectedCluster2}, }, { c: errC, desc: "Error with DescribeCluster", - clusterId: "foo", errMsg: "Boom!", }, { c: nilC, desc: "DescribeCluster returned nil", - clusterId: "foo", errMsg: "missing clusters content", }, } From 153e31112fe8dd0431905e4d400bfb78913401cc Mon Sep 17 00:00:00 2001 From: yaellec Date: Fri, 4 Mar 2022 15:09:46 +0100 Subject: [PATCH 5/5] Updated clusters to be GET method --- pkg/redshift/routes/routes.go | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/pkg/redshift/routes/routes.go b/pkg/redshift/routes/routes.go index 9c575f48..f462356b 100644 --- a/pkg/redshift/routes/routes.go +++ b/pkg/redshift/routes/routes.go @@ -34,13 +34,7 @@ func (r *RedshiftResourceHandler) secret(rw http.ResponseWriter, req *http.Reque } func (r *RedshiftResourceHandler) clusters(rw http.ResponseWriter, req *http.Request) { - reqBody, err := routes.ParseBody(req.Body) - if err != nil { - rw.WriteHeader(http.StatusBadRequest) - routes.Write(rw, []byte(err.Error())) - return - } - clusters, err := r.redshift.Clusters(req.Context(), reqBody) + clusters, err := r.redshift.Clusters(req.Context(), sqlds.Options{}) routes.SendResources(rw, clusters, err) }