Skip to content
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

Add client-side functions to export multiple authorities #51189

Merged
merged 6 commits into from
Jan 22, 2025

Conversation

codingllama
Copy link
Contributor

Add "ExportAll" variants of ExportAuthorities and ExportAuthoritiesSecrets that can gracefully handle multiple active CAs.

ExportAll functions return an []*ExportedAuthority, so future iterations could easily include (and differentiate) CertAuthoritySpecV2.AdditionalTrustedKeys, plus whatever other data is necessary.

Subsequent PRs will take advantage of the new functions on both tctl and Web API. After the follow-ups the "unary" Export functions are to be removed.

Similar to #35754 (minus the frontend parts).

#35444

@codingllama codingllama added the no-changelog Indicates that a PR does not require a changelog entry label Jan 17, 2025
@codingllama
Copy link
Contributor Author

May be reviewed as a whole or commit-by-commit, as preferred. Commit 41ff204 refactors existing tests without changing functionality, so I suggest taking a look at it regardless. There are no changes in the test table itself (apart from ident), it only removes the outer loop in favor of a couple of explicit t.Run calls (which we built into in the next commits).

@codingllama
Copy link
Contributor Author

FYI @GavinFrazar, this tackles the same issues as #35754 (only I'll do the "frontend" parts in a follow up).

@codingllama
Copy link
Contributor Author

Optimistically adding backport labels. Once I mail the tctl/Web API parts we'll see how far back this will actually go.

@codingllama
Copy link
Contributor Author

Friendly ping @eriktate @hugoShaka

@codingllama codingllama force-pushed the codingllama/export-all-funcs branch from 3a27791 to ac7524a Compare January 21, 2025 17:18
@codingllama
Copy link
Contributor Author

Rebased onto master, no changes.

Copy link
Contributor

@hugoShaka hugoShaka left a comment

Choose a reason for hiding this comment

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

Sounds good, it took me some time to understand that ExportAllAuthorities did not export authorities of all kinds, but all authorities of a kind. It might be useful to drop a few words in the godocs to remove the ambiguity and help Teleporters who have no prior CA export context.

Also, I'm curious about which property an integration CA has, compared to a regular one. Linking to a RFD, PR, or godoc explaining the concept would help understand why they need special treatment.

@codingllama
Copy link
Contributor Author

Thanks, Hugo!

Sounds good, it took me some time to understand that ExportAllAuthorities did not export authorities of all kinds, but all authorities of a kind. It might be useful to drop a few words in the godocs to remove the ambiguity and help Teleporters who have no prior CA export context.

Done! (fa41436)

Also, I'm curious about which property an integration CA has, compared to a regular one. Linking to a RFD, PR, or godoc explaining the concept would help understand why they need special treatment.

Replied on #51189 (comment).

lib/client/ca_export.go Outdated Show resolved Hide resolved
@public-teleport-github-review-bot public-teleport-github-review-bot bot removed the request for review from eriktate January 21, 2025 21:33
@codingllama codingllama enabled auto-merge January 21, 2025 21:51
@codingllama
Copy link
Contributor Author

Thanks, everyone!

@codingllama codingllama added this pull request to the merge queue Jan 21, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to no response for status checks Jan 21, 2025
@codingllama codingllama added this pull request to the merge queue Jan 22, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jan 22, 2025
@codingllama codingllama added this pull request to the merge queue Jan 22, 2025
Merged via the queue into master with commit e442174 Jan 22, 2025
42 checks passed
@codingllama codingllama deleted the codingllama/export-all-funcs branch January 22, 2025 15:44
@public-teleport-github-review-bot

@codingllama See the table below for backport results.

Branch Result
branch/v15 Failed
branch/v16 Failed
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.

3 participants