feat: allow argocd-update steps to learn desired revisions for source…

…s from previous steps (akuity#2585)

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

internal/directives/argocd_revisions.go

@@ -35,10 +35,25 @@ func (a *argocdUpdater) getDesiredRevisions(
 	}
 	revisions := make([]string, len(sources))
 	for i, src := range sources {
+		sourceUpdate := a.findSourceUpdate(update, src)
+		// If there is a source update that targets this source, it might be
+		// specific about a previous step whose output should be used as the desired
+		// revision.
+		if sourceUpdate != nil {
+			var err error
+			if revisions[i], err = getCommitFromStep(
+				stepCtx.SharedState,
+				sourceUpdate.DesiredCommitFromStep,
+			); err != nil {
+				return nil, err
+			}
+			if revisions[i] != "" {
+				continue
+			}
+		}
 		var desiredOrigin *kargoapi.FreightOrigin
 		// If there is a source update that targets this source, it might be
 		// specific about which origin the desired revision should come from.
-		sourceUpdate := a.findSourceUpdate(update, src)
 		if sourceUpdate != nil {
 			desiredOrigin = getDesiredOrigin(stepCfg, sourceUpdate)
 		} else {
@@ -147,3 +162,31 @@ func (a *argocdUpdater) findSourceUpdate(
 	}
 	return nil
 }
+
+func getCommitFromStep(sharedState State, stepAlias string) (string, error) {
+	if stepAlias == "" {
+		return "", nil
+	}
+	stepOutput, exists := sharedState.Get(stepAlias)
+	if !exists {
+		return "", fmt.Errorf("no output found from step with alias %q", stepAlias)
+	}
+	stepOutputState, ok := stepOutput.(State)
+	if !ok {
+		return "",
+			fmt.Errorf("output from step with alias %q is not a State", stepAlias)
+	}
+	commitAny, exists := stepOutputState.Get(commitKey)
+	if !exists {
+		return "",
+			fmt.Errorf("no commit found in output from step with alias %q", stepAlias)
+	}
+	commit, ok := commitAny.(string)
+	if !ok {
+		return "", fmt.Errorf(
+			"commit in output from step with alias %q is not a string",
+			stepAlias,
+		)
+	}
+	return commit, nil
+}

internal/directives/git_commiter.go

@@ -114,9 +114,9 @@ func (g *gitCommitter) buildCommitMessage(
 	var commitMsg string
 	if cfg.Message != "" {
 		commitMsg = cfg.Message
-	} else if len(cfg.MessageFrom) > 0 {
-		commitMsgParts := make([]string, len(cfg.MessageFrom))
-		for i, alias := range cfg.MessageFrom {
+	} else if len(cfg.MessageFromSteps) > 0 {
+		commitMsgParts := make([]string, len(cfg.MessageFromSteps))
+		for i, alias := range cfg.MessageFromSteps {
 			stepOutput, exists := sharedState.Get(alias)
 			if !exists {
 				return "", fmt.Errorf(

internal/directives/git_commiter_test.go

@@ -37,17 +37,17 @@ func Test_gitCommitter_validate(t *testing.T) {
 			},
 		},
 		{
-			name:   "neither message nor messageFrom is specified",
+			name:   "neither message nor messageFromSteps is specified",
 			config: Config{},
 			expectedProblems: []string{
 				"(root): Must validate one and only one schema",
 			},
 		},
 		{
-			name: "both message and messageFrom are specified",
+			name: "both message and messageFromSteps are specified",
 			config: Config{
-				"message":     "fake commit message",
-				"messageFrom": []string{"fake-step-alias"},
+				"message":          "fake commit message",
+				"messageFromSteps": []string{"fake-step-alias"},
 			},
 			expectedProblems: []string{
 				"(root): Must validate one and only one schema",
@@ -63,21 +63,21 @@ func Test_gitCommitter_validate(t *testing.T) {
 			},
 		},
 		{
-			name: "messageFrom is empty array",
+			name: "messageFromSteps is empty array",
 			config: Config{
-				"messageFrom": []string{},
+				"messageFromSteps": []string{},
 			},
 			expectedProblems: []string{
-				"messageFrom: Array must have at least 1 items",
+				"messageFromSteps: Array must have at least 1 items",
 			},
 		},
 		{
-			name: "messageFrom array contains an empty string",
+			name: "messageFromSteps array contains an empty string",
 			config: Config{
-				"messageFrom": []string{""},
+				"messageFromSteps": []string{""},
 			},
 			expectedProblems: []string{
-				"messageFrom.0: String length must be greater than or equal to 1",
+				"messageFromSteps.0: String length must be greater than or equal to 1",
 			},
 		},
 		{
@@ -250,7 +250,7 @@ func Test_gitCommitter_buildCommitMessage(t *testing.T) {
 		{
 			name:        "no output from step with alias",
 			sharedState: State{},
-			cfg:         GitCommitConfig{MessageFrom: []string{"fake-step-alias"}},
+			cfg:         GitCommitConfig{MessageFromSteps: []string{"fake-step-alias"}},
 			assertions: func(t *testing.T, _ string, err error) {
 				require.ErrorContains(t, err, "no output found from step with alias")
 			},
@@ -260,7 +260,7 @@ func Test_gitCommitter_buildCommitMessage(t *testing.T) {
 			sharedState: State{
 				"fake-step-alias": "not a State",
 			},
-			cfg: GitCommitConfig{MessageFrom: []string{"fake-step-alias"}},
+			cfg: GitCommitConfig{MessageFromSteps: []string{"fake-step-alias"}},
 			assertions: func(t *testing.T, _ string, err error) {
 				require.ErrorContains(t, err, "output from step with alias")
 				require.ErrorContains(t, err, "is not a State")
@@ -271,7 +271,7 @@ func Test_gitCommitter_buildCommitMessage(t *testing.T) {
 			sharedState: State{
 				"fake-step-alias": State{},
 			},
-			cfg: GitCommitConfig{MessageFrom: []string{"fake-step-alias"}},
+			cfg: GitCommitConfig{MessageFromSteps: []string{"fake-step-alias"}},
 			assertions: func(t *testing.T, _ string, err error) {
 				require.ErrorContains(
 					t, err, "no commit message found in output from step with alias",
@@ -285,7 +285,7 @@ func Test_gitCommitter_buildCommitMessage(t *testing.T) {
 					"commitMessage": 42,
 				},
 			},
-			cfg: GitCommitConfig{MessageFrom: []string{"fake-step-alias"}},
+			cfg: GitCommitConfig{MessageFromSteps: []string{"fake-step-alias"}},
 			assertions: func(t *testing.T, _ string, err error) {
 				require.ErrorContains(
 					t, err, "commit message in output from step with alias",
@@ -304,7 +304,7 @@ func Test_gitCommitter_buildCommitMessage(t *testing.T) {
 				},
 			},
 			cfg: GitCommitConfig{
-				MessageFrom: []string{
+				MessageFromSteps: []string{
 					"fake-step-alias",
 					"another-fake-step-alias",
 				},

internal/directives/git_pr_opener.go

@@ -182,32 +182,32 @@ func (g *gitPROpener) runPromotionStep(
 
 func getSourceBranch(sharedState State, cfg GitOpenPRConfig) (string, error) {
 	sourceBranch := cfg.SourceBranch
-	if cfg.SourceBranchFrom != "" {
-		stepOutput, exists := sharedState.Get(cfg.SourceBranchFrom)
+	if cfg.SourceBranchFromStep != "" {
+		stepOutput, exists := sharedState.Get(cfg.SourceBranchFromStep)
 		if !exists {
 			return "", fmt.Errorf(
 				"no output found from step with alias %q",
-				cfg.SourceBranchFrom,
+				cfg.SourceBranchFromStep,
 			)
 		}
 		stepOutputState, ok := stepOutput.(State)
 		if !ok {
 			return "", fmt.Errorf(
 				"output from step with alias %q is not a State",
-				cfg.SourceBranchFrom,
+				cfg.SourceBranchFromStep,
 			)
 		}
 		sourceBranchAny, exists := stepOutputState.Get(branchKey)
 		if !exists {
 			return "", fmt.Errorf(
 				"no branch found in output from step with alias %q",
-				cfg.SourceBranchFrom,
+				cfg.SourceBranchFromStep,
 			)
 		}
 		if sourceBranch, ok = sourceBranchAny.(string); !ok {
 			return "", fmt.Errorf(
 				"branch name in output from step with alias %q is not a string",
-				cfg.SourceBranchFrom,
+				cfg.SourceBranchFromStep,
 			)
 		}
 	}

internal/directives/git_pr_opener_test.go

@@ -54,17 +54,17 @@ func Test_gitPROpener_validate(t *testing.T) {
 			},
 		},
 		{
-			name:   "neither sourceBranch nor sourceBranchFrom specified",
+			name:   "neither sourceBranch nor sourceBranchFromStep specified",
 			config: Config{},
 			expectedProblems: []string{
 				"(root): Must validate one and only one schema",
 			},
 		},
 		{
-			name: "both sourceBranch and sourceBranchFrom specified",
+			name: "both sourceBranch and sourceBranchFromStep specified",
 			config: Config{
-				"sourceBranch":     "main",
-				"sourceBranchFrom": "push",
+				"sourceBranch":         "main",
+				"sourceBranchFromStep": "push",
 			},
 			expectedProblems: []string{
 				"(root): Must validate one and only one schema",
@@ -99,10 +99,10 @@ func Test_gitPROpener_validate(t *testing.T) {
 		{
 			name: "valid with source branch from push",
 			config: Config{
-				"provider":         "github",
-				"repoURL":          "https://github.com/example/repo.git",
-				"sourceBranchFrom": "fake-step",
-				"targetBranch":     "fake-branch",
+				"provider":             "github",
+				"repoURL":              "https://github.com/example/repo.git",
+				"sourceBranchFromStep": "fake-step",
+				"targetBranch":         "fake-branch",
 			},
 		},
 	}
@@ -207,10 +207,10 @@ func Test_gitPROpener_runPromotionStep(t *testing.T) {
 		GitOpenPRConfig{
 			RepoURL: testRepoURL,
 			// We get slightly better coverage by using this option
-			SourceBranchFrom:   "fake-step",
-			TargetBranch:       testTargetBranch,
-			CreateTargetBranch: true,
-			Provider:           ptr.To(Provider(fakeGitProviderName)),
+			SourceBranchFromStep: "fake-step",
+			TargetBranch:         testTargetBranch,
+			CreateTargetBranch:   true,
+			Provider:             ptr.To(Provider(fakeGitProviderName)),
 		},
 	)
 	require.NoError(t, err)

internal/directives/git_pr_waiter.go

@@ -124,7 +124,10 @@ func (g *gitPRWaiter) runPromotionStep(
 			fmt.Errorf("pull request %d was closed without being merged", prNumber)
 	}
 
-	return PromotionStepResult{Status: PromotionStatusSuccess}, nil
+	return PromotionStepResult{
+		Status: PromotionStatusSuccess,
+		Output: State{commitKey: pr.MergeCommitSHA},
+	}, nil
 }
 
 func getPRNumber(sharedState State, cfg GitWaitForPRConfig) (int64, error) {

internal/directives/schemas/argocd-update-config.json

@@ -43,6 +43,11 @@
           "description": "If applicable, identifies a specific chart within the Helm chart repository specified by the 'repoURL' field. When the source to be updated references a Helm chart repository, the values of the 'repoURL' and 'chart' fields should exactly match the values of the same fields in the source. i.e. Do not match the values of these two fields to your Warehouse; match them to the Application source you wish to update.",
           "minLength": 1
         },
+        "desiredCommitFromStep": {
+          "type": "string",
+          "description": "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. If this is left undefined, the desired revision will be determined by Freight (if possible). Note that the source's 'targetRevision' will not be updated to this commit unless 'updateTargetRevision=true' is set. The utility of this field is to ensure that health checks on Argo CD ApplicationSources can account for scenarios where the desired revision differs from what may be found in Freight, likely due to the use of rendered branches and/or PR-based promotion workflows.",
+          "minLength": 1
+        },
         "fromOrigin": {
           "$ref": "#definitions/origin"
         },

internal/directives/schemas/git-commit-config.json

@@ -30,10 +30,10 @@
     },
     "message": {
       "type": "string",
-      "description": "The commit message. Mutually exclusive with 'messageFrom'.",
+      "description": "The commit message. Mutually exclusive with 'messageFromSteps'.",
       "minLength": 1
     },
-    "messageFrom": {
+    "messageFromSteps": {
       "type": "array",
       "description": "TODO",
       "minItems": 1,
@@ -52,11 +52,11 @@
     {
       "required": ["message"],
       "properties": {
-        "messageFrom": { "enum": [null] }
+        "messageFromSteps": { "enum": [null] }
       }
     },
     {
-      "required": ["messageFrom"],
+      "required": ["messageFromSteps"],
       "properties": {
         "message": { "enum": [null] }
       }

internal/directives/schemas/git-open-pr-config.json

@@ -29,7 +29,7 @@
       "description": "The branch containing the changes to be merged. This branch must already exist and be up to date on the remote.",
       "minLength": 1
     },
-    "sourceBranchFrom": {
+    "sourceBranchFromStep": {
       "type": "string",
       "description": "References the 'branch' output from a previous step. This step will use that value as the source branch.",
       "minLength": 1
@@ -44,11 +44,11 @@
     {
       "required": ["sourceBranch"],
       "properties": {
-        "sourceBranchFrom": {  "enum": ["", null] }
+        "sourceBranchFromStep": {  "enum": ["", null] }
       }
     },
     {
-      "required": ["sourceBranchFrom"],
+      "required": ["sourceBranchFromStep"],
       "properties": {
         "sourceBranch": {  "enum": ["", null] }
       }