From 307445f9d8254c5d7f73fb775d697037713e5e47 Mon Sep 17 00:00:00 2001 From: Jim Lambert Date: Wed, 12 Aug 2020 17:59:11 -0400 Subject: [PATCH] add update external kms config --- internal/kms/repository.go | 64 +++++++ internal/kms/repository_test.go | 284 +++++++++++++++++++++++++++++++- 2 files changed, 347 insertions(+), 1 deletion(-) diff --git a/internal/kms/repository.go b/internal/kms/repository.go index d0d8f6ec21..2831a74d3a 100644 --- a/internal/kms/repository.go +++ b/internal/kms/repository.go @@ -4,8 +4,10 @@ import ( "context" "errors" "fmt" + "strings" "github.com/hashicorp/boundary/internal/db" + dbcommon "github.com/hashicorp/boundary/internal/db/common" "github.com/hashicorp/boundary/internal/oplog" wrapping "github.com/hashicorp/go-kms-wrapping" ) @@ -137,3 +139,65 @@ func (r *Repository) DeleteExternalConfig(ctx context.Context, privateId string, } return rowsDeleted, nil } + +// UpdateExternalConfig will update an external config in the repository and +// return the written config. fieldMaskPaths provides field_mask.proto paths for +// fields that should be updated. Fields will be set to NULL if the field is a +// zero value and included in fieldMask. Config is the only updatable field, +// If no updatable fields are included in the fieldMaskPaths, then an error is +// returned. +func (r *Repository) UpdateExternalConfig(ctx context.Context, conf *ExternalConfig, version uint32, fieldMaskPaths []string, opt ...Option) (*ExternalConfig, int, error) { + if conf == nil { + return nil, db.NoRowsAffected, fmt.Errorf("update external config: missing conf %w", db.ErrNilParameter) + } + if conf.PrivateId == "" { + return nil, db.NoRowsAffected, fmt.Errorf("update external config: missing conf private id %w", db.ErrInvalidParameter) + } + for _, f := range fieldMaskPaths { + switch { + case strings.EqualFold("config", f): + default: + return nil, db.NoRowsAffected, fmt.Errorf("update external config: field: %s: %w", f, db.ErrInvalidFieldMask) + } + } + var dbMask, nullFields []string + dbMask, nullFields = dbcommon.BuildUpdatePaths( + map[string]interface{}{ + "config": conf.Config, + }, + fieldMaskPaths, + ) + if len(dbMask) == 0 && len(nullFields) == 0 { + return nil, db.NoRowsAffected, fmt.Errorf("update external config: %w", db.ErrEmptyFieldMask) + } + + var rowsUpdated int + var returnedCfg interface{} + _, err := r.writer.DoTx( + ctx, + db.StdRetryCnt, + db.ExpBackoff{}, + func(_ db.Reader, w db.Writer) error { + metadata := conf.oplog(oplog.OpType_OP_TYPE_UPDATE) + returnedCfg = conf.Clone() + var err error + rowsUpdated, err = w.Update( + ctx, + returnedCfg, + dbMask, + nullFields, + db.WithOplog(r.wrapper, metadata), + ) + if err == nil && rowsUpdated > 1 { + // return err, which will result in a rollback of the update + return errors.New("error more than 1 resource would have been updated ") + } + return err + }, + ) + if err != nil { + return nil, db.NoRowsAffected, fmt.Errorf("update external config: %w for %s", err, conf.PrivateId) + } + + return returnedCfg.(*ExternalConfig), rowsUpdated, err +} diff --git a/internal/kms/repository_test.go b/internal/kms/repository_test.go index 11fb9a855e..0f21a48db9 100644 --- a/internal/kms/repository_test.go +++ b/internal/kms/repository_test.go @@ -318,7 +318,6 @@ func TestRepository_DeleteExternalConfig(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { assert, require := assert.New(t), require.New(t) - conn.LogMode(true) deletedRows, err := repo.DeleteExternalConfig(context.Background(), tt.args.conf.PrivateId, tt.args.opt...) if tt.wantErr { require.Error(err) @@ -343,3 +342,286 @@ func TestRepository_DeleteExternalConfig(t *testing.T) { }) } } + +func TestRepository_UpdateExternalConfig(t *testing.T) { + t.Parallel() + conn, _ := db.TestSetup(t, "postgres") + a := assert.New(t) + rw := db.New(conn) + wrapper := db.TestWrapper(t) + repo, err := NewRepository(rw, rw, wrapper) + a.NoError(err) + + org, _ := iam.TestScopes(t, conn) + privId := func(s string) *string { return &s } + + type args struct { + config *ExternalConfig + fieldMaskPaths []string + opt []Option + PrivateId *string + version uint32 + } + tests := []struct { + name string + newScopeId string + newGrpOpts []Option + args args + wantRowsUpdate int + wantErr bool + wantIsError error + }{ + { + name: "valid", + args: args{ + fieldMaskPaths: []string{"Config"}, + config: func() *ExternalConfig { + c := allocExternalConfig() + c.Config = `{"alice":"bob"}` + return &c + }(), + version: uint32(1), + }, + newScopeId: org.PublicId, + wantErr: false, + wantRowsUpdate: 1, + }, + { + name: "not-found", + args: args{ + fieldMaskPaths: []string{"Config"}, + config: func() *ExternalConfig { + c := allocExternalConfig() + c.Config = `{"alice":"bob"}` + return &c + }(), + version: uint32(1), + PrivateId: func() *string { s := "1"; return &s }(), + }, + newScopeId: org.PublicId, + wantErr: true, + wantRowsUpdate: 0, + wantIsError: db.ErrRecordNotFound, + }, + // { + // name: "null-name", + // args: args{ + // name: "", + // fieldMaskPaths: []string{"Name"}, + // ScopeId: org.PublicId, + // }, + // newScopeId: org.PublicId, + // newGrpOpts: []Option{WithName("null-name" + id)}, + // wantErr: false, + // wantRowsUpdate: 1, + // }, + // { + // name: "null-description", + // args: args{ + // name: "", + // fieldMaskPaths: []string{"Description"}, + // ScopeId: org.PublicId, + // }, + // newScopeId: org.PublicId, + // newGrpOpts: []Option{WithDescription("null-description" + id)}, + // wantErr: false, + // wantRowsUpdate: 1, + // }, + { + name: "empty-field-mask", + args: args{ + fieldMaskPaths: []string{}, + config: func() *ExternalConfig { + c := allocExternalConfig() + c.Config = `{"alice":"bob"}` + return &c + }(), + version: uint32(1), + }, + newScopeId: org.PublicId, + wantErr: true, + wantRowsUpdate: 0, + wantIsError: db.ErrEmptyFieldMask, + }, + { + name: "nil-fieldmask", + args: args{ + fieldMaskPaths: nil, + config: func() *ExternalConfig { + c := allocExternalConfig() + c.Config = `{"alice":"bob"}` + return &c + }(), + version: uint32(1), + }, + newScopeId: org.PublicId, + wantErr: true, + wantRowsUpdate: 0, + wantIsError: db.ErrEmptyFieldMask, + }, + { + name: "read-only-create-time", + args: args{ + fieldMaskPaths: []string{"CreateTime"}, + config: func() *ExternalConfig { + c := allocExternalConfig() + c.Config = `{"alice":"bob"}` + return &c + }(), + version: uint32(1), + }, + newScopeId: org.PublicId, + wantErr: true, + wantRowsUpdate: 0, + wantIsError: db.ErrInvalidFieldMask, + }, + { + name: "read-only-update-time", + args: args{ + fieldMaskPaths: []string{"UpdateTime"}, + config: func() *ExternalConfig { + c := allocExternalConfig() + c.Config = `{"alice":"bob"}` + return &c + }(), + version: uint32(1), + }, + newScopeId: org.PublicId, + wantErr: true, + wantRowsUpdate: 0, + wantIsError: db.ErrInvalidFieldMask, + }, + { + name: "read-only-privateId", + args: args{ + fieldMaskPaths: []string{"PrivateId"}, + config: func() *ExternalConfig { + c := allocExternalConfig() + c.PrivateId = org.PublicId + return &c + }(), + version: uint32(1), + }, + newScopeId: org.PublicId, + wantErr: true, + wantRowsUpdate: 0, + wantIsError: db.ErrInvalidFieldMask, + }, + { + name: "read-only-scopeId", + args: args{ + fieldMaskPaths: []string{"ScopeId"}, + config: func() *ExternalConfig { + c := allocExternalConfig() + c.ScopeId = org.PublicId + return &c + }(), + version: uint32(1), + }, + newScopeId: org.PublicId, + wantErr: true, + wantRowsUpdate: 0, + wantIsError: db.ErrInvalidFieldMask, + }, + { + name: "read-only-type", + args: args{ + fieldMaskPaths: []string{"type"}, + config: func() *ExternalConfig { + c := allocExternalConfig() + c.Type = AliCloudKms.String() + return &c + }(), + version: uint32(1), + }, + newScopeId: org.PublicId, + wantErr: true, + wantRowsUpdate: 0, + wantIsError: db.ErrInvalidFieldMask, + }, + { + name: "read-only-version", + args: args{ + fieldMaskPaths: []string{"version"}, + config: func() *ExternalConfig { + c := allocExternalConfig() + c.Version = 100 + return &c + }(), + version: uint32(1), + }, + newScopeId: org.PublicId, + wantErr: true, + wantRowsUpdate: 0, + wantIsError: db.ErrInvalidFieldMask, + }, + { + name: "unknown-fields", + args: args{ + fieldMaskPaths: []string{"Alice"}, + config: func() *ExternalConfig { + c := allocExternalConfig() + c.Config = `{"alice":"bob"}` + return &c + }(), + version: uint32(1), + }, + newScopeId: org.PublicId, + wantErr: true, + wantRowsUpdate: 0, + wantIsError: db.ErrInvalidFieldMask, + }, + { + name: "no-private-id", + args: args{ + fieldMaskPaths: []string{"Name"}, + config: func() *ExternalConfig { + c := allocExternalConfig() + c.Config = `{"alice":"bob"}` + return &c + }(), + version: uint32(1), + PrivateId: privId(""), + }, + newScopeId: org.PublicId, + wantErr: true, + wantIsError: db.ErrInvalidParameter, + wantRowsUpdate: 0, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert, require := assert.New(t), require.New(t) + + c := TestExternalConfig(t, conn, tt.newScopeId, DevKms, "{}") + + tt.args.config.PrivateId = c.PrivateId + if tt.args.PrivateId != nil { + tt.args.config.PrivateId = *tt.args.PrivateId + } + var cfgAfterUpdate *ExternalConfig + var updatedRows int + var err error + cfgAfterUpdate, updatedRows, err = repo.UpdateExternalConfig(context.Background(), tt.args.config, tt.args.version, tt.args.fieldMaskPaths, tt.args.opt...) + if tt.wantErr { + assert.Error(err) + if tt.wantIsError != nil { + assert.True(errors.Is(err, tt.wantIsError)) + } + assert.Nil(cfgAfterUpdate) + assert.Equal(0, updatedRows) + err = db.TestVerifyOplog(t, rw, c.PrivateId, db.WithOperation(oplog.OpType_OP_TYPE_UPDATE), db.WithCreateNotBefore(10*time.Second)) + assert.Error(err) + assert.True(errors.Is(db.ErrRecordNotFound, err)) + return + } + assert.NoError(err) + assert.Equal(tt.wantRowsUpdate, updatedRows) + foundCfg, err := repo.LookupExternalConfig(context.Background(), c.PrivateId) + require.NoError(err) + assert.True(proto.Equal(cfgAfterUpdate, foundCfg)) + err = db.TestVerifyOplog(t, rw, c.PrivateId, db.WithOperation(oplog.OpType_OP_TYPE_UPDATE), db.WithCreateNotBefore(10*time.Second)) + assert.NoError(err) + }) + } +}