Do not delete integration CAs on auth init #51938
Open
+308
−36
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
greedy52
commented
Feb 6, 2025
| @@ -637,6 +639,22 @@ func initializeAuthorities(ctx context.Context, asrv *Server, cfg *InitConfig) e | |||
| return trace.Wrap(err) | |||
| } | |||
|
|
|||
| // Collect CAs from integrations to avoid deleting them. | |||
There was a problem hiding this comment.
I attempted to create a UT for this but keyStore.DeleteUnusedKeys has no effect on software keys. I've also tried test with TELEPORT_TEST_AWS_KMS_ACCOUNT but keyStore.DeleteUnusedKeys does not delete keys created in the last 5 minutes. I might leave as it is without tests
greedy52
commented
Feb 6, 2025
| @@ -132,3 +132,38 @@ func GetIntegrationRef(ctx context.Context, integration string, igGetter Integra | |||
| } | |||
| return ref, nil | |||
| } | |||
|
|
|||
| // GetIntegrationCertAuthorities attempts to retrieve certificate authorities | |||
There was a problem hiding this comment.
not new logic, moved from lib/auth/integration/integrationsv1 to here. added some extra testing tho.
zmb3
reviewed
Feb 7, 2025
zmb3
reviewed
Feb 7, 2025
|
|
||
| // IterateResources is a helper that iterates through each resource from all | ||
| // pages and passes them one by one to the provided callback. | ||
| func IterateResources[T any]( |
There was a problem hiding this comment.
Should we make this a proper go iterator and return an iter.Seq instead of a manual loop with a callback?
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.
Fixes #51920
changelog: Fix an issue that GitHub integration CA gets deleted during Auth restart for non-software key stores like KMS. For broken GitHub integrations, the
integrationresource must be deleted and recreated.