From 25673ac5125d62c480098eb778e31fefd83ae74d Mon Sep 17 00:00:00 2001
From: Soule BA <soule@weave.works>
Date: Wed, 19 Oct 2022 01:30:47 +0200
Subject: [PATCH] addressing review comments

Signed-off-by: Soule BA <soule@weave.works>
---
 controllers/helmchart_controller.go          |  3 ++-
 internal/helm/registry/auth.go               | 10 ++++++----
 internal/helm/repository/chart_repository.go |  2 +-
 internal/oci/verifier.go                     |  2 +-
 4 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/controllers/helmchart_controller.go b/controllers/helmchart_controller.go
index 773bce46..d74a7ae1 100644
--- a/controllers/helmchart_controller.go
+++ b/controllers/helmchart_controller.go
@@ -214,6 +214,7 @@ func (r *HelmChartReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
 		summarizeHelper := summarize.NewHelper(r.EventRecorder, patchHelper)
 		summarizeOpts := []summarize.Option{
 			summarize.WithConditions(helmChartReadyCondition),
+			summarize.WithBiPolarityConditionTypes(sourcev1.SourceVerifiedCondition),
 			summarize.WithReconcileResult(recResult),
 			summarize.WithReconcileError(retErr),
 			summarize.WithIgnoreNotFound(),
@@ -648,7 +649,7 @@ func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj *
 		ValuesFiles: obj.GetValuesFiles(),
 		Force:       obj.Generation != obj.Status.ObservedGeneration,
 		// The remote builder will not attempt to download the chart if
-		// an artifact exist with the same name and version and the force is false.
+		// an artifact exists with the same name and version and `Force` is false.
 		// It will try to verify the chart if:
 		// - we are on the first reconciliation
 		// - the HelmChart spec has changed (generation drift)
diff --git a/internal/helm/registry/auth.go b/internal/helm/registry/auth.go
index 4914c568..debe87ea 100644
--- a/internal/helm/registry/auth.go
+++ b/internal/helm/registry/auth.go
@@ -95,7 +95,7 @@ func KeychainAdaptHelper(keyChain authn.Keychain) func(string) (registry.LoginOp
 		if err != nil {
 			return nil, fmt.Errorf("unable to parse registry URL '%s'", registryURL)
 		}
-		authenticator, err := keyChain.Resolve(resource{parsedURL.Host})
+		authenticator, err := keyChain.Resolve(stringResource{parsedURL.Host})
 		if err != nil {
 			return nil, fmt.Errorf("unable to resolve credentials for registry '%s': %w", registryURL, err)
 		}
@@ -126,14 +126,16 @@ func AuthAdaptHelper(auth authn.Authenticator) (registry.LoginOption, error) {
 	return registry.LoginOptBasicAuth(username, password), nil
 }
 
-type resource struct {
+// stringResource is there to satisfy the github.com/google/go-containerregistry/pkg/authn.Resource interface.
+// It merely wraps a given string and returns it for all of the interface's methods.
+type stringResource struct {
 	registry string
 }
 
-func (r resource) String() string {
+func (r stringResource) String() string {
 	return r.registry
 }
 
-func (r resource) RegistryStr() string {
+func (r stringResource) RegistryStr() string {
 	return r.registry
 }
diff --git a/internal/helm/repository/chart_repository.go b/internal/helm/repository/chart_repository.go
index 3997d5f3..15e62432 100644
--- a/internal/helm/repository/chart_repository.go
+++ b/internal/helm/repository/chart_repository.go
@@ -528,5 +528,5 @@ func (r *ChartRepository) RemoveCache() error {
 func (r *ChartRepository) VerifyChart(_ context.Context, _ *repo.ChartVersion) error {
 	// no-op
 	// this is a no-op because this is not implemented yet.
-	return nil
+	return fmt.Errorf("not implemented")
 }
diff --git a/internal/oci/verifier.go b/internal/oci/verifier.go
index 490b3ef4..23f8f090 100644
--- a/internal/oci/verifier.go
+++ b/internal/oci/verifier.go
@@ -45,7 +45,7 @@ type options struct {
 	ROpt      []remote.Option
 }
 
-// Options is a function that configures the options applied to a CosignVerifier.
+// Options is a function that configures the options applied to a Verifier.
 type Options func(opts *options)
 
 // WithPublicKey sets the public key.