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

WIP: ✨ Feature/permission preflight #1282

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

everettraven
Copy link
Contributor

Description

Reviewer Checklist

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

@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Sep 18, 2024
Copy link

netlify bot commented Sep 18, 2024

Deploy Preview for olmv1 ready!

Name Link
🔨 Latest commit d9c105c
🔍 Latest deploy log https://app.netlify.com/sites/olmv1/deploys/66eb12ff0ef23a0008bc7a81
😎 Deploy Preview https://deploy-preview-1282--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.

@everettraven
Copy link
Contributor Author

Note: preflight checks are run after a helm dry run, so there may be cases where permission errors are returned from our helm use before we ever hit the preflight checks. I'm not sure there is much way around this, but I haven't spent too much time thinking on this. Open to suggestions!

@everettraven
Copy link
Contributor Author

everettraven commented Sep 18, 2024

Also, with this change the new status message for insufficient permissions from the preflight check looks something like:

- lastTransitionTime: "2024-09-18T14:52:12Z"
      message: |-
        not permitted to "create" /v1, Resource=serviceaccounts
        not permitted to "update" /v1, Resource=serviceaccounts
        not permitted to "patch" /v1, Resource=serviceaccounts
        not permitted to "delete" /v1, Resource=serviceaccounts
        not permitted to "list" /v1, Resource=serviceaccounts
        not permitted to "watch" /v1, Resource=serviceaccounts
        not permitted to "create" /v1, Resource=configmaps
        not permitted to "update" /v1, Resource=configmaps
        not permitted to "patch" /v1, Resource=configmaps
        not permitted to "delete" /v1, Resource=configmaps
        not permitted to "list" /v1, Resource=configmaps
        not permitted to "watch" /v1, Resource=configmaps
        not permitted to "create" apiextensions.k8s.io/v1, Resource=customresourcedefinitions
        not permitted to "update" apiextensions.k8s.io/v1, Resource=customresourcedefinitions
        not permitted to "patch" apiextensions.k8s.io/v1, Resource=customresourcedefinitions
        not permitted to "delete" apiextensions.k8s.io/v1, Resource=customresourcedefinitions
        not permitted to "list" apiextensions.k8s.io/v1, Resource=customresourcedefinitions
        not permitted to "watch" apiextensions.k8s.io/v1, Resource=customresourcedefinitions
        not permitted to "create" apiextensions.k8s.io/v1, Resource=customresourcedefinitions
        not permitted to "update" apiextensions.k8s.io/v1, Resource=customresourcedefinitions
        not permitted to "patch" apiextensions.k8s.io/v1, Resource=customresourcedefinitions
        not permitted to "delete" apiextensions.k8s.io/v1, Resource=customresourcedefinitions
        not permitted to "list" apiextensions.k8s.io/v1, Resource=customresourcedefinitions
        not permitted to "watch" apiextensions.k8s.io/v1, Resource=customresourcedefinitions
        not permitted to "create" apiextensions.k8s.io/v1, Resource=customresourcedefinitions
        not permitted to "update" apiextensions.k8s.io/v1, Resource=customresourcedefinitions
        not permitted to "patch" apiextensions.k8s.io/v1, Resource=customresourcedefinitions
        not permitted to "delete" apiextensions.k8s.io/v1, Resource=customresourcedefinitions
        not permitted to "list" apiextensions.k8s.io/v1, Resource=customresourcedefinitions
        not permitted to "watch" apiextensions.k8s.io/v1, Resource=customresourcedefinitions
        not permitted to "create" apiextensions.k8s.io/v1, Resource=customresourcedefinitions
        not permitted to "update" apiextensions.k8s.io/v1, Resource=customresourcedefinitions
        not permitted to "patch" apiextensions.k8s.io/v1, Resource=customresourcedefinitions
        not permitted to "delete" apiextensions.k8s.io/v1, Resource=customresourcedefinitions
        not permitted to "list" apiextensions.k8s.io/v1, Resource=customresourcedefinitions
        not permitted to "watch" apiextensions.k8s.io/v1, Resource=customresourcedefinitions
        not permitted to "create" apiextensions.k8s.io/v1, Resource=customresourcedefinitions
        not permitted to "update" apiextensions.k8s.io/v1, Resource=customresourcedefinitions
        not permitted to "patch" apiextensions.k8s.io/v1, Resource=customresourcedefinitions
        not permitted to "delete" apiextensions.k8s.io/v1, Resource=customresourcedefinitions
        not permitted to "list" apiextensions.k8s.io/v1, Resource=customresourcedefinitions
        not permitted to "watch" apiextensions.k8s.io/v1, Resource=customresourcedefinitions
        not permitted to "create" rbac.authorization.k8s.io/v1, Resource=clusterroles
        not permitted to "update" rbac.authorization.k8s.io/v1, Resource=clusterroles
        not permitted to "patch" rbac.authorization.k8s.io/v1, Resource=clusterroles
        not permitted to "delete" rbac.authorization.k8s.io/v1, Resource=clusterroles
        not permitted to "list" rbac.authorization.k8s.io/v1, Resource=clusterroles
        not permitted to "watch" rbac.authorization.k8s.io/v1, Resource=clusterroles
        not permitted to "create" rbac.authorization.k8s.io/v1, Resource=clusterroles
        not permitted to "update" rbac.authorization.k8s.io/v1, Resource=clusterroles
        not permitted to "patch" rbac.authorization.k8s.io/v1, Resource=clusterroles
        not permitted to "delete" rbac.authorization.k8s.io/v1, Resource=clusterroles
        not permitted to "list" rbac.authorization.k8s.io/v1, Resource=clusterroles
        not permitted to "watch" rbac.authorization.k8s.io/v1, Resource=clusterroles
        not permitted to "create" rbac.authorization.k8s.io/v1, Resource=clusterroles
        not permitted to "update" rbac.authorization.k8s.io/v1, Resource=clusterroles
        not permitted to "patch" rbac.authorization.k8s.io/v1, Resource=clusterroles
        not permitted to "delete" rbac.authorization.k8s.io/v1, Resource=clusterroles
        not permitted to "list" rbac.authorization.k8s.io/v1, Resource=clusterroles
        not permitted to "watch" rbac.authorization.k8s.io/v1, Resource=clusterroles
        not permitted to "create" rbac.authorization.k8s.io/v1, Resource=clusterrolebindings
        not permitted to "update" rbac.authorization.k8s.io/v1, Resource=clusterrolebindings
        not permitted to "patch" rbac.authorization.k8s.io/v1, Resource=clusterrolebindings
        not permitted to "delete" rbac.authorization.k8s.io/v1, Resource=clusterrolebindings
        not permitted to "list" rbac.authorization.k8s.io/v1, Resource=clusterrolebindings
        not permitted to "watch" rbac.authorization.k8s.io/v1, Resource=clusterrolebindings
        not permitted to "create" rbac.authorization.k8s.io/v1, Resource=clusterrolebindings
        not permitted to "update" rbac.authorization.k8s.io/v1, Resource=clusterrolebindings
        not permitted to "patch" rbac.authorization.k8s.io/v1, Resource=clusterrolebindings
        not permitted to "delete" rbac.authorization.k8s.io/v1, Resource=clusterrolebindings
        not permitted to "list" rbac.authorization.k8s.io/v1, Resource=clusterrolebindings
        not permitted to "watch" rbac.authorization.k8s.io/v1, Resource=clusterrolebindings
        not permitted to "create" /v1, Resource=services
        not permitted to "update" /v1, Resource=services
        not permitted to "patch" /v1, Resource=services
        not permitted to "delete" /v1, Resource=services
        not permitted to "list" /v1, Resource=services
        not permitted to "watch" /v1, Resource=services
        not permitted to "create" apps/v1, Resource=deployments
        not permitted to "update" apps/v1, Resource=deployments
        not permitted to "patch" apps/v1, Resource=deployments
        not permitted to "delete" apps/v1, Resource=deployments
        not permitted to "list" apps/v1, Resource=deployments
        not permitted to "watch" apps/v1, Resource=deployments
      observedGeneration: 2
      reason: Failed
      status: "False"
      type: Installed

(I still have some work to do to de-duplicate the messages)

Copy link

codecov bot commented Sep 18, 2024

Codecov Report

Attention: Patch coverage is 26.92308% with 38 lines in your changes missing coverage. Please review.

Project coverage is 75.42%. Comparing base (06424ef) to head (d9c105c).
Report is 22 commits behind head on main.

Files with missing lines Patch % Lines
...ernal/rukpak/preflights/permissions/permissions.go 21.27% 37 Missing ⚠️
internal/applier/helm.go 50.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1282      +/-   ##
==========================================
- Coverage   76.49%   75.42%   -1.07%     
==========================================
  Files          39       40       +1     
  Lines        2361     2409      +48     
==========================================
+ Hits         1806     1817      +11     
- Misses        389      426      +37     
  Partials      166      166              
Flag Coverage Δ
e2e 57.20% <25.00%> (-0.70%) ⬇️
unit 51.84% <3.84%> (-1.19%) ⬇️

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.

@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 28, 2024
@openshift-merge-robot
Copy link

PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants