From 34ba7acdb8eb9f31d534d881f154bc448906fc37 Mon Sep 17 00:00:00 2001 From: Nick Cabatoff Date: Tue, 17 Oct 2023 09:46:54 -0400 Subject: [PATCH] Start using entAddExtPlugins instead of the init hook addExternalPlugins (#23665) This causes the registry to now contain ent plugins on ent; previously it did not, though that appears to have been the intention. I believe this is because of the order in which inits were run. Having changed this, various tests broke that were relying on the incorrect behaviour. Several tests were changed to rely less on opaque counts of expected plugins, instead they're now using explicit comparison by name. --- command/auth_enable_test.go | 19 ++++++++------- command/base_predict_test.go | 15 ++++++++++-- command/secrets_enable_test.go | 20 ++++++++-------- helper/builtinplugins/registry.go | 2 +- helper/builtinplugins/registry_test.go | 32 +++++++++++++++++++------- scripts/gen_openapi.sh | 1 + 6 files changed, 58 insertions(+), 31 deletions(-) diff --git a/command/auth_enable_test.go b/command/auth_enable_test.go index f58546d8d4..93c685819e 100644 --- a/command/auth_enable_test.go +++ b/command/auth_enable_test.go @@ -5,21 +5,18 @@ package command import ( "io/ioutil" + "sort" "strings" "testing" "github.com/go-test/deep" + "github.com/google/go-cmp/cmp" "github.com/hashicorp/vault/helper/builtinplugins" "github.com/hashicorp/vault/sdk/helper/consts" + "github.com/hashicorp/vault/sdk/helper/strutil" "github.com/mitchellh/cli" ) -// credentialBackendAdjustmentFactor allows for adjusting test assertions for -// credential backends. Add 1 to account for the "token" backend, which is visible -// when you walk the filesystem but is treated as special and excluded from the registry. -// Subtract 1 to account for "oidc" which is an alias of "jwt" and not a separate plugin. -var credentialBackendAdjustmentFactor = 1 - 1 - func testAuthEnableCommand(tb testing.TB) (*cli.MockUi, *AuthEnableCommand) { tb.Helper() @@ -186,7 +183,7 @@ func TestAuthEnableCommand_Run(t *testing.T) { var backends []string for _, f := range files { - if f.IsDir() { + if f.IsDir() && f.Name() != "token" { backends = append(backends, f.Name()) } } @@ -211,9 +208,11 @@ func TestAuthEnableCommand_Run(t *testing.T) { // of credential backends. backends = append(backends, "pcf") - expected := len(builtinplugins.Registry.Keys(consts.PluginTypeCredential)) + credentialBackendAdjustmentFactor - if len(backends) != expected { - t.Fatalf("expected %d credential backends, got %d", expected, len(backends)) + regkeys := strutil.StrListDelete(builtinplugins.Registry.Keys(consts.PluginTypeCredential), "oidc") + sort.Strings(regkeys) + sort.Strings(backends) + if d := cmp.Diff(regkeys, backends); len(d) > 0 { + t.Fatalf("found credential registry mismatch: %v", d) } for _, b := range backends { diff --git a/command/base_predict_test.go b/command/base_predict_test.go index 2d752ed635..778245830f 100644 --- a/command/base_predict_test.go +++ b/command/base_predict_test.go @@ -7,6 +7,8 @@ import ( "reflect" "testing" + "github.com/google/go-cmp/cmp" + "github.com/hashicorp/go-secure-stdlib/strutil" "github.com/hashicorp/vault/api" "github.com/posener/complete" @@ -389,6 +391,7 @@ func TestPredict_Plugins(t *testing.T) { "redis-database-plugin", "redis-elasticache-database-plugin", "redshift-database-plugin", + "saml", "snowflake-database-plugin", "ssh", "terraform", @@ -435,8 +438,16 @@ func TestPredict_Plugins(t *testing.T) { } } } - if !reflect.DeepEqual(act, tc.exp) { - t.Errorf("expected: %q, got: %q, diff: %v", tc.exp, act, strutil.Difference(act, tc.exp, true)) + if !strutil.StrListContains(act, "saml") { + for i, v := range tc.exp { + if v == "saml" { + tc.exp = append(tc.exp[:i], tc.exp[i+1:]...) + break + } + } + } + if d := cmp.Diff(act, tc.exp); len(d) > 0 { + t.Errorf("expected: %q, got: %q, diff: %v", tc.exp, act, d) } }) } diff --git a/command/secrets_enable_test.go b/command/secrets_enable_test.go index 99c8573775..488d25d369 100644 --- a/command/secrets_enable_test.go +++ b/command/secrets_enable_test.go @@ -7,20 +7,18 @@ import ( "errors" "io/ioutil" "os" + "sort" "strings" "testing" "github.com/go-test/deep" + "github.com/google/go-cmp/cmp" + "github.com/hashicorp/go-secure-stdlib/strutil" "github.com/hashicorp/vault/helper/builtinplugins" "github.com/hashicorp/vault/sdk/helper/consts" "github.com/mitchellh/cli" ) -// logicalBackendAdjustmentFactor is set to plus 1 for the database backend -// which is a plugin but not found in go.mod files, and minus 1 for the ldap -// and openldap secret backends which have the same underlying plugin. -var logicalBackendAdjustmentFactor = 1 - 1 - func testSecretsEnableCommand(tb testing.TB) (*cli.MockUi, *SecretsEnableCommand) { tb.Helper() @@ -218,7 +216,7 @@ func TestSecretsEnableCommand_Run(t *testing.T) { var backends []string for _, f := range files { if f.IsDir() { - if f.Name() == "plugin" { + if f.Name() == "plugin" || f.Name() == "database" { continue } if _, err := os.Stat("../builtin/logical/" + f.Name() + "/backend.go"); errors.Is(err, os.ErrNotExist) { @@ -245,10 +243,12 @@ func TestSecretsEnableCommand_Run(t *testing.T) { } } - // backends are found by walking the directory, which includes the database backend, - // however, the plugins registry omits that one - if len(backends) != len(builtinplugins.Registry.Keys(consts.PluginTypeSecrets))+logicalBackendAdjustmentFactor { - t.Fatalf("expected %d logical backends, got %d", len(builtinplugins.Registry.Keys(consts.PluginTypeSecrets))+logicalBackendAdjustmentFactor, len(backends)) + regkeys := strutil.StrListDelete(builtinplugins.Registry.Keys(consts.PluginTypeSecrets), "ldap") + sort.Strings(regkeys) + sort.Strings(backends) + + if d := cmp.Diff(regkeys, backends); len(d) > 0 { + t.Fatalf("found logical registry mismatch: %v", d) } for _, b := range backends { diff --git a/helper/builtinplugins/registry.go b/helper/builtinplugins/registry.go index 3c531a9e1f..462b858824 100644 --- a/helper/builtinplugins/registry.go +++ b/helper/builtinplugins/registry.go @@ -200,7 +200,7 @@ func newRegistry() *registry { }, } - addExternalPlugins(reg) + entAddExtPlugins(reg) return reg } diff --git a/helper/builtinplugins/registry_test.go b/helper/builtinplugins/registry_test.go index 77012886ee..99bbd05e61 100644 --- a/helper/builtinplugins/registry_test.go +++ b/helper/builtinplugins/registry_test.go @@ -12,6 +12,7 @@ import ( "testing" credUserpass "github.com/hashicorp/vault/builtin/credential/userpass" + "github.com/hashicorp/vault/helper/constants" dbMysql "github.com/hashicorp/vault/plugins/database/mysql" "github.com/hashicorp/vault/sdk/helper/consts" @@ -87,6 +88,7 @@ func Test_RegistryKeyCounts(t *testing.T) { name string pluginType consts.PluginType want int // use slice length as test condition + entWant int wantOk bool }{ { @@ -98,6 +100,7 @@ func Test_RegistryKeyCounts(t *testing.T) { name: "number of auth plugins", pluginType: consts.PluginTypeCredential, want: 19, + entWant: 1, }, { name: "number of database plugins", @@ -108,13 +111,18 @@ func Test_RegistryKeyCounts(t *testing.T) { name: "number of secrets plugins", pluginType: consts.PluginTypeSecrets, want: 19, + entWant: 3, }, } 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) + want := tt.want + if constants.IsEnterprise { + want += tt.entWant + } + if len(keys) != want { + t.Fatalf("got size: %d, want size: %d", len(keys), want) } }) } @@ -240,12 +248,20 @@ func Test_RegistryMatchesGenOpenapi(t *testing.T) { } defer f.Close() + // This is a hack: the gen_openapi script contains a conditional block to + // enable the enterprise plugins, whose lines are indented. Tweak the + // regexp to only include the indented lines on enterprise. + leading := "^" + if constants.IsEnterprise { + leading = "^ *" + } + var ( credentialBackends []string - credentialBackendsRe = regexp.MustCompile(`^vault auth enable (?:-.+ )*(?:"([a-zA-Z]+)"|([a-zA-Z]+))$`) + credentialBackendsRe = regexp.MustCompile(leading + `vault auth enable (?:-.+ )*(?:"([a-zA-Z]+)"|([a-zA-Z]+))$`) secretsBackends []string - secretsBackendsRe = regexp.MustCompile(`^vault secrets enable (?:-.+ )*(?:"([a-zA-Z]+)"|([a-zA-Z]+))$`) + secretsBackendsRe = regexp.MustCompile(leading + `vault secrets enable (?:-.+ )*(?:"([a-zA-Z]+)"|([a-zA-Z]+))$`) ) scanner := bufio.NewScanner(f) @@ -280,15 +296,15 @@ func Test_RegistryMatchesGenOpenapi(t *testing.T) { deprecationStatus, ok := Registry.DeprecationStatus(name, pluginType) if !ok { - t.Fatalf("%q %s backend is missing from registry.go; please remove it from gen_openapi.sh", name, pluginType) + t.Errorf("%q %s backend is missing from registry.go; please remove it from gen_openapi.sh", name, pluginType) } if deprecationStatus == consts.Removed { - t.Fatalf("%q %s backend is marked 'removed' in registry.go; please remove it from gen_openapi.sh", name, pluginType) + t.Errorf("%q %s backend is marked 'removed' in registry.go; please remove it from gen_openapi.sh", name, pluginType) } } - // ensureInScript ensures that the given plugin name in in gen_openapi.sh script + // ensureInScript ensures that the given plugin name is in gen_openapi.sh script ensureInScript := func(t *testing.T, scriptBackends []string, name string) { t.Helper() @@ -302,7 +318,7 @@ func Test_RegistryMatchesGenOpenapi(t *testing.T) { } if !slices.Contains(scriptBackends, name) { - t.Fatalf("%q backend could not be found in gen_openapi.sh, please add it there", name) + t.Errorf("%q backend could not be found in gen_openapi.sh, please add it there", name) } } diff --git a/scripts/gen_openapi.sh b/scripts/gen_openapi.sh index f523ea3343..ef2acca0a0 100755 --- a/scripts/gen_openapi.sh +++ b/scripts/gen_openapi.sh @@ -94,6 +94,7 @@ if [[ -n "${VAULT_LICENSE:-}" ]]; then vault secrets enable "keymgmt" vault secrets enable "kmip" vault secrets enable "transform" + vault auth enable "saml" fi # Output OpenAPI, optionally formatted