Merge pull request #788 from fluxcd/tidy-nits

go.mod

@@ -41,6 +41,7 @@ require (
 	sigs.k8s.io/cli-utils v0.35.0
 	sigs.k8s.io/controller-runtime v0.15.1
 	sigs.k8s.io/kustomize/api v0.13.4
+	sigs.k8s.io/kustomize/kyaml v0.14.2
 	sigs.k8s.io/yaml v1.3.0
 )
 
@@ -167,6 +168,5 @@ require (
 	k8s.io/kubectl v0.27.3 // indirect
 	oras.land/oras-go v1.2.3 // indirect
 	sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd // indirect
-	sigs.k8s.io/kustomize/kyaml v0.14.2 // indirect
 	sigs.k8s.io/structured-merge-diff/v4 v4.3.0 // indirect
 )

internal/cmp/simple_unstructured.go

@@ -106,5 +106,4 @@ func writePathString(path cmp.Path, sb *strings.Builder) {
 			sb.WriteString(fmt.Sprintf("%v", t.String()))
 		}
 	}
-	return
 }

internal/controller/helmrelease_controller.go

@@ -189,7 +189,7 @@ func (r *HelmReleaseReconciler) Reconcile(ctx context.Context, req ctrl.Request)
 	}
 
 	// Log reconciliation duration
-	durationMsg := fmt.Sprintf("reconciliation finished in %s", time.Now().Sub(start).String())
+	durationMsg := fmt.Sprintf("reconciliation finished in %s", time.Since(start).String())
 	if result.RequeueAfter > 0 {
 		durationMsg = fmt.Sprintf("%s, next run in %s", durationMsg, result.RequeueAfter.String())
 	}
@@ -440,7 +440,7 @@ func (r *HelmReleaseReconciler) reconcileRelease(ctx context.Context,
 		// Remediate deployment failure if necessary.
 		if !remediation.RetriesExhausted(hr) || remediation.MustRemediateLastFailure() {
 			if util.ReleaseRevision(rel) <= releaseRevision {
-				log.Info(fmt.Sprintf("skipping remediation, no new release revision created"))
+				log.Info("skipping remediation, no new release revision created")
 			} else {
 				var remediationErr error
 				switch remediation.GetStrategy() {
@@ -771,13 +771,13 @@ func (r *HelmReleaseReconciler) requestsForHelmChartChange(ctx context.Context,
 	}
 
 	var reqs []reconcile.Request
-	for _, hr := range list.Items {
+	for i, hr := range list.Items {
 		// If the HelmRelease is ready and the revision of the artifact equals to the
 		// last attempted revision, we should not make a request for this HelmRelease
-		if conditions.IsReady(&hr) && hc.GetArtifact().HasRevision(hr.Status.LastAttemptedRevision) {
+		if conditions.IsReady(&list.Items[i]) && hc.GetArtifact().HasRevision(hr.Status.LastAttemptedRevision) {
 			continue
 		}
-		reqs = append(reqs, reconcile.Request{NamespacedName: client.ObjectKeyFromObject(&hr)})
+		reqs = append(reqs, reconcile.Request{NamespacedName: client.ObjectKeyFromObject(&list.Items[i])})
 	}
 	return reqs
 }

internal/controller/helmrelease_controller_chart.go

@@ -95,18 +95,6 @@ func (r *HelmReleaseReconciler) reconcileChart(ctx context.Context, hr *v2.HelmR
 	return &helmChart, nil
 }
 
-// getHelmChart retrieves the v1beta2.HelmChart for the given
-// v2beta1.HelmRelease using the name that is advertised in the status
-// object. It returns the v1beta2.HelmChart, or an error.
-func (r *HelmReleaseReconciler) getHelmChart(ctx context.Context, hr *v2.HelmRelease) (*sourcev1b2.HelmChart, error) {
-	namespace, name := hr.Status.GetHelmChart()
-	hc := &sourcev1b2.HelmChart{}
-	if err := r.Client.Get(ctx, types.NamespacedName{Namespace: namespace, Name: name}, hc); err != nil {
-		return nil, err
-	}
-	return hc, nil
-}
-
 // loadHelmChart attempts to download the artifact from the provided source,
 // loads it into a chart.Chart, and removes the downloaded artifact.
 // It returns the loaded chart.Chart on success, or an error.

internal/controller/helmrelease_controller_chart_test.go

@@ -178,15 +178,13 @@ func TestHelmReleaseReconciler_reconcileChart(t *testing.T) {
 			g.Expect(v2.AddToScheme(scheme.Scheme)).To(Succeed())
 			g.Expect(sourcev1.AddToScheme(scheme.Scheme)).To(Succeed())
 
-			var c client.Client
+			c := fake.NewClientBuilder().WithScheme(scheme.Scheme)
 			if tt.hc != nil {
-				c = fake.NewFakeClientWithScheme(scheme.Scheme, tt.hc)
-			} else {
-				c = fake.NewFakeClientWithScheme(scheme.Scheme)
+				c.WithObjects(tt.hc)
 			}
 
 			r := &HelmReleaseReconciler{
-				Client:              c,
+				Client:              c.Build(),
 				NoCrossNamespaceRef: tt.noCrossNamspaceRef,
 			}
 
@@ -203,7 +201,7 @@ func TestHelmReleaseReconciler_reconcileChart(t *testing.T) {
 
 			if tt.expectGC {
 				objKey := client.ObjectKeyFromObject(tt.hc)
-				err = c.Get(context.TODO(), objKey, tt.hc.DeepCopy())
+				err = r.Get(context.TODO(), objKey, tt.hc.DeepCopy())
 				g.Expect(apierrors.IsNotFound(err)).To(BeTrue())
 			}
 		})
@@ -259,15 +257,13 @@ func TestHelmReleaseReconciler_deleteHelmChart(t *testing.T) {
 			g.Expect(v2.AddToScheme(scheme.Scheme)).To(Succeed())
 			g.Expect(sourcev1.AddToScheme(scheme.Scheme)).To(Succeed())
 
-			var c client.Client
+			c := fake.NewClientBuilder().WithScheme(scheme.Scheme)
 			if tt.hc != nil {
-				c = fake.NewFakeClientWithScheme(scheme.Scheme, tt.hc)
-			} else {
-				c = fake.NewFakeClientWithScheme(scheme.Scheme)
+				c.WithObjects(tt.hc)
 			}
 
 			r := &HelmReleaseReconciler{
-				Client: c,
+				Client: c.Build(),
 			}
 
 			err := r.deleteHelmChart(context.TODO(), tt.hr)

internal/controller/helmrelease_controller_fuzz_test.go

@@ -28,6 +28,7 @@ import (
 	apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
+	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/client/fake"
 	"sigs.k8s.io/yaml"
 
@@ -122,7 +123,7 @@ other: values
 		// v2.ValuesReference. Therefore a static value here suffices, and instead we just
 		// play with the objects presence/absence.
 		objectName := "values"
-		resources := []runtime.Object{}
+		var resources []client.Object
 
 		if createObject {
 			resources = append(resources,
@@ -146,7 +147,7 @@ other: values
 			},
 		}
 
-		c := fake.NewFakeClientWithScheme(scheme, resources...)
+		c := fake.NewClientBuilder().WithScheme(scheme).WithObjects(resources...).Build()
 		r := &HelmReleaseReconciler{Client: c}
 		var values *apiextensionsv1.JSON
 		if hrValues != "" {
@@ -221,13 +222,13 @@ other: values
 		hc.ObjectMeta.Name = hr.GetHelmChartName()
 		hc.ObjectMeta.Namespace = hr.Spec.Chart.GetNamespace(hr.Namespace)
 
-		resources := []runtime.Object{
+		resources := []client.Object{
 			valuesConfigMap("values", map[string]string{valuesKey: configData}),
 			valuesSecret("values", map[string][]byte{valuesKey: secretData}),
 			&hc,
 		}
 
-		c := fake.NewFakeClientWithScheme(scheme, resources...)
+		c := fake.NewClientBuilder().WithScheme(scheme).WithObjects(resources...).Build()
 		r := &HelmReleaseReconciler{
 			Client:        c,
 			EventRecorder: &DummyRecorder{},

internal/controller/helmrelease_controller_test.go

@@ -43,15 +43,15 @@ func TestHelmReleaseReconciler_composeValues(t *testing.T) {
 
 	tests := []struct {
 		name       string
-		resources  []runtime.Object
+		resources  []client.Object
 		references []v2.ValuesReference
 		values     string
 		want       chartutil.Values
 		wantErr    bool
 	}{
 		{
 			name: "merges",
-			resources: []runtime.Object{
+			resources: []client.Object{
 				valuesConfigMap("values", map[string]string{
 					"values.yaml": `flat: value
 nested:
@@ -88,7 +88,7 @@ other: values
 		},
 		{
 			name: "target path",
-			resources: []runtime.Object{
+			resources: []client.Object{
 				valuesSecret("values", map[string][]byte{"single": []byte("value")}),
 			},
 			references: []v2.ValuesReference{
@@ -111,7 +111,7 @@ other: values
 		},
 		{
 			name: "target path with boolean value",
-			resources: []runtime.Object{
+			resources: []client.Object{
 				valuesSecret("values", map[string][]byte{"single": []byte("true")}),
 			},
 			references: []v2.ValuesReference{
@@ -134,7 +134,7 @@ other: values
 		},
 		{
 			name: "target path with set-string behavior",
-			resources: []runtime.Object{
+			resources: []client.Object{
 				valuesSecret("values", map[string][]byte{"single": []byte("\"true\"")}),
 			},
 			references: []v2.ValuesReference{
@@ -201,7 +201,7 @@ other: values
 		},
 		{
 			name: "missing secret key",
-			resources: []runtime.Object{
+			resources: []client.Object{
 				valuesSecret("values", nil),
 			},
 			references: []v2.ValuesReference{
@@ -215,7 +215,7 @@ other: values
 		},
 		{
 			name: "missing config map key",
-			resources: []runtime.Object{
+			resources: []client.Object{
 				valuesConfigMap("values", nil),
 			},
 			references: []v2.ValuesReference{
@@ -238,7 +238,7 @@ other: values
 		},
 		{
 			name: "invalid values",
-			resources: []runtime.Object{
+			resources: []client.Object{
 				valuesConfigMap("values", map[string]string{
 					"values.yaml": `
 invalid`,
@@ -256,7 +256,7 @@ invalid`,
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			c := fake.NewFakeClientWithScheme(scheme, tt.resources...)
+			c := fake.NewClientBuilder().WithScheme(scheme).WithObjects(tt.resources...).Build()
 			r := &HelmReleaseReconciler{Client: c}
 			var values *apiextensionsv1.JSON
 			if tt.values != "" {

internal/runner/log_buffer_test.go

@@ -37,7 +37,6 @@ func TestLogBuffer_Log(t *testing.T) {
 			var count int
 			l := NewLogBuffer(func(format string, v ...interface{}) {
 				count++
-				return
 			}, tt.size)
 			for _, v := range tt.fill {
 				l.Log("%s", v)

internal/runner/post_renderer_kustomize.go

@@ -21,10 +21,10 @@ import (
 	"encoding/json"
 	"sync"
 
-	"sigs.k8s.io/kustomize/api/filesys"
 	"sigs.k8s.io/kustomize/api/krusty"
 	"sigs.k8s.io/kustomize/api/resmap"
 	kustypes "sigs.k8s.io/kustomize/api/types"
+	"sigs.k8s.io/kustomize/kyaml/filesys"
 
 	"github.com/fluxcd/pkg/apis/kustomize"
 
@@ -46,8 +46,10 @@ func writeToFile(fs filesys.FileSystem, path string, content []byte) error {
 	if err != nil {
 		return err
 	}
-	helmOutput.Write(content)
-	if err := helmOutput.Close(); err != nil {
+	if _, err = helmOutput.Write(content); err != nil {
+		return err
+	}
+	if err = helmOutput.Close(); err != nil {
 		return err
 	}
 	return nil
@@ -58,8 +60,10 @@ func writeFile(fs filesys.FileSystem, path string, content *bytes.Buffer) error
 	if err != nil {
 		return err
 	}
-	content.WriteTo(helmOutput)
-	if err := helmOutput.Close(); err != nil {
+	if _, err = content.WriteTo(helmOutput); err != nil {
+		return err
+	}
+	if err = helmOutput.Close(); err != nil {
 		return err
 	}
 	return nil
@@ -119,14 +123,14 @@ func (k *postRendererKustomize) Run(renderedManifests *bytes.Buffer) (modifiedMa
 	}
 
 	// Add JSON 6902 patches.
-	for _, m := range k.spec.PatchesJSON6902 {
+	for i, m := range k.spec.PatchesJSON6902 {
 		patch, err := json.Marshal(m.Patch)
 		if err != nil {
 			return nil, err
 		}
 		cfg.PatchesJson6902 = append(cfg.PatchesJson6902, kustypes.Patch{
 			Patch:  string(patch),
-			Target: adaptSelector(&m.Target),
+			Target: adaptSelector(&k.spec.PatchesJSON6902[i].Target),
 		})
 	}
 

internal/runner/runner.go

@@ -246,7 +246,7 @@ func (r *Runner) applyCRDs(policy v2.CRDsPolicy, chart *chart.Chart, visitorFunc
 		res, err := r.config.KubeClient.Build(bytes.NewBuffer(obj.File.Data), false)
 		if err != nil {
 			r.config.Log("failed to parse CRDs from %s: %s", obj.Name, err)
-			return errors.New(fmt.Sprintf("failed to parse CRDs from %s: %s", obj.Name, err))
+			return fmt.Errorf("failed to parse CRDs from %s: %w", obj.Name, err)
 		}
 		allCrds = append(allCrds, res...)
 	}
@@ -275,7 +275,7 @@ func (r *Runner) applyCRDs(policy v2.CRDsPolicy, chart *chart.Chart, visitorFunc
 					continue
 				}
 				r.config.Log("failed to create CRD %s: %s", crdName, err)
-				return errors.New(fmt.Sprintf("failed to create CRD %s: %s", crdName, err))
+				return fmt.Errorf("failed to create CRD %s: %w", crdName, err)
 			} else {
 				if rr != nil && rr.Created != nil {
 					totalItems = append(totalItems, rr.Created...)
@@ -329,7 +329,7 @@ func (r *Runner) applyCRDs(policy v2.CRDsPolicy, chart *chart.Chart, visitorFunc
 		// Send them to Kube
 		if rr, err := r.config.KubeClient.Update(original, allCrds, true); err != nil {
 			r.config.Log("failed to apply CRD %s", err)
-			return errors.New(fmt.Sprintf("failed to apply CRD %s", err))
+			return fmt.Errorf("failed to apply CRD: %w", err)
 		} else {
 			if rr != nil {
 				if rr.Created != nil {