Skip to content

Commit

Permalink
Added cleanup of previous default issuers
Browse files Browse the repository at this point in the history
  • Loading branch information
Chris S. Kim committed Sep 13, 2023
1 parent 09fffd4 commit edcd84e
Show file tree
Hide file tree
Showing 2 changed files with 54 additions and 7 deletions.
21 changes: 20 additions & 1 deletion agent/connect/ca/provider_vault.go
Original file line number Diff line number Diff line change
Expand Up @@ -659,14 +659,28 @@ func (v *VaultProvider) setDefaultIntermediateIssuer(vaultResp *vaultapi.Secret,
return fmt.Errorf("could not read from /config/issuers: %w", err)
}
issuersConf := resp.Data
defaultIssuer := issuersConf["default"].(string)

if defaultIssuer == intermediateId {
return nil
}

// Overwrite the default issuer
issuersConf["default"] = intermediateId

_, err = v.writeNamespaced(v.config.IntermediatePKINamespace, v.config.IntermediatePKIPath+"config/issuers", issuersConf)
if err != nil {
return fmt.Errorf("could not write default issuer to /config/issuers: %w", err)
}

// Delete the previously known default issuer to prevent the number of unused
// issuers from increasing too much.
_, err = v.deleteNamespaced(v.config.IntermediatePKINamespace, v.config.IntermediatePKIPath+"issuer/"+defaultIssuer)
if err != nil {
v.logger.Warn("Could not delete previous issuer. Manually delete from Vault to prevent the list of issuers from growing too large.",
"prev_issuer_id", defaultIssuer,
"error", err)
}

return nil
}

Expand Down Expand Up @@ -843,6 +857,11 @@ func (v *VaultProvider) writeNamespaced(namespace string, resource string, data
return v.client.Logical().Write(resource, data)
}

func (v *VaultProvider) deleteNamespaced(namespace string, resource string) (*vaultapi.Secret, error) {
defer v.setNamespace(namespace)()
return v.client.Logical().Delete(resource)
}

func (v *VaultProvider) setNamespace(namespace string) func() {
if namespace != "" {
v.clientMutex.Lock()
Expand Down
40 changes: 34 additions & 6 deletions agent/connect/ca/provider_vault_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -574,12 +574,6 @@ func TestVaultCAProvider_CrossSignCA(t *testing.T) {
run := func(t *testing.T, tc CASigningKeyTypes, withSudo, expectFailure bool) {
t.Parallel()

if tc.SigningKeyType != tc.CSRKeyType {
// TODO: uncomment since the bug is closed
// See https://github.com/hashicorp/vault/issues/7709
t.Skip("Vault doesn't support cross-signing different key types yet.")
}

testVault1 := NewTestVaultServer(t)

attr1 := &VaultTokenAttributes{
Expand Down Expand Up @@ -1195,6 +1189,40 @@ func TestVaultCAProvider_AutoTidyExpiredIssuers(t *testing.T) {
require.Contains(t, errStr, "permission denied")
}

func TestVaultCAProvider_DeletePreviousIssuer(t *testing.T) {
SkipIfVaultNotPresent(t)
t.Parallel()

testVault := NewTestVaultServer(t)
attr := &VaultTokenAttributes{
RootPath: "pki-root",
IntermediatePath: "pki-intermediate",
ConsulManaged: true,
}
token := CreateVaultTokenWithAttrs(t, testVault.client, attr)
provider := createVaultProvider(t, true, testVault.Addr, token,
map[string]any{
"RootPKIPath": "pki-root/",
"IntermediatePKIPath": "pki-intermediate/",
})
res, err := testVault.Client().Logical().List("pki-intermediate/issuers")
require.NoError(t, err)
// Why 2 keys? There is always an initial non-default key that
// gets created before we manage the lifecycle of issuers.
// Since we're asserting that the number of keys don't grow
// this isn't too much of a concern.
require.Len(t, res.Data["keys"], 2)

for i := 0; i < 3; i++ {
_, err := provider.GenerateLeafSigningCert()
require.NoError(t, err)

res, err := testVault.Client().Logical().List("pki-intermediate/issuers")
require.NoError(t, err)
require.Len(t, res.Data["keys"], 2)
}
}

func TestVaultCAProvider_GenerateIntermediate_inSecondary(t *testing.T) {
SkipIfVaultNotPresent(t)

Expand Down

0 comments on commit edcd84e

Please sign in to comment.