-
Notifications
You must be signed in to change notification settings - Fork 11
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
feat: pepr store crd applied through kfc #580
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Case Wylie <cmwylie19@defenseunicorns.com>
Signed-off-by: Case Wylie <cmwylie19@defenseunicorns.com>
Signed-off-by: Case Wylie <cmwylie19@defenseunicorns.com>
Signed-off-by: Case Wylie <cmwylie19@defenseunicorns.com>
Signed-off-by: Case Wylie <cmwylie19@defenseunicorns.com>
cmwylie19
requested review from
jeff-mccoy,
btlghrants and
schaeferka
as code owners
February 16, 2024 13:40
cmwylie19
commented
Feb 16, 2024
Signed-off-by: Case Wylie <cmwylie19@defenseunicorns.com>
Signed-off-by: Case Wylie <cmwylie19@defenseunicorns.com>
Signed-off-by: Case Wylie <cmwylie19@defenseunicorns.com>
schaeferka
previously approved these changes
Feb 16, 2024
schaeferka
approved these changes
Feb 16, 2024
cmwylie19
added a commit
that referenced
this pull request
Mar 14, 2024
## Description #577 UDS ran into this problem: ``` Helm Problem Each Pepr module creates the CRD for PeprStore. A problem arrises when multiple Pepr modules are packaged in a helm chart. Helm will not render resources that already exist. ``` We fixed it in #580 which used the KFC to ServerSide Apply the PeprStore `CustomResourceDefinition`. This led to a regression when building the pepr moduled with scoped rbac `npx pepr build --rbac-mode=scoped` ```json { "level": 50, "time": 1710435728180, "pid": 1, "hostname": "pepr-6e43c347-0370-5954-bda3-552d74a5e3bd-6f8ddbb9dd-fvjrf", "data": { "kind": "Status", "apiVersion": "v1", "metadata": {}, "status": "Failure", "message": "customresourcedefinitions.apiextensions.k8s.io \"peprstores.pepr.dev\" is forbidden: ", "reason": "Forbidden", "details": { "name": "peprstores.pepr.dev", "group": "apiextensions.k8s.io", "kind": "customresourcedefinitions" }, "code": 403 }, "ok": false, "status": 403, "statusText": "Forbidden" } { "level": 50, "time": 1710435597286, "pid": 1, "hostname": "pepr-6e43c347-0370-5954-bda3-552d74a5e3bd-6f8ddbb9dd-2q5vr", "data": { "kind": "Status", "apiVersion": "v1", "metadata": {}, "status": "Failure", "message": "customresourcedefinitions.apiextensions.k8s.io \"peprstores.pepr.dev\" is forbidden: User \"system:serviceaccount:pepr-system:pepr-6e43c347-0370-5954-bda3-552d74a5e3bd\" cannot patch resource \"customresourcedefinitions\" in API group \"apiextensions.k8s.io\" at the cluster scope", "reason": "Forbidden", "details": { "name": "peprstores.pepr.dev", "group": "apiextensions.k8s.io", "kind": "customresourcedefinitions" }, "code": 403 }, ``` The code that generates the RBAC did not take into account this new criteria that it needed permissions to `patch`,`update` `CustomResourceDefinitions`. This PR adds the necessary code so that RBAC mode gives the Pepr service account appropriate permissions to do the job issue: Pods do not come up, permanent `CrashLoopBackOff` ```bash ┌─[cmwylie19@Cases-MacBook-Pro] - [~/deadass] - [2024-03-14 12:57:06] └─[0] <git:(main✈) > k get po -n pepr-system NAME READY STATUS RESTARTS AGE pepr-6e43c347-0370-5954-bda3-552d74a5e3bd-6f8ddbb9dd-shj6j 0/1 CrashLoopBackOff 1 (3s ago) 8s pepr-6e43c347-0370-5954-bda3-552d74a5e3bd-6f8ddbb9dd-mndd6 0/1 CrashLoopBackOff 1 (3s ago) 8s ``` ## Related Issue Fixes # <!-- or --> Relates to # ## Type of change - [x] Bug fix (non-breaking change which fixes an issue) - [ ] New feature (non-breaking change which adds functionality) - [ ] Other (security config, docs update, etc) ## Checklist before merging - [x] Test, docs, adr added or updated as needed - [x] [Contributor Guide Steps](https://github.com/defenseunicorns/pepr/blob/main/CONTRIBUTING.md#submitting-a-pull-request) followed --------- Signed-off-by: Case Wylie <cmwylie19@defenseunicorns.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
KFC to apply the PeprStore CRD to enable multiple Pepr modules in a helm package.
I considered creating an ownerRef on the PeprStore CRD but then it became unclear which module would own the CRD. Since the point of this issue is to be able to deploy multiple modules it did not make sense. Although, it is not best practice to deploy multiple Pepr modules. Modules should be consolidated where possible https://docs.pepr.dev/main/best-practices/#multiple-modules-or-multiple-capabilities
Related Issue
Fixes #577
Relates to #
Type of change
Checklist before merging