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

📖 [Docs] Document the granting of API access #1407

Merged
merged 18 commits into from
Nov 7, 2024

Conversation

bentito
Copy link
Contributor

@bentito bentito commented Oct 24, 2024

Description

When operators or cluster extensions are installed through OLM, they often provide Custom Resource Definitions (CRDs) that expose new API resources. By default, cluster administrators may have full access to manage these resources, but non-admin users often require specific permissions to create, view, or edit these custom resource objects.

This PR adds a document to the Concepts dir to show how this works.

Fixes #1383

Reviewer Checklist

  • API Go Documentation
  • Tests: Unit Tests (and E2E Tests, if appropriate)
  • Comprehensive Commit Messages
  • Links to related GitHub Issue(s)

Signed-off-by: Brett Tofel <[email protected]>
@bentito bentito requested a review from a team as a code owner October 24, 2024 19:55
Copy link

netlify bot commented Oct 24, 2024

Deploy Preview for olmv1 ready!

Name Link
🔨 Latest commit 99d27c4
🔍 Latest deploy log https://app.netlify.com/sites/olmv1/deploys/672cd4df92a4140008912e69
😎 Deploy Preview https://deploy-preview-1407--olmv1.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

codecov bot commented Oct 24, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 73.26%. Comparing base (1cb254c) to head (99d27c4).
Report is 26 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1407      +/-   ##
==========================================
- Coverage   73.43%   73.26%   -0.18%     
==========================================
  Files          42       42              
  Lines        3200     3063     -137     
==========================================
- Hits         2350     2244     -106     
+ Misses        663      644      -19     
+ Partials      187      175      -12     
Flag Coverage Δ
e2e 54.91% <ø> (-0.12%) ⬇️
unit 52.56% <ø> (-0.72%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Signed-off-by: Brett Tofel <[email protected]>
docs/concepts/how-to-grant-api-access.md Outdated Show resolved Hide resolved
@LalatenduMohanty
Copy link
Member

Fixes #1383

Signed-off-by: Brett Tofel <[email protected]>
@joelanford
Copy link
Member

I advocate for telling the story like this:

  1. Here's how you find the groups and resources from the name of a ClusterExtension
  2. Here's how you build view/edit/admin roles for those groups/resources
  3. Here's how to bind those roles to users (or use role aggregation)

Right now, we have (1) at the end of the doc, and I kind of think that's burying the lead.

Signed-off-by: Brett Tofel <[email protected]>
@bentito
Copy link
Contributor Author

bentito commented Nov 4, 2024

@joelanford check out the reordered document

@camilamacedo86

This comment was marked as outdated.

@openshift-ci openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 6, 2024
@camilamacedo86
Copy link
Contributor

/lgtm cancel

to allow others review it

@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Nov 6, 2024
@camilamacedo86

This comment was marked as outdated.

@openshift-ci openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 6, 2024
Copy link
Contributor

@michaelryanpeter michaelryanpeter left a comment

Choose a reason for hiding this comment

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

I left a few suggestions. Please feel free to take what is useful.

Please remember to update the mkdocs.yaml so that the content appears in the table of contents.

Lastly, I would format the procedures a little differently, but this is not a blocking issue. After the PR merges, I will open a follow up PR to show you what I have in mind, and maybe we can use that to make a template for future use.

Great work!

docs/concepts/how-to-grant-api-access.md Outdated Show resolved Hide resolved
docs/concepts/how-to-grant-api-access.md Outdated Show resolved Hide resolved
docs/concepts/how-to-grant-api-access.md Outdated Show resolved Hide resolved
docs/concepts/how-to-grant-api-access.md Outdated Show resolved Hide resolved
mkdocs.yml Outdated Show resolved Hide resolved
camilamacedo86

This comment was marked as off-topic.

@jianzhangbjz
Copy link

cc: @kuiwang02 ^^

@joelanford joelanford enabled auto-merge November 7, 2024 17:16
@joelanford joelanford dismissed LalatenduMohanty’s stale review November 7, 2024 17:17

This review comment has been addressed.

@joelanford joelanford added this pull request to the merge queue Nov 7, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Nov 7, 2024
@bentito bentito added this pull request to the merge queue Nov 7, 2024
Merged via the queue into operator-framework:main with commit 8d535a6 Nov 7, 2024
17 of 18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Docs: Authorization - Granting user access to API resources
8 participants