Skip to content

Fix cleanup of unused GCP KMS keys#34052

Merged
nklaassen merged 1 commit intomasterfrom
nklaassen/fix-kms-deleteunusedkeys
Nov 10, 2023
Merged

Fix cleanup of unused GCP KMS keys#34052
nklaassen merged 1 commit intomasterfrom
nklaassen/fix-kms-deleteunusedkeys

Conversation

@nklaassen
Copy link
Copy Markdown
Contributor

@nklaassen nklaassen commented Oct 30, 2023

Fixes #31375

This PR fixes the cleanup code for unused keys in GCP KMS which is currently failing to delete unused keys and emitting confusing warning logs whenever keys have been generated by multiple different Auth servers (no actual functionality is currently broken).

This bug was introduced in #25025. This issue introduced there is that the function now checks that all currently active keys have actually been found in the keyring, but the ListCryptoKeys call used a filter that excluded all keys created by different Auth servers.

This fix improves some of the error messages, and also uses a more permissive filter in the ListCryptoKeys call to make sure we can list keys created by any auth server, but will only delete keys created by the local auth server.

changelog: Fix cleanup of unused GCP KMS keys

@github-actions
Copy link
Copy Markdown
Contributor

The PR changelog entry failed validation: Changelog entry not found in the PR body. Please add a "no-changelog" label to the PR, or changelog lines starting with changelog: followed by the changelog entries for the PR.

@github-actions
Copy link
Copy Markdown
Contributor

The PR changelog entry failed validation: Changelog entry not found in the PR body. Please add a "no-changelog" label to the PR, or changelog lines starting with changelog: followed by the changelog entries for the PR.

@nklaassen nklaassen force-pushed the nklaassen/fix-kms-deleteunusedkeys branch from 9d499a2 to 6f14411 Compare October 30, 2023 23:56
@github-actions
Copy link
Copy Markdown
Contributor

The PR changelog entry failed validation: Changelog entry not found in the PR body. Please add a "no-changelog" label to the PR, or changelog lines starting with changelog: followed by the changelog entries for the PR.

Comment thread lib/auth/keystore/gcp_kms.go
Comment thread lib/auth/keystore/gcp_kms.go
Comment thread lib/auth/keystore/gcp_kms_test.go Outdated
Copy link
Copy Markdown
Contributor

@codingllama codingllama left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only minor comments, looks good.

Comment thread lib/auth/keystore/gcp_kms.go Outdated
Comment thread lib/auth/keystore/gcp_kms.go
Comment thread lib/auth/keystore/gcp_kms.go
Comment thread lib/auth/keystore/gcp_kms.go
Comment thread lib/auth/keystore/gcp_kms.go Outdated
Comment thread lib/auth/keystore/gcp_kms_test.go Outdated
This PR fixes the cleanup code for unused keys in GCP KMS which is
currently failing to delete unused keys and emitting confusing warning
logs whenever keys have been generated by multiple different Auth
servers (no actual functionality is currently broken).

This bug was introduced in
#25025. This issue
introduced there is that the function now checks that all currently
active keys have actually been found in the keyring, but the
ListCryptoKeys call used a filter that excluded all keys created by
different Auth servers.

This fix improves some of the error messages, and also uses a more
permissive filter in the ListCryptoKeys call to make sure we can list
keys created by any auth server, but will only delete keys created by
the local auth server.

changelog: Fix cleanup of unused GCP KMS keys
@nklaassen nklaassen force-pushed the nklaassen/fix-kms-deleteunusedkeys branch from 3f8e328 to 41c55b0 Compare November 10, 2023 19:58
@nklaassen nklaassen enabled auto-merge November 10, 2023 19:58
@nklaassen nklaassen added this pull request to the merge queue Nov 10, 2023
Merged via the queue into master with commit 3994ff3 Nov 10, 2023
@nklaassen nklaassen deleted the nklaassen/fix-kms-deleteunusedkeys branch November 10, 2023 20:33
@public-teleport-github-review-bot
Copy link
Copy Markdown

@nklaassen See the table below for backport results.

Branch Result
branch/v12 Create PR
branch/v13 Create PR
branch/v14 Create PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

GCP KMS "cannot find currently active CA key"

5 participants