Skip to content
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.

Sign up for GitHub

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

Fix incorrect use of format strings with the conditions package. #1025

Merged
merged 1 commit into from
Jul 12, 2024
Merged

Fix incorrect use of format strings with the conditions package. #1025

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 14 additions & 14 deletions internal/controller/helmrelease_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,7 @@ func (r *HelmReleaseReconciler) reconcileRelease(ctx context.Context, patchHelpe
if err := r.checkDependencies(ctx, obj); err != nil {
msg := fmt.Sprintf("dependencies do not meet ready condition (%s): retrying in %s",
err.Error(), r.requeueDependency.String())
conditions.MarkFalse(obj, meta.ReadyCondition, v2.DependencyNotReadyReason, err.Error())
conditions.MarkFalse(obj, meta.ReadyCondition, v2.DependencyNotReadyReason, "%s", err)
r.Eventf(obj, corev1.EventTypeNormal, v2.DependencyNotReadyReason, err.Error())
log.Info(msg)

Expand All @@ -272,8 +272,8 @@ func (r *HelmReleaseReconciler) reconcileRelease(ctx context.Context, patchHelpe
source, err := r.getSource(ctx, obj)
if err != nil {
if acl.IsAccessDenied(err) {
conditions.MarkStalled(obj, aclv1.AccessDeniedReason, err.Error())
conditions.MarkFalse(obj, meta.ReadyCondition, aclv1.AccessDeniedReason, err.Error())
conditions.MarkStalled(obj, aclv1.AccessDeniedReason, "%s", err)
conditions.MarkFalse(obj, meta.ReadyCondition, aclv1.AccessDeniedReason, "%s", err)
conditions.Delete(obj, meta.ReconcilingCondition)
r.Eventf(obj, corev1.EventTypeWarning, aclv1.AccessDeniedReason, err.Error())

Expand All @@ -284,7 +284,7 @@ func (r *HelmReleaseReconciler) reconcileRelease(ctx context.Context, patchHelpe
}

msg := fmt.Sprintf("could not get Source object: %s", err.Error())
conditions.MarkFalse(obj, meta.ReadyCondition, v2.ArtifactFailedReason, msg)
conditions.MarkFalse(obj, meta.ReadyCondition, v2.ArtifactFailedReason, "%s", msg)
return ctrl.Result{}, err
}
// Remove any stale corresponding Ready=False condition with Unknown.
Expand All @@ -295,7 +295,7 @@ func (r *HelmReleaseReconciler) reconcileRelease(ctx context.Context, patchHelpe
// Check if the source is ready.
if ready, msg := isSourceReady(source); !ready {
log.Info(msg)
conditions.MarkFalse(obj, meta.ReadyCondition, "SourceNotReady", msg)
conditions.MarkFalse(obj, meta.ReadyCondition, "SourceNotReady", "%s", msg)
// Do not requeue immediately, when the artifact is created
// the watcher should trigger a reconciliation.
return jitter.JitteredRequeueInterval(ctrl.Result{RequeueAfter: obj.GetRequeueAfter()}), errWaitForChart
Expand All @@ -308,7 +308,7 @@ func (r *HelmReleaseReconciler) reconcileRelease(ctx context.Context, patchHelpe
// Compose values based from the spec and references.
values, err := chartutil.ChartValuesFromReferences(ctx, r.Client, obj.Namespace, obj.GetValues(), obj.Spec.ValuesFrom...)
if err != nil {
conditions.MarkFalse(obj, meta.ReadyCondition, "ValuesError", err.Error())
conditions.MarkFalse(obj, meta.ReadyCondition, "ValuesError", "%s", err)
r.Eventf(obj, corev1.EventTypeWarning, "ValuesError", err.Error())
return ctrl.Result{}, err
}
Expand All @@ -322,12 +322,12 @@ func (r *HelmReleaseReconciler) reconcileRelease(ctx context.Context, patchHelpe
if err != nil {
if errors.Is(err, loader.ErrFileNotFound) {
msg := fmt.Sprintf("Source not ready: artifact not found. Retrying in %s", r.requeueDependency.String())
conditions.MarkFalse(obj, meta.ReadyCondition, v2.ArtifactFailedReason, msg)
conditions.MarkFalse(obj, meta.ReadyCondition, v2.ArtifactFailedReason, "%s", msg)
log.Info(msg)
return ctrl.Result{RequeueAfter: r.requeueDependency}, errWaitForDependency
}

conditions.MarkFalse(obj, meta.ReadyCondition, v2.ArtifactFailedReason, fmt.Sprintf("Could not load chart: %s", err.Error()))
conditions.MarkFalse(obj, meta.ReadyCondition, v2.ArtifactFailedReason, "Could not load chart: %s", err)
r.Eventf(obj, corev1.EventTypeWarning, v2.ArtifactFailedReason, err.Error())
return ctrl.Result{}, err
}
Expand All @@ -338,14 +338,14 @@ func (r *HelmReleaseReconciler) reconcileRelease(ctx context.Context, patchHelpe

ociDigest, err := mutateChartWithSourceRevision(loadedChart, source)
if err != nil {
conditions.MarkFalse(obj, meta.ReadyCondition, "ChartMutateError", err.Error())
conditions.MarkFalse(obj, meta.ReadyCondition, "ChartMutateError", "%s", err)
return ctrl.Result{}, err
}

// Build the REST client getter.
getter, err := r.buildRESTClientGetter(ctx, obj)
if err != nil {
conditions.MarkFalse(obj, meta.ReadyCondition, "RESTClientError", err.Error())
conditions.MarkFalse(obj, meta.ReadyCondition, "RESTClientError", "%s", err)
return ctrl.Result{}, err
}
// Remove any stale corresponding Ready=False condition with Unknown.
Expand Down Expand Up @@ -403,7 +403,7 @@ func (r *HelmReleaseReconciler) reconcileRelease(ctx context.Context, patchHelpe
action.WithStorageLog(action.NewDebugLog(ctrl.LoggerFrom(ctx).V(logger.TraceLevel))),
)
if err != nil {
conditions.MarkFalse(obj, meta.ReadyCondition, "FactoryError", err.Error())
conditions.MarkFalse(obj, meta.ReadyCondition, "FactoryError", "%s", err)
return ctrl.Result{}, err
}
// Remove any stale corresponding Ready=False condition with Unknown.
Expand Down Expand Up @@ -491,7 +491,7 @@ func (r *HelmReleaseReconciler) reconcileReleaseDeletion(ctx context.Context, ob
}

conditions.MarkFalse(obj, meta.ReadyCondition, v2.UninstallFailedReason,
"failed to build REST client getter to uninstall release: %s", err.Error())
"failed to build REST client getter to uninstall release: %s", err)
return err
}

Expand Down Expand Up @@ -525,7 +525,7 @@ func (r *HelmReleaseReconciler) reconcileReleaseDeletion(ctx context.Context, ob
}

conditions.MarkFalse(obj, meta.ReadyCondition, v2.UninstallFailedReason,
"failed to confirm ServiceAccount '%s' can be used to uninstall release: %s", serviceAccount, err.Error())
"failed to confirm ServiceAccount '%s' can be used to uninstall release: %s", serviceAccount, err)
return err
}
}
Expand Down Expand Up @@ -562,7 +562,7 @@ func (r *HelmReleaseReconciler) reconcileUninstall(ctx context.Context, getter g
action.WithStorageLog(action.NewDebugLog(ctrl.LoggerFrom(ctx).V(logger.TraceLevel))),
)
if err != nil {
conditions.MarkFalse(obj, meta.ReadyCondition, "ConfigFactoryErr", err.Error())
conditions.MarkFalse(obj, meta.ReadyCondition, "ConfigFactoryErr", "%s", err)
return err
}

Expand Down
14 changes: 7 additions & 7 deletions internal/reconcile/atomic_release.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ func (r *AtomicRelease) Reconcile(ctx context.Context, req *Request) error {
log.V(logger.DebugLevel).Info("determining current state of Helm release")
state, err := DetermineReleaseState(ctx, r.configFactory, req)
if err != nil {
conditions.MarkFalse(req.Object, meta.ReadyCondition, "StateError", fmt.Sprintf("Could not determine release state: %s", err.Error()))
conditions.MarkFalse(req.Object, meta.ReadyCondition, "StateError", "Could not determine release state: %s", err)
return fmt.Errorf("cannot determine release state: %w", err)
}

Expand All @@ -198,7 +198,7 @@ func (r *AtomicRelease) Reconcile(ctx context.Context, req *Request) error {
return err
}
if errors.Is(err, ErrMissingRollbackTarget) {
conditions.MarkStalled(req.Object, "MissingRollbackTarget", "Failed to perform remediation: %s", err.Error())
conditions.MarkStalled(req.Object, "MissingRollbackTarget", "Failed to perform remediation: %s", err)
return err
}
return err
Expand Down Expand Up @@ -231,7 +231,7 @@ func (r *AtomicRelease) Reconcile(ctx context.Context, req *Request) error {
)

if remediation := req.Object.GetActiveRemediation(); remediation == nil || !remediation.RetriesExhausted(req.Object) {
conditions.MarkReconciling(req.Object, meta.ProgressingWithRetryReason, conditions.GetMessage(req.Object, meta.ReadyCondition))
conditions.MarkReconciling(req.Object, meta.ProgressingWithRetryReason, "%s", conditions.GetMessage(req.Object, meta.ReadyCondition))
return ErrMustRequeue
}

Expand All @@ -243,14 +243,14 @@ func (r *AtomicRelease) Reconcile(ctx context.Context, req *Request) error {
// This to show continuous progress, as Helm actions can be long-running.
reconcilingMsg := fmt.Sprintf("Running '%s' action with timeout of %s",
next.Name(), timeoutForAction(next, req.Object).String())
conditions.MarkReconciling(req.Object, meta.ProgressingReason, reconcilingMsg)
conditions.MarkReconciling(req.Object, meta.ProgressingReason, "%s", reconcilingMsg)

// If the next action is a release action, we can mark the release
// as progressing in terms of readiness as well. Doing this for any
// other action type is not useful, as it would potentially
// overwrite more important failure state from an earlier action.
if next.Type() == ReconcilerTypeRelease {
conditions.MarkUnknown(req.Object, meta.ReadyCondition, meta.ProgressingReason, reconcilingMsg)
conditions.MarkUnknown(req.Object, meta.ReadyCondition, meta.ProgressingReason, "%s", reconcilingMsg)
}

// Patch the object to reflect the new condition.
Expand All @@ -262,7 +262,7 @@ func (r *AtomicRelease) Reconcile(ctx context.Context, req *Request) error {
log.Info(fmt.Sprintf("running '%s' action with timeout of %s", next.Name(), timeoutForAction(next, req.Object).String()))
if err = next.Reconcile(ctx, req); err != nil {
if conditions.IsReady(req.Object) {
conditions.MarkFalse(req.Object, meta.ReadyCondition, "ReconcileError", err.Error())
conditions.MarkFalse(req.Object, meta.ReadyCondition, "ReconcileError", "%s", err)
}
return err
}
Expand All @@ -275,7 +275,7 @@ func (r *AtomicRelease) Reconcile(ctx context.Context, req *Request) error {

remediation := req.Object.GetActiveRemediation()
if remediation == nil || !remediation.RetriesExhausted(req.Object) {
conditions.MarkReconciling(req.Object, meta.ProgressingWithRetryReason, conditions.GetMessage(req.Object, meta.ReadyCondition))
conditions.MarkReconciling(req.Object, meta.ProgressingWithRetryReason, "%s", conditions.GetMessage(req.Object, meta.ReadyCondition))
return ErrMustRequeue
}
// Check if retries have exhausted after remediation for early
Expand Down
4 changes: 2 additions & 2 deletions internal/reconcile/install.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ func (r *Install) failure(req *Request, buffer *action.LogBuffer, err error) {

// Mark install failure on object.
req.Object.Status.Failures++
conditions.MarkFalse(req.Object, v2.ReleasedCondition, v2.InstallFailedReason, msg)
conditions.MarkFalse(req.Object, v2.ReleasedCondition, v2.InstallFailedReason, "%s", msg)

// Record warning event, this message contains more data than the
// Condition summary.
Expand All @@ -173,7 +173,7 @@ func (r *Install) success(req *Request) {
msg := fmt.Sprintf(fmtInstallSuccess, cur.FullReleaseName(), cur.VersionedChartName())

// Mark install success on object.
conditions.MarkTrue(req.Object, v2.ReleasedCondition, v2.InstallSucceededReason, msg)
conditions.MarkTrue(req.Object, v2.ReleasedCondition, v2.InstallSucceededReason, "%s", msg)
if req.Object.GetTest().Enable && !cur.HasBeenTested() {
conditions.MarkUnknown(req.Object, v2.TestSuccessCondition, "AwaitingTests", fmtTestPending,
cur.FullReleaseName(), cur.VersionedChartName())
Expand Down
4 changes: 2 additions & 2 deletions internal/reconcile/rollback_remediation.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ func (r *RollbackRemediation) failure(req *Request, prev *v2.Snapshot, buffer *a

// Mark remediation failure on object.
req.Object.Status.Failures++
conditions.MarkFalse(req.Object, v2.RemediatedCondition, v2.RollbackFailedReason, msg)
conditions.MarkFalse(req.Object, v2.RemediatedCondition, v2.RollbackFailedReason, "%s", msg)

// Record warning event, this message contains more data than the
// Condition summary.
Expand All @@ -158,7 +158,7 @@ func (r *RollbackRemediation) success(req *Request, prev *v2.Snapshot) {
msg := fmt.Sprintf(fmtRollbackRemediationSuccess, prev.FullReleaseName(), prev.VersionedChartName())

// Mark remediation success on object.
conditions.MarkTrue(req.Object, v2.RemediatedCondition, v2.RollbackSucceededReason, msg)
conditions.MarkTrue(req.Object, v2.RemediatedCondition, v2.RollbackSucceededReason, "%s", msg)

// Record event.
r.eventRecorder.AnnotatedEventf(
Expand Down
4 changes: 2 additions & 2 deletions internal/reconcile/test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ func (r *Test) failure(req *Request, err error) {

// Mark test failure on object.
req.Object.Status.Failures++
conditions.MarkFalse(req.Object, v2.TestSuccessCondition, v2.TestFailedReason, msg)
conditions.MarkFalse(req.Object, v2.TestSuccessCondition, v2.TestFailedReason, "%s", msg)

// Record warning event, this message contains more data than the
// Condition summary.
Expand Down Expand Up @@ -176,7 +176,7 @@ func (r *Test) success(req *Request) {
msg := fmt.Sprintf(fmtTestSuccess, cur.FullReleaseName(), cur.VersionedChartName(), hookMsg)

// Mark test success on object.
conditions.MarkTrue(req.Object, v2.TestSuccessCondition, v2.TestSucceededReason, msg)
conditions.MarkTrue(req.Object, v2.TestSuccessCondition, v2.TestSucceededReason, "%s", msg)

// Record event.
r.eventRecorder.AnnotatedEventf(
Expand Down
4 changes: 2 additions & 2 deletions internal/reconcile/uninstall.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ func (r *Uninstall) failure(req *Request, buffer *action.LogBuffer, err error) {

// Mark remediation failure on object.
req.Object.Status.Failures++
conditions.MarkFalse(req.Object, v2.ReleasedCondition, v2.UninstallFailedReason, msg)
conditions.MarkFalse(req.Object, v2.ReleasedCondition, v2.UninstallFailedReason, "%s", msg)

// Record warning event, this message contains more data than the
// Condition summary.
Expand All @@ -195,7 +195,7 @@ func (r *Uninstall) success(req *Request) {
msg := fmt.Sprintf(fmtUninstallSuccess, cur.FullReleaseName(), cur.VersionedChartName())

// Mark remediation success on object.
conditions.MarkFalse(req.Object, v2.ReleasedCondition, v2.UninstallSucceededReason, msg)
conditions.MarkFalse(req.Object, v2.ReleasedCondition, v2.UninstallSucceededReason, "%s", msg)

// Record warning event, this message contains more data than the
// Condition summary.
Expand Down
4 changes: 2 additions & 2 deletions internal/reconcile/uninstall_remediation.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ func (r *UninstallRemediation) failure(req *Request, buffer *action.LogBuffer, e

// Mark uninstall failure on object.
req.Object.Status.Failures++
conditions.MarkFalse(req.Object, v2.RemediatedCondition, v2.UninstallFailedReason, msg)
conditions.MarkFalse(req.Object, v2.RemediatedCondition, v2.UninstallFailedReason, "%s", msg)

// Record warning event, this message contains more data than the
// Condition summary.
Expand All @@ -170,7 +170,7 @@ func (r *UninstallRemediation) success(req *Request) {
msg := fmt.Sprintf(fmtUninstallRemediationSuccess, cur.FullReleaseName(), cur.VersionedChartName())

// Mark remediation success on object.
conditions.MarkTrue(req.Object, v2.RemediatedCondition, v2.UninstallSucceededReason, msg)
conditions.MarkTrue(req.Object, v2.RemediatedCondition, v2.UninstallSucceededReason, "%s", msg)

// Record event.
r.eventRecorder.AnnotatedEventf(
Expand Down
4 changes: 2 additions & 2 deletions internal/reconcile/unlock.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ func (r *Unlock) failure(req *Request, cur *v2.Snapshot, status helmrelease.Stat

// Mark unlock failure on object.
req.Object.Status.Failures++
conditions.MarkFalse(req.Object, v2.ReleasedCondition, "PendingRelease", msg)
conditions.MarkFalse(req.Object, v2.ReleasedCondition, "PendingRelease", "%s", msg)

// Record warning event.
r.eventRecorder.AnnotatedEventf(
Expand All @@ -133,7 +133,7 @@ func (r *Unlock) success(req *Request, cur *v2.Snapshot, status helmrelease.Stat
msg := fmt.Sprintf(fmtUnlockSuccess, cur.FullReleaseName(), cur.VersionedChartName(), status.String())

// Mark unlock success on object.
conditions.MarkFalse(req.Object, v2.ReleasedCondition, "PendingRelease", msg)
conditions.MarkFalse(req.Object, v2.ReleasedCondition, "PendingRelease", "%s", msg)

// Record event.
r.eventRecorder.AnnotatedEventf(
Expand Down
4 changes: 2 additions & 2 deletions internal/reconcile/upgrade.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ func (r *Upgrade) failure(req *Request, buffer *action.LogBuffer, err error) {

// Mark upgrade failure on object.
req.Object.Status.Failures++
conditions.MarkFalse(req.Object, v2.ReleasedCondition, v2.UpgradeFailedReason, msg)
conditions.MarkFalse(req.Object, v2.ReleasedCondition, v2.UpgradeFailedReason, "%s", msg)

// Record warning event, this message contains more data than the
// Condition summary.
Expand All @@ -163,7 +163,7 @@ func (r *Upgrade) success(req *Request) {
msg := fmt.Sprintf(fmtUpgradeSuccess, cur.FullReleaseName(), cur.VersionedChartName())

// Mark upgrade success on object.
conditions.MarkTrue(req.Object, v2.ReleasedCondition, v2.UpgradeSucceededReason, msg)
conditions.MarkTrue(req.Object, v2.ReleasedCondition, v2.UpgradeSucceededReason, "%s", msg)
if req.Object.GetTest().Enable && !cur.HasBeenTested() {
conditions.MarkUnknown(req.Object, v2.TestSuccessCondition, "AwaitingTests", fmtTestPending,
cur.FullReleaseName(), cur.VersionedChartName())
Expand Down