From 3da78aa7e4ac61c26e852a69c61823655f0e0303 Mon Sep 17 00:00:00 2001 From: Jim Date: Mon, 11 Jul 2022 11:40:03 -0400 Subject: [PATCH] refactor (kms): Update dependency required a small refactor (#2253) --- go.mod | 4 +- go.sum | 8 +- .../38/06_kms_collection_version.up.sql | 34 ++++++ .../migrations/oss/postgres_32_01_test.go | 111 ------------------ internal/kms/kms.go | 14 +-- internal/kms/kms_ext_test.go | 3 - internal/kms/kms_test.go | 4 +- 7 files changed, 44 insertions(+), 134 deletions(-) create mode 100644 internal/db/schema/migrations/oss/postgres/38/06_kms_collection_version.up.sql delete mode 100644 internal/db/schema/migrations/oss/postgres_32_01_test.go diff --git a/go.mod b/go.mod index 02973bb8e..3293e7923 100644 --- a/go.mod +++ b/go.mod @@ -30,7 +30,7 @@ require ( github.com/hashicorp/go-bexpr v0.1.10 github.com/hashicorp/go-cleanhttp v0.5.2 github.com/hashicorp/go-hclog v1.1.0 - github.com/hashicorp/go-kms-wrapping/v2 v2.0.6-0.20220512190651-85bdcdd684e6 + github.com/hashicorp/go-kms-wrapping/v2 v2.0.6-0.20220711120347-32232bae6803 github.com/hashicorp/go-multierror v1.1.1 github.com/hashicorp/go-retryablehttp v0.7.0 github.com/hashicorp/go-rootcerts v1.0.2 @@ -91,7 +91,7 @@ require github.com/hashicorp/go-dbw v0.0.0-20220412153211-c470aec9369f // this i require ( github.com/DATA-DOG/go-sqlmock v1.5.0 - github.com/hashicorp/go-kms-wrapping/extras/kms/v2 v2.0.0-20220515130442-cac0b5ac133b + github.com/hashicorp/go-kms-wrapping/extras/kms/v2 v2.0.0-20220711120347-32232bae6803 github.com/hashicorp/nodeenrollment v0.1.7 ) diff --git a/go.sum b/go.sum index 48faf1bdf..e58749f7d 100644 --- a/go.sum +++ b/go.sum @@ -677,12 +677,12 @@ github.com/hashicorp/go-immutable-radix v1.0.0/go.mod h1:0y9vanUI8NX6FsYoO3zeMjh github.com/hashicorp/go-immutable-radix v1.3.1 h1:DKHmCUm2hRBK510BaiZlwvpD40f8bJFeZnpfm2KLowc= github.com/hashicorp/go-immutable-radix v1.3.1/go.mod h1:0y9vanUI8NX6FsYoO3zeMjhV/C5i9g4Q3DwcSNZ4P60= github.com/hashicorp/go-kms-wrapping/entropy v0.1.0/go.mod h1:d1g9WGtAunDNpek8jUIEJnBlbgKS1N2Q61QkHiZyR1g= -github.com/hashicorp/go-kms-wrapping/extras/kms/v2 v2.0.0-20220515130442-cac0b5ac133b h1:NmqGrxmUGR1XRhmYA3wsD2BktB9PfbCDIhXd4FM0hbk= -github.com/hashicorp/go-kms-wrapping/extras/kms/v2 v2.0.0-20220515130442-cac0b5ac133b/go.mod h1:puo5dXNWmXx4xoKCzbKSevcFQ4w+z72k3ZF3EVo6zI4= +github.com/hashicorp/go-kms-wrapping/extras/kms/v2 v2.0.0-20220711120347-32232bae6803 h1:PdSXkY+aAn/19A+lorRNXkfu5zQ9eZTBMNyLdHpeW0c= +github.com/hashicorp/go-kms-wrapping/extras/kms/v2 v2.0.0-20220711120347-32232bae6803/go.mod h1:tTUGkZXcXW2y6A2Z8o2CVxHKSPNgDfUHLBhHCJb0Lts= github.com/hashicorp/go-kms-wrapping/plugin/v2 v2.0.2 h1:it114Kjpk79JVh6AfxFMP5oi1gb+RPzcSLiG3zF/J0w= github.com/hashicorp/go-kms-wrapping/plugin/v2 v2.0.2/go.mod h1:b9mq0bG5xJ10ZcTEQJLZ22O9y71hIt2MGTLqROA/SAM= -github.com/hashicorp/go-kms-wrapping/v2 v2.0.6-0.20220512190651-85bdcdd684e6 h1:isD0/0pGWppbxkakRvnX4M4k65+ZSRW0yELW+twLPDk= -github.com/hashicorp/go-kms-wrapping/v2 v2.0.6-0.20220512190651-85bdcdd684e6/go.mod h1:sDQAfwJGv25uGPZA04x87ERglCG6avnRcBT9wYoMII8= +github.com/hashicorp/go-kms-wrapping/v2 v2.0.6-0.20220711120347-32232bae6803 h1:Ec2MHYwwxfUYNDJFJggeykfAblLwLiwNRYC5hQUDbrI= +github.com/hashicorp/go-kms-wrapping/v2 v2.0.6-0.20220711120347-32232bae6803/go.mod h1:sDQAfwJGv25uGPZA04x87ERglCG6avnRcBT9wYoMII8= github.com/hashicorp/go-multierror v0.0.0-20161216184304-ed905158d874/go.mod h1:JMRHfdO9jKNzS/+BTlxCjKNQHg/jZAft8U7LloJvN7I= github.com/hashicorp/go-multierror v1.0.0/go.mod h1:dHtQlpGsu+cZNNAkkCN/P3hoUDHhCYQXV3UM06sGGrk= github.com/hashicorp/go-multierror v1.1.0/go.mod h1:spPvp8C1qA32ftKqdAHm4hHTbPw+vmowP0z+KUhOZdA= diff --git a/internal/db/schema/migrations/oss/postgres/38/06_kms_collection_version.up.sql b/internal/db/schema/migrations/oss/postgres/38/06_kms_collection_version.up.sql new file mode 100644 index 000000000..83cf81e02 --- /dev/null +++ b/internal/db/schema/migrations/oss/postgres/38/06_kms_collection_version.up.sql @@ -0,0 +1,34 @@ + +-- make the required schema changes to upgrade the dependency: +-- github.com/hashicorp/go-kms-wrapping/extras/kms/v2 +-- this migration is from: +-- https://github.com/hashicorp/go-kms-wrapping/blob/main/extras/kms/migrations/postgres/06_kms_collection_version.up.sql + +begin; + +create table kms_collection_version ( + version kms_version, + create_time timestamp not null default current_timestamp, + update_time timestamp not null default current_timestamp +); + +-- ensure that it's only ever one row +create unique index kms_collection_version_one_row +ON kms_collection_version((version is not null)); + +create trigger kms_immutable_columns +before +update on kms_collection_version + for each row execute procedure kms_immutable_columns('create_time'); + +create trigger kms_update_time_column +before +update on kms_collection_version + for each row execute procedure kms_update_time_column(); + + +insert into kms_collection_version(version) values(1); + +update kms_schema_version set version = 'v0.0.2'; + +commit; \ No newline at end of file diff --git a/internal/db/schema/migrations/oss/postgres_32_01_test.go b/internal/db/schema/migrations/oss/postgres_32_01_test.go deleted file mode 100644 index 0cbcc48e0..000000000 --- a/internal/db/schema/migrations/oss/postgres_32_01_test.go +++ /dev/null @@ -1,111 +0,0 @@ -package oss_test - -import ( - "context" - "testing" - - "github.com/hashicorp/boundary/internal/credential/vault" - "github.com/hashicorp/boundary/internal/db" - "github.com/hashicorp/boundary/internal/db/common" - "github.com/hashicorp/boundary/internal/db/schema" - "github.com/hashicorp/boundary/internal/iam" - "github.com/hashicorp/boundary/testing/dbtest" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" -) - -func TestMigrations_user_password_Migration(t *testing.T) { - const ( - priorMigration = 31002 - currentMigration = 32001 - ) - - t.Parallel() - ctx := context.Background() - dialect := dbtest.Postgres - - c, u, _, err := dbtest.StartUsingTemplate(dialect, dbtest.WithTemplate(dbtest.Template1)) - require.NoError(t, err) - t.Cleanup(func() { - require.NoError(t, c()) - }) - d, err := common.SqlOpen(dialect, u) - require.NoError(t, err) - - // migration to the prior migration (before the one we want to test) - m, err := schema.NewManager(ctx, schema.Dialect(dialect), d, schema.WithEditions( - schema.TestCreatePartialEditions(schema.Dialect(dialect), schema.PartialEditions{"oss": priorMigration}), - )) - require.NoError(t, err) - - require.NoError(t, m.ApplyMigrations(ctx)) - state, err := m.CurrentState(ctx) - require.NoError(t, err) - want := &schema.State{ - Initialized: true, - Editions: []schema.EditionState{ - { - Name: "oss", - BinarySchemaVersion: priorMigration, - DatabaseSchemaVersion: priorMigration, - DatabaseSchemaState: schema.Equal, - }, - }, - } - require.Equal(t, want, state) - - // get a connection - dbType, err := db.StringToDbType(dialect) - require.NoError(t, err) - conn, err := db.Open(ctx, dbType, u) - require.NoError(t, err) - rw := db.New(conn) - - rootWrapper := db.TestWrapper(t) - iamRepo := iam.TestRepo(t, conn, rootWrapper) - _, prj := iam.TestScopes(t, iamRepo) - - cs, err := vault.NewCredentialStore(prj.PublicId, "https://vault", []byte("token")) - cs.PublicId = "csvlt_test1234" - require.NoError(t, rw.Create(context.Background(), cs)) - - upLib, err := vault.NewCredentialLibrary(cs.PublicId, "vault_path", vault.WithMethod("GET"), vault.WithCredentialType("user_password")) - upLib.PublicId = "clvlt_testuplib" - require.NoError(t, rw.Create(context.Background(), upLib)) - - lib, err := vault.NewCredentialLibrary(cs.PublicId, "vault_path", vault.WithMethod("GET")) - lib.PublicId = "clvlt_testlib" - require.NoError(t, rw.Create(context.Background(), lib)) - - // now we're ready for the migration we want to test. - m, err = schema.NewManager(ctx, schema.Dialect(dialect), d, schema.WithEditions( - schema.TestCreatePartialEditions(schema.Dialect(dialect), schema.PartialEditions{"oss": currentMigration}), - )) - require.NoError(t, err) - - require.NoError(t, m.ApplyMigrations(ctx)) - state, err = m.CurrentState(ctx) - require.NoError(t, err) - want = &schema.State{ - Initialized: true, - Editions: []schema.EditionState{ - { - Name: "oss", - BinarySchemaVersion: currentMigration, - DatabaseSchemaVersion: currentMigration, - DatabaseSchemaState: schema.Equal, - }, - }, - } - require.Equal(t, want, state) - - // Validate uplib was migrated to username_password - err = rw.LookupByPublicId(context.Background(), upLib) - require.NoError(t, err) - assert.Equal(t, "username_password", upLib.GetCredentialType()) - - // Validate lib was left as unspecified - err = rw.LookupByPublicId(context.Background(), lib) - require.NoError(t, err) - assert.Equal(t, "unspecified", lib.GetCredentialType()) -} diff --git a/internal/kms/kms.go b/internal/kms/kms.go index c26e869bb..c45082023 100644 --- a/internal/kms/kms.go +++ b/internal/kms/kms.go @@ -39,7 +39,7 @@ func New(ctx context.Context, reader *db.Db, writer *db.Db, _ ...Option) (*Kms, r := dbw.New(reader.UnderlyingDB()) w := dbw.New(writer.UnderlyingDB()) - k, err := wrappingKms.New(r, w, purposes, wrappingKms.WithCache(true)) + k, err := wrappingKms.New(r, w, purposes) if err != nil { return nil, errors.Wrap(ctx, err, op, errors.WithMsg("error creating new in-memory kms")) } @@ -69,7 +69,7 @@ func NewUsingReaderWriter(ctx context.Context, reader db.Reader, writer db.Write if err != nil { return nil, errors.Wrap(ctx, err, op) } - k, err := wrappingKms.New(r, w, purposes, wrappingKms.WithCache(true)) + k, err := wrappingKms.New(r, w, purposes) if err != nil { return nil, errors.Wrap(ctx, err, op, errors.WithMsg("error creating new in-memory kms")) } @@ -228,16 +228,6 @@ func (k *Kms) CreateKeys(ctx context.Context, scopeId string, opt ...Option) err return nil } -// ClearCache will clear the kms's cache which is useful after a scope has been -// deleted. -func (k *Kms) ClearCache(ctx context.Context) error { - const op = "kms.(Kms).ClearCache" - if err := k.underlying.ClearCache(ctx); err != nil { - return errors.Wrap(ctx, err, op) - } - return nil -} - // VerifyGlobalRoot will verify that the global root wrapper is reasonable. func (k *Kms) VerifyGlobalRoot(ctx context.Context) error { const op = "kms.(Kms).VerifyGlobalRoot" diff --git a/internal/kms/kms_ext_test.go b/internal/kms/kms_ext_test.go index 8bf676411..3fe83598e 100644 --- a/internal/kms/kms_ext_test.go +++ b/internal/kms/kms_ext_test.go @@ -125,9 +125,6 @@ func TestKms_ReconcileKeys(t *testing.T) { reader: rand.Reader, setup: func(k *kms.Kms) { kms.TestKmsDeleteKeyPurpose(t, conn, kms.KeyPurposeAudit) - // make sure the kms is in the proper state for the unit test - // before proceeding. - k.ClearCache(testCtx) _, err := k.GetWrapper(testCtx, scope.Global.String(), kms.KeyPurposeAudit) require.Error(t, err) }, diff --git a/internal/kms/kms_test.go b/internal/kms/kms_test.go index f0c5bf91f..0d6eb93db 100644 --- a/internal/kms/kms_test.go +++ b/internal/kms/kms_test.go @@ -57,7 +57,7 @@ func Test_New(t *testing.T) { r := dbw.New(rw.UnderlyingDB()) w := dbw.New(rw.UnderlyingDB()) - wrapped, err := wrappingKms.New(r, w, purposes, wrappingKms.WithCache(true)) + wrapped, err := wrappingKms.New(r, w, purposes) require.NoError(t, err) return wrapped }(), @@ -140,7 +140,7 @@ func Test_NewUsingReaderWriter(t *testing.T) { purposes := stdNewKmsPurposes() r := dbw.New(rw.UnderlyingDB()) w := dbw.New(rw.UnderlyingDB()) - wrapped, err := wrappingKms.New(r, w, purposes, wrappingKms.WithCache(true)) + wrapped, err := wrappingKms.New(r, w, purposes) require.NoError(t, err) return wrapped }(),