From acf164c46ec4ec67db444344f7437641aafbbb66 Mon Sep 17 00:00:00 2001 From: Somtochi Onyekwere Date: Fri, 28 Jan 2022 11:14:35 +0100 Subject: [PATCH] Add flag to disable cross namespace references Signed-off-by: Somtochi Onyekwere --- controllers/helmrelease_controller.go | 10 ++++++ controllers/helmrelease_controller_chart.go | 9 ++++++ .../helmrelease_controller_chart_test.go | 31 ++++++++++++++++++- docs/spec/v2beta1/helmreleases.md | 4 +++ go.mod | 3 +- go.sum | 2 ++ main.go | 4 +++ 7 files changed, 61 insertions(+), 2 deletions(-) diff --git a/controllers/helmrelease_controller.go b/controllers/helmrelease_controller.go index 4b284a145..0802ded3e 100644 --- a/controllers/helmrelease_controller.go +++ b/controllers/helmrelease_controller.go @@ -48,7 +48,9 @@ import ( "sigs.k8s.io/controller-runtime/pkg/reconcile" "sigs.k8s.io/controller-runtime/pkg/source" + apiacl "github.com/fluxcd/pkg/apis/acl" "github.com/fluxcd/pkg/apis/meta" + "github.com/fluxcd/pkg/runtime/acl" "github.com/fluxcd/pkg/runtime/events" "github.com/fluxcd/pkg/runtime/metrics" "github.com/fluxcd/pkg/runtime/predicates" @@ -78,6 +80,7 @@ type HelmReleaseReconciler struct { EventRecorder kuberecorder.EventRecorder ExternalEventRecorder *events.Recorder MetricsRecorder *metrics.Recorder + NoCrossNamespaceRef bool } func (r *HelmReleaseReconciler) SetupWithManager(mgr ctrl.Manager, opts HelmReleaseReconcilerOptions) error { @@ -213,6 +216,13 @@ func (r *HelmReleaseReconciler) reconcile(ctx context.Context, hr v2.HelmRelease // Reconcile chart based on the HelmChartTemplate hc, reconcileErr := r.reconcileChart(ctx, &hr) if reconcileErr != nil { + if acl.IsAccessDenied(reconcileErr) { + log.Error(reconcileErr, "access denied to cross-namespace source") + r.event(ctx, hr, hr.Status.LastAttemptedRevision, events.EventSeverityError, reconcileErr.Error()) + return v2.HelmReleaseNotReady(hr, apiacl.AccessDeniedReason, reconcileErr.Error()), + ctrl.Result{RequeueAfter: hr.Spec.Interval.Duration}, nil + } + msg := fmt.Sprintf("chart reconciliation failed: %s", reconcileErr.Error()) r.event(ctx, hr, hr.Status.LastAttemptedRevision, events.EventSeverityError, msg) return v2.HelmReleaseNotReady(hr, v2.ArtifactFailedReason, msg), ctrl.Result{Requeue: true}, reconcileErr diff --git a/controllers/helmrelease_controller_chart.go b/controllers/helmrelease_controller_chart.go index 5bd0204bf..00ed2c800 100644 --- a/controllers/helmrelease_controller_chart.go +++ b/controllers/helmrelease_controller_chart.go @@ -28,6 +28,7 @@ import ( "reflect" "strings" + "github.com/fluxcd/pkg/runtime/acl" "github.com/hashicorp/go-retryablehttp" "helm.sh/helm/v3/pkg/chart" "helm.sh/helm/v3/pkg/chart/loader" @@ -47,6 +48,14 @@ func (r *HelmReleaseReconciler) reconcileChart(ctx context.Context, hr *v2.HelmR Name: hr.GetHelmChartName(), } + 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, + })) + } + // Garbage collect the previous HelmChart if the namespace named changed. if hr.Status.HelmChart != "" && hr.Status.HelmChart != chartName.String() { if err := r.deleteHelmChart(ctx, hr); err != nil { diff --git a/controllers/helmrelease_controller_chart_test.go b/controllers/helmrelease_controller_chart_test.go index 499b22d6a..082a1a525 100644 --- a/controllers/helmrelease_controller_chart_test.go +++ b/controllers/helmrelease_controller_chart_test.go @@ -41,6 +41,7 @@ func TestHelmReleaseReconciler_reconcileChart(t *testing.T) { expectHelmChartStatus string expectGC bool expectErr bool + noCrossNamspaceRef bool }{ { name: "new HelmChart", @@ -140,6 +141,33 @@ func TestHelmReleaseReconciler_reconcileChart(t *testing.T) { expectHelmChartStatus: "cross/default-test-release", expectGC: true, }, + { + name: "block cross namespace access when flag is set", + hr: &v2.HelmRelease{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-release", + Namespace: "default", + }, + Spec: v2.HelmReleaseSpec{ + Interval: metav1.Duration{Duration: time.Minute}, + Chart: v2.HelmChartTemplate{ + Spec: v2.HelmChartTemplateSpec{ + Chart: "chart", + SourceRef: v2.CrossNamespaceObjectReference{ + Name: "test-repository", + Kind: "HelmRepository", + Namespace: "cross", + }, + }, + }, + }, + Status: v2.HelmReleaseStatus{ + HelmChart: "", + }, + }, + noCrossNamspaceRef: true, + expectErr: true, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -156,7 +184,8 @@ func TestHelmReleaseReconciler_reconcileChart(t *testing.T) { } r := &HelmReleaseReconciler{ - Client: c, + Client: c, + NoCrossNamespaceRef: tt.noCrossNamspaceRef, } hc, err := r.reconcileChart(logr.NewContext(context.TODO(), logr.Discard()), tt.hr) diff --git a/docs/spec/v2beta1/helmreleases.md b/docs/spec/v2beta1/helmreleases.md index b33436bb6..507ee6cd0 100644 --- a/docs/spec/v2beta1/helmreleases.md +++ b/docs/spec/v2beta1/helmreleases.md @@ -687,6 +687,10 @@ Supported source types: The `HelmChart` is created in the same namespace as the `sourceRef`, with a name matching the `HelmRelease` `-`. +> **Note** that on multi-tenant clusters, platform admins can disable cross-namespace references +> with the `--no-cross-namespace-refs=true` flag. When this flag is set, the helmrelease can only +> refer to sources in the same namespace as the helmrelease object. + The `chart.spec.chart` can either contain: - The name of the chart as made available by the `HelmRepository` diff --git a/go.mod b/go.mod index f45e8b384..7d7a12024 100644 --- a/go.mod +++ b/go.mod @@ -6,9 +6,10 @@ replace github.com/fluxcd/helm-controller/api => ./api require ( github.com/fluxcd/helm-controller/api v0.15.0 + github.com/fluxcd/pkg/apis/acl v0.0.3 github.com/fluxcd/pkg/apis/kustomize v0.3.1 github.com/fluxcd/pkg/apis/meta v0.10.2 - github.com/fluxcd/pkg/runtime v0.12.3 + github.com/fluxcd/pkg/runtime v0.12.4 github.com/fluxcd/source-controller/api v0.20.1 github.com/go-logr/logr v1.2.2 github.com/hashicorp/go-retryablehttp v0.6.8 diff --git a/go.sum b/go.sum index 7b47433ed..9ced11d51 100644 --- a/go.sum +++ b/go.sum @@ -320,6 +320,8 @@ github.com/fluxcd/pkg/apis/meta v0.10.2 h1:pnDBBEvfs4HaKiVAYgz+e/AQ8dLvcgmVfSeBr github.com/fluxcd/pkg/apis/meta v0.10.2/go.mod h1:KQ2er9xa6koy7uoPMZjIjNudB5p4tXs+w0GO6fRcy7I= github.com/fluxcd/pkg/runtime v0.12.3 h1:h21AZ3YG5MAP7DxFF9hfKrP+vFzys2L7CkUbPFjbP/0= github.com/fluxcd/pkg/runtime v0.12.3/go.mod h1:imJ2xYy/d4PbSinX2IefmZk+iS2c1P5fY0js8mCE4SM= +github.com/fluxcd/pkg/runtime v0.12.4 h1:gA27RG/+adN2/7Qe03PB46Iwmye/MusPCpuS4zQ2fW0= +github.com/fluxcd/pkg/runtime v0.12.4/go.mod h1:gspNvhAqodZgSmK1ZhMtvARBf/NGAlxmaZaIOHkJYsc= github.com/fluxcd/source-controller/api v0.20.1 h1:BfYw1gNHykiCVFNtDz3atcf3Vph+arfuveKmouI98wE= github.com/fluxcd/source-controller/api v0.20.1/go.mod h1:Ab2qDmAUz6ZCp8UaHYLYzxyFrC1FQqEqjxiROb/Rdiw= github.com/form3tech-oss/jwt-go v3.2.2+incompatible/go.mod h1:pbq4aXjuKjdthFRnoDwaVPLA+WlJuPGy+QneDUgJi2k= diff --git a/main.go b/main.go index 1a59bf5a3..87fc51b54 100644 --- a/main.go +++ b/main.go @@ -30,6 +30,7 @@ import ( ctrl "sigs.k8s.io/controller-runtime" crtlmetrics "sigs.k8s.io/controller-runtime/pkg/metrics" + "github.com/fluxcd/pkg/runtime/acl" "github.com/fluxcd/pkg/runtime/client" "github.com/fluxcd/pkg/runtime/events" "github.com/fluxcd/pkg/runtime/leaderelection" @@ -70,6 +71,7 @@ func main() { httpRetry int clientOptions client.Options logOptions logger.Options + aclOptions acl.Options leaderElectionOptions leaderelection.Options ) @@ -83,6 +85,7 @@ func main() { flag.IntVar(&httpRetry, "http-retry", 9, "The maximum number of retries when failing to fetch artifacts over HTTP.") clientOptions.BindFlags(flag.CommandLine) logOptions.BindFlags(flag.CommandLine) + aclOptions.BindFlags(flag.CommandLine) leaderElectionOptions.BindFlags(flag.CommandLine) flag.Parse() @@ -139,6 +142,7 @@ func main() { EventRecorder: mgr.GetEventRecorderFor(controllerName), ExternalEventRecorder: eventRecorder, MetricsRecorder: metricsRecorder, + NoCrossNamespaceRef: aclOptions.NoCrossNamespaceRefs, }).SetupWithManager(mgr, controllers.HelmReleaseReconcilerOptions{ MaxConcurrentReconciles: concurrent, DependencyRequeueInterval: requeueDependency,