-
Notifications
You must be signed in to change notification settings - Fork 69
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
Flux Operator v0.15 #631
base: main
Are you sure you want to change the base?
Flux Operator v0.15 #631
Conversation
WalkthroughThis update increments version fields in the Helm chart metadata for both the Flux Operator and Flux Instance, changing the app and chart versions from 0.13.0 to 0.15.0. The version badges in the corresponding README files have been updated accordingly. New Custom Resource Definitions for Changes
Possibly related PRs
Suggested reviewers
Poem
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 (
|
This flux-operator release adds the new |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/system/fluxcd/charts/flux-instance/values.schema.json (1)
134-136
: Add pattern validation for interval fieldConsider adding a pattern validation for the interval field to ensure valid time duration format (e.g., "1m", "5m", "1h").
"interval": { "type": "string" + "pattern": "^([0-9]+(\\.[0-9]+)?(ms|s|m|h))+$" }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
packages/system/fluxcd-operator/charts/flux-operator/Chart.yaml
(2 hunks)packages/system/fluxcd-operator/charts/flux-operator/README.md
(1 hunks)packages/system/fluxcd-operator/charts/flux-operator/templates/crds.yaml
(2 hunks)packages/system/fluxcd/charts/flux-instance/Chart.yaml
(2 hunks)packages/system/fluxcd/charts/flux-instance/README.md
(2 hunks)packages/system/fluxcd/charts/flux-instance/templates/instance.yaml
(2 hunks)packages/system/fluxcd/charts/flux-instance/values.schema.json
(2 hunks)packages/system/fluxcd/charts/flux-instance/values.yaml
(2 hunks)
✅ Files skipped from review due to trivial changes (3)
- packages/system/fluxcd-operator/charts/flux-operator/Chart.yaml
- packages/system/fluxcd/charts/flux-instance/Chart.yaml
- packages/system/fluxcd-operator/charts/flux-operator/README.md
🔇 Additional comments (6)
packages/system/fluxcd/charts/flux-instance/templates/instance.yaml (2)
20-22
: LGTM: Artifact pull secret support addedThe conditional inclusion of
artifactPullSecret
is well implemented, allowing for optional authentication when pulling artifacts.
43-43
: LGTM: Sync interval configuration addedThe sync interval field is properly integrated into the sync configuration block.
packages/system/fluxcd/charts/flux-instance/values.yaml (2)
12-12
: LGTM: Empty default for artifactPullSecretEmpty string default is appropriate for an optional authentication configuration.
41-41
: LGTM: Default sync interval of 1mThe 1-minute default interval provides a good balance between responsiveness and resource usage.
packages/system/fluxcd/charts/flux-instance/README.md (1)
3-3
: LGTM: Documentation updated for v0.14.0Version badges and values documentation are properly updated to reflect the new fields and version.
Also applies to: 43-43, 47-47
packages/system/fluxcd-operator/charts/flux-operator/templates/crds.yaml (1)
743-1250
: LGTM: Well-structured ResourceSet CRDsThe new ResourceSetInputProvider and ResourceSet CRDs are well-defined with:
- Comprehensive schema validation
- Informative printer columns
- Thorough documentation
- Proper status subresources
e403b31
to
92b5b54
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/system/fluxcd-operator/charts/flux-operator/templates/crds.yaml (2)
743-974
: New CRD: ResourceSetInputProvider
This entire CRD definition forResourceSetInputProvider
(with short namersip
) is a solid addition. It correctly defines both the desired state (inspec
) and the observed state (instatus
). The use of additional printer columns and detailed nested properties (such as forcertSecretRef
,defaultValues
,filter
,secretRef
,type
, andurl
) adheres to Kubernetes API conventions. One minor suggestion would be to review the URL pattern and consider if any further validation (or examples in documentation) might be useful for users.
975-1251
: New CRD: ResourceSet
The CRD forResourceSet
is well structured and consistent with Kubernetes API standards. All key fields—includingcommonMetadata
,dependsOn
,inputs
,inputsFrom
,resources
,resourcesTemplate
,serviceAccountName
, andwait
—are clearly defined. The inclusion of a short name (rset
) simplifies CLI usage, and the status section (includingconditions
,inventory
,lastAppliedRevision
, andlastHandledReconcileAt
) provides a comprehensive view of the resource’s state. Consider adding an example or additional documentation regarding the intended use ofresourcesTemplate
for users who might need guidance on constructing multi-document YAML templates.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
packages/system/fluxcd-operator/charts/flux-operator/Chart.yaml
(2 hunks)packages/system/fluxcd-operator/charts/flux-operator/README.md
(1 hunks)packages/system/fluxcd-operator/charts/flux-operator/templates/crds.yaml
(2 hunks)packages/system/fluxcd/charts/flux-instance/Chart.yaml
(2 hunks)packages/system/fluxcd/charts/flux-instance/README.md
(2 hunks)packages/system/fluxcd/charts/flux-instance/templates/instance.yaml
(2 hunks)packages/system/fluxcd/charts/flux-instance/values.schema.json
(2 hunks)packages/system/fluxcd/charts/flux-instance/values.yaml
(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (7)
- packages/system/fluxcd/charts/flux-instance/Chart.yaml
- packages/system/fluxcd/charts/flux-instance/values.schema.json
- packages/system/fluxcd/charts/flux-instance/values.yaml
- packages/system/fluxcd/charts/flux-instance/README.md
- packages/system/fluxcd-operator/charts/flux-operator/Chart.yaml
- packages/system/fluxcd/charts/flux-instance/templates/instance.yaml
- packages/system/fluxcd-operator/charts/flux-operator/README.md
🔇 Additional comments (1)
packages/system/fluxcd-operator/charts/flux-operator/templates/crds.yaml (1)
145-150
: ArtifactPullSecret Field Addition
A new fieldartifactPullSecret
has been added under the distribution properties. The description clearly states that it specifies the Kubernetes secret for pulling the manifests. Verify whether this field is meant to be optional or if it should also be included in the required list based on your deployment needs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Signed-off-by: Kingdon B <[email protected]>
Let's merge this together with #640 and target Flux Operator 0.15.0, which just released |
Signed-off-by: Kingdon B <[email protected]>
Signed-off-by: Kingdon B <[email protected]>
(No changes in here, this was bumped to match the new flux-operator release at v0.15.0) Signed-off-by: Kingdon B <[email protected]>
92b5b54
to
403addb
Compare
This one should get a new review, because it has added some minor changes to the way that RBAC is defined. (I don't think there's anything new in here except for some chart parameters that we aren't using, but please feel free to double-check me!) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (5)
packages/system/fluxcd/charts/flux-instance/README.md (1)
43-47
: Improve Markdown URL formatting.Several bare URLs are used within the table descriptions (e.g. in the "Distribution" and "Sync" columns). Converting these into proper Markdown links (e.g.,
[Documentation](https://fluxcd.control-plane.io/operator/fluxinstance/#distribution-configuration)
) will resolve MD034 warnings and enhance readability.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
43-43: Bare URL used
null(MD034, no-bare-urls)
44-44: Bare URL used
null(MD034, no-bare-urls)
45-45: Bare URL used
null(MD034, no-bare-urls)
46-46: Bare URL used
null(MD034, no-bare-urls)
47-47: Bare URL used
null(MD034, no-bare-urls)
packages/system/fluxcd-operator/charts/flux-operator/templates/admin-clusterrole.yaml (1)
1-23
: Templating and YAML Lint Consideration in Admin ClusterRoleBinding
The conditional block using{{- if .Values.rbac.create }}
cleanly wraps the ClusterRoleBinding resource definition. Note that YAML linters may flag the templating syntax as a syntax error (as reported by YAMLlint), but this is a known false positive when processing Helm charts. Please ensure your linting configuration ignores templating markers.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
[warning] 9-9: wrong indentation: expected 2 but found 4
(indentation)
[warning] 10-10: wrong indentation: expected 2 but found 4
(indentation)
packages/system/fluxcd-operator/charts/flux-operator/templates/crds.yaml (2)
743-974
: New CRD:ResourceSetInputProvider
Definition
This new CRD is well structured with clear properties defined in thespec
section. The following points are noteworthy:
- Required Fields: The spec requires only
type
andurl
, which is appropriate given that other properties (likecertSecretRef
,defaultValues
,filter
, andsecretRef
) are optional.- Validation and Documentation: The descriptions for each field (e.g., for
certSecretRef
andfilter
) are detailed and explain the expected formats. The regex for the URL field ensures proper HTTP/S endpoints.- Short Names: The inclusion of
shortNames: [rsip]
will help with CLI usability.Overall, the CRD adheres to Kubernetes API conventions. You might consider (as a future enhancement) including an example in the documentation to illustrate how the optional fields may be used in practice.
975-1251
: New CRD:ResourceSet
Definition
The newResourceSet
CRD appears to be implemented correctly with an extensive set of properties in both thespec
andstatus
sections. Key observations:
- Spec Structure: The fields like
commonMetadata
,dependsOn
,inputs
,inputsFrom
,resources
,resourcesTemplate
,serviceAccountName
, andwait
are defined with appropriate types and detailed descriptions. The use of a Go template string forresourcesTemplate
is clearly documented.- Short Names & Naming Conventions: The
shortNames: [rset]
is a useful addition for usability. Naming conventions are consistent throughout.- Status Section: The
status
block, which includes conditions, inventory, lastAppliedRevision, and lastHandledReconcileAt, is comprehensive and follows common Kubernetes patterns.A minor suggestion would be to ensure that any complex fields (for instance, the CEL-ready expression in
dependsOn.readyExpr
) have examples in the external documentation to aid users in understanding the expected input.packages/system/fluxcd-operator/charts/flux-operator/values.schema.json (1)
172-184
: Validate Multitenancy Schema Definition
The schema formultitenancy
correctly requires thedefaultServiceAccount
while defining an optionalenabled
boolean. Consider adding a default value offalse
forenabled
in the schema to align with the values provided invalues.yaml
for improved consistency.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
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/admin-clusterrole.yaml
(2 hunks)packages/system/fluxcd-operator/charts/flux-operator/templates/aggregate-clusterrole.yaml
(1 hunks)packages/system/fluxcd-operator/charts/flux-operator/templates/crds.yaml
(2 hunks)packages/system/fluxcd-operator/charts/flux-operator/templates/deployment.yaml
(1 hunks)packages/system/fluxcd-operator/charts/flux-operator/values.schema.json
(2 hunks)packages/system/fluxcd-operator/charts/flux-operator/values.yaml
(2 hunks)packages/system/fluxcd/charts/flux-instance/Chart.yaml
(2 hunks)packages/system/fluxcd/charts/flux-instance/README.md
(2 hunks)packages/system/fluxcd/charts/flux-instance/templates/instance.yaml
(2 hunks)packages/system/fluxcd/charts/flux-instance/values.schema.json
(2 hunks)packages/system/fluxcd/charts/flux-instance/values.yaml
(2 hunks)packages/system/fluxcd/values.yaml
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
- packages/system/fluxcd/charts/flux-instance/values.yaml
- packages/system/fluxcd-operator/charts/flux-operator/Chart.yaml
- packages/system/fluxcd/charts/flux-instance/Chart.yaml
- packages/system/fluxcd/charts/flux-instance/values.schema.json
- packages/system/fluxcd/charts/flux-instance/templates/instance.yaml
- packages/system/fluxcd-operator/charts/flux-operator/README.md
🧰 Additional context used
🪛 YAMLlint (1.35.1)
packages/system/fluxcd-operator/charts/flux-operator/templates/admin-clusterrole.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
packages/system/fluxcd-operator/charts/flux-operator/templates/aggregate-clusterrole.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
🪛 markdownlint-cli2 (0.17.2)
packages/system/fluxcd/charts/flux-instance/README.md
43-43: Bare URL used
null
(MD034, no-bare-urls)
44-44: Bare URL used
null
(MD034, no-bare-urls)
45-45: Bare URL used
null
(MD034, no-bare-urls)
46-46: Bare URL used
null
(MD034, no-bare-urls)
47-47: Bare URL used
null
(MD034, no-bare-urls)
🔇 Additional comments (9)
packages/system/fluxcd/values.yaml (1)
7-7
: Update Flux instance distribution version.The distribution version has been updated to
2.5.x
, which reflects the new Flux release. Please ensure that this update remains consistent with related files (e.g.Chart.yaml
) and upstream documentation.packages/system/fluxcd/charts/flux-instance/README.md (2)
3-3
: Version badges updated to 0.15.0.The badge images now indicate version 0.15.0 for the Flux Operator, ensuring that users see the current release information at a glance.
43-47
: Enhanced distribution and sync configuration details.The table row for
instance.distribution
now includes the newartifactPullSecret
field, and theinstance.sync
entry shows a default"interval": "1m"
. Please verify that the"version"
field in the distribution JSON (currently set to"2.x"
) is intentionally left different from the2.5.x
value invalues.yaml
. Ensuring this consistency (or documenting the intentional difference) would help avoid confusion.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
43-43: Bare URL used
null(MD034, no-bare-urls)
44-44: Bare URL used
null(MD034, no-bare-urls)
45-45: Bare URL used
null(MD034, no-bare-urls)
46-46: Bare URL used
null(MD034, no-bare-urls)
47-47: Bare URL used
null(MD034, no-bare-urls)
packages/system/fluxcd-operator/charts/flux-operator/templates/aggregate-clusterrole.yaml (1)
1-57
: Conditional Aggregated ClusterRoles Implementation
The introduction of the two ClusterRole resources (for “-edit” and “-view”) under the conditional{{- if .Values.rbac.createAggregation }}
is well implemented. The use of the document separator (---
) to distinguish the two resources is appropriate. Similar to the previous file, YAML linters might misinterpret the templating syntax; ensure your linter is configured to handle these cases. Overall, the new RBAC aggregation resources appear aligned with the release objectives.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
packages/system/fluxcd-operator/charts/flux-operator/templates/deployment.yaml (1)
53-55
: Addition of Multitenancy Argument in Deployment
The new conditional block that appends{{- if .Values.multitenancy.enabled }} - --default-service-account={{ .Values.multitenancy.defaultServiceAccount }} {{- end }}
to the container arguments is clear and properly tied to the multitenancy configuration. This change enhances deployment flexibility by allowing a default service account to be specified when multitenancy is enabled. Please verify that the referenced value in
Values.multitenancy.defaultServiceAccount
is correctly set in thevalues.yaml
file.packages/system/fluxcd-operator/charts/flux-operator/templates/crds.yaml (1)
145-149
: New Distribution Field:artifactPullSecret
Addition
The newartifactPullSecret
field under thedistribution
property is added correctly with a clear description and string type. Ensure that downstream consumers of this CRD support this new field and that it is documented in the chart’s README and values file.packages/system/fluxcd-operator/charts/flux-operator/values.yaml (2)
6-10
: Assess Multitenancy Configuration
The new multitenancy section is clearly documented and includes a helpful reference link to the ResourceSet API RBAC guidelines. Verify that the chosen default service account ("flux-operator"
) is correct for your deployment scenarios.
63-68
: Review RBAC Settings
The introduced RBAC section properly defines two key settings:create
for cluster-admin privileges andcreateAggregation
for granting access to ResourceSet APIs. Confirm that these RBAC flags match the intended security model and that the chart templates appropriately condition their creation based on these values.packages/system/fluxcd-operator/charts/flux-operator/values.schema.json (1)
200-210
: Review RBAC Schema Properties
The schema segment forrbac
is straightforward, specifying boolean types for bothcreate
andcreateAggregation
. Ensure that downstream chart templates are prepared to handle cases when these keys are omitted and that the intended default behaviors fromvalues.yaml
are maintained.
A new release of the Flux Operator (v0.15.0) - to go with the newly created Flux v2.5.0 release
(And to go with that, a new version of the flux-instance chart.)
Summary by CodeRabbit
Summary by CodeRabbit
New Features
ResourceSetInputProvider
andResourceSet
.Documentation