Merge pull request #677 from weaveworks/no-cross-ns-controller

Add --no-cross-namespace-refs to tf-controller

cmd/manager/main.go

@@ -23,6 +23,7 @@ import (
  "github.com/weaveworks/tf-controller/mtls"
  "github.com/weaveworks/tf-controller/runner"
 
+ "github.com/fluxcd/pkg/runtime/acl"
  "github.com/fluxcd/pkg/runtime/client"
  runtimeCtrl "github.com/fluxcd/pkg/runtime/controller"
  "github.com/fluxcd/pkg/runtime/events"
@@ -90,6 +91,7 @@ func main() {
  runnerGRPCMaxMessageSize int
  allowBreakTheGlass bool
  clusterDomain string
+ aclOptions acl.Options
  )
 
  flag.StringVar(&metricsAddr, "metrics-addr", ":8080", "The address the metric endpoint binds to.")
@@ -115,6 +117,7 @@ func main() {
  clientOptions.BindFlags(flag.CommandLine)
  logOptions.BindFlags(flag.CommandLine)
  leaderElectionOptions.BindFlags(flag.CommandLine)
+ aclOptions.BindFlags(flag.CommandLine)
  flag.Parse()
 
  ctrl.SetLogger(logger.NewLogger(logOptions))
@@ -195,6 +198,7 @@ func main() {
  RunnerGRPCMaxMessageSize: runnerGRPCMaxMessageSize,
  AllowBreakTheGlass: allowBreakTheGlass,
  ClusterDomain: clusterDomain,
+ NoCrossNamespaceRefs: aclOptions.NoCrossNamespaceRefs,
  }
 
  if err = reconciler.SetupWithManager(mgr, concurrent, httpRetry); err != nil {

controllers/tc000015_source_cross_ns_denied_test.go

@@ -0,0 +1,96 @@
+package controllers
+
+import (
+ "context"
+ "testing"
+ "time"
+
+ . "github.com/onsi/gomega"
+ "github.com/onsi/gomega/gstruct"
+
+ sourcev1 "github.com/fluxcd/source-controller/api/v1"
+ infrav1 "github.com/weaveworks/tf-controller/api/v1alpha2"
+ "k8s.io/apimachinery/pkg/api/meta"
+ metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+ "sigs.k8s.io/controller-runtime/pkg/client"
+)
+
+func Test_000015_cross_namespace_source_denied_test(t *testing.T) {
+ Spec("This spec describes the behaviour of a Terraform resource when source is in another namespace and cross-namespace refs are not allowed.")
+ It("should be reconciled to have a Source error.")
+
+ By("setting the reconciler to disallow cross-namespace refs")
+ defer func(original bool) {
+ reconciler.NoCrossNamespaceRefs = original
+ }(reconciler.NoCrossNamespaceRefs)
+ reconciler.NoCrossNamespaceRefs = true
+
+ const (
+ sourceName = "gr-source"
+ terraformName = "tf-cross-ns"
+ )
+ g := NewWithT(t)
+ ctx := context.Background()
+
+ Given("a GitRepository")
+ By("defining a new GitRepository resource.")
+ testRepo := sourcev1.GitRepository{
+ ObjectMeta: metav1.ObjectMeta{
+ Name: sourceName,
+ Namespace: "flux-system",
+ },
+ Spec: sourcev1.GitRepositorySpec{
+ URL: "https://github.com/openshift-fluxv2-poc/podinfo",
+ Reference: &sourcev1.GitRepositoryRef{
+ Branch: "master",
+ },
+ Interval: metav1.Duration{Duration: time.Second * 30},
+ },
+ }
+ By("creating the GitRepository resource in the cluster.")
+ It("should be created successfully.")
+ g.Expect(k8sClient.Create(ctx, &testRepo)).Should(Succeed())
+ t.Cleanup(func() { g.Expect(k8sClient.Delete(ctx, &testRepo)).Should(Succeed()) })
+
+ Given("a Terraform resource, attached to a GitRepository resource in another namespace.")
+ By("creating a new TF resource and attaching to the repo via `sourceRef`.")
+ helloWorldTF := infrav1.Terraform{
+ ObjectMeta: metav1.ObjectMeta{
+ Name: terraformName,
+ Namespace: "default",
+ },
+ Spec: infrav1.TerraformSpec{
+ ApprovePlan: "auto",
+ Path: "./terraform-hello-world-example",
+ SourceRef: infrav1.CrossNamespaceSourceReference{
+ Kind: "GitRepository",
+ Name: sourceName,
+ Namespace: "flux-system",
+ },
+ Interval: metav1.Duration{Duration: time.Second * 10},
+ },
+ }
+ It("should be created and attached successfully.")
+ g.Expect(k8sClient.Create(ctx, &helloWorldTF)).Should(Succeed())
+ t.Cleanup(func() { g.Expect(k8sClient.Delete(ctx, &helloWorldTF)).Should(Succeed()) })
+
+ It("should be access denied error.")
+ By("checking that the Ready's reason of the TF resource become `ArtifactFailed`.")
+
+ helloWorldTFKey := client.ObjectKeyFromObject(&helloWorldTF)
+ var readyCondition *metav1.Condition
+ g.Eventually(func() interface{} {
+ var createdHelloWorldTF infrav1.Terraform
+ g.Expect(k8sClient.Get(ctx, helloWorldTFKey, &createdHelloWorldTF)).To(Succeed())
+ conditions := createdHelloWorldTF.Status.Conditions
+ readyCondition = meta.FindStatusCondition(conditions, "Ready")
+ return readyCondition
+ }, timeout, interval).ShouldNot(BeNil())
+
+ g.Expect(*readyCondition).To(
+ gstruct.MatchFields(gstruct.IgnoreExtras, gstruct.Fields{
+ "Type": Equal("Ready"),
+ "Reason": Equal(infrav1.ArtifactFailedReason),
+ "Message": Equal("Source 'GitRepository/flux-system/gr-source' access denied"),
+ }))
+}

controllers/tf_controller.go

@@ -29,6 +29,7 @@ import (
 
  eventv1 "github.com/fluxcd/pkg/apis/event/v1beta1"
  "github.com/fluxcd/pkg/apis/meta"
+ "github.com/fluxcd/pkg/runtime/acl"
  runtimeCtrl "github.com/fluxcd/pkg/runtime/controller"
  "github.com/fluxcd/pkg/runtime/dependency"
  "github.com/fluxcd/pkg/runtime/logger"
@@ -77,6 +78,7 @@ type TerraformReconciler struct {
  RunnerGRPCMaxMessageSize int
  AllowBreakTheGlass bool
  ClusterDomain string
+ NoCrossNamespaceRefs bool
 }
 
 //+kubebuilder:rbac:groups=infra.contrib.fluxcd.io,resources=terraforms,verbs=get;list;watch;create;update;patch;delete
@@ -189,6 +191,19 @@ func (r *TerraformReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
  log.Info(msg)
  // do not requeue immediately, when the source is created the watcher should trigger a reconciliation
  return ctrl.Result{RequeueAfter: terraform.GetRetryInterval()}, nil
+ } else if acl.IsAccessDenied(err) {
+ traceLog.Info("The cross-namespace Source was denied by reconciler.NoCrossNamespaceRefs")
+ msg := fmt.Sprintf("Source '%s' access denied", terraform.Spec.SourceRef.String())
+ terraform = infrav1.TerraformNotReady(terraform, "", infrav1.ArtifactFailedReason, msg)
+ traceLog.Info("Patch the Terraform resource Status with NotReady")
+ if err := r.patchStatus(ctx, req.NamespacedName, terraform.Status); err != nil {
+ log.Error(err, "unable to update status for source access denied")
+ return ctrl.Result{Requeue: true}, err
+ }
+ r.recordReadinessMetric(ctx, terraform)
+ log.Info(msg)
+ // don't requeue to retry; it won't succeed unless the sourceRef changes
+ return ctrl.Result{}, nil
  } else {
  // retry on transient errors
  log.Error(err, "retry")
@@ -600,6 +615,12 @@ func (r *TerraformReconciler) getSource(ctx context.Context, terraform infrav1.T
  Namespace: sourceNamespace,
  Name: terraform.Spec.SourceRef.Name,
  }
+ if r.NoCrossNamespaceRefs && namespacedName.Namespace != terraform.GetNamespace() {
+ return sourceObj, acl.AccessDeniedError(
+ fmt.Sprintf("cannot access %s/%s, cross-namespace references have been disabled", terraform.Spec.SourceRef.Kind, namespacedName),
+ )
+ }
+
  switch terraform.Spec.SourceRef.Kind {
  case sourcev1.GitRepositoryKind:
  var repository sourcev1.GitRepository