From 043dffcf0ea7a043e769daea76d1a2b39cee944f Mon Sep 17 00:00:00 2001 From: Tibor Vukovic Date: Tue, 23 Jun 2026 20:31:01 +0000 Subject: [PATCH] trigger: expose triggering comment ID and URL in JOB_SPEC Parsing arguments out of a command (e.g. "/test ") is currently unreliable for jobs: a job only receives the PR refs, not the comment that triggered it. Reconstructing the comment from the PR number alone is racy - concurrent or edited /test comments resolve to the wrong body, and UI reruns have no comment at all. This captures the triggering comment in ProwJobSpec via a new optional trigger_comment object and surfaces it to comment-triggered presubmits as two env vars: TRIGGER_COMMENT_ID - numeric GitHub comment ID TRIGGER_COMMENT_URL - full HTML URL to the comment Both are also available in the JSON-encoded JOB_SPEC under trigger_comment.id / trigger_comment.html_url. The comment ID is a stable, unique reference to the exact comment that triggered the run, so a job can fetch its body via the GitHub API directly. The URL also enables a clickable link in reports or distinguishing a manual rerun from an automatic one. The field uses omitempty and is nil for jobs triggered automatically (PR open/update, Tide, periodics), so this is fully backward compatible. --- .../prowjob_customresourcedefinition.yaml | 22 ++++ pkg/apis/prowjobs/v1/types.go | 23 +++- pkg/apis/prowjobs/v1/zz_generated.deepcopy.go | 21 ++++ pkg/plugins/trigger/generic-comment.go | 7 +- pkg/plugins/trigger/trigger.go | 13 ++- pkg/plugins/trigger/trigger_test.go | 9 +- pkg/pod-utils/downwardapi/jobspec.go | 21 +++- pkg/pod-utils/downwardapi/jobspec_test.go | 100 +++++++++++++++++- site/content/en/docs/jobs.md | 10 ++ .../config/prow/cluster/50_crd.yaml | 10 ++ 10 files changed, 221 insertions(+), 15 deletions(-) diff --git a/config/prow/cluster/prowjob-crd/prowjob_customresourcedefinition.yaml b/config/prow/cluster/prowjob-crd/prowjob_customresourcedefinition.yaml index ed0d1e7d3b..83772805ae 100644 --- a/config/prow/cluster/prowjob-crd/prowjob_customresourcedefinition.yaml +++ b/config/prow/cluster/prowjob-crd/prowjob_customresourcedefinition.yaml @@ -9968,6 +9968,28 @@ spec: type: object x-kubernetes-preserve-unknown-fields: true type: object + trigger_comment: + description: |- + TriggerComment, when set, references the GitHub comment whose command + triggered this job (e.g. /test, /retest, /ok-to-test). It is nil for jobs + that were triggered automatically (e.g. when a PR is opened or updated, by + Tide, or for periodics). + properties: + html_url: + description: |- + HTMLURL is the HTML URL of the comment. Unlike the bare ID it is always + set for comment triggers and unambiguously identifies the comment + regardless of its kind (issue comment, review, or review comment). + type: string + id: + description: |- + ID is the numeric ID of the GitHub comment. It is set for issue/PR + comments and review comments, but may be 0 for triggers that have no + dedicated comment ID (e.g. a command in a review body or a PR + description). Together with Refs it can be used to construct the GitHub + API or HTML URL for the comment. + type: integer + type: object type: description: |- Type is the type of job and informs how diff --git a/pkg/apis/prowjobs/v1/types.go b/pkg/apis/prowjobs/v1/types.go index d7bed01405..3a5e865f58 100644 --- a/pkg/apis/prowjobs/v1/types.go +++ b/pkg/apis/prowjobs/v1/types.go @@ -78,7 +78,8 @@ func GetAllProwJobStates() []ProwJobState { SuccessState, FailureState, AbortedState, - ErrorState} + ErrorState, + } } // ProwJobAgent specifies the controller (such as plank or jenkins-agent) that runs the job. @@ -234,6 +235,26 @@ type ProwJobSpec struct { // This behaviour may be superseded by MaxConcurrency field, if it // is set to a constraining value. JobQueueName string `json:"job_queue_name,omitempty"` + + // TriggerComment, when set, references the GitHub comment whose command + // triggered this job (e.g. /test, /retest, /ok-to-test). It is nil for jobs + // that were triggered automatically (e.g. when a PR is opened or updated, by + // Tide, or for periodics). + TriggerComment *TriggerComment `json:"trigger_comment,omitempty"` +} + +// TriggerComment references the GitHub comment whose command triggered a job. +type TriggerComment struct { + // ID is the numeric ID of the GitHub comment. It is set for issue/PR + // comments and review comments, but may be 0 for triggers that have no + // dedicated comment ID (e.g. a command in a review body or a PR + // description). Together with Refs it can be used to construct the GitHub + // API or HTML URL for the comment. + ID int `json:"id,omitempty"` + // HTMLURL is the HTML URL of the comment. Unlike the bare ID it is always + // set for comment triggers and unambiguously identifies the comment + // regardless of its kind (issue comment, review, or review comment). + HTMLURL string `json:"html_url,omitempty"` } func (pjs ProwJobSpec) HasPipelineRunSpec() bool { diff --git a/pkg/apis/prowjobs/v1/zz_generated.deepcopy.go b/pkg/apis/prowjobs/v1/zz_generated.deepcopy.go index 800f272dd6..3f368c7691 100644 --- a/pkg/apis/prowjobs/v1/zz_generated.deepcopy.go +++ b/pkg/apis/prowjobs/v1/zz_generated.deepcopy.go @@ -477,6 +477,11 @@ func (in *ProwJobSpec) DeepCopyInto(out *ProwJobSpec) { *out = new(ProwJobDefault) (*in).DeepCopyInto(*out) } + if in.TriggerComment != nil { + in, out := &in.TriggerComment, &out.TriggerComment + *out = new(TriggerComment) + **out = **in + } return } @@ -774,6 +779,22 @@ func (in *TektonPipelineRunSpec) DeepCopy() *TektonPipelineRunSpec { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *TriggerComment) DeepCopyInto(out *TriggerComment) { + *out = *in + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new TriggerComment. +func (in *TriggerComment) DeepCopy() *TriggerComment { + if in == nil { + return nil + } + out := new(TriggerComment) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *UtilityImages) DeepCopyInto(out *UtilityImages) { *out = *in diff --git a/pkg/plugins/trigger/generic-comment.go b/pkg/plugins/trigger/generic-comment.go index f677da1d76..92cc197f26 100644 --- a/pkg/plugins/trigger/generic-comment.go +++ b/pkg/plugins/trigger/generic-comment.go @@ -23,6 +23,7 @@ import ( "sigs.k8s.io/prow/pkg/kube" "k8s.io/apimachinery/pkg/util/sets" + prowapi "sigs.k8s.io/prow/pkg/apis/prowjobs/v1" "sigs.k8s.io/prow/pkg/config" "sigs.k8s.io/prow/pkg/github" "sigs.k8s.io/prow/pkg/labels" @@ -203,7 +204,11 @@ func handleGenericComment(c Client, cp commentPruner, trigger plugins.Trigger, g }) } - return RunRequestedWithLabels(c, pr, baseSHA, toTest, gc.GUID, additionalLabels) + triggerComment := &prowapi.TriggerComment{HTMLURL: gc.HTMLURL} + if gc.CommentID != nil { + triggerComment.ID = *gc.CommentID + } + return RunRequestedWithLabels(c, pr, baseSHA, toTest, gc.GUID, additionalLabels, triggerComment) } func HonorOkToTest(trigger plugins.Trigger) bool { diff --git a/pkg/plugins/trigger/trigger.go b/pkg/plugins/trigger/trigger.go index 9a5b66fcf9..321985e3ee 100644 --- a/pkg/plugins/trigger/trigger.go +++ b/pkg/plugins/trigger/trigger.go @@ -326,15 +326,17 @@ func validateContextOverlap(toRun, toSkip []config.Presubmit) error { // RunRequested executes the config.Presubmits that are requested func RunRequested(c Client, pr *github.PullRequest, baseSHA string, requestedJobs []config.Presubmit, eventGUID string) error { - return runRequested(c, pr, baseSHA, requestedJobs, eventGUID, nil) + return runRequested(c, pr, baseSHA, requestedJobs, eventGUID, nil, nil) } -// RunRequestedWithLabels executes the config.Presubmits that are requested with the additional labels -func RunRequestedWithLabels(c Client, pr *github.PullRequest, baseSHA string, requestedJobs []config.Presubmit, eventGUID string, labels map[string]string) error { - return runRequested(c, pr, baseSHA, requestedJobs, eventGUID, labels) +// RunRequestedWithLabels executes the config.Presubmits that are requested with the additional labels. +// triggerComment, when non-nil, references the GitHub comment whose command triggered the jobs; pass +// nil when the jobs were not triggered by a comment. +func RunRequestedWithLabels(c Client, pr *github.PullRequest, baseSHA string, requestedJobs []config.Presubmit, eventGUID string, labels map[string]string, triggerComment *prowapi.TriggerComment) error { + return runRequested(c, pr, baseSHA, requestedJobs, eventGUID, labels, triggerComment) } -func runRequested(c Client, pr *github.PullRequest, baseSHA string, requestedJobs []config.Presubmit, eventGUID string, labels map[string]string, millisecondOverride ...time.Duration) error { +func runRequested(c Client, pr *github.PullRequest, baseSHA string, requestedJobs []config.Presubmit, eventGUID string, labels map[string]string, triggerComment *prowapi.TriggerComment, millisecondOverride ...time.Duration) error { var errors []error // If the PR is not mergeable (e.g. due to merge conflicts),we will not trigger any jobs, @@ -346,6 +348,7 @@ func runRequested(c Client, pr *github.PullRequest, baseSHA string, requestedJob for _, job := range requestedJobs { c.Logger.Infof("Starting %s build.", job.Name) pj := pjutil.NewPresubmit(*pr, baseSHA, job, eventGUID, labels, pjutil.RequireScheduling(c.Config.Scheduler.Enabled)) + pj.Spec.TriggerComment = triggerComment c.Logger.WithFields(pjutil.ProwJobFields(&pj)).Info("Creating a new prowjob.") if err := createWithRetry(context.TODO(), c.ProwJobClient, &pj, millisecondOverride...); err != nil { c.Logger.WithError(err).Error("Failed to create prowjob.") diff --git a/pkg/plugins/trigger/trigger_test.go b/pkg/plugins/trigger/trigger_test.go index e8c97f960d..392d053943 100644 --- a/pkg/plugins/trigger/trigger_test.go +++ b/pkg/plugins/trigger/trigger_test.go @@ -82,7 +82,7 @@ func TestHelpProvider(t *testing.T) { } func TestRunRequested(t *testing.T) { - var testCases = []struct { + testCases := []struct { name string pr *github.PullRequest @@ -242,7 +242,7 @@ func TestRunRequested(t *testing.T) { Logger: logrus.WithField("testcase", testCase.name), } - err := runRequested(client, testCase.pr, fakegithub.TestRef, testCase.requestedJobs, "event-guid", nil, time.Nanosecond) + err := runRequested(client, testCase.pr, fakegithub.TestRef, testCase.requestedJobs, "event-guid", nil, nil, time.Nanosecond) if err == nil && testCase.expectedErr { t.Error("failed to receive an error") } @@ -271,7 +271,7 @@ func TestRunRequested(t *testing.T) { } func TestValidateContextOverlap(t *testing.T) { - var testCases = []struct { + testCases := []struct { name string toRun, toSkip []config.Presubmit expectedErr bool @@ -323,7 +323,7 @@ func TestValidateContextOverlap(t *testing.T) { } func TestTrustedUser(t *testing.T) { - var testcases = []struct { + testcases := []struct { name string onlyOrgMembers bool @@ -633,7 +633,6 @@ func TestCreateWithRetry(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - fakeProwJobClient := fake.NewSimpleClientset() fakeProwJobClient.PrependReactor("*", "*", func(action clienttesting.Action) (handled bool, ret runtime.Object, err error) { if _, ok := action.(clienttesting.CreateActionImpl); ok && tc.numFailedCreate > 0 { diff --git a/pkg/pod-utils/downwardapi/jobspec.go b/pkg/pod-utils/downwardapi/jobspec.go index bde3b2687d..cc32bddf6d 100644 --- a/pkg/pod-utils/downwardapi/jobspec.go +++ b/pkg/pod-utils/downwardapi/jobspec.go @@ -48,6 +48,11 @@ type JobSpec struct { DecorationConfig *prowapi.DecorationConfig `json:"decoration_config,omitempty"` + // TriggerComment, when set, references the GitHub comment whose command + // triggered this job. It is only set for comment-triggered presubmits and + // is nil otherwise. + TriggerComment *prowapi.TriggerComment `json:"trigger_comment,omitempty"` + // we need to keep track of the agent until we // migrate everyone away from using the $BUILD_NUMBER // environment variable @@ -64,6 +69,7 @@ func NewJobSpec(spec prowapi.ProwJobSpec, buildID, prowJobID string) JobSpec { Refs: spec.Refs, ExtraRefs: spec.ExtraRefs, DecorationConfig: spec.DecorationConfig, + TriggerComment: spec.TriggerComment, agent: spec.Agent, } } @@ -109,6 +115,9 @@ const ( PullPullShaEnv = "PULL_PULL_SHA" PullHeadRefEnv = "PULL_HEAD_REF" PullTitleEnv = "PULL_TITLE" + + TriggerCommentIDEnv = "TRIGGER_COMMENT_ID" + TriggerCommentURLEnv = "TRIGGER_COMMENT_URL" ) // EnvForSpec returns a mapping of environment variables @@ -180,6 +189,16 @@ func EnvForSpec(spec JobSpec) (map[string]string, error) { env[PullHeadRefEnv] = spec.Refs.Pulls[0].HeadRef env[PullTitleEnv] = spec.Refs.Pulls[0].Title + // Only set for jobs triggered by a comment command (e.g. /test, /retest). + if spec.TriggerComment != nil { + if spec.TriggerComment.ID != 0 { + env[TriggerCommentIDEnv] = strconv.Itoa(spec.TriggerComment.ID) + } + if spec.TriggerComment.HTMLURL != "" { + env[TriggerCommentURLEnv] = spec.TriggerComment.HTMLURL + } + } + return env, nil } @@ -187,7 +206,7 @@ func EnvForSpec(spec JobSpec) (map[string]string, error) { func EnvForType(jobType prowapi.ProwJobType) []string { baseEnv := []string{CI, JobNameEnv, JobSpecEnv, JobTypeEnv, ProwJobIDEnv, BuildIDEnv, ProwBuildIDEnv} refsEnv := []string{RepoOwnerEnv, RepoNameEnv, SrcBaseEnv, SrcHostEnv, PullBaseRefEnv, PullBaseShaEnv, PullRefsEnv} - pullEnv := []string{PullNumberEnv, PullPullShaEnv, PullHeadRefEnv, PullTitleEnv} + pullEnv := []string{PullNumberEnv, PullPullShaEnv, PullHeadRefEnv, PullTitleEnv, TriggerCommentIDEnv, TriggerCommentURLEnv} switch jobType { case prowapi.PeriodicJob: diff --git a/pkg/pod-utils/downwardapi/jobspec_test.go b/pkg/pod-utils/downwardapi/jobspec_test.go index 392e71f031..7b00e0d035 100644 --- a/pkg/pod-utils/downwardapi/jobspec_test.go +++ b/pkg/pod-utils/downwardapi/jobspec_test.go @@ -25,7 +25,7 @@ import ( ) func TestEnvironmentForSpec(t *testing.T) { - var tests = []struct { + tests := []struct { name string spec JobSpec expected map[string]string @@ -159,6 +159,102 @@ func TestEnvironmentForSpec(t *testing.T) { "PULL_TITLE": "pull-title", }, }, + { + name: "presubmit job triggered by a comment", + spec: JobSpec{ + Type: prowapi.PresubmitJob, + Job: "job-name", + BuildID: "0", + ProwJobID: "prowjob", + Refs: &prowapi.Refs{ + Org: "org-name", + Repo: "repo-name", + BaseRef: "base-ref", + BaseSHA: "base-sha", + Pulls: []prowapi.Pull{{ + Number: 1, + Author: "author-name", + SHA: "pull-sha", + HeadRef: "branch-name", + Title: "pull-title", + }}, + }, + TriggerComment: &prowapi.TriggerComment{ + ID: 987654321, + HTMLURL: "https://github.com/org-name/repo-name/pull/1#issuecomment-987654321", + }, + }, + expected: map[string]string{ + "CI": "true", + "JOB_NAME": "job-name", + "BUILD_ID": "0", + "PROW_JOB_ID": "prowjob", + "JOB_TYPE": "presubmit", + "JOB_SPEC": `{"type":"presubmit","job":"job-name","buildid":"0","prowjobid":"prowjob","refs":{"org":"org-name","repo":"repo-name","base_ref":"base-ref","base_sha":"base-sha","pulls":[{"number":1,"author":"author-name","sha":"pull-sha","title":"pull-title","head_ref":"branch-name"}]},"trigger_comment":{"id":987654321,"html_url":"https://github.com/org-name/repo-name/pull/1#issuecomment-987654321"}}`, + "REPO_OWNER": "org-name", + "REPO_NAME": "repo-name", + "SRC_BASE": "org-name/repo-name", + "SRC_HOST": "github.com", + "PULL_BASE_REF": "base-ref", + "PULL_BASE_SHA": "base-sha", + "PULL_REFS": "base-ref:base-sha,1:pull-sha", + "PULL_HEAD_REF": "branch-name", + "PULL_NUMBER": "1", + "PULL_PULL_SHA": "pull-sha", + "PULL_TITLE": "pull-title", + "TRIGGER_COMMENT_ID": "987654321", + "TRIGGER_COMMENT_URL": "https://github.com/org-name/repo-name/pull/1#issuecomment-987654321", + }, + }, + { + // Review-body and PR-body events have a URL but no comment ID (ID==0). + // Only TRIGGER_COMMENT_URL should be emitted; TRIGGER_COMMENT_ID must be absent. + name: "presubmit job triggered by a review (url-only, no comment id)", + spec: JobSpec{ + Type: prowapi.PresubmitJob, + Job: "job-name", + BuildID: "0", + ProwJobID: "prowjob", + Refs: &prowapi.Refs{ + Org: "org-name", + Repo: "repo-name", + BaseRef: "base-ref", + BaseSHA: "base-sha", + Pulls: []prowapi.Pull{{ + Number: 1, + Author: "author-name", + SHA: "pull-sha", + HeadRef: "branch-name", + Title: "pull-title", + }}, + }, + TriggerComment: &prowapi.TriggerComment{ + // ID intentionally zero — mirrors a PR review event + HTMLURL: "https://github.com/org-name/repo-name/pull/1#pullrequestreview-111", + }, + }, + expected: map[string]string{ + "CI": "true", + "JOB_NAME": "job-name", + "BUILD_ID": "0", + "PROW_JOB_ID": "prowjob", + "JOB_TYPE": "presubmit", + "JOB_SPEC": `{"type":"presubmit","job":"job-name","buildid":"0","prowjobid":"prowjob","refs":{"org":"org-name","repo":"repo-name","base_ref":"base-ref","base_sha":"base-sha","pulls":[{"number":1,"author":"author-name","sha":"pull-sha","title":"pull-title","head_ref":"branch-name"}]},"trigger_comment":{"html_url":"https://github.com/org-name/repo-name/pull/1#pullrequestreview-111"}}`, + "REPO_OWNER": "org-name", + "REPO_NAME": "repo-name", + "SRC_BASE": "org-name/repo-name", + "SRC_HOST": "github.com", + "PULL_BASE_REF": "base-ref", + "PULL_BASE_SHA": "base-sha", + "PULL_REFS": "base-ref:base-sha,1:pull-sha", + "PULL_HEAD_REF": "branch-name", + "PULL_NUMBER": "1", + "PULL_PULL_SHA": "pull-sha", + "PULL_TITLE": "pull-title", + "TRIGGER_COMMENT_URL": "https://github.com/org-name/repo-name/pull/1#pullrequestreview-111", + // TRIGGER_COMMENT_ID must NOT be present when ID is zero + }, + }, { name: "postsubmit job with path alias", spec: JobSpec{ @@ -272,7 +368,7 @@ func TestEnvironmentForSpec(t *testing.T) { } func TestGetRevisionFromSpec(t *testing.T) { - var tests = []struct { + tests := []struct { name string spec JobSpec expected string diff --git a/site/content/en/docs/jobs.md b/site/content/en/docs/jobs.md index c5959ceea9..7cf50c1611 100644 --- a/site/content/en/docs/jobs.md +++ b/site/content/en/docs/jobs.md @@ -342,6 +342,16 @@ the build. | `PULL_PULL_SHA` | | | | ✓ | Pull request head SHA. | `qwe456` | | `PULL_HEAD_REF` | | | | ✓ | Pull request branch name. | `fixup-some-stuff` | | `PULL_TITLE` | | | | ✓ | Pull request title. | `Add something` | +| `TRIGGER_COMMENT_ID` | | | | ✓ | ID of the GitHub comment whose command triggered the job. Only set when the job was triggered by a comment (e.g. `/test`, `/retest`). | `987654321` | +| `TRIGGER_COMMENT_URL` | | | | ✓ | HTML URL of the GitHub comment whose command triggered the job. Only set for comment-triggered jobs. | `https://github.com/org/repo/pull/5#issuecomment-987654321` | + +Note: `TRIGGER_COMMENT_ID` and `TRIGGER_COMMENT_URL` (and the corresponding +`trigger_comment` object with `id` / `html_url` fields in `JOB_SPEC`) are only +present for presubmits that were triggered by a comment command. They are absent +for jobs triggered automatically (on PR creation/update, by Tide, or for +periodics). The ID alone is ambiguous about the comment kind, so consumers that +need to call the GitHub API should prefer the URL or combine the ID with the +refs in `JOB_SPEC`. Examples of the JSON-encoded job specification follow for the different job types: diff --git a/test/integration/config/prow/cluster/50_crd.yaml b/test/integration/config/prow/cluster/50_crd.yaml index 2c26068ede..7c867f091c 100644 --- a/test/integration/config/prow/cluster/50_crd.yaml +++ b/test/integration/config/prow/cluster/50_crd.yaml @@ -52697,6 +52697,16 @@ spec: x-kubernetes-list-type: atomic type: object type: object + trigger_comment: + description: TriggerComment, when set, references the GitHub comment whose command triggered this job (e.g. /test, /retest, /ok-to-test). It is nil for jobs that were triggered automatically (e.g. when a PR is opened or updated, by Tide, or for periodics). + properties: + html_url: + description: HTMLURL is the HTML URL of the comment. Unlike the bare ID it is always set for comment triggers and unambiguously identifies the comment regardless of its kind (issue comment, review, or review comment). + type: string + id: + description: ID is the numeric ID of the GitHub comment. It is set for issue/PR comments and review comments, but may be 0 for triggers that have no dedicated comment ID (e.g. a command in a review body or a PR description). Together with Refs it can be used to construct the GitHub API or HTML URL for the comment. + type: integer + type: object type: description: Type is the type of job and informs how the jobs is triggered enum: