From a06b14cff5b5ff664bba938359e2a695bedc41cf Mon Sep 17 00:00:00 2001 From: Billy Date: Thu, 29 Jul 2021 14:38:27 +0000 Subject: [PATCH] merge write groups into read groups (#13) --- internal/api/v1/credentials.go | 46 +++------ internal/api/v1/credentials_test.go | 37 +++----- internal/api/v1/payload_test.go | 25 +++++ internal/sql/client.go | 47 +++++++--- internal/sql/client_test.go | 140 +++++++++++++++------------- 5 files changed, 160 insertions(+), 135 deletions(-) diff --git a/internal/api/v1/credentials.go b/internal/api/v1/credentials.go index dcac275..c99c1eb 100644 --- a/internal/api/v1/credentials.go +++ b/internal/api/v1/credentials.go @@ -6,7 +6,6 @@ import ( "strings" "github.com/gin-gonic/gin" - "github.com/google/uuid" "github.com/jinzhu/gorm" "github.com/homedepot/cloud-runner/internal/fiat" "github.com/homedepot/cloud-runner/internal/sql" @@ -43,6 +42,13 @@ func (cc *Controller) CreateCredentials(c *gin.Context) { } } + // Make sure credentials read groups contain all write groups. + for _, wg := range creds.WriteGroups { + if !contains(creds.ReadGroups, wg) { + creds.ReadGroups = append(creds.ReadGroups, wg) + } + } + err = cc.SqlClient.CreateCredentials(creds) if err != nil { c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()}) @@ -50,40 +56,18 @@ func (cc *Controller) CreateCredentials(c *gin.Context) { return } - // TODO make these calls part of the sql client to create the credentials, - // additionally they should be part of a transaction that will be rolled - // back if any fail. - for _, group := range creds.ReadGroups { - rp := cloudrunner.CredentialsReadPermission{ - ID: uuid.New().String(), - Account: creds.Account, - ReadGroup: group, - } - - err = cc.SqlClient.CreateReadPermission(rp) - if err != nil { - c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()}) - - return - } - } - - for _, group := range creds.WriteGroups { - wp := cloudrunner.CredentialsWritePermission{ - ID: uuid.New().String(), - Account: creds.Account, - WriteGroup: group, - } - - err = cc.SqlClient.CreateWritePermission(wp) - if err != nil { - c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()}) + c.JSON(http.StatusCreated, creds) +} - return +// contains returns true if slice s contains element e (case insensitive). +func contains(s []string, e string) bool { + for _, a := range s { + if strings.EqualFold(a, e) { + return true } } - c.JSON(http.StatusCreated, creds) + return false } // DeleteCredentials deletes credentials from the DB by account name. diff --git a/internal/api/v1/credentials_test.go b/internal/api/v1/credentials_test.go index 15de56c..b7fab36 100644 --- a/internal/api/v1/credentials_test.go +++ b/internal/api/v1/credentials_test.go @@ -5,12 +5,12 @@ import ( "errors" "net/http" - "github.com/homedepot/cloud-runner/internal/fiat" - "github.com/homedepot/cloud-runner/internal/sql" - cloudrunner "github.com/homedepot/cloud-runner/pkg" "github.com/jinzhu/gorm" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" + "github.com/homedepot/cloud-runner/internal/fiat" + "github.com/homedepot/cloud-runner/internal/sql" + cloudrunner "github.com/homedepot/cloud-runner/pkg" ) var _ = Describe("Credentials", func() { @@ -91,38 +91,29 @@ var _ = Describe("Credentials", func() { }) }) - When("creating a read group returns an error", func() { - BeforeEach(func() { - fakeSQLClient.CreateReadPermissionReturns(errors.New("error creating read permission")) - }) - - It("returns status internal server error", func() { - Expect(res.StatusCode).To(Equal(http.StatusInternalServerError)) - validateResponse(payloadErrorCreatingReadPermission) - }) - }) - - When("creating a write group returns an error", func() { + When("the account is not defined", func() { BeforeEach(func() { - fakeSQLClient.CreateWritePermissionReturns(errors.New("error creating write permission")) + body = &bytes.Buffer{} + body.Write([]byte(`{"projectID": "test-project-id"}`)) + createRequest(http.MethodPost) }) - It("returns status internal server error", func() { - Expect(res.StatusCode).To(Equal(http.StatusInternalServerError)) - validateResponse(payloadErrorCreatingWritePermission) + It("generates the account name", func() { + Expect(res.StatusCode).To(Equal(http.StatusCreated)) + validateResponse(payloadCredentialsCreatedNoAccountProvided) }) }) - When("the account is not defined", func() { + When("the request contains write groups that are not present in read groups", func() { BeforeEach(func() { body = &bytes.Buffer{} - body.Write([]byte(`{"projectID": "test-project-id"}`)) + body.Write([]byte(payloadRequestCredentialsMismatchedGroups)) createRequest(http.MethodPost) }) - It("generates the account name", func() { + It("merges the write groups into the read groups", func() { Expect(res.StatusCode).To(Equal(http.StatusCreated)) - validateResponse(payloadCredentialsCreatedNoAccountProvided) + validateResponse(payloadCredentialsCreatedMergedGroups) }) }) diff --git a/internal/api/v1/payload_test.go b/internal/api/v1/payload_test.go index b0043b2..f5750a2 100644 --- a/internal/api/v1/payload_test.go +++ b/internal/api/v1/payload_test.go @@ -15,6 +15,18 @@ const payloadRequestCredentials = `{ ] }` +const payloadRequestCredentialsMismatchedGroups = `{ + "account": "test-account", + "projectID": "test-project-id", + "readGroups": [ + "gg_test1" + ], + "writeGroups": [ + "gg_test1", + "gg_test2" + ] + }` + const payloadRequestDeployment = `{ "account": "cr-test-project-id", "allowUnauthenticated": false, @@ -74,6 +86,19 @@ const payloadCredentialsCreatedNoAccountProvided = `{ "projectID": "test-project-id" }` +const payloadCredentialsCreatedMergedGroups = `{ + "account": "test-account", + "projectID": "test-project-id", + "readGroups": [ + "gg_test1", + "gg_test2" + ], + "writeGroups": [ + "gg_test1", + "gg_test2" + ] + }` + const payloadCredentialsCreated = `{ "account": "test-account", "projectID": "test-project-id", diff --git a/internal/sql/client.go b/internal/sql/client.go index 5653383..6367443 100644 --- a/internal/sql/client.go +++ b/internal/sql/client.go @@ -7,6 +7,7 @@ import ( "sort" "time" + "github.com/google/uuid" "github.com/jinzhu/gorm" cloudrunner "github.com/homedepot/cloud-runner/pkg" @@ -35,8 +36,6 @@ type Client interface { Connection() (string, string) CreateCredentials(cloudrunner.Credentials) error CreateDeployment(cloudrunner.Deployment) error - CreateReadPermission(cloudrunner.CredentialsReadPermission) error - CreateWritePermission(cloudrunner.CredentialsWritePermission) error DB() *gorm.DB DeleteCredentials(string) error GetCredentials(string) (cloudrunner.Credentials, error) @@ -100,9 +99,39 @@ func (c *client) Connection() (string, string) { c.user, c.pass, c.host, c.name) } -// CreateCredentials inserts a new set of credentials into the DB. +// CreateCredentials inserts a new set of credentials into the DB along with +// its associated read/write groups. It uses a transaction, so that if any +// operation fails the operations are rolled back. func (c *client) CreateCredentials(credentials cloudrunner.Credentials) error { - return c.db.Create(&credentials).Error + return c.db.Transaction(func(tx *gorm.DB) error { + for _, group := range credentials.ReadGroups { + r := cloudrunner.CredentialsReadPermission{ + ID: uuid.New().String(), + Account: credentials.Account, + ReadGroup: group, + } + + err := tx.Create(&r).Error + if err != nil { + return err + } + } + + for _, group := range credentials.WriteGroups { + w := cloudrunner.CredentialsWritePermission{ + ID: uuid.New().String(), + Account: credentials.Account, + WriteGroup: group, + } + + err := tx.Create(&w).Error + if err != nil { + return err + } + } + + return tx.Create(&credentials).Error + }) } // CreateDeployment inserts a new deployment into the DB. @@ -110,16 +139,6 @@ func (c *client) CreateDeployment(d cloudrunner.Deployment) error { return c.db.Create(&d).Error } -// CreateReadPermission inserts a read permission into the database. -func (c *client) CreateReadPermission(r cloudrunner.CredentialsReadPermission) error { - return c.db.Create(&r).Error -} - -// CreateWritePermission inserts a write permission into the database. -func (c *client) CreateWritePermission(w cloudrunner.CredentialsWritePermission) error { - return c.db.Create(&w).Error -} - // DB returns the underlying db. func (c *client) DB() *gorm.DB { return c.db diff --git a/internal/sql/client_test.go b/internal/sql/client_test.go index 39e742e..8fb4bd4 100644 --- a/internal/sql/client_test.go +++ b/internal/sql/client_test.go @@ -83,7 +83,15 @@ var _ = Describe("Sql", func() { BeforeEach(func() { credentials = cloudrunner.Credentials{ - Account: "test-account", + Account: "test-account", + ProjectID: "test-project-id", + ReadGroups: []string{ + "group1", + "group2", + }, + WriteGroups: []string{ + "group1", + }, } }) @@ -91,77 +99,53 @@ var _ = Describe("Sql", func() { err = c.CreateCredentials(credentials) }) - When("it creates the credentials", func() { + When("inserting the read permission returns an error", func() { BeforeEach(func() { mock.ExpectBegin() - mock.ExpectExec(`(?i)^INSERT INTO "credentials" \(` + + mock.ExpectExec(`(?i)^INSERT INTO "credentials_read_permissions" \(` + `"account",` + - `"project_id"` + - `\) VALUES \(\?,\?\)$`). - WillReturnResult(sqlmock.NewResult(1, 1)) - mock.ExpectCommit() + `"id",` + + `"read_group"` + + `\) VALUES \(\?,\?,\?\)$`). + WillReturnError(errors.New("error inserting read permission")) }) - It("succeeds", func() { - Expect(err).To(BeNil()) + It("returns an error", func() { + Expect(err).ToNot(BeNil()) + Expect(err.Error()).To(Equal("error inserting read permission")) }) }) - }) - - Describe("#CreateDeployment", func() { - var deployment cloudrunner.Deployment - BeforeEach(func() { - t := internal.CurrentTimeUTC() - deployment = cloudrunner.Deployment{ - EndTime: &t, - ID: "test-id", - StartTime: &t, - Status: "RUNNING", - Output: "test-output", - } - }) - - JustBeforeEach(func() { - err = c.CreateDeployment(deployment) - }) - - When("it creates the deployment", func() { + When("inserting the write permission returns an error", func() { BeforeEach(func() { mock.ExpectBegin() - mock.ExpectExec(`(?i)^INSERT INTO "deployments" \(` + - `"end_time",` + + mock.ExpectExec(`(?i)^INSERT INTO "credentials_read_permissions" \(` + + `"account",` + `"id",` + - `"start_time",` + - `"status",` + - `"output"` + - `\) VALUES \(\?,\?,\?,\?,\?\)$`). + `"read_group"` + + `\) VALUES \(\?,\?,\?\)$`). WillReturnResult(sqlmock.NewResult(1, 1)) - mock.ExpectCommit() + mock.ExpectExec(`(?i)^INSERT INTO "credentials_read_permissions" \(` + + `"account",` + + `"id",` + + `"read_group"` + + `\) VALUES \(\?,\?,\?\)$`). + WillReturnResult(sqlmock.NewResult(1, 1)) + mock.ExpectExec(`(?i)^INSERT INTO "credentials_write_permissions" \(` + + `"account",` + + `"id",` + + `"write_group"` + + `\) VALUES \(\?,\?,\?\)$`). + WillReturnError(errors.New("error inserting write permission")) }) - It("succeeds", func() { - Expect(err).To(BeNil()) + It("returns an error", func() { + Expect(err).ToNot(BeNil()) + Expect(err.Error()).To(Equal("error inserting write permission")) }) }) - }) - - Describe("#CreateReadPermission", func() { - var rp cloudrunner.CredentialsReadPermission - - BeforeEach(func() { - rp = cloudrunner.CredentialsReadPermission{ - ID: "test-id", - Account: "test-account-name", - ReadGroup: "test-write-group", - } - }) - JustBeforeEach(func() { - err = c.CreateReadPermission(rp) - }) - - When("it creates the read permissions", func() { + When("it creates the credentials", func() { BeforeEach(func() { mock.ExpectBegin() mock.ExpectExec(`(?i)^INSERT INTO "credentials_read_permissions" \(` + @@ -170,6 +154,23 @@ var _ = Describe("Sql", func() { `"read_group"` + `\) VALUES \(\?,\?,\?\)$`). WillReturnResult(sqlmock.NewResult(1, 1)) + mock.ExpectExec(`(?i)^INSERT INTO "credentials_read_permissions" \(` + + `"account",` + + `"id",` + + `"read_group"` + + `\) VALUES \(\?,\?,\?\)$`). + WillReturnResult(sqlmock.NewResult(1, 1)) + mock.ExpectExec(`(?i)^INSERT INTO "credentials_write_permissions" \(` + + `"account",` + + `"id",` + + `"write_group"` + + `\) VALUES \(\?,\?,\?\)$`). + WillReturnResult(sqlmock.NewResult(1, 1)) + mock.ExpectExec(`(?i)^INSERT INTO "credentials" \(` + + `"account",` + + `"project_id"` + + `\) VALUES \(\?,\?\)$`). + WillReturnResult(sqlmock.NewResult(1, 1)) mock.ExpectCommit() }) @@ -179,29 +180,34 @@ var _ = Describe("Sql", func() { }) }) - Describe("#CreateWritePermission", func() { - var wp cloudrunner.CredentialsWritePermission + Describe("#CreateDeployment", func() { + var deployment cloudrunner.Deployment BeforeEach(func() { - wp = cloudrunner.CredentialsWritePermission{ - ID: "test-id", - Account: "test-account-name", - WriteGroup: "test-write-group", + t := internal.CurrentTimeUTC() + deployment = cloudrunner.Deployment{ + EndTime: &t, + ID: "test-id", + StartTime: &t, + Status: "RUNNING", + Output: "test-output", } }) JustBeforeEach(func() { - err = c.CreateWritePermission(wp) + err = c.CreateDeployment(deployment) }) - When("it creates the write permissions", func() { + When("it creates the deployment", func() { BeforeEach(func() { mock.ExpectBegin() - mock.ExpectExec(`(?i)^INSERT INTO "credentials_write_permissions" \(` + - `"account",` + + mock.ExpectExec(`(?i)^INSERT INTO "deployments" \(` + + `"end_time",` + `"id",` + - `"write_group"` + - `\) VALUES \(\?,\?,\?\)$`). + `"start_time",` + + `"status",` + + `"output"` + + `\) VALUES \(\?,\?,\?,\?,\?\)$`). WillReturnResult(sqlmock.NewResult(1, 1)) mock.ExpectCommit() })