From aae67ff2a40645c7d107c4f47e672c98ec200eaa Mon Sep 17 00:00:00 2001 From: Jordan Carter Date: Mon, 18 Nov 2024 10:48:24 +1300 Subject: [PATCH 1/2] Remove signal reason unable_to_verify_signature and replace with signature_rejected --- .../job_verification_integration_test.go | 2 +- agent/run_job.go | 13 ++++++------- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/agent/integration/job_verification_integration_test.go b/agent/integration/job_verification_integration_test.go index f3cf0b7ed1..c73e88ecf6 100644 --- a/agent/integration/job_verification_integration_test.go +++ b/agent/integration/job_verification_integration_test.go @@ -445,7 +445,7 @@ func TestJobVerification(t *testing.T) { verificationJWKS: nil, mockBootstrapExpectation: func(bt *bintest.Mock) { bt.Expect().NotCalled() }, expectedExitStatus: "-1", - expectedSignalReason: agent.SignalReasonUnableToVerifySignature, + expectedSignalReason: agent.SignalReasonSignatureRejected, expectLogsContain: []string{ "+++ ⛔", "cannot verify signature. JWK for pipeline verification is not configured", diff --git a/agent/run_job.go b/agent/run_job.go index a2f916d57e..4175f41b4c 100644 --- a/agent/run_job.go +++ b/agent/run_job.go @@ -24,12 +24,11 @@ import ( ) const ( - SignalReasonAgentRefused = "agent_refused" - SignalReasonAgentStop = "agent_stop" - SignalReasonCancel = "cancel" - SignalReasonSignatureRejected = "signature_rejected" - SignalReasonUnableToVerifySignature = "unable_to_verify_signature" - SignalReasonProcessRunError = "process_run_error" + SignalReasonAgentRefused = "agent_refused" + SignalReasonAgentStop = "agent_stop" + SignalReasonCancel = "cancel" + SignalReasonSignatureRejected = "signature_rejected" + SignalReasonProcessRunError = "process_run_error" ) type missingKeyError struct { @@ -98,7 +97,7 @@ func (r *JobRunner) Run(ctx context.Context) error { if r.VerificationFailureBehavior == VerificationBehaviourBlock { exit.Status = -1 - exit.SignalReason = SignalReasonUnableToVerifySignature + exit.SignalReason = SignalReasonSignatureRejected return nil } } From ca34567fb504919b1b8fa66c7fa65ca48af7623b Mon Sep 17 00:00:00 2001 From: Jordan Carter Date: Mon, 18 Nov 2024 11:07:02 +1300 Subject: [PATCH 2/2] Add comment about not adding more signal reasons --- agent/run_job.go | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/agent/run_job.go b/agent/run_job.go index 4175f41b4c..4a79cae884 100644 --- a/agent/run_job.go +++ b/agent/run_job.go @@ -24,11 +24,23 @@ import ( ) const ( + // Signal reasons SignalReasonAgentRefused = "agent_refused" SignalReasonAgentStop = "agent_stop" SignalReasonCancel = "cancel" SignalReasonSignatureRejected = "signature_rejected" SignalReasonProcessRunError = "process_run_error" + // Don't add more signal reasons. If you must add a new signal reason, it must also be added to + // the Job::Event::SignalReason enum in the rails app. + // + // They are meant to represent the reason a job was stopped, but they've also been used to + // represent the reason a job wasn't started at all. This is fine but we don't want to pile more + // on as customers catch these signal reasons when configuring retry attempts. When we add more + // signal reasons we force customers to update their retry configurations to catch the new signal + // reasons. + // + // We should consider adding new fields 'not_run_reason' and 'not_run_details' instead of adding + // more signal reasons. ) type missingKeyError struct {