Skip to content

Commit 37f2793

Browse files
fix(hydrator): omit Argocd- trailers from hydrator.metadata (cherry-pick #23463) (#23621)
Signed-off-by: Michael Crenshaw <[email protected]> Co-authored-by: Michael Crenshaw <[email protected]>
1 parent 7224a15 commit 37f2793

File tree

6 files changed

+124
-77
lines changed

6 files changed

+124
-77
lines changed

commitserver/commit/commit.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -220,6 +220,7 @@ type hydratorMetadataFile struct {
220220
// Subject is the subject line of the DRY commit message, i.e. `git show --format=%s`.
221221
Subject string `json:"subject,omitempty"`
222222
// Body is the body of the DRY commit message, excluding the subject line, i.e. `git show --format=%b`.
223+
// Known Argocd- trailers with valid values are removed, but all other trailers are kept.
223224
Body string `json:"body,omitempty"`
224225
References []v1alpha1.RevisionReference `json:"references,omitempty"`
225226
}

commitserver/commit/hydratorhelper.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import (
1818

1919
"github.com/argoproj/argo-cd/v3/commitserver/apiclient"
2020
appv1 "github.com/argoproj/argo-cd/v3/pkg/apis/application/v1alpha1"
21+
"github.com/argoproj/argo-cd/v3/util/git"
2122
"github.com/argoproj/argo-cd/v3/util/io"
2223
)
2324

@@ -48,8 +49,10 @@ func WriteForPaths(root *os.Root, repoUrl, drySha string, dryCommitMetadata *app
4849

4950
subject, body, _ := strings.Cut(message, "\n\n")
5051

52+
_, bodyMinusTrailers := git.GetReferences(log.WithFields(log.Fields{"repo": repoUrl, "revision": drySha}), body)
53+
5154
// Write the top-level readme.
52-
err := writeMetadata(root, "", hydratorMetadataFile{DrySHA: drySha, RepoURL: repoUrl, Author: author, Subject: subject, Body: body, Date: date, References: references})
55+
err := writeMetadata(root, "", hydratorMetadataFile{DrySHA: drySha, RepoURL: repoUrl, Author: author, Subject: subject, Body: bodyMinusTrailers, Date: date, References: references})
5356
if err != nil {
5457
return fmt.Errorf("failed to write top-level hydrator metadata: %w", err)
5558
}

commitserver/commit/hydratorhelper_test.go

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ import (
99
"os"
1010
"path"
1111
"path/filepath"
12-
"strings"
1312
"testing"
1413
"time"
1514

@@ -73,9 +72,13 @@ func TestWriteForPaths(t *testing.T) {
7372

7473
now := metav1.NewTime(time.Now())
7574
metadata := &appsv1.RevisionMetadata{
76-
Author: "test-author",
77-
Date: &now,
78-
Message: "test-message",
75+
Author: "test-author",
76+
Date: &now,
77+
Message: `test-message
78+
79+
Signed-off-by: Test User <[email protected]>
80+
Argocd-reference-commit-sha: abc123
81+
`,
7982
References: []appsv1.RevisionReference{
8083
{
8184
Commit: &appsv1.CommitMetadata{
@@ -97,16 +100,15 @@ func TestWriteForPaths(t *testing.T) {
97100
topMetadataBytes, err := os.ReadFile(topMetadataPath)
98101
require.NoError(t, err)
99102

100-
expectedSubject, expectedBody, _ := strings.Cut(metadata.Message, "\n\n")
101-
102103
var topMetadata hydratorMetadataFile
103104
err = json.Unmarshal(topMetadataBytes, &topMetadata)
104105
require.NoError(t, err)
105106
assert.Equal(t, repoURL, topMetadata.RepoURL)
106107
assert.Equal(t, drySha, topMetadata.DrySHA)
107108
assert.Equal(t, metadata.Author, topMetadata.Author)
108-
assert.Equal(t, expectedSubject, topMetadata.Subject)
109-
assert.Equal(t, expectedBody, topMetadata.Body)
109+
assert.Equal(t, "test-message", topMetadata.Subject)
110+
// The body should exclude the Argocd- trailers.
111+
assert.Equal(t, "Signed-off-by: Test User <[email protected]>\n", topMetadata.Body)
110112
assert.Equal(t, metadata.Date.Format(time.RFC3339), topMetadata.Date)
111113
assert.Equal(t, metadata.References, topMetadata.References)
112114

docs/user-guide/source-hydrator.md

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -230,6 +230,12 @@ The commit metadata will appear in the hydrated commit's root hydrator.metadata
230230
231231
```json
232232
{
233+
"author": "CI <[email protected]>",
234+
"subject": "chore: bump image to b82add2",
235+
"date": "2025-06-09T13:50:08-04:00",
236+
"body": "Signed-off-by: CI <[email protected]>\n",
237+
"drySha": "6cb951525937865dced818bbdd78c89b2d2b3045",
238+
"repoURL": "https://git.example.com/owner/manifests-repo",
233239
"references": [
234240
{
235241
"commit": {
@@ -248,6 +254,10 @@ The commit metadata will appear in the hydrated commit's root hydrator.metadata
248254
}
249255
```
250256
257+
The top-level "body" field contains the commit message of the DRY commit minus the subject line and any
258+
`Argocd-reference-commit-*` trailers that were used in `references`. Unrecognized or invalid trailers are preserved in
259+
the body.
260+
251261
Although `references` is an array, the source hydrator currently only supports a single related commit. If a trailer is
252262
specified more than once, the last one will be used.
253263

util/git/client.go

Lines changed: 67 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -790,7 +790,7 @@ func (m *nativeGitClient) RevisionMetadata(revision string) (*RevisionMetadata,
790790
if err != nil {
791791
return nil, fmt.Errorf("failed to interpret trailers for revision %q in repo %q: %w", revision, m.repoURL, err)
792792
}
793-
relatedCommits := getReferences(log.WithFields(log.Fields{"repo": m.repoURL, "revision": revision}), out)
793+
relatedCommits, _ := GetReferences(log.WithFields(log.Fields{"repo": m.repoURL, "revision": revision}), out)
794794

795795
out, err = m.runCmd("tag", "--points-at", revision)
796796
if err != nil {
@@ -816,65 +816,23 @@ func truncate(str string) string {
816816

817817
var shaRegex = regexp.MustCompile(`^[0-9a-f]{5,40}$`)
818818

819-
// getReferences extracts related commit metadata from the commit message trailers. If referenced commit
819+
// GetReferences extracts related commit metadata from the commit message trailers. If referenced commit
820820
// metadata is present, we return a slice containing a single metadata object. If no related commit metadata is found,
821821
// we return a nil slice.
822822
//
823823
// If a trailer fails validation, we log an error and skip that trailer. We truncate the trailer values to 100
824824
// characters to avoid excessively long log messages.
825-
func getReferences(logCtx *log.Entry, commitMessageBody string) []RevisionReference {
825+
//
826+
// We also return the commit message body with all valid Argocd-reference-commit-* trailers removed.
827+
func GetReferences(logCtx *log.Entry, commitMessageBody string) ([]RevisionReference, string) {
828+
unrelatedLines := strings.Builder{}
826829
var relatedCommit CommitMetadata
827830
scanner := bufio.NewScanner(strings.NewReader(commitMessageBody))
828831
for scanner.Scan() {
829832
line := scanner.Text()
830-
if !strings.HasPrefix(line, "Argocd-reference-commit-") {
831-
continue
832-
}
833-
parts := strings.SplitN(line, ": ", 2)
834-
if len(parts) != 2 {
835-
continue
836-
}
837-
trailerKey := parts[0]
838-
trailerValue := parts[1]
839-
switch trailerKey {
840-
case "Argocd-reference-commit-repourl":
841-
_, err := url.Parse(trailerValue)
842-
if err != nil {
843-
logCtx.Errorf("failed to parse repo URL %q: %v", truncate(trailerValue), err)
844-
continue
845-
}
846-
relatedCommit.RepoURL = trailerValue
847-
case "Argocd-reference-commit-author":
848-
address, err := mail.ParseAddress(trailerValue)
849-
if err != nil || address == nil {
850-
logCtx.Errorf("failed to parse author email %q: %v", truncate(trailerValue), err)
851-
continue
852-
}
853-
relatedCommit.Author = *address
854-
case "Argocd-reference-commit-date":
855-
// Validate that it's the correct date format.
856-
t, err := time.Parse(time.RFC3339, trailerValue)
857-
if err != nil {
858-
logCtx.Errorf("failed to parse date %q with RFC3339 format: %v", truncate(trailerValue), err)
859-
continue
860-
}
861-
relatedCommit.Date = t.Format(time.RFC3339)
862-
case "Argocd-reference-commit-subject":
863-
relatedCommit.Subject = trailerValue
864-
case "Argocd-reference-commit-body":
865-
body := ""
866-
err := json.Unmarshal([]byte(trailerValue), &body)
867-
if err != nil {
868-
logCtx.Errorf("failed to parse body %q as JSON: %v", truncate(trailerValue), err)
869-
continue
870-
}
871-
relatedCommit.Body = body
872-
case "Argocd-reference-commit-sha":
873-
if !shaRegex.MatchString(trailerValue) {
874-
logCtx.Errorf("invalid commit SHA %q in trailer %s: must be a lowercase hex string 5-40 characters long", truncate(trailerValue), trailerKey)
875-
continue
876-
}
877-
relatedCommit.SHA = trailerValue
833+
updated := updateCommitMetadata(logCtx, &relatedCommit, line)
834+
if !updated {
835+
unrelatedLines.WriteString(line + "\n")
878836
}
879837
}
880838
var relatedCommits []RevisionReference
@@ -883,7 +841,64 @@ func getReferences(logCtx *log.Entry, commitMessageBody string) []RevisionRefere
883841
Commit: &relatedCommit,
884842
})
885843
}
886-
return relatedCommits
844+
return relatedCommits, unrelatedLines.String()
845+
}
846+
847+
// updateCommitMetadata checks if the line is a valid Argocd-reference-commit-* trailer. If so, it updates
848+
// the relatedCommit object and returns true. If the line is not a valid trailer, it returns false.
849+
func updateCommitMetadata(logCtx *log.Entry, relatedCommit *CommitMetadata, line string) bool {
850+
if !strings.HasPrefix(line, "Argocd-reference-commit-") {
851+
return false
852+
}
853+
parts := strings.SplitN(line, ": ", 2)
854+
if len(parts) != 2 {
855+
return false
856+
}
857+
trailerKey := parts[0]
858+
trailerValue := parts[1]
859+
switch trailerKey {
860+
case "Argocd-reference-commit-repourl":
861+
_, err := url.Parse(trailerValue)
862+
if err != nil {
863+
logCtx.Errorf("failed to parse repo URL %q: %v", truncate(trailerValue), err)
864+
return false
865+
}
866+
relatedCommit.RepoURL = trailerValue
867+
case "Argocd-reference-commit-author":
868+
address, err := mail.ParseAddress(trailerValue)
869+
if err != nil || address == nil {
870+
logCtx.Errorf("failed to parse author email %q: %v", truncate(trailerValue), err)
871+
return false
872+
}
873+
relatedCommit.Author = *address
874+
case "Argocd-reference-commit-date":
875+
// Validate that it's the correct date format.
876+
t, err := time.Parse(time.RFC3339, trailerValue)
877+
if err != nil {
878+
logCtx.Errorf("failed to parse date %q with RFC3339 format: %v", truncate(trailerValue), err)
879+
return false
880+
}
881+
relatedCommit.Date = t.Format(time.RFC3339)
882+
case "Argocd-reference-commit-subject":
883+
relatedCommit.Subject = trailerValue
884+
case "Argocd-reference-commit-body":
885+
body := ""
886+
err := json.Unmarshal([]byte(trailerValue), &body)
887+
if err != nil {
888+
logCtx.Errorf("failed to parse body %q as JSON: %v", truncate(trailerValue), err)
889+
return false
890+
}
891+
relatedCommit.Body = body
892+
case "Argocd-reference-commit-sha":
893+
if !shaRegex.MatchString(trailerValue) {
894+
logCtx.Errorf("invalid commit SHA %q in trailer %s: must be a lowercase hex string 5-40 characters long", truncate(trailerValue), trailerKey)
895+
return false
896+
}
897+
relatedCommit.SHA = trailerValue
898+
default:
899+
return false
900+
}
901+
return true
887902
}
888903

889904
// VerifyCommitSignature Runs verify-commit on a given revision and returns the output

util/git/client_test.go

Lines changed: 32 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1103,20 +1103,22 @@ func (m *mockCreds) GetUserInfo(_ context.Context) (string, string, error) {
11031103
return "", "", nil
11041104
}
11051105

1106-
func Test_getReferences(t *testing.T) {
1106+
func Test_GetReferences(t *testing.T) {
11071107
t.Parallel()
11081108

11091109
now := time.Now()
11101110

11111111
tests := []struct {
1112-
name string
1113-
input string
1114-
expected []RevisionReference
1112+
name string
1113+
input string
1114+
expectedReferences []RevisionReference
1115+
expectedMessage string
11151116
}{
11161117
{
1117-
name: "No trailers",
1118-
input: "This is a commit message without trailers.",
1119-
expected: nil,
1118+
name: "No trailers",
1119+
input: "This is a commit message without trailers.",
1120+
expectedReferences: nil,
1121+
expectedMessage: "This is a commit message without trailers.\n",
11201122
},
11211123
{
11221124
name: "Invalid trailers",
@@ -1126,25 +1128,36 @@ Argocd-reference-commit-sha: xyz123
11261128
Argocd-reference-commit-body: this isn't json
11271129
Argocd-reference-commit-author: % not email %
11281130
Argocd-reference-commit-bogus:`,
1129-
expected: nil,
1131+
expectedReferences: nil,
1132+
expectedMessage: `Argocd-reference-commit-repourl: % invalid %
1133+
Argocd-reference-commit-date: invalid-date
1134+
Argocd-reference-commit-sha: xyz123
1135+
Argocd-reference-commit-body: this isn't json
1136+
Argocd-reference-commit-author: % not email %
1137+
Argocd-reference-commit-bogus:
1138+
`,
11301139
},
11311140
{
1132-
name: "Unknown trailers",
1133-
input: "Argocd-reference-commit-unknown: foobar",
1134-
expected: nil,
1141+
name: "Unknown trailers",
1142+
input: "Argocd-reference-commit-unknown: foobar",
1143+
expectedReferences: nil,
1144+
expectedMessage: "Argocd-reference-commit-unknown: foobar\n",
11351145
},
11361146
{
11371147
name: "Some valid and Invalid trailers",
11381148
input: `Argocd-reference-commit-sha: abc123
11391149
Argocd-reference-commit-repourl: % invalid %
11401150
Argocd-reference-commit-date: invalid-date`,
1141-
expected: []RevisionReference{
1151+
expectedReferences: []RevisionReference{
11421152
{
11431153
Commit: &CommitMetadata{
11441154
SHA: "abc123",
11451155
},
11461156
},
11471157
},
1158+
expectedMessage: `Argocd-reference-commit-repourl: % invalid %
1159+
Argocd-reference-commit-date: invalid-date
1160+
`,
11481161
},
11491162
{
11501163
name: "Valid trailers",
@@ -1154,7 +1167,7 @@ Argocd-reference-commit-date: %s
11541167
Argocd-reference-commit-subject: Fix bug
11551168
Argocd-reference-commit-body: "Fix bug\n\nSome: trailer"
11561169
Argocd-reference-commit-sha: abc123`, now.Format(time.RFC3339)),
1157-
expected: []RevisionReference{
1170+
expectedReferences: []RevisionReference{
11581171
{
11591172
Commit: &CommitMetadata{
11601173
Author: mail.Address{
@@ -1169,18 +1182,20 @@ Argocd-reference-commit-sha: abc123`, now.Format(time.RFC3339)),
11691182
},
11701183
},
11711184
},
1185+
expectedMessage: "",
11721186
},
11731187
{
11741188
name: "Duplicate trailers",
11751189
input: `Argocd-reference-commit-repourl: https://github.com/org/repo.git
11761190
Argocd-reference-commit-repourl: https://github.com/another/repo.git`,
1177-
expected: []RevisionReference{
1191+
expectedReferences: []RevisionReference{
11781192
{
11791193
Commit: &CommitMetadata{
11801194
RepoURL: "https://github.com/another/repo.git",
11811195
},
11821196
},
11831197
},
1198+
expectedMessage: "",
11841199
},
11851200
}
11861201

@@ -1189,8 +1204,9 @@ Argocd-reference-commit-repourl: https://github.com/another/repo.git`,
11891204
t.Parallel()
11901205

11911206
logCtx := log.WithFields(log.Fields{})
1192-
result := getReferences(logCtx, tt.input)
1193-
assert.Equal(t, tt.expected, result)
1207+
result, message := GetReferences(logCtx, tt.input)
1208+
assert.Equal(t, tt.expectedReferences, result)
1209+
assert.Equal(t, tt.expectedMessage, message)
11941210
})
11951211
}
11961212
}

0 commit comments

Comments
 (0)