From 7a5f1ec7c0cd37c0846da1efca0f392ea4dcb250 Mon Sep 17 00:00:00 2001 From: Kent Rancourt Date: Tue, 3 Dec 2024 16:48:00 -0500 Subject: [PATCH] fix(controller)!: argocd-update: require desired revision to be specified when using updateTargetRevision (#3042) Signed-off-by: Kent Rancourt --- docs/docs/35-references/10-promotion-steps.md | 2 +- internal/directives/argocd_updater_test.go | 14 ++++++++++++++ .../directives/schemas/argocd-update-config.json | 5 +++-- internal/directives/simple_engine_promote.go | 4 ++++ internal/directives/zz_config_types.go | 3 ++- ui/src/gen/directives/argocd-update-config.json | 6 +++--- 6 files changed, 27 insertions(+), 7 deletions(-) diff --git a/docs/docs/35-references/10-promotion-steps.md b/docs/docs/35-references/10-promotion-steps.md index 3669f05d8..2ea6ad45d 100644 --- a/docs/docs/35-references/10-promotion-steps.md +++ b/docs/docs/35-references/10-promotion-steps.md @@ -1026,7 +1026,7 @@ spec: | `apps[].sources[].chart` | `string` | N | Applicable only when the target `ApplicationSource` references a Helm chart repository, the value of the target `ApplicationSource`'s own `chart` field. This must match exactly. | | `apps[].sources[].desiredRevision` | `string` | N | Specifies the desired revision for the source. i.e. The revision to which the source must be observably synced when performing a health check. This field is mutually exclusive with `desiredCommitFromStep`. Prior to v1.1.0, if both were left undefined, the desired revision was determined by Freight (if possible). Beginning with v1.1.0, if both are left undefined, Kargo will not require the source to be observably synced to any particular source to be considered healthy. Note that the source's `targetRevision` will not be updated to this revision unless `updateTargetRevision=true` is also set. | | `apps[].sources[].desiredCommitFromStep` | `string` | N | Applicable only when `repoURL` references a Git repository, this field references the `commit` output from a previous step and uses it as the desired revision for the source. i.e. The revision to which the source must be observably synced when performing a health check. This field is mutually exclusive with `desiredRevisionFromStep`. Prior to v1.1.0, if both were left undefined, the desired revision was determined by Freight (if possible). Beginning with v1.1.0, if both are left undefined, Kargo will not require the source to be observably synced to any particular source to be considered healthy. Note that the source's `targetRevision` will not be updated to this commit unless `updateTargetRevision=true` is also set.

__Deprecated: Use `desiredRevision` with an expression instead. Will be removed in v1.3.0.__ | -| `apps[].sources[].updateTargetRevision` | `boolean` | Y | Indicates whether the target `ApplicationSource` should be updated such that its `targetRevision` field points directly at the desired revision. A `true` value in this field has no effect if `desiredRevision` and `desiredCommitFromStep` are both undefined. | +| `apps[].sources[].updateTargetRevision` | `boolean` | Y | Indicates whether the target `ApplicationSource` should be updated such that its `targetRevision` field points directly at the desired revision. A `true` value in this field requires exactly one of `desiredCommitFromStep` or `desiredRevision` to be specified. | | `apps[].sources[].kustomize` | `object` | N | Describes updates to an Argo CD `ApplicationSource`'s Kustomize-specific properties. | | `apps[].sources[].kustomize.images` | `[]object` | Y | Describes how to update an Argo CD `ApplicationSource`'s Kustomize-specific properties to reference specific versions of container images. | | `apps[].sources[].kustomize.images[].repoURL` | `string` | Y | URL of the image being updated. | diff --git a/internal/directives/argocd_updater_test.go b/internal/directives/argocd_updater_test.go index b06b5fb09..f008ef8fe 100644 --- a/internal/directives/argocd_updater_test.go +++ b/internal/directives/argocd_updater_test.go @@ -136,6 +136,19 @@ func Test_argoCDUpdater_validate(t *testing.T) { "apps.0.sources.0: Must validate one and only one schema", }, }, + { + name: "targetRevision=true with desiredCommitFromStep and desiredRevision unspecified", + config: Config{ + "apps": []Config{{ + "sources": []Config{{ + "updateTargetRevision": true, + }}, + }}, + }, + expectedProblems: []string{ + "apps.0.sources.0: Must validate one and only one schema", + }, + }, { name: "helm images is empty array", config: Config{ @@ -347,6 +360,7 @@ func Test_argoCDUpdater_validate(t *testing.T) { "namespace": "argocd", "sources": []Config{{ "repoURL": "fake-git-url", + "desiredRevision": "fake-commit", "updateTargetRevision": true, "helm": Config{ "images": []Config{ diff --git a/internal/directives/schemas/argocd-update-config.json b/internal/directives/schemas/argocd-update-config.json index 83ee387e3..e6b075d89 100644 --- a/internal/directives/schemas/argocd-update-config.json +++ b/internal/directives/schemas/argocd-update-config.json @@ -69,14 +69,15 @@ }, "updateTargetRevision": { "type": "boolean", - "description": "Indicates whether the source should be updated such that its 'targetRevision' field points directly at the desired revision." + "description": "Indicates whether the source should be updated such that its 'targetRevision' field points directly at the desired revision. If set to true, exactly one of 'desiredCommitFromStep' or 'desiredRevision' must be specified." } }, "oneOf": [ { "properties": { "desiredCommitFromStep": { "enum": ["", null] }, - "desiredRevision": { "enum": ["", null] } + "desiredRevision": { "enum": ["", null] }, + "updateTargetRevision": { "enum": [false, null] } } }, { diff --git a/internal/directives/simple_engine_promote.go b/internal/directives/simple_engine_promote.go index b62593653..cb221a04a 100644 --- a/internal/directives/simple_engine_promote.go +++ b/internal/directives/simple_engine_promote.go @@ -179,6 +179,10 @@ func (e *SimpleEngine) executeStep( ) (PromotionStepResult, error) { stepCtx, err := e.preparePromotionStepContext(ctx, promoCtx, step, reg.Permissions, workDir, state) if err != nil { + // TODO(krancour): We're not yet distinguishing between retryable and + // non-retryable errors. When we start to do this, failure to prepare the + // step context (likely due to invalid configuration) should be considered + // non-retryable. return PromotionStepResult{ Status: kargoapi.PromotionPhaseErrored, }, err diff --git a/internal/directives/zz_config_types.go b/internal/directives/zz_config_types.go index b9939c717..6108aa303 100644 --- a/internal/directives/zz_config_types.go +++ b/internal/directives/zz_config_types.go @@ -59,7 +59,8 @@ type ArgoCDAppSourceUpdate struct { // to your Warehouse; match them to the Application source you wish to update. RepoURL string `json:"repoURL"` // Indicates whether the source should be updated such that its 'targetRevision' field - // points directly at the desired revision. + // points directly at the desired revision. If set to true, exactly one of + // 'desiredCommitFromStep' or 'desiredRevision' must be specified. UpdateTargetRevision bool `json:"updateTargetRevision,omitempty"` } diff --git a/ui/src/gen/directives/argocd-update-config.json b/ui/src/gen/directives/argocd-update-config.json index 449f39ba2..e67bdf0ec 100644 --- a/ui/src/gen/directives/argocd-update-config.json +++ b/ui/src/gen/directives/argocd-update-config.json @@ -223,7 +223,7 @@ }, "updateTargetRevision": { "type": "boolean", - "description": "Indicates whether the source should be updated such that its 'targetRevision' field points directly at the desired revision." + "description": "Indicates whether the source should be updated such that its 'targetRevision' field points directly at the desired revision. If set to true, exactly one of 'desiredCommitFromStep' or 'desiredRevision' must be specified." } } } @@ -416,7 +416,7 @@ }, "updateTargetRevision": { "type": "boolean", - "description": "Indicates whether the source should be updated such that its 'targetRevision' field points directly at the desired revision." + "description": "Indicates whether the source should be updated such that its 'targetRevision' field points directly at the desired revision. If set to true, exactly one of 'desiredCommitFromStep' or 'desiredRevision' must be specified." } } }, @@ -893,7 +893,7 @@ }, "updateTargetRevision": { "type": "boolean", - "description": "Indicates whether the source should be updated such that its 'targetRevision' field points directly at the desired revision." + "description": "Indicates whether the source should be updated such that its 'targetRevision' field points directly at the desired revision. If set to true, exactly one of 'desiredCommitFromStep' or 'desiredRevision' must be specified." } } }