mirror of
https://github.com/argoproj/argo-cd
synced 2026-04-21 17:07:16 +00:00
fix(hydrator): omit Argocd- trailers from hydrator.metadata (#23463)
Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
This commit is contained in:
parent
6ec1aa1b84
commit
c60a727524
6 changed files with 124 additions and 77 deletions
|
|
@ -220,6 +220,7 @@ type hydratorMetadataFile struct {
|
|||
// Subject is the subject line of the DRY commit message, i.e. `git show --format=%s`.
|
||||
Subject string `json:"subject,omitempty"`
|
||||
// Body is the body of the DRY commit message, excluding the subject line, i.e. `git show --format=%b`.
|
||||
// Known Argocd- trailers with valid values are removed, but all other trailers are kept.
|
||||
Body string `json:"body,omitempty"`
|
||||
References []v1alpha1.RevisionReference `json:"references,omitempty"`
|
||||
}
|
||||
|
|
|
|||
|
|
@ -18,6 +18,7 @@ import (
|
|||
|
||||
"github.com/argoproj/argo-cd/v3/commitserver/apiclient"
|
||||
appv1 "github.com/argoproj/argo-cd/v3/pkg/apis/application/v1alpha1"
|
||||
"github.com/argoproj/argo-cd/v3/util/git"
|
||||
"github.com/argoproj/argo-cd/v3/util/io"
|
||||
)
|
||||
|
||||
|
|
@ -48,8 +49,10 @@ func WriteForPaths(root *os.Root, repoUrl, drySha string, dryCommitMetadata *app
|
|||
|
||||
subject, body, _ := strings.Cut(message, "\n\n")
|
||||
|
||||
_, bodyMinusTrailers := git.GetReferences(log.WithFields(log.Fields{"repo": repoUrl, "revision": drySha}), body)
|
||||
|
||||
// Write the top-level readme.
|
||||
err := writeMetadata(root, "", hydratorMetadataFile{DrySHA: drySha, RepoURL: repoUrl, Author: author, Subject: subject, Body: body, Date: date, References: references})
|
||||
err := writeMetadata(root, "", hydratorMetadataFile{DrySHA: drySha, RepoURL: repoUrl, Author: author, Subject: subject, Body: bodyMinusTrailers, Date: date, References: references})
|
||||
if err != nil {
|
||||
return fmt.Errorf("failed to write top-level hydrator metadata: %w", err)
|
||||
}
|
||||
|
|
|
|||
|
|
@ -9,7 +9,6 @@ import (
|
|||
"os"
|
||||
"path"
|
||||
"path/filepath"
|
||||
"strings"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
|
|
@ -73,9 +72,13 @@ func TestWriteForPaths(t *testing.T) {
|
|||
|
||||
now := metav1.NewTime(time.Now())
|
||||
metadata := &appsv1.RevisionMetadata{
|
||||
Author: "test-author",
|
||||
Date: &now,
|
||||
Message: "test-message",
|
||||
Author: "test-author",
|
||||
Date: &now,
|
||||
Message: `test-message
|
||||
|
||||
Signed-off-by: Test User <test@example.com>
|
||||
Argocd-reference-commit-sha: abc123
|
||||
`,
|
||||
References: []appsv1.RevisionReference{
|
||||
{
|
||||
Commit: &appsv1.CommitMetadata{
|
||||
|
|
@ -97,16 +100,15 @@ func TestWriteForPaths(t *testing.T) {
|
|||
topMetadataBytes, err := os.ReadFile(topMetadataPath)
|
||||
require.NoError(t, err)
|
||||
|
||||
expectedSubject, expectedBody, _ := strings.Cut(metadata.Message, "\n\n")
|
||||
|
||||
var topMetadata hydratorMetadataFile
|
||||
err = json.Unmarshal(topMetadataBytes, &topMetadata)
|
||||
require.NoError(t, err)
|
||||
assert.Equal(t, repoURL, topMetadata.RepoURL)
|
||||
assert.Equal(t, drySha, topMetadata.DrySHA)
|
||||
assert.Equal(t, metadata.Author, topMetadata.Author)
|
||||
assert.Equal(t, expectedSubject, topMetadata.Subject)
|
||||
assert.Equal(t, expectedBody, topMetadata.Body)
|
||||
assert.Equal(t, "test-message", topMetadata.Subject)
|
||||
// The body should exclude the Argocd- trailers.
|
||||
assert.Equal(t, "Signed-off-by: Test User <test@example.com>\n", topMetadata.Body)
|
||||
assert.Equal(t, metadata.Date.Format(time.RFC3339), topMetadata.Date)
|
||||
assert.Equal(t, metadata.References, topMetadata.References)
|
||||
|
||||
|
|
|
|||
|
|
@ -230,6 +230,12 @@ The commit metadata will appear in the hydrated commit's root hydrator.metadata
|
|||
|
||||
```json
|
||||
{
|
||||
"author": "CI <ci@example.com>",
|
||||
"subject": "chore: bump image to b82add2",
|
||||
"date": "2025-06-09T13:50:08-04:00",
|
||||
"body": "Signed-off-by: CI <ci@example.com>\n",
|
||||
"drySha": "6cb951525937865dced818bbdd78c89b2d2b3045",
|
||||
"repoURL": "https://git.example.com/owner/manifests-repo",
|
||||
"references": [
|
||||
{
|
||||
"commit": {
|
||||
|
|
@ -248,6 +254,10 @@ The commit metadata will appear in the hydrated commit's root hydrator.metadata
|
|||
}
|
||||
```
|
||||
|
||||
The top-level "body" field contains the commit message of the DRY commit minus the subject line and any
|
||||
`Argocd-reference-commit-*` trailers that were used in `references`. Unrecognized or invalid trailers are preserved in
|
||||
the body.
|
||||
|
||||
Although `references` is an array, the source hydrator currently only supports a single related commit. If a trailer is
|
||||
specified more than once, the last one will be used.
|
||||
|
||||
|
|
|
|||
|
|
@ -790,7 +790,7 @@ func (m *nativeGitClient) RevisionMetadata(revision string) (*RevisionMetadata,
|
|||
if err != nil {
|
||||
return nil, fmt.Errorf("failed to interpret trailers for revision %q in repo %q: %w", revision, m.repoURL, err)
|
||||
}
|
||||
relatedCommits := getReferences(log.WithFields(log.Fields{"repo": m.repoURL, "revision": revision}), out)
|
||||
relatedCommits, _ := GetReferences(log.WithFields(log.Fields{"repo": m.repoURL, "revision": revision}), out)
|
||||
|
||||
out, err = m.runCmd("tag", "--points-at", revision)
|
||||
if err != nil {
|
||||
|
|
@ -816,65 +816,23 @@ func truncate(str string) string {
|
|||
|
||||
var shaRegex = regexp.MustCompile(`^[0-9a-f]{5,40}$`)
|
||||
|
||||
// getReferences extracts related commit metadata from the commit message trailers. If referenced commit
|
||||
// GetReferences extracts related commit metadata from the commit message trailers. If referenced commit
|
||||
// metadata is present, we return a slice containing a single metadata object. If no related commit metadata is found,
|
||||
// we return a nil slice.
|
||||
//
|
||||
// If a trailer fails validation, we log an error and skip that trailer. We truncate the trailer values to 100
|
||||
// characters to avoid excessively long log messages.
|
||||
func getReferences(logCtx *log.Entry, commitMessageBody string) []RevisionReference {
|
||||
//
|
||||
// We also return the commit message body with all valid Argocd-reference-commit-* trailers removed.
|
||||
func GetReferences(logCtx *log.Entry, commitMessageBody string) ([]RevisionReference, string) {
|
||||
unrelatedLines := strings.Builder{}
|
||||
var relatedCommit CommitMetadata
|
||||
scanner := bufio.NewScanner(strings.NewReader(commitMessageBody))
|
||||
for scanner.Scan() {
|
||||
line := scanner.Text()
|
||||
if !strings.HasPrefix(line, "Argocd-reference-commit-") {
|
||||
continue
|
||||
}
|
||||
parts := strings.SplitN(line, ": ", 2)
|
||||
if len(parts) != 2 {
|
||||
continue
|
||||
}
|
||||
trailerKey := parts[0]
|
||||
trailerValue := parts[1]
|
||||
switch trailerKey {
|
||||
case "Argocd-reference-commit-repourl":
|
||||
_, err := url.Parse(trailerValue)
|
||||
if err != nil {
|
||||
logCtx.Errorf("failed to parse repo URL %q: %v", truncate(trailerValue), err)
|
||||
continue
|
||||
}
|
||||
relatedCommit.RepoURL = trailerValue
|
||||
case "Argocd-reference-commit-author":
|
||||
address, err := mail.ParseAddress(trailerValue)
|
||||
if err != nil || address == nil {
|
||||
logCtx.Errorf("failed to parse author email %q: %v", truncate(trailerValue), err)
|
||||
continue
|
||||
}
|
||||
relatedCommit.Author = *address
|
||||
case "Argocd-reference-commit-date":
|
||||
// Validate that it's the correct date format.
|
||||
t, err := time.Parse(time.RFC3339, trailerValue)
|
||||
if err != nil {
|
||||
logCtx.Errorf("failed to parse date %q with RFC3339 format: %v", truncate(trailerValue), err)
|
||||
continue
|
||||
}
|
||||
relatedCommit.Date = t.Format(time.RFC3339)
|
||||
case "Argocd-reference-commit-subject":
|
||||
relatedCommit.Subject = trailerValue
|
||||
case "Argocd-reference-commit-body":
|
||||
body := ""
|
||||
err := json.Unmarshal([]byte(trailerValue), &body)
|
||||
if err != nil {
|
||||
logCtx.Errorf("failed to parse body %q as JSON: %v", truncate(trailerValue), err)
|
||||
continue
|
||||
}
|
||||
relatedCommit.Body = body
|
||||
case "Argocd-reference-commit-sha":
|
||||
if !shaRegex.MatchString(trailerValue) {
|
||||
logCtx.Errorf("invalid commit SHA %q in trailer %s: must be a lowercase hex string 5-40 characters long", truncate(trailerValue), trailerKey)
|
||||
continue
|
||||
}
|
||||
relatedCommit.SHA = trailerValue
|
||||
updated := updateCommitMetadata(logCtx, &relatedCommit, line)
|
||||
if !updated {
|
||||
unrelatedLines.WriteString(line + "\n")
|
||||
}
|
||||
}
|
||||
var relatedCommits []RevisionReference
|
||||
|
|
@ -883,7 +841,64 @@ func getReferences(logCtx *log.Entry, commitMessageBody string) []RevisionRefere
|
|||
Commit: &relatedCommit,
|
||||
})
|
||||
}
|
||||
return relatedCommits
|
||||
return relatedCommits, unrelatedLines.String()
|
||||
}
|
||||
|
||||
// updateCommitMetadata checks if the line is a valid Argocd-reference-commit-* trailer. If so, it updates
|
||||
// the relatedCommit object and returns true. If the line is not a valid trailer, it returns false.
|
||||
func updateCommitMetadata(logCtx *log.Entry, relatedCommit *CommitMetadata, line string) bool {
|
||||
if !strings.HasPrefix(line, "Argocd-reference-commit-") {
|
||||
return false
|
||||
}
|
||||
parts := strings.SplitN(line, ": ", 2)
|
||||
if len(parts) != 2 {
|
||||
return false
|
||||
}
|
||||
trailerKey := parts[0]
|
||||
trailerValue := parts[1]
|
||||
switch trailerKey {
|
||||
case "Argocd-reference-commit-repourl":
|
||||
_, err := url.Parse(trailerValue)
|
||||
if err != nil {
|
||||
logCtx.Errorf("failed to parse repo URL %q: %v", truncate(trailerValue), err)
|
||||
return false
|
||||
}
|
||||
relatedCommit.RepoURL = trailerValue
|
||||
case "Argocd-reference-commit-author":
|
||||
address, err := mail.ParseAddress(trailerValue)
|
||||
if err != nil || address == nil {
|
||||
logCtx.Errorf("failed to parse author email %q: %v", truncate(trailerValue), err)
|
||||
return false
|
||||
}
|
||||
relatedCommit.Author = *address
|
||||
case "Argocd-reference-commit-date":
|
||||
// Validate that it's the correct date format.
|
||||
t, err := time.Parse(time.RFC3339, trailerValue)
|
||||
if err != nil {
|
||||
logCtx.Errorf("failed to parse date %q with RFC3339 format: %v", truncate(trailerValue), err)
|
||||
return false
|
||||
}
|
||||
relatedCommit.Date = t.Format(time.RFC3339)
|
||||
case "Argocd-reference-commit-subject":
|
||||
relatedCommit.Subject = trailerValue
|
||||
case "Argocd-reference-commit-body":
|
||||
body := ""
|
||||
err := json.Unmarshal([]byte(trailerValue), &body)
|
||||
if err != nil {
|
||||
logCtx.Errorf("failed to parse body %q as JSON: %v", truncate(trailerValue), err)
|
||||
return false
|
||||
}
|
||||
relatedCommit.Body = body
|
||||
case "Argocd-reference-commit-sha":
|
||||
if !shaRegex.MatchString(trailerValue) {
|
||||
logCtx.Errorf("invalid commit SHA %q in trailer %s: must be a lowercase hex string 5-40 characters long", truncate(trailerValue), trailerKey)
|
||||
return false
|
||||
}
|
||||
relatedCommit.SHA = trailerValue
|
||||
default:
|
||||
return false
|
||||
}
|
||||
return true
|
||||
}
|
||||
|
||||
// VerifyCommitSignature Runs verify-commit on a given revision and returns the output
|
||||
|
|
|
|||
|
|
@ -1103,20 +1103,22 @@ func (m *mockCreds) GetUserInfo(_ context.Context) (string, string, error) {
|
|||
return "", "", nil
|
||||
}
|
||||
|
||||
func Test_getReferences(t *testing.T) {
|
||||
func Test_GetReferences(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
now := time.Now()
|
||||
|
||||
tests := []struct {
|
||||
name string
|
||||
input string
|
||||
expected []RevisionReference
|
||||
name string
|
||||
input string
|
||||
expectedReferences []RevisionReference
|
||||
expectedMessage string
|
||||
}{
|
||||
{
|
||||
name: "No trailers",
|
||||
input: "This is a commit message without trailers.",
|
||||
expected: nil,
|
||||
name: "No trailers",
|
||||
input: "This is a commit message without trailers.",
|
||||
expectedReferences: nil,
|
||||
expectedMessage: "This is a commit message without trailers.\n",
|
||||
},
|
||||
{
|
||||
name: "Invalid trailers",
|
||||
|
|
@ -1126,25 +1128,36 @@ Argocd-reference-commit-sha: xyz123
|
|||
Argocd-reference-commit-body: this isn't json
|
||||
Argocd-reference-commit-author: % not email %
|
||||
Argocd-reference-commit-bogus:`,
|
||||
expected: nil,
|
||||
expectedReferences: nil,
|
||||
expectedMessage: `Argocd-reference-commit-repourl: % invalid %
|
||||
Argocd-reference-commit-date: invalid-date
|
||||
Argocd-reference-commit-sha: xyz123
|
||||
Argocd-reference-commit-body: this isn't json
|
||||
Argocd-reference-commit-author: % not email %
|
||||
Argocd-reference-commit-bogus:
|
||||
`,
|
||||
},
|
||||
{
|
||||
name: "Unknown trailers",
|
||||
input: "Argocd-reference-commit-unknown: foobar",
|
||||
expected: nil,
|
||||
name: "Unknown trailers",
|
||||
input: "Argocd-reference-commit-unknown: foobar",
|
||||
expectedReferences: nil,
|
||||
expectedMessage: "Argocd-reference-commit-unknown: foobar\n",
|
||||
},
|
||||
{
|
||||
name: "Some valid and Invalid trailers",
|
||||
input: `Argocd-reference-commit-sha: abc123
|
||||
Argocd-reference-commit-repourl: % invalid %
|
||||
Argocd-reference-commit-date: invalid-date`,
|
||||
expected: []RevisionReference{
|
||||
expectedReferences: []RevisionReference{
|
||||
{
|
||||
Commit: &CommitMetadata{
|
||||
SHA: "abc123",
|
||||
},
|
||||
},
|
||||
},
|
||||
expectedMessage: `Argocd-reference-commit-repourl: % invalid %
|
||||
Argocd-reference-commit-date: invalid-date
|
||||
`,
|
||||
},
|
||||
{
|
||||
name: "Valid trailers",
|
||||
|
|
@ -1154,7 +1167,7 @@ Argocd-reference-commit-date: %s
|
|||
Argocd-reference-commit-subject: Fix bug
|
||||
Argocd-reference-commit-body: "Fix bug\n\nSome: trailer"
|
||||
Argocd-reference-commit-sha: abc123`, now.Format(time.RFC3339)),
|
||||
expected: []RevisionReference{
|
||||
expectedReferences: []RevisionReference{
|
||||
{
|
||||
Commit: &CommitMetadata{
|
||||
Author: mail.Address{
|
||||
|
|
@ -1169,18 +1182,20 @@ Argocd-reference-commit-sha: abc123`, now.Format(time.RFC3339)),
|
|||
},
|
||||
},
|
||||
},
|
||||
expectedMessage: "",
|
||||
},
|
||||
{
|
||||
name: "Duplicate trailers",
|
||||
input: `Argocd-reference-commit-repourl: https://github.com/org/repo.git
|
||||
Argocd-reference-commit-repourl: https://github.com/another/repo.git`,
|
||||
expected: []RevisionReference{
|
||||
expectedReferences: []RevisionReference{
|
||||
{
|
||||
Commit: &CommitMetadata{
|
||||
RepoURL: "https://github.com/another/repo.git",
|
||||
},
|
||||
},
|
||||
},
|
||||
expectedMessage: "",
|
||||
},
|
||||
}
|
||||
|
||||
|
|
@ -1189,8 +1204,9 @@ Argocd-reference-commit-repourl: https://github.com/another/repo.git`,
|
|||
t.Parallel()
|
||||
|
||||
logCtx := log.WithFields(log.Fields{})
|
||||
result := getReferences(logCtx, tt.input)
|
||||
assert.Equal(t, tt.expected, result)
|
||||
result, message := GetReferences(logCtx, tt.input)
|
||||
assert.Equal(t, tt.expectedReferences, result)
|
||||
assert.Equal(t, tt.expectedMessage, message)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in a new issue