Merge pull request #1198 from octo/fix-conditions-usage

Fix incorrect use of format strings with the `conditions` package.

internal/controller/kustomization_controller.go

@@ -223,7 +223,7 @@ func (r *KustomizationReconciler) Reconcile(ctx context.Context, req ctrl.Reques
  // Resolve the source reference and requeue the reconciliation if the source is not found.
  artifactSource, err := r.getSource(ctx, obj)
  if err != nil {
- conditions.MarkFalse(obj, meta.ReadyCondition, meta.ArtifactFailedReason, err.Error())
+ conditions.MarkFalse(obj, meta.ReadyCondition, meta.ArtifactFailedReason, "%s", err)
 
  if apierrors.IsNotFound(err) {
  msg := fmt.Sprintf("Source '%s' not found", obj.Spec.SourceRef.String())
@@ -232,7 +232,7 @@ func (r *KustomizationReconciler) Reconcile(ctx context.Context, req ctrl.Reques
  }
 
  if acl.IsAccessDenied(err) {
- conditions.MarkFalse(obj, meta.ReadyCondition, apiacl.AccessDeniedReason, err.Error())
+ conditions.MarkFalse(obj, meta.ReadyCondition, apiacl.AccessDeniedReason, "%s", err)
  log.Error(err, "Access denied to cross-namespace source")
  r.event(obj, "unknown", eventv1.EventSeverityError, err.Error(), nil)
  return ctrl.Result{RequeueAfter: obj.GetRetryInterval()}, nil
@@ -245,15 +245,15 @@ func (r *KustomizationReconciler) Reconcile(ctx context.Context, req ctrl.Reques
  // Requeue the reconciliation if the source artifact is not found.
  if artifactSource.GetArtifact() == nil {
  msg := fmt.Sprintf("Source artifact not found, retrying in %s", r.requeueDependency.String())
- conditions.MarkFalse(obj, meta.ReadyCondition, meta.ArtifactFailedReason, msg)
+ conditions.MarkFalse(obj, meta.ReadyCondition, meta.ArtifactFailedReason, "%s", msg)
  log.Info(msg)
  return ctrl.Result{RequeueAfter: r.requeueDependency}, nil
  }
 
  // Check dependencies and requeue the reconciliation if the check fails.
  if len(obj.Spec.DependsOn) > 0 {
  if err := r.checkDependencies(ctx, obj, artifactSource); err != nil {
- conditions.MarkFalse(obj, meta.ReadyCondition, meta.DependencyNotReadyReason, err.Error())
+ conditions.MarkFalse(obj, meta.ReadyCondition, meta.DependencyNotReadyReason, "%s", err)
  msg := fmt.Sprintf("Dependencies do not meet ready condition, retrying in %s", r.requeueDependency.String())
  log.Info(msg)
  r.event(obj, artifactSource.GetArtifact().Revision, eventv1.EventSeverityInfo, msg, nil)
@@ -268,7 +268,7 @@ func (r *KustomizationReconciler) Reconcile(ctx context.Context, req ctrl.Reques
  // Requeue at the specified retry interval if the artifact tarball is not found.
  if errors.Is(reconcileErr, fetch.ErrFileNotFound) {
  msg := fmt.Sprintf("Source is not ready, artifact not found, retrying in %s", r.requeueDependency.String())
- conditions.MarkFalse(obj, meta.ReadyCondition, meta.ArtifactFailedReason, msg)
+ conditions.MarkFalse(obj, meta.ReadyCondition, meta.ArtifactFailedReason, "%s", msg)
  log.Info(msg)
  return ctrl.Result{RequeueAfter: r.requeueDependency}, nil
  }
@@ -299,8 +299,8 @@ func (r *KustomizationReconciler) reconcile(
  // Update status with the reconciliation progress.
  revision := src.GetArtifact().Revision
  progressingMsg := fmt.Sprintf("Fetching manifests for revision %s with a timeout of %s", revision, obj.GetTimeout().String())
- conditions.MarkUnknown(obj, meta.ReadyCondition, meta.ProgressingReason, "Reconciliation in progress")
- conditions.MarkReconciling(obj, meta.ProgressingReason, progressingMsg)
+ conditions.MarkUnknown(obj, meta.ReadyCondition, meta.ProgressingReason, "%s", "Reconciliation in progress")
+ conditions.MarkReconciling(obj, meta.ProgressingReason, "%s", progressingMsg)
  if err := r.patch(ctx, obj, patcher); err != nil {
  return fmt.Errorf("failed to update status: %w", err)
  }
@@ -315,7 +315,7 @@ func (r *KustomizationReconciler) reconcile(
  tmpDir, err := MkdirTempAbs("", "kustomization-")
  if err != nil {
  err = fmt.Errorf("tmp dir error: %w", err)
- conditions.MarkFalse(obj, meta.ReadyCondition, sourcev1.DirCreationFailedReason, err.Error())
+ conditions.MarkFalse(obj, meta.ReadyCondition, sourcev1.DirCreationFailedReason, "%s", err)
  return err
  }
 
@@ -333,27 +333,27 @@ func (r *KustomizationReconciler) reconcile(
  os.Getenv("SOURCE_CONTROLLER_LOCALHOST"),
  ctrl.LoggerFrom(ctx),
  ).Fetch(src.GetArtifact().URL, src.GetArtifact().Digest, tmpDir); err != nil {
- conditions.MarkFalse(obj, meta.ReadyCondition, meta.ArtifactFailedReason, err.Error())
+ conditions.MarkFalse(obj, meta.ReadyCondition, meta.ArtifactFailedReason, "%s", err)
  return err
  }
 
  // check build path exists
  dirPath, err := securejoin.SecureJoin(tmpDir, obj.Spec.Path)
  if err != nil {
- conditions.MarkFalse(obj, meta.ReadyCondition, meta.ArtifactFailedReason, err.Error())
+ conditions.MarkFalse(obj, meta.ReadyCondition, meta.ArtifactFailedReason, "%s", err)
  return err
  }
 
  if _, err := os.Stat(dirPath); err != nil {
  err = fmt.Errorf("kustomization path not found: %w", err)
- conditions.MarkFalse(obj, meta.ReadyCondition, meta.ArtifactFailedReason, err.Error())
+ conditions.MarkFalse(obj, meta.ReadyCondition, meta.ArtifactFailedReason, "%s", err)
  return err
  }
 
  // Report progress and set last attempted revision in status.
  obj.Status.LastAttemptedRevision = revision
  progressingMsg = fmt.Sprintf("Building manifests for revision %s with a timeout of %s", revision, obj.GetTimeout().String())
- conditions.MarkReconciling(obj, meta.ProgressingReason, progressingMsg)
+ conditions.MarkReconciling(obj, meta.ProgressingReason, "%s", progressingMsg)
  if err := r.patch(ctx, obj, patcher); err != nil {
  return fmt.Errorf("failed to update status: %w", err)
  }
@@ -373,33 +373,33 @@ func (r *KustomizationReconciler) reconcile(
  // Create the Kubernetes client that runs under impersonation.
  kubeClient, statusPoller, err := impersonation.GetClient(ctx)
  if err != nil {
- conditions.MarkFalse(obj, meta.ReadyCondition, meta.ReconciliationFailedReason, err.Error())
+ conditions.MarkFalse(obj, meta.ReadyCondition, meta.ReconciliationFailedReason, "%s", err)
  return fmt.Errorf("failed to build kube client: %w", err)
  }
 
  // Generate kustomization.yaml if needed.
  k, err := runtime.DefaultUnstructuredConverter.ToUnstructured(obj)
  if err != nil {
- conditions.MarkFalse(obj, meta.ReadyCondition, meta.BuildFailedReason, err.Error())
+ conditions.MarkFalse(obj, meta.ReadyCondition, meta.BuildFailedReason, "%s", err)
  return err
  }
  err = r.generate(unstructured.Unstructured{Object: k}, tmpDir, dirPath)
  if err != nil {
- conditions.MarkFalse(obj, meta.ReadyCondition, meta.BuildFailedReason, err.Error())
+ conditions.MarkFalse(obj, meta.ReadyCondition, meta.BuildFailedReason, "%s", err)
  return err
  }
 
  // Build the Kustomize overlay and decrypt secrets if needed.
  resources, err := r.build(ctx, obj, unstructured.Unstructured{Object: k}, tmpDir, dirPath)
  if err != nil {
- conditions.MarkFalse(obj, meta.ReadyCondition, meta.BuildFailedReason, err.Error())
+ conditions.MarkFalse(obj, meta.ReadyCondition, meta.BuildFailedReason, "%s", err)
  return err
  }
 
  // Convert the build result into Kubernetes unstructured objects.
  objects, err := ssautil.ReadObjects(bytes.NewReader(resources))
  if err != nil {
- conditions.MarkFalse(obj, meta.ReadyCondition, meta.BuildFailedReason, err.Error())
+ conditions.MarkFalse(obj, meta.ReadyCondition, meta.BuildFailedReason, "%s", err)
  return err
  }
 
@@ -413,23 +413,23 @@ func (r *KustomizationReconciler) reconcile(
 
  // Update status with the reconciliation progress.
  progressingMsg = fmt.Sprintf("Detecting drift for revision %s with a timeout of %s", revision, obj.GetTimeout().String())
- conditions.MarkReconciling(obj, meta.ProgressingReason, progressingMsg)
+ conditions.MarkReconciling(obj, meta.ProgressingReason, "%s", progressingMsg)
  if err := r.patch(ctx, obj, patcher); err != nil {
  return fmt.Errorf("failed to update status: %w", err)
  }
 
  // Validate and apply resources in stages.
  drifted, changeSet, err := r.apply(ctx, resourceManager, obj, revision, objects)
  if err != nil {
- conditions.MarkFalse(obj, meta.ReadyCondition, meta.ReconciliationFailedReason, err.Error())
+ conditions.MarkFalse(obj, meta.ReadyCondition, meta.ReconciliationFailedReason, "%s", err)
  return err
  }
 
  // Create an inventory from the reconciled resources.
  newInventory := inventory.New()
  err = inventory.AddChangeSet(newInventory, changeSet)
  if err != nil {
- conditions.MarkFalse(obj, meta.ReadyCondition, meta.ReconciliationFailedReason, err.Error())
+ conditions.MarkFalse(obj, meta.ReadyCondition, meta.ReconciliationFailedReason, "%s", err)
  return err
  }
 
@@ -439,13 +439,13 @@ func (r *KustomizationReconciler) reconcile(
  // Detect stale resources which are subject to garbage collection.
  staleObjects, err := inventory.Diff(oldInventory, newInventory)
  if err != nil {
- conditions.MarkFalse(obj, meta.ReadyCondition, meta.ReconciliationFailedReason, err.Error())
+ conditions.MarkFalse(obj, meta.ReadyCondition, meta.ReconciliationFailedReason, "%s", err)
  return err
  }
 
  // Run garbage collection for stale resources that do not have pruning disabled.
  if _, err := r.prune(ctx, resourceManager, obj, revision, staleObjects); err != nil {
- conditions.MarkFalse(obj, meta.ReadyCondition, meta.PruneFailedReason, err.Error())
+ conditions.MarkFalse(obj, meta.ReadyCondition, meta.PruneFailedReason, "%s", err)
  return err
  }
 
@@ -459,7 +459,7 @@ func (r *KustomizationReconciler) reconcile(
  isNewRevision,
  drifted,
  changeSet.ToObjMetadataSet()); err != nil {
- conditions.MarkFalse(obj, meta.ReadyCondition, meta.HealthCheckFailedReason, err.Error())
+ conditions.MarkFalse(obj, meta.ReadyCondition, meta.HealthCheckFailedReason, "%s", err)
  return err
  }
 
@@ -890,8 +890,8 @@ func (r *KustomizationReconciler) checkHealth(ctx context.Context,
 
  // Update status with the reconciliation progress.
  message := fmt.Sprintf("Running health checks for revision %s with a timeout of %s", revision, obj.GetTimeout().String())
- conditions.MarkReconciling(obj, meta.ProgressingReason, message)
- conditions.MarkUnknown(obj, meta.HealthyCondition, meta.ProgressingReason, message)
+ conditions.MarkReconciling(obj, meta.ProgressingReason, "%s", message)
+ conditions.MarkUnknown(obj, meta.HealthyCondition, meta.ProgressingReason, "%s", message)
  if err := r.patch(ctx, obj, patcher); err != nil {
  return fmt.Errorf("unable to update the healthy status to progressing: %w", err)
  }
@@ -902,8 +902,8 @@ func (r *KustomizationReconciler) checkHealth(ctx context.Context,
  Timeout: obj.GetTimeout(),
  FailFast: r.FailFast,
  }); err != nil {
- conditions.MarkFalse(obj, meta.ReadyCondition, meta.HealthCheckFailedReason, err.Error())
- conditions.MarkFalse(obj, meta.HealthyCondition, meta.HealthCheckFailedReason, err.Error())
+ conditions.MarkFalse(obj, meta.ReadyCondition, meta.HealthCheckFailedReason, "%s", err)
+ conditions.MarkFalse(obj, meta.HealthyCondition, meta.HealthCheckFailedReason, "%s", err)
  return fmt.Errorf("health check failed after %s: %w", time.Since(checkStart).String(), err)
  }
 
@@ -913,7 +913,7 @@ func (r *KustomizationReconciler) checkHealth(ctx context.Context,
  r.event(obj, revision, eventv1.EventSeverityInfo, msg, nil)
  }
 
- conditions.MarkTrue(obj, meta.HealthyCondition, meta.SucceededReason, msg)
+ conditions.MarkTrue(obj, meta.HealthyCondition, meta.SucceededReason, "%s", msg)
  if err := r.patch(ctx, obj, patcher); err != nil {
  return fmt.Errorf("unable to update the healthy status to progressing: %w", err)
  }