[FEATURE] - Provider Scope moved to Cluster (#116)

Moving Providers to Cluster Scope

Currently the providers are namespaced but the controller doesn't actually support local namespaced providers as their secrets should be located in the same namespace as the controller itself. Given design to would make more sense to move the providers into a cluster scope and perhaps if a geniune requirement later down the line, introduce a LocalProvider.

Note this is a break change can requires a migration.

* updated the api type for the provider to be cluster scoped
* updated the auto generated code from moving the scope of the provider crd
* updated the examples
* updated the e2e tests to remove the need for a namespace
* updated the references to the namespace from the provider
* fixing up the unit tests to reflect the removal of the namespace
* added a validation check to ensure that static secrets are located within the same namespace as the controller
* renaming the variable name frome JobNamespace to ControllerNamespace
* added the namespace field back, but changing as a deprecated field; which make migrations easier

charts/terraform-controller/crds/terraform.appvia.io_configurations.yaml

@@ -76,7 +76,6 @@ spec:
                       type: string
                   required:
                     - name
-                    - namespace
                   type: object
                 terraformVersion:
                   description: TerraformVersion provides the ability to override the default terraform version. Before changing this field its best to consult with platform administrator. As the value of this field is used to change the tag of the terraform container image.

charts/terraform-controller/crds/terraform.appvia.io_providers.yaml

@@ -14,7 +14,7 @@ spec:
     listKind: ProviderList
     plural: providers
     singular: provider
-  scope: Namespaced
+  scope: Cluster
   versions:
     - additionalPrinterColumns:
         - jsonPath: .spec.source

examples/configuration.yaml

@@ -8,7 +8,6 @@ spec:
   module: https://github.com/terraform-aws-modules/terraform-aws-s3-bucket.git?ref=v3.1.0
 
   providerRef:
-    namespace: terraform-system
     name: aws
 
   writeConnectionSecretToRef:

examples/database.yaml

@@ -7,7 +7,6 @@ spec:
   module: github.com/terraform-aws-modules/terraform-aws-rds-aurora
 
   providerRef:
-    namespace: default
     name: aws
 
   writeConnectionSecretToRef:

pkg/apis/terraform/v1alpha1/configuration_types.go

@@ -102,8 +102,9 @@ type ProviderReference struct {
 	// +kubebuilder:validation:Required
 	Name string `json:"name"`
 	// Namespace is the namespace of the provider itself.
-	// +kubebuilder:validation:Required
-	Namespace string `json:"namespace"`
+	// +kubebuilder:validation:Optional
+	// +kubebuilder:deprecatedversion:warning="namespace is a deprecated field for provider references"
+	Namespace string `json:"namespace,omitempty"`
 }
 
 // WriteConnectionSecret defines the options around the secret produced by the terraform code

pkg/apis/terraform/v1alpha1/provider_types.go

@@ -94,7 +94,7 @@ type ProviderSpec struct {
 // +k8s:openapi-gen=true
 // +kubebuilder:object:root=true
 // +kubebuilder:subresource:status
-// +kubebuilder:resource:path=providers,scope=Namespaced,categories={terraform}
+// +kubebuilder:resource:path=providers,scope=Cluster,categories={terraform}
 // +kubebuilder:printcolumn:name="Source",type="string",JSONPath=".spec.source"
 // +kubebuilder:printcolumn:name="Provider",type="string",JSONPath=".spec.provider"
 // +kubebuilder:printcolumn:name="Age",type="date",JSONPath=".metadata.creationTimestamp"
@@ -108,10 +108,7 @@ type Provider struct {
 
 // GetNamespacedName returns the namespaced name type
 func (p *Provider) GetNamespacedName() types.NamespacedName {
-	return types.NamespacedName{
-		Namespace: p.Namespace,
-		Name:      p.Name,
-	}
+	return types.NamespacedName{Name: p.Name}
 }
 
 // ProviderStatus defines the observed state of a provider

pkg/controller/configuration/controller.go

@@ -60,6 +60,8 @@ type Controller struct {
 	cache *cache.Cache
 	// recorder is the kubernetes event recorder
 	recorder record.EventRecorder
+	// ControllerNamespace is the namespace where the runner is running
+	ControllerNamespace string
 	// EnableInfracosts enables the cost analytics via infracost
 	EnableInfracosts bool
 	// EnableWatchers indicates we should create watcher jobs in the user namespace
@@ -72,8 +74,6 @@ type Controller struct {
 	InfracostsImage string
 	// InfracostsSecretName is the name of the secret containing the api and token
 	InfracostsSecretName string
-	// JobNamespace is the namespace where the runner is running
-	JobNamespace string
 	// JobTemplate is a custom override for the template to use
 	JobTemplate string
 	// PolicyImage is the image to use for all policy / checkov jobs
@@ -87,13 +87,13 @@ func (c *Controller) Add(mgr manager.Manager) error {
 	log.WithFields(log.Fields{
 		"enable_costs":    c.EnableInfracosts,
 		"enable_watchers": c.EnableWatchers,
-		"namespace":       c.JobNamespace,
+		"namespace":       c.ControllerNamespace,
 		"policy_image":    c.PolicyImage,
 		"terraform_image": c.TerraformImage,
 	}).Info("adding the configuration controller")
 
 	switch {
-	case c.JobNamespace == "":
+	case c.ControllerNamespace == "":
 		return errors.New("job namespace is required")
 	case c.TerraformImage == "":
 		return errors.New("terraform image is required")
@@ -160,21 +160,21 @@ func (c *Controller) Add(mgr manager.Manager) error {
 			handler.EnqueueRequestsFromMapFunc(func(a client.Object) []reconcile.Request {
 				return []ctrl.Request{
 					{NamespacedName: client.ObjectKey{
-						Namespace: c.JobNamespace,
+						Namespace: c.ControllerNamespace,
 						Name:      a.GetLabels()["job-name"],
 					}},
 				}
 			}),
 			// we only care about jobs in our namespace
 			builder.WithPredicates(predicate.Funcs{
 				GenericFunc: func(e event.GenericEvent) bool {
-					return e.Object.GetNamespace() == c.JobNamespace
+					return e.Object.GetNamespace() == c.ControllerNamespace
 				},
 				CreateFunc: func(e event.CreateEvent) bool {
-					return e.Object.GetNamespace() == c.JobNamespace
+					return e.Object.GetNamespace() == c.ControllerNamespace
 				},
 				UpdateFunc: func(e event.UpdateEvent) bool {
-					return e.ObjectNew.GetNamespace() == c.JobNamespace
+					return e.ObjectNew.GetNamespace() == c.ControllerNamespace
 				},
 				DeleteFunc: func(e event.DeleteEvent) bool {
 					return false

pkg/controller/configuration/delete.go

@@ -50,7 +50,7 @@ func (c *Controller) ensureTerraformDestroy(configuration *terraformv1alphav1.Co
 
 		// @step: check we have a terraform state - else we can just continue
 		secret := &v1.Secret{}
-		secret.Namespace = c.JobNamespace
+		secret.Namespace = c.ControllerNamespace
 		secret.Name = configuration.GetTerraformStateSecretName()
 
 		found, err := kubernetes.GetIfExists(ctx, c.cc, secret)
@@ -79,7 +79,7 @@ func (c *Controller) ensureTerraformDestroy(configuration *terraformv1alphav1.Co
 			ExecutorImage:    c.ExecutorImage,
 			InfracostsImage:  c.InfracostsImage,
 			InfracostsSecret: c.InfracostsSecretName,
-			Namespace:        c.JobNamespace,
+			Namespace:        c.ControllerNamespace,
 			Template:         state.jobTemplate,
 			TerraformImage:   GetTerraformImage(configuration, c.TerraformImage),
 		})
@@ -132,7 +132,7 @@ func (c *Controller) ensureTerraformConfigDeleted(configuration *terraformv1alph
 
 	return func(ctx context.Context) (reconcile.Result, error) {
 		cm := &v1.ConfigMap{}
-		cm.Namespace = c.JobNamespace
+		cm.Namespace = c.ControllerNamespace
 		cm.Name = name
 
 		if err := kubernetes.DeleteIfExists(ctx, c.cc, cm); err != nil {
@@ -191,7 +191,7 @@ func (c *Controller) ensureConfigurationSecretsDeleted(configuration *terraformv
 
 		for _, name := range names {
 			secret := &v1.Secret{}
-			secret.Namespace = c.JobNamespace
+			secret.Namespace = c.ControllerNamespace
 			secret.Name = name
 
 			if err := kubernetes.DeleteIfExists(ctx, c.cc, secret); err != nil {

pkg/controller/configuration/ensure.go

@@ -60,7 +60,7 @@ func (c *Controller) ensureCapturedState(configuration *terraformv1alphav1.Confi
 
 		// @step: retrieve a list of jobs
 		jobs := &batchv1.JobList{}
-		if err := c.cc.List(ctx, jobs, client.InNamespace(c.JobNamespace)); err != nil {
+		if err := c.cc.List(ctx, jobs, client.InNamespace(c.ControllerNamespace)); err != nil {
 			cond.Failed(err, "Failed to list the jobs in controller namespace")
 
 			return reconcile.Result{}, err
@@ -125,7 +125,7 @@ func (c *Controller) ensureCostSecret(configuration *terraformv1alphav1.Configur
 		}
 
 		secret := &v1.Secret{}
-		secret.Namespace = c.JobNamespace
+		secret.Namespace = c.ControllerNamespace
 		secret.Name = c.InfracostsSecretName
 
 		found, err := kubernetes.GetIfExists(ctx, c.cc, secret)
@@ -210,7 +210,7 @@ func (c *Controller) ensureCustomJobTemplate(configuration *terraformv1alphav1.C
 		}
 
 		cm := &v1.ConfigMap{}
-		cm.Namespace = c.JobNamespace
+		cm.Namespace = c.ControllerNamespace
 		cm.Name = c.JobTemplate
 
 		found, err := kubernetes.GetIfExists(ctx, c.cc, cm)
@@ -220,15 +220,15 @@ func (c *Controller) ensureCustomJobTemplate(configuration *terraformv1alphav1.C
 			return reconcile.Result{}, err
 		}
 		if !found {
-			cond.ActionRequired("Custom job template (%s/%s) does not exists", c.JobNamespace, c.JobTemplate)
+			cond.ActionRequired("Custom job template (%s/%s) does not exists", c.ControllerNamespace, c.JobTemplate)
 
 			return reconcile.Result{}, controller.ErrIgnore
 		}
 
 		template, found := cm.Data[terraformv1alphav1.TerraformJobTemplateConfigMapKey]
 		if !found {
 			cond.ActionRequired("Custom job template (%s/%s) does not contain the %q key",
-				c.JobNamespace, c.JobTemplate, terraformv1alphav1.TerraformJobTemplateConfigMapKey)
+				c.ControllerNamespace, c.JobTemplate, terraformv1alphav1.TerraformJobTemplateConfigMapKey)
 
 			return reconcile.Result{}, controller.ErrIgnore
 		}
@@ -275,17 +275,16 @@ func (c *Controller) ensureProviderReady(configuration *terraformv1alphav1.Confi
 
 	return func(ctx context.Context) (reconcile.Result, error) {
 		provider := &terraformv1alphav1.Provider{}
-		provider.Namespace = configuration.Spec.ProviderRef.Namespace
 		provider.Name = configuration.Spec.ProviderRef.Name
 
 		found, err := kubernetes.GetIfExists(ctx, c.cc, provider)
 		if err != nil {
-			cond.Failed(err, "Failed to retrieve the provider for the configuration: (%s/%s)", provider.Namespace, provider.Name)
+			cond.Failed(err, "Failed to retrieve the provider for the configuration: %q", provider.Name)
 
 			return reconcile.Result{}, err
 		}
 		if !found {
-			cond.ActionRequired("Provider referenced (%s/%s) does not exist", provider.Namespace, provider.Name)
+			cond.ActionRequired("Provider referenced %q does not exist", provider.Name)
 
 			return reconcile.Result{RequeueAfter: 5 * time.Minute}, nil
 		}
@@ -337,7 +336,7 @@ func (c *Controller) ensureJobConfigurationSecret(configuration *terraformv1alph
 
 	return func(ctx context.Context) (reconcile.Result, error) {
 		secret := &v1.Secret{}
-		secret.Namespace = c.JobNamespace
+		secret.Namespace = c.ControllerNamespace
 		secret.Name = name
 
 		if _, err := kubernetes.GetIfExists(ctx, c.cc, secret); err != nil {
@@ -353,7 +352,7 @@ func (c *Controller) ensureJobConfigurationSecret(configuration *terraformv1alph
 
 		// @step: generate the terraform backend configuration - this creates a kubernetes terraform
 		// backend pointing at a secret
-		cfg, err := terraform.NewKubernetesBackend(c.JobNamespace, backend)
+		cfg, err := terraform.NewKubernetesBackend(c.ControllerNamespace, backend)
 		if err != nil {
 			cond.Failed(err, "Failed to generate the terraform backend configuration")
 
@@ -457,7 +456,7 @@ func (c *Controller) ensureTerraformPlan(configuration *terraformv1alphav1.Confi
 			ExecutorImage:    c.ExecutorImage,
 			InfracostsImage:  c.InfracostsImage,
 			InfracostsSecret: c.InfracostsSecretName,
-			Namespace:        c.JobNamespace,
+			Namespace:        c.ControllerNamespace,
 			PolicyImage:      c.PolicyImage,
 			PolicyConstraint: state.checkovConstraint,
 			Template:         state.jobTemplate,
@@ -553,7 +552,7 @@ func (c *Controller) ensureCostStatus(configuration *terraformv1alphav1.Configur
 		}
 
 		secret := &v1.Secret{}
-		secret.Namespace = c.JobNamespace
+		secret.Namespace = c.ControllerNamespace
 		secret.Name = configuration.GetTerraformCostSecretName()
 
 		found, err := kubernetes.GetIfExists(ctx, c.cc, secret)
@@ -615,7 +614,7 @@ func (c *Controller) ensurePolicyStatus(configuration *terraformv1alphav1.Config
 
 		// @step: retrieve the uploaded scan
 		secret := &v1.Secret{}
-		secret.Namespace = c.JobNamespace
+		secret.Namespace = c.ControllerNamespace
 		secret.Name = configuration.GetTerraformPolicySecretName()
 
 		found, err := kubernetes.GetIfExists(ctx, c.cc, secret)
@@ -625,7 +624,7 @@ func (c *Controller) ensurePolicyStatus(configuration *terraformv1alphav1.Config
 			return reconcile.Result{}, err
 		}
 		if !found {
-			cond.Warning("Failed to find the secret: (%s/%s) containing checkov scan", c.JobNamespace, configuration.GetTerraformPolicySecretName())
+			cond.Warning("Failed to find the secret: (%s/%s) containing checkov scan", c.ControllerNamespace, configuration.GetTerraformPolicySecretName())
 
 			return reconcile.Result{RequeueAfter: 10 * time.Minute}, nil
 		}
@@ -699,7 +698,7 @@ func (c *Controller) ensureDriftDetection(configuration *terraformv1alphav1.Conf
 		// @step: retrive a list of pods related to the job
 		pods := &v1.PodList{}
 		filters := client.MatchingLabels{"job-name": job.GetName()}
-		if err := c.cc.List(ctx, pods, client.InNamespace(c.JobNamespace), filters); err != nil {
+		if err := c.cc.List(ctx, pods, client.InNamespace(c.ControllerNamespace), filters); err != nil {
 			cond.Failed(err, "Failed to list the terraform plan pods")
 
 			return reconcile.Result{}, err
@@ -760,7 +759,7 @@ func (c *Controller) ensureTerraformApply(configuration *terraformv1alphav1.Conf
 			ExecutorImage:    c.ExecutorImage,
 			InfracostsImage:  c.InfracostsImage,
 			InfracostsSecret: c.InfracostsSecretName,
-			Namespace:        c.JobNamespace,
+			Namespace:        c.ControllerNamespace,
 			Template:         state.jobTemplate,
 			TerraformImage:   GetTerraformImage(configuration, c.TerraformImage),
 		})
@@ -829,16 +828,16 @@ func (c *Controller) ensureConnectionSecret(configuration *terraformv1alphav1.Co
 	return func(ctx context.Context) (reconcile.Result, error) {
 		secret := &v1.Secret{}
 		secret.Name = configuration.GetTerraformStateSecretName()
-		secret.Namespace = c.JobNamespace
+		secret.Namespace = c.ControllerNamespace
 
 		found, err := kubernetes.GetIfExists(ctx, c.cc, secret)
 		if err != nil {
-			cond.Failed(err, "Failed to get terraform state secret (%s/%s)", c.JobNamespace, secret.Name)
+			cond.Failed(err, "Failed to get terraform state secret (%s/%s)", c.ControllerNamespace, secret.Name)
 
 			return reconcile.Result{}, err
 		}
 		if !found {
-			cond.Failed(nil, "Terraform state secret (%s/%s) not found", c.JobNamespace, secret.Name)
+			cond.Failed(nil, "Terraform state secret (%s/%s) not found", c.ControllerNamespace, secret.Name)
 
 			return reconcile.Result{}, controller.ErrIgnore
 		}

pkg/controller/configuration/helper.go

@@ -64,7 +64,7 @@ func GetTerraformImage(configuration *terraformv1alphav1.Configuration, image st
 
 // CreateWatcher is responsible for ensuring the logger is running in the application namespace
 func (c Controller) CreateWatcher(ctx context.Context, configuration *terraformv1alphav1.Configuration, stage string) error {
-	watcher := jobs.New(configuration, nil).NewJobWatch(c.JobNamespace, stage)
+	watcher := jobs.New(configuration, nil).NewJobWatch(c.ControllerNamespace, stage)
 
 	// @step: check if the logger has been created
 	found, err := kubernetes.GetIfExists(ctx, c.cc, watcher.DeepCopy())