Add AccessPolicies status #524
Conversation
WalkthroughThe changes involve enhancements to the Kubernetes API definitions and associated logic for managing access policies across various resources. New fields and print columns for access policies have been added to multiple structs and YAML configurations, improving the representation and reporting of access control information. Additionally, modifications to reconciler methods streamline the handling of application and job statuses based on access policy validity. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ApplicationReconciler
participant SKIPJobReconciler
participant API
User->>API: Request Application List
API->>ApplicationReconciler: Fetch Application Data
ApplicationReconciler->>ApplicationReconciler: Check Access Policies
ApplicationReconciler->>API: Return Application List with AccessPolicies
User->>API: Request SKIPJob Status
API->>SKIPJobReconciler: Fetch SKIPJob Data
SKIPJobReconciler->>SKIPJobReconciler: Validate Access Policies
SKIPJobReconciler->>API: Return SKIPJob Status with AccessPolicies
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (1)
tests/application/access-policy/no-policy-assert.yaml (1)
17-17: Add a newline at the end of the file.Adding a newline at the end of the file is a best practice to avoid potential issues with version control systems and to adhere to POSIX standards.
Apply this diff to add a newline at the end of the file:
status: accessPolicies: Ready +Tools
yamllint
[error] 17-17: no new line character at the end of file
(new-line-at-end-of-file)
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (14)
- api/v1alpha1/application_types.go (1 hunks)
- api/v1alpha1/skipjob_types.go (2 hunks)
- api/v1alpha1/status_types.go (3 hunks)
- config/crd/skiperator.kartverket.no_applications.yaml (3 hunks)
- config/crd/skiperator.kartverket.no_routings.yaml (2 hunks)
- config/crd/skiperator.kartverket.no_skipjobs.yaml (3 hunks)
- internal/controllers/application.go (2 hunks)
- internal/controllers/skipjob.go (1 hunks)
- tests/application/access-policy/advanced-assert.yaml (1 hunks)
- tests/application/access-policy/bad-policy-assert.yaml (1 hunks)
- tests/application/access-policy/no-policy-assert.yaml (1 hunks)
- tests/skipjob/access-policy-job/application-assert.yaml (1 hunks)
- tests/skipjob/access-policy-job/skipjob-assert.yaml (1 hunks)
- tests/skipjob/conditions/skipjob-assert.yaml (3 hunks)
Additional context used
yamllint
tests/application/access-policy/no-policy-assert.yaml
[error] 17-17: no new line character at the end of file
(new-line-at-end-of-file)
Additional comments not posted (27)
tests/application/access-policy/no-policy-assert.yaml (1)
16-17: LGTM!The addition of the
statussection with theaccessPoliciesfield enhances the semantic meaning of the configuration.Tools
yamllint
[error] 17-17: no new line character at the end of file
(new-line-at-end-of-file)
tests/skipjob/access-policy-job/application-assert.yaml (1)
8-9: LGTM!The addition of the
statussection with theaccessPoliciesfield enhances the semantic meaning of the configuration.tests/skipjob/conditions/skipjob-assert.yaml (3)
6-6: LGTM!The addition of the
accessPoliciesfield enhances the semantic meaning of the configuration.
22-22: LGTM!The addition of the
accessPoliciesfield enhances the semantic meaning of the configuration.
38-38: LGTM!The addition of the
accessPoliciesfield enhances the semantic meaning of the configuration.tests/application/access-policy/bad-policy-assert.yaml (1)
13-13: LGTM!The addition of the
accessPoliciesfield with the valueInvalidConfigenhances the visibility of the access policy configuration status.tests/skipjob/access-policy-job/skipjob-assert.yaml (1)
94-94: LGTM!The addition of the
accessPoliciesfield with the valueReadyenhances the visibility of the access policy configuration status.api/v1alpha1/status_types.go (3)
16-17: LGTM!The addition of the
AccessPoliciesfield enhances the status representation of theSkiperatorStatusby providing more detailed information related to access policies.
39-40: LGTM!The addition of the
READYandINVALIDCONFIGconstants broadens the range of possible status values that can be represented, enhancing the functionality of theStatusNamestype.
69-69: LGTM!The update to the
SetSummaryProgressingmethod ensures that theAccessPoliciesfield is set appropriately during the state transition, maintaining consistency in the status representation.tests/application/access-policy/advanced-assert.yaml (1)
127-127: LGTM!The addition of the
accessPoliciesfield enhances the status reporting of access policies.config/crd/skiperator.kartverket.no_routings.yaml (2)
79-81: LGTM!The addition of the
accessPoliciesfield enhances the status reporting of access policies and the description provides clarity on its purpose.
175-175: LGTM!The addition of the
accessPoliciesfield to the list of required fields ensures consistent status reporting.api/v1alpha1/skipjob_types.go (2)
Line range hint
1-36: LGTM!The removal of the
JobCreatedConditionconstant simplifies the state handling for SKIP jobs.
37-37: LGTM!The addition of the
AccessPoliciesprint column enhances the status reporting of SKIP jobs.internal/controllers/skipjob.go (2)
329-329: LGTM! But verify the function usage in the codebase.The code changes are approved. However, ensure that all function calls to
updateConditionscorrectly handle the new status updates.Run the following script to verify the function usage:
332-332: LGTM! But verify the function usage in the codebase.The code changes are approved. However, ensure that all function calls to
updateConditionscorrectly handle the new status updates.Run the following script to verify the function usage:
Verification successful
Function Usage Verified: Status Updates Handled Correctly
The
updateConditionsfunction in bothskipjob.goandapplication.gocorrectly handles the status updates, settingAccessPoliciestoREADYwhen appropriate. No further action is needed.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `updateConditions` handle the new status updates. # Test: Search for the function usage. Expect: Proper handling of the new status updates. rg --type go -A 5 $'updateConditions'Length of output: 1906
Script:
#!/bin/bash # Verify the implementation of `updateConditions` in `application.go` to ensure it handles status updates correctly. ast-grep --lang go --pattern $'func (r *ApplicationReconciler) updateConditions(app *skiperatorv1alpha1.Application) { $$$ app.Status.AccessPolicies = $_ $$$ }'Length of output: 167
Script:
#!/bin/bash # Search for occurrences of `app.Status.AccessPolicies` in `application.go` to understand how the status is being updated. rg --type go 'app.Status.AccessPolicies' -A 5Length of output: 768
api/v1alpha1/application_types.go (1)
32-32: LGTM!The code changes are approved. The new print column annotation for "AccessPolicies" enhances the representation of the
ApplicationList.internal/controllers/application.go (3)
254-254: LGTM!The code changes are approved. The change in parameter order may enhance readability or consistency with other method signatures.
246-246: LGTM! But verify the function usage in the codebase.The code changes are approved. However, ensure that all function calls to
updateConditionscorrectly handle the new status updates.Run the following script to verify the function usage:
249-249: LGTM! But verify the function usage in the codebase.The code changes are approved. However, ensure that all function calls to
updateConditionscorrectly handle the new status updates.Run the following script to verify the function usage:
Verification successful
Function
updateConditionshandles status updates correctly.The
updateConditionsfunction ininternal/controllers/application.gocorrectly updates theapp.Status.AccessPoliciesbased on the validity of the internal rules and updates the conditions accordingly. No further action is needed.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `updateConditions` handle the new status updates. # Test: Search for the function usage. Expect: Proper handling of the new status updates. rg --type go -A 5 $'updateConditions'Length of output: 1906
Script:
#!/bin/bash # Verify the complete implementation of the `updateConditions` function in `internal/controllers/application.go` ast-grep --lang go --pattern $'func (r *ApplicationReconciler) updateConditions(app *skiperatorv1alpha1.Application) { $$$ }'Length of output: 1202
config/crd/skiperator.kartverket.no_skipjobs.yaml (3)
21-23: LGTM!The addition of the
accessPoliciesfield in theadditionalPrinterColumnssection improves the visibility of access policies in the resource status.
859-861: LGTM!The addition of the
accessPoliciesfield in thestatusproperties section enhances the resource's metadata by providing additional context regarding access control.
955-955: LGTM!Marking the
accessPoliciesfield as required ensures that the access policy configuration is always validated.config/crd/skiperator.kartverket.no_applications.yaml (3)
23-25: LGTM!The addition of the
accessPoliciesfield in theadditionalPrinterColumnssection improves the visibility of access policies in the resource status.
1105-1107: LGTM!The addition of the
accessPoliciesfield in thestatusproperties section enhances the resource's metadata by providing additional context regarding access control.
1201-1201: LGTM!Marking the
accessPoliciesfield as required ensures that the access policy configuration is always validated.
Yeah, but I couldn't figure out how. AccessPolicies status basically just takes the result of the condition and displays it. If none are defined then it gets the status "Ready" |
This adds the printer column "AccessPolicies" and gives us feedback if the access policy configuration is valid
Summary by CodeRabbit
New Features
ApplicationListfor enhanced visibility of access control configurations.accessPoliciesfield in multiple resource definitions, indicating the validity of access policies.Bug Fixes
READYorINVALIDCONFIG.Tests
accessPoliciesstatus, improving monitoring and validation of access policies across different scenarios.