From 495fce75f317bde9c841e10680d201394154c32a Mon Sep 17 00:00:00 2001 From: Mike Palmiotto Date: Tue, 23 Aug 2022 14:04:32 -0400 Subject: [PATCH 1/3] plugins: Add Deprecation Status to builtinRegistry --- helper/builtinplugins/registry.go | 206 +++++++++++++++-------- helper/builtinplugins/registry_test.go | 207 ++++++++++++++++++++++++ sdk/helper/consts/deprecation_status.go | 29 ++++ vault/core.go | 1 + vault/testing.go | 4 + 5 files changed, 376 insertions(+), 71 deletions(-) create mode 100644 helper/builtinplugins/registry_test.go create mode 100644 sdk/helper/consts/deprecation_status.go diff --git a/helper/builtinplugins/registry.go b/helper/builtinplugins/registry.go index 3157159162c0..c27fd0ceb821 100644 --- a/helper/builtinplugins/registry.go +++ b/helper/builtinplugins/registry.go @@ -68,74 +68,111 @@ var addExternalPlugins = addExtPluginsImpl // the plugin's New() func. type BuiltinFactory func() (interface{}, error) +// There are three forms of Backends which exist in the BuiltinRegistry. +type credentialBackend struct { + logical.Factory + consts.DeprecationStatus +} + +type databasePlugin struct { + Factory BuiltinFactory + consts.DeprecationStatus +} + +type logicalBackend struct { + logical.Factory + consts.DeprecationStatus +} + func newRegistry() *registry { reg := ®istry{ - credentialBackends: map[string]logical.Factory{ - "alicloud": credAliCloud.Factory, - "app-id": credAppId.Factory, - "approle": credAppRole.Factory, - "aws": credAws.Factory, - "azure": credAzure.Factory, - "centrify": credCentrify.Factory, - "cert": credCert.Factory, - "cf": credCF.Factory, - "gcp": credGcp.Factory, - "github": credGitHub.Factory, - "jwt": credJWT.Factory, - "kerberos": credKerb.Factory, - "kubernetes": credKube.Factory, - "ldap": credLdap.Factory, - "oci": credOCI.Factory, - "oidc": credJWT.Factory, - "okta": credOkta.Factory, - "pcf": credCF.Factory, // Deprecated. - "radius": credRadius.Factory, - "userpass": credUserpass.Factory, + credentialBackends: map[string]credentialBackend{ + "alicloud": {Factory: credAliCloud.Factory}, + "app-id": { + Factory: credAppId.Factory, + DeprecationStatus: consts.PendingRemoval, + }, + "approle": {Factory: credAppRole.Factory}, + "aws": {Factory: credAws.Factory}, + "azure": {Factory: credAzure.Factory}, + "centrify": {Factory: credCentrify.Factory}, + "cert": {Factory: credCert.Factory}, + "cf": {Factory: credCF.Factory}, + "gcp": {Factory: credGcp.Factory}, + "github": {Factory: credGitHub.Factory}, + "jwt": {Factory: credJWT.Factory}, + "kerberos": {Factory: credKerb.Factory}, + "kubernetes": {Factory: credKube.Factory}, + "ldap": {Factory: credLdap.Factory}, + "oci": {Factory: credOCI.Factory}, + "oidc": {Factory: credJWT.Factory}, + "okta": {Factory: credOkta.Factory}, + "pcf": { + Factory: credCF.Factory, + DeprecationStatus: consts.Deprecated, + }, + "radius": {Factory: credRadius.Factory}, + "userpass": {Factory: credUserpass.Factory}, }, - databasePlugins: map[string]BuiltinFactory{ + databasePlugins: map[string]databasePlugin{ // These four plugins all use the same mysql implementation but with // different username settings passed by the constructor. - "mysql-database-plugin": dbMysql.New(dbMysql.DefaultUserNameTemplate), - "mysql-aurora-database-plugin": dbMysql.New(dbMysql.DefaultLegacyUserNameTemplate), - "mysql-rds-database-plugin": dbMysql.New(dbMysql.DefaultLegacyUserNameTemplate), - "mysql-legacy-database-plugin": dbMysql.New(dbMysql.DefaultLegacyUserNameTemplate), - - "cassandra-database-plugin": dbCass.New, - "couchbase-database-plugin": dbCouchbase.New, - "elasticsearch-database-plugin": dbElastic.New, - "hana-database-plugin": dbHana.New, - "influxdb-database-plugin": dbInflux.New, - "mongodb-database-plugin": dbMongo.New, - "mongodbatlas-database-plugin": dbMongoAtlas.New, - "mssql-database-plugin": dbMssql.New, - "postgresql-database-plugin": dbPostgres.New, - "redshift-database-plugin": dbRedshift.New, - "snowflake-database-plugin": dbSnowflake.New, + "mysql-database-plugin": {Factory: dbMysql.New(dbMysql.DefaultUserNameTemplate)}, + "mysql-aurora-database-plugin": {Factory: dbMysql.New(dbMysql.DefaultLegacyUserNameTemplate)}, + "mysql-rds-database-plugin": {Factory: dbMysql.New(dbMysql.DefaultLegacyUserNameTemplate)}, + "mysql-legacy-database-plugin": {Factory: dbMysql.New(dbMysql.DefaultLegacyUserNameTemplate)}, + + "cassandra-database-plugin": {Factory: dbCass.New}, + "couchbase-database-plugin": {Factory: dbCouchbase.New}, + "elasticsearch-database-plugin": {Factory: dbElastic.New}, + "hana-database-plugin": {Factory: dbHana.New}, + "influxdb-database-plugin": {Factory: dbInflux.New}, + "mongodb-database-plugin": {Factory: dbMongo.New}, + "mongodbatlas-database-plugin": {Factory: dbMongoAtlas.New}, + "mssql-database-plugin": {Factory: dbMssql.New}, + "postgresql-database-plugin": {Factory: dbPostgres.New}, + "redshift-database-plugin": {Factory: dbRedshift.New}, + "snowflake-database-plugin": {Factory: dbSnowflake.New}, }, - logicalBackends: map[string]logical.Factory{ - "ad": logicalAd.Factory, - "alicloud": logicalAlicloud.Factory, - "aws": logicalAws.Factory, - "azure": logicalAzure.Factory, - "cassandra": logicalCass.Factory, // Deprecated - "consul": logicalConsul.Factory, - "gcp": logicalGcp.Factory, - "gcpkms": logicalGcpKms.Factory, - "kubernetes": logicalKube.Factory, - "kv": logicalKv.Factory, - "mongodb": logicalMongo.Factory, // Deprecated - "mongodbatlas": logicalMongoAtlas.Factory, - "mssql": logicalMssql.Factory, // Deprecated - "mysql": logicalMysql.Factory, // Deprecated - "nomad": logicalNomad.Factory, - "openldap": logicalOpenLDAP.Factory, - "pki": logicalPki.Factory, - "postgresql": logicalPostgres.Factory, // Deprecated - "rabbitmq": logicalRabbit.Factory, - "ssh": logicalSsh.Factory, - "terraform": logicalTerraform.Factory, - "totp": logicalTotp.Factory, - "transit": logicalTransit.Factory, + logicalBackends: map[string]logicalBackend{ + "ad": {Factory: logicalAd.Factory}, + "alicloud": {Factory: logicalAlicloud.Factory}, + "aws": {Factory: logicalAws.Factory}, + "azure": {Factory: logicalAzure.Factory}, + "cassandra": { + Factory: logicalCass.Factory, + DeprecationStatus: consts.Deprecated, + }, + "consul": {Factory: logicalConsul.Factory}, + "gcp": {Factory: logicalGcp.Factory}, + "gcpkms": {Factory: logicalGcpKms.Factory}, + "kubernetes": {Factory: logicalKube.Factory}, + "kv": {Factory: logicalKv.Factory}, + "mongodb": { + Factory: logicalMongo.Factory, + DeprecationStatus: consts.Deprecated, + }, + "mongodbatlas": {Factory: logicalMongoAtlas.Factory}, + "mssql": { + Factory: logicalMssql.Factory, + DeprecationStatus: consts.Deprecated, + }, + "mysql": { + Factory: logicalMysql.Factory, + DeprecationStatus: consts.Deprecated, + }, + "nomad": {Factory: logicalNomad.Factory}, + "openldap": {Factory: logicalOpenLDAP.Factory}, + "pki": {Factory: logicalPki.Factory}, + "postgresql": { + Factory: logicalPostgres.Factory, + DeprecationStatus: consts.Deprecated, + }, + "rabbitmq": {Factory: logicalRabbit.Factory}, + "ssh": {Factory: logicalSsh.Factory}, + "terraform": {Factory: logicalTerraform.Factory}, + "totp": {Factory: logicalTotp.Factory}, + "transit": {Factory: logicalTransit.Factory}, }, } @@ -147,9 +184,9 @@ func newRegistry() *registry { func addExtPluginsImpl(r *registry) {} type registry struct { - credentialBackends map[string]logical.Factory - databasePlugins map[string]BuiltinFactory - logicalBackends map[string]logical.Factory + credentialBackends map[string]credentialBackend + databasePlugins map[string]databasePlugin + logicalBackends map[string]logicalBackend } // Get returns the Factory func for a particular backend plugin from the @@ -157,17 +194,22 @@ type registry struct { func (r *registry) Get(name string, pluginType consts.PluginType) (func() (interface{}, error), bool) { switch pluginType { case consts.PluginTypeCredential: - f, ok := r.credentialBackends[name] - return toFunc(f), ok + if f, ok := r.credentialBackends[name]; ok { + return toFunc(f.Factory), ok + } case consts.PluginTypeSecrets: - f, ok := r.logicalBackends[name] - return toFunc(f), ok + if f, ok := r.logicalBackends[name]; ok { + return toFunc(f.Factory), ok + } case consts.PluginTypeDatabase: - f, ok := r.databasePlugins[name] - return f, ok + if f, ok := r.databasePlugins[name]; ok { + return f.Factory, ok + } default: return nil, false } + + return nil, false } // Keys returns the list of plugin names that are considered builtin plugins. @@ -199,6 +241,28 @@ func (r *registry) Contains(name string, pluginType consts.PluginType) bool { return false } +// DeprecationStatus returns the Deprecation status for a builtin with type `pluginType` +func (r *registry) DeprecationStatus(name string, pluginType consts.PluginType) (consts.DeprecationStatus, bool) { + switch pluginType { + case consts.PluginTypeCredential: + if f, ok := r.credentialBackends[name]; ok { + return f.DeprecationStatus, ok + } + case consts.PluginTypeSecrets: + if f, ok := r.logicalBackends[name]; ok { + return f.DeprecationStatus, ok + } + case consts.PluginTypeDatabase: + if f, ok := r.databasePlugins[name]; ok { + return f.DeprecationStatus, ok + } + default: + return consts.Unknown, false + } + + return consts.Unknown, false +} + func toFunc(ifc interface{}) func() (interface{}, error) { return func() (interface{}, error) { return ifc, nil diff --git a/helper/builtinplugins/registry_test.go b/helper/builtinplugins/registry_test.go new file mode 100644 index 000000000000..25785f56375c --- /dev/null +++ b/helper/builtinplugins/registry_test.go @@ -0,0 +1,207 @@ +package builtinplugins + +import ( + "reflect" + "testing" + + credAppId "github.com/hashicorp/vault/builtin/credential/app-id" + dbMysql "github.com/hashicorp/vault/plugins/database/mysql" + "github.com/hashicorp/vault/sdk/helper/consts" +) + +// Test_RegistryGet exercises the (registry).Get functionality by comparing +// factory types and ok response. +func Test_RegistryGet(t *testing.T) { + tests := []struct { + name string + builtin string + pluginType consts.PluginType + want BuiltinFactory + wantOk bool + }{ + { + name: "non-existent builtin", + builtin: "foo", + pluginType: consts.PluginTypeCredential, + want: nil, + wantOk: false, + }, + { + name: "bad plugin type", + builtin: "app-id", + pluginType: 9000, + want: nil, + wantOk: false, + }, + { + name: "known builtin lookup", + builtin: "app-id", + pluginType: consts.PluginTypeCredential, + want: toFunc(credAppId.Factory), + wantOk: true, + }, + { + name: "known builtin lookup", + builtin: "mysql-database-plugin", + pluginType: consts.PluginTypeDatabase, + want: dbMysql.New(dbMysql.DefaultUserNameTemplate), + wantOk: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var got BuiltinFactory + got, ok := Registry.Get(tt.builtin, tt.pluginType) + if ok { + if reflect.TypeOf(got) != reflect.TypeOf(tt.want) { + t.Fatalf("got type: %T, want type: %T", got, tt.want) + } + } + if tt.wantOk != ok { + t.Fatalf("error: got %v, want %v", ok, tt.wantOk) + } + }) + } +} + +// Test_RegistryKeyCounts is a light unit test used to check the builtin +// registry lists for each plugin type and make sure they match in length. +func Test_RegistryKeyCounts(t *testing.T) { + tests := []struct { + name string + pluginType consts.PluginType + want int // use slice length as test condition + wantOk bool + }{ + { + name: "bad plugin type", + pluginType: 9001, + want: 0, + }, + { + name: "number of auth plugins", + pluginType: consts.PluginTypeCredential, + want: 20, + }, + { + name: "number of database plugins", + pluginType: consts.PluginTypeDatabase, + want: 15, + }, + { + name: "number of secrets plugins", + pluginType: consts.PluginTypeSecrets, + want: 23, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + keys := Registry.Keys(tt.pluginType) + if len(keys) != tt.want { + t.Fatalf("got size: %d, want size: %d", len(keys), tt.want) + } + }) + } +} + +// Test_RegistryContains exercises the (registry).Contains functionality. +func Test_RegistryContains(t *testing.T) { + tests := []struct { + name string + builtin string + pluginType consts.PluginType + want bool + }{ + { + name: "non-existent builtin", + builtin: "foo", + pluginType: consts.PluginTypeCredential, + want: false, + }, + { + name: "bad plugin type", + builtin: "app-id", + pluginType: 9001, + want: false, + }, + { + name: "known builtin lookup", + builtin: "app-id", + pluginType: consts.PluginTypeCredential, + want: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := Registry.Contains(tt.builtin, tt.pluginType) + if got != tt.want { + t.Fatalf("error: got %v, wanted %v", got, tt.want) + } + }) + } +} + +// Test_RegistryStatus exercises the (registry).Status functionality. +func Test_RegistryStatus(t *testing.T) { + tests := []struct { + name string + builtin string + pluginType consts.PluginType + want consts.DeprecationStatus + wantOk bool + }{ + { + name: "non-existent builtin and valid type", + builtin: "foo", + pluginType: consts.PluginTypeCredential, + want: consts.Unknown, + wantOk: false, + }, + { + name: "mismatch builtin and plugin type", + builtin: "app-id", + pluginType: consts.PluginTypeSecrets, + want: consts.Unknown, + wantOk: false, + }, + { + name: "existing builtin and invalid plugin type", + builtin: "app-id", + pluginType: 9000, + want: consts.Unknown, + wantOk: false, + }, + { + name: "supported builtin lookup", + builtin: "approle", + pluginType: consts.PluginTypeCredential, + want: consts.Supported, + wantOk: true, + }, + { + name: "deprecated builtin lookup", + builtin: "mongodb", + pluginType: consts.PluginTypeSecrets, + want: consts.Deprecated, + wantOk: true, + }, + { + name: "pending removal builtin lookup", + builtin: "app-id", + pluginType: consts.PluginTypeCredential, + want: consts.PendingRemoval, + wantOk: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, ok := Registry.DeprecationStatus(tt.builtin, tt.pluginType) + if got != tt.want { + t.Fatalf("got %+v, wanted %+v", got, tt.want) + } + if ok != tt.wantOk { + t.Fatalf("got ok: %t, want ok: %t", ok, tt.wantOk) + } + }) + } +} diff --git a/sdk/helper/consts/deprecation_status.go b/sdk/helper/consts/deprecation_status.go new file mode 100644 index 000000000000..cffee1bd131e --- /dev/null +++ b/sdk/helper/consts/deprecation_status.go @@ -0,0 +1,29 @@ +package consts + +// DeprecationStatus represents the current deprecation state for builtins +type DeprecationStatus uint32 + +// These are the states of deprecation for builtin plugins +const ( + Supported = iota + Deprecated + PendingRemoval + Removed + Unknown +) + +// String returns the string representation of a builtin deprecation status +func (s DeprecationStatus) String() string { + switch s { + case Supported: + return "supported" + case Deprecated: + return "deprecated" + case PendingRemoval: + return "pending removal" + case Removed: + return "removed" + default: + return "" + } +} diff --git a/vault/core.go b/vault/core.go index 763ab180d13a..2e3fd4a0a390 100644 --- a/vault/core.go +++ b/vault/core.go @@ -2954,6 +2954,7 @@ type BuiltinRegistry interface { Contains(name string, pluginType consts.PluginType) bool Get(name string, pluginType consts.PluginType) (func() (interface{}, error), bool) Keys(pluginType consts.PluginType) []string + DeprecationStatus(name string, pluginType consts.PluginType) (consts.DeprecationStatus, bool) } func (c *Core) AuditLogger() AuditLogger { diff --git a/vault/testing.go b/vault/testing.go index f5978ce144dd..8a42c617309e 100644 --- a/vault/testing.go +++ b/vault/testing.go @@ -2247,6 +2247,10 @@ func (m *mockBuiltinRegistry) Contains(name string, pluginType consts.PluginType return false } +func (m *mockBuiltinRegistry) DeprecationStatus(name string, pluginType consts.PluginType) (consts.DeprecationStatus, bool) { + return consts.Supported, true +} + type NoopAudit struct { Config *audit.BackendConfig ReqErr error From 6f7d77f81783a337b12f19b4e72caf121a845bb8 Mon Sep 17 00:00:00 2001 From: Mike Palmiotto Date: Tue, 23 Aug 2022 15:17:04 -0400 Subject: [PATCH 2/3] changelog: Deprecation Status method --- changelog/status_method.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 changelog/status_method.txt diff --git a/changelog/status_method.txt b/changelog/status_method.txt new file mode 100644 index 000000000000..9439547a0581 --- /dev/null +++ b/changelog/status_method.txt @@ -0,0 +1,3 @@ +```release-note:enhancement +plugins: Add Deprecation Status method to builtinregistry. +``` From d69b3bcb9816849a1d51a52fe43416f97319fd6e Mon Sep 17 00:00:00 2001 From: Mike Palmiotto Date: Tue, 23 Aug 2022 16:09:00 -0400 Subject: [PATCH 3/3] changelog: Fixup changelog placeholder --- changelog/{status_method.txt => 16846.txt} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename changelog/{status_method.txt => 16846.txt} (100%) diff --git a/changelog/status_method.txt b/changelog/16846.txt similarity index 100% rename from changelog/status_method.txt rename to changelog/16846.txt