-
Notifications
You must be signed in to change notification settings - Fork 4.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update vault ca provider namespace configuration #19095
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
```release-note:bug | ||
ca: ensure Vault CA provider respects Vault Enterprise namespace configuration. | ||
``` |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -61,6 +61,10 @@ type CASigningKeyTypes struct { | |
CSRKeyBits int | ||
} | ||
|
||
type vaultRequirements struct { | ||
Enterprise bool | ||
} | ||
Comment on lines
+64
to
+66
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe this mechanism is overkill but it would allow us to have other tests be discriminant about what Vault features are available to test with. For example, we could skip tests for certain Vault versions if the feature isn't applicable. Maybe this falls in the category of YNGNI, but it was super easy to implement it this way, so I did. |
||
|
||
// CASigningKeyTypeCases returns the cross-product of the important supported CA | ||
// key types for generating table tests for CA signing tests (CrossSignCA and | ||
// SignIntermediate). | ||
|
@@ -93,7 +97,7 @@ func TestConsulProvider(t testing.T, d ConsulProviderStateDelegate) *ConsulProvi | |
// | ||
// These tests may be skipped in CI. They are run as part of a separate | ||
// integration test suite. | ||
func SkipIfVaultNotPresent(t testing.T) { | ||
func SkipIfVaultNotPresent(t testing.T, reqs ...vaultRequirements) { | ||
// Try to safeguard against tests that will never run in CI. | ||
// This substring should match the pattern used by the | ||
// test-connect-ca-providers CI job. | ||
|
@@ -110,6 +114,16 @@ func SkipIfVaultNotPresent(t testing.T) { | |
if err != nil || path == "" { | ||
t.Skipf("%q not found on $PATH - download and install to run this test", vaultBinaryName) | ||
} | ||
|
||
// Check for any additional Vault requirements. | ||
for _, r := range reqs { | ||
if r.Enterprise { | ||
ver := vaultVersion(t, vaultBinaryName) | ||
if !strings.Contains(ver, "+ent") { | ||
t.Skipf("%q is not a Vault Enterprise version", ver) | ||
} | ||
} | ||
} | ||
} | ||
|
||
func NewTestVaultServer(t testing.T) *TestVaultServer { | ||
|
@@ -239,8 +253,8 @@ func requireTrailingNewline(t testing.T, leafPEM string) { | |
if len(leafPEM) == 0 { | ||
t.Fatalf("cert is empty") | ||
} | ||
if '\n' != rune(leafPEM[len(leafPEM)-1]) { | ||
t.Fatalf("cert do not end with a new line") | ||
if rune(leafPEM[len(leafPEM)-1]) != '\n' { | ||
t.Fatalf("cert does not end with a new line") | ||
Comment on lines
+256
to
+257
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just cleaning up code flagged by |
||
} | ||
} | ||
|
||
|
@@ -367,3 +381,10 @@ func createVaultTokenAndPolicy(t testing.T, client *vaultapi.Client, policyName, | |
require.NoError(t, err) | ||
return tok.Auth.ClientToken | ||
} | ||
|
||
func vaultVersion(t testing.T, vaultBinaryName string) string { | ||
cmd := exec.Command(vaultBinaryName, []string{"version"}...) | ||
output, err := cmd.Output() | ||
require.NoError(t, err) | ||
return string(output[:len(output)-1]) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This check here isn't strictly necessary but it does duplicate the problem reported in the issue where the provider fails to initialize because the namespace doesn't exist.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good