-
Notifications
You must be signed in to change notification settings - Fork 9
feat(jira): add --jira-trailer flag to extract Jira issue key from git trailer #1109
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -29,6 +29,7 @@ type attestJiraOptions struct { | |
| projectKeys []string | ||
| issueFields string | ||
| secondarySource string | ||
| trailerKey string | ||
| ignoreBranchMatch bool | ||
| assert bool | ||
| payload JiraAttestationPayload | ||
|
|
@@ -260,6 +261,7 @@ func newAttestJiraCmd(out io.Writer) *cobra.Command { | |
| cmd.Flags().StringSliceVar(&o.projectKeys, "jira-project-key", []string{}, jiraProjectKeyFlag) | ||
| cmd.Flags().StringVar(&o.issueFields, "jira-issue-fields", "", jiraIssueFieldFlag) | ||
| cmd.Flags().StringVar(&o.secondarySource, "jira-secondary-source", "", jiraSecondarySourceFlag) | ||
| cmd.Flags().StringVar(&o.trailerKey, "jira-trailer", "", jiraTrailerFlag) | ||
| cmd.Flags().BoolVar(&o.ignoreBranchMatch, "ignore-branch-match", false, ignoreBranchMatchFlag) | ||
| cmd.Flags().BoolVar(&o.assert, "assert", false, attestationAssertFlag) | ||
|
|
||
|
|
@@ -301,19 +303,27 @@ func (o *attestJiraOptions) run(args []string) error { | |
| return err | ||
| } | ||
|
|
||
| // Search commit message, branch name, and secondary source for Jira issue keys, | ||
| // filtering out false positives from multi-segment identifiers like CVE-2026-41284. | ||
| searchTexts := []string{commitInfo.Message} | ||
| if !o.ignoreBranchMatch { | ||
| searchTexts = append(searchTexts, commitInfo.Branch) | ||
| } | ||
| if o.secondarySource != "" { | ||
| searchTexts = append(searchTexts, o.secondarySource) | ||
| // Find Jira issue keys either from a named git trailer or by scanning the | ||
| // commit message, branch name, and secondary source. | ||
| var issueIDs []string | ||
| if o.trailerKey != "" { | ||
| trailerValues := gitview.GetTrailerValues(commitInfo.Message, o.trailerKey) | ||
| combinedTrailerText := strings.Join(trailerValues, "\n") | ||
| issueIDs = jira.FindJiraIssueKeys(combinedTrailerText, o.projectKeys) | ||
| logger.Debug("Checked for Jira issue references in trailer '%s' of Git commit %s: %v", o.trailerKey, commitInfo.Sha1, trailerValues) | ||
| } else { | ||
| searchTexts := []string{commitInfo.Message} | ||
| if !o.ignoreBranchMatch { | ||
| searchTexts = append(searchTexts, commitInfo.Branch) | ||
| } | ||
| if o.secondarySource != "" { | ||
| searchTexts = append(searchTexts, o.secondarySource) | ||
| } | ||
| combinedText := strings.Join(searchTexts, "\n") | ||
| issueIDs = jira.FindJiraIssueKeys(combinedText, o.projectKeys) | ||
| logger.Debug("Checked for Jira issue references in Git commit %s on branch %s commit message:\n%s", commitInfo.Sha1, commitInfo.Branch, commitInfo.Message) | ||
| } | ||
| combinedText := strings.Join(searchTexts, "\n") | ||
| issueIDs := jira.FindJiraIssueKeys(combinedText, o.projectKeys) | ||
| logger.Debug("Checked for Jira issue references in Git commit %s on branch %s commit message:\n%s", commitInfo.Sha1, commitInfo.Branch, commitInfo.Message) | ||
| logger.Debug("the following Jira references are found in commit message or branch name: %v", issueIDs) | ||
| logger.Debug("the following Jira references are found: %v", issueIDs) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The Both assert paths below hardcode the old wording:
With Test 29 pins the wrong wording as golden ( Threading the source through both messages keeps them accurate in either mode: searchedIn := "commit message or branch name"
if o.trailerKey != "" {
searchedIn = fmt.Sprintf("the '%s' trailer of the commit message", o.trailerKey)
}then |
||
|
|
||
| issueLog := "" | ||
| issueFoundCount := 0 | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -331,6 +331,41 @@ func (suite *AttestJiraCommandTestSuite) TestAttestJiraCmd() { | |||||||||||||||||||||||||||||||||||||||||||||||
| cmd: fmt.Sprintf("attest jira --name .foo --commit HEAD --jira-base-url https://kosli-test.atlassian.net %s", suite.defaultKosliArguments), | ||||||||||||||||||||||||||||||||||||||||||||||||
| golden: "Error: failed to parse attestation name: invalid attestation name format: .foo\n", | ||||||||||||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||||||||||
| name: "27 can attest jira using --jira-trailer to extract issue key from commit trailer", | ||||||||||||||||||||||||||||||||||||||||||||||||
| cmd: fmt.Sprintf(`attest jira --name bar | ||||||||||||||||||||||||||||||||||||||||||||||||
| --jira-base-url https://kosli-test.atlassian.net | ||||||||||||||||||||||||||||||||||||||||||||||||
| --jira-trailer Jira | ||||||||||||||||||||||||||||||||||||||||||||||||
| --repo-root %s %s`, suite.tmpDir, suite.defaultKosliArguments), | ||||||||||||||||||||||||||||||||||||||||||||||||
| golden: "jira attestation 'bar' is reported to trail: test-123\n", | ||||||||||||||||||||||||||||||||||||||||||||||||
| additionalConfig: jiraTestsAdditionalConfig{ | ||||||||||||||||||||||||||||||||||||||||||||||||
| commitMessage: "fix: some change\n\nJira: EX-1\nOna-Environment-Id: ONA-999", | ||||||||||||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+334
to
+344
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Test 27 does not actually exercise the new behaviour. Its Adding
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||||||||||
| name: "28 --jira-trailer with no matching trailer produces no issue IDs (non-compliant but reported)", | ||||||||||||||||||||||||||||||||||||||||||||||||
| cmd: fmt.Sprintf(`attest jira --name bar | ||||||||||||||||||||||||||||||||||||||||||||||||
| --jira-base-url https://kosli-test.atlassian.net | ||||||||||||||||||||||||||||||||||||||||||||||||
| --jira-trailer Jira | ||||||||||||||||||||||||||||||||||||||||||||||||
| --repo-root %s %s`, suite.tmpDir, suite.defaultKosliArguments), | ||||||||||||||||||||||||||||||||||||||||||||||||
| golden: "jira attestation 'bar' is reported to trail: test-123\n", | ||||||||||||||||||||||||||||||||||||||||||||||||
| additionalConfig: jiraTestsAdditionalConfig{ | ||||||||||||||||||||||||||||||||||||||||||||||||
| commitMessage: "fix: some change with no jira trailer", | ||||||||||||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||||||||||
| wantError: true, | ||||||||||||||||||||||||||||||||||||||||||||||||
| name: "29 --jira-trailer with --assert fails when trailer is absent", | ||||||||||||||||||||||||||||||||||||||||||||||||
| cmd: fmt.Sprintf(`attest jira --name bar | ||||||||||||||||||||||||||||||||||||||||||||||||
| --jira-base-url https://kosli-test.atlassian.net | ||||||||||||||||||||||||||||||||||||||||||||||||
| --jira-trailer Jira | ||||||||||||||||||||||||||||||||||||||||||||||||
| --assert | ||||||||||||||||||||||||||||||||||||||||||||||||
| --repo-root %s %s`, suite.tmpDir, suite.defaultKosliArguments), | ||||||||||||||||||||||||||||||||||||||||||||||||
| golden: "jira attestation 'bar' is reported to trail: test-123\nError: no Jira references are found in commit message or branch name\n", | ||||||||||||||||||||||||||||||||||||||||||||||||
| additionalConfig: jiraTestsAdditionalConfig{ | ||||||||||||||||||||||||||||||||||||||||||||||||
| commitMessage: "fix: some change with no jira trailer", | ||||||||||||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| for _, test := range tests { | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -169,6 +169,7 @@ The ^.kosli_ignore^ will be treated as part of the artifact like any other file, | |
| jiraIssueFieldFlag = "[optional] The comma separated list of fields to include from the Jira issue. Default no fields are included. '*all' will give all fields." | ||
| jiraSecondarySourceFlag = "[optional] An optional string to search for Jira ticket reference, e.g. '--jira-secondary-source ${{ github.head_ref }}'" | ||
| ignoreBranchMatchFlag = "Ignore branch name when searching for Jira ticket reference." | ||
| jiraTrailerFlag = "[optional] The git trailer key to use as the sole source of Jira issue references (e.g. '--jira-trailer Jira' extracts the value of 'Jira: <issue-key>' lines from the commit message). When set, the commit message body and branch name are not scanned." | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Flag help is clear and accurate. The gap is the command's
Per the slice checklist in |
||
| envDescriptionFlag = "[optional] The environment description." | ||
| flowDescriptionFlag = "[optional] The Kosli flow description." | ||
| trailDescriptionFlag = "[optional] The Kosli trail description." | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -316,6 +316,23 @@ func (gv *GitView) MatchPatternInCommitMessageORBranchName(pattern, commitSHA, s | |||||||||||||||
| return matches, commitInfo, nil | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| // GetTrailerValues extracts the values of all trailer lines in a commit message | ||||||||||||||||
| // that match the given key. The key comparison is case-insensitive. Trailer lines | ||||||||||||||||
| // have the format "<key>: <value>". Returns an empty (non-nil) slice if none are found. | ||||||||||||||||
|
Comment on lines
+319
to
+321
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Naming/semantics nit worth resolving before this ships, since the name sets an expectation the implementation doesn't meet: real git trailers (per For the current use case that leniency is harmless (the value goes through the Jira key regex anyway), but the doc comment should say so explicitly rather than calling them "trailer lines", e.g. "matches any line of the form |
||||||||||||||||
| func GetTrailerValues(message, key string) []string { | ||||||||||||||||
| result := []string{} | ||||||||||||||||
| prefix := strings.ToLower(key) + ":" | ||||||||||||||||
| for _, line := range strings.Split(message, "\n") { | ||||||||||||||||
| if strings.HasPrefix(strings.ToLower(line), prefix) { | ||||||||||||||||
| value := strings.TrimSpace(line[len(prefix):]) | ||||||||||||||||
|
Comment on lines
+325
to
+327
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Two edge cases in the prefix match:
Trimming both sides handles (1) cheaply and keeps everything else identical (
Suggested change
For (2), consider |
||||||||||||||||
| if value != "" { | ||||||||||||||||
| result = append(result, value) | ||||||||||||||||
| } | ||||||||||||||||
| } | ||||||||||||||||
| } | ||||||||||||||||
| return result | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| // ResolveRevision returns an explicit commit SHA1 from commit SHA or ref (e.g. HEAD~2) | ||||||||||||||||
| func (gv *GitView) ResolveRevision(commitSHAOrRef string) (string, error) { | ||||||||||||||||
| hash, err := gv.repository.ResolveRevision(plumbing.Revision(commitSHAOrRef)) | ||||||||||||||||
|
|
||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
--jira-trailersilently disables--jira-secondary-sourceand--ignore-branch-match. Both are accepted without complaint and then have no effect, which is easy to get wrong in a CI pipeline (--jira-secondary-source ${{ github.head_ref }}quietly becoming a no-op is a compliance-relevant silent change). This file already uses the repo's helper for exactly this shape of problem (lines 220–233), so:For
--ignore-branch-matchalogger.Warninrun()would be enough, since it's already implied by the trailer mode.Separately: trailer values are still fed through
jira.FindJiraIssueKeys, soJira: EX1orJira: 1234yields nothing at all — no warning, just a non-compliant attestation. Worth alogger.Warnwhenlen(trailerValues) > 0 && len(issueIDs) == 0, since in trailer mode the user has explicitly declared where the key lives and a mismatch is almost certainly a mistake rather than an absent reference.