chore: add policy v2 flag #18
Conversation
adityachoudhari26
requested review from
jsbroks and
zacharyblasczyk
as code owners
adityachoudhari26
had a problem deploying
to
Helm Charts
GitHub Actions
Failure
WalkthroughThis change introduces a new global configuration parameter, Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant HelmChart
participant Kubernetes
participant EventWorker
participant WebService
User->>HelmChart: Set or use default enableNewPolicyEngine
HelmChart->>Kubernetes: Render deployment templates with ENABLE_NEW_POLICY_ENGINE env var
Kubernetes->>EventWorker: Deploy with ENABLE_NEW_POLICY_ENGINE
Kubernetes->>WebService: Deploy with ENABLE_NEW_POLICY_ENGINE
Suggested reviewers
Poem
Tip ⚡💬 Agentic Chat (Pro Plan, General Availability)
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
🪧 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: 2
🧹 Nitpick comments (1)
charts/ctrlplane/values.yaml (1)
6-6: Add global feature flagenableNewPolicyEngine
The new global configuration flag is correctly introduced underglobalwith a defaultfalse. Please update any related documentation or avalues.schema.json(if maintained) to include this new parameter for completeness.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
charts/ctrlplane/Chart.yaml(1 hunks)charts/ctrlplane/charts/event-worker/templates/deployment.yaml(1 hunks)charts/ctrlplane/charts/webservice/templates/deployment.yaml(1 hunks)charts/ctrlplane/values.yaml(1 hunks)
🧰 Additional context used
🪛 GitHub Actions: Lint and Test Charts
charts/ctrlplane/charts/webservice/templates/deployment.yaml
[error] 127-127: Helm lint error: template execution failed at line 127 due to nil pointer evaluating interface {}.global.enableNewPolicyEngine
🔇 Additional comments (1)
charts/ctrlplane/Chart.yaml (1)
5-5: Bump chart version
Updating the chart version to0.3.13aligns with the addition of the new policy engine flag. No further modifications are required here.
| - name: ENABLE_NEW_POLICY_ENGINE | ||
| value: {{ .Values.global.enableNewPolicyEngine | quote }} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Prevent nil pointer on .Values.global.enableNewPolicyEngine
Directly referencing .Values.global.enableNewPolicyEngine causes a nil pointer error when global isn’t defined in this subchart’s values.yaml. You have two options:
- Define a
globalkey incharts/webservice/values.yaml(e.g.,global: {}) so that.Values.globalalways exists. - Guard the block and use Helm’s
defaultfunction:
- - name: ENABLE_NEW_POLICY_ENGINE
- value: {{ .Values.global.enableNewPolicyEngine | quote }}
+{{- if .Values.global }}
+ - name: ENABLE_NEW_POLICY_ENGINE
+ value: {{ default false .Values.global.enableNewPolicyEngine | quote }}
+{{- end }}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - name: ENABLE_NEW_POLICY_ENGINE | |
| value: {{ .Values.global.enableNewPolicyEngine | quote }} | |
| {{- if .Values.global }} | |
| - name: ENABLE_NEW_POLICY_ENGINE | |
| value: {{ default false .Values.global.enableNewPolicyEngine | quote }} | |
| {{- end }} |
🧰 Tools
🪛 GitHub Actions: Lint and Test Charts
[error] 127-127: Helm lint error: template execution failed at line 127 due to nil pointer evaluating interface {}.global.enableNewPolicyEngine
| - name: ENABLE_NEW_POLICY_ENGINE | ||
| value: {{ .Values.global.enableNewPolicyEngine | quote }} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Prevent nil pointer on .Values.global.enableNewPolicyEngine
As with the webservice subchart, a direct reference to the global flag can trigger a nil pointer. Either add a global entry in charts/event-worker/values.yaml or wrap this in an if guard and apply a default:
- - name: ENABLE_NEW_POLICY_ENGINE
- value: {{ .Values.global.enableNewPolicyEngine | quote }}
+{{- if .Values.global }}
+ - name: ENABLE_NEW_POLICY_ENGINE
+ value: {{ default false .Values.global.enableNewPolicyEngine | quote }}
+{{- end }}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - name: ENABLE_NEW_POLICY_ENGINE | |
| value: {{ .Values.global.enableNewPolicyEngine | quote }} | |
| {{- if .Values.global }} | |
| - name: ENABLE_NEW_POLICY_ENGINE | |
| value: {{ default false .Values.global.enableNewPolicyEngine | quote }} | |
| {{- end }} |
adityachoudhari26
had a problem deploying
to
Helm Charts
GitHub Actions
Failure
zacharyblasczyk
temporarily deployed
to
Helm Charts
GitHub Actions
Inactive
Summary by CodeRabbit
New Features
Chores