diff --git a/controllers/helmrelease_controller.go b/controllers/helmrelease_controller.go index cd43b5c..af86812 100644 --- a/controllers/helmrelease_controller.go +++ b/controllers/helmrelease_controller.go @@ -20,16 +20,13 @@ import ( "context" "errors" "fmt" - "strings" "time" "github.com/hashicorp/go-retryablehttp" "helm.sh/helm/v3/pkg/chart" "helm.sh/helm/v3/pkg/chartutil" "helm.sh/helm/v3/pkg/storage/driver" - "helm.sh/helm/v3/pkg/strvals" corev1 "k8s.io/api/core/v1" - apierrors "k8s.io/apimachinery/pkg/api/errors" apimeta "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -56,11 +53,12 @@ import ( "github.com/fluxcd/pkg/runtime/events" "github.com/fluxcd/pkg/runtime/metrics" "github.com/fluxcd/pkg/runtime/predicates" - "github.com/fluxcd/pkg/runtime/transform" sourcev1 "github.com/fluxcd/source-controller/api/v1beta2" v2 "github.com/fluxcd/helm-controller/api/v2beta1" + intchartutil "github.com/fluxcd/helm-controller/internal/chartutil" "github.com/fluxcd/helm-controller/internal/kube" + "github.com/fluxcd/helm-controller/internal/loader" "github.com/fluxcd/helm-controller/internal/runner" "github.com/fluxcd/helm-controller/internal/util" ) @@ -75,15 +73,21 @@ import ( // HelmReleaseReconciler reconciles a HelmRelease object type HelmReleaseReconciler struct { client.Client - httpClient *retryablehttp.Client - Config *rest.Config - Scheme *runtime.Scheme - requeueDependency time.Duration - EventRecorder kuberecorder.EventRecorder - MetricsRecorder *metrics.Recorder - DefaultServiceAccount string - NoCrossNamespaceRef bool - KubeConfigOpts fluxClient.KubeConfigOptions + httpClient *retryablehttp.Client + Config *rest.Config + Scheme *runtime.Scheme + requeueDependency time.Duration + EventRecorder kuberecorder.EventRecorder + MetricsRecorder *metrics.Recorder + NoCrossNamespaceRef bool + KubeConfigOpts fluxClient.KubeConfigOptions +} + +type HelmReleaseReconcilerOptions struct { + MaxConcurrentReconciles int + HTTPRetry int + DependencyRequeueInterval time.Duration + RateLimiter ratelimiter.RateLimiter } func (r *HelmReleaseReconciler) SetupWithManager(mgr ctrl.Manager, opts HelmReleaseReconcilerOptions) error { @@ -261,14 +265,14 @@ func (r *HelmReleaseReconciler) reconcile(ctx context.Context, hr v2.HelmRelease } // Compose values - values, err := r.composeValues(ctx, hr) + values, err := intchartutil.ChartValuesFromReferences(ctx, r.Client, hr.Namespace, hr.GetValues(), hr.Spec.ValuesFrom...) if err != nil { r.event(ctx, hr, hr.Status.LastAttemptedRevision, events.EventSeverityError, err.Error()) return v2.HelmReleaseNotReady(hr, v2.InitFailedReason, err.Error()), ctrl.Result{Requeue: true}, nil } // Load chart from artifact - chart, err := r.loadHelmChart(hc) + chart, err := loader.SecureLoadChartFromURL(r.httpClient, hc.GetArtifact().URL, hc.GetArtifact().Checksum) if err != nil { r.event(ctx, hr, hr.Status.LastAttemptedRevision, events.EventSeverityError, err.Error()) return v2.HelmReleaseNotReady(hr, v2.ArtifactFailedReason, err.Error()), ctrl.Result{Requeue: true}, nil @@ -283,19 +287,12 @@ func (r *HelmReleaseReconciler) reconcile(ctx context.Context, hr v2.HelmRelease return reconciledHr, ctrl.Result{RequeueAfter: hr.Spec.Interval.Duration}, reconcileErr } -type HelmReleaseReconcilerOptions struct { - MaxConcurrentReconciles int - HTTPRetry int - DependencyRequeueInterval time.Duration - RateLimiter ratelimiter.RateLimiter -} - func (r *HelmReleaseReconciler) reconcileRelease(ctx context.Context, hr v2.HelmRelease, chart *chart.Chart, values chartutil.Values) (v2.HelmRelease, error) { log := ctrl.LoggerFrom(ctx) // Initialize Helm action runner - getter, err := r.getRESTClientGetter(ctx, hr) + getter, err := r.buildRESTClientGetter(ctx, hr) if err != nil { return v2.HelmReleaseNotReady(hr, v2.InitFailedReason, err.Error()), err } @@ -472,23 +469,11 @@ func (r *HelmReleaseReconciler) checkDependencies(hr v2.HelmRelease) error { return nil } -func (r *HelmReleaseReconciler) setImpersonationConfig(restConfig *rest.Config, hr v2.HelmRelease) string { - name := r.DefaultServiceAccount - if sa := hr.Spec.ServiceAccountName; sa != "" { - name = sa +func (r *HelmReleaseReconciler) buildRESTClientGetter(ctx context.Context, hr v2.HelmRelease) (genericclioptions.RESTClientGetter, error) { + var opts []kube.ClientGetterOption + if hr.Spec.ServiceAccountName != "" { + opts = append(opts, kube.WithImpersonate(hr.Spec.ServiceAccountName)) } - if name != "" { - username := fmt.Sprintf("system:serviceaccount:%s:%s", hr.GetNamespace(), name) - restConfig.Impersonate = rest.ImpersonationConfig{UserName: username} - return username - } - return "" -} - -func (r *HelmReleaseReconciler) getRESTClientGetter(ctx context.Context, hr v2.HelmRelease) (genericclioptions.RESTClientGetter, error) { - config := *r.Config - impersonateAccount := r.setImpersonationConfig(&config, hr) - if hr.Spec.KubeConfig != nil { secretName := types.NamespacedName{ Namespace: hr.GetNamespace(), @@ -498,147 +483,34 @@ func (r *HelmReleaseReconciler) getRESTClientGetter(ctx context.Context, hr v2.H if err := r.Get(ctx, secretName, &secret); err != nil { return nil, fmt.Errorf("could not find KubeConfig secret '%s': %w", secretName, err) } - - var kubeConfig []byte - switch { - case hr.Spec.KubeConfig.SecretRef.Key != "": - key := hr.Spec.KubeConfig.SecretRef.Key - kubeConfig = secret.Data[key] - if kubeConfig == nil { - return nil, fmt.Errorf("KubeConfig secret '%s' does not contain a '%s' key with a kubeconfig", secretName, key) - } - case secret.Data["value"] != nil: - kubeConfig = secret.Data["value"] - case secret.Data["value.yaml"] != nil: - kubeConfig = secret.Data["value.yaml"] - default: - // User did not specify a key, and the 'value' key was not defined. - return nil, fmt.Errorf("KubeConfig secret '%s' does not contain a 'value' key with a kubeconfig", secretName) + kubeConfig, err := kube.ConfigFromSecret(&secret, hr.Spec.KubeConfig.SecretRef.Key) + if err != nil { + return nil, err } - - return kube.NewMemoryRESTClientGetter(kubeConfig, hr.GetReleaseNamespace(), impersonateAccount, r.Config.QPS, r.Config.Burst, r.KubeConfigOpts), nil + opts = append(opts, kube.WithKubeConfig(kubeConfig, r.Config.QPS, r.Config.Burst, r.KubeConfigOpts)) } - - if r.DefaultServiceAccount != "" || hr.Spec.ServiceAccountName != "" { - return kube.NewInClusterRESTClientGetter(&config, hr.GetReleaseNamespace()), nil - } - - return kube.NewInClusterRESTClientGetter(r.Config, hr.GetReleaseNamespace()), nil + return kube.BuildClientGetter(r.Config, hr.GetReleaseNamespace(), opts...), nil } -// composeValues attempts to resolve all v2beta1.ValuesReference resources -// and merges them as defined. Referenced resources are only retrieved once -// to ensure a single version is taken into account during the merge. -func (r *HelmReleaseReconciler) composeValues(ctx context.Context, hr v2.HelmRelease) (chartutil.Values, error) { - result := chartutil.Values{} - - configMaps := make(map[string]*corev1.ConfigMap) - secrets := make(map[string]*corev1.Secret) - - for _, v := range hr.Spec.ValuesFrom { - namespacedName := types.NamespacedName{Namespace: hr.Namespace, Name: v.Name} - var valuesData []byte - - switch v.Kind { - case "ConfigMap": - resource, ok := configMaps[namespacedName.String()] - if !ok { - // The resource may not exist, but we want to act on a single version - // of the resource in case the values reference is marked as optional. - configMaps[namespacedName.String()] = nil - - resource = &corev1.ConfigMap{} - if err := r.Get(ctx, namespacedName, resource); err != nil { - if apierrors.IsNotFound(err) { - if v.Optional { - (ctrl.LoggerFrom(ctx)). - Info(fmt.Sprintf("could not find optional %s '%s'", v.Kind, namespacedName)) - continue - } - return nil, fmt.Errorf("could not find %s '%s'", v.Kind, namespacedName) - } - return nil, err - } - configMaps[namespacedName.String()] = resource - } - if resource == nil { - if v.Optional { - (ctrl.LoggerFrom(ctx)).Info(fmt.Sprintf("could not find optional %s '%s'", v.Kind, namespacedName)) - continue - } - return nil, fmt.Errorf("could not find %s '%s'", v.Kind, namespacedName) - } - if data, ok := resource.Data[v.GetValuesKey()]; !ok { - return nil, fmt.Errorf("missing key '%s' in %s '%s'", v.GetValuesKey(), v.Kind, namespacedName) - } else { - valuesData = []byte(data) - } - case "Secret": - resource, ok := secrets[namespacedName.String()] - if !ok { - // The resource may not exist, but we want to act on a single version - // of the resource in case the values reference is marked as optional. - secrets[namespacedName.String()] = nil - - resource = &corev1.Secret{} - if err := r.Get(ctx, namespacedName, resource); err != nil { - if apierrors.IsNotFound(err) { - if v.Optional { - (ctrl.LoggerFrom(ctx)). - Info(fmt.Sprintf("could not find optional %s '%s'", v.Kind, namespacedName)) - continue - } - return nil, fmt.Errorf("could not find %s '%s'", v.Kind, namespacedName) - } - return nil, err - } - secrets[namespacedName.String()] = resource - } - if resource == nil { - if v.Optional { - (ctrl.LoggerFrom(ctx)).Info(fmt.Sprintf("could not find optional %s '%s'", v.Kind, namespacedName)) - continue - } - return nil, fmt.Errorf("could not find %s '%s'", v.Kind, namespacedName) - } - if data, ok := resource.Data[v.GetValuesKey()]; !ok { - return nil, fmt.Errorf("missing key '%s' in %s '%s'", v.GetValuesKey(), v.Kind, namespacedName) - } else { - valuesData = data - } - default: - return nil, fmt.Errorf("unsupported ValuesReference kind '%s'", v.Kind) - } - switch v.TargetPath { - case "": - values, err := chartutil.ReadValues(valuesData) - if err != nil { - return nil, fmt.Errorf("unable to read values from key '%s' in %s '%s': %w", v.GetValuesKey(), v.Kind, namespacedName, err) - } - result = transform.MergeMaps(result, values) - default: - // TODO(hidde): this is a bit of hack, as it mimics the way the option string is passed - // to Helm from a CLI perspective. Given the parser is however not publicly accessible - // while it contains all logic around parsing the target path, it is a fair trade-off. - stringValuesData := string(valuesData) - const singleQuote = "'" - const doubleQuote = "\"" - var err error - if (strings.HasPrefix(stringValuesData, singleQuote) && strings.HasSuffix(stringValuesData, singleQuote)) || (strings.HasPrefix(stringValuesData, doubleQuote) && strings.HasSuffix(stringValuesData, doubleQuote)) { - stringValuesData = strings.Trim(stringValuesData, singleQuote+doubleQuote) - singleValue := v.TargetPath + "=" + stringValuesData - err = strvals.ParseIntoString(singleValue, result) - } else { - singleValue := v.TargetPath + "=" + stringValuesData - err = strvals.ParseInto(singleValue, result) - } - if err != nil { - return nil, fmt.Errorf("unable to merge value from key '%s' in %s '%s' into target path '%s': %w", v.GetValuesKey(), v.Kind, namespacedName, v.TargetPath, err) - } - } +// 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) (*sourcev1.HelmChart, error) { + namespace, name := hr.Status.GetHelmChart() + chartName := types.NamespacedName{Namespace: namespace, Name: name} + if r.NoCrossNamespaceRef && chartName.Namespace != hr.Namespace { + return nil, acl.AccessDeniedError(fmt.Sprintf("can't access '%s/%s', cross-namespace references have been blocked", + hr.Spec.Chart.Spec.SourceRef.Kind, types.NamespacedName{ + Namespace: hr.Spec.Chart.Spec.SourceRef.Namespace, + Name: hr.Spec.Chart.Spec.SourceRef.Name, + })) } - return transform.MergeMaps(result, hr.GetValues()), nil + hc := sourcev1.HelmChart{} + if err := r.Client.Get(ctx, chartName, &hc); err != nil { + return nil, err + } + return &hc, nil } // reconcileDelete deletes the v1beta2.HelmChart of the v2beta1.HelmRelease, @@ -648,7 +520,7 @@ func (r *HelmReleaseReconciler) reconcileDelete(ctx context.Context, hr v2.HelmR // Only uninstall the Helm Release if the resource is not suspended. if !hr.Spec.Suspend { - getter, err := r.getRESTClientGetter(ctx, hr) + getter, err := r.buildRESTClientGetter(ctx, hr) if err != nil { return ctrl.Result{}, err } diff --git a/controllers/helmrelease_controller_chart.go b/controllers/helmrelease_controller_chart.go deleted file mode 100644 index db6fd19..0000000 --- a/controllers/helmrelease_controller_chart.go +++ /dev/null @@ -1,126 +0,0 @@ -/* -Copyright 2020 The Flux authors - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package controllers - -import ( - "bytes" - "context" - "crypto/sha256" - "fmt" - "io" - "net/http" - "net/url" - "os" - - "github.com/fluxcd/pkg/runtime/acl" - sourcev1 "github.com/fluxcd/source-controller/api/v1beta2" - "github.com/hashicorp/go-retryablehttp" - "helm.sh/helm/v3/pkg/chart" - "helm.sh/helm/v3/pkg/chart/loader" - "k8s.io/apimachinery/pkg/types" - - v2 "github.com/fluxcd/helm-controller/api/v2beta1" -) - -const ( - // EnvArtifactHostOverwrite can be used to overwrite the hostname. - // The main purpose is while running controllers locally with e.g. mocked - // storage data during development. - EnvArtifactHostOverwrite = "ARTIFACT_HOST_OVERWRITE" -) - -// 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) (*sourcev1.HelmChart, error) { - namespace, name := hr.Status.GetHelmChart() - chartName := types.NamespacedName{Namespace: namespace, Name: name} - if r.NoCrossNamespaceRef && chartName.Namespace != hr.Namespace { - return nil, acl.AccessDeniedError(fmt.Sprintf("can't access '%s/%s', cross-namespace references have been blocked", - hr.Spec.Chart.Spec.SourceRef.Kind, types.NamespacedName{ - Namespace: hr.Spec.Chart.Spec.SourceRef.Namespace, - Name: hr.Spec.Chart.Spec.SourceRef.Name, - })) - } - hc := sourcev1.HelmChart{} - if err := r.Client.Get(ctx, chartName, &hc); err != nil { - return nil, err - } - return &hc, nil -} - -// loadHelmChart attempts to download the advertised v1beta2.Artifact from the -// provided v1beta2.HelmChart. The SHA256 sum of the Artifact is confirmed to -// equal to the checksum of the retrieved bytes before loading the chart. -// It returns the loaded chart.Chart, or an error. -func (r *HelmReleaseReconciler) loadHelmChart(source *sourcev1.HelmChart) (*chart.Chart, error) { - artifactURL := source.GetArtifact().URL - if hostname := os.Getenv(EnvArtifactHostOverwrite); hostname != "" { - if replacedArtifactURL, err := replaceHostname(artifactURL, hostname); err == nil { - artifactURL = replacedArtifactURL - } - } - - req, err := retryablehttp.NewRequest(http.MethodGet, artifactURL, nil) - if err != nil { - return nil, fmt.Errorf("failed to create a new request for artifact '%s': %w", source.GetArtifact().URL, err) - } - - resp, err := r.httpClient.Do(req) - if err != nil || resp != nil && resp.StatusCode != http.StatusOK { - return nil, fmt.Errorf("artifact '%s' download failed: %w", source.GetArtifact().URL, err) - } - - var c bytes.Buffer - if err := copyAndVerifyArtifact(source.GetArtifact(), resp.Body, &c); err != nil { - return nil, fmt.Errorf("artifact '%s' download failed: %w", source.GetArtifact().URL, err) - } - - if err := resp.Body.Close(); err != nil { - return nil, fmt.Errorf("artifact '%s' download failed: %w", source.GetArtifact().URL, err) - } - - return loader.LoadArchive(&c) -} - -// copyAndVerifyArtifact copies from reader into writer while confirming the -// SHA256 checksum of the copied data matches the checksum from the provided -// v1beta2.Artifact. If this does not match, it returns an error. -func copyAndVerifyArtifact(artifact *sourcev1.Artifact, reader io.Reader, writer io.Writer) error { - hasher := sha256.New() - mw := io.MultiWriter(hasher, writer) - if _, err := io.Copy(mw, reader); err != nil { - return fmt.Errorf("failed to verify artifact: %w", err) - } - if checksum := fmt.Sprintf("%x", hasher.Sum(nil)); checksum != artifact.Checksum { - return fmt.Errorf("failed to verify artifact: computed checksum '%s' doesn't match advertised '%s'", - checksum, artifact.Checksum) - } - return nil -} - -// replaceHostname parses the given URL and replaces the Host in the parsed -// result with the provided hostname. It returns the string result, or an -// error. -func replaceHostname(URL, hostname string) (string, error) { - parsedURL, err := url.Parse(URL) - if err != nil { - return "", err - } - parsedURL.Host = hostname - return parsedURL.String(), nil -} diff --git a/controllers/helmrelease_controller_chart_test.go b/controllers/helmrelease_controller_chart_test.go deleted file mode 100644 index e8e8bd4..0000000 --- a/controllers/helmrelease_controller_chart_test.go +++ /dev/null @@ -1,326 +0,0 @@ -/* -Copyright 2020 The Flux authors - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package controllers - -import ( - "bytes" - "context" - "crypto/sha256" - "fmt" - "io" - "net/http" - "net/http/httptest" - "os" - "strings" - "testing" - - v2 "github.com/fluxcd/helm-controller/api/v2beta1" - sourcev1 "github.com/fluxcd/source-controller/api/v1beta2" - "github.com/hashicorp/go-retryablehttp" - . "github.com/onsi/gomega" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/runtime" - "k8s.io/client-go/tools/record" - "sigs.k8s.io/controller-runtime/pkg/client/fake" -) - -func TestHelmReleaseReconciler_getHelmChart(t *testing.T) { - g := NewWithT(t) - - scheme := runtime.NewScheme() - g.Expect(v2.AddToScheme(scheme)).To(Succeed()) - g.Expect(sourcev1.AddToScheme(scheme)).To(Succeed()) - - chart := &sourcev1.HelmChart{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: "some-namespace", - Name: "some-chart-name", - }, - } - - tests := []struct { - name string - rel *v2.HelmRelease - chart *sourcev1.HelmChart - expectChart bool - wantErr bool - disallowCrossNS bool - }{ - { - name: "retrieves HelmChart object from Status", - rel: &v2.HelmRelease{ - Status: v2.HelmReleaseStatus{ - HelmChart: "some-namespace/some-chart-name", - }, - }, - chart: chart, - expectChart: true, - }, - { - name: "no HelmChart found", - rel: &v2.HelmRelease{ - Status: v2.HelmReleaseStatus{ - HelmChart: "some-namespace/some-chart-name", - }, - }, - chart: nil, - expectChart: false, - wantErr: true, - }, - { - name: "no HelmChart in Status", - rel: &v2.HelmRelease{ - Status: v2.HelmReleaseStatus{ - HelmChart: "", - }, - }, - chart: chart, - expectChart: false, - wantErr: true, - }, - { - name: "ACL disallows cross namespace", - rel: &v2.HelmRelease{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: "default", - }, - Status: v2.HelmReleaseStatus{ - HelmChart: "some-namespace/some-chart-name", - }, - }, - chart: chart, - expectChart: false, - wantErr: true, - disallowCrossNS: true, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - builder := fake.NewClientBuilder() - builder.WithScheme(scheme) - if tt.chart != nil { - builder.WithObjects(tt.chart) - } - - r := &HelmReleaseReconciler{ - Client: builder.Build(), - EventRecorder: record.NewFakeRecorder(32), - NoCrossNamespaceRef: tt.disallowCrossNS, - } - - got, err := r.getHelmChart(context.TODO(), tt.rel) - if tt.wantErr { - g.Expect(err).To(HaveOccurred()) - g.Expect(got).To(BeNil()) - return - } - g.Expect(err).ToNot(HaveOccurred()) - expect := g.Expect(got.ObjectMeta) - if tt.expectChart { - expect.To(BeEquivalentTo(tt.chart.ObjectMeta)) - } else { - expect.To(BeNil()) - } - }) - } -} - -func TestHelmReleaseReconciler_loadHelmChart(t *testing.T) { - g := NewWithT(t) - - b, err := os.ReadFile("testdata/chart-0.1.0.tgz") - g.Expect(err).ToNot(HaveOccurred()) - g.Expect(b).ToNot(BeNil()) - checksum := fmt.Sprintf("%x", sha256.Sum256(b)) - - const chartPath = "/chart.tgz" - server := httptest.NewServer(http.HandlerFunc(func(res http.ResponseWriter, req *http.Request) { - if req.URL.Path == chartPath { - res.WriteHeader(http.StatusOK) - _, _ = res.Write(b) - return - } - res.WriteHeader(http.StatusInternalServerError) - return - })) - t.Cleanup(func() { - server.Close() - }) - - chartURL := server.URL + chartPath - - client := retryablehttp.NewClient() - client.Logger = nil - client.RetryMax = 2 - - t.Run("loads HelmChart from Artifact URL", func(t *testing.T) { - g := NewWithT(t) - - r := &HelmReleaseReconciler{ - Client: fake.NewClientBuilder().Build(), - EventRecorder: record.NewFakeRecorder(32), - httpClient: client, - } - got, err := r.loadHelmChart(&sourcev1.HelmChart{ - Status: sourcev1.HelmChartStatus{ - Artifact: &sourcev1.Artifact{ - URL: chartURL, - Checksum: checksum, - }, - }, - }) - g.Expect(err).ToNot(HaveOccurred()) - g.Expect(got).ToNot(BeNil()) - g.Expect(got.Name()).To(Equal("chart")) - g.Expect(got.Metadata.Version).To(Equal("0.1.0")) - }) - - t.Run("error on Artifact checksum mismatch", func(t *testing.T) { - g := NewWithT(t) - - r := &HelmReleaseReconciler{ - Client: fake.NewClientBuilder().Build(), - EventRecorder: record.NewFakeRecorder(32), - httpClient: client, - } - got, err := r.loadHelmChart(&sourcev1.HelmChart{ - Status: sourcev1.HelmChartStatus{ - Artifact: &sourcev1.Artifact{ - URL: chartURL, - Checksum: "", - }, - }, - }) - g.Expect(err).To(HaveOccurred()) - g.Expect(got).To(BeNil()) - }) - - t.Run("error on server error", func(t *testing.T) { - g := NewWithT(t) - - r := &HelmReleaseReconciler{ - Client: fake.NewClientBuilder().Build(), - EventRecorder: record.NewFakeRecorder(32), - httpClient: client, - } - got, err := r.loadHelmChart(&sourcev1.HelmChart{ - Status: sourcev1.HelmChartStatus{ - Artifact: &sourcev1.Artifact{ - URL: server.URL + "/invalid.tgz", - Checksum: "", - }, - }, - }) - g.Expect(err).To(HaveOccurred()) - g.Expect(got).To(BeNil()) - }) - - t.Run("EnvArtifactHostOverwrite overwrites Artifact hostname", func(t *testing.T) { - g := NewWithT(t) - - t.Setenv(EnvArtifactHostOverwrite, strings.TrimPrefix(server.URL, "http://")) - r := &HelmReleaseReconciler{ - Client: fake.NewClientBuilder().Build(), - EventRecorder: record.NewFakeRecorder(32), - httpClient: client, - } - got, err := r.loadHelmChart(&sourcev1.HelmChart{ - Status: sourcev1.HelmChartStatus{ - Artifact: &sourcev1.Artifact{ - URL: "http://example.com" + chartPath, - Checksum: checksum, - }, - }, - }) - g.Expect(err).To(Not(HaveOccurred())) - g.Expect(got).ToNot(BeNil()) - }) -} - -func Test_copyAndVerifyArtifact(t *testing.T) { - g := NewWithT(t) - - tmpDir := t.TempDir() - closedF, err := os.CreateTemp(tmpDir, "closed.txt") - g.Expect(err).ToNot(HaveOccurred()) - g.Expect(closedF.Close()).ToNot(HaveOccurred()) - - tests := []struct { - name string - checksum string - in io.Reader - out io.Writer - wantErr bool - }{ - { - name: "checksum match", - checksum: "2c26b46b68ffc68ff99b453c1d30413413422d706483bfa0f98a5e886266e7ae", - in: bytes.NewReader([]byte("foo")), - out: io.Discard, - }, - { - name: "checksum mismatch", - checksum: "2c26b46b68ffc68ff99b453c1d30413413422d706483bfa0f98a5e886266e7ae", - in: bytes.NewReader([]byte("bar")), - out: io.Discard, - wantErr: true, - }, - { - name: "copy failure (closed file)", - checksum: "2c26b46b68ffc68ff99b453c1d30413413422d706483bfa0f98a5e886266e7ae", - in: bytes.NewReader([]byte("foo")), - out: closedF, - wantErr: true, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - g := NewWithT(t) - - err := copyAndVerifyArtifact(&sourcev1.Artifact{Checksum: tt.checksum}, tt.in, tt.out) - g.Expect(err != nil).To(Equal(tt.wantErr), err) - }) - } -} - -func Test_replaceHostname(t *testing.T) { - tests := []struct { - name string - URL string - hostname string - want string - wantErr bool - }{ - {"hostname overwrite", "https://example.com/file.txt", "overwrite.com", "https://overwrite.com/file.txt", false}, - {"hostname overwrite with port", "https://example.com:8080/file.txt", "overwrite.com:6666", "https://overwrite.com:6666/file.txt", false}, - {"invalid url", ":malformed./com", "", "", true}, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - g := NewWithT(t) - - got, err := replaceHostname(tt.URL, tt.hostname) - if tt.wantErr { - g.Expect(err).To(HaveOccurred()) - g.Expect(got).To(BeEmpty()) - return - } - g.Expect(err).ToNot(HaveOccurred()) - g.Expect(got).To(Equal(tt.want)) - }) - } -} diff --git a/controllers/helmrelease_controller_test.go b/controllers/helmrelease_controller_test.go index 020667a..1ec0774 100644 --- a/controllers/helmrelease_controller_test.go +++ b/controllers/helmrelease_controller_test.go @@ -18,276 +18,114 @@ package controllers import ( "context" - "reflect" "testing" - "github.com/go-logr/logr" - "helm.sh/helm/v3/pkg/chartutil" - corev1 "k8s.io/api/core/v1" - apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" + v2 "github.com/fluxcd/helm-controller/api/v2beta1" + sourcev1 "github.com/fluxcd/source-controller/api/v1beta2" + . "github.com/onsi/gomega" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/client-go/tools/record" "sigs.k8s.io/controller-runtime/pkg/client/fake" - "sigs.k8s.io/yaml" - - v2 "github.com/fluxcd/helm-controller/api/v2beta1" ) -func TestHelmReleaseReconciler_composeValues(t *testing.T) { - scheme := runtime.NewScheme() - _ = corev1.AddToScheme(scheme) - _ = v2.AddToScheme(scheme) +func TestHelmReleaseReconciler_getHelmChart(t *testing.T) { + g := NewWithT(t) - tests := []struct { - name string - resources []runtime.Object - references []v2.ValuesReference - values string - want chartutil.Values - wantErr bool - }{ - { - name: "merges", - resources: []runtime.Object{ - valuesConfigMap("values", map[string]string{ - "values.yaml": `flat: value -nested: - configuration: value -`, - }), - valuesSecret("values", map[string][]byte{ - "values.yaml": []byte(`flat: - nested: value -nested: value -`), - }), - }, - references: []v2.ValuesReference{ - { - Kind: "ConfigMap", - Name: "values", - }, - { - Kind: "Secret", - Name: "values", - }, - }, - values: ` -other: values -`, - want: chartutil.Values{ - "flat": map[string]interface{}{ - "nested": "value", - }, - "nested": "value", - "other": "values", - }, - }, - { - name: "target path", - resources: []runtime.Object{ - valuesSecret("values", map[string][]byte{"single": []byte("value")}), - }, - references: []v2.ValuesReference{ - { - Kind: "Secret", - Name: "values", - ValuesKey: "single", - TargetPath: "merge.at.specific.path", - }, - }, - want: chartutil.Values{ - "merge": map[string]interface{}{ - "at": map[string]interface{}{ - "specific": map[string]interface{}{ - "path": "value", - }, - }, - }, - }, - }, - { - name: "target path with boolean value", - resources: []runtime.Object{ - valuesSecret("values", map[string][]byte{"single": []byte("true")}), - }, - references: []v2.ValuesReference{ - { - Kind: "Secret", - Name: "values", - ValuesKey: "single", - TargetPath: "merge.at.specific.path", - }, - }, - want: chartutil.Values{ - "merge": map[string]interface{}{ - "at": map[string]interface{}{ - "specific": map[string]interface{}{ - "path": true, - }, - }, - }, - }, - }, - { - name: "target path with set-string behavior", - resources: []runtime.Object{ - valuesSecret("values", map[string][]byte{"single": []byte("\"true\"")}), - }, - references: []v2.ValuesReference{ - { - Kind: "Secret", - Name: "values", - ValuesKey: "single", - TargetPath: "merge.at.specific.path", - }, - }, - want: chartutil.Values{ - "merge": map[string]interface{}{ - "at": map[string]interface{}{ - "specific": map[string]interface{}{ - "path": "true", - }, - }, - }, - }, - }, - { - name: "values reference to non existing secret", - references: []v2.ValuesReference{ - { - Kind: "Secret", - Name: "missing", - }, - }, - wantErr: true, - }, - { - name: "optional values reference to non existing secret", - references: []v2.ValuesReference{ - { - Kind: "Secret", - Name: "missing", - Optional: true, - }, - }, - want: chartutil.Values{}, - wantErr: false, - }, - { - name: "values reference to non existing config map", - references: []v2.ValuesReference{ - { - Kind: "ConfigMap", - Name: "missing", - }, - }, - wantErr: true, - }, - { - name: "optional values reference to non existing config map", - references: []v2.ValuesReference{ - { - Kind: "ConfigMap", - Name: "missing", - Optional: true, - }, - }, - want: chartutil.Values{}, - wantErr: false, - }, - { - name: "missing secret key", - resources: []runtime.Object{ - valuesSecret("values", nil), - }, - references: []v2.ValuesReference{ - { - Kind: "Secret", - Name: "values", - ValuesKey: "nonexisting", - }, - }, - wantErr: true, - }, - { - name: "missing config map key", - resources: []runtime.Object{ - valuesConfigMap("values", nil), - }, - references: []v2.ValuesReference{ - { - Kind: "ConfigMap", - Name: "values", - ValuesKey: "nonexisting", - }, - }, - wantErr: true, - }, - { - name: "unsupported values reference kind", - references: []v2.ValuesReference{ - { - Kind: "Unsupported", - }, - }, - wantErr: true, - }, - { - name: "invalid values", - resources: []runtime.Object{ - valuesConfigMap("values", map[string]string{ - "values.yaml": ` -invalid`, - }), - }, - references: []v2.ValuesReference{ - { - Kind: "ConfigMap", - Name: "values", - }, - }, - wantErr: true, + scheme := runtime.NewScheme() + g.Expect(v2.AddToScheme(scheme)).To(Succeed()) + g.Expect(sourcev1.AddToScheme(scheme)).To(Succeed()) + + chart := &sourcev1.HelmChart{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "some-namespace", + Name: "some-chart-name", }, } + tests := []struct { + name string + rel *v2.HelmRelease + chart *sourcev1.HelmChart + expectChart bool + wantErr bool + disallowCrossNS bool + }{ + { + name: "retrieves HelmChart object from Status", + rel: &v2.HelmRelease{ + Status: v2.HelmReleaseStatus{ + HelmChart: "some-namespace/some-chart-name", + }, + }, + chart: chart, + expectChart: true, + }, + { + name: "no HelmChart found", + rel: &v2.HelmRelease{ + Status: v2.HelmReleaseStatus{ + HelmChart: "some-namespace/some-chart-name", + }, + }, + chart: nil, + expectChart: false, + wantErr: true, + }, + { + name: "no HelmChart in Status", + rel: &v2.HelmRelease{ + Status: v2.HelmReleaseStatus{ + HelmChart: "", + }, + }, + chart: chart, + expectChart: false, + wantErr: true, + }, + { + name: "ACL disallows cross namespace", + rel: &v2.HelmRelease{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "default", + }, + Status: v2.HelmReleaseStatus{ + HelmChart: "some-namespace/some-chart-name", + }, + }, + chart: chart, + expectChart: false, + wantErr: true, + disallowCrossNS: true, + }, + } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - c := fake.NewFakeClientWithScheme(scheme, tt.resources...) - r := &HelmReleaseReconciler{Client: c} - var values *apiextensionsv1.JSON - if tt.values != "" { - v, _ := yaml.YAMLToJSON([]byte(tt.values)) - values = &apiextensionsv1.JSON{Raw: v} + builder := fake.NewClientBuilder() + builder.WithScheme(scheme) + if tt.chart != nil { + builder.WithObjects(tt.chart) } - hr := v2.HelmRelease{ - Spec: v2.HelmReleaseSpec{ - ValuesFrom: tt.references, - Values: values, - }, + + r := &HelmReleaseReconciler{ + Client: builder.Build(), + EventRecorder: record.NewFakeRecorder(32), + NoCrossNamespaceRef: tt.disallowCrossNS, } - got, err := r.composeValues(logr.NewContext(context.TODO(), logr.Discard()), hr) - if (err != nil) != tt.wantErr { - t.Errorf("composeValues() error = %v, wantErr %v", err, tt.wantErr) + + got, err := r.getHelmChart(context.TODO(), tt.rel) + if tt.wantErr { + g.Expect(err).To(HaveOccurred()) + g.Expect(got).To(BeNil()) return } - if !reflect.DeepEqual(got, tt.want) { - t.Errorf("composeValues() got = %v, want %v", got, tt.want) + g.Expect(err).ToNot(HaveOccurred()) + expect := g.Expect(got.ObjectMeta) + if tt.expectChart { + expect.To(BeEquivalentTo(tt.chart.ObjectMeta)) + } else { + expect.To(BeNil()) } }) } } - -func valuesSecret(name string, data map[string][]byte) *corev1.Secret { - return &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{Name: name}, - Data: data, - } -} - -func valuesConfigMap(name string, data map[string]string) *corev1.ConfigMap { - return &corev1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{Name: name}, - Data: data, - } -} diff --git a/go.mod b/go.mod index 4a023a3..bd6ac76 100644 --- a/go.mod +++ b/go.mod @@ -24,6 +24,7 @@ require ( k8s.io/apimachinery v0.23.6 k8s.io/cli-runtime v0.23.6 k8s.io/client-go v0.23.6 + k8s.io/utils v0.0.0-20220210201930-3a6ce19ff2f9 sigs.k8s.io/controller-runtime v0.11.2 sigs.k8s.io/kustomize/api v0.11.4 sigs.k8s.io/yaml v1.3.0 @@ -165,7 +166,6 @@ require ( k8s.io/klog/v2 v2.50.0 // indirect k8s.io/kube-openapi v0.0.0-20211115234752-e816edb12b65 // indirect k8s.io/kubectl v0.23.5 // indirect - k8s.io/utils v0.0.0-20220210201930-3a6ce19ff2f9 // indirect oras.land/oras-go v1.1.1 // indirect sigs.k8s.io/json v0.0.0-20211208200746-9f7c6b3444d2 // indirect sigs.k8s.io/kustomize/kyaml v0.13.6 // indirect diff --git a/internal/chartutil/values.go b/internal/chartutil/values.go new file mode 100644 index 0000000..34c4d27 --- /dev/null +++ b/internal/chartutil/values.go @@ -0,0 +1,271 @@ +/* +Copyright 2022 The Flux authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package chartutil + +import ( + "context" + "errors" + "fmt" + "strings" + + "helm.sh/helm/v3/pkg/chartutil" + "helm.sh/helm/v3/pkg/strvals" + corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/types" + ctrl "sigs.k8s.io/controller-runtime" + kubeclient "sigs.k8s.io/controller-runtime/pkg/client" + + "github.com/fluxcd/pkg/runtime/transform" + + "github.com/fluxcd/helm-controller/api/v2beta1" +) + +// ErrValuesRefReason is the descriptive reason for an ErrValuesReference. +type ErrValuesRefReason error + +var ( + // ErrResourceNotFound signals the referenced values resource could not be + // found. + ErrResourceNotFound = errors.New("resource not found") + // ErrKeyNotFound signals the key could not be found in the referenced + // values resource. + ErrKeyNotFound = errors.New("key not found") + // ErrUnsupportedRefKind signals the values reference kind is not + // supported. + ErrUnsupportedRefKind = errors.New("unsupported values reference kind") + // ErrValuesDataRead signals the referenced resource's values data could + // not be read. + ErrValuesDataRead = errors.New("failed to read values data") + // ErrValueMerge signals a single value could not be merged into the + // values. + ErrValueMerge = errors.New("failed to merge value") + // ErrUnknown signals the reason an error occurred is unknown. + ErrUnknown = errors.New("unknown error") +) + +// ErrValuesReference is returned by ChartValuesFromReferences +type ErrValuesReference struct { + // Reason for the values reference error. Nil equals ErrUnknown. + // Can be used with Is to reason about a returned error: + // err := &ErrValuesReference{Reason: ErrResourceNotFound, ...} + // errors.Is(err, ErrResourceNotFound) + Reason ErrValuesRefReason + // Kind of the values reference the error is being reported for. + Kind string + // Name of the values reference the error is being reported for. + Name types.NamespacedName + // Key of the values reference the error is being reported for. + Key string + // Optional indicates if the error is being reported for an optional values + // reference. + Optional bool + // Err contains the further error chain leading to this error, it can be + // nil. + Err error +} + +// Error returns an error string constructed out of the state of +// ErrValuesReference. +func (e *ErrValuesReference) Error() string { + b := strings.Builder{} + b.WriteString("could not resolve") + if e.Optional { + b.WriteString(" optional") + } + if kind := e.Kind; kind != "" { + b.WriteString(" " + kind) + } + b.WriteString(" chart values reference") + if name := e.Name.String(); name != "" { + b.WriteString(fmt.Sprintf(" '%s'", name)) + } + if key := e.Key; key != "" { + b.WriteString(fmt.Sprintf(" with key '%s'", key)) + } + reason := e.Reason.Error() + if reason == "" && e.Err == nil { + reason = ErrUnknown.Error() + } + if e.Err != nil { + reason = e.Err.Error() + } + b.WriteString(": " + reason) + return b.String() +} + +// Is returns if target == Reason, or target == Err. +// Can be used to Reason about a returned error: +// err := &ErrValuesReference{Reason: ErrResourceNotFound, ...} +// errors.Is(err, ErrResourceNotFound) +func (e *ErrValuesReference) Is(target error) bool { + reason := e.Reason + if reason == nil { + reason = ErrUnknown + } + if reason == target { + return true + } + return errors.Is(e.Err, target) +} + +// Unwrap returns the wrapped Err. +func (e *ErrValuesReference) Unwrap() error { + return e.Err +} + +// NewErrValuesReference returns a new ErrValuesReference constructed from the +// provided values. +func NewErrValuesReference(name types.NamespacedName, ref v2beta1.ValuesReference, reason ErrValuesRefReason, err error) *ErrValuesReference { + return &ErrValuesReference{ + Reason: reason, + Kind: ref.Kind, + Name: name, + Key: ref.GetValuesKey(), + Optional: ref.Optional, + Err: err, + } +} + +const ( + kindConfigMap = "ConfigMap" + kindSecret = "Secret" +) + +// ChartValuesFromReferences attempts to construct new chart values by resolving +// the provided references using the client, merging them in the order given. +// If provided, the values map is merged in last. Overwriting values from +// references. It returns the merged values, or an ErrValuesReference error. +func ChartValuesFromReferences(ctx context.Context, client kubeclient.Client, namespace string, + values map[string]interface{}, refs ...v2beta1.ValuesReference) (chartutil.Values, error) { + + log := ctrl.LoggerFrom(ctx) + + result := chartutil.Values{} + resources := make(map[string]kubeclient.Object) + + for _, ref := range refs { + namespacedName := types.NamespacedName{Namespace: namespace, Name: ref.Name} + var valuesData []byte + + switch ref.Kind { + case kindConfigMap, kindSecret: + index := ref.Kind + namespacedName.String() + + resource, ok := resources[index] + if !ok { + // The resource may not exist, but we want to act on a single version + // of the resource in case the values reference is marked as optional. + resources[index] = nil + + switch ref.Kind { + case kindSecret: + resource = &corev1.Secret{} + case kindConfigMap: + resource = &corev1.ConfigMap{} + } + + if resource != nil { + if err := client.Get(ctx, namespacedName, resource); err != nil { + if apierrors.IsNotFound(err) { + err := NewErrValuesReference(namespacedName, ref, ErrResourceNotFound, err) + if err.Optional { + log.Info(err.Error()) + continue + } + return nil, err + } + return nil, err + } + resources[index] = resource + } + } + + if resource == nil { + if ref.Optional { + continue + } + return nil, NewErrValuesReference(namespacedName, ref, ErrResourceNotFound, nil) + } + + switch resource.(type) { + case *corev1.Secret: + data, ok := resource.(*corev1.Secret).Data[ref.GetValuesKey()] + if !ok { + err := NewErrValuesReference(namespacedName, ref, ErrKeyNotFound, nil) + if ref.Optional { + log.Info(err.Error()) + continue + } + return nil, NewErrValuesReference(namespacedName, ref, ErrKeyNotFound, nil) + } + valuesData = data + case *corev1.ConfigMap: + data, ok := resource.(*corev1.ConfigMap).Data[ref.GetValuesKey()] + if !ok { + err := NewErrValuesReference(namespacedName, ref, ErrKeyNotFound, nil) + if ref.Optional { + log.Info(err.Error()) + continue + } + return nil, err + } + valuesData = []byte(data) + default: + return nil, NewErrValuesReference(namespacedName, ref, ErrUnsupportedRefKind, nil) + } + default: + return nil, NewErrValuesReference(namespacedName, ref, ErrUnsupportedRefKind, nil) + } + + if ref.TargetPath != "" { + // TODO(hidde): this is a bit of hack, as it mimics the way the option string is passed + // to Helm from a CLI perspective. Given the parser is however not publicly accessible + // while it contains all logic around parsing the target path, it is a fair trade-off. + if err := ReplacePathValue(result, ref.TargetPath, string(valuesData)); err != nil { + return nil, NewErrValuesReference(namespacedName, ref, ErrValueMerge, err) + } + continue + } + + values, err := chartutil.ReadValues(valuesData) + if err != nil { + return nil, NewErrValuesReference(namespacedName, ref, ErrValuesDataRead, err) + } + result = transform.MergeMaps(result, values) + } + return transform.MergeMaps(result, values), nil +} + +// ReplacePathValue replaces the value at the dot notation path with the given +// value using Helm's string value parser using strvals.ParseInto. Single or +// double-quoted values are merged using strvals.ParseIntoString. +func ReplacePathValue(values chartutil.Values, path string, value string) error { + const ( + singleQuote = "'" + doubleQuote = `"` + ) + isSingleQuoted := strings.HasPrefix(value, singleQuote) && strings.HasSuffix(value, singleQuote) + isDoubleQuoted := strings.HasPrefix(value, doubleQuote) && strings.HasSuffix(value, doubleQuote) + if isSingleQuoted || isDoubleQuoted { + value = strings.Trim(value, singleQuote+doubleQuote) + value = path + "=" + value + return strvals.ParseIntoString(value, values) + } + value = path + "=" + value + return strvals.ParseInto(value, values) +} diff --git a/internal/chartutil/values_test.go b/internal/chartutil/values_test.go new file mode 100644 index 0000000..b8034ed --- /dev/null +++ b/internal/chartutil/values_test.go @@ -0,0 +1,390 @@ +/* +Copyright 2022 The Flux authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package chartutil + +import ( + "context" + "testing" + + v2 "github.com/fluxcd/helm-controller/api/v2beta1" + "github.com/go-logr/logr" + . "github.com/onsi/gomega" + "helm.sh/helm/v3/pkg/chartutil" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "sigs.k8s.io/controller-runtime/pkg/client/fake" +) + +func TestChartValuesFromReferences(t *testing.T) { + scheme := runtime.NewScheme() + _ = corev1.AddToScheme(scheme) + _ = v2.AddToScheme(scheme) + + tests := []struct { + name string + resources []runtime.Object + namespace string + references []v2.ValuesReference + values string + want chartutil.Values + wantErr bool + }{ + { + name: "merges", + resources: []runtime.Object{ + mockConfigMap("values", map[string]string{ + "values.yaml": `flat: value +nested: + configuration: value +`, + }), + mockSecret("values", map[string][]byte{ + "values.yaml": []byte(`flat: + nested: value +nested: value +`), + }), + }, + references: []v2.ValuesReference{ + { + Kind: kindConfigMap, + Name: "values", + }, + { + Kind: kindSecret, + Name: "values", + }, + }, + values: ` +other: values +`, + want: chartutil.Values{ + "flat": map[string]interface{}{ + "nested": "value", + }, + "nested": "value", + "other": "values", + }, + }, + { + name: "with target path", + resources: []runtime.Object{ + mockSecret("values", map[string][]byte{"single": []byte("value")}), + }, + references: []v2.ValuesReference{ + { + Kind: kindSecret, + Name: "values", + ValuesKey: "single", + TargetPath: "merge.at.specific.path", + }, + }, + want: chartutil.Values{ + "merge": map[string]interface{}{ + "at": map[string]interface{}{ + "specific": map[string]interface{}{ + "path": "value", + }, + }, + }, + }, + }, + { + name: "target path for string type array item", + resources: []runtime.Object{ + mockConfigMap("values", map[string]string{ + "values.yaml": `flat: value +nested: + configuration: + - list + - item + - option +`, + }), + mockSecret("values", map[string][]byte{ + "values.yaml": []byte(`foo`), + }), + }, + references: []v2.ValuesReference{ + { + Kind: kindConfigMap, + Name: "values", + }, + { + Kind: kindSecret, + Name: "values", + TargetPath: "nested.configuration[1]", + }, + }, + values: ` +other: values +`, + want: chartutil.Values{ + "flat": "value", + "nested": map[string]interface{}{ + "configuration": []interface{}{"list", "foo", "option"}, + }, + "other": "values", + }, + }, + { + name: "values reference to non existing secret", + references: []v2.ValuesReference{ + { + Kind: kindSecret, + Name: "missing", + }, + }, + wantErr: true, + }, + { + name: "optional values reference to non existing secret", + references: []v2.ValuesReference{ + { + Kind: kindSecret, + Name: "missing", + Optional: true, + }, + }, + want: chartutil.Values{}, + wantErr: false, + }, + { + name: "values reference to non existing config map", + references: []v2.ValuesReference{ + { + Kind: kindConfigMap, + Name: "missing", + }, + }, + wantErr: true, + }, + { + name: "optional values reference to non existing config map", + references: []v2.ValuesReference{ + { + Kind: kindConfigMap, + Name: "missing", + Optional: true, + }, + }, + want: chartutil.Values{}, + wantErr: false, + }, + { + name: "missing secret key", + resources: []runtime.Object{ + mockSecret("values", nil), + }, + references: []v2.ValuesReference{ + { + Kind: kindSecret, + Name: "values", + ValuesKey: "nonexisting", + }, + }, + wantErr: true, + }, + { + name: "missing config map key", + resources: []runtime.Object{ + mockConfigMap("values", nil), + }, + references: []v2.ValuesReference{ + { + Kind: kindConfigMap, + Name: "values", + ValuesKey: "nonexisting", + }, + }, + wantErr: true, + }, + { + name: "unsupported values reference kind", + references: []v2.ValuesReference{ + { + Kind: "Unsupported", + }, + }, + wantErr: true, + }, + { + name: "invalid values", + resources: []runtime.Object{ + mockConfigMap("values", map[string]string{ + "values.yaml": ` +invalid`, + }), + }, + references: []v2.ValuesReference{ + { + Kind: kindConfigMap, + Name: "values", + }, + }, + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + + c := fake.NewClientBuilder().WithScheme(scheme).WithRuntimeObjects(tt.resources...) + var values map[string]interface{} + if tt.values != "" { + m, err := chartutil.ReadValues([]byte(tt.values)) + g.Expect(err).ToNot(HaveOccurred()) + values = m + } + ctx := logr.NewContext(context.TODO(), logr.Discard()) + got, err := ChartValuesFromReferences(ctx, c.Build(), tt.namespace, values, tt.references...) + if tt.wantErr { + g.Expect(err).To(HaveOccurred()) + g.Expect(got).To(BeNil()) + return + } + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(got).To(Equal(tt.want)) + }) + } +} + +// This tests compatability with the formats described in: +// https://helm.sh/docs/intro/using_helm/#the-format-and-limitations-of---set +func TestReplacePathValue(t *testing.T) { + tests := []struct { + name string + value []byte + path string + want map[string]interface{} + wantErr bool + }{ + { + name: "outer inner", + value: []byte("value"), + path: "outer.inner", + want: map[string]interface{}{ + "outer": map[string]interface{}{ + "inner": "value", + }, + }, + }, + { + name: "inline list", + value: []byte("{a,b,c}"), + path: "name", + want: map[string]interface{}{ + // TODO(hidde): figure out why the cap is off by len+1 + "name": append(make([]interface{}, 0, 4), []interface{}{"a", "b", "c"}...), + }, + }, + { + name: "with escape", + value: []byte(`value1\,value2`), + path: "name", + want: map[string]interface{}{ + "name": "value1,value2", + }, + }, + { + name: "target path with boolean value", + value: []byte("true"), + path: "merge.at.specific.path", + want: chartutil.Values{ + "merge": map[string]interface{}{ + "at": map[string]interface{}{ + "specific": map[string]interface{}{ + "path": true, + }, + }, + }, + }, + }, + { + name: "target path with set-string behavior", + value: []byte(`"true"`), + path: "merge.at.specific.path", + want: chartutil.Values{ + "merge": map[string]interface{}{ + "at": map[string]interface{}{ + "specific": map[string]interface{}{ + "path": "true", + }, + }, + }, + }, + }, + { + name: "target path with array item", + value: []byte("value"), + path: "merge.at[2]", + want: chartutil.Values{ + "merge": map[string]interface{}{ + "at": []interface{}{nil, nil, "value"}, + }, + }, + }, + { + name: "dot sequence escaping path", + value: []byte("master"), + path: `nodeSelector.kubernetes\.io/role`, + want: map[string]interface{}{ + "nodeSelector": map[string]interface{}{ + "kubernetes.io/role": "master", + }, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + values := map[string]interface{}{} + err := ReplacePathValue(values, tt.path, string(tt.value)) + if tt.wantErr { + g.Expect(err).To(HaveOccurred()) + g.Expect(values).To(BeNil()) + return + } + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(values).To(Equal(tt.want)) + }) + } +} + +func mockSecret(name string, data map[string][]byte) *corev1.Secret { + return &corev1.Secret{ + TypeMeta: metav1.TypeMeta{ + Kind: kindSecret, + APIVersion: "v1", + }, + ObjectMeta: metav1.ObjectMeta{Name: name}, + Data: data, + } +} + +func mockConfigMap(name string, data map[string]string) *corev1.ConfigMap { + return &corev1.ConfigMap{ + TypeMeta: metav1.TypeMeta{ + Kind: kindConfigMap, + APIVersion: "v1", + }, + ObjectMeta: metav1.ObjectMeta{Name: name}, + Data: data, + } +} diff --git a/internal/kube/builder.go b/internal/kube/builder.go new file mode 100644 index 0000000..de6928b --- /dev/null +++ b/internal/kube/builder.go @@ -0,0 +1,86 @@ +/* +Copyright 2022 The Flux authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package kube + +import ( + "k8s.io/cli-runtime/pkg/genericclioptions" + "k8s.io/client-go/rest" + + "github.com/fluxcd/pkg/runtime/client" +) + +const ( + // DefaultKubeConfigSecretKey is the default data key ConfigFromSecret + // looks at when no data key is provided. + DefaultKubeConfigSecretKey = "value" + // DefaultKubeConfigSecretKeyExt is the default data key ConfigFromSecret + // looks at when no data key is provided, and DefaultKubeConfigSecretKey + // does not exist. + DefaultKubeConfigSecretKeyExt = DefaultKubeConfigSecretKey + ".yaml" +) + +// clientGetterOptions used to BuildClientGetter. +type clientGetterOptions struct { + config *rest.Config + namespace string + kubeConfig []byte + burst int + qps float32 + impersonateAccount string + kubeConfigOptions client.KubeConfigOptions +} + +// ClientGetterOption configures a genericclioptions.RESTClientGetter. +type ClientGetterOption func(o *clientGetterOptions) + +// WithKubeConfig creates a MemoryRESTClientGetter configured with the provided +// KubeConfig and other values. +func WithKubeConfig(kubeConfig []byte, qps float32, burst int, opts client.KubeConfigOptions) func(o *clientGetterOptions) { + return func(o *clientGetterOptions) { + o.kubeConfig = kubeConfig + o.qps = qps + o.burst = burst + o.kubeConfigOptions = opts + } +} + +// WithImpersonate configures the genericclioptions.RESTClientGetter to +// impersonate the provided account name. +func WithImpersonate(accountName string) func(o *clientGetterOptions) { + return func(o *clientGetterOptions) { + o.impersonateAccount = accountName + } +} + +// BuildClientGetter builds a genericclioptions.RESTClientGetter based on the +// provided options and returns the result. config and namespace are mandatory, +// and not expected to be nil or empty. +func BuildClientGetter(config *rest.Config, namespace string, opts ...ClientGetterOption) genericclioptions.RESTClientGetter { + o := &clientGetterOptions{ + config: config, + namespace: namespace, + } + for _, opt := range opts { + opt(o) + } + if len(o.kubeConfig) > 0 { + return NewMemoryRESTClientGetter(o.kubeConfig, namespace, o.impersonateAccount, o.qps, o.burst, o.kubeConfigOptions) + } + cfg := *config + SetImpersonationConfig(&cfg, namespace, o.impersonateAccount) + return NewInClusterRESTClientGetter(&cfg, namespace) +} diff --git a/internal/kube/builder_test.go b/internal/kube/builder_test.go new file mode 100644 index 0000000..20c9b5a --- /dev/null +++ b/internal/kube/builder_test.go @@ -0,0 +1,109 @@ +/* +Copyright 2022 The Flux authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package kube + +import ( + "testing" + + "github.com/fluxcd/pkg/runtime/client" + . "github.com/onsi/gomega" + "k8s.io/cli-runtime/pkg/genericclioptions" + "k8s.io/client-go/rest" +) + +func TestBuildClientGetter(t *testing.T) { + t.Run("with config and namespace", func(t *testing.T) { + g := NewWithT(t) + + cfg := &rest.Config{ + BearerToken: "a-token", + } + namespace := "a-namespace" + getter := BuildClientGetter(cfg, namespace) + g.Expect(getter).To(BeAssignableToTypeOf(&genericclioptions.ConfigFlags{})) + + flags := getter.(*genericclioptions.ConfigFlags) + g.Expect(flags.BearerToken).ToNot(BeNil()) + g.Expect(*flags.BearerToken).To(Equal(cfg.BearerToken)) + g.Expect(flags.Namespace).ToNot(BeNil()) + g.Expect(*flags.Namespace).To(Equal(namespace)) + }) + + t.Run("with kubeconfig and impersonate", func(t *testing.T) { + g := NewWithT(t) + + namespace := "a-namespace" + cfg := []byte(`apiVersion: v1 +clusters: +- cluster: + server: https://example.com + name: example-cluster +contexts: +- context: + cluster: example-cluster + namespace: flux-system +kind: Config +preferences: {} +users:`) + qps := float32(600) + burst := 1000 + cfgOpts := client.KubeConfigOptions{InsecureTLS: true} + + impersonate := "jane" + + getter := BuildClientGetter(&rest.Config{}, namespace, WithKubeConfig(cfg, qps, burst, cfgOpts), WithImpersonate(impersonate)) + g.Expect(getter).To(BeAssignableToTypeOf(&MemoryRESTClientGetter{})) + + got := getter.(*MemoryRESTClientGetter) + g.Expect(got.namespace).To(Equal(namespace)) + g.Expect(got.kubeConfig).To(Equal(cfg)) + g.Expect(got.qps).To(Equal(qps)) + g.Expect(got.burst).To(Equal(burst)) + g.Expect(got.kubeConfigOpts).To(Equal(cfgOpts)) + g.Expect(got.impersonateAccount).To(Equal(impersonate)) + }) + + t.Run("with config and impersonate account", func(t *testing.T) { + g := NewWithT(t) + + namespace := "a-namespace" + impersonate := "frank" + getter := BuildClientGetter(&rest.Config{}, namespace, WithImpersonate(impersonate)) + g.Expect(getter).To(BeAssignableToTypeOf(&genericclioptions.ConfigFlags{})) + + flags := getter.(*genericclioptions.ConfigFlags) + g.Expect(flags.Namespace).ToNot(BeNil()) + g.Expect(*flags.Namespace).To(Equal(namespace)) + g.Expect(flags.Impersonate).ToNot(BeNil()) + g.Expect(*flags.Impersonate).To(Equal("system:serviceaccount:a-namespace:frank")) + }) + + t.Run("with config and DefaultServiceAccount", func(t *testing.T) { + g := NewWithT(t) + + namespace := "a-namespace" + DefaultServiceAccountName = "frank" + getter := BuildClientGetter(&rest.Config{}, namespace) + g.Expect(getter).To(BeAssignableToTypeOf(&genericclioptions.ConfigFlags{})) + + flags := getter.(*genericclioptions.ConfigFlags) + g.Expect(flags.Namespace).ToNot(BeNil()) + g.Expect(*flags.Namespace).To(Equal(namespace)) + g.Expect(flags.Impersonate).ToNot(BeNil()) + g.Expect(*flags.Impersonate).To(Equal("system:serviceaccount:a-namespace:frank")) + }) +} diff --git a/internal/kube/client.go b/internal/kube/client.go index b8f35ec..98e5aba 100644 --- a/internal/kube/client.go +++ b/internal/kube/client.go @@ -24,53 +24,69 @@ import ( "k8s.io/client-go/rest" "k8s.io/client-go/restmapper" "k8s.io/client-go/tools/clientcmd" + "k8s.io/utils/pointer" "github.com/fluxcd/pkg/runtime/client" ) +// NewInClusterRESTClientGetter creates a new genericclioptions.RESTClientGetter +// using genericclioptions.NewConfigFlags, and configures it with the server, +// authentication, impersonation, and burst and QPS settings, and the provided +// namespace. func NewInClusterRESTClientGetter(cfg *rest.Config, namespace string) genericclioptions.RESTClientGetter { flags := genericclioptions.NewConfigFlags(false) - flags.APIServer = &cfg.Host - flags.BearerToken = &cfg.BearerToken - flags.CAFile = &cfg.CAFile - flags.Namespace = &namespace + flags.APIServer = pointer.String(cfg.Host) + flags.BearerToken = pointer.String(cfg.BearerToken) + flags.CAFile = pointer.String(cfg.CAFile) + flags.Namespace = pointer.String(namespace) flags.WithDiscoveryBurst(cfg.Burst) flags.WithDiscoveryQPS(cfg.QPS) if sa := cfg.Impersonate.UserName; sa != "" { - flags.Impersonate = &sa + flags.Impersonate = pointer.String(sa) } - return flags } // MemoryRESTClientGetter is an implementation of the genericclioptions.RESTClientGetter, // capable of working with an in-memory kubeconfig file. type MemoryRESTClientGetter struct { - kubeConfig []byte - namespace string + // kubeConfig used to load a rest.Config, after being sanitized. + kubeConfig []byte + // kubeConfigOpts control the sanitization of the kubeConfig. + kubeConfigOpts client.KubeConfigOptions + // namespace specifies the namespace the client is configured to. + namespace string + // impersonateAccount configures the rest.ImpersonationConfig account name. impersonateAccount string - qps float32 - burst int - kubeConfigOpts client.KubeConfigOptions + // qps configures the QPS on the discovery.DiscoveryClient. + qps float32 + // burst configures the burst on the discovery.DiscoveryClient. + burst int } +// NewMemoryRESTClientGetter returns a MemoryRESTClientGetter configured with +// the provided values and client.KubeConfigOptions. The provided KubeConfig is +// sanitized, configure the settings for this using client.KubeConfigOptions. func NewMemoryRESTClientGetter( kubeConfig []byte, namespace string, - impersonateAccount string, + impersonate string, qps float32, burst int, kubeConfigOpts client.KubeConfigOptions) genericclioptions.RESTClientGetter { return &MemoryRESTClientGetter{ kubeConfig: kubeConfig, namespace: namespace, - impersonateAccount: impersonateAccount, + impersonateAccount: impersonate, qps: qps, burst: burst, kubeConfigOpts: kubeConfigOpts, } } +// ToRESTConfig creates a rest.Config with the rest.ImpersonationConfig configured +// with to the impersonation account. It loads the config the KubeConfig bytes and +// sanitizes it using the client.KubeConfigOptions. func (c *MemoryRESTClientGetter) ToRESTConfig() (*rest.Config, error) { cfg, err := clientcmd.RESTConfigFromKubeConfig(c.kubeConfig) if err != nil { @@ -83,23 +99,25 @@ func (c *MemoryRESTClientGetter) ToRESTConfig() (*rest.Config, error) { return cfg, nil } +// ToDiscoveryClient returns a discovery.CachedDiscoveryInterface configured +// with ToRESTConfig, and the QPS and Burst settings. func (c *MemoryRESTClientGetter) ToDiscoveryClient() (discovery.CachedDiscoveryInterface, error) { config, err := c.ToRESTConfig() if err != nil { return nil, err } - if c.impersonateAccount != "" { - config.Impersonate = rest.ImpersonationConfig{UserName: c.impersonateAccount} - } - config.QPS = c.qps config.Burst = c.burst - discoveryClient, _ := discovery.NewDiscoveryClientForConfig(config) + discoveryClient, err := discovery.NewDiscoveryClientForConfig(config) + if err != nil { + return nil, err + } return memory.NewMemCacheClient(discoveryClient), nil } +// ToRESTMapper returns a RESTMapper constructed from ToDiscoveryClient. func (c *MemoryRESTClientGetter) ToRESTMapper() (meta.RESTMapper, error) { discoveryClient, err := c.ToDiscoveryClient() if err != nil { @@ -111,6 +129,9 @@ func (c *MemoryRESTClientGetter) ToRESTMapper() (meta.RESTMapper, error) { return expander, nil } +// ToRawKubeConfigLoader returns a clientcmd.ClientConfig using +// clientcmd.DefaultClientConfig. With clientcmd.ClusterDefaults, namespace, and +// impersonate configured as overwrites. func (c *MemoryRESTClientGetter) ToRawKubeConfigLoader() clientcmd.ClientConfig { loadingRules := clientcmd.NewDefaultClientConfigLoadingRules() // use the standard defaults for this client command diff --git a/internal/kube/client_test.go b/internal/kube/client_test.go new file mode 100644 index 0000000..a232f7a --- /dev/null +++ b/internal/kube/client_test.go @@ -0,0 +1,179 @@ +/* +Copyright 2022 The Flux authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package kube + +import ( + "testing" + + "github.com/fluxcd/pkg/runtime/client" + . "github.com/onsi/gomega" + "k8s.io/cli-runtime/pkg/genericclioptions" + "k8s.io/client-go/rest" +) + +var cfg = []byte(`current-context: federal-context +apiVersion: v1 +clusters: +- cluster: + api-version: v1 + server: http://cow.org:8080 + insecure-skip-tls-verify: true + name: cow-cluster +contexts: +- context: + cluster: cow-cluster + user: blue-user + name: federal-context +kind: Config +users: +- name: blue-user + user: + token: foo`) + +func TestNewInClusterRESTClientGetter(t *testing.T) { + t.Run("api server config", func(t *testing.T) { + g := NewWithT(t) + + cfg := &rest.Config{ + Host: "https://example.com", + BearerToken: "chase-the-honey", + TLSClientConfig: rest.TLSClientConfig{ + CAFile: "afile", + }, + } + + got := NewInClusterRESTClientGetter(cfg, "") + g.Expect(got).To(BeAssignableToTypeOf(&genericclioptions.ConfigFlags{})) + + flags := got.(*genericclioptions.ConfigFlags) + fields := map[*string]*string{ + flags.APIServer: &cfg.Host, + flags.BearerToken: &cfg.BearerToken, + flags.CAFile: &cfg.CAFile, + } + for f, ff := range fields { + g.Expect(f).ToNot(BeNil()) + g.Expect(f).To(Equal(ff)) + g.Expect(f).ToNot(BeIdenticalTo(ff)) + } + }) + + t.Run("namespace", func(t *testing.T) { + g := NewWithT(t) + + got := NewInClusterRESTClientGetter(&rest.Config{}, "a-space") + g.Expect(got).To(BeAssignableToTypeOf(&genericclioptions.ConfigFlags{})) + + flags := got.(*genericclioptions.ConfigFlags) + g.Expect(flags.Namespace).ToNot(BeNil()) + g.Expect(*flags.Namespace).To(Equal("a-space")) + }) + + t.Run("impersonation", func(t *testing.T) { + g := NewWithT(t) + + cfg := &rest.Config{ + Impersonate: rest.ImpersonationConfig{ + UserName: "system:serviceaccount:namespace:foo", + }, + } + + got := NewInClusterRESTClientGetter(cfg, "") + g.Expect(got).To(BeAssignableToTypeOf(&genericclioptions.ConfigFlags{})) + + flags := got.(*genericclioptions.ConfigFlags) + g.Expect(flags.Impersonate).ToNot(BeNil()) + g.Expect(*flags.Impersonate).To(Equal(cfg.Impersonate.UserName)) + }) +} + +func TestMemoryRESTClientGetter_ToRESTConfig(t *testing.T) { + t.Run("loads REST config from KubeConfig", func(t *testing.T) { + g := NewWithT(t) + getter := NewMemoryRESTClientGetter(cfg, "", "", 0, 0, client.KubeConfigOptions{}) + + got, err := getter.ToRESTConfig() + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(got.Host).To(Equal("http://cow.org:8080")) + g.Expect(got.TLSClientConfig.Insecure).To(BeFalse()) + }) + + t.Run("sets ImpersonationConfig", func(t *testing.T) { + g := NewWithT(t) + getter := NewMemoryRESTClientGetter(cfg, "", "someone", 0, 0, client.KubeConfigOptions{}) + + got, err := getter.ToRESTConfig() + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(got.Impersonate.UserName).To(Equal("someone")) + }) + + t.Run("uses KubeConfigOptions", func(t *testing.T) { + g := NewWithT(t) + + agent := "a static string forever," + + "but static strings can have dreams and hope too" + getter := NewMemoryRESTClientGetter(cfg, "", "someone", 0, 0, client.KubeConfigOptions{ + UserAgent: agent, + }) + + got, err := getter.ToRESTConfig() + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(got.UserAgent).To(Equal(agent)) + }) + + t.Run("invalid config", func(t *testing.T) { + g := NewWithT(t) + + getter := NewMemoryRESTClientGetter([]byte(`invalid`), "", "", 0, 0, client.KubeConfigOptions{}) + got, err := getter.ToRESTConfig() + g.Expect(err).To(HaveOccurred()) + g.Expect(got).To(BeNil()) + }) +} + +func TestMemoryRESTClientGetter_ToDiscoveryClient(t *testing.T) { + g := NewWithT(t) + + getter := NewMemoryRESTClientGetter(cfg, "", "", 400, 800, client.KubeConfigOptions{}) + got, err := getter.ToDiscoveryClient() + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(got).ToNot(BeNil()) +} + +func TestMemoryRESTClientGetter_ToRESTMapper(t *testing.T) { + g := NewWithT(t) + + getter := NewMemoryRESTClientGetter(cfg, "", "", 400, 800, client.KubeConfigOptions{}) + got, err := getter.ToRESTMapper() + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(got).ToNot(BeNil()) +} + +func TestMemoryRESTClientGetter_ToRawKubeConfigLoader(t *testing.T) { + g := NewWithT(t) + + getter := NewMemoryRESTClientGetter(cfg, "a-namespace", "impersonate", 0, 0, client.KubeConfigOptions{}) + got := getter.ToRawKubeConfigLoader() + g.Expect(got).ToNot(BeNil()) + + cfg, err := got.ClientConfig() + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(cfg.Impersonate.UserName).To(Equal("impersonate")) + ns, _, err := got.Namespace() + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(ns).To(Equal("a-namespace")) +} diff --git a/internal/kube/config.go b/internal/kube/config.go new file mode 100644 index 0000000..d39e6ec --- /dev/null +++ b/internal/kube/config.go @@ -0,0 +1,51 @@ +/* +Copyright 2022 The Flux authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package kube + +import ( + "fmt" + + corev1 "k8s.io/api/core/v1" +) + +// ConfigFromSecret returns the KubeConfig data from the provided key in the +// given Secret, or attempts to load the data from the default `value` and +// `value.yaml` keys. If a Secret is provided but no key with data can be +// found, an error is returned. The secret may be nil, in which case no bytes +// nor error are returned. Validation of the data is expected to happen while +// decoding the bytes. +func ConfigFromSecret(secret *corev1.Secret, key string) ([]byte, error) { + var kubeConfig []byte + if secret != nil { + secretName := fmt.Sprintf("%s/%s", secret.Namespace, secret.Name) + switch { + case key != "": + kubeConfig = secret.Data[key] + if kubeConfig == nil { + return nil, fmt.Errorf("KubeConfig secret '%s' does not contain a '%s' key with data", secretName, key) + } + case secret.Data[DefaultKubeConfigSecretKey] != nil: + kubeConfig = secret.Data[DefaultKubeConfigSecretKey] + case secret.Data[DefaultKubeConfigSecretKeyExt] != nil: + kubeConfig = secret.Data[DefaultKubeConfigSecretKeyExt] + default: + // User did not specify a key, and the 'value' key was not defined. + return nil, fmt.Errorf("KubeConfig secret '%s' does not contain a '%s' or '%s' key with data", secretName, DefaultKubeConfigSecretKey, DefaultKubeConfigSecretKeyExt) + } + } + return kubeConfig, nil +} diff --git a/internal/kube/config_test.go b/internal/kube/config_test.go new file mode 100644 index 0000000..58c52d5 --- /dev/null +++ b/internal/kube/config_test.go @@ -0,0 +1,143 @@ +/* +Copyright 2022 The Flux authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package kube + +import ( + "testing" + + . "github.com/onsi/gomega" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +func TestConfigFromSecret(t *testing.T) { + t.Run("with default key", func(t *testing.T) { + g := NewWithT(t) + + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "super-secret", + Namespace: "vault", + }, + Data: map[string][]byte{ + DefaultKubeConfigSecretKey: []byte("good"), + // Also confirm priority. + DefaultKubeConfigSecretKeyExt: []byte("bad"), + }, + } + got, err := ConfigFromSecret(secret, "") + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(got).To(Equal(secret.Data[DefaultKubeConfigSecretKey])) + }) + + t.Run("with default key with ext", func(t *testing.T) { + g := NewWithT(t) + + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "super-secret", + Namespace: "vault", + }, + Data: map[string][]byte{ + DefaultKubeConfigSecretKeyExt: []byte("good"), + }, + } + got, err := ConfigFromSecret(secret, "") + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(got).To(Equal(secret.Data[DefaultKubeConfigSecretKeyExt])) + }) + + t.Run("with key", func(t *testing.T) { + g := NewWithT(t) + + key := "cola.recipe" + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "super-secret", + Namespace: "vault", + }, + Data: map[string][]byte{ + key: []byte("snow"), + }, + } + got, err := ConfigFromSecret(secret, key) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(got).To(Equal(secret.Data[key])) + }) + + t.Run("invalid key", func(t *testing.T) { + g := NewWithT(t) + + key := "black-hole" + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "super-secret", + Namespace: "vault", + }, + Data: map[string][]byte{}, + } + got, err := ConfigFromSecret(secret, key) + g.Expect(err).To(HaveOccurred()) + g.Expect(got).To(BeNil()) + g.Expect(err.Error()).To(ContainSubstring("secret 'vault/super-secret' does not contain a 'black-hole' key ")) + g.Expect(got).To(Equal(secret.Data[key])) + }) + + t.Run("key without data", func(t *testing.T) { + g := NewWithT(t) + + key := "void" + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "super-secret", + Namespace: "vault", + }, + Data: map[string][]byte{ + key: nil, + }, + } + got, err := ConfigFromSecret(secret, key) + g.Expect(err).To(HaveOccurred()) + g.Expect(got).To(BeNil()) + g.Expect(err.Error()).To(ContainSubstring("does not contain a 'void' key with data")) + }) + + t.Run("no keys", func(t *testing.T) { + g := NewWithT(t) + + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "super-secret", + Namespace: "vault", + }, + Data: map[string][]byte{}, + } + + got, err := ConfigFromSecret(secret, "") + g.Expect(err).To(HaveOccurred()) + g.Expect(got).To(BeNil()) + g.Expect(err.Error()).To(ContainSubstring("does not contain a 'value' or 'value.yaml'")) + }) + + t.Run("nil secret", func(t *testing.T) { + g := NewWithT(t) + + got, err := ConfigFromSecret(nil, "") + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(got).To(BeNil()) + }) +} diff --git a/internal/kube/impersonate.go b/internal/kube/impersonate.go new file mode 100644 index 0000000..e7176fc --- /dev/null +++ b/internal/kube/impersonate.go @@ -0,0 +1,47 @@ +/* +Copyright 2022 The Flux authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package kube + +import ( + "fmt" + + "k8s.io/client-go/rest" +) + +// DefaultServiceAccountName can be set at runtime to enable a fallback account +// name when no service account name is provided to SetImpersonationConfig. +var DefaultServiceAccountName string + +// userNameFormat is the format of a system service account user name string. +// It formats into `system:serviceaccount:namespace:name`. +const userNameFormat = "system:serviceaccount:%s:%s" + +// SetImpersonationConfig configures the provided service account name if +// given, or the DefaultServiceAccountName as a fallback if set. It returns +// the configured impersonation username, or an empty string. +func SetImpersonationConfig(cfg *rest.Config, namespace, serviceAccount string) string { + name := DefaultServiceAccountName + if serviceAccount != "" { + name = serviceAccount + } + if name != "" && namespace != "" { + username := fmt.Sprintf(userNameFormat, namespace, name) + cfg.Impersonate = rest.ImpersonationConfig{UserName: username} + return username + } + return "" +} diff --git a/internal/kube/impersonate_test.go b/internal/kube/impersonate_test.go new file mode 100644 index 0000000..d9a2d4c --- /dev/null +++ b/internal/kube/impersonate_test.go @@ -0,0 +1,75 @@ +/* +Copyright 2022 The Flux authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package kube + +import ( + "testing" + + . "github.com/onsi/gomega" + "k8s.io/client-go/rest" +) + +func TestSetImpersonationConfig(t *testing.T) { + t.Run("DefaultServiceAccountName", func(t *testing.T) { + g := NewWithT(t) + + DefaultServiceAccountName = "default" + namespace := "test" + expect := "system:serviceaccount:" + namespace + ":" + DefaultServiceAccountName + + cfg := &rest.Config{} + name := SetImpersonationConfig(cfg, namespace, "") + g.Expect(name).To(Equal(expect)) + g.Expect(cfg.Impersonate.UserName).ToNot(BeEmpty()) + g.Expect(cfg.Impersonate.UserName).To(Equal(name)) + }) + + t.Run("overwrite DefaultServiceAccountName", func(t *testing.T) { + g := NewWithT(t) + + DefaultServiceAccountName = "default" + namespace := "test" + serviceAccount := "different" + expect := "system:serviceaccount:" + namespace + ":" + serviceAccount + + cfg := &rest.Config{} + name := SetImpersonationConfig(cfg, namespace, serviceAccount) + g.Expect(name).To(Equal(expect)) + g.Expect(cfg.Impersonate.UserName).ToNot(BeEmpty()) + g.Expect(cfg.Impersonate.UserName).To(Equal(name)) + }) + + t.Run("without namespace", func(t *testing.T) { + g := NewWithT(t) + + serviceAccount := "account" + + cfg := &rest.Config{} + name := SetImpersonationConfig(cfg, "", serviceAccount) + g.Expect(name).To(BeEmpty()) + g.Expect(cfg.Impersonate.UserName).To(BeEmpty()) + }) + + t.Run("no arguments", func(t *testing.T) { + g := NewWithT(t) + + cfg := &rest.Config{} + name := SetImpersonationConfig(cfg, "", "") + g.Expect(name).To(BeEmpty()) + g.Expect(cfg.Impersonate.UserName).To(BeEmpty()) + }) +} diff --git a/internal/loader/artifact_url.go b/internal/loader/artifact_url.go new file mode 100644 index 0000000..702c848 --- /dev/null +++ b/internal/loader/artifact_url.go @@ -0,0 +1,80 @@ +/* +Copyright 2022 The Flux authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package loader + +import ( + "bytes" + "crypto/sha256" + "errors" + "fmt" + "io" + "net/http" + + "github.com/hashicorp/go-retryablehttp" + "helm.sh/helm/v3/pkg/chart" + "helm.sh/helm/v3/pkg/chart/loader" +) + +var ( + // ErrIntegrity signals a chart loader failed to verify the integrity of + // a chart, for example due to a checksum mismatch. + ErrIntegrity = errors.New("integrity failure") +) + +// SecureLoadChartFromURL attempts to download a Helm chart from the given URL +// using the provided client. The SHA256 sum of the retrieved data is confirmed +// to equal the given checksum before loading the chart. It returns the loaded +// chart.Chart, or an error. +func SecureLoadChartFromURL(client *retryablehttp.Client, URL, checksum string) (*chart.Chart, error) { + req, err := retryablehttp.NewRequest(http.MethodGet, URL, nil) + if err != nil { + return nil, err + } + + resp, err := client.Do(req) + if err != nil || resp != nil && resp.StatusCode != http.StatusOK { + if err != nil { + return nil, err + } + return nil, fmt.Errorf("failed to load chart from '%s': %s", URL, resp.Status) + } + + var c bytes.Buffer + if err := copyAndSHA256Check(checksum, resp.Body, &c); err != nil { + return nil, err + } + + if err := resp.Body.Close(); err != nil { + return nil, err + } + return loader.LoadArchive(&c) +} + +// copyAndSHA256Check copies from reader into writer while confirming the +// SHA256 checksum of the copied data matches the provided checksum. If +// this does not match, it returns an error. +func copyAndSHA256Check(checksum string, reader io.Reader, writer io.Writer) error { + hasher := sha256.New() + mw := io.MultiWriter(hasher, writer) + if _, err := io.Copy(mw, reader); err != nil { + return fmt.Errorf("failed to verify artifact: %w", err) + } + if calc := fmt.Sprintf("%x", hasher.Sum(nil)); checksum != calc { + return fmt.Errorf("%w: computed checksum '%s' doesn't match '%s'", ErrIntegrity, calc, checksum) + } + return nil +} diff --git a/internal/loader/artifact_url_test.go b/internal/loader/artifact_url_test.go new file mode 100644 index 0000000..5044016 --- /dev/null +++ b/internal/loader/artifact_url_test.go @@ -0,0 +1,133 @@ +/* +Copyright 2022 The Flux authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package loader + +import ( + "bytes" + "crypto/sha256" + "fmt" + "io" + "net/http" + "net/http/httptest" + "os" + "testing" + + "github.com/hashicorp/go-retryablehttp" + . "github.com/onsi/gomega" + _ "helm.sh/helm/v3/pkg/chart" +) + +func TestSecureLoadChartFromURL(t *testing.T) { + g := NewWithT(t) + + b, err := os.ReadFile("testdata/chart-0.1.0.tgz") + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(b).ToNot(BeNil()) + checksum := fmt.Sprintf("%x", sha256.Sum256(b)) + + const chartPath = "/chart.tgz" + server := httptest.NewServer(http.HandlerFunc(func(res http.ResponseWriter, req *http.Request) { + if req.URL.Path == chartPath { + res.WriteHeader(http.StatusOK) + _, _ = res.Write(b) + return + } + res.WriteHeader(http.StatusInternalServerError) + return + })) + t.Cleanup(func() { + server.Close() + }) + + chartURL := server.URL + chartPath + + client := retryablehttp.NewClient() + client.Logger = nil + client.RetryMax = 2 + + t.Run("loads Helm chart from URL", func(t *testing.T) { + g := NewWithT(t) + + got, err := SecureLoadChartFromURL(client, chartURL, checksum) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(got).ToNot(BeNil()) + g.Expect(got.Name()).To(Equal("chart")) + g.Expect(got.Metadata.Version).To(Equal("0.1.0")) + }) + + t.Run("error on chart data checksum mismatch", func(t *testing.T) { + g := NewWithT(t) + + got, err := SecureLoadChartFromURL(client, chartURL, "") + g.Expect(err).To(HaveOccurred()) + g.Expect(got).To(BeNil()) + }) + + t.Run("error on server error", func(t *testing.T) { + g := NewWithT(t) + + got, err := SecureLoadChartFromURL(client, server.URL+"/invalid.tgz", checksum) + g.Expect(err).To(HaveOccurred()) + g.Expect(got).To(BeNil()) + }) +} + +func Test_copyAndSHA256Check(t *testing.T) { + g := NewWithT(t) + + tmpDir := t.TempDir() + closedF, err := os.CreateTemp(tmpDir, "closed.txt") + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(closedF.Close()).ToNot(HaveOccurred()) + + tests := []struct { + name string + checksum string + in io.Reader + out io.Writer + wantErr bool + }{ + { + name: "checksum match", + checksum: "2c26b46b68ffc68ff99b453c1d30413413422d706483bfa0f98a5e886266e7ae", + in: bytes.NewReader([]byte("foo")), + out: io.Discard, + }, + { + name: "checksum mismatch", + checksum: "2c26b46b68ffc68ff99b453c1d30413413422d706483bfa0f98a5e886266e7ae", + in: bytes.NewReader([]byte("bar")), + out: io.Discard, + wantErr: true, + }, + { + name: "copy failure (closed file)", + checksum: "2c26b46b68ffc68ff99b453c1d30413413422d706483bfa0f98a5e886266e7ae", + in: bytes.NewReader([]byte("foo")), + out: closedF, + wantErr: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + + err := copyAndSHA256Check(tt.checksum, tt.in, tt.out) + g.Expect(err != nil).To(Equal(tt.wantErr), err) + }) + } +} diff --git a/controllers/testdata/chart-0.1.0.tgz b/internal/loader/testdata/chart-0.1.0.tgz similarity index 100% rename from controllers/testdata/chart-0.1.0.tgz rename to internal/loader/testdata/chart-0.1.0.tgz diff --git a/main.go b/main.go index 2eaeea4..3605b43 100644 --- a/main.go +++ b/main.go @@ -21,6 +21,7 @@ import ( "os" "time" + intkube "github.com/fluxcd/helm-controller/internal/kube" flag "github.com/spf13/pflag" "helm.sh/helm/v3/pkg/kube" "k8s.io/apimachinery/pkg/runtime" @@ -74,7 +75,6 @@ func main() { aclOptions acl.Options leaderElectionOptions leaderelection.Options rateLimiterOptions helper.RateLimiterOptions - defaultServiceAccount string ) flag.StringVar(&metricsAddr, "metrics-addr", ":8080", "The address the metric endpoint binds to.") @@ -85,7 +85,7 @@ func main() { flag.BoolVar(&watchAllNamespaces, "watch-all-namespaces", true, "Watch for custom resources in all namespaces, if set to false it will only watch the runtime namespace.") flag.IntVar(&httpRetry, "http-retry", 9, "The maximum number of retries when failing to fetch artifacts over HTTP.") - flag.StringVar(&defaultServiceAccount, "default-service-account", "", "Default service account used for impersonation.") + flag.StringVar(&intkube.DefaultServiceAccountName, "default-service-account", "", "Default service account used for impersonation.") clientOptions.BindFlags(flag.CommandLine) logOptions.BindFlags(flag.CommandLine) aclOptions.BindFlags(flag.CommandLine) @@ -151,14 +151,13 @@ func main() { } if err = (&controllers.HelmReleaseReconciler{ - Client: mgr.GetClient(), - Config: mgr.GetConfig(), - Scheme: mgr.GetScheme(), - EventRecorder: eventRecorder, - MetricsRecorder: metricsH.MetricsRecorder, - NoCrossNamespaceRef: aclOptions.NoCrossNamespaceRefs, - DefaultServiceAccount: defaultServiceAccount, - KubeConfigOpts: kubeConfigOpts, + Client: mgr.GetClient(), + Config: mgr.GetConfig(), + Scheme: mgr.GetScheme(), + EventRecorder: eventRecorder, + MetricsRecorder: metricsH.MetricsRecorder, + NoCrossNamespaceRef: aclOptions.NoCrossNamespaceRefs, + KubeConfigOpts: kubeConfigOpts, }).SetupWithManager(mgr, controllers.HelmReleaseReconcilerOptions{ MaxConcurrentReconciles: concurrent, DependencyRequeueInterval: requeueDependency,