Skip to content

[v17] Update second_factors docs for v16 compatibility issue#54909

Merged
Joerger merged 2 commits intobranch/v17from
joerger/v17/second-factors-warning-docs
May 21, 2025
Merged

[v17] Update second_factors docs for v16 compatibility issue#54909
Joerger merged 2 commits intobranch/v17from
joerger/v17/second-factors-warning-docs

Conversation

@Joerger
Copy link
Copy Markdown
Contributor

@Joerger Joerger commented May 16, 2025

Docs followup to #54735

Like its predecessor, this PR is straight for v17.

@Joerger Joerger requested review from espadolini and hugoShaka May 16, 2025 23:26
@Joerger Joerger added the no-changelog Indicates that a PR does not require a changelog entry label May 16, 2025
@github-actions github-actions Bot requested a review from marcoandredinis May 16, 2025 23:26
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 16, 2025

Amplify deployment status

Branch Commit Job ID Status Preview Updated (UTC)
joerger/v17/second-factors-warning-docs 7de7e6c 2 ✅SUCCEED joerger-v17-second-factors-warning-docs 2025-05-21 18:18:51

// 1 is "otp", 2 is "webauthn", 3 is "sso",
// If unspecified, the current default value is [1], or ["otp"].
//
// WARNING: only set SecondFactors if your cluster is fully upgraded to v17+.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
// WARNING: only set SecondFactors if your cluster is fully upgraded to v17+.
// WARNING: only set second_factors if your cluster is fully upgraded to v17+.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Upper camel case is intentional here

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This ends up populating user facing documentation in the terraform docs tho.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I see how that is awkward, but if anything that points to this type of docs generation being non-ideal. We would need to change line 2296 as well to get the right output for terraform docs:

second_factors (List of Number) SecondFactors is a list of supported multi-factor types...

@public-teleport-github-review-bot public-teleport-github-review-bot Bot removed the request for review from hugoShaka May 19, 2025 07:59
@Joerger Joerger added this pull request to the merge queue May 21, 2025
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to failed status checks May 21, 2025
@Joerger Joerger enabled auto-merge May 21, 2025 18:12
@Joerger Joerger added this pull request to the merge queue May 21, 2025
Merged via the queue into branch/v17 with commit 2ab2a9f May 21, 2025
42 checks passed
@Joerger Joerger deleted the joerger/v17/second-factors-warning-docs branch May 21, 2025 18:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport documentation no-changelog Indicates that a PR does not require a changelog entry size/sm

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants