fix(controller)!: argocd-update: require desired revision to be speci…

…fied when using updateTargetRevision (akuity#3042)

Signed-off-by: Kent Rancourt <kent.rancourt@gmail.com>

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.<br/><br/>__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. |

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{

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] }
           }
         },
         {

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

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."
         }
        }
       }