Skip to content

only preload local cluster cas #48483

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

Merged
merged 1 commit into from
Nov 6, 2024

Conversation

fspmarshall
Copy link
Contributor

The original implementation of the client TLS config generator tried to always pre-generate all TLS configs for all known CAs on all changes. This was nice in theory, but created spurious error logs due to remote clusters not having all requisite CAs stored, and was wasted effort in many cases.

This PR tweaks the event-handling logic of the client TLS config generator to only aggressively generate TLS configs in response to modification events for the local cluster. For all other clusters, it invalidates any now stale state, but waits until a connection actually comes in before nothing to try to build the TLS config.

Fixes #48331

Copy link

This pull request is automatically being deployed by Amplify Hosting (learn more).

Access this pull request here: https://pr-48483.d3pp5qlev8mo18.amplifyapp.com

@fspmarshall fspmarshall added no-changelog Indicates that a PR does not require a changelog entry backport/branch/v15 backport/branch/v16 backport/branch/v17 labels Nov 5, 2024
bl-nero

This comment was marked as duplicate.

Copy link
Contributor

@bl-nero bl-nero left a comment

Choose a reason for hiding this comment

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

Sorry for duplicate comments, but I somehow managed to mishandle them: Consider also updating the watchForCAChanges documentation comment, as it's now outdated.

Copy link
Member

@ravicious ravicious left a comment

Choose a reason for hiding this comment

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

Approving because the PR description and the code seem to make sense, though I know very little about how we handle CAs.

@fspmarshall fspmarshall force-pushed the fspmarshall/only-preload-local-cas branch from 4d7bf1f to 5c96d26 Compare November 6, 2024 15:16
@fspmarshall fspmarshall enabled auto-merge November 6, 2024 15:18
@fspmarshall fspmarshall force-pushed the fspmarshall/only-preload-local-cas branch from 5c96d26 to 3eca95b Compare November 6, 2024 15:59
@fspmarshall fspmarshall added this pull request to the merge queue Nov 6, 2024
Merged via the queue into master with commit d809f95 Nov 6, 2024
40 checks passed
@fspmarshall fspmarshall deleted the fspmarshall/only-preload-local-cas branch November 6, 2024 16:34
@public-teleport-github-review-bot

@fspmarshall See the table below for backport results.

Branch Result
branch/v15 Create PR
branch/v16 Create PR
branch/v17 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.

Trusted Cluster failed to retrieve client cert pool
3 participants