From e508b13eacdca1b57fe247cdab805161f282ec76 Mon Sep 17 00:00:00 2001 From: Ben Tranter Date: Wed, 24 Jul 2024 16:21:49 -0400 Subject: [PATCH] databases: update connection pool (#1557) * db: add update pool command * db: fix cmd, add basic tests * db: fix conn pool cmds test * db: add integration tests --- commands/databases.go | 73 ++++- commands/databases_test.go | 50 ++++ do/databases.go | 6 + do/mocks/DatabasesService.go | 14 + integration/database_connection_pool_test.go | 300 +++++++++++++++++++ 5 files changed, 441 insertions(+), 2 deletions(-) create mode 100644 integration/database_connection_pool_test.go diff --git a/commands/databases.go b/commands/databases.go index 17ac6d5e5..5eba81dcf 100644 --- a/commands/databases.go +++ b/commands/databases.go @@ -1107,6 +1107,19 @@ We recommend starting with a pool size of about half your available connections "The name of the specific database within the database cluster", requiredOpt()) cmdDatabasePoolCreate.Example = `The following example creates a connection pool named ` + "`" + `example-pool` + "`" + ` for a database cluster with the ID ` + "`" + `ca9f591d-f38h-5555-a0ef-1c02d1d1e35` + "`" + `. The command uses the ` + "`" + `--size` + "`" + ` flag to set the pool size to 10 and sets the user to the database's default user: doctl databases pool create ca9f591d-f38h-5555-a0ef-1c02d1d1e35 example-pool --size 10` + cmdDatabasePoolUpdate := CmdBuilder(cmd, RunDatabasePoolUpdate, + "update ", "Update a connection pool for a database", `Updates the specified connection pool for the specified database cluster.`+getPoolDetails, Writer, + aliasOpt("u"), + ) + AddStringFlag(cmdDatabasePoolUpdate, doctl.ArgDatabasePoolMode, "", + "transaction", "The pool mode for the connection pool, such as `session`, `transaction`, and `statement`") + AddIntFlag(cmdDatabasePoolUpdate, doctl.ArgSizeSlug, "", 0, "pool size") + AddStringFlag(cmdDatabasePoolUpdate, doctl.ArgDatabasePoolDBName, "", "", + "The name of the specific database within the database cluster") + AddStringFlag(cmdDatabasePoolUpdate, doctl.ArgDatabasePoolUserName, "", "", + "The username for the database user") + cmdDatabasePoolUpdate.Example = `The following example updates a connection pool named ` + "`" + `example-pool` + "`" + ` for a database cluster with the ID ` + "`" + `ca9f591d-f38h-5555-a0ef-1c02d1d1e35` + "`" + `. The command uses the ` + "`" + `--size` + "`" + ` flag to set the pool size to 10 and sets the user to the database's default user: doctl databases pool update ca9f591d-f38h-5555-a0ef-1c02d1d1e35 example-pool --size 10` + cmdDatabasePoolDelete := CmdBuilder(cmd, RunDatabasePoolDelete, "delete ", "Delete a connection pool for a database", `Deletes the specified connection pool for the specified database cluster.`+getPoolDetails, Writer, aliasOpt("rm")) @@ -1202,6 +1215,62 @@ func buildDatabaseCreatePoolRequestFromArgs(c *CmdConfig) (*godo.DatabaseCreateP return req, nil } +// RunDatabasePoolUpdate updates a database pool. +func RunDatabasePoolUpdate(c *CmdConfig) error { + if len(c.Args) < 2 { + return doctl.NewMissingArgsErr(c.NS) + } + + databaseID := c.Args[0] + poolName := c.Args[1] + + pool, err := c.Databases().GetPool(databaseID, poolName) + if err != nil { + return err + } + + req := &godo.DatabaseUpdatePoolRequest{} + size, err := c.Doit.GetInt(c.NS, doctl.ArgDatabasePoolSize) + if err != nil { + return err + } + if size != 0 { + req.Size = size + } else { + req.Size = pool.Size + } + + db, err := c.Doit.GetString(c.NS, doctl.ArgDatabasePoolDBName) + if err != nil { + return err + } + if db != "" { + req.Database = db + } else { + req.Database = pool.Database + } + + mode, err := c.Doit.GetString(c.NS, doctl.ArgDatabasePoolMode) + if err != nil { + return err + } + if mode != "" { + req.Mode = mode + } else { + req.Mode = pool.Mode + } + + user, err := c.Doit.GetString(c.NS, doctl.ArgDatabasePoolUserName) + if err != nil { + return err + } + if user != "" { + req.User = user + } + + return c.Databases().UpdatePool(databaseID, poolName, req) +} + // RunDatabasePoolDelete deletes a database pool func RunDatabasePoolDelete(c *CmdConfig) error { if len(c.Args) < 2 { @@ -1992,7 +2061,7 @@ This command requires the ID of a database cluster, which you can retrieve by ca databaseFirewallRulesTxt := `A comma-separated list of firewall rules, in ` + "`" + `type:value` + "`" + ` format.` databaseFirewallUpdateDetails := ` -Replace the firewall rules for a specified database. This command requires the ` + "`" + `--rule` + "`" + ` flag. +Replace the firewall rules for a specified database. This command requires the ` + "`" + `--rule` + "`" + ` flag. You can configure multiple rules for the firewall by passing additional arguments in a comma-separated list with the ` + "`" + `--rule` + "`" + ` flag. Each rule passed using the ` + "`" + `--rule` + "`" + ` flag must be in a ` + "`" + `:` + "`" + ` format where: ` + "`" + `type` + "`" + ` is the type of resource that the firewall rule allows to access the database cluster. Possible values are: ` + "`" + `droplet` + "`" + `, ` + "`" + `k8s` + "`" + `, ` + "`" + `ip_addr` + "`" + `, ` + "`" + `tag` + "`" + `, ` + "`" + `app` + "`" + ` @@ -2001,7 +2070,7 @@ You can configure multiple rules for the firewall by passing additional argument databaseFirewallAddDetails := ` -Appends a single rule to the existing firewall rules of the specified database. +Appends a single rule to the existing firewall rules of the specified database. This command requires the ` + "`" + `--rule` + "`" + ` flag specifying the resource or resources allowed to access the database cluster. The rule passed to the ` + "`" + `--rule` + "`" + ` flag must be in a : format where: - ` + "`" + `type` + "`" + ` is the type of resource that the firewall rule allows to access the database cluster. Possible values are: ` + "`" + `droplet` + "`" + `, ` + "`" + `k8s", ` + "`" + `ip_addr` + "`" + `, ` + "`" + `tag` + "`" + `, ` + "`" + `app` + "`" + ` diff --git a/commands/databases_test.go b/commands/databases_test.go index 6c5ba073d..ebe39ffc9 100644 --- a/commands/databases_test.go +++ b/commands/databases_test.go @@ -293,6 +293,7 @@ func TestDatabasePoolCommand(t *testing.T) { "list", "get", "create", + "update", "delete", ) } @@ -1170,6 +1171,55 @@ func TestDatabasePoolCreate_InboundUser(t *testing.T) { }) } +func TestDatabasePoolUpdate(t *testing.T) { + pool := *(testDBPool.DatabasePool) + pool.Connection = nil + + r := &godo.DatabaseUpdatePoolRequest{ + User: pool.User, + Mode: pool.Mode, + Size: pool.Size, + Database: pool.Database, + } + + // Successful call + withTestClient(t, func(config *CmdConfig, tm *tcMocks) { + tm.databases.EXPECT().GetPool(testDBCluster.ID, pool.Name).Return(&do.DatabasePool{DatabasePool: &pool}, nil) + tm.databases.EXPECT().UpdatePool(testDBCluster.ID, pool.Name, r).Return(nil) + + config.Args = append(config.Args, testDBCluster.ID, testDBPool.Name) + config.Doit.Set(config.NS, doctl.ArgDatabasePoolMode, testDBPool.Mode) + config.Doit.Set(config.NS, doctl.ArgDatabasePoolSize, testDBPool.Size) + config.Doit.Set(config.NS, doctl.ArgDatabasePoolDBName, testDBPool.Database) + config.Doit.Set(config.NS, doctl.ArgDatabasePoolUserName, testDBPool.User) + + err := RunDatabasePoolUpdate(config) + assert.NoError(t, err) + }) + + // Error + withTestClient(t, func(config *CmdConfig, tm *tcMocks) { + tm.databases.EXPECT().GetPool(testDBCluster.ID, pool.Name).Return(&do.DatabasePool{ + DatabasePool: &godo.DatabasePool{ + User: pool.User, + Name: pool.Name, + Mode: pool.Mode, + Size: pool.Size, + Database: pool.Database, + }, + }, nil) + tm.databases.EXPECT().UpdatePool( + testDBCluster.ID, + pool.Name, + gomock.AssignableToTypeOf(&godo.DatabaseUpdatePoolRequest{}), + ).Return(errTest) + + config.Args = append(config.Args, testDBCluster.ID, testDBPool.Name) + err := RunDatabasePoolUpdate(config) + assert.EqualError(t, err, "error") + }) +} + func TestDatabasesPoolDelete(t *testing.T) { // Successful withTestClient(t, func(config *CmdConfig, tm *tcMocks) { diff --git a/do/databases.go b/do/databases.go index bb865cae3..cf5f9ae41 100644 --- a/do/databases.go +++ b/do/databases.go @@ -161,6 +161,7 @@ type DatabasesService interface { ListPools(string) (DatabasePools, error) CreatePool(string, *godo.DatabaseCreatePoolRequest) (*DatabasePool, error) GetPool(string, string) (*DatabasePool, error) + UpdatePool(string, string, *godo.DatabaseUpdatePoolRequest) error DeletePool(string, string) error GetReplica(string, string) (*DatabaseReplica, error) @@ -492,6 +493,11 @@ func (ds *databasesService) GetPool(databaseID, poolName string) (*DatabasePool, return &DatabasePool{DatabasePool: p}, nil } +func (ds *databasesService) UpdatePool(databaseID, poolName string, req *godo.DatabaseUpdatePoolRequest) error { + _, err := ds.client.Databases.UpdatePool(context.TODO(), databaseID, poolName, req) + return err +} + func (ds *databasesService) DeletePool(databaseID, poolName string) error { _, err := ds.client.Databases.DeletePool(context.TODO(), databaseID, poolName) diff --git a/do/mocks/DatabasesService.go b/do/mocks/DatabasesService.go index a85ebf4d5..6ebecf288 100644 --- a/do/mocks/DatabasesService.go +++ b/do/mocks/DatabasesService.go @@ -677,6 +677,20 @@ func (mr *MockDatabasesServiceMockRecorder) UpdateMySQLConfiguration(databaseID, return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdateMySQLConfiguration", reflect.TypeOf((*MockDatabasesService)(nil).UpdateMySQLConfiguration), databaseID, confString) } +// UpdatePool mocks base method. +func (m *MockDatabasesService) UpdatePool(arg0, arg1 string, arg2 *godo.DatabaseUpdatePoolRequest) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "UpdatePool", arg0, arg1, arg2) + ret0, _ := ret[0].(error) + return ret0 +} + +// UpdatePool indicates an expected call of UpdatePool. +func (mr *MockDatabasesServiceMockRecorder) UpdatePool(arg0, arg1, arg2 any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdatePool", reflect.TypeOf((*MockDatabasesService)(nil).UpdatePool), arg0, arg1, arg2) +} + // UpdatePostgreSQLConfiguration mocks base method. func (m *MockDatabasesService) UpdatePostgreSQLConfiguration(databaseID, confString string) error { m.ctrl.T.Helper() diff --git a/integration/database_connection_pool_test.go b/integration/database_connection_pool_test.go new file mode 100644 index 000000000..a5e500317 --- /dev/null +++ b/integration/database_connection_pool_test.go @@ -0,0 +1,300 @@ +package integration + +import ( + "bytes" + "encoding/json" + "fmt" + "html/template" + "io" + "net/http" + "net/http/httptest" + "net/http/httputil" + "os/exec" + "strings" + "testing" + + "github.com/sclevine/spec" + "github.com/stretchr/testify/require" +) + +var _ = suite("database/pool/list", func(t *testing.T, when spec.G, it spec.S) { + var ( + expect *require.Assertions + server *httptest.Server + ) + + type poolReq struct { + Name string `json:"name"` + Mode string `json:"mode"` + Size int `json:"size"` + DB string `json:"db"` + User string `json:"user"` + } + + render := func(pr *poolReq) string { + t, err := template.New("response").Parse(databaseConnectionPoolTpl) + expect.NoError(err) + + buf := &bytes.Buffer{} + err = t.Execute(buf, pr) + expect.NoError(err) + return buf.String() + } + + it.Before(func() { + expect = require.New(t) + + server = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { + switch req.URL.Path { + case "/v2/databases/some-database-id/pools": + auth := req.Header.Get("Authorization") + if auth != "Bearer some-magic-token" { + w.WriteHeader(http.StatusUnauthorized) + } + + switch req.Method { + case http.MethodGet: + w.Header().Set("Content-Type", "application/json") + w.Write([]byte(databaseConnectionPoolList)) + + case http.MethodPost: + reqBody, err := io.ReadAll(req.Body) + expect.NoError(err) + + request := &poolReq{} + + err = json.Unmarshal(reqBody, request) + expect.NoError(err) + + w.Header().Set("Content-Type", "application/json") + w.Write([]byte(render(request))) + + default: + w.WriteHeader(http.StatusMethodNotAllowed) + return + } + + case "/v2/databases/some-database-id/pools/reporting-pool": + auth := req.Header.Get("Authorization") + if auth != "Bearer some-magic-token" { + w.WriteHeader(http.StatusUnauthorized) + } + + switch req.Method { + case http.MethodGet: + w.Header().Set("Content-Type", "application/json") + w.Write([]byte(databaseConnectionPoolGet)) + + case http.MethodPut: + reqBody, err := io.ReadAll(req.Body) + expect.NoError(err) + + request := &poolReq{} + + err = json.Unmarshal(reqBody, request) + expect.NoError(err) + + w.Header().Set("Content-Type", "application/json") + w.Write([]byte(render(request))) + + case http.MethodDelete: + + default: + w.WriteHeader(http.StatusMethodNotAllowed) + return + } + + default: + dump, err := httputil.DumpRequest(req, true) + if err != nil { + t.Fatal("failed to dump request") + } + + t.Fatalf("received unknown request: %s", dump) + } + })) + }) + + when("command is list", func() { + it("lists the database pools", func() { + cmd := exec.Command(builtBinaryPath, + "-t", "some-magic-token", + "-u", server.URL, + "databases", + "pool", + "list", + "some-database-id", + ) + + output, err := cmd.CombinedOutput() + expect.NoError(err, fmt.Sprintf("received error output: %s", output)) + expect.Equal(strings.TrimSpace(databaseConnectionPoolListOutput), strings.TrimSpace(string(output))) + }) + }) + + when("command is get", func() { + it("gets a database pool", func() { + cmd := exec.Command(builtBinaryPath, + "-t", "some-magic-token", + "-u", server.URL, + "databases", + "pool", + "get", + "some-database-id", + "reporting-pool", + ) + + output, err := cmd.CombinedOutput() + expect.NoError(err, fmt.Sprintf("received error output: %s", output)) + expect.Equal(strings.TrimSpace(databaseConnectionPoolGetOutput), strings.TrimSpace(string(output))) + }) + }) + + when("command is create", func() { + it("creates a database pool", func() { + cmd := exec.Command(builtBinaryPath, + "-t", "some-magic-token", + "-u", server.URL, + "databases", + "pool", + "create", + "some-database-id", + "reporting-pool", + "--user", "doadmin", + "--size", "10", + "--db", "defaultdb", + "--mode", "session", + ) + + output, err := cmd.CombinedOutput() + expect.NoError(err, fmt.Sprintf("received error output: %s", output)) + expect.Equal(strings.TrimSpace(databaseConnectionPoolGetOutput), strings.TrimSpace(string(output))) + }) + }) + + when("command is update", func() { + it("updates a database pool", func() { + cmd := exec.Command(builtBinaryPath, + "-t", "some-magic-token", + "-u", server.URL, + "databases", + "pool", + "update", + "some-database-id", + "reporting-pool", + "--size", "20", + ) + + output, err := cmd.CombinedOutput() + expect.NoError(err, fmt.Sprintf("received error output: %s", output)) + }) + }) + + when("command is delete", func() { + it("deletes a database pool", func() { + cmd := exec.Command(builtBinaryPath, + "-t", "some-magic-token", + "-u", server.URL, + "databases", + "pool", + "delete", + "some-database-id", + "reporting-pool", + "--force", + ) + + output, err := cmd.CombinedOutput() + expect.NoError(err, fmt.Sprintf("received error output: %s", output)) + }) + }) +}) + +const ( + databaseConnectionPoolListOutput = ` +User Name Size Database Mode URI +doadmin reporting-pool 10 defaultdb session postgres://doadmin:wv78n3zpz42xezdk@backend-do-user-19081923-0.db.ondigitalocean.com:25061/foo?sslmode=require +doadmin backend-pool 10 defaultdb transaction postgres://doadmin:wv78n3zpz42xezdk@backend-do-user-19081923-0.db.ondigitalocean.com:25061/backend-pool?sslmode=require +` + + databaseConnectionPoolGetOutput = ` +User Name Size Database Mode URI +doadmin reporting-pool 10 defaultdb session postgres://doadmin:wv78n3zpz42xezdk@backend-do-user-19081923-0.db.ondigitalocean.com:25061/foo?sslmode=require +` + + databaseConnectionPoolList = `{ + "pools":[ + { + "user":"doadmin", + "name":"reporting-pool", + "size":10, + "db":"defaultdb", + "mode":"session", + "connection":{ + "uri":"postgres://doadmin:wv78n3zpz42xezdk@backend-do-user-19081923-0.db.ondigitalocean.com:25061/foo?sslmode=require", + "database":"foo", + "host":"backend-do-user-19081923-0.db.ondigitalocean.com", + "port":25061, + "user":"doadmin", + "password":"wv78n3zpz42xezdk", + "ssl":true + } + }, + { + "user":"doadmin", + "name":"backend-pool", + "size":10, + "db":"defaultdb", + "mode":"transaction", + "connection":{ + "uri":"postgres://doadmin:wv78n3zpz42xezdk@backend-do-user-19081923-0.db.ondigitalocean.com:25061/backend-pool?sslmode=require", + "database":"backend-pool", + "host":"backend-do-user-19081923-0.db.ondigitalocean.com", + "port":25061, + "user":"doadmin", + "password":"wv78n3zpz42xezdk", + "ssl":true + } + } + ] +} +` + + databaseConnectionPoolGet = `{ + "pool":{ + "user":"doadmin", + "name":"reporting-pool", + "size":10, + "db":"defaultdb", + "mode":"session", + "connection":{ + "uri":"postgres://doadmin:wv78n3zpz42xezdk@backend-do-user-19081923-0.db.ondigitalocean.com:25061/foo?sslmode=require", + "database":"foo", + "host":"backend-do-user-19081923-0.db.ondigitalocean.com", + "port":25061, + "user":"doadmin", + "password":"wv78n3zpz42xezdk", + "ssl":true + } + } +} +` + + databaseConnectionPoolTpl = `{ + "pool":{ + "user": "{{ .User }}", + "name": "{{ .Name }}", + "size": {{ .Size }}, + "db": "{{ .DB }}", + "mode": "{{ .Mode }}", + "connection":{ + "uri":"postgres://doadmin:wv78n3zpz42xezdk@backend-do-user-19081923-0.db.ondigitalocean.com:25061/foo?sslmode=require", + "database":"foo", + "host":"backend-do-user-19081923-0.db.ondigitalocean.com", + "port":25061, + "user":"doadmin", + "password":"wv78n3zpz42xezdk", + "ssl":true + } + } +}` +)