chore: upgrade to Argo CD v3.2.1 and Go to 1.25 #1989
Conversation
WalkthroughBump release to v0.18.0: updates default/version variables and base images (Argo CD, Redis), adds new CRD fields and stricter path validations, introduces multiple new OLM catalog CRD and manifest files, upgrades Go toolchain and dependencies, and adds controller manager/service/ RBAC manifests for 0.18.0. Changes
Sequence Diagram(s)(omitted) Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
![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
♻️ Duplicate comments (4)
bundle/manifests/argoproj.io_applications.yaml (4)
1949-1952:syncPolicy.retry.refreshmirrors operation-level retry flagNice to see the same
refreshcontrol exposed at policy level; schema and description are consistent with the earlieroperation.retry.refreshfield.
2921-2925: StatusoperationState.operation.retry.refreshkeeps spec/status symmetryIncluding
refreshin status ensures round-tripping and observability of the retry behavior; matches the spec-side shape and wording.
4896-4902: StatussourceHydrator.currentOperation.syncSourceconstraints match specThe updated path description,
minLength,pattern, andtargetBranchtext correctly mirror the specsourceHydrator.syncSourceblock, preserving validation and documentation symmetry between desired and current state.Also applies to: 4904-4907
4981-4985: StatussourceHydrator.lastSuccessfulOperation.syncSourcealso alignedSame path/branch semantics and validation applied to the last successful operation snapshot; this keeps all three hydrator locations consistent.
Also applies to: 4988-4989
🧹 Nitpick comments (7)
deploy/olm-catalog/argocd-operator/0.18.0/argocd-operator-webhook-service_v1_service.yaml (2)
7-10: Consider naming the port for consistency.The webhook service port is unnamed, while the metrics service in this release uses a named port. Naming ports is a Kubernetes best practice that improves service mesh compatibility and manifest readability.
Apply this diff to add a port name:
ports: - - port: 443 + - name: webhook + port: 443 protocol: TCP targetPort: 9443
13-14: Optional: Remove empty status block.The
statusfield is managed by Kubernetes at runtime and doesn't need to be included in static manifests.Apply this diff to remove it:
selector: control-plane: controller-manager -status: - loadBalancer: {}deploy/olm-catalog/argocd-operator/0.18.0/argocd-operator-controller-manager-metrics-service_v1_service.yaml (1)
15-16: Optional: Remove empty status block.The
statusfield is managed by Kubernetes at runtime and doesn't need to be included in static manifests.Apply this diff to remove it:
selector: control-plane: controller-manager -status: - loadBalancer: {}deploy/olm-catalog/argocd-operator/0.18.0/argoproj.io_namespacemanagements.yaml (1)
8-15: Consider adding printer columns for better kubectl UX.While not required, adding
additionalPrinterColumnswould improve thekubectl get namespacemanagementsoutput by displaying useful fields likemanagedByand condition status. This is a nice-to-have enhancement for operator UX.Example addition after line 15:
scope: Namespaced versions: - additionalPrinterColumns: - jsonPath: .spec.managedBy name: Managed By type: string - jsonPath: .metadata.creationTimestamp name: Age type: date name: v1beta1deploy/olm-catalog/argocd-operator/0.18.0/argoproj.io_appprojects.yaml (1)
108-110: Consider adding format validations for better UX.Adding validation patterns would catch configuration errors earlier:
serverfields (lines 108-110, 134-137): addformat: urischedulefield (line 322-324): add regex pattern for cron format validationkeyIDfield (line 257-259): add pattern for hexadecimal validationThis would provide immediate feedback at apply time rather than waiting for runtime errors.
Also applies to: 134-137, 257-259, 321-324
deploy/olm-catalog/argocd-operator/0.18.0/argoproj.io_notificationsconfigurations.yaml (1)
79-80: Status subresource enabled without schema definition.The CRD enables the status subresource, but the OpenAPI schema (lines 22-76) doesn't define any status properties. This is valid and allows controllers to update status freely, but consider whether specific status fields should be documented in the schema for better validation and API clarity. If this matches the upstream Argo CD CRD, this design is likely intentional.
bundle/manifests/argoproj.io_applications.yaml (1)
1481-1485:syncSource.pathvalidation andtargetBranchdocs look coherent (minor regex nuance)The added description plus
minLength: 1andpattern: ^.{2,}|[^./]$effectively prevent empty / single-char.or/paths, and the TargetBranch explanation clarifies behavior whenhydrateTois set vs not.Note: this regex still permits values like
"./"or"../"; if the intent is to forbid all root-equivalent paths, you’d need a stricter pattern, but that might diverge from upstream Argo CD. As long as you’re mirroring upstream CRDs, this is fine to keep as-is.Also applies to: 1488-1491
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (24)
Makefile(1 hunks)build/util/Dockerfile(1 hunks)bundle/manifests/argocd-operator.clusterserviceversion.yaml(3 hunks)bundle/manifests/argoproj.io_applications.yaml(6 hunks)bundle/manifests/argoproj.io_applicationsets.yaml(53 hunks)bundle/manifests/argoproj.io_argocdexports.yaml(1 hunks)common/defaults.go(2 hunks)config/crd/bases/argoproj.io_applications.yaml(7 hunks)config/crd/bases/argoproj.io_applicationsets.yaml(54 hunks)config/crd/bases/argoproj.io_appprojects.yaml(1 hunks)config/crd/bases/argoproj.io_argocdexports.yaml(1 hunks)config/manager/kustomization.yaml(1 hunks)deploy/olm-catalog/argocd-operator/0.18.0/argocd-image-updater.argoproj.io_imageupdaters.yaml(1 hunks)deploy/olm-catalog/argocd-operator/0.18.0/argocd-operator-controller-manager-metrics-service_v1_service.yaml(1 hunks)deploy/olm-catalog/argocd-operator/0.18.0/argocd-operator-manager-config_v1_configmap.yaml(1 hunks)deploy/olm-catalog/argocd-operator/0.18.0/argocd-operator-metrics-reader_rbac.authorization.k8s.io_v1_clusterrole.yaml(1 hunks)deploy/olm-catalog/argocd-operator/0.18.0/argocd-operator-namespacemanagement-editor-role_rbac.authorization.k8s.io_v1_clusterrole.yaml(1 hunks)deploy/olm-catalog/argocd-operator/0.18.0/argocd-operator-namespacemanagement-viewer-role_rbac.authorization.k8s.io_v1_clusterrole.yaml(1 hunks)deploy/olm-catalog/argocd-operator/0.18.0/argocd-operator-webhook-service_v1_service.yaml(1 hunks)deploy/olm-catalog/argocd-operator/0.18.0/argoproj.io_appprojects.yaml(1 hunks)deploy/olm-catalog/argocd-operator/0.18.0/argoproj.io_argocdexports.yaml(1 hunks)deploy/olm-catalog/argocd-operator/0.18.0/argoproj.io_namespacemanagements.yaml(1 hunks)deploy/olm-catalog/argocd-operator/0.18.0/argoproj.io_notificationsconfigurations.yaml(1 hunks)go.mod(9 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Run end-to-end tests (v1.27.1)
- GitHub Check: Run golangci-lint and gosec
- GitHub Check: build-operator
- GitHub Check: build
🔇 Additional comments (43)
deploy/olm-catalog/argocd-operator/0.18.0/argocd-operator-metrics-reader_rbac.authorization.k8s.io_v1_clusterrole.yaml (1)
1-10: LGTM!The ClusterRole is properly configured to grant read-only access to the
/metricsendpoint using the appropriate non-resource URL pattern.deploy/olm-catalog/argocd-operator/0.18.0/argocd-operator-controller-manager-metrics-service_v1_service.yaml (1)
9-12: Verify the port name matches the protocol.The port is named "https" but targets port 8080, which is conventionally used for HTTP. Verify whether:
- Port 8080 actually serves HTTPS (in which case the name is correct but the port choice is unconventional), or
- The port should be named "http" to match typical HTTP traffic on 8080.
If port 8080 serves HTTP, apply this diff:
ports: - - name: https + - name: http port: 8443 targetPort: 8080deploy/olm-catalog/argocd-operator/0.18.0/argocd-operator-namespacemanagement-viewer-role_rbac.authorization.k8s.io_v1_clusterrole.yaml (1)
1-20: LGTM! Standard viewer role definition.The ClusterRole correctly grants read-only permissions (get, list, watch) on the
namespacemanagementsresource and get on the status subresource, following standard Kubernetes RBAC patterns for viewer roles.deploy/olm-catalog/argocd-operator/0.18.0/argocd-operator-namespacemanagement-editor-role_rbac.authorization.k8s.io_v1_clusterrole.yaml (1)
1-24: LGTM! Verify status subresource permissions are intentional.The ClusterRole correctly grants full CRUD permissions on the
namespacemanagementsresource. The status subresource only hasgetpermission (lines 19-24), which is typical for operator-managed resources where the controller updates status rather than users. Please confirm this aligns with the intended access model.deploy/olm-catalog/argocd-operator/0.18.0/argoproj.io_namespacemanagements.yaml (2)
1-114: LGTM! Well-structured CRD following Kubernetes conventions.The CRD definition is correct and follows standard Kubernetes patterns:
- Proper OpenAPI v3 schema validation
- Status conditions follow the metav1.Condition pattern with appropriate constraints
- Subresources properly configured with status support
- Controller-gen v0.18.0 annotation matches the tooling
115-120: Status placeholder values are standard for generated CRDs.The empty
acceptedNamesand nullconditions/storedVersionsin the status section are standard placeholders for generated CRD manifests. These will be populated by the Kubernetes API server when the CRD is installed.deploy/olm-catalog/argocd-operator/0.18.0/argoproj.io_appprojects.yaml (3)
1-20: LGTM: Standard CRD structure.The CRD metadata, naming conventions, and scope definitions follow Kubernetes best practices correctly.
332-373: LGTM: Status schema and version settings are correct.The status schema, required fields, and version settings follow Kubernetes CRD conventions. The null values in the status section (lines 368-373) are expected for a CRD manifest before it's applied to a cluster.
21-331: Clarify version relationship and verify schema currency.This file is from argocd-operator v0.18.0's OLM catalog, not Argo CD v3.2.1. The original review conflates operator and Argo CD versions. Confirm which Argo CD version should bundle with operator 0.18.0, then verify the AppProject schema matches that release. If this operator targets Argo CD 2.x or early 3.x versions, the schema may be outdated relative to current Argo CD releases.
deploy/olm-catalog/argocd-operator/0.18.0/argoproj.io_notificationsconfigurations.yaml (3)
1-7: LGTM! Standard CRD metadata structure.The metadata follows Kubernetes conventions with proper naming (plural.group format) and uses controller-gen v0.18.0 for generation.
81-86: LGTM! Standard CRD status placeholder.The empty status fields are expected for generated CRD manifests and will be populated by the Kubernetes API server when applied.
40-75: The NotificationsConfiguration CRD schema is consistent and correct.The spec schema in the 0.18.0 version matches the source definition in
config/crd/bases/argoproj.io_notificationsconfigurations.yamlexactly. All five spec fields (context, services, subscriptions, templates, triggers) properly useadditionalProperties: type: stringwith no required fields or validation constraints—a permissive design intentional for flexible notification configuration management.Makefile (1)
6-6: LGTM!The version bump to 0.18.0 aligns with the release upgrade to Argo CD v3.2.1.
config/manager/kustomization.yaml (1)
16-16: LGTM!The controller image tag update to v0.18.0 is consistent with the version bump across the project.
config/crd/bases/argoproj.io_appprojects.yaml (1)
366-366: LGTM!The storage version flag is correctly set for the v1alpha1 version of the AppProject CRD.
deploy/olm-catalog/argocd-operator/0.18.0/argocd-operator-manager-config_v1_configmap.yaml (1)
1-17: LGTM!The manager configuration follows standard controller-runtime patterns with appropriate settings for health probes, metrics, webhook, and leader election.
go.mod (4)
33-33: Review the new indirect dependency.A new indirect dependency
sigs.k8s.io/structured-merge-diff/v6has been introduced. This is likely pulled in by the Kubernetes v0.34.0 upgrade.
3-3: Verify Go 1.25.0 availability and stability.Go version upgraded to 1.25.0. Confirm this is a stable, released version and compatible with all project dependencies.
6-7: Check Argo CD v3.2.1 is stable and review v3.1 to v3.2 breaking changes.v3.2.1 is a valid patch release containing bug fixes and documentation updates. Patch releases do not introduce breaking changes, but since this upgrade crosses a minor version boundary (v3.1.9 → v3.2.1), the v3.2 minor release includes breaking changes such as source hydration now requiring applications to specify a non-root path instead of using repository root. Review the v3.1 to v3.2 upgrade documentation to ensure compatibility with your configuration.
23-27: Kubernetes v0.34.0 dependencies are valid. The go.mod file correctly specifies v0.34.0 for Kubernetes client libraries, which corresponds to Kubernetes 1.34.x. Kubernetes v1.34.0 was released on August 27, 2025, and is compatible with Argo CD v3.2.1 based on Argo CD's support policy for N and N-1 Kubernetes minor versions. No issues identified.common/defaults.go (2)
73-73: Argo CD v3.2.1 default image digest is consistent and correct.The digest
sha256:a8532a23ed5f6e65afaf2a082b65fc74614549e54774f6b25fe3c993faa7bea3incommon/defaults.gomatches the digest used inbuild/util/Dockerfileand corresponds to the official v3.2.1 release.
194-197: Redis 8.2.2 upgrade is compatible with Argo CD v3.2.1 with no migration risks for cache-only workloads.Argo CD v3.2 explicitly bumps Redis from 8.2.1 to 8.2.2 to address a vulnerability, confirming compatibility. Redis is used only as a throw-away cache by Argo CD and can be lost without service impact, so breaking changes between Redis 7 and 8 (primarily affecting ACL categories and command validation rather than basic cache operations) do not apply. Redis upgrades can proceed with no downtime as Argo CD does not use it as a persistent store.
The digest cannot be verified from public sources, but the upgrade is safe to proceed. Consider validating the image digest in your registry scanning tool if required by security policy.
build/util/Dockerfile (1)
1-2: Verify the Argo CD v3.2.1 image digest using cosign or registry tools.Argo CD v3.2.1 container images are signed by cosign with SLSA Level 3 provenance. The provided digest
sha256:a8532a23ed5f6e65afaf2a082b65fc74614549e54774f6b25fe3c993faa7bea3requires verification against quay.io using tools likecrane digestor cosign to ensure it corresponds to the v3.2.1 release tag.bundle/manifests/argoproj.io_applicationsets.yaml (5)
706-707: Schema extension forrefreshfield looks correct.The optional
refreshboolean field is consistently added across all syncPolicy/retry objects in the CRD schema. This aligns with the Argo CD v3.2.1 upgrade and is backward compatible.
6302-6312: New filter and error handling fields are appropriate additions.The
continueOnRepoNotFoundErrorboolean andtitleMatchstring fields extend the pullRequest generator schema appropriately for the v3.2.1 upgrade. Both are optional and backward compatible.
17044-17045: StrategydeletionOrderfield addition is appropriate.This new optional string field extends the strategy schema for the v3.2.1 upgrade. If specific values are expected (e.g., enum), consider verifying against upstream Argo CD schema.
17827-17829: Status fieldresourcesCountaddition looks correct.The
resourcesCountinteger field withint64format is properly defined for tracking resource counts in the ApplicationSet status.
413-414: Review regex pattern that allows potential path traversal sequences.The pattern
^.{2,}|[^./]$permits ".." and "./" strings (both match the first alternation^.{2,}for 2+ character strings), which could enable directory traversal despite blocking single "." or "/" characters. Since this pattern originates from Argo CD v3.2.1's auto-generated CRD, verify whether this is intentional design (e.g., to allow relative paths in specific contexts) or if runtime validation provides additional protections for thesyncSource.pathfield.config/crd/bases/argoproj.io_applicationsets.yaml (6)
705-706: LGTM: New refresh field added to retry configuration.The refresh boolean field is added to the Retry section of sync operations, enabling retry operations to trigger a refresh of application state. This is a backward-compatible addition that expands the retry capabilities in Argo CD v3.2.1.
Also applies to: 1391-1392, 2078-2079, 2743-2744, 3433-3434, 4119-4120, 4806-4807, 5471-5472, 6144-6145, 7044-7045, 7935-7936, 8617-8618, 9309-9310, 9995-9996, 10682-10683, 11347-11348, 12020-12021, 12920-12921, 13811-13812, 14497-14498, 15169-15170, 16069-16070, 16960-16961, 17719-17720
6301-6302: LGTM: New filter fields for PR generator.The addition of
continueOnRepoNotFoundErrorandtitleMatchfields enhances the pull request generator's filtering and error handling capabilities. These are optional fields that extend functionality without breaking existing configurations.Also applies to: 6310-6311, 12177-12178, 12186-12187, 15326-15327, 15335-15336
17043-17044: LGTM: New deletion ordering strategy field.The
deletionOrderfield allows configuring the order in which ApplicationSet-managed applications are deleted, providing finer control over cleanup operations.
17826-17828: LGTM: Resource count tracking in status.The
resourcesCountfield enables tracking the number of resources managed by the ApplicationSet, useful for observability and monitoring.
17837-17837: LGTM: Status subresource enabled.Enabling the status subresource is a Kubernetes best practice that allows separate RBAC control for status updates and improves controller performance by reducing conflicts on spec updates.
411-413: Verify path validation pattern matches upstream Argo CD v3.2.1 CRD definitions.The path validation adds
minLength: 1andpattern: ^.{2,}|[^./]$across 24 syncSource.path fields. The pattern allows strings with 2+ characters or single non-slash/non-dot characters, effectively blocking empty strings and single-character paths "." and "/" while permitting relative paths like ".." and "./subdir". Confirm this matches the official v3.2.1 ApplicationSet CRD and clarify the intended behavior regarding parent directory references like "..".bundle/manifests/argocd-operator.clusterserviceversion.yaml (1)
1940-1940: LGTM: Image reference updated to v0.18.0.The operator image is correctly updated to
quay.io/argoprojlabs/argocd-operator:v0.18.0, consistent with the version bump in this release.bundle/manifests/argoproj.io_argocdexports.yaml (1)
230-242: LGTM: UpdatedvolumeAttributesClassNamedescription reflects Kubernetes API changes.The updated description correctly reflects the current Kubernetes behavior:
- Allows resetting to empty/nil on Infeasible error state
- Removes the beta feature gate note (VolumeAttributesClass is GA in Kubernetes 1.31+)
This aligns with upstream Kubernetes PVC spec changes.
deploy/olm-catalog/argocd-operator/0.18.0/argoproj.io_argocdexports.yaml (1)
1-291: LGTM: ArgoCDExport CRD correctly added to 0.18.0 catalog.The CRD schema is consistent with the bundle manifest version and includes the updated
volumeAttributesClassNamedescription. The file is correctly placed in the0.18.0catalog directory.config/crd/bases/argoproj.io_argocdexports.yaml (1)
230-242: LGTM: Base CRD updated consistently.The
volumeAttributesClassNamedescription update in the base CRD matches the bundle and OLM catalog manifests, ensuring consistency across all deployment artifacts.deploy/olm-catalog/argocd-operator/0.18.0/argocd-image-updater.argoproj.io_imageupdaters.yaml (1)
1-541: Address controller-gen version inconsistency for ImageUpdater CRD.The ImageUpdater CRD schema is well-defined with appropriate OpenAPI v3 validations, including the CEL rule ensuring mutual exclusivity of
helmandkustomizeinmanifestTargets. However, this CRD usescontroller-genv0.16.1 while all other CRD files in the same deployment catalog (argoproj.io_argocds.yaml,argoproj.io_argocdexports.yaml, etc.) use v0.18.0. Verify whether this version difference is intentional or if the CRD should be regenerated with v0.18.0 to maintain consistency across the catalog.config/crd/bases/argoproj.io_applications.yaml (3)
113-116: Retryrefreshflag is wired consistently across spec and statusThe new
refreshboolean is added in all three relevant retry schemas (operation.retry,spec.syncPolicy.retry, andstatus.operationState.operation.retry) with identical description and type, so spec, policy, and status stay in sync. No schema issues from this addition.Also applies to: 1948-1951, 2920-2924
1478-1484: HydratorsyncSourcepath validation and TargetBranch docs look correctThe tightened
syncSource.pathschema (minLength: 1plus the regex) and the expanded description clearly enforce non‑root paths while still allowing normal subdirectories. Mirroring the same constraints and the clarifiedTargetBranchdescription into the status mirrors (currentOperation.sourceHydrator.syncSourceandlastSuccessfulOperation.sourceHydrator.syncSource) keeps spec and status structurally aligned.Also applies to: 1487-1490, 4895-4900, 4903-4906, 4978-4983, 4986-4989
5915-5915: Explicitsubresources: {}is harmless and matches typical CRD layoutAdding an explicit empty
subresources: {}to the version block is a no‑op behavior‑wise compared to omitting it, and is consistent with how many generated CRDs are emitted. Nothing to change here.bundle/manifests/argoproj.io_applications.yaml (1)
114-117: Newoperation.retry.refreshflag is well-formed and consistentBoolean type and description are clear and align with the expected retry semantics; schema structure looks correct and optional (default false by omission). No issues from the CRD perspective.
Signed-off-by: Jonathan West <jonwest@redhat.com>
jgwest
force-pushed
the
upgrade-to-argocd-321-dec-2025
branch
from
0bbaec0 to
44c84ee
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (25)
Dockerfile(1 hunks)Makefile(1 hunks)build/util/Dockerfile(1 hunks)bundle/manifests/argocd-operator.clusterserviceversion.yaml(3 hunks)bundle/manifests/argoproj.io_applications.yaml(6 hunks)bundle/manifests/argoproj.io_applicationsets.yaml(53 hunks)bundle/manifests/argoproj.io_argocdexports.yaml(1 hunks)common/defaults.go(2 hunks)config/crd/bases/argoproj.io_applications.yaml(7 hunks)config/crd/bases/argoproj.io_applicationsets.yaml(54 hunks)config/crd/bases/argoproj.io_appprojects.yaml(1 hunks)config/crd/bases/argoproj.io_argocdexports.yaml(1 hunks)config/manager/kustomization.yaml(1 hunks)deploy/olm-catalog/argocd-operator/0.18.0/argocd-image-updater.argoproj.io_imageupdaters.yaml(1 hunks)deploy/olm-catalog/argocd-operator/0.18.0/argocd-operator-controller-manager-metrics-service_v1_service.yaml(1 hunks)deploy/olm-catalog/argocd-operator/0.18.0/argocd-operator-manager-config_v1_configmap.yaml(1 hunks)deploy/olm-catalog/argocd-operator/0.18.0/argocd-operator-metrics-reader_rbac.authorization.k8s.io_v1_clusterrole.yaml(1 hunks)deploy/olm-catalog/argocd-operator/0.18.0/argocd-operator-namespacemanagement-editor-role_rbac.authorization.k8s.io_v1_clusterrole.yaml(1 hunks)deploy/olm-catalog/argocd-operator/0.18.0/argocd-operator-namespacemanagement-viewer-role_rbac.authorization.k8s.io_v1_clusterrole.yaml(1 hunks)deploy/olm-catalog/argocd-operator/0.18.0/argocd-operator-webhook-service_v1_service.yaml(1 hunks)deploy/olm-catalog/argocd-operator/0.18.0/argoproj.io_appprojects.yaml(1 hunks)deploy/olm-catalog/argocd-operator/0.18.0/argoproj.io_argocdexports.yaml(1 hunks)deploy/olm-catalog/argocd-operator/0.18.0/argoproj.io_namespacemanagements.yaml(1 hunks)deploy/olm-catalog/argocd-operator/0.18.0/argoproj.io_notificationsconfigurations.yaml(1 hunks)go.mod(9 hunks)
✅ Files skipped from review due to trivial changes (1)
- deploy/olm-catalog/argocd-operator/0.18.0/argocd-operator-metrics-reader_rbac.authorization.k8s.io_v1_clusterrole.yaml
🚧 Files skipped from review as they are similar to previous changes (10)
- config/manager/kustomization.yaml
- deploy/olm-catalog/argocd-operator/0.18.0/argocd-operator-webhook-service_v1_service.yaml
- build/util/Dockerfile
- Makefile
- config/crd/bases/argoproj.io_appprojects.yaml
- deploy/olm-catalog/argocd-operator/0.18.0/argocd-operator-controller-manager-metrics-service_v1_service.yaml
- deploy/olm-catalog/argocd-operator/0.18.0/argoproj.io_notificationsconfigurations.yaml
- bundle/manifests/argoproj.io_argocdexports.yaml
- deploy/olm-catalog/argocd-operator/0.18.0/argocd-operator-namespacemanagement-viewer-role_rbac.authorization.k8s.io_v1_clusterrole.yaml
- deploy/olm-catalog/argocd-operator/0.18.0/argoproj.io_namespacemanagements.yaml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: build-operator
- GitHub Check: build
- GitHub Check: Run golangci-lint and gosec
- GitHub Check: Run end-to-end tests (v1.27.1)
🔇 Additional comments (27)
bundle/manifests/argocd-operator.clusterserviceversion.yaml (1)
260-260: LGTM! Version bump to 0.18.0 is consistent.The ClusterServiceVersion manifest has been correctly updated with the new version (v0.18.0) across all occurrences including metadata, image references, and version fields. The timestamp update reflects the new release date.
Also applies to: 266-266, 1940-1940, 2033-2033
config/crd/bases/argoproj.io_argocdexports.yaml (1)
230-242: LGTM! CRD reflects Kubernetes API evolution.The updated
volumeAttributesClassNamedescription clarifies the semantics for nil/empty values and introduces reset capability for Infeasible states. The removal of the Beta feature gate note suggests this field is graduating to GA in the Kubernetes version being targeted.deploy/olm-catalog/argocd-operator/0.18.0/argocd-operator-namespacemanagement-editor-role_rbac.authorization.k8s.io_v1_clusterrole.yaml (1)
1-24: LGTM! Standard RBAC editor role.The new ClusterRole for namespace management follows Kubernetes RBAC best practices with appropriate permissions for an editor role (full CRUD on resources, read-only on status).
deploy/olm-catalog/argocd-operator/0.18.0/argoproj.io_argocdexports.yaml (1)
1-291: LGTM! CRD manifest consistent with base definition.The new ArgoCDExport CRD manifest for the 0.18.0 OLM catalog correctly includes the same
volumeAttributesClassNameupdates as the base CRD definition.deploy/olm-catalog/argocd-operator/0.18.0/argocd-operator-manager-config_v1_configmap.yaml (1)
1-17: LGTM! Standard controller-runtime configuration.The new ConfigMap contains appropriate controller-runtime settings with secure defaults (metrics on localhost only, proper leader election configuration).
common/defaults.go (2)
73-73: LGTM! Argo CD version update aligns with PR objectives.The Argo CD default version has been correctly updated to v3.2.1 as per the PR objectives.
194-197: Verify Redis 8.x compatibility with Argo CD before upgrading.Redis 8.0 introduces breaking changes related to ACL rules and improved streaming and cluster management capabilities. Before deploying this Redis upgrade:
- Confirm Argo CD v3.2.1 officially supports Redis 8.x
- Review Redis Query Engine and data structure behavior changes
- Validate Redis 7.x data compatibility and any required migrations
go.mod (1)
3-3: LGTM! Go toolchain update aligns with Dockerfile.The Go toolchain upgrade to 1.25.0 is consistent with the Dockerfile change.
deploy/olm-catalog/argocd-operator/0.18.0/argoproj.io_appprojects.yaml (1)
1-373: LGTM - Standard AppProject CRD from Argo CD upstream.This CRD definition follows the expected schema for Argo CD's AppProject resource. The validation rules for required fields (e.g.,
groupandkindfor GroupKind objects,iatfor JWTToken,keyIDfor SignatureKeys) are properly defined.Note: Unlike the ImageUpdater CRD, this CRD does not define a status subresource. This is consistent with how Argo CD handles AppProject status updates directly on the resource.
deploy/olm-catalog/argocd-operator/0.18.0/argocd-image-updater.argoproj.io_imageupdaters.yaml (2)
1-541: LGTM - Well-structured ImageUpdater CRD with proper validations.The CRD is well-designed with:
- Proper use of
x-kubernetes-list-type: mapwithx-kubernetes-list-map-keysfor strategic merge behavior onapplicationRefsandimagesarrays- CEL validation rule (line 221-223) correctly enforces mutual exclusivity between
helmandkustomizeinmanifestTargets- Sensible defaults:
forceUpdate: false,updateStrategy: semver,method: argocd- Standard Kubernetes condition struct in status with all required fields
- Status subresource properly enabled
220-223: Verify CEL rule behavior with emptymanifestTargetsobject.The validation rule enforces exactly one of
helmorkustomizewhenmanifestTargetsis present. If a user specifies an emptymanifestTargets: {}block, this validation will reject it, which appears intentional based on the error message. Please confirm this is the desired behavior.config/crd/bases/argoproj.io_applicationsets.yaml (6)
705-706: Newrefreshboolean field in retry configuration looks correct.The addition of
refresh: type: booleanto the retry configuration across all applicable locations is consistent with the Argo CD v3.2.1 upgrade. This field enables triggering an application refresh after retry attempts.Also applies to: 1391-1392, 2078-2079, 2743-2744, 3433-3434, 4119-4120, 4806-4807, 5471-5472, 6144-6145, 7044-7045, 7935-7936, 8617-8618, 9309-9310, 9995-9996, 10682-10683, 11347-11348, 12020-12021, 12920-12921, 13811-13812, 14497-14498, 15169-15170, 16069-16070, 16960-16961, 17719-17720
6301-6311: New pull request generator fields added correctly.The
continueOnRepoNotFoundError: booleanandtitleMatch: stringfilter fields are new additions for the pull request generators. These expand configurability for PR-based ApplicationSet generation.Also applies to: 12177-12187, 15326-15336
17043-17044: NewdeletionOrderfield in strategy configuration.The
deletionOrder: type: stringfield understrategyproperties allows controlling the order in which applications are deleted during rollout. This is a valid addition from the Argo CD upgrade.
17826-17828: NewresourcesCountstatus field for tracking resource counts.The
resourcesCountfield (format: int64,type: integer) in the status section enables tracking the count of managed resources. This aligns with the AI summary mentioning resource count typings.
17837-17837: Status subresource declaration is correctly formatted.The
status: {}declaration properly enables the status subresource for the CRD, allowing independent status updates.
412-413: Verify the path validation regex pattern against upstream Argo CD v3.2.1.The new
pathfield validation (minLength: 1,pattern: ^.{2,}|[^./]$) is applied consistently across all syncSource definitions in this auto-generated CRD. The regex pattern behavior:
- Allows paths with 2+ characters (including
..)- Blocks single-character paths ending in
.or/(i.e.,.and/)- Permits single characters not ending in
.or/(i.e.,a)Without access to the upstream Argo CD v3.2.1 ApplicationSet CRD, this pattern cannot be confirmed as matching the upstream source. If this is a direct code generation from upstream as indicated in the original review context, it should match. Cross-reference with the v3.2.1 release to confirm the pattern is unchanged and intentional.
bundle/manifests/argoproj.io_applications.yaml (2)
114-118:retry.refreshfield is added consistently across spec and statusThe new
refreshboolean on the variousretryblocks is wired in consistently (spec.operation.retry, spec.syncPolicy.retry, and status.operationState.operation.retry) with matching descriptions and types, and without changing any required fields. This looks aligned and non-breaking for existing CRD consumers.Also applies to: 1949-1953, 2921-2926
1480-1495: HydratorsyncSource.pathvalidation and docs look coherent and symmetricThe updated
syncSource.pathdescription plusminLength: 1andpattern: ^.{2,}|[^./]$(and the clarifiedtargetBranchdescription) are applied uniformly in:
spec.sourceHydrator.syncSourcestatus.sourceHydrator.currentOperation.sourceHydrator.syncSourcestatus.sourceHydrator.lastSuccessfulOperation.sourceHydrator.syncSourceThis keeps spec and status schemas in sync and enforces a more sensible non-root path constraint for hydrated manifests. No issues from the CRD perspective.
If this CRD is meant to mirror upstream Argo CD v3.2.1 exactly, please double‑check these blocks against the upstream CRD to avoid future drift when bumping to 3.3.x.
Also applies to: 4895-4911, 4978-4994
bundle/manifests/argoproj.io_applicationsets.yaml (5)
706-707: LGTM - newrefreshboolean field.Additive schema field that provides declarative control over refresh behavior in sync policies. This change is backward-compatible and consistent with the Argo CD v3.2.1 upgrade.
6302-6312: LGTM - new pull request generator fields.The additions of
continueOnRepoNotFoundErrorfor resilience andtitleMatchfor enhanced filtering are useful extensions to the PR generator schema. Both are optional and backward-compatible.
17044-17045: LGTM - newdeletionOrderstrategy field.This field enables control over the order in which applications are deleted during rollout strategies, enhancing flexibility for progressive rollouts.
17827-17829: LGTM - newresourcesCountstatus field.Useful addition for tracking the number of resources managed by the ApplicationSet, aiding observability and monitoring use cases.
413-414: Path validation regex aligns with upstream Argo CD v3.2.1.The pattern
^.{2,}|[^./]$withminLength: 1is present in the upstream Argo CD v3.2.1 CRD and correctly applied across allsyncSource.pathfields. The pattern intentionally allows..and../prefixes for relative git repository paths while blocking invalid single-character paths (.and/).config/crd/bases/argoproj.io_applications.yaml (3)
113-116: Newrefreshretry flag is consistent; verify alignment with Argo CD v3.2.1 typesAdding the optional boolean
refreshunderoperation.retry,syncPolicy.retry, andstatus.operationState.operation.retrymatches the documented Argo CD retry flag (“use latest revision on retry instead of the initial one”) and is backward compatible as a non‑required field.(argo-cd.readthedocs.io)Please just confirm that this CRD is still generated directly from the Argo CD v3.2.1 Application types/CRDs so it won’t drift from upstream on future upgrades.
Also applies to: 1948-1951, 2920-2924
1479-1485: StrictersyncSource.pathvalidation and clarifiedtargetBranchsemanticsThe updated
syncSource.pathdescription plusminLength: 1andpattern: ^.{2,}|[^./]$effectively disallow only empty,".", and"/"paths, which aligns with the comment that the path must not be the repo root. The same schema and the expandedtargetBranchdescription are consistently applied in spec and both status locations, which is good.Please double‑check that:
- No existing
Applicationresources (or examples/docs) rely on"."or"/"forsourceHydrator.syncSource.path, and- The described
targetBranchbehavior matches what the controller actually does whenhydrateTois present/absent.Also applies to: 1487-1490, 4895-4901, 4903-4906, 4979-4985, 4987-4989
5913-5915:subresources: {}is effectively a no‑op; confirm no expectation of/statussubresourceDeclaring
subresources: {}on thev1alpha1version is syntactically valid and does not enable any subresources (nostatus/scaleentries), so behavior should be unchanged compared to havingsubresourcesomitted entirely.If any controller or tooling around
Applicationis expecting a/statussubresource (e.g., usingUpdateStatusrather than full-object updates), please confirm that the upstream Argo CD CRD for v3.2.1 also omits thestatussubresource so everything stays consistent.
| @@ -1,5 +1,5 @@ | |||
| # Build the manager binary | |||
| FROM golang:1.24 AS builder | |||
| FROM golang:1.25 AS builder | |||
There was a problem hiding this comment.
Go 1.25 upgrade is compatible; verify environment-specific changes.
The golang base image upgrade from 1.24 to 1.25 is a standard minor version bump. Ensure your build environment meets the macOS 12 Monterey or later requirement if applicable. The upgrade includes updated GOMAXPROCS behavior in containerized environments—verify this aligns with your deployment setup.
🤖 Prompt for AI Agents
Dockerfile lines 2-2: base image was bumped to golang:1.25; confirm and adjust
environment constraints and runtime settings to accommodate this minor Go
upgrade. Verify CI and any developer macOS hosts meet macOS Monterey (12) or
later requirements if using local macOS toolchains; run full build/test matrix
in CI (linux/macos/arm64/x86_64) to catch platform issues. Check containerized
GOMAXPROCS behavior changes by explicitly setting GOMAXPROCS (via ENV GOMAXPROCS
or runtime configuration) or relying on GOTRACEBACK/GODEBUG settings as
appropriate for your deployment, and update any docs/README and deployment
manifests to reflect the new Go version and required environment constraints.
| github.com/argoproj/argo-cd/v3 v3.2.1 | ||
| github.com/argoproj/gitops-engine v0.7.1-0.20251108235403-13d5172d3ee2 |
There was a problem hiding this comment.
Clarify dependency version terminology and verify Kubernetes 1.34 compatibility.
The review comment contains terminology inaccuracy and compatibility concerns:
-
Version Clarity: "Kubernetes v0.34.0" should be "client-go v0.34.0" (which corresponds to Kubernetes v1.34.0). The upgrade from v0.33.1 to v0.34.0 is a minor version bump (1.33 → 1.34), not a major API surface change.
-
Argo CD v3.2.1 Compatibility: Argo CD v3.1 is compatible with Kubernetes versions v1.33 to v1.30. Verify that Argo CD v3.2.1 extends support to Kubernetes v1.34.0.
-
structured-merge-diff: Since Kubernetes v1.17.0, when syncing code from the staging area, the Kubernetes version tags are synced to client-go. From Kubernetes v1.17.0, matching semver v0.x.y tags are created for each v1.x.y Kubernetes release. Confirm whether structured-merge-diff/v6 is a new addition or a standard dependency in the upgrade.
-
Compatibility Model: Argo CD follows a support scheme for N, N-1 Kubernetes versions, which aligns with standard Kubernetes release practices.
🤖 Prompt for AI Agents
In go.mod around lines 6-7, the review points out terminology and compatibility
inaccuracies: change any reference that says "Kubernetes v0.34.0" to "client-go
v0.34.0 (Kubernetes v1.34.0)", verify Argo CD v3.2.1 explicitly supports
Kubernetes v1.34.0 by checking Argo CD release notes and adjust the argo-cd
module or compatibility comment accordingly, confirm whether
structured-merge-diff/v6 should be present (if it was introduced as part of
client-go sync or is required by the new client-go) and add/remove or pin the
structured-merge-diff version as appropriate, update go.mod to the correct
client-go version if bumping (and update go.sum), run go mod tidy and run
unit/integration tests to validate the compatibility change, and add a short
comment in go.mod clarifying the client-go vs Kubernetes wording and the
supported Kubernetes versions for Argo CD.
Signed-off-by: Jonathan West <jonwest@redhat.com> Signed-off-by: akhil nittala <nakhil@redhat.com>
2 participants
What type of PR is this?
/kind chore
What does this PR do / why we need it:
Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.