Merge pull request #177 from alexander-demicev/secretnamespace

k8s-ci-robot · web-flow · commit 9c380ac446d8 · 2023-06-22T05:39:40.000-07:00
✨  Add ability to specify a namespace for provider secret
diff --git a/Makefile b/Makefile
@@ -294,6 +294,7 @@ generate-manifests: $(CONTROLLER_GEN) ## Generate manifests for the operator e.g
 	$(CONTROLLER_GEN) \
 		paths=./api/... \
 		paths=./internal/controller/... \
+		paths=./internal/webhook/... \
 		crd:crdVersions=v1 \
 		rbac:roleName=manager-role \
 		output:crd:dir=./config/crd/bases \
diff --git a/api/v1alpha1/provider_types.go b/api/v1alpha1/provider_types.go
@@ -51,6 +51,10 @@ type ProviderSpec struct {
 	// +optional
 	SecretName string `json:"secretName,omitempty"`
 
+	// SecretNamespace is the namespace of the Secret providing the configuration variables. If not specified,
+	// the namespace of the provider will be used.
+	SecretNamespace string `json:"secretNamespace,omitempty"`
+
 	// FetchConfig determines how the operator will fetch the components and metadata for the provider.
 	// If nil, the operator will try to fetch components according to default
 	// embedded fetch configuration for the given kind and `ObjectMeta.Name`.
diff --git a/config/chart/webhookcainjection_patch.yaml b/config/chart/webhookcainjection_patch.yaml
@@ -1,6 +1,13 @@
 # This patch add annotation to admission webhook config and
 # the variables $(CERTIFICATE_NAMESPACE) and $(CERTIFICATE_NAME) will be substituted by kustomize.
 apiVersion: admissionregistration.k8s.io/v1
+kind: MutatingWebhookConfiguration
+metadata:
+  name: mutating-webhook-configuration
+  annotations:
+    cert-manager.io/inject-ca-from: $(CERTIFICATE_NAMESPACE)/$(CERTIFICATE_NAME)
+---
+apiVersion: admissionregistration.k8s.io/v1
 kind: ValidatingWebhookConfiguration
 metadata:
   name: validating-webhook-configuration
diff --git a/config/crd/bases/operator.cluster.x-k8s.io_bootstrapproviders.yaml b/config/crd/bases/operator.cluster.x-k8s.io_bootstrapproviders.yaml
@@ -1450,6 +1450,11 @@ spec:
                   name should be updated. The contents should be in the form of key:value.
                   This secret must be in the same namespace as the provider.
                 type: string
+              secretNamespace:
+                description: SecretNamespace is the namespace of the Secret providing
+                  the configuration variables. If not specified, the namespace of
+                  the provider will be used.
+                type: string
               version:
                 description: Version indicates the provider version.
                 type: string
diff --git a/config/crd/bases/operator.cluster.x-k8s.io_controlplaneproviders.yaml b/config/crd/bases/operator.cluster.x-k8s.io_controlplaneproviders.yaml
@@ -1451,6 +1451,11 @@ spec:
                   name should be updated. The contents should be in the form of key:value.
                   This secret must be in the same namespace as the provider.
                 type: string
+              secretNamespace:
+                description: SecretNamespace is the namespace of the Secret providing
+                  the configuration variables. If not specified, the namespace of
+                  the provider will be used.
+                type: string
               version:
                 description: Version indicates the provider version.
                 type: string
diff --git a/config/crd/bases/operator.cluster.x-k8s.io_coreproviders.yaml b/config/crd/bases/operator.cluster.x-k8s.io_coreproviders.yaml
@@ -1450,6 +1450,11 @@ spec:
                   name should be updated. The contents should be in the form of key:value.
                   This secret must be in the same namespace as the provider.
                 type: string
+              secretNamespace:
+                description: SecretNamespace is the namespace of the Secret providing
+                  the configuration variables. If not specified, the namespace of
+                  the provider will be used.
+                type: string
               version:
                 description: Version indicates the provider version.
                 type: string
diff --git a/config/crd/bases/operator.cluster.x-k8s.io_infrastructureproviders.yaml b/config/crd/bases/operator.cluster.x-k8s.io_infrastructureproviders.yaml
@@ -1451,6 +1451,11 @@ spec:
                   name should be updated. The contents should be in the form of key:value.
                   This secret must be in the same namespace as the provider.
                 type: string
+              secretNamespace:
+                description: SecretNamespace is the namespace of the Secret providing
+                  the configuration variables. If not specified, the namespace of
+                  the provider will be used.
+                type: string
               version:
                 description: Version indicates the provider version.
                 type: string
diff --git a/config/default/webhookcainjection_patch.yaml b/config/default/webhookcainjection_patch.yaml
@@ -1,6 +1,13 @@
 # This patch add annotation to admission webhook config and
 # the variables $(CERTIFICATE_NAMESPACE) and $(CERTIFICATE_NAME) will be substituted by kustomize.
 apiVersion: admissionregistration.k8s.io/v1
+kind: MutatingWebhookConfiguration
+metadata:
+  name: mutating-webhook-configuration
+  annotations:
+    cert-manager.io/inject-ca-from: $(CERTIFICATE_NAMESPACE)/$(CERTIFICATE_NAME)
+---
+apiVersion: admissionregistration.k8s.io/v1
 kind: ValidatingWebhookConfiguration
 metadata:
   name: validating-webhook-configuration
diff --git a/config/webhook/manifests.yaml b/config/webhook/manifests.yaml
@@ -1,18 +1,113 @@
 ---
 apiVersion: admissionregistration.k8s.io/v1
+kind: MutatingWebhookConfiguration
+metadata:
+  name: mutating-webhook-configuration
+webhooks:
+- admissionReviewVersions:
+  - v1
+  - v1beta1
+  clientConfig:
+    service:
+      name: webhook-service
+      namespace: system
+      path: /mutate-operator-cluster-x-k8s-io-v1alpha1-bootstrapprovider
+  failurePolicy: Fail
+  matchPolicy: Equivalent
+  name: vbootstrapprovider.kb.io
+  rules:
+  - apiGroups:
+    - operator.cluster.x-k8s.io
+    apiVersions:
+    - v1alpha1
+    operations:
+    - CREATE
+    - UPDATE
+    resources:
+    - bootstrapproviders
+  sideEffects: None
+- admissionReviewVersions:
+  - v1
+  - v1beta1
+  clientConfig:
+    service:
+      name: webhook-service
+      namespace: system
+      path: /mutate-operator-cluster-x-k8s-io-v1alpha1-controlplaneprovider
+  failurePolicy: Fail
+  matchPolicy: Equivalent
+  name: vcontrolplaneprovider.kb.io
+  rules:
+  - apiGroups:
+    - operator.cluster.x-k8s.io
+    apiVersions:
+    - v1alpha1
+    operations:
+    - CREATE
+    - UPDATE
+    resources:
+    - controlplaneproviders
+  sideEffects: None
+- admissionReviewVersions:
+  - v1
+  - v1beta1
+  clientConfig:
+    service:
+      name: webhook-service
+      namespace: system
+      path: /mutate-operator-cluster-x-k8s-io-v1alpha1-coreprovider
+  failurePolicy: Fail
+  matchPolicy: Equivalent
+  name: vcoreprovider.kb.io
+  rules:
+  - apiGroups:
+    - operator.cluster.x-k8s.io
+    apiVersions:
+    - v1alpha1
+    operations:
+    - CREATE
+    - UPDATE
+    resources:
+    - coreproviders
+  sideEffects: None
+- admissionReviewVersions:
+  - v1
+  - v1beta1
+  clientConfig:
+    service:
+      name: webhook-service
+      namespace: system
+      path: /mutate-operator-cluster-x-k8s-io-v1alpha1-infrastructureprovider
+  failurePolicy: Fail
+  matchPolicy: Equivalent
+  name: vinfrastructureprovider.kb.io
+  rules:
+  - apiGroups:
+    - operator.cluster.x-k8s.io
+    apiVersions:
+    - v1alpha1
+    operations:
+    - CREATE
+    - UPDATE
+    resources:
+    - infrastructureproviders
+  sideEffects: None
+---
+apiVersion: admissionregistration.k8s.io/v1
 kind: ValidatingWebhookConfiguration
 metadata:
   name: validating-webhook-configuration
 webhooks:
 - admissionReviewVersions:
   - v1
-  - v1alpha1
+  - v1beta1
   clientConfig:
     service:
       name: webhook-service
       namespace: system
       path: /validate-operator-cluster-x-k8s-io-v1alpha1-bootstrapprovider
   failurePolicy: Fail
+  matchPolicy: Equivalent
   name: vbootstrapprovider.kb.io
   rules:
   - apiGroups:
@@ -27,13 +122,14 @@ webhooks:
   sideEffects: None
 - admissionReviewVersions:
   - v1
-  - v1alpha1
+  - v1beta1
   clientConfig:
     service:
       name: webhook-service
       namespace: system
       path: /validate-operator-cluster-x-k8s-io-v1alpha1-controlplaneprovider
   failurePolicy: Fail
+  matchPolicy: Equivalent
   name: vcontrolplaneprovider.kb.io
   rules:
   - apiGroups:
@@ -48,13 +144,14 @@ webhooks:
   sideEffects: None
 - admissionReviewVersions:
   - v1
-  - v1alpha1
+  - v1beta1
   clientConfig:
     service:
       name: webhook-service
       namespace: system
       path: /validate-operator-cluster-x-k8s-io-v1alpha1-coreprovider
   failurePolicy: Fail
+  matchPolicy: Equivalent
   name: vcoreprovider.kb.io
   rules:
   - apiGroups:
@@ -69,13 +166,14 @@ webhooks:
   sideEffects: None
 - admissionReviewVersions:
   - v1
-  - v1alpha1
+  - v1beta1
   clientConfig:
     service:
       name: webhook-service
       namespace: system
       path: /validate-operator-cluster-x-k8s-io-v1alpha1-infrastructureprovider
   failurePolicy: Fail
+  matchPolicy: Equivalent
   name: vinfrastructureprovider.kb.io
   rules:
   - apiGroups:
diff --git a/internal/controller/phases.go b/internal/controller/phases.go
@@ -195,7 +195,7 @@ func (p *phaseReconciler) secretReader(ctx context.Context) (configclient.Reader
 	// Fetch configuration variables from the secret. See API field docs for more info.
 	if p.provider.GetSpec().SecretName != "" {
 		secret := &corev1.Secret{}
-		key := types.NamespacedName{Namespace: p.provider.GetNamespace(), Name: p.provider.GetSpec().SecretName}
+		key := types.NamespacedName{Namespace: p.provider.GetSpec().SecretNamespace, Name: p.provider.GetSpec().SecretName}
 
 		if err := p.ctrlClient.Get(ctx, key, secret); err != nil {
 			return nil, err
diff --git a/internal/controller/phases_test.go b/internal/controller/phases_test.go
@@ -38,6 +38,7 @@ func TestSecretReader(t *testing.T) {
 	fakeclient := fake.NewClientBuilder().WithObjects().Build()
 
 	secretName := "test-secret"
+	secretNamespace := "test-secret-namespace"
 	namespace := "test-namespace"
 
 	p := &phaseReconciler{
@@ -54,7 +55,8 @@ func TestSecretReader(t *testing.T) {
 				},
 				Spec: operatorv1.CoreProviderSpec{
 					ProviderSpec: operatorv1.ProviderSpec{
-						SecretName: secretName,
+						SecretName:      secretName,
+						SecretNamespace: secretNamespace,
 						FetchConfig: &operatorv1.FetchConfiguration{
 							URL: "https://example.com",
 						},
@@ -72,7 +74,7 @@ func TestSecretReader(t *testing.T) {
 	g.Expect(fakeclient.Create(ctx, &corev1.Secret{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      secretName,
-			Namespace: namespace,
+			Namespace: secretNamespace,
 		},
 		Data: map[string][]byte{
 			testKey1: []byte(testValue1),
diff --git a/internal/controller/preflight_checks.go b/internal/controller/preflight_checks.go
@@ -117,7 +117,7 @@ func preflightChecks(ctx context.Context, c client.Client, provider genericprovi
 	// Validate that provided github token works and has repository access.
 	if spec.SecretName != "" {
 		secret := &corev1.Secret{}
-		key := types.NamespacedName{Namespace: provider.GetNamespace(), Name: provider.GetSpec().SecretName}
+		key := types.NamespacedName{Namespace: provider.GetSpec().SecretNamespace, Name: provider.GetSpec().SecretName}
 
 		if err := c.Get(ctx, key, secret); err != nil {
 			return ctrl.Result{}, fmt.Errorf("failed to get providers secret: %w", err)
diff --git a/internal/webhook/bootstrapprovider_webhook.go b/internal/webhook/bootstrapprovider_webhook.go
@@ -18,7 +18,9 @@ package webhook
 
 import (
 	"context"
+	"fmt"
 
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/runtime"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/webhook"
@@ -32,12 +34,17 @@ func (r *BootstrapProviderWebhook) SetupWebhookWithManager(mgr ctrl.Manager) err
 	return ctrl.NewWebhookManagedBy(mgr).
 		For(&operatorv1.BootstrapProvider{}).
 		WithValidator(r).
+		WithDefaulter(r).
 		Complete()
 }
 
-//+kubebuilder:webhook:verbs=create;update,path=/validate-operator-cluster-x-k8s-io-v1alpha1-bootstrapprovider,mutating=false,failurePolicy=fail,groups=operator.cluster.x-k8s.io,resources=bootstrapproviders,versions=v1alpha1,name=vbootstrapprovider.kb.io,sideEffects=None,admissionReviewVersions=v1;v1alpha1
+//+kubebuilder:webhook:verbs=create;update,path=/validate-operator-cluster-x-k8s-io-v1alpha1-bootstrapprovider,mutating=false,failurePolicy=fail,matchPolicy=Equivalent,groups=operator.cluster.x-k8s.io,resources=bootstrapproviders,versions=v1alpha1,name=vbootstrapprovider.kb.io,sideEffects=None,admissionReviewVersions=v1;v1beta1
+//+kubebuilder:webhook:verbs=create;update,path=/mutate-operator-cluster-x-k8s-io-v1alpha1-bootstrapprovider,mutating=true,failurePolicy=fail,matchPolicy=Equivalent,matchPolicy=Equivalent,groups=operator.cluster.x-k8s.io,resources=bootstrapproviders,versions=v1alpha1,name=vbootstrapprovider.kb.io,sideEffects=None,admissionReviewVersions=v1;v1beta1
 
-var _ webhook.CustomValidator = &BootstrapProviderWebhook{}
+var (
+	_ webhook.CustomValidator = &BootstrapProviderWebhook{}
+	_ webhook.CustomDefaulter = &BootstrapProviderWebhook{}
+)
 
 // ValidateCreate implements webhook.Validator so a webhook will be registered for the type.
 func (r *BootstrapProviderWebhook) ValidateCreate(ctx context.Context, obj runtime.Object) error {
@@ -53,3 +60,15 @@ func (r *BootstrapProviderWebhook) ValidateUpdate(ctx context.Context, oldObj, n
 func (r *BootstrapProviderWebhook) ValidateDelete(_ context.Context, obj runtime.Object) error {
 	return nil
 }
+
+// Default implements webhook.Default so a webhook will be registered for the type.
+func (r *BootstrapProviderWebhook) Default(ctx context.Context, obj runtime.Object) error {
+	bootstrapProvider, ok := obj.(*operatorv1.BootstrapProvider)
+	if !ok {
+		return apierrors.NewBadRequest(fmt.Sprintf("expected a BootstrapProvider but got a %T", obj))
+	}
+
+	setDefaultProviderSpec(&bootstrapProvider.Spec.ProviderSpec, bootstrapProvider.Namespace)
+
+	return nil
+}
diff --git a/internal/webhook/controlplaneprovider_webhook.go b/internal/webhook/controlplaneprovider_webhook.go
@@ -18,7 +18,9 @@ package webhook
 
 import (
 	"context"
+	"fmt"
 
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/runtime"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/webhook"
@@ -32,12 +34,17 @@ func (r *ControlPlaneProviderWebhook) SetupWebhookWithManager(mgr ctrl.Manager)
 	return ctrl.NewWebhookManagedBy(mgr).
 		For(&operatorv1.ControlPlaneProvider{}).
 		WithValidator(r).
+		WithDefaulter(r).
 		Complete()
 }
 
-//+kubebuilder:webhook:verbs=create;update,path=/validate-operator-cluster-x-k8s-io-v1alpha1-controlplaneprovider,mutating=false,failurePolicy=fail,groups=operator.cluster.x-k8s.io,resources=controlplaneproviders,versions=v1alpha1,name=vcontrolplaneprovider.kb.io,sideEffects=None,admissionReviewVersions=v1;v1alpha1
+//+kubebuilder:webhook:verbs=create;update,path=/validate-operator-cluster-x-k8s-io-v1alpha1-controlplaneprovider,mutating=false,failurePolicy=fail,matchPolicy=Equivalent,groups=operator.cluster.x-k8s.io,resources=controlplaneproviders,versions=v1alpha1,name=vcontrolplaneprovider.kb.io,sideEffects=None,admissionReviewVersions=v1;v1beta1
+//+kubebuilder:webhook:verbs=create;update,path=/mutate-operator-cluster-x-k8s-io-v1alpha1-controlplaneprovider,mutating=true,failurePolicy=fail,matchPolicy=Equivalent,groups=operator.cluster.x-k8s.io,resources=controlplaneproviders,versions=v1alpha1,name=vcontrolplaneprovider.kb.io,sideEffects=None,admissionReviewVersions=v1;v1beta1
 
-var _ webhook.CustomValidator = &ControlPlaneProviderWebhook{}
+var (
+	_ webhook.CustomValidator = &ControlPlaneProviderWebhook{}
+	_ webhook.CustomDefaulter = &ControlPlaneProviderWebhook{}
+)
 
 // ValidateCreate implements webhook.Validator so a webhook will be registered for the type.
 func (r *ControlPlaneProviderWebhook) ValidateCreate(ctx context.Context, obj runtime.Object) error {
@@ -53,3 +60,15 @@ func (r *ControlPlaneProviderWebhook) ValidateUpdate(ctx context.Context, oldObj
 func (r *ControlPlaneProviderWebhook) ValidateDelete(_ context.Context, obj runtime.Object) error {
 	return nil
 }
+
+// Default implements webhook.Default so a webhook will be registered for the type.
+func (r *ControlPlaneProviderWebhook) Default(ctx context.Context, obj runtime.Object) error {
+	controlPlaneProvider, ok := obj.(*operatorv1.ControlPlaneProvider)
+	if !ok {
+		return apierrors.NewBadRequest(fmt.Sprintf("expected a ControlPlaneProvider but got a %T", obj))
+	}
+
+	setDefaultProviderSpec(&controlPlaneProvider.Spec.ProviderSpec, controlPlaneProvider.Namespace)
+
+	return nil
+}
diff --git a/internal/webhook/coreprovider_webhook.go b/internal/webhook/coreprovider_webhook.go
diff --git a/internal/webhook/infrastructureprovider_webhook.go b/internal/webhook/infrastructureprovider_webhook.go
diff --git a/internal/webhook/provider_webhook.go b/internal/webhook/provider_webhook.go
diff --git a/internal/webhook/provider_webhook_test.go b/internal/webhook/provider_webhook_test.go
