Clean up unused CRL entries when issuer is removed #23007
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
When a issuer is removed, the space utilized by its CRL was not freed, both from the CRL config mapping issuer IDs to CRL IDs and from the CRL storage entry. We thus implement a two step cleanup, wherein orphaned CRL IDs are removed from the config and any remaining full CRL entries are removed from disk. This relates to a Consul<->Vault interop issue (#22980), wherein Consul creates a new issuer on every leadership election, causing this config to grow. Deleting issuers manually does not entirely solve this problem as the config does not fully reclaim space used in this entry. Notably, an observation that when deleting issuers, the CRL was rebuilt on secondary clusters (due to the invalidation not caring about type of the operation); for consistency and to clean up the unified CRLs, we also need to run the rebuild on the active primary cluster that deleted the issuer as well. This approach does allow cleanup on existing impacted clusters by simply rebuilding the CRL. Co-authored-by: Steven Clark <steven.clark@hashicorp.com> Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
github-actions
bot
added
the
hashicorp-contributed-pr
|
CI Results: |
|
Build Results: |
cipherboy
added
backport/1.12.x
backport/1.13.x
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
cipherboy
force-pushed
the
cipherboy-fix-crl-removal
branch
from
62aca49
to
97a359c
Compare
1 task
cipherboy
added
backport/1.13.x
cipherboy
added
backport/1.14.x
This was referenced Sep 12, 2023
This was referenced Sep 13, 2023
VioletHynes
pushed a commit
that referenced
this pull request
Sep 19, 2023
* Clean up unused CRL entries when issuer is removed When a issuer is removed, the space utilized by its CRL was not freed, both from the CRL config mapping issuer IDs to CRL IDs and from the CRL storage entry. We thus implement a two step cleanup, wherein orphaned CRL IDs are removed from the config and any remaining full CRL entries are removed from disk. This relates to a Consul<->Vault interop issue (#22980), wherein Consul creates a new issuer on every leadership election, causing this config to grow. Deleting issuers manually does not entirely solve this problem as the config does not fully reclaim space used in this entry. Notably, an observation that when deleting issuers, the CRL was rebuilt on secondary clusters (due to the invalidation not caring about type of the operation); for consistency and to clean up the unified CRLs, we also need to run the rebuild on the active primary cluster that deleted the issuer as well. This approach does allow cleanup on existing impacted clusters by simply rebuilding the CRL. Co-authored-by: Steven Clark <steven.clark@hashicorp.com> Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com> * Add test case on CRL removal Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com> * Add changelog entry Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com> --------- Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com> Co-authored-by: Steven Clark <steven.clark@hashicorp.com>
VioletHynes
added a commit
that referenced
this pull request
Oct 6, 2023
* VAULT-19237 Add mount_type to secret response * VAULT-19237 changelog * VAULT-19237 make MountType generic * VAULT-19237 clean up comment * VAULT-19237 update changelog * VAULT-19237 update test, remove mounttype from wrapped responses * VAULT-19237 fix a lot of tests * VAULT-19237 standby test * ensure -log-level is added to core config (#23017) * Feature/document tls servername (#22714) * Add Raft TLS Helm examples Co-authored-by: Pascal Reeb <pascal.reeb@adfinis.com> --------- * Clean up unused CRL entries when issuer is removed (#23007) * Clean up unused CRL entries when issuer is removed When a issuer is removed, the space utilized by its CRL was not freed, both from the CRL config mapping issuer IDs to CRL IDs and from the CRL storage entry. We thus implement a two step cleanup, wherein orphaned CRL IDs are removed from the config and any remaining full CRL entries are removed from disk. This relates to a Consul<->Vault interop issue (#22980), wherein Consul creates a new issuer on every leadership election, causing this config to grow. Deleting issuers manually does not entirely solve this problem as the config does not fully reclaim space used in this entry. Notably, an observation that when deleting issuers, the CRL was rebuilt on secondary clusters (due to the invalidation not caring about type of the operation); for consistency and to clean up the unified CRLs, we also need to run the rebuild on the active primary cluster that deleted the issuer as well. This approach does allow cleanup on existing impacted clusters by simply rebuilding the CRL. Co-authored-by: Steven Clark <steven.clark@hashicorp.com> Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com> * Add test case on CRL removal Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com> * Add changelog entry Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com> --------- Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com> Co-authored-by: Steven Clark <steven.clark@hashicorp.com> * UI: Handle control group error on SSH (#23025) * Handle control group error on SSH * Add changelog * Fix enterprise failure of TestCRLIssuerRemoval (#23038) This fixes the enterprise failure of the test ``` === FAIL: builtin/logical/pki TestCRLIssuerRemoval (0.00s) crl_test.go:1456: Error Trace: /home/runner/actions-runner/_work/vault-enterprise/vault-enterprise/builtin/logical/pki/crl_test.go:1456 Error: Received unexpected error: Global, cross-cluster revocation queue cannot be enabled when auto rebuilding is disabled as the local cluster may not have the certificate entry! Test: TestCRLIssuerRemoval Messages: failed enabling unified CRLs on enterprise ``` * fix LDAP auto auth changelog (#23027) * VAULT-19233 First part of caching static secrets work * VAULT-19233 update godoc * VAULT-19233 invalidate cache on non-GET * VAULT-19233 add locking to proxy cache writes * VAULT-19233 add caching of capabilities map, and some additional test coverage * VAULT-19233 Additional testing * VAULT-19233 namespaces for cache ids * VAULT-19233 cache-clear testing and implementation * VAULT-19233 adjust format, add more tests * VAULT-19233 some more docs * VAULT-19233 Add RLock holding for map access * VAULT-19233 PR comments * VAULT-19233 Different table for capabilities indexes * VAULT-19233 keep unique for request path * VAULT-19233 passthrough for non-v1 requests * VAULT-19233 some renames/PR comment updates * VAULT-19233 remove type from capabilities index * VAULT-19233 remove obsolete capabilities * VAULT-19233 remove erroneous capabilities * VAULT-19233 woops, missed a test * VAULT-19233 typo * VAULT-19233 add custom error for cachememdb * VAULT-19233 fix cachememdb test --------- Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com> Co-authored-by: Chris Capurso <1036769+ccapurso@users.noreply.github.com> Co-authored-by: Andreas Gruhler <andreas.gruhler@adfinis.com> Co-authored-by: Alexander Scheel <alex.scheel@hashicorp.com> Co-authored-by: Steven Clark <steven.clark@hashicorp.com> Co-authored-by: Chelsea Shaw <82459713+hashishaw@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When a issuer is removed, the space utilized by its CRL was not freed, both from the CRL config mapping issuer IDs to CRL IDs and from the CRL storage entry. We thus implement a two step cleanup, wherein orphaned CRL IDs are removed from the config and any remaining full CRL entries are removed from disk.
This relates to a Consul<->Vault interop issue (#22980), wherein Consul creates a new issuer on every leadership election, causing this config to grow. Deleting issuers manually does not entirely solve this problem as the config does not fully reclaim space used in this entry.
Notably, an observation that when deleting issuers, the CRL was rebuilt on secondary clusters (due to the invalidation not caring about type of the operation); for consistency and to clean up the unified CRLs, we also need to run the rebuild on the active primary cluster that deleted the issuer as well.
This approach does allow cleanup on existing impacted clusters by simply rebuilding the CRL.
TODO: