-
Notifications
You must be signed in to change notification settings - Fork 428
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
Clean up Azure AD Workload ID docs #4776
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #4776 +/- ##
=======================================
Coverage 62.48% 62.48%
=======================================
Files 196 196
Lines 16188 16188
=======================================
Hits 10115 10115
Misses 5333 5333
Partials 740 740 ☔ View full report in Codecov by Sentry. |
/hold So I don't forget to squash after reviewing. |
50deca7
to
3baf8e5
Compare
/hold cancel Squashed, awaiting further review. |
Just had one more comment in case you missed it, otherwise lgtm! #4776 (comment) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
LGTM label has been added. Git tree hash: a5e1d9e6e6136622812183821f2c46d956a4c655
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
/approve
- Edit the generated `azwi-quickstart.yaml` to make the following changes for | ||
workload identity to the `AzureClusterIdentity` object. | ||
workload identity to the `AzureClusterIdentity` object: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just noting here that this step won't be necessary after #4765 merges since that changes all the templates to use WorkloadIdentity by default. I don't see this causing any issues though even after that, but would be some easy cleanup.
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: nojnhuh The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/kind documentation
What this PR does / why we need it:
Updates the Azure AD Workload ID doc for better formatting and tighter grammar.
Which issue(s) this PR fixes:
Refs #4138
Special notes for your reviewer:
There are probably more substantial changes to be made here to make this an easier document to follow, but I wanted to start by fixing the obvious things, mostly Markdown formatting.
TODOs:
Release note: