From 1070d1287aa053d5a6354358e9908ad98c54b3f1 Mon Sep 17 00:00:00 2001 From: Max Jonas Werner Date: Fri, 20 May 2022 21:14:34 +0200 Subject: [PATCH 01/10] fix nil pointer dereference When the Secret referenced in an OCI HelmRepository doesn't contain a username and password, the controller doesn't panic, anymore. Signed-off-by: Max Jonas Werner --- controllers/helmrepository_controller_oci.go | 4 +- .../helmrepository_controller_oci_test.go | 197 ++++++++++-------- 2 files changed, 110 insertions(+), 91 deletions(-) diff --git a/controllers/helmrepository_controller_oci.go b/controllers/helmrepository_controller_oci.go index 05da9af0c..676cee43c 100644 --- a/controllers/helmrepository_controller_oci.go +++ b/controllers/helmrepository_controller_oci.go @@ -284,7 +284,9 @@ func (r *HelmRepositoryOCIReconciler) reconcileSource(ctx context.Context, obj * return sreconcile.ResultEmpty, e } - logOpts = append(logOpts, logOpt) + if logOpt != nil { + logOpts = append(logOpts, logOpt) + } } if result, err := r.validateSource(ctx, obj, logOpts...); err != nil || result == sreconcile.ResultEmpty { diff --git a/controllers/helmrepository_controller_oci_test.go b/controllers/helmrepository_controller_oci_test.go index 6069fe8ca..068dce55a 100644 --- a/controllers/helmrepository_controller_oci_test.go +++ b/controllers/helmrepository_controller_oci_test.go @@ -34,99 +34,116 @@ import ( ) func TestHelmRepositoryOCIReconciler_Reconcile(t *testing.T) { - g := NewWithT(t) - - ns, err := testEnv.CreateNamespace(ctx, "helmrepository-oci-reconcile-test") - g.Expect(err).ToNot(HaveOccurred()) - defer func() { g.Expect(testEnv.Delete(ctx, ns)).To(Succeed()) }() - - secret := &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - GenerateName: "helmrepository-", - Namespace: ns.Name, + tests := []struct { + name string + secretData map[string][]byte + }{ + { + name: "valid auth data", + secretData: map[string][]byte{ + "username": []byte(testUsername), + "password": []byte(testPassword), + }, }, - Data: map[string][]byte{ - "username": []byte(testUsername), - "password": []byte(testPassword), + { + name: "no auth data", + secretData: nil, }, } - g.Expect(testEnv.CreateAndWait(ctx, secret)).To(Succeed()) - - obj := &sourcev1.HelmRepository{ - ObjectMeta: metav1.ObjectMeta{ - GenerateName: "helmrepository-oci-reconcile-", - Namespace: ns.Name, - }, - Spec: sourcev1.HelmRepositorySpec{ - Interval: metav1.Duration{Duration: interval}, - URL: fmt.Sprintf("oci://%s", testRegistryserver.DockerRegistryHost), - SecretRef: &meta.LocalObjectReference{ - Name: secret.Name, - }, - Type: sourcev1.HelmRepositoryTypeOCI, - }, + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + + ns, err := testEnv.CreateNamespace(ctx, "helmrepository-oci-reconcile-test") + g.Expect(err).ToNot(HaveOccurred()) + defer func() { g.Expect(testEnv.Delete(ctx, ns)).To(Succeed()) }() + + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + GenerateName: "helmrepository-", + Namespace: ns.Name, + }, + Data: tt.secretData, + } + + g.Expect(testEnv.CreateAndWait(ctx, secret)).To(Succeed()) + + obj := &sourcev1.HelmRepository{ + ObjectMeta: metav1.ObjectMeta{ + GenerateName: "helmrepository-oci-reconcile-", + Namespace: ns.Name, + }, + Spec: sourcev1.HelmRepositorySpec{ + Interval: metav1.Duration{Duration: interval}, + URL: fmt.Sprintf("oci://%s", testRegistryserver.DockerRegistryHost), + SecretRef: &meta.LocalObjectReference{ + Name: secret.Name, + }, + Type: sourcev1.HelmRepositoryTypeOCI, + }, + } + g.Expect(testEnv.Create(ctx, obj)).To(Succeed()) + + key := client.ObjectKey{Name: obj.Name, Namespace: obj.Namespace} + + // Wait for finalizer to be set + g.Eventually(func() bool { + if err := testEnv.Get(ctx, key, obj); err != nil { + return false + } + return len(obj.Finalizers) > 0 + }, timeout).Should(BeTrue()) + + // Wait for HelmRepository to be Ready + g.Eventually(func() bool { + if err := testEnv.Get(ctx, key, obj); err != nil { + return false + } + if !conditions.IsReady(obj) { + return false + } + readyCondition := conditions.Get(obj, meta.ReadyCondition) + return obj.Generation == readyCondition.ObservedGeneration && + obj.Generation == obj.Status.ObservedGeneration + }, timeout).Should(BeTrue()) + + // Check if the object status is valid. + condns := &status.Conditions{NegativePolarity: helmRepositoryReadyCondition.NegativePolarity} + checker := status.NewChecker(testEnv.Client, condns) + checker.CheckErr(ctx, obj) + + // kstatus client conformance check. + u, err := patch.ToUnstructured(obj) + g.Expect(err).ToNot(HaveOccurred()) + res, err := kstatus.Compute(u) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(res.Status).To(Equal(kstatus.CurrentStatus)) + + // Patch the object with reconcile request annotation. + patchHelper, err := patch.NewHelper(obj, testEnv.Client) + g.Expect(err).ToNot(HaveOccurred()) + annotations := map[string]string{ + meta.ReconcileRequestAnnotation: "now", + } + obj.SetAnnotations(annotations) + g.Expect(patchHelper.Patch(ctx, obj)).ToNot(HaveOccurred()) + g.Eventually(func() bool { + if err := testEnv.Get(ctx, key, obj); err != nil { + return false + } + return obj.Status.LastHandledReconcileAt == "now" + }, timeout).Should(BeTrue()) + + g.Expect(testEnv.Delete(ctx, obj)).To(Succeed()) + + // Wait for HelmRepository to be deleted + g.Eventually(func() bool { + if err := testEnv.Get(ctx, key, obj); err != nil { + return apierrors.IsNotFound(err) + } + return false + }, timeout).Should(BeTrue()) + }) } - g.Expect(testEnv.Create(ctx, obj)).To(Succeed()) - - key := client.ObjectKey{Name: obj.Name, Namespace: obj.Namespace} - - // Wait for finalizer to be set - g.Eventually(func() bool { - if err := testEnv.Get(ctx, key, obj); err != nil { - return false - } - return len(obj.Finalizers) > 0 - }, timeout).Should(BeTrue()) - - // Wait for HelmRepository to be Ready - g.Eventually(func() bool { - if err := testEnv.Get(ctx, key, obj); err != nil { - return false - } - if !conditions.IsReady(obj) { - return false - } - readyCondition := conditions.Get(obj, meta.ReadyCondition) - return obj.Generation == readyCondition.ObservedGeneration && - obj.Generation == obj.Status.ObservedGeneration - }, timeout).Should(BeTrue()) - - // Check if the object status is valid. - condns := &status.Conditions{NegativePolarity: helmRepositoryReadyCondition.NegativePolarity} - checker := status.NewChecker(testEnv.Client, condns) - checker.CheckErr(ctx, obj) - - // kstatus client conformance check. - u, err := patch.ToUnstructured(obj) - g.Expect(err).ToNot(HaveOccurred()) - res, err := kstatus.Compute(u) - g.Expect(err).ToNot(HaveOccurred()) - g.Expect(res.Status).To(Equal(kstatus.CurrentStatus)) - - // Patch the object with reconcile request annotation. - patchHelper, err := patch.NewHelper(obj, testEnv.Client) - g.Expect(err).ToNot(HaveOccurred()) - annotations := map[string]string{ - meta.ReconcileRequestAnnotation: "now", - } - obj.SetAnnotations(annotations) - g.Expect(patchHelper.Patch(ctx, obj)).ToNot(HaveOccurred()) - g.Eventually(func() bool { - if err := testEnv.Get(ctx, key, obj); err != nil { - return false - } - return obj.Status.LastHandledReconcileAt == "now" - }, timeout).Should(BeTrue()) - - g.Expect(testEnv.Delete(ctx, obj)).To(Succeed()) - - // Wait for HelmRepository to be deleted - g.Eventually(func() bool { - if err := testEnv.Get(ctx, key, obj); err != nil { - return apierrors.IsNotFound(err) - } - return false - }, timeout).Should(BeTrue()) - } From bb4d886ba26ec9bba77fed0e33ddd1e8776ca04e Mon Sep 17 00:00:00 2001 From: Max Jonas Werner Date: Fri, 20 May 2022 20:59:08 +0200 Subject: [PATCH 02/10] dockerconfigjson for OCI registry authentication `loginOptionFromSecret` now derives username/password from a docker config stored in Secrets of type "kubernetes.io/dockerconfigjson". Signed-off-by: Max Jonas Werner --- controllers/helmchart_controller.go | 2 +- controllers/helmchart_controller_test.go | 30 ++++++++++++++++++ controllers/helmrepository_controller_oci.go | 31 +++++++++++++++++-- .../helmrepository_controller_oci_test.go | 14 +++++++++ 4 files changed, 73 insertions(+), 4 deletions(-) diff --git a/controllers/helmchart_controller.go b/controllers/helmchart_controller.go index 8afb96c77..1a1092bb4 100644 --- a/controllers/helmchart_controller.go +++ b/controllers/helmchart_controller.go @@ -492,7 +492,7 @@ func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj * } // Build registryClient options from secret - logOpt, err := loginOptionFromSecret(*secret) + logOpt, err := loginOptionFromSecret(repo.Spec.URL, *secret) if err != nil { e := &serror.Event{ Err: fmt.Errorf("failed to configure Helm client with secret data: %w", err), diff --git a/controllers/helmchart_controller_test.go b/controllers/helmchart_controller_test.go index 9bc5a39e1..9796ea6e7 100644 --- a/controllers/helmchart_controller_test.go +++ b/controllers/helmchart_controller_test.go @@ -19,6 +19,7 @@ package controllers import ( "bytes" "context" + "encoding/base64" "errors" "fmt" "io" @@ -825,6 +826,35 @@ func TestHelmChartReconciler_buildFromOCIHelmRepository(t *testing.T) { assertFunc func(g *WithT, obj *sourcev1.HelmChart, build chart.Build) cleanFunc func(g *WithT, build *chart.Build) }{ + { + name: "Reconciles chart build with docker repository credentials", + secret: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "auth", + }, + Type: corev1.SecretTypeDockerConfigJson, + Data: map[string][]byte{ + ".dockerconfigjson": []byte(`{"auths":{"` + + testRegistryserver.DockerRegistryHost + `":{"` + + `auth":"` + base64.StdEncoding.EncodeToString([]byte(testUsername+":"+testPassword)) + `"}}}`), + }, + }, + beforeFunc: func(obj *sourcev1.HelmChart, repository *sourcev1.HelmRepository) { + obj.Spec.Chart = metadata.Name + obj.Spec.Version = metadata.Version + repository.Spec.SecretRef = &meta.LocalObjectReference{Name: "auth"} + }, + want: sreconcile.ResultSuccess, + assertFunc: func(g *WithT, _ *sourcev1.HelmChart, build chart.Build) { + g.Expect(build.Name).To(Equal(metadata.Name)) + g.Expect(build.Version).To(Equal(metadata.Version)) + g.Expect(build.Path).ToNot(BeEmpty()) + g.Expect(build.Path).To(BeARegularFile()) + }, + cleanFunc: func(g *WithT, build *chart.Build) { + g.Expect(os.Remove(build.Path)).To(Succeed()) + }, + }, { name: "Reconciles chart build with repository credentials", secret: &corev1.Secret{ diff --git a/controllers/helmrepository_controller_oci.go b/controllers/helmrepository_controller_oci.go index 676cee43c..6cd516136 100644 --- a/controllers/helmrepository_controller_oci.go +++ b/controllers/helmrepository_controller_oci.go @@ -17,12 +17,15 @@ limitations under the License. package controllers import ( + "bytes" "context" "fmt" + "net/url" "os" "strings" "time" + "github.com/docker/cli/cli/config" "github.com/fluxcd/pkg/apis/meta" "github.com/fluxcd/pkg/runtime/conditions" helper "github.com/fluxcd/pkg/runtime/controller" @@ -273,7 +276,7 @@ func (r *HelmRepositoryOCIReconciler) reconcileSource(ctx context.Context, obj * } // Construct actual options - logOpt, err := loginOptionFromSecret(secret) + logOpt, err := loginOptionFromSecret(obj.Spec.URL, secret) if err != nil { e := &serror.Event{ Err: fmt.Errorf("failed to configure Helm client with secret data: %w", err), @@ -352,8 +355,30 @@ func (r *HelmRepositoryOCIReconciler) validateSource(ctx context.Context, obj *s return sreconcile.ResultSuccess, nil } -func loginOptionFromSecret(secret corev1.Secret) (registry.LoginOption, error) { - username, password := string(secret.Data["username"]), string(secret.Data["password"]) +// loginOptionFromSecret derives authentication data from a Secret to login to an OCI registry. This Secret +// may either hold "username" and "password" fields or be of the corev1.SecretTypeDockerConfigJson type and hold +// a corev1.DockerConfigJsonKey field with a complete Docker configuration. If both, "username" and "password" are +// empty, a nil LoginOption and a nil error will be returned. +func loginOptionFromSecret(registryURL string, secret corev1.Secret) (registry.LoginOption, error) { + var username, password string + if secret.Type == corev1.SecretTypeDockerConfigJson { + dockerCfg, err := config.LoadFromReader(bytes.NewReader(secret.Data[corev1.DockerConfigJsonKey])) + if err != nil { + return nil, fmt.Errorf("unable to load Docker config: %w", err) + } + parsedURL, err := url.Parse(registryURL) + if err != nil { + return nil, fmt.Errorf("unable to parse registry URL: %w", err) + } + authConfig, err := dockerCfg.GetAuthConfig(parsedURL.Host) + if err != nil { + return nil, fmt.Errorf("unable to get authentication data from Secret: %w", err) + } + username = authConfig.Username + password = authConfig.Password + } else { + username, password = string(secret.Data["username"]), string(secret.Data["password"]) + } switch { case username == "" && password == "": return nil, nil diff --git a/controllers/helmrepository_controller_oci_test.go b/controllers/helmrepository_controller_oci_test.go index 068dce55a..21a221ef2 100644 --- a/controllers/helmrepository_controller_oci_test.go +++ b/controllers/helmrepository_controller_oci_test.go @@ -17,6 +17,7 @@ limitations under the License. package controllers import ( + "encoding/base64" "fmt" "testing" @@ -36,6 +37,7 @@ import ( func TestHelmRepositoryOCIReconciler_Reconcile(t *testing.T) { tests := []struct { name string + secretType corev1.SecretType secretData map[string][]byte }{ { @@ -49,6 +51,15 @@ func TestHelmRepositoryOCIReconciler_Reconcile(t *testing.T) { name: "no auth data", secretData: nil, }, + { + name: "dockerconfigjson Secret", + secretType: corev1.SecretTypeDockerConfigJson, + secretData: map[string][]byte{ + ".dockerconfigjson": []byte(`{"auths":{"` + + testRegistryserver.DockerRegistryHost + `":{"` + + `auth":"` + base64.StdEncoding.EncodeToString([]byte(testUsername+":"+testPassword)) + `"}}}`), + }, + }, } for _, tt := range tests { @@ -66,6 +77,9 @@ func TestHelmRepositoryOCIReconciler_Reconcile(t *testing.T) { }, Data: tt.secretData, } + if tt.secretType != "" { + secret.Type = tt.secretType + } g.Expect(testEnv.CreateAndWait(ctx, secret)).To(Succeed()) From ce072c7eda080ba0616c448c866c94e43cc187f1 Mon Sep 17 00:00:00 2001 From: Max Jonas Werner Date: Sun, 22 May 2022 20:10:03 +0200 Subject: [PATCH 03/10] better variable names; improved logging When setup of one of the two controller reconciling HelmRepositories fails, it's now possible to judge from the log which setup call failed by regarding the "type" log field. Signed-off-by: Max Jonas Werner --- controllers/helmchart_controller.go | 13 ++++++------- controllers/helmrepository_controller_oci.go | 10 +++++----- main.go | 4 ++-- 3 files changed, 13 insertions(+), 14 deletions(-) diff --git a/controllers/helmchart_controller.go b/controllers/helmchart_controller.go index 1a1092bb4..2fb9e70a1 100644 --- a/controllers/helmchart_controller.go +++ b/controllers/helmchart_controller.go @@ -447,7 +447,7 @@ func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj * repo *sourcev1.HelmRepository, b *chart.Build) (sreconcile.Result, error) { var ( tlsConfig *tls.Config - logOpts []registry.LoginOption + loginOpts []registry.LoginOption ) // Construct the Getter options from the HelmRepository data @@ -492,7 +492,7 @@ func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj * } // Build registryClient options from secret - logOpt, err := loginOptionFromSecret(repo.Spec.URL, *secret) + loginOpt, err := loginOptionFromSecret(repo.Spec.URL, *secret) if err != nil { e := &serror.Event{ Err: fmt.Errorf("failed to configure Helm client with secret data: %w", err), @@ -503,7 +503,7 @@ func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj * return sreconcile.ResultEmpty, e } - logOpts = append([]registry.LoginOption{}, logOpt) + loginOpts = append([]registry.LoginOption{}, loginOpt) } // Initialize the chart repository @@ -519,7 +519,7 @@ func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj * // this is needed because otherwise the credentials are stored in ~/.docker/config.json. // TODO@souleb: remove this once the registry move to Oras v2 // or rework to enable reusing credentials to avoid the unneccessary handshake operations - registryClient, file, err := r.RegistryClientGenerator(logOpts != nil) + registryClient, file, err := r.RegistryClientGenerator(loginOpts != nil) if err != nil { return chartRepoErrorReturn(err, obj) } @@ -540,14 +540,13 @@ func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj * // If login options are configured, use them to login to the registry // The OCIGetter will later retrieve the stored credentials to pull the chart - if logOpts != nil { - err = ociChartRepo.Login(logOpts...) + if loginOpts != nil { + err = ociChartRepo.Login(loginOpts...) if err != nil { return chartRepoErrorReturn(err, obj) } } default: - var httpChartRepo *repository.ChartRepository httpChartRepo, err := repository.NewChartRepository(repo.Spec.URL, r.Storage.LocalPath(*repo.GetArtifact()), r.Getters, tlsConfig, clientOpts, repository.WithMemoryCache(r.Storage.LocalPath(*repo.GetArtifact()), r.Cache, r.TTL, func(event string) { r.IncCacheEvents(event, obj.Name, obj.Namespace) diff --git a/controllers/helmrepository_controller_oci.go b/controllers/helmrepository_controller_oci.go index 6cd516136..7702e446d 100644 --- a/controllers/helmrepository_controller_oci.go +++ b/controllers/helmrepository_controller_oci.go @@ -257,7 +257,7 @@ func (r *HelmRepositoryOCIReconciler) reconcile(ctx context.Context, obj *source } func (r *HelmRepositoryOCIReconciler) reconcileSource(ctx context.Context, obj *sourcev1.HelmRepository) (sreconcile.Result, error) { - var logOpts []registry.LoginOption + var loginOpts []registry.LoginOption // Configure any authentication related options if obj.Spec.SecretRef != nil { // Attempt to retrieve secret @@ -276,7 +276,7 @@ func (r *HelmRepositoryOCIReconciler) reconcileSource(ctx context.Context, obj * } // Construct actual options - logOpt, err := loginOptionFromSecret(obj.Spec.URL, secret) + loginOpt, err := loginOptionFromSecret(obj.Spec.URL, secret) if err != nil { e := &serror.Event{ Err: fmt.Errorf("failed to configure Helm client with secret data: %w", err), @@ -287,12 +287,12 @@ func (r *HelmRepositoryOCIReconciler) reconcileSource(ctx context.Context, obj * return sreconcile.ResultEmpty, e } - if logOpt != nil { - logOpts = append(logOpts, logOpt) + if loginOpt != nil { + loginOpts = append(loginOpts, loginOpt) } } - if result, err := r.validateSource(ctx, obj, logOpts...); err != nil || result == sreconcile.ResultEmpty { + if result, err := r.validateSource(ctx, obj, loginOpts...); err != nil || result == sreconcile.ResultEmpty { return result, err } diff --git a/main.go b/main.go index 88f4ad2d0..5088d599f 100644 --- a/main.go +++ b/main.go @@ -229,7 +229,7 @@ func main() { MaxConcurrentReconciles: concurrent, RateLimiter: helper.GetRateLimiter(rateLimiterOptions), }); err != nil { - setupLog.Error(err, "unable to create controller", "controller", sourcev1.HelmRepositoryKind) + setupLog.Error(err, "unable to create controller", "controller", sourcev1.HelmRepositoryKind, "type", "default") os.Exit(1) } @@ -244,7 +244,7 @@ func main() { MaxConcurrentReconciles: concurrent, RateLimiter: helper.GetRateLimiter(rateLimiterOptions), }); err != nil { - setupLog.Error(err, "unable to create controller", "controller", sourcev1.HelmRepositoryKind) + setupLog.Error(err, "unable to create controller", "controller", sourcev1.HelmRepositoryKind, "type", "OCI") os.Exit(1) } From d5e3c37833d40e650ecfc5aeea2765edac821697 Mon Sep 17 00:00:00 2001 From: Max Jonas Werner Date: Mon, 23 May 2022 08:50:27 +0200 Subject: [PATCH 04/10] fix code formatting Signed-off-by: Max Jonas Werner --- controllers/helmchart_controller.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/controllers/helmchart_controller.go b/controllers/helmchart_controller.go index 2fb9e70a1..80bb773fc 100644 --- a/controllers/helmchart_controller.go +++ b/controllers/helmchart_controller.go @@ -447,7 +447,7 @@ func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj * repo *sourcev1.HelmRepository, b *chart.Build) (sreconcile.Result, error) { var ( tlsConfig *tls.Config - loginOpts []registry.LoginOption + loginOpts []registry.LoginOption ) // Construct the Getter options from the HelmRepository data From ace21c56660cb44e51343f486f419a5723894ee0 Mon Sep 17 00:00:00 2001 From: Max Jonas Werner Date: Mon, 23 May 2022 11:11:27 +0200 Subject: [PATCH 05/10] make tidy Signed-off-by: Max Jonas Werner --- go.mod | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go.mod b/go.mod index 0cc5df239..a35292829 100644 --- a/go.mod +++ b/go.mod @@ -18,6 +18,7 @@ require ( github.com/cyphar/filepath-securejoin v0.2.3 github.com/darkowlzz/controller-check v0.0.0-20220325122359-11f5827b7981 github.com/distribution/distribution/v3 v3.0.0-20211118083504-a29a3c99a684 + github.com/docker/cli v20.10.11+incompatible github.com/docker/go-units v0.4.0 github.com/elazarl/goproxy v0.0.0-20220417044921-416226498f94 github.com/fluxcd/gitkit v0.5.0 @@ -101,7 +102,6 @@ require ( github.com/chai2010/gettext-go v0.0.0-20160711120539-c6fed771bfd5 // indirect github.com/containerd/containerd v1.6.1 // indirect github.com/davecgh/go-spew v1.1.1 // indirect - github.com/docker/cli v20.10.11+incompatible // indirect github.com/docker/distribution v2.8.0+incompatible // indirect github.com/docker/docker v20.10.12+incompatible // indirect github.com/docker/docker-credential-helpers v0.6.4 // indirect From c795da2280468f8a4324c58e624bfeab177e9006 Mon Sep 17 00:00:00 2001 From: Max Jonas Werner Date: Mon, 23 May 2022 11:11:53 +0200 Subject: [PATCH 06/10] introduce `internal/helm/registry` package This new package holds all Helm OCI registry-specific code now so we have a single location to look for such code which makes it easier to find yourself around. Signed-off-by: Max Jonas Werner --- controllers/helmchart_controller.go | 15 ++++--- controllers/helmchart_controller_test.go | 10 ++--- controllers/helmrepository_controller_oci.go | 47 +++----------------- controllers/suite_test.go | 18 ++++---- internal/helm/registry/auth.go | 44 ++++++++++++++++++ internal/helm/{util => registry}/client.go | 4 +- main.go | 6 +-- 7 files changed, 77 insertions(+), 67 deletions(-) create mode 100644 internal/helm/registry/auth.go rename internal/helm/{util => registry}/client.go (94%) diff --git a/controllers/helmchart_controller.go b/controllers/helmchart_controller.go index 80bb773fc..07efa41ed 100644 --- a/controllers/helmchart_controller.go +++ b/controllers/helmchart_controller.go @@ -29,7 +29,7 @@ import ( "time" helmgetter "helm.sh/helm/v3/pkg/getter" - "helm.sh/helm/v3/pkg/registry" + helmreg "helm.sh/helm/v3/pkg/registry" corev1 "k8s.io/api/core/v1" apierrs "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -64,6 +64,7 @@ import ( sreconcile "github.com/fluxcd/source-controller/internal/reconcile" "github.com/fluxcd/source-controller/internal/reconcile/summarize" "github.com/fluxcd/source-controller/internal/util" + "github.com/fluxcd/source-controller/internal/helm/registry" ) // helmChartReadyCondition contains all the conditions information @@ -380,7 +381,7 @@ func (r *HelmChartReconciler) reconcileSource(ctx context.Context, obj *sourcev1 // Assert source has an artifact if s.GetArtifact() == nil || !r.Storage.ArtifactExist(*s.GetArtifact()) { - if helmRepo, ok := s.(*sourcev1.HelmRepository); !ok || !registry.IsOCI(helmRepo.Spec.URL) { + if helmRepo, ok := s.(*sourcev1.HelmRepository); !ok || !helmreg.IsOCI(helmRepo.Spec.URL) { conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, "NoSourceArtifact", "no artifact available for %s source '%s'", obj.Spec.SourceRef.Kind, obj.Spec.SourceRef.Name) r.eventLogf(ctx, obj, events.EventTypeTrace, "NoSourceArtifact", @@ -447,7 +448,7 @@ func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj * repo *sourcev1.HelmRepository, b *chart.Build) (sreconcile.Result, error) { var ( tlsConfig *tls.Config - loginOpts []registry.LoginOption + loginOpts []helmreg.LoginOption ) // Construct the Getter options from the HelmRepository data @@ -492,7 +493,7 @@ func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj * } // Build registryClient options from secret - loginOpt, err := loginOptionFromSecret(repo.Spec.URL, *secret) + loginOpt, err := registry.LoginOptionFromSecret(repo.Spec.URL, *secret) if err != nil { e := &serror.Event{ Err: fmt.Errorf("failed to configure Helm client with secret data: %w", err), @@ -503,14 +504,14 @@ func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj * return sreconcile.ResultEmpty, e } - loginOpts = append([]registry.LoginOption{}, loginOpt) + loginOpts = append([]helmreg.LoginOption{}, loginOpt) } // Initialize the chart repository var chartRepo chart.Remote switch repo.Spec.Type { case sourcev1.HelmRepositoryTypeOCI: - if !registry.IsOCI(repo.Spec.URL) { + if !helmreg.IsOCI(repo.Spec.URL) { err := fmt.Errorf("invalid OCI registry URL: %s", repo.Spec.URL) return chartRepoErrorReturn(err, obj) } @@ -551,7 +552,7 @@ func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj * repository.WithMemoryCache(r.Storage.LocalPath(*repo.GetArtifact()), r.Cache, r.TTL, func(event string) { r.IncCacheEvents(event, obj.Name, obj.Namespace) })) - if err != nil { + if err != nil { return chartRepoErrorReturn(err, obj) } chartRepo = httpChartRepo diff --git a/controllers/helmchart_controller_test.go b/controllers/helmchart_controller_test.go index 9796ea6e7..59ff1d0b1 100644 --- a/controllers/helmchart_controller_test.go +++ b/controllers/helmchart_controller_test.go @@ -36,7 +36,7 @@ import ( . "github.com/onsi/gomega" hchart "helm.sh/helm/v3/pkg/chart" "helm.sh/helm/v3/pkg/chart/loader" - "helm.sh/helm/v3/pkg/registry" + helmreg "helm.sh/helm/v3/pkg/registry" corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -54,7 +54,7 @@ import ( sourcev1 "github.com/fluxcd/source-controller/api/v1beta2" serror "github.com/fluxcd/source-controller/internal/error" "github.com/fluxcd/source-controller/internal/helm/chart" - "github.com/fluxcd/source-controller/internal/helm/util" + "github.com/fluxcd/source-controller/internal/helm/registry" sreconcile "github.com/fluxcd/source-controller/internal/reconcile" "github.com/fluxcd/source-controller/internal/reconcile/summarize" ) @@ -793,8 +793,8 @@ func TestHelmChartReconciler_buildFromOCIHelmRepository(t *testing.T) { // Login to the registry err := testRegistryserver.RegistryClient.Login(testRegistryserver.DockerRegistryHost, - registry.LoginOptBasicAuth(testUsername, testPassword), - registry.LoginOptInsecure(true)) + helmreg.LoginOptBasicAuth(testUsername, testPassword), + helmreg.LoginOptInsecure(true)) g.Expect(err).NotTo(HaveOccurred()) // Load a test chart @@ -975,7 +975,7 @@ func TestHelmChartReconciler_buildFromOCIHelmRepository(t *testing.T) { EventRecorder: record.NewFakeRecorder(32), Getters: testGetters, Storage: storage, - RegistryClientGenerator: util.RegistryClientGenerator, + RegistryClientGenerator: registry.ClientGenerator, } repository := &sourcev1.HelmRepository{ diff --git a/controllers/helmrepository_controller_oci.go b/controllers/helmrepository_controller_oci.go index 7702e446d..ba2d356d6 100644 --- a/controllers/helmrepository_controller_oci.go +++ b/controllers/helmrepository_controller_oci.go @@ -17,15 +17,12 @@ limitations under the License. package controllers import ( - "bytes" "context" "fmt" - "net/url" "os" "strings" "time" - "github.com/docker/cli/cli/config" "github.com/fluxcd/pkg/apis/meta" "github.com/fluxcd/pkg/runtime/conditions" helper "github.com/fluxcd/pkg/runtime/controller" @@ -33,12 +30,13 @@ import ( "github.com/fluxcd/pkg/runtime/predicates" sourcev1 "github.com/fluxcd/source-controller/api/v1beta2" serror "github.com/fluxcd/source-controller/internal/error" + "github.com/fluxcd/source-controller/internal/helm/registry" "github.com/fluxcd/source-controller/internal/helm/repository" intpredicates "github.com/fluxcd/source-controller/internal/predicates" sreconcile "github.com/fluxcd/source-controller/internal/reconcile" "github.com/fluxcd/source-controller/internal/reconcile/summarize" helmgetter "helm.sh/helm/v3/pkg/getter" - "helm.sh/helm/v3/pkg/registry" + helmreg "helm.sh/helm/v3/pkg/registry" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/types" kuberecorder "k8s.io/client-go/tools/record" @@ -94,7 +92,7 @@ type HelmRepositoryOCIReconciler struct { // and an optional file name. // The file is used to store the registry client credentials. // The caller is responsible for deleting the file. -type RegistryClientGeneratorFunc func(isLogin bool) (*registry.Client, string, error) +type RegistryClientGeneratorFunc func(isLogin bool) (*helmreg.Client, string, error) // helmRepositoryOCIReconcileFunc is the function type for all the // v1beta2.HelmRepository (sub)reconcile functions for OCI type. The type implementations @@ -257,7 +255,7 @@ func (r *HelmRepositoryOCIReconciler) reconcile(ctx context.Context, obj *source } func (r *HelmRepositoryOCIReconciler) reconcileSource(ctx context.Context, obj *sourcev1.HelmRepository) (sreconcile.Result, error) { - var loginOpts []registry.LoginOption + var loginOpts []helmreg.LoginOption // Configure any authentication related options if obj.Spec.SecretRef != nil { // Attempt to retrieve secret @@ -276,7 +274,7 @@ func (r *HelmRepositoryOCIReconciler) reconcileSource(ctx context.Context, obj * } // Construct actual options - loginOpt, err := loginOptionFromSecret(obj.Spec.URL, secret) + loginOpt, err := registry.LoginOptionFromSecret(obj.Spec.URL, secret) if err != nil { e := &serror.Event{ Err: fmt.Errorf("failed to configure Helm client with secret data: %w", err), @@ -301,7 +299,7 @@ func (r *HelmRepositoryOCIReconciler) reconcileSource(ctx context.Context, obj * // validateSource the HelmRepository object by checking the url and connecting to the underlying registry // with he provided credentials. -func (r *HelmRepositoryOCIReconciler) validateSource(ctx context.Context, obj *sourcev1.HelmRepository, logOpts ...registry.LoginOption) (sreconcile.Result, error) { +func (r *HelmRepositoryOCIReconciler) validateSource(ctx context.Context, obj *sourcev1.HelmRepository, logOpts ...helmreg.LoginOption) (sreconcile.Result, error) { registryClient, file, err := r.RegistryClientGenerator(logOpts != nil) if err != nil { e := &serror.Stalling{ @@ -354,36 +352,3 @@ func (r *HelmRepositoryOCIReconciler) validateSource(ctx context.Context, obj *s return sreconcile.ResultSuccess, nil } - -// loginOptionFromSecret derives authentication data from a Secret to login to an OCI registry. This Secret -// may either hold "username" and "password" fields or be of the corev1.SecretTypeDockerConfigJson type and hold -// a corev1.DockerConfigJsonKey field with a complete Docker configuration. If both, "username" and "password" are -// empty, a nil LoginOption and a nil error will be returned. -func loginOptionFromSecret(registryURL string, secret corev1.Secret) (registry.LoginOption, error) { - var username, password string - if secret.Type == corev1.SecretTypeDockerConfigJson { - dockerCfg, err := config.LoadFromReader(bytes.NewReader(secret.Data[corev1.DockerConfigJsonKey])) - if err != nil { - return nil, fmt.Errorf("unable to load Docker config: %w", err) - } - parsedURL, err := url.Parse(registryURL) - if err != nil { - return nil, fmt.Errorf("unable to parse registry URL: %w", err) - } - authConfig, err := dockerCfg.GetAuthConfig(parsedURL.Host) - if err != nil { - return nil, fmt.Errorf("unable to get authentication data from Secret: %w", err) - } - username = authConfig.Username - password = authConfig.Password - } else { - username, password = string(secret.Data["username"]), string(secret.Data["password"]) - } - switch { - case username == "" && password == "": - return nil, nil - case username == "" || password == "": - return nil, fmt.Errorf("invalid '%s' secret data: required fields 'username' and 'password'", secret.Name) - } - return registry.LoginOptBasicAuth(username, password), nil -} diff --git a/controllers/suite_test.go b/controllers/suite_test.go index 7cef15e39..6531d633f 100644 --- a/controllers/suite_test.go +++ b/controllers/suite_test.go @@ -30,7 +30,7 @@ import ( "golang.org/x/crypto/bcrypt" "helm.sh/helm/v3/pkg/getter" - "helm.sh/helm/v3/pkg/registry" + helmreg "helm.sh/helm/v3/pkg/registry" utilruntime "k8s.io/apimachinery/pkg/util/runtime" "k8s.io/client-go/kubernetes/scheme" "k8s.io/client-go/tools/record" @@ -49,7 +49,7 @@ import ( sourcev1 "github.com/fluxcd/source-controller/api/v1beta2" "github.com/fluxcd/source-controller/internal/cache" "github.com/fluxcd/source-controller/internal/features" - "github.com/fluxcd/source-controller/internal/helm/util" + "github.com/fluxcd/source-controller/internal/helm/registry" // +kubebuilder:scaffold:imports ) @@ -94,7 +94,7 @@ var ( ) var ( - testRegistryClient *registry.Client + testRegistryClient *helmreg.Client testRegistryserver *RegistryClientTestServer ) @@ -113,7 +113,7 @@ type RegistryClientTestServer struct { Out io.Writer DockerRegistryHost string WorkspaceDir string - RegistryClient *registry.Client + RegistryClient *helmreg.Client } func SetupServer(server *RegistryClientTestServer) string { @@ -129,9 +129,9 @@ func SetupServer(server *RegistryClientTestServer) string { server.Out = &out // init test client - server.RegistryClient, err = registry.NewClient( - registry.ClientOptDebug(true), - registry.ClientOptWriter(server.Out), + server.RegistryClient, err = helmreg.NewClient( + helmreg.ClientOptDebug(true), + helmreg.ClientOptWriter(server.Out), ) if err != nil { panic(fmt.Sprintf("failed to create registry client: %s", err)) @@ -202,7 +202,7 @@ func TestMain(m *testing.M) { testRegistryserver = &RegistryClientTestServer{} registryWorkspaceDir := SetupServer(testRegistryserver) - testRegistryClient, err = registry.NewClient(registry.ClientOptWriter(os.Stdout)) + testRegistryClient, err = helmreg.NewClient(helmreg.ClientOptWriter(os.Stdout)) if err != nil { panic(fmt.Sprintf("Failed to create OCI registry client")) } @@ -241,7 +241,7 @@ func TestMain(m *testing.M) { EventRecorder: record.NewFakeRecorder(32), Metrics: testMetricsH, Getters: testGetters, - RegistryClientGenerator: util.RegistryClientGenerator, + RegistryClientGenerator: registry.ClientGenerator, }).SetupWithManager(testEnv); err != nil { panic(fmt.Sprintf("Failed to start HelmRepositoryOCIReconciler: %v", err)) } diff --git a/internal/helm/registry/auth.go b/internal/helm/registry/auth.go new file mode 100644 index 000000000..64922cdd9 --- /dev/null +++ b/internal/helm/registry/auth.go @@ -0,0 +1,44 @@ +package registry + +import ( + "bytes" + "fmt" + "net/url" + + "github.com/docker/cli/cli/config" + "helm.sh/helm/v3/pkg/registry" + corev1 "k8s.io/api/core/v1" +) + +// LoginOptionFromSecret derives authentication data from a Secret to login to an OCI registry. This Secret +// may either hold "username" and "password" fields or be of the corev1.SecretTypeDockerConfigJson type and hold +// a corev1.DockerConfigJsonKey field with a complete Docker configuration. If both, "username" and "password" are +// empty, a nil LoginOption and a nil error will be returned. +func LoginOptionFromSecret(registryURL string, secret corev1.Secret) (registry.LoginOption, error) { + var username, password string + if secret.Type == corev1.SecretTypeDockerConfigJson { + dockerCfg, err := config.LoadFromReader(bytes.NewReader(secret.Data[corev1.DockerConfigJsonKey])) + if err != nil { + return nil, fmt.Errorf("unable to load Docker config: %w", err) + } + parsedURL, err := url.Parse(registryURL) + if err != nil { + return nil, fmt.Errorf("unable to parse registry URL: %w", err) + } + authConfig, err := dockerCfg.GetAuthConfig(parsedURL.Host) + if err != nil { + return nil, fmt.Errorf("unable to get authentication data from Secret: %w", err) + } + username = authConfig.Username + password = authConfig.Password + } else { + username, password = string(secret.Data["username"]), string(secret.Data["password"]) + } + switch { + case username == "" && password == "": + return nil, nil + case username == "" || password == "": + return nil, fmt.Errorf("invalid '%s' secret data: required fields 'username' and 'password'", secret.Name) + } + return registry.LoginOptBasicAuth(username, password), nil +} diff --git a/internal/helm/util/client.go b/internal/helm/registry/client.go similarity index 94% rename from internal/helm/util/client.go rename to internal/helm/registry/client.go index 1bd8944f6..0e835e8f7 100644 --- a/internal/helm/util/client.go +++ b/internal/helm/registry/client.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package util +package registry import ( "io" @@ -26,7 +26,7 @@ import ( // RegistryClientGenerator generates a registry client and a temporary credential file. // The client is meant to be used for a single reconciliation. // The file is meant to be used for a single reconciliation and deleted after. -func RegistryClientGenerator(isLogin bool) (*registry.Client, string, error) { +func ClientGenerator(isLogin bool) (*registry.Client, string, error) { if isLogin { // create a temporary file to store the credentials // this is needed because otherwise the credentials are stored in ~/.docker/config.json. diff --git a/main.go b/main.go index 5088d599f..a4b878a2c 100644 --- a/main.go +++ b/main.go @@ -42,7 +42,7 @@ import ( "github.com/fluxcd/pkg/runtime/pprof" "github.com/fluxcd/pkg/runtime/probes" "github.com/fluxcd/source-controller/internal/features" - "github.com/fluxcd/source-controller/internal/helm/util" + "github.com/fluxcd/source-controller/internal/helm/registry" sourcev1 "github.com/fluxcd/source-controller/api/v1beta2" "github.com/fluxcd/source-controller/controllers" @@ -239,7 +239,7 @@ func main() { Metrics: metricsH, Getters: getters, ControllerName: controllerName, - RegistryClientGenerator: util.RegistryClientGenerator, + RegistryClientGenerator: registry.ClientGenerator, }).SetupWithManagerAndOptions(mgr, controllers.HelmRepositoryReconcilerOptions{ MaxConcurrentReconciles: concurrent, RateLimiter: helper.GetRateLimiter(rateLimiterOptions), @@ -270,7 +270,7 @@ func main() { if err = (&controllers.HelmChartReconciler{ Client: mgr.GetClient(), - RegistryClientGenerator: util.RegistryClientGenerator, + RegistryClientGenerator: registry.ClientGenerator, Storage: storage, Getters: getters, EventRecorder: eventRecorder, From a3be7e5d3d7bb924a066ea7253eb29fd2fb8a25a Mon Sep 17 00:00:00 2001 From: Max Jonas Werner Date: Mon, 23 May 2022 11:28:01 +0200 Subject: [PATCH 07/10] document generateBuildResult Signed-off-by: Max Jonas Werner --- internal/helm/chart/builder_remote.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/helm/chart/builder_remote.go b/internal/helm/chart/builder_remote.go index 97de68137..d170ec29b 100644 --- a/internal/helm/chart/builder_remote.go +++ b/internal/helm/chart/builder_remote.go @@ -199,9 +199,9 @@ func (b *remoteChartBuilder) downloadFromRepository(remote *repository.ChartRepo if err != nil { return nil, err } + *buildResult = *result if shouldReturn { - *buildResult = *result return nil, nil } @@ -212,11 +212,11 @@ func (b *remoteChartBuilder) downloadFromRepository(remote *repository.ChartRepo return nil, &BuildError{Reason: ErrChartPull, Err: err} } - *buildResult = *result - return res, nil } +// generateBuildResult returns a Build object generated from the given chart version and build options. It also returns +// true if the given chart can be retrieved from cache and doesn't need to be downloaded again. func generateBuildResult(cv *repo.ChartVersion, opts BuildOptions) (*Build, bool, error) { result := &Build{} result.Version = cv.Version From 09a2458cfd27139e0e93909db1c928c54c064506 Mon Sep 17 00:00:00 2001 From: Max Jonas Werner Date: Mon, 23 May 2022 15:10:21 +0200 Subject: [PATCH 08/10] fix import order Signed-off-by: Max Jonas Werner --- controllers/helmchart_controller.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/controllers/helmchart_controller.go b/controllers/helmchart_controller.go index 07efa41ed..a294c8cba 100644 --- a/controllers/helmchart_controller.go +++ b/controllers/helmchart_controller.go @@ -60,11 +60,11 @@ import ( serror "github.com/fluxcd/source-controller/internal/error" "github.com/fluxcd/source-controller/internal/helm/chart" "github.com/fluxcd/source-controller/internal/helm/getter" + "github.com/fluxcd/source-controller/internal/helm/registry" "github.com/fluxcd/source-controller/internal/helm/repository" sreconcile "github.com/fluxcd/source-controller/internal/reconcile" "github.com/fluxcd/source-controller/internal/reconcile/summarize" "github.com/fluxcd/source-controller/internal/util" - "github.com/fluxcd/source-controller/internal/helm/registry" ) // helmChartReadyCondition contains all the conditions information @@ -552,7 +552,7 @@ func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj * repository.WithMemoryCache(r.Storage.LocalPath(*repo.GetArtifact()), r.Cache, r.TTL, func(event string) { r.IncCacheEvents(event, obj.Name, obj.Namespace) })) - if err != nil { + if err != nil { return chartRepoErrorReturn(err, obj) } chartRepo = httpChartRepo From 7cfd94effba6b5984100dd43f52e3c038e4e7546 Mon Sep 17 00:00:00 2001 From: Max Jonas Werner Date: Mon, 23 May 2022 15:54:28 +0200 Subject: [PATCH 09/10] fix func doc Signed-off-by: Max Jonas Werner --- internal/helm/registry/client.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/helm/registry/client.go b/internal/helm/registry/client.go index 0e835e8f7..9cb68a451 100644 --- a/internal/helm/registry/client.go +++ b/internal/helm/registry/client.go @@ -23,7 +23,7 @@ import ( "helm.sh/helm/v3/pkg/registry" ) -// RegistryClientGenerator generates a registry client and a temporary credential file. +// ClientGenerator generates a registry client and a temporary credential file. // The client is meant to be used for a single reconciliation. // The file is meant to be used for a single reconciliation and deleted after. func ClientGenerator(isLogin bool) (*registry.Client, string, error) { From bb569bec1fd46de57bd8b1a496a86e3bd5be5240 Mon Sep 17 00:00:00 2001 From: Max Jonas Werner Date: Tue, 24 May 2022 10:30:32 +0200 Subject: [PATCH 10/10] include Secret name in returned errors Signed-off-by: Max Jonas Werner --- internal/helm/registry/auth.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/internal/helm/registry/auth.go b/internal/helm/registry/auth.go index 64922cdd9..a37e4c658 100644 --- a/internal/helm/registry/auth.go +++ b/internal/helm/registry/auth.go @@ -19,15 +19,16 @@ func LoginOptionFromSecret(registryURL string, secret corev1.Secret) (registry.L if secret.Type == corev1.SecretTypeDockerConfigJson { dockerCfg, err := config.LoadFromReader(bytes.NewReader(secret.Data[corev1.DockerConfigJsonKey])) if err != nil { - return nil, fmt.Errorf("unable to load Docker config: %w", err) + return nil, fmt.Errorf("unable to load Docker config from Secret '%s': %w", secret.Name, err) } parsedURL, err := url.Parse(registryURL) if err != nil { - return nil, fmt.Errorf("unable to parse registry URL: %w", err) + return nil, fmt.Errorf("unable to parse registry URL '%s' while reconciling Secret '%s': %w", + registryURL, secret.Name, err) } authConfig, err := dockerCfg.GetAuthConfig(parsedURL.Host) if err != nil { - return nil, fmt.Errorf("unable to get authentication data from Secret: %w", err) + return nil, fmt.Errorf("unable to get authentication data from Secret '%s': %w", secret.Name, err) } username = authConfig.Username password = authConfig.Password