Skip to content

Commit 61aa30c

Browse files
committed
Use formatting placeholders for errors in events
This should prevent events from containing malformed messages, should error messages ever contain `%` symbols.
1 parent f9e94db commit 61aa30c

5 files changed

Lines changed: 41 additions & 11 deletions

File tree

internal/cmd/controller/gitops/reconciler/gitjob_controller.go

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -207,7 +207,8 @@ func (r *GitJobReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr
207207
corev1.EventTypeWarning,
208208
"FailedToApplyRestrictions",
209209
"ApplyGitRepoRestrictions",
210-
err.Error(),
210+
"%v",
211+
err,
211212
)
212213

213214
return ctrl.Result{}, updateErrorStatus(ctx, r.Client, req.NamespacedName, *oldStatus, err)
@@ -319,7 +320,8 @@ func (r *GitJobReconciler) manageGitJob(ctx context.Context, logger logr.Logger,
319320
corev1.EventTypeWarning,
320321
"FailedToGetGitJob",
321322
"GetGitJob",
322-
err.Error(),
323+
"%v",
324+
err,
323325
)
324326

325327
return ctrl.Result{}, err
@@ -334,7 +336,8 @@ func (r *GitJobReconciler) manageGitJob(ctx context.Context, logger logr.Logger,
334336
corev1.EventTypeWarning,
335337
"FailedValidatingSecret",
336338
"ValidateSecret",
337-
err.Error(),
339+
"%v",
340+
err,
338341
)
339342
return ctrl.Result{}, fmt.Errorf("error validating external secrets: %w", err)
340343
}
@@ -360,7 +363,8 @@ func (r *GitJobReconciler) manageGitJob(ctx context.Context, logger logr.Logger,
360363
corev1.EventTypeWarning,
361364
"Failed",
362365
"MonitorLatestCommit",
363-
err.Error(),
366+
"%v",
367+
err,
364368
)
365369
} else if oldCommit != gitrepo.Status.Commit {
366370
r.Recorder.Eventf(
@@ -383,7 +387,8 @@ func (r *GitJobReconciler) manageGitJob(ctx context.Context, logger logr.Logger,
383387
corev1.EventTypeWarning,
384388
"FailedValidatingSecret",
385389
"ValidateSecret",
386-
err.Error(),
390+
"%v",
391+
err,
387392
)
388393
return ctrl.Result{}, fmt.Errorf("error validating external secrets: %w", err)
389394
}

internal/cmd/controller/gitops/reconciler/gitjob_test.go

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,24 @@ func (m gitRepoPointerMatcher) String() string {
8585
return ""
8686
}
8787

88+
// errorMatcher implements a gomock matcher on error message strings.
89+
type errorMatcher struct {
90+
errMsg string
91+
}
92+
93+
func (m errorMatcher) Matches(x interface{}) bool {
94+
err, ok := x.(error)
95+
if !ok {
96+
return false
97+
}
98+
99+
return err.Error() == m.errMsg
100+
}
101+
102+
func (m errorMatcher) String() string {
103+
return fmt.Sprintf("matches error %q", m.errMsg)
104+
}
105+
88106
func TestReconcile_Error_WhenGitrepoRestrictionsAreNotMet(t *testing.T) {
89107
mockCtrl := gomock.NewController(t)
90108
defer mockCtrl.Finish()
@@ -135,7 +153,8 @@ func TestReconcile_Error_WhenGitrepoRestrictionsAreNotMet(t *testing.T) {
135153
corev1.EventTypeWarning,
136154
"FailedToApplyRestrictions",
137155
"ApplyGitRepoRestrictions",
138-
"empty targetNamespace denied, because allowedTargetNamespaces restriction is present",
156+
"%v",
157+
errorMatcher{"empty targetNamespace denied, because allowedTargetNamespaces restriction is present"},
139158
)
140159

141160
r := GitJobReconciler{
@@ -212,7 +231,8 @@ func TestReconcile_Error_WhenGetGitJobErrors(t *testing.T) {
212231
corev1.EventTypeWarning,
213232
"FailedToGetGitJob",
214233
"GetGitJob",
215-
"error retrieving git job: GITJOB ERROR",
234+
"%v",
235+
errorMatcher{"error retrieving git job: GITJOB ERROR"},
216236
)
217237

218238
r := GitJobReconciler{
@@ -285,7 +305,8 @@ func TestReconcile_Error_WhenSecretDoesNotExist(t *testing.T) {
285305
corev1.EventTypeWarning,
286306
"FailedValidatingSecret",
287307
"ValidateSecret",
288-
"failed to look up HelmSecretNameForPaths, error: SECRET ERROR",
308+
"%v",
309+
errorMatcher{"failed to look up HelmSecretNameForPaths, error: SECRET ERROR"},
289310
)
290311

291312
statusClient := mocks.NewMockStatusWriter(mockCtrl)

internal/cmd/controller/gitops/reconciler/polling_job.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,8 @@ func (j *gitPollingJob) pollGitRepo(ctx context.Context) error {
9595
corev1.EventTypeWarning,
9696
"FailedToCheckCommit",
9797
"CheckCommit",
98-
origErr.Error(),
98+
"%v",
99+
origErr,
99100
)
100101

101102
return j.updateErrorStatus(ctx, gitrepo, pollingTimestamp, origErr)

internal/cmd/controller/helmops/reconciler/polling_job.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,8 @@ func (j *helmPollingJob) pollHelm(ctx context.Context) error {
124124
corev1.EventTypeWarning,
125125
eventReason,
126126
eventAction,
127-
origErr.Error(),
127+
"%v",
128+
origErr,
128129
)
129130
}
130131

@@ -156,6 +157,7 @@ func (j *helmPollingJob) pollHelm(ctx context.Context) error {
156157
corev1.EventTypeNormal,
157158
"GotNewChartVersion",
158159
"GetNewChartVersion",
160+
"%s",
159161
version,
160162
)
161163
}

internal/cmd/controller/reconciler/bundle_controller.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -913,7 +913,8 @@ func (r *BundleReconciler) maybeDeleteOCIArtifact(ctx context.Context, bundle *f
913913
"DeleteOCIArtifact",
914914
"deleting OCI artifact %q: %v",
915915
bundle.Spec.ContentsID,
916-
err.Error(),
916+
"%v",
917+
err,
917918
)
918919
}
919920

0 commit comments

Comments
 (0)