Merge pull request #1257 from fluxcd/tidy-nits

Address miscellaneous issues throughout code base

go.mod

@@ -68,7 +68,7 @@ require (
 	k8s.io/api v0.27.4
 	k8s.io/apimachinery v0.27.4
 	k8s.io/client-go v0.27.4
-	k8s.io/utils v0.0.0-20230505201702-9f6742963106
+	k8s.io/utils v0.0.0-20230726121419-3b25d923346b
 	sigs.k8s.io/cli-utils v0.35.0
 	sigs.k8s.io/controller-runtime v0.15.1
 	sigs.k8s.io/yaml v1.3.0

go.sum

@@ -1808,8 +1808,8 @@ k8s.io/kube-openapi v0.0.0-20230515203736-54b630e78af5 h1:azYPdzztXxPSa8wb+hksEK
 k8s.io/kube-openapi v0.0.0-20230515203736-54b630e78af5/go.mod h1:kzo02I3kQ4BTtEfVLaPbjvCkX97YqGve33wzlb3fofQ=
 k8s.io/kubectl v0.27.3 h1:HyC4o+8rCYheGDWrkcOQHGwDmyLKR5bxXFgpvF82BOw=
 k8s.io/kubectl v0.27.3/go.mod h1:g9OQNCC2zxT+LT3FS09ZYqnDhlvsKAfFq76oyarBcq4=
-k8s.io/utils v0.0.0-20230505201702-9f6742963106 h1:EObNQ3TW2D+WptiYXlApGNLVy0zm/JIBVY9i+M4wpAU=
+k8s.io/utils v0.0.0-20230726121419-3b25d923346b h1:sgn3ZU783SCgtaSJjpcVVlRqd6GSnlTLKgpAAttJvpI=
-k8s.io/utils v0.0.0-20230505201702-9f6742963106/go.mod h1:OLgZIPagt7ERELqWJFomSt595RzquPNLL48iOWgYOg0=
+k8s.io/utils v0.0.0-20230726121419-3b25d923346b/go.mod h1:OLgZIPagt7ERELqWJFomSt595RzquPNLL48iOWgYOg0=
 oras.land/oras-go v1.2.3 h1:v8PJl+gEAntI1pJ/LCrDgsuk+1PKVavVEPsYIHFE5uY=
 oras.land/oras-go v1.2.3/go.mod h1:M/uaPdYklze0Vf3AakfarnpoEckvw0ESbRdN8Z1vdJg=
 rsc.io/binaryregexp v0.2.0/go.mod h1:qTv7/COck+e2FymRvadv62gMdZztPaShugOCi3I+8D8=

internal/controller/bucket_controller_fetch_test.go

@@ -41,7 +41,7 @@ type mockBucketClient struct {
 	objects    map[string]mockBucketObject
 }
 
-var mockNotFound = fmt.Errorf("not found")
+var errMockNotFound = fmt.Errorf("not found")
 
 func (m mockBucketClient) BucketExists(_ context.Context, name string) (bool, error) {
 	return name == m.bucketName, nil
@@ -57,7 +57,7 @@ func (m mockBucketClient) FGetObject(_ context.Context, bucket, obj, path string
 	}
 	object, ok := m.objects[obj]
 	if !ok {
-		return "", mockNotFound
+		return "", errMockNotFound
 	}
 	if err := os.WriteFile(path, []byte(object.data), os.FileMode(0660)); err != nil {
 		return "", err
@@ -66,7 +66,7 @@ func (m mockBucketClient) FGetObject(_ context.Context, bucket, obj, path string
 }
 
 func (m mockBucketClient) ObjectIsNotFound(e error) bool {
-	return e == mockNotFound
+	return e == errMockNotFound
 }
 
 func (m mockBucketClient) VisitObjects(_ context.Context, _ string, f func(key, etag string) error) error {
@@ -78,9 +78,7 @@ func (m mockBucketClient) VisitObjects(_ context.Context, _ string, f func(key,
 	return nil
 }
 
-func (m mockBucketClient) Close(_ context.Context) {
+func (m mockBucketClient) Close(_ context.Context) {}
-	return
-}
 
 func (m *mockBucketClient) addObject(key string, object mockBucketObject) {
 	if m.objects == nil {

internal/controller/bucket_controller_test.go

@@ -258,7 +258,7 @@ func TestBucketReconciler_reconcileStorage(t *testing.T) {
 			name: "notices missing artifact in storage",
 			beforeFunc: func(obj *bucketv1.Bucket, storage *Storage) error {
 				obj.Status.Artifact = &sourcev1.Artifact{
-					Path:     fmt.Sprintf("/reconcile-storage/invalid.txt"),
+					Path:     "/reconcile-storage/invalid.txt",
 					Revision: "d",
 				}
 				storage.SetArtifactURL(obj.Status.Artifact)
@@ -339,7 +339,7 @@ func TestBucketReconciler_reconcileStorage(t *testing.T) {
 			name: "updates hostname on diff from current",
 			beforeFunc: func(obj *bucketv1.Bucket, storage *Storage) error {
 				obj.Status.Artifact = &sourcev1.Artifact{
-					Path:     fmt.Sprintf("/reconcile-storage/hostname.txt"),
+					Path:     "/reconcile-storage/hostname.txt",
 					Revision: "f",
 					Digest:   "sha256:3b9c358f36f0a31b6ad3e14f309c7cf198ac9246e8316f9ce543d5b19ac02b80",
 					URL:      "http://outdated.com/reconcile-storage/hostname.txt",
@@ -1211,8 +1211,8 @@ func TestBucketReconciler_reconcileArtifact(t *testing.T) {
 				// path.
 				t.Expect(os.RemoveAll(dir)).ToNot(HaveOccurred())
 				f, err := os.Create(dir)
-				defer f.Close()
 				t.Expect(err).ToNot(HaveOccurred())
+				t.Expect(f.Close()).ToNot(HaveOccurred())
 				conditions.MarkReconciling(obj, meta.ProgressingReason, "foo")
 				conditions.MarkUnknown(obj, meta.ReadyCondition, "foo", "bar")
 			},
@@ -1293,6 +1293,7 @@ func TestBucketReconciler_statusConditions(t *testing.T) {
 		name             string
 		beforeFunc       func(obj *bucketv1.Bucket)
 		assertConditions []metav1.Condition
+		wantErr          bool
 	}{
 		{
 			name: "positive conditions only",
@@ -1317,6 +1318,7 @@ func TestBucketReconciler_statusConditions(t *testing.T) {
 				*conditions.TrueCondition(sourcev1.StorageOperationFailedCondition, sourcev1.DirCreationFailedReason, "failed to create directory"),
 				*conditions.TrueCondition(sourcev1.ArtifactOutdatedCondition, "NewRevision", "some error"),
 			},
+			wantErr: true,
 		},
 		{
 			name: "mixed positive and negative conditions",
@@ -1329,6 +1331,7 @@ func TestBucketReconciler_statusConditions(t *testing.T) {
 				*conditions.TrueCondition(sourcev1.FetchFailedCondition, sourcev1.AuthenticationFailedReason, "failed to get secret"),
 				*conditions.TrueCondition(sourcev1.ArtifactInStorageCondition, meta.SucceededReason, "stored artifact for revision"),
 			},
+			wantErr: true,
 		},
 	}
 
@@ -1360,21 +1363,18 @@ func TestBucketReconciler_statusConditions(t *testing.T) {
 			}
 
 			ctx := context.TODO()
-			recResult := sreconcile.ResultSuccess
-			var retErr error
-
 			summarizeHelper := summarize.NewHelper(record.NewFakeRecorder(32), serialPatcher)
 			summarizeOpts := []summarize.Option{
 				summarize.WithConditions(bucketReadyCondition),
-				summarize.WithReconcileResult(recResult),
+				summarize.WithReconcileResult(sreconcile.ResultSuccess),
-				summarize.WithReconcileError(retErr),
 				summarize.WithIgnoreNotFound(),
 				summarize.WithResultBuilder(sreconcile.AlwaysRequeueResultBuilder{
 					RequeueAfter: jitter.JitteredIntervalDuration(obj.GetRequeueAfter()),
 				}),
 				summarize.WithPatchFieldOwner("source-controller"),
 			}
-			_, retErr = summarizeHelper.SummarizeAndPatch(ctx, obj, summarizeOpts...)
+			_, err := summarizeHelper.SummarizeAndPatch(ctx, obj, summarizeOpts...)
+			g.Expect(err != nil).To(Equal(tt.wantErr))
 
 			key := client.ObjectKeyFromObject(obj)
 			g.Expect(c.Get(ctx, key, obj)).ToNot(HaveOccurred())

internal/controller/gitrepository_controller.go

@@ -33,7 +33,7 @@ import (
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/types"
 	kuberecorder "k8s.io/client-go/tools/record"
-	"k8s.io/utils/pointer"
+	"k8s.io/utils/ptr"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/builder"
 	"sigs.k8s.io/controller-runtime/pkg/client"
@@ -366,7 +366,7 @@ func (r *GitRepositoryReconciler) shouldNotify(oldObj, newObj *sourcev1.GitRepos
 	if resErr != nil && res == sreconcile.ResultEmpty && newObj.Status.Artifact != nil {
 		// Convert to Generic error and check for ignore.
 		if ge, ok := resErr.(*serror.Generic); ok {
-			return ge.Ignore == true
+			return ge.Ignore
 		}
 	}
 	return false
@@ -521,7 +521,7 @@ func (r *GitRepositoryReconciler) reconcileSource(ctx context.Context, sp *patch
 
 	// Observe if the artifacts still match the previous included ones
 	if artifacts.Diff(obj.Status.IncludedArtifacts) {
-		message := fmt.Sprintf("included artifacts differ from last observed includes")
+		message := "included artifacts differ from last observed includes"
 		if obj.Status.IncludedArtifacts != nil {
 			conditions.MarkTrue(obj, sourcev1.ArtifactOutdatedCondition, "IncludeChange", message)
 		}
@@ -1106,7 +1106,7 @@ func (r *GitRepositoryReconciler) eventLogf(ctx context.Context, obj runtime.Obj
 // changed and requires rebuilding the artifact. Rebuilding the artifact is also
 // required if the object needs to be (re)verified.
 func gitContentConfigChanged(obj *sourcev1.GitRepository, includes *artifactSet) bool {
-	if !pointer.StringEqual(obj.Spec.Ignore, obj.Status.ObservedIgnore) {
+	if !ptr.Equal(obj.Spec.Ignore, obj.Status.ObservedIgnore) {
 		return true
 	}
 	if obj.Spec.RecurseSubmodules != obj.Status.ObservedRecurseSubmodules {

internal/controller/gitrepository_controller_test.go

@@ -40,7 +40,7 @@ import (
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/client-go/tools/record"
-	"k8s.io/utils/pointer"
+	"k8s.io/utils/ptr"
 	kstatus "sigs.k8s.io/cli-utils/pkg/kstatus/status"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
@@ -827,7 +827,7 @@ func TestGitRepositoryReconciler_reconcileSource_checkoutStrategy(t *testing.T)
 			},
 			beforeFunc: func(obj *sourcev1.GitRepository, latestRev string) {
 				// Set new ignore value.
-				obj.Spec.Ignore = pointer.StringPtr("foo")
+				obj.Spec.Ignore = ptr.To("foo")
 				// Add existing artifact on the object and storage.
 				obj.Status = sourcev1.GitRepositoryStatus{
 					Artifact: &sourcev1.Artifact{
@@ -1001,7 +1001,7 @@ func TestGitRepositoryReconciler_reconcileArtifact(t *testing.T) {
 			dir:  "testdata/git/repository",
 			beforeFunc: func(obj *sourcev1.GitRepository) {
 				obj.Spec.Interval = metav1.Duration{Duration: interval}
-				obj.Spec.Ignore = pointer.StringPtr("!**.txt\n")
+				obj.Spec.Ignore = ptr.To("!**.txt\n")
 			},
 			afterFunc: func(t *WithT, obj *sourcev1.GitRepository) {
 				t.Expect(obj.GetArtifact()).ToNot(BeNil())
@@ -1849,6 +1849,41 @@ func TestGitRepositoryReconciler_verifySignature(t *testing.T) {
 				*conditions.FalseCondition(sourcev1.SourceVerifiedCondition, "InvalidCommitSignature", "signature verification of commit 'shasum' failed: unable to verify Git commit: unable to verify payload with any of the given key rings"),
 			},
 		},
+		{
+			name: "Invalid tag signature with mode=tag makes SourceVerifiedCondition=False",
+			secret: &corev1.Secret{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "existing",
+				},
+				Data: map[string][]byte{
+					"foo": []byte(armoredKeyRingFixture),
+				},
+			},
+			commit: git.Commit{
+				ReferencingTag: &git.Tag{
+					Name:      "v0.1.0",
+					Hash:      []byte("shasum"),
+					Encoded:   []byte(malformedEncodedTagFixture),
+					Signature: signatureTagFixture,
+				},
+			},
+			beforeFunc: func(obj *sourcev1.GitRepository) {
+				obj.Spec.Reference = &sourcev1.GitRepositoryRef{
+					Tag: "v0.1.0",
+				}
+				obj.Spec.Interval = metav1.Duration{Duration: interval}
+				obj.Spec.Verification = &sourcev1.GitRepositoryVerification{
+					Mode: sourcev1.ModeGitTag,
+					SecretRef: meta.LocalObjectReference{
+						Name: "existing",
+					},
+				}
+			},
+			wantErr: true,
+			assertConditions: []metav1.Condition{
+				*conditions.FalseCondition(sourcev1.SourceVerifiedCondition, "InvalidTagSignature", "signature verification of tag 'v0.1.0@shasum' failed: unable to verify Git tag: unable to verify payload with any of the given key rings"),
+			},
+		},
 		{
 			name: "Invalid PGP key makes SourceVerifiedCondition=False and returns error",
 			secret: &corev1.Secret{
@@ -2328,6 +2363,7 @@ func TestGitRepositoryReconciler_statusConditions(t *testing.T) {
 		name             string
 		beforeFunc       func(obj *sourcev1.GitRepository)
 		assertConditions []metav1.Condition
+		wantErr          bool
 	}{
 		{
 			name: "multiple positive conditions",
@@ -2356,6 +2392,7 @@ func TestGitRepositoryReconciler_statusConditions(t *testing.T) {
 				*conditions.TrueCondition(sourcev1.StorageOperationFailedCondition, sourcev1.DirCreationFailedReason, "failed to create directory"),
 				*conditions.TrueCondition(sourcev1.ArtifactOutdatedCondition, "NewRevision", "some error"),
 			},
+			wantErr: true,
 		},
 		{
 			name: "mixed positive and negative conditions",
@@ -2368,6 +2405,7 @@ func TestGitRepositoryReconciler_statusConditions(t *testing.T) {
 				*conditions.TrueCondition(sourcev1.FetchFailedCondition, sourcev1.AuthenticationFailedReason, "failed to get secret"),
 				*conditions.TrueCondition(sourcev1.ArtifactInStorageCondition, meta.SucceededReason, "stored artifact for revision"),
 			},
+			wantErr: true,
 		},
 	}
 
@@ -2400,22 +2438,19 @@ func TestGitRepositoryReconciler_statusConditions(t *testing.T) {
 			}
 
 			ctx := context.TODO()
-			recResult := sreconcile.ResultSuccess
-			var retErr error
-
 			summarizeHelper := summarize.NewHelper(record.NewFakeRecorder(32), serialPatcher)
 			summarizeOpts := []summarize.Option{
 				summarize.WithConditions(gitRepositoryReadyCondition),
 				summarize.WithBiPolarityConditionTypes(sourcev1.SourceVerifiedCondition),
-				summarize.WithReconcileResult(recResult),
+				summarize.WithReconcileResult(sreconcile.ResultSuccess),
-				summarize.WithReconcileError(retErr),
 				summarize.WithIgnoreNotFound(),
 				summarize.WithResultBuilder(sreconcile.AlwaysRequeueResultBuilder{
 					RequeueAfter: jitter.JitteredIntervalDuration(obj.GetRequeueAfter()),
 				}),
 				summarize.WithPatchFieldOwner("source-controller"),
 			}
-			_, retErr = summarizeHelper.SummarizeAndPatch(ctx, obj, summarizeOpts...)
+			_, err := summarizeHelper.SummarizeAndPatch(ctx, obj, summarizeOpts...)
+			g.Expect(err != nil).To(Equal(tt.wantErr))
 
 			key := client.ObjectKeyFromObject(obj)
 			g.Expect(c.Get(ctx, key, obj)).ToNot(HaveOccurred())
@@ -2833,15 +2868,15 @@ func TestGitContentConfigChanged(t *testing.T) {
 		{
 			name: "unobserved ignore",
 			obj: sourcev1.GitRepository{
-				Spec: sourcev1.GitRepositorySpec{Ignore: pointer.String("foo")},
+				Spec: sourcev1.GitRepositorySpec{Ignore: ptr.To("foo")},
 			},
 			want: true,
 		},
 		{
 			name: "observed ignore",
 			obj: sourcev1.GitRepository{
-				Spec:   sourcev1.GitRepositorySpec{Ignore: pointer.String("foo")},
+				Spec:   sourcev1.GitRepositorySpec{Ignore: ptr.To("foo")},
-				Status: sourcev1.GitRepositoryStatus{ObservedIgnore: pointer.String("foo")},
+				Status: sourcev1.GitRepositoryStatus{ObservedIgnore: ptr.To("foo")},
 			},
 			want: false,
 		},

internal/controller/helmchart_controller.go

@@ -1129,9 +1129,9 @@ func (r *HelmChartReconciler) requestsForHelmRepositoryChange(ctx context.Contex
 	}
 
 	var reqs []reconcile.Request
-	for _, i := range list.Items {
+	for i, v := range list.Items {
-		if i.Status.ObservedSourceArtifactRevision != repo.GetArtifact().Revision {
+		if v.Status.ObservedSourceArtifactRevision != repo.GetArtifact().Revision {
-			reqs = append(reqs, reconcile.Request{NamespacedName: client.ObjectKeyFromObject(&i)})
+			reqs = append(reqs, reconcile.Request{NamespacedName: client.ObjectKeyFromObject(&list.Items[i])})
 		}
 	}
 	return reqs
@@ -1159,9 +1159,9 @@ func (r *HelmChartReconciler) requestsForGitRepositoryChange(ctx context.Context
 	}
 
 	var reqs []reconcile.Request
-	for _, i := range list.Items {
+	for i, v := range list.Items {
-		if !repo.GetArtifact().HasRevision(i.Status.ObservedSourceArtifactRevision) {
+		if !repo.GetArtifact().HasRevision(v.Status.ObservedSourceArtifactRevision) {
-			reqs = append(reqs, reconcile.Request{NamespacedName: client.ObjectKeyFromObject(&i)})
+			reqs = append(reqs, reconcile.Request{NamespacedName: client.ObjectKeyFromObject(&list.Items[i])})
 		}
 	}
 	return reqs
@@ -1189,9 +1189,9 @@ func (r *HelmChartReconciler) requestsForBucketChange(ctx context.Context, o cli
 	}
 
 	var reqs []reconcile.Request
-	for _, i := range list.Items {
+	for i, v := range list.Items {
-		if !bucket.GetArtifact().HasRevision(i.Status.ObservedSourceArtifactRevision) {
+		if !bucket.GetArtifact().HasRevision(v.Status.ObservedSourceArtifactRevision) {
-			reqs = append(reqs, reconcile.Request{NamespacedName: client.ObjectKeyFromObject(&i)})
+			reqs = append(reqs, reconcile.Request{NamespacedName: client.ObjectKeyFromObject(&list.Items[i])})
 		}
 	}
 	return reqs

internal/controller/helmchart_controller_test.go

@@ -2029,6 +2029,7 @@ func TestHelmChartReconciler_statusConditions(t *testing.T) {
 		name             string
 		beforeFunc       func(obj *helmv1.HelmChart)
 		assertConditions []metav1.Condition
+		wantErr          bool
 	}{
 		{
 			name: "positive conditions only",
@@ -2055,6 +2056,7 @@ func TestHelmChartReconciler_statusConditions(t *testing.T) {
 				*conditions.TrueCondition(sourcev1.BuildFailedCondition, "ChartPackageError", "some error"),
 				*conditions.TrueCondition(sourcev1.ArtifactOutdatedCondition, "NewRevision", "some error"),
 			},
+			wantErr: true,
 		},
 		{
 			name: "mixed positive and negative conditions",
@@ -2067,6 +2069,7 @@ func TestHelmChartReconciler_statusConditions(t *testing.T) {
 				*conditions.TrueCondition(sourcev1.FetchFailedCondition, sourcev1.AuthenticationFailedReason, "failed to get secret"),
 				*conditions.TrueCondition(sourcev1.ArtifactInStorageCondition, meta.SucceededReason, "stored artifact for revision"),
 			},
+			wantErr: true,
 		},
 	}
 
@@ -2098,22 +2101,19 @@ func TestHelmChartReconciler_statusConditions(t *testing.T) {
 			}
 
 			ctx := context.TODO()
-			recResult := sreconcile.ResultSuccess
-			var retErr error
-
 			summarizeHelper := summarize.NewHelper(record.NewFakeRecorder(32), serialPatcher)
 			summarizeOpts := []summarize.Option{
 				summarize.WithConditions(helmChartReadyCondition),
 				summarize.WithBiPolarityConditionTypes(sourcev1.SourceVerifiedCondition),
-				summarize.WithReconcileResult(recResult),
+				summarize.WithReconcileResult(sreconcile.ResultSuccess),
-				summarize.WithReconcileError(retErr),
 				summarize.WithIgnoreNotFound(),
 				summarize.WithResultBuilder(sreconcile.AlwaysRequeueResultBuilder{
 					RequeueAfter: jitter.JitteredIntervalDuration(obj.GetRequeueAfter()),
 				}),
 				summarize.WithPatchFieldOwner("source-controller"),
 			}
-			_, retErr = summarizeHelper.SummarizeAndPatch(ctx, obj, summarizeOpts...)
+			_, err := summarizeHelper.SummarizeAndPatch(ctx, obj, summarizeOpts...)
+			g.Expect(err != nil).To(Equal(tt.wantErr))
 
 			key := client.ObjectKeyFromObject(obj)
 			g.Expect(c.Get(ctx, key, obj)).ToNot(HaveOccurred())

internal/controller/helmrepository_controller_test.go

@@ -1254,6 +1254,7 @@ func TestHelmRepositoryReconciler_statusConditions(t *testing.T) {
 		name             string
 		beforeFunc       func(obj *helmv1.HelmRepository)
 		assertConditions []metav1.Condition
+		wantErr          bool
 	}{
 		{
 			name: "positive conditions only",
@@ -1264,6 +1265,7 @@ func TestHelmRepositoryReconciler_statusConditions(t *testing.T) {
 				*conditions.TrueCondition(meta.ReadyCondition, meta.SucceededReason, "stored artifact for revision"),
 				*conditions.TrueCondition(sourcev1.ArtifactInStorageCondition, meta.SucceededReason, "stored artifact for revision"),
 			},
+			wantErr: false,
 		},
 		{
 			name: "multiple failures",
@@ -1278,6 +1280,7 @@ func TestHelmRepositoryReconciler_statusConditions(t *testing.T) {
 				*conditions.TrueCondition(sourcev1.StorageOperationFailedCondition, sourcev1.DirCreationFailedReason, "failed to create directory"),
 				*conditions.TrueCondition(sourcev1.ArtifactOutdatedCondition, "NewRevision", "some error"),
 			},
+			wantErr: true,
 		},
 		{
 			name: "mixed positive and negative conditions",
@@ -1290,6 +1293,7 @@ func TestHelmRepositoryReconciler_statusConditions(t *testing.T) {
 				*conditions.TrueCondition(sourcev1.FetchFailedCondition, sourcev1.AuthenticationFailedReason, "failed to get secret"),
 				*conditions.TrueCondition(sourcev1.ArtifactInStorageCondition, meta.SucceededReason, "stored artifact for revision"),
 			},
+			wantErr: true,
 		},
 	}
 
@@ -1321,19 +1325,16 @@ func TestHelmRepositoryReconciler_statusConditions(t *testing.T) {
 			}
 
 			ctx := context.TODO()
-			recResult := sreconcile.ResultSuccess
-			var retErr error
-
 			summarizeHelper := summarize.NewHelper(record.NewFakeRecorder(32), serialPatcher)
 			summarizeOpts := []summarize.Option{
 				summarize.WithConditions(helmRepositoryReadyCondition),
-				summarize.WithReconcileResult(recResult),
+				summarize.WithReconcileResult(sreconcile.ResultSuccess),
-				summarize.WithReconcileError(retErr),
 				summarize.WithIgnoreNotFound(),
 				summarize.WithResultBuilder(sreconcile.AlwaysRequeueResultBuilder{RequeueAfter: obj.GetRequeueAfter()}),
 				summarize.WithPatchFieldOwner("source-controller"),
 			}
-			_, retErr = summarizeHelper.SummarizeAndPatch(ctx, obj, summarizeOpts...)
+			_, err := summarizeHelper.SummarizeAndPatch(ctx, obj, summarizeOpts...)
+			g.Expect(err != nil).To(Equal(tt.wantErr))
 
 			key := client.ObjectKeyFromObject(obj)
 			g.Expect(c.Get(ctx, key, obj)).ToNot(HaveOccurred())

internal/controller/ocirepository_controller.go

@@ -34,14 +34,13 @@ import (
 	"github.com/google/go-containerregistry/pkg/authn/k8schain"
 	"github.com/google/go-containerregistry/pkg/name"
 	gcrv1 "github.com/google/go-containerregistry/pkg/v1"
-	v1 "github.com/google/go-containerregistry/pkg/v1"
 	"github.com/google/go-containerregistry/pkg/v1/remote"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/apimachinery/pkg/util/sets"
 	kuberecorder "k8s.io/client-go/tools/record"
-	"k8s.io/utils/pointer"
+	"k8s.io/utils/ptr"
 
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/builder"
@@ -578,13 +577,13 @@ func (r *OCIRepositoryReconciler) selectLayer(obj *ociv1.OCIRepository, image gc
 func (r *OCIRepositoryReconciler) getRevision(ref name.Reference, options []remote.Option) (string, error) {
 	switch ref := ref.(type) {
 	case name.Digest:
-		digest, err := v1.NewHash(ref.DigestStr())
+		digest, err := gcrv1.NewHash(ref.DigestStr())
 		if err != nil {
 			return "", err
 		}
 		return digest.String(), nil
 	case name.Tag:
-		var digest v1.Hash
+		var digest gcrv1.Hash
 
 		desc, err := remote.Head(ref, options...)
 		if err == nil {
@@ -1176,7 +1175,7 @@ type remoteOptions []remote.Option
 // of the artifact in the status to determine if artifact content configuration
 // has changed and requires rebuilding the artifact.
 func ociContentConfigChanged(obj *ociv1.OCIRepository) bool {
-	if !pointer.StringEqual(obj.Spec.Ignore, obj.Status.ObservedIgnore) {
+	if !ptr.Equal(obj.Spec.Ignore, obj.Status.ObservedIgnore) {
 		return true
 	}
 

internal/controller/ocirepository_controller_test.go

@@ -17,15 +17,10 @@ limitations under the License.
 package controller
 
 import (
-	"crypto/rand"
 	"crypto/tls"
-	cryptotls "crypto/tls"
 	"crypto/x509"
-	"crypto/x509/pkix"
-	"encoding/pem"
 	"errors"
 	"fmt"
-	"math/big"
 	"net/http"
 	"net/url"
 	"os"
@@ -48,7 +43,7 @@ import (
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/client-go/tools/record"
-	"k8s.io/utils/pointer"
+	"k8s.io/utils/ptr"
 	kstatus "sigs.k8s.io/cli-utils/pkg/kstatus/status"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
@@ -119,6 +114,7 @@ func TestOCIRepository_Reconcile(t *testing.T) {
 	})
 
 	podinfoVersions, err := pushMultiplePodinfoImages(regServer.registryHost, true, "6.1.4", "6.1.5", "6.1.6")
+	g.Expect(err).ToNot(HaveOccurred())
 
 	tests := []struct {
 		name           string
@@ -305,6 +301,7 @@ func TestOCIRepository_Reconcile_MediaType(t *testing.T) {
 	})
 
 	podinfoVersions, err := pushMultiplePodinfoImages(regServer.registryHost, true, "6.1.4", "6.1.5", "6.1.6")
+	g.Expect(err).ToNot(HaveOccurred())
 
 	tests := []struct {
 		name      string
@@ -828,7 +825,7 @@ func TestOCIRepository_reconcileSource_authStrategy(t *testing.T) {
 func makeTransport(insecure bool) http.RoundTripper {
 	transport := remote.DefaultTransport.(*http.Transport).Clone()
 	if insecure {
-		transport.TLSClientConfig = &cryptotls.Config{
+		transport.TLSClientConfig = &tls.Config{
 			InsecureSkipVerify: true,
 		}
 	}
@@ -997,6 +994,8 @@ func TestOCIRepository_reconcileSource_remoteReference(t *testing.T) {
 	})
 
 	podinfoVersions, err := pushMultiplePodinfoImages(server.registryHost, true, "6.1.4", "6.1.5", "6.1.6")
+	g.Expect(err).ToNot(HaveOccurred())
+
 	img6 := podinfoVersions["6.1.6"]
 	img5 := podinfoVersions["6.1.5"]
 
@@ -1479,7 +1478,7 @@ func TestOCIRepository_reconcileSource_noop(t *testing.T) {
 		{
 			name: "full reconcile - same rev, unobserved ignore",
 			beforeFunc: func(obj *ociv1.OCIRepository) {
-				obj.Status.ObservedIgnore = pointer.String("aaa")
+				obj.Status.ObservedIgnore = ptr.To("aaa")
 				obj.Status.Artifact = &sourcev1.Artifact{
 					Revision: testRevision,
 				}
@@ -1491,8 +1490,8 @@ func TestOCIRepository_reconcileSource_noop(t *testing.T) {
 		{
 			name: "noop - same rev, observed ignore",
 			beforeFunc: func(obj *ociv1.OCIRepository) {
-				obj.Spec.Ignore = pointer.String("aaa")
+				obj.Spec.Ignore = ptr.To("aaa")
-				obj.Status.ObservedIgnore = pointer.String("aaa")
+				obj.Status.ObservedIgnore = ptr.To("aaa")
 				obj.Status.Artifact = &sourcev1.Artifact{
 					Revision: testRevision,
 				}
@@ -1647,7 +1646,7 @@ func TestOCIRepository_reconcileArtifact(t *testing.T) {
 			targetPath: "testdata/oci/repository",
 			artifact:   &sourcev1.Artifact{Revision: "revision"},
 			beforeFunc: func(obj *ociv1.OCIRepository) {
-				obj.Spec.Ignore = pointer.String("foo.txt")
+				obj.Spec.Ignore = ptr.To("foo.txt")
 			},
 			want: sreconcile.ResultSuccess,
 			assertPaths: []string{
@@ -1687,7 +1686,7 @@ func TestOCIRepository_reconcileArtifact(t *testing.T) {
 			},
 			beforeFunc: func(obj *ociv1.OCIRepository) {
 				obj.Status.Artifact = &sourcev1.Artifact{Revision: "revision"}
-				obj.Spec.Ignore = pointer.String("aaa")
+				obj.Spec.Ignore = ptr.To("aaa")
 			},
 			want: sreconcile.ResultSuccess,
 			assertPaths: []string{
@@ -1754,10 +1753,10 @@ func TestOCIRepository_reconcileArtifact(t *testing.T) {
 				Revision: "revision",
 			},
 			beforeFunc: func(obj *ociv1.OCIRepository) {
-				obj.Spec.Ignore = pointer.String("aaa")
+				obj.Spec.Ignore = ptr.To("aaa")
 				obj.Spec.LayerSelector = &ociv1.OCILayerSelector{MediaType: "foo"}
 				obj.Status.Artifact = &sourcev1.Artifact{Revision: "revision"}
-				obj.Status.ObservedIgnore = pointer.String("aaa")
+				obj.Status.ObservedIgnore = ptr.To("aaa")
 				obj.Status.ObservedLayerSelector = &ociv1.OCILayerSelector{MediaType: "foo"}
 			},
 			want: sreconcile.ResultSuccess,
@@ -2491,45 +2490,6 @@ func setPodinfoImageAnnotations(img gcrv1.Image, tag string) gcrv1.Image {
 	return mutate.Annotations(img, metadata).(gcrv1.Image)
 }
 
-// These two taken verbatim from https://ericchiang.github.io/post/go-tls/
-func certTemplate() (*x509.Certificate, error) {
-	// generate a random serial number (a real cert authority would
-	// have some logic behind this)
-	serialNumberLimit := new(big.Int).Lsh(big.NewInt(1), 128)
-	serialNumber, err := rand.Int(rand.Reader, serialNumberLimit)
-	if err != nil {
-		return nil, errors.New("failed to generate serial number: " + err.Error())
-	}
-
-	tmpl := x509.Certificate{
-		SerialNumber:          serialNumber,
-		Subject:               pkix.Name{Organization: []string{"Flux project"}},
-		SignatureAlgorithm:    x509.SHA256WithRSA,
-		NotBefore:             time.Now(),
-		NotAfter:              time.Now().Add(time.Hour), // valid for an hour
-		BasicConstraintsValid: true,
-	}
-	return &tmpl, nil
-}
-
-func createCert(template, parent *x509.Certificate, pub interface{}, parentPriv interface{}) (
-	cert *x509.Certificate, certPEM []byte, err error) {
-
-	certDER, err := x509.CreateCertificate(rand.Reader, template, parent, pub, parentPriv)
-	if err != nil {
-		return
-	}
-	// parse the resulting certificate so we can use it again
-	cert, err = x509.ParseCertificate(certDER)
-	if err != nil {
-		return
-	}
-	// PEM encode the certificate (this is a standard TLS encoding)
-	b := pem.Block{Type: "CERTIFICATE", Bytes: certDER}
-	certPEM = pem.EncodeToMemory(&b)
-	return
-}
-
 func TestOCIContentConfigChanged(t *testing.T) {
 	tests := []struct {
 		name   string
@@ -2540,34 +2500,34 @@ func TestOCIContentConfigChanged(t *testing.T) {
 		{
 			name: "same ignore, no layer selector",
 			spec: ociv1.OCIRepositorySpec{
-				Ignore: pointer.String("nnn"),
+				Ignore: ptr.To("nnn"),
 			},
 			status: ociv1.OCIRepositoryStatus{
-				ObservedIgnore: pointer.String("nnn"),
+				ObservedIgnore: ptr.To("nnn"),
 			},
 			want: false,
 		},
 		{
 			name: "different ignore, no layer selector",
 			spec: ociv1.OCIRepositorySpec{
-				Ignore: pointer.String("nnn"),
+				Ignore: ptr.To("nnn"),
 			},
 			status: ociv1.OCIRepositoryStatus{
-				ObservedIgnore: pointer.String("mmm"),
+				ObservedIgnore: ptr.To("mmm"),
 			},
 			want: true,
 		},
 		{
 			name: "same ignore, same layer selector",
 			spec: ociv1.OCIRepositorySpec{
-				Ignore: pointer.String("nnn"),
+				Ignore: ptr.To("nnn"),
 				LayerSelector: &ociv1.OCILayerSelector{
 					MediaType: "foo",
 					Operation: ociv1.OCILayerExtract,
 				},
 			},
 			status: ociv1.OCIRepositoryStatus{
-				ObservedIgnore: pointer.String("nnn"),
+				ObservedIgnore: ptr.To("nnn"),
 				ObservedLayerSelector: &ociv1.OCILayerSelector{
 					MediaType: "foo",
 					Operation: ociv1.OCILayerExtract,
@@ -2578,14 +2538,14 @@ func TestOCIContentConfigChanged(t *testing.T) {
 		{
 			name: "same ignore, different layer selector operation",
 			spec: ociv1.OCIRepositorySpec{
-				Ignore: pointer.String("nnn"),
+				Ignore: ptr.To("nnn"),
 				LayerSelector: &ociv1.OCILayerSelector{
 					MediaType: "foo",
 					Operation: ociv1.OCILayerCopy,
 				},
 			},
 			status: ociv1.OCIRepositoryStatus{
-				ObservedIgnore: pointer.String("nnn"),
+				ObservedIgnore: ptr.To("nnn"),
 				ObservedLayerSelector: &ociv1.OCILayerSelector{
 					MediaType: "foo",
 					Operation: ociv1.OCILayerExtract,
@@ -2596,14 +2556,14 @@ func TestOCIContentConfigChanged(t *testing.T) {
 		{
 			name: "same ignore, different layer selector mediatype",
 			spec: ociv1.OCIRepositorySpec{
-				Ignore: pointer.String("nnn"),
+				Ignore: ptr.To("nnn"),
 				LayerSelector: &ociv1.OCILayerSelector{
 					MediaType: "bar",
 					Operation: ociv1.OCILayerExtract,
 				},
 			},
 			status: ociv1.OCIRepositoryStatus{
-				ObservedIgnore: pointer.String("nnn"),
+				ObservedIgnore: ptr.To("nnn"),
 				ObservedLayerSelector: &ociv1.OCILayerSelector{
 					MediaType: "foo",
 					Operation: ociv1.OCILayerExtract,

internal/controller/suite_test.go

@@ -23,7 +23,6 @@ import (
 	"crypto/x509"
 	"fmt"
 	"io"
-	"io/ioutil"
 	"log"
 	"math/rand"
 	"net"
@@ -117,10 +116,6 @@ var (
 	testCache          *cache.Cache
 )
 
-func init() {
-	rand.Seed(time.Now().UnixNano())
-}
-
 type registryClientTestServer struct {
 	out            io.Writer
 	registryHost   string
@@ -174,7 +169,7 @@ func setupRegistryServer(ctx context.Context, workspaceDir string, opts registry
 		server.registryHost = fmt.Sprintf("example.com:%d", port)
 		// Disable DNS server logging as it is extremely chatty.
 		dnsLog := log.Default()
-		dnsLog.SetOutput(ioutil.Discard)
+		dnsLog.SetOutput(io.Discard)
 		server.dnsServer, err = mockdns.NewServerWithLogger(map[string]mockdns.Zone{
 			"example.com.": {
 				A: []string{"127.0.0.1"},

internal/helm/chart/builder_test.go

@@ -17,8 +17,8 @@ limitations under the License.
 package chart
 
 import (
+	"crypto/rand"
 	"encoding/hex"
-	"math/rand"
 	"os"
 	"path/filepath"
 	"testing"

internal/helm/getter/client_opts.go

@@ -216,7 +216,7 @@ func storeTLSCertificateFiles(tlsBytes *stls.TLSBytes, path string) (string, str
 
 func writeToFile(data []byte, filename, tmpDir string) (string, error) {
 	file := path.Join(tmpDir, filename)
-	err := os.WriteFile(file, data, 0o644)
+	err := os.WriteFile(file, data, 0o600)
 	if err != nil {
 		return "", err
 	}

internal/helm/registry/auth.go

@@ -26,7 +26,6 @@ import (
 	"github.com/fluxcd/source-controller/internal/oci"
 	"github.com/google/go-containerregistry/pkg/authn"
 	"helm.sh/helm/v3/pkg/registry"
-	helmreg "helm.sh/helm/v3/pkg/registry"
 	corev1 "k8s.io/api/core/v1"
 )
 
@@ -143,7 +142,7 @@ func (r stringResource) RegistryStr() string {
 
 // NewLoginOption returns a registry login option for the given HelmRepository.
 // If the HelmRepository does not specify a secretRef, a nil login option is returned.
-func NewLoginOption(auth authn.Authenticator, keychain authn.Keychain, registryURL string) (helmreg.LoginOption, error) {
+func NewLoginOption(auth authn.Authenticator, keychain authn.Keychain, registryURL string) (registry.LoginOption, error) {
 	if auth != nil {
 		return AuthAdaptHelper(auth)
 	}

internal/mock/gcs/server.go

@@ -32,7 +32,7 @@ import (
 )
 
 var (
-	ObjectNotFound = errors.New("object not found")
+	ErrObjectNotFound = errors.New("object not found")
 )
 
 // Object is a mock Server object.
@@ -101,7 +101,7 @@ func (s *Server) getObjectFile(key string, generation int64) ([]byte, error) {
 			}
 		}
 	}
-	return nil, ObjectNotFound
+	return nil, ErrObjectNotFound
 }
 
 func (s *Server) handler(w http.ResponseWriter, r *http.Request) {

internal/reconcile/summarize/summary_test.go

@@ -373,10 +373,6 @@ func TestSummarizeAndPatch(t *testing.T) {
 				tt.afterFunc(g, obj)
 			}
 
-			if obj == nil {
-				t.Fail()
-			}
-
 			// Check if the object status is valid as per kstatus.
 			condns := &conditionscheck.Conditions{NegativePolarity: testReadyConditions.NegativePolarity}
 			checker := conditionscheck.NewChecker(c, condns)

main.go

@@ -31,7 +31,7 @@ import (
 	clientgoscheme "k8s.io/client-go/kubernetes/scheme"
 	_ "k8s.io/client-go/plugin/pkg/client/auth/gcp"
 	"k8s.io/client-go/tools/record"
-	"k8s.io/utils/pointer"
+	"k8s.io/utils/ptr"
 	ctrl "sigs.k8s.io/controller-runtime"
 	ctrlcache "sigs.k8s.io/controller-runtime/pkg/cache"
 	ctrlclient "sigs.k8s.io/controller-runtime/pkg/client"
@@ -371,7 +371,7 @@ func mustSetupManager(metricsAddr, healthAddr string, maxConcurrent int,
 			Namespaces: []string{watchNamespace},
 		},
 		Controller: ctrlcfg.Controller{
-			RecoverPanic:            pointer.Bool(true),
+			RecoverPanic:            ptr.To(true),
 			MaxConcurrentReconciles: maxConcurrent,
 		},
 	})

pkg/azure/blob.go

@@ -31,7 +31,6 @@ import (
 	"github.com/Azure/azure-sdk-for-go/sdk/azcore/cloud"
 	"github.com/Azure/azure-sdk-for-go/sdk/azcore/to"
 	"github.com/Azure/azure-sdk-for-go/sdk/azidentity"
-	_ "github.com/Azure/azure-sdk-for-go/sdk/azidentity"
 	"github.com/Azure/azure-sdk-for-go/sdk/storage/azblob"
 	"github.com/Azure/azure-sdk-for-go/sdk/storage/azblob/bloberror"
 	corev1 "k8s.io/api/core/v1"
@@ -193,7 +192,7 @@ func (c *BlobClient) BucketExists(ctx context.Context, bucketName string) (bool,
 
 		// For a container-level SASToken, we get an AuthenticationFailed when the bucket doesn't exist
 		if bloberror.HasCode(err, bloberror.AuthenticationFailed) {
-			return false, fmt.Errorf("Bucket name may be incorrect, it does not exist or caller does not have enough permissions: %w", err)
+			return false, fmt.Errorf("the specified bucket name may be incorrect, nonexistent, or the caller might lack sufficient permissions to access it: %w", err)
 		}
 
 		return false, err
@@ -286,9 +285,7 @@ func (c *BlobClient) VisitObjects(ctx context.Context, bucketName string, visit
 }
 
 // Close has no effect on BlobClient.
-func (c *BlobClient) Close(_ context.Context) {
+func (c *BlobClient) Close(_ context.Context) {}
-	return
-}
 
 // ObjectIsNotFound checks if the error provided is an azblob.StorageError with
 // an azblob.StorageErrorCodeBlobNotFound error code.

pkg/azure/blob_integration_test.go

@@ -21,11 +21,11 @@ package azure
 import (
 	"context"
 	"crypto/md5"
+	"crypto/rand"
 	"encoding/hex"
 	"errors"
 	"fmt"
 	"log"
-	"math/rand"
 	"os"
 	"path/filepath"
 	"strings"
@@ -81,10 +81,6 @@ test: file2
 	}
 )
 
-func init() {
-	rand.Seed(time.Now().UnixNano())
-}
-
 func TestMain(m *testing.M) {
 	var err error
 	cred, err = blob.NewSharedKeyCredential(testAccountName, testAccountKey)

pkg/azure/blob_test.go

@@ -364,16 +364,15 @@ func Test_sasTokenFromSecret(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			g := NewWithT(t)
 
-			_, err := url.ParseQuery("")
 			got, err := sasTokenFromSecret(tt.endpoint, tt.secret)
 			g.Expect(err != nil).To(Equal(tt.wantErr))
 			if tt.want != "" {
-				ttVaules, err := url.Parse(tt.want)
+				ttValues, err := url.Parse(tt.want)
 				g.Expect(err).To(BeNil())
 
 				gotValues, err := url.Parse(got)
 				g.Expect(err).To(BeNil())
-				g.Expect(gotValues.Query()).To(Equal(ttVaules.Query()))
+				g.Expect(gotValues.Query()).To(Equal(ttValues.Query()))
 				return
 			}
 			g.Expect(got).To(Equal(""))