Update Flux Operator (0.24.0) - #1167
Conversation
Signed-off-by: Kingdon B <[email protected]>
Signed-off-by: Kingdon B <[email protected]>
WalkthroughThis update increments Helm chart versions for both Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Helm
participant Kubernetes
participant FluxOperator Pod
User->>Helm: Install/upgrade flux-operator chart with extraVolumes/extraVolumeMounts values
Helm->>Kubernetes: Rendered manifests with additional volumes/mounts
Kubernetes->>FluxOperator Pod: Create pod with specified extra volumes/mounts
FluxOperator Pod-->>Kubernetes: Pod runs with enhanced configuration
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (5)
packages/system/fluxcd-operator/charts/flux-operator/README.md (2)
3-3: Minor spacing nitpick after badgesThere are two consecutive spaces before the line break; drop one to satisfy markdown linters.
- +
41-42: Docs for new values are present – consider adding a usage example
extraVolumeMountsandextraVolumesare now documented, nice.
Including a short YAML snippet showing how to supply one volume & mount would make adoption easier.packages/system/fluxcd-operator/charts/flux-operator/values.yaml (2)
119-121: Schema tag missing default/description detailsThe new
extraVolumeslist lacks hints for expected structure (e.g.,name,emptyDir,configMap, …).
Adding an inline comment or@schemaexample improves IDE validation.
128-130: Mirror validation advice forextraVolumeMountsSame comment as above – a brief schema/example (e.g.,
{ name: foo, mountPath: /data }) would help users avoid Helm-time errors.packages/system/fluxcd-operator/charts/flux-operator/templates/crds.yaml (1)
909-942: Filter semantics: clarify precedence betweeninclude*,exclude*, andsemver.Nice addition, but the CRD description doesn’t specify how
semverinteracts
withincludeTag/excludeTag. Users may assumesemverfiltering happens
after the include/exclude regexes, but that is not obvious.A one-line clarification in the field description will save trouble later.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
packages/system/fluxcd-operator/charts/flux-operator/Chart.yaml(2 hunks)packages/system/fluxcd-operator/charts/flux-operator/README.md(2 hunks)packages/system/fluxcd-operator/charts/flux-operator/templates/crds.yaml(7 hunks)packages/system/fluxcd-operator/charts/flux-operator/templates/deployment.yaml(1 hunks)packages/system/fluxcd-operator/charts/flux-operator/values.yaml(1 hunks)packages/system/fluxcd/charts/flux-instance/Chart.yaml(2 hunks)packages/system/fluxcd/charts/flux-instance/README.md(1 hunks)
🧰 Additional context used
🪛 LanguageTool
packages/system/fluxcd/charts/flux-instance/README.md
[grammar] ~3-~3: Use proper spacing conventions.
Context: ...cation-informational?style=flat-square) 
(QB_NEW_EN_OTHER_ERROR_IDS_000007)
packages/system/fluxcd-operator/charts/flux-operator/README.md
[grammar] ~3-~3: Use proper spacing conventions.
Context: ...cation-informational?style=flat-square)
(QB_NEW_EN_OTHER_ERROR_IDS_000007)
🔇 Additional comments (5)
packages/system/fluxcd/charts/flux-instance/README.md (1)
3-3: Version badge update looks goodChart README is now in sync with Chart.yaml
version/appVersion.
No further action needed.packages/system/fluxcd-operator/charts/flux-operator/Chart.yaml (1)
11-11: ✅ No stale v0.23.0 references detectedBoth
appVersionand chartversionare correctly bumped to v0.24.0, and searches across the entireflux-operatorchart show no remainingv0.23.0or0.23.0occurrences. Ready to merge.packages/system/fluxcd/charts/flux-instance/Chart.yaml (1)
11-11: Chart metadata aligned with operator – good
appVersion/versionbumped consistently to 0.24.0.
No issues spotted.Also applies to: 28-28
packages/system/fluxcd-operator/charts/flux-operator/templates/crds.yaml (2)
589-592: Backwards-compat break – new required fields will reject all existing FluxReports.
platformandserverVersionare now required. Any cluster already running
FluxReport objects that omit these properties will start failing validation
once the CRD is replaced, blocking further updates.Consider one of:
- Make the fields optional and mark them
nullable: true.- Provide a default via
default:and drop them fromrequired:.- Ship a migration Job that patches existing objects before the CRD update.
Please verify the upgrade path on live clusters.
643-659: New.spec.operatorblock marked required – same migration concern.For every existing FluxReport the new
operatorobject (with three required
scalar fields) will be missing, again causing CRD validation errors after
upgrade.Unless you plan to recreate all FluxReport resources, treat this the same way
as the previous comment: optional/nullable or an automated data migration.
| - name: temp | ||
| mountPath: /tmp | ||
| {{- if .Values.extraVolumeMounts }} | ||
| {{- toYaml .Values.extraVolumeMounts | nindent 12 }} | ||
| {{- end }} | ||
| volumes: | ||
| - name: temp | ||
| emptyDir: {} | ||
| {{- if .Values.extraVolumes }} | ||
| {{- toYaml .Values.extraVolumes | nindent 8 }} | ||
| {{- end }} |
There was a problem hiding this comment.
Empty-slice renders an invalid [] entry – guard for length, not only existence.
.Values.extraVolumeMounts and .Values.extraVolumes default to an empty list ([]).
In Go templates an empty (but non-nil) slice still evaluates to true, so the current if
block emits the YAML representation of an empty slice ([]).
Placing [] under volumeMounts:/volumes: breaks the Pod spec.
Apply a length check before rendering:
-{{- if .Values.extraVolumeMounts }}
- {{- toYaml .Values.extraVolumeMounts | nindent 12 }}
-{{- end }}
+{{- $m := .Values.extraVolumeMounts }}
+{{- if and $m (gt (len $m) 0) }}
+ {{- toYaml $m | nindent 12 }}
+{{- end }}and likewise for extraVolumes.
Repeatable pattern keeps the template safe even when the value is left at its
(default) empty slice.
🤖 Prompt for AI Agents
In
packages/system/fluxcd-operator/charts/flux-operator/templates/deployment.yaml
around lines 100 to 110, the template conditionals for rendering
extraVolumeMounts and extraVolumes only check for existence, causing empty
slices to render as invalid `[]` in the YAML. Update the conditionals to check
the length of .Values.extraVolumeMounts and .Values.extraVolumes instead of just
their existence, ensuring the blocks only render when the slices are non-empty.
This prevents invalid empty slice entries under volumeMounts and volumes in the
Pod spec.
| - message: spec.url must start with 'http://' or 'https://' when spec.type | ||
| is a Git provider | ||
| rule: '!self.type.startsWith(''Git'') || self.url.startsWith(''http'')' | ||
| - message: spec.url must start with 'http://' or 'https://' when spec.type | ||
| is a Git provider | ||
| rule: '!self.type.startsWith(''AzureDevOps'') || self.url.startsWith(''http'')' | ||
| - message: spec.url must start with 'oci://' when spec.type is an OCI | ||
| provider | ||
| rule: '!self.type.endsWith(''ArtifactTag'') || self.url.startsWith(''oci'')' | ||
| - message: cannot specify spec.serviceAccountName when spec.type is not | ||
| one of AzureDevOps* or *ArtifactTag | ||
| rule: '!has(self.serviceAccountName) || self.type.startsWith(''AzureDevOps'') | ||
| || self.type.endsWith(''ArtifactTag'')' | ||
| - message: cannot specify spec.certSecretRef when spec.type is one of | ||
| Static, AzureDevOps*, ACRArtifactTag, ECRArtifactTag or GARArtifactTag | ||
| rule: '!has(self.certSecretRef) || !(self.url == ''Static'' || self.type.startsWith(''AzureDevOps'') | ||
| || (self.type.endsWith(''ArtifactTag'') && self.type != ''OCIArtifactTag''))' | ||
| - message: cannot specify spec.secretRef when spec.type is one of Static, | ||
| ACRArtifactTag, ECRArtifactTag or GARArtifactTag | ||
| rule: '!has(self.secretRef) || !(self.url == ''Static'' || (self.type.endsWith(''ArtifactTag'') | ||
| && self.type != ''OCIArtifactTag''))' | ||
| status: |
There was a problem hiding this comment.
Validation rules use self.url where they should reference self.type.
Both rules below compare the type but mistakenly reference self.url,
rendering the constraint ineffective and potentially blocking valid specs.
rule: '!has(self.certSecretRef) || !(self.url == ''Static'' || ... )'
^^^^^ ↑
rule: '!has(self.secretRef) || !(self.url == ''Static'' || ... )'Fix:
- !has(self.certSecretRef) || !(self.url == 'Static' || ...
+ !has(self.certSecretRef) || !(self.type == 'Static' || ...
- !has(self.secretRef) || !(self.url == 'Static' || ...
+ !has(self.secretRef) || !(self.type == 'Static' || ...Without this change the rules silently allow forbidden combinations and fail
for the wrong reasons.
🤖 Prompt for AI Agents
In packages/system/fluxcd-operator/charts/flux-operator/templates/crds.yaml
around lines 1038 to 1059, the validation rules incorrectly use self.url when
they should reference self.type for checking specific provider types. To fix
this, replace self.url with self.type in the rules that check for 'Static' and
ArtifactTag types in the certSecretRef and secretRef validation rules, ensuring
the conditions correctly validate the resource type instead of the URL.
4 participants
This PR updates Flux Operator to 0.24.0 - some changes have been undertaken to make upgrading Flux on any version of the flux-operator more reliable - these are related to
spec.distribution.artifactwhich I think you have already seenhttps://fluxcd.control-plane.io/operator/fluxinstance/#distribution-artifact
May be relevant to air-gapped environments.
Summary by CodeRabbit
New Features
Documentation
Chores