diff --git a/api/v1/nbsetupkey_types.go b/api/v1/nbsetupkey_types.go index 11bf1eb..679aa9e 100644 --- a/api/v1/nbsetupkey_types.go +++ b/api/v1/nbsetupkey_types.go @@ -33,6 +33,7 @@ const ( // NBSetupKeySpec defines the desired state of NBSetupKey. type NBSetupKeySpec struct { // SecretKeyRef is a reference to the secret containing the setup key + // +kubebuilder:validation:XValidation:rule="self.name.size() > 0",reason="FieldValueRequired",message="secret name needs to be set",fieldPath=".name" SecretKeyRef corev1.SecretKeySelector `json:"secretKeyRef"` // ManagementURL optional, override operator management URL ManagementURL string `json:"managementURL,omitempty"` diff --git a/cmd/main.go b/cmd/main.go index 5fbbf21..fa1ba85 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -208,11 +208,6 @@ func main() { setupLog.Error(err, "unable to create webhook", "webhook", "Pod") os.Exit(1) } - - if err = webhooknetbirdiov1.SetupNBSetupKeyWebhookWithManager(mgr); err != nil { - setupLog.Error(err, "unable to create webhook", "webhook", "NBSetupKey") - os.Exit(1) - } } if len(netbirdAPIKey) > 0 { diff --git a/helm/kubernetes-operator/crds/netbird.io_nbsetupkeys.yaml b/helm/kubernetes-operator/crds/netbird.io_nbsetupkeys.yaml index f730c43..46b33a3 100644 --- a/helm/kubernetes-operator/crds/netbird.io_nbsetupkeys.yaml +++ b/helm/kubernetes-operator/crds/netbird.io_nbsetupkeys.yaml @@ -67,6 +67,11 @@ spec: - key type: object x-kubernetes-map-type: atomic + x-kubernetes-validations: + - fieldPath: .name + message: secret name needs to be set + reason: FieldValueRequired + rule: self.name.size() > 0 volumeMounts: description: VolumeMounts optional, additional volumeMounts for NetBird container diff --git a/helm/kubernetes-operator/templates/webhook.yaml b/helm/kubernetes-operator/templates/webhook.yaml index e96d8bc..9209de9 100644 --- a/helm/kubernetes-operator/templates/webhook.yaml +++ b/helm/kubernetes-operator/templates/webhook.yaml @@ -51,46 +51,6 @@ webhooks: resources: - pods sideEffects: None ---- -apiVersion: admissionregistration.k8s.io/v1 -kind: ValidatingWebhookConfiguration -metadata: -{{- if $.Values.webhook.enableCertManager }} - annotations: - cert-manager.io/inject-ca-from: {{ .Release.Namespace }}/{{ template "kubernetes-operator.fullname" . }}-serving-cert -{{- end }} - name: {{ include "kubernetes-operator.fullname" . }}-vnbsetupkey-webhook - labels: - {{- include "kubernetes-operator.labels" . | nindent 4 }} -webhooks: -- clientConfig: - {{- if not $.Values.webhook.enableCertManager }} - caBundle: {{ $tls.caCert }} - {{ end }} - service: - name: {{ template "kubernetes-operator.webhookService" . }} - namespace: {{ $.Release.Namespace }} - path: /validate-netbird-io-v1-nbsetupkey - failurePolicy: {{ .Values.webhook.failurePolicy }} - name: vnbsetupkey-v1.netbird.io - admissionReviewVersions: - - v1 - {{- if .Values.webhook.namespaceSelectors }} - namespaceSelector: - matchExpressions: - {{ toYaml .Values.webhook.namespaceSelectors | nindent 4 }} - {{ end }} - rules: - - apiGroups: - - netbird.io - apiVersions: - - v1 - operations: - - CREATE - - UPDATE - resources: - - "nbsetupkeys" - sideEffects: None {{- if and $.Values.ingress.enabled (or .Values.netbirdAPI.key .Values.netbirdAPI.keyFromSecret) }} --- apiVersion: admissionregistration.k8s.io/v1 @@ -172,4 +132,4 @@ metadata: {{ include "kubernetes-operator.labels" . | indent 4 }} spec: selfSigned: {} -{{- end }} \ No newline at end of file +{{- end }} diff --git a/internal/webhook/v1/nbsetupkey_webhook.go b/internal/webhook/v1/nbsetupkey_webhook.go deleted file mode 100644 index 0945156..0000000 --- a/internal/webhook/v1/nbsetupkey_webhook.go +++ /dev/null @@ -1,103 +0,0 @@ -package v1 - -import ( - "context" - "fmt" - "strings" - - "github.com/google/uuid" - corev1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/api/errors" - "k8s.io/apimachinery/pkg/types" - ctrl "sigs.k8s.io/controller-runtime" - "sigs.k8s.io/controller-runtime/pkg/client" - logf "sigs.k8s.io/controller-runtime/pkg/log" - "sigs.k8s.io/controller-runtime/pkg/webhook/admission" - - netbirdiov1 "github.com/netbirdio/kubernetes-operator/api/v1" -) - -// nolint:unused -// log is for logging in this package. -var nbsetupkeylog = logf.Log.WithName("nbsetupkey-resource") - -// SetupNBSetupKeyWebhookWithManager registers the webhook for NBSetupKey in the manager. -func SetupNBSetupKeyWebhookWithManager(mgr ctrl.Manager) error { - return ctrl.NewWebhookManagedBy(mgr, &netbirdiov1.NBSetupKey{}). - WithValidator(&NBSetupKeyCustomValidator{client: mgr.GetClient()}). - Complete() -} - -// NBSetupKeyCustomValidator struct is responsible for validating the NBSetupKey resource -// when it is created, updated, or deleted. -type NBSetupKeyCustomValidator struct { - client client.Client -} - -var _ admission.Validator[*netbirdiov1.NBSetupKey] = &NBSetupKeyCustomValidator{} - -// ValidateCreate implements webhook.CustomValidator so a webhook will be registered for the type NBSetupKey. -func (v *NBSetupKeyCustomValidator) ValidateCreate(ctx context.Context, nbSetupKey *netbirdiov1.NBSetupKey) (admission.Warnings, error) { - nbsetupkeylog.Info("Validating NBSetupKey", "namespace", nbSetupKey.Namespace, "name", nbSetupKey.Name) - - if nbSetupKey.Spec.SecretKeyRef.Name == "" { - return nil, fmt.Errorf("spec.secretKeyRef.name is required") - } - - if nbSetupKey.Spec.SecretKeyRef.Key == "" { - return nil, fmt.Errorf("spec.secretKeyRef.key is required") - } - - var secret corev1.Secret - err := v.client.Get(ctx, types.NamespacedName{Namespace: nbSetupKey.Namespace, Name: nbSetupKey.Spec.SecretKeyRef.Name}, &secret) - if err != nil { - if errors.IsNotFound(err) { - return admission.Warnings{fmt.Sprintf("secret %s/%s not found", nbSetupKey.Namespace, nbSetupKey.Spec.SecretKeyRef.Name)}, nil - } - return nil, err - } - - uuidBytes, ok := secret.Data[nbSetupKey.Spec.SecretKeyRef.Key] - if !ok { - return admission.Warnings{fmt.Sprintf("key %s in secret %s/%s not found", nbSetupKey.Spec.SecretKeyRef.Key, nbSetupKey.Namespace, nbSetupKey.Spec.SecretKeyRef.Name)}, nil - } - - _, err = uuid.Parse(string(uuidBytes)) - if err != nil { - return admission.Warnings{fmt.Sprintf("setupkey %s in secret %s/%s is not a valid setup key", nbSetupKey.Spec.SecretKeyRef.Key, nbSetupKey.Namespace, nbSetupKey.Spec.SecretKeyRef.Name)}, nil - } - - return nil, nil -} - -// ValidateUpdate implements webhook.CustomValidator so a webhook will be registered for the type NBSetupKey. -func (v *NBSetupKeyCustomValidator) ValidateUpdate(ctx context.Context, old, new *netbirdiov1.NBSetupKey) (admission.Warnings, error) { - return v.ValidateCreate(ctx, new) -} - -// ValidateDelete implements webhook.CustomValidator so a webhook will be registered for the type NBSetupKey. -func (v *NBSetupKeyCustomValidator) ValidateDelete(ctx context.Context, nbSetupKey *netbirdiov1.NBSetupKey) (admission.Warnings, error) { - nbsetupkeylog.Info("Validating NBSetupKey deletion", "namespace", nbSetupKey.Namespace, "name", nbSetupKey.Name) - - var pods corev1.PodList - err := v.client.List(ctx, &pods, client.InNamespace(nbSetupKey.Namespace)) - if err != nil { - return nil, err - } - - //nolint:prealloc - var invalidPods []string - for _, p := range pods.Items { - // If annotation doesn't exist, or doesn't match NBSetupKey being deleted, ignore - if v, ok := p.Annotations[setupKeyAnnotation]; !ok || v != nbSetupKey.Name { - continue - } - invalidPods = append(invalidPods, p.Name) - } - - if len(invalidPods) > 0 { - return nil, fmt.Errorf("NBSetupKey is in-use by %d pods: %s", len(invalidPods), strings.Join(invalidPods, ",")) - } - - return nil, nil -} diff --git a/internal/webhook/v1/nbsetupkey_webhook_test.go b/internal/webhook/v1/nbsetupkey_webhook_test.go deleted file mode 100644 index e7b9309..0000000 --- a/internal/webhook/v1/nbsetupkey_webhook_test.go +++ /dev/null @@ -1,210 +0,0 @@ -package v1 - -import ( - "context" - - . "github.com/onsi/ginkgo/v2" - . "github.com/onsi/gomega" - corev1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/api/errors" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/types" - - netbirdiov1 "github.com/netbirdio/kubernetes-operator/api/v1" -) - -var _ = Describe("NBSetupKey Webhook", func() { - var ( - obj *netbirdiov1.NBSetupKey - validator NBSetupKeyCustomValidator - resourceName = "test" - secret *corev1.Secret - ) - - BeforeEach(func() { - obj = &netbirdiov1.NBSetupKey{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test", - Namespace: "default", - }, - } - validator = NBSetupKeyCustomValidator{ - client: k8sClient, - } - }) - - Context("When creating or updating NBSetupKey under Validating Webhook", func() { - When("secretKeyRef is empty", func() { - It("Should fail", func() { - obj.Spec = netbirdiov1.NBSetupKeySpec{} - warnings, err := validator.ValidateCreate(context.Background(), obj) - Expect(err).To(HaveOccurred()) - Expect(warnings).To(BeEmpty()) - }) - }) - - When("secret doesn't exist", func() { - It("Should fail", func() { - obj.Spec = netbirdiov1.NBSetupKeySpec{ - SecretKeyRef: corev1.SecretKeySelector{ - LocalObjectReference: corev1.LocalObjectReference{ - Name: resourceName, - }, - Key: "setupkey", - }, - } - warnings, err := validator.ValidateCreate(context.Background(), obj) - Expect(err).NotTo(HaveOccurred()) - Expect(warnings).NotTo(BeEmpty()) - }) - }) - - Context("secret exists", Ordered, func() { - createSecret := func(secretkey, setupkey string) { - resource := &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: "default", - Name: resourceName, - }, - Data: map[string][]byte{ - secretkey: []byte(setupkey), - }, - } - - secret = &corev1.Secret{} - err := k8sClient.Get(ctx, types.NamespacedName{Namespace: "default", Name: resourceName}, secret) - if err == nil { - Expect(k8sClient.Delete(ctx, secret)).To(Succeed()) - } - Expect(k8sClient.Create(ctx, resource)).To(Succeed()) - } - - BeforeEach(func() { - obj.Spec = netbirdiov1.NBSetupKeySpec{ - SecretKeyRef: corev1.SecretKeySelector{ - LocalObjectReference: corev1.LocalObjectReference{ - Name: resourceName, - }, - Key: "setupkey", - }, - } - }) - - When("secret key doesn't exist", func() { - It("Should fail", func() { - createSecret("wrongkey", "EEEEEEEE-EEEE-EEEE-EEEE-EEEEEEEEEEEE") - warnings, err := validator.ValidateCreate(context.Background(), obj) - Expect(err).NotTo(HaveOccurred()) - Expect(warnings).NotTo(BeEmpty()) - }) - }) - When("setup key is invalid", func() { - It("Should fail", func() { - createSecret("setupkey", "EEEEEEEE") - warnings, err := validator.ValidateCreate(context.Background(), obj) - Expect(err).NotTo(HaveOccurred()) - Expect(warnings).NotTo(BeEmpty()) - }) - }) - When("setup key is valid", func() { - It("Should allow creation", func() { - createSecret("setupkey", "EEEEEEEE-EEEE-EEEE-EEEE-EEEEEEEEEEEE") - warnings, err := validator.ValidateCreate(context.Background(), obj) - Expect(err).NotTo(HaveOccurred()) - Expect(warnings).To(BeEmpty()) - }) - }) - }) - - Context("Delete", func() { - When("No pods exist with annotation", func() { - BeforeEach(func() { - pod := &corev1.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: "default", - Name: "notannotated", - }, - Spec: corev1.PodSpec{ - Containers: []corev1.Container{ - {Name: "test", Image: "test"}, - }, - }, - } - Expect(k8sClient.Create(ctx, pod)).To(Succeed()) - }) - - AfterEach(func() { - pod := &corev1.Pod{} - err := k8sClient.Get(ctx, types.NamespacedName{Namespace: "default", Name: "notannotated"}, pod) - if !errors.IsNotFound(err) { - Expect(err).NotTo(HaveOccurred()) - if len(pod.Finalizers) > 0 { - pod.Finalizers = nil - Expect(k8sClient.Update(ctx, pod)).To(Succeed()) - } - err = k8sClient.Delete(ctx, pod) - if !errors.IsNotFound(err) { - Expect(err).NotTo(HaveOccurred()) - } - } - }) - - It("should allow delete", func() { - obj = &netbirdiov1.NBSetupKey{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test", - Namespace: "default", - }, - } - Expect(validator.ValidateDelete(ctx, obj)).Error().NotTo(HaveOccurred()) - }) - }) - When("Pods exist with annotation", func() { - BeforeEach(func() { - pod := &corev1.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: "default", - Name: "annotated", - Annotations: map[string]string{ - setupKeyAnnotation: "test", - }, - }, - Spec: corev1.PodSpec{ - Containers: []corev1.Container{ - {Name: "test", Image: "test"}, - }, - }, - } - Expect(k8sClient.Create(ctx, pod)).To(Succeed()) - }) - - AfterEach(func() { - pod := &corev1.Pod{} - err := k8sClient.Get(ctx, types.NamespacedName{Namespace: "default", Name: "annotated"}, pod) - if !errors.IsNotFound(err) { - Expect(err).NotTo(HaveOccurred()) - if len(pod.Finalizers) > 0 { - pod.Finalizers = nil - Expect(k8sClient.Update(ctx, pod)).To(Succeed()) - } - err = k8sClient.Delete(ctx, pod) - if !errors.IsNotFound(err) { - Expect(err).NotTo(HaveOccurred()) - } - } - }) - - It("should deny delete", func() { - obj = &netbirdiov1.NBSetupKey{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test", - Namespace: "default", - }, - } - Expect(validator.ValidateDelete(ctx, obj)).Error().To(HaveOccurred()) - }) - }) - }) - }) - -}) diff --git a/internal/webhook/v1/webhook_suite_test.go b/internal/webhook/v1/webhook_suite_test.go index 457ca5e..59c3a8a 100644 --- a/internal/webhook/v1/webhook_suite_test.go +++ b/internal/webhook/v1/webhook_suite_test.go @@ -121,9 +121,6 @@ var _ = BeforeSuite(func() { err = SetupPodWebhookWithManager(mgr, "", "") Expect(err).NotTo(HaveOccurred()) - err = SetupNBSetupKeyWebhookWithManager(mgr) - Expect(err).NotTo(HaveOccurred()) - err = SetupNBGroupWebhookWithManager(mgr) Expect(err).NotTo(HaveOccurred()) diff --git a/test/e2e/e2e_test.go b/test/e2e/e2e_test.go index 20c569c..0a98dd3 100644 --- a/test/e2e/e2e_test.go +++ b/test/e2e/e2e_test.go @@ -264,20 +264,6 @@ var _ = Describe("Manager", Ordered, func() { Eventually(verifyCAInjection).Should(Succeed()) }) - It("should have CA injection for validating webhooks", func() { - By("checking CA injection for validating webhooks") - verifyCAInjection := func(g Gomega) { - cmd := exec.Command("kubectl", "get", - "validatingwebhookconfigurations.admissionregistration.k8s.io", - "kubernetes-operator-vnbsetupkey-webhook", - "-o", "go-template={{ range .webhooks }}{{ .clientConfig.caBundle }}{{ end }}") - vwhOutput, err := utils.Run(cmd) - g.Expect(err).NotTo(HaveOccurred()) - g.Expect(len(vwhOutput)).To(BeNumerically(">", 10)) - } - Eventually(verifyCAInjection).Should(Succeed()) - }) - Context("NBSetupKey", Ordered, func() { Describe("Basic functionality", Ordered, func() { BeforeAll(func() {