fluxcd
/
kustomize-controller
mirror of https://github.com/fluxcd/kustomize-controller.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

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

The `Mark…` functions in the `conditions` package accept a format string and
(optional) arguments, just like `fmt.Printf` and friends.

In many places, the code passed an error message as the format string, causing
it to be interpreted as a format string by the `fmt` package. This leads to
issues when the message contains percent signs, e.g. URL-encoded values.

This PR adds a format string and shortens `err.Error()` to `err`, which yields
the same output.

This change is identical in principle to
https://github.com/fluxcd/source-controller/pull/1529.

Signed-off-by: Florian Forster <fforster@gitlab.com>
This commit is contained in:
Florian Forster 2024-07-08 08:27:13 +02:00
parent e70e5b36a3
commit ad38b1cb84
No known key found for this signature in database
1 changed files with 28 additions and 28 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

56
internal/controller/kustomization_controller.go
View File

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