diff --git a/api/v1alpha1/dnsclass_webhook.go b/api/v1alpha1/dnsclass_webhook.go index 1a8f0b3..cfdef9b 100644 --- a/api/v1alpha1/dnsclass_webhook.go +++ b/api/v1alpha1/dnsclass_webhook.go @@ -59,18 +59,25 @@ type DNSClassCustomDefaulter struct { var _ webhook.CustomDefaulter = &DNSClassCustomDefaulter{} -// Default implements webhook.Defaulter so a webhook will be registered for the type +// Default implements webhook.CustomDefaulter so a webhook will be registered for the Kind DNSClass func (r *DNSClassCustomDefaulter) Default(ctx context.Context, obj runtime.Object) error { - dnsclass := obj.(*DNSClass) - dnsclasslog.Info("default", "name", dnsclass.Name) + dnsclass, ok := obj.(*DNSClass) + if !ok { + return fmt.Errorf("expected a DNSClass object but got %T", obj) + } + + dnsclasslog.Info("Defaulting process started", "name", dnsclass.GetName()) if dnsclass.Spec.AllowedDNSPolicies == nil { dnsclass.Spec.AllowedDNSPolicies = DefaultDNSPolicies + dnsclasslog.Info("Applied default AllowedDNSPolicies", "name", dnsclass.GetName(), "DefaultPolicies", DefaultDNSPolicies) } if dnsclass.Spec.DNSPolicy == "" { dnsclass.Spec.DNSPolicy = corev1.DNSNone + dnsclasslog.Info("Applied default DNSPolicy", "name", dnsclass.GetName(), "DefaultPolicy", corev1.DNSNone) } + dnsclasslog.Info("Defaulting process completed", "name", dnsclass.GetName()) return nil } @@ -81,21 +88,27 @@ type DNSClassCustomValidator struct { var _ webhook.CustomValidator = &DNSClassCustomValidator{} -// ValidateCreate implements webhook.Validator so a webhook will be registered for the type +// ValidateCreate implements webhook.CustomValidator so a webhook will be registered for the Kind DNSClass func (r *DNSClassCustomValidator) ValidateCreate(ctx context.Context, obj runtime.Object) (admission.Warnings, error) { - dnsclass := obj.(*DNSClass) - dnsclasslog.Info("validate create", "name", dnsclass.Name) + dnsclass, ok := obj.(*DNSClass) + if !ok { + return nil, fmt.Errorf("expected a DNSClass object but got %T", obj) + } + dnsclasslog.Info("Validation for DNSClass upon creation", "name", dnsclass.GetName()) return r.validate(dnsclass) } -// ValidateUpdate implements webhook.Validator so a webhook will be registered for the type +// ValidateUpdate implements webhook.CustomValidator so a webhook will be registered for the Kind DNSClass func (r *DNSClassCustomValidator) ValidateUpdate(ctx context.Context, oldObj, newObj runtime.Object) (admission.Warnings, error) { - dnsclass := newObj.(*DNSClass) - dnsclasslog.Info("validate update", "name", dnsclass.Name) + dnsclass, ok := newObj.(*DNSClass) + if !ok { + return nil, fmt.Errorf("expected a DNSClass object but got %T", newObj) + } + dnsclasslog.Info("Validation for DNSClass upon update", "name", dnsclass.GetName()) return r.validate(dnsclass) } -// ValidateDelete implements webhook.Validator so a webhook will be registered for the type +// ValidateDelete implements webhook.CustomValidator so a webhook will be registered for the Kind DNSClass func (r *DNSClassCustomValidator) ValidateDelete(ctx context.Context, obj runtime.Object) (admission.Warnings, error) { // No validation required on delete for now return nil, nil diff --git a/api/v1alpha1/dnsclass_webhook_test.go b/api/v1alpha1/dnsclass_webhook_test.go index b31b000..4a101cc 100644 --- a/api/v1alpha1/dnsclass_webhook_test.go +++ b/api/v1alpha1/dnsclass_webhook_test.go @@ -17,6 +17,7 @@ limitations under the License. package v1alpha1 import ( + "context" "fmt" "github.com/eminaktas/kubedns-shepherd/test/utils" @@ -28,15 +29,16 @@ import ( var _ = Describe("DNSClass Webhook", Ordered, func() { var dnsclass *DNSClass - AfterEach(func() { - By("Cleaning up the DNSClass instance") - if dnsclass != nil { - Expect(utils.DeleteDNSClass(ctx, k8sClient, dnsclass)).Should(Succeed()) - dnsclass = nil - } - }) - Context("When creating DNSClass with Defaulting Webhook", func() { + AfterEach(func() { + By("Cleaning up the DNSClass instance") + if dnsclass != nil { + Expect(utils.DeleteDNSClass(ctx, k8sClient, dnsclass)).Should(Succeed()) + // Reset DNSClass variable for the next test + dnsclass = nil + } + }) + It("Should apply default values when not provided", func() { By("Creating DNSClass with no fields set") dnsclass = &DNSClass{} @@ -46,17 +48,41 @@ var _ = Describe("DNSClass Webhook", Ordered, func() { Expect(dnsclass.Spec.DNSPolicy).Should(Equal(corev1.DNSNone), "DNSPolicy should default to DNSNone") Expect(dnsclass.Spec.AllowedDNSPolicies).Should(Equal(DefaultDNSPolicies), "AllowedDNSPolicies should default to valid defaults") }) - }) - // Helper to create DNSClass and validate reconciliation - createAndValidateDNSClass := func() { - Expect(utils.CreateDNSClass(ctx, k8sClient, dnsclass)).Should(Succeed(), "Failed to create DNSClass") - By("Syncing DNSClass") - Expect(dnsclass.GetName()).Should(BeEmpty()) - } + It("Should allow updating DNSClass fields", func() { + By("Creating a valid DNSClass") + dnsclass = &DNSClass{} + Expect(utils.CreateDNSClass(ctx, k8sClient, dnsclass)).Should(Succeed()) + + By("Updating the AllowedNamespaces field") + updatedAllowedNamespaces := []string{"default", "another-namespace"} + dnsclass.Spec.AllowedNamespaces = updatedAllowedNamespaces + Expect(k8sClient.Update(ctx, dnsclass)).Should(Succeed()) + + By("Verifying that the DNSClass was updated correctly") + Expect(utils.GetDNSClass(ctx, k8sClient, dnsclass)).Should(Succeed()) + Expect(dnsclass.Spec.AllowedNamespaces).Should(Equal(updatedAllowedNamespaces), "AllowedNamespaces should be updated correctly") + }) + }) Context("When creating DNSClass under Validating Webhook", func() { - It("Should reject invalid AllowedDNSPolicies", func() { + // Helper to create DNSClass and validate reconciliation for validating + createAndValidateDNSClass := func() { + Expect(utils.CreateDNSClass(ctx, k8sClient, dnsclass)).Should(Succeed(), "Failed to create DNSClass") + By("Syncing DNSClass") + Expect(dnsclass.GetName()).Should(BeEmpty()) + } + + AfterEach(func() { + By("Cleaning up the DNSClass instance") + if dnsclass != nil { + Expect(utils.DeleteDNSClass(ctx, k8sClient, dnsclass)).Should(Succeed()) + // Reset DNSClass variable for the next test + dnsclass = nil + } + }) + + It("should reject invalid AllowedDNSPolicies", func() { dnsclass = &DNSClass{ Spec: DNSClassSpec{ AllowedDNSPolicies: []corev1.DNSPolicy{corev1.DNSNone, "InvalidDNSPolicy"}, @@ -65,7 +91,7 @@ var _ = Describe("DNSClass Webhook", Ordered, func() { createAndValidateDNSClass() }) - It("Should reject invalid DNSPolicy", func() { + It("should reject invalid DNSPolicy", func() { By("Creating DNSClass with an invalid DNSPolicy") dnsclass = &DNSClass{ Spec: DNSClassSpec{ @@ -75,7 +101,7 @@ var _ = Describe("DNSClass Webhook", Ordered, func() { createAndValidateDNSClass() }) - It("Should reject DNSConfig when DNSPolicy is not `None`", func() { + It("should reject DNSConfig when DNSPolicy is not `None`", func() { By("Creating DNSClass with DNSConfig but without DNSPolicy None") dnsconfig := &corev1.PodDNSConfig{ Nameservers: []string{utils.ClusterDNS}, @@ -91,7 +117,7 @@ var _ = Describe("DNSClass Webhook", Ordered, func() { createAndValidateDNSClass() }) - It("Should reject DNSConfig with invalid template keys", func() { + It("should reject DNSConfig with invalid template keys", func() { By("Creating DNSClass with invalid template keys in DNSConfig.Searches") dnsconfig := &corev1.PodDNSConfig{ Searches: []string{"svc.{{ .podNamespace }}", "{{ .clusterDomainInvalid }}"}, @@ -104,20 +130,34 @@ var _ = Describe("DNSClass Webhook", Ordered, func() { } createAndValidateDNSClass() }) + }) - It("Should allow updating DNSClass fields", func() { - By("Creating a valid DNSClass") - dnsclass = &DNSClass{} - Expect(utils.CreateDNSClass(ctx, k8sClient, dnsclass)).Should(Succeed()) + Context("When calling functions directly", func() { + It("should return error when calling Default function with different object", func() { + dnsCustomDefault := &DNSClassCustomDefaulter{} + err := dnsCustomDefault.Default(context.TODO(), &corev1.Pod{}) + Expect(err).Should(HaveOccurred()) + }) - By("Updating the AllowedNamespaces field") - updatedAllowedNamespaces := []string{"default", "another-namespace"} - dnsclass.Spec.AllowedNamespaces = updatedAllowedNamespaces - Expect(k8sClient.Update(ctx, dnsclass)).Should(Succeed()) + It("should return error when calling ValidateCreate with different object", func() { + dnsCustomValidator := &DNSClassCustomValidator{} + warn, err := dnsCustomValidator.ValidateCreate(context.TODO(), &corev1.Pod{}) + Expect(warn).Should(BeNil()) + Expect(err).Should(HaveOccurred()) + }) - By("Verifying that the DNSClass was updated correctly") - Expect(utils.GetDNSClass(ctx, k8sClient, dnsclass)).Should(Succeed()) - Expect(dnsclass.Spec.AllowedNamespaces).Should(Equal(updatedAllowedNamespaces), "AllowedNamespaces should be updated correctly") + It("should return error when calling ValidateUpdate with different object", func() { + dnsCustomValidator := &DNSClassCustomValidator{} + warn, err := dnsCustomValidator.ValidateUpdate(context.TODO(), nil, &corev1.Pod{}) + Expect(warn).Should(BeNil()) + Expect(err).Should(HaveOccurred()) + }) + + It("should return error when calling ValidateDelete with different object", func() { + dnsCustomValidator := &DNSClassCustomValidator{} + warn, err := dnsCustomValidator.ValidateDelete(context.TODO(), &corev1.Pod{}) + Expect(warn).Should(BeNil()) + Expect(err).Should(BeNil()) }) }) }) diff --git a/cmd/main.go b/cmd/main.go index 9440035..0033901 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -197,7 +197,8 @@ func main() { os.Exit(1) } if err = (&controller.DNSClassReconciler{ - Client: mgr.GetClient(), + Client: mgr.GetClient(), + EventRecorder: mgr.GetEventRecorderFor("dnsclass-controller"), MaxConcurrentReconcilesForDNSClassReconciler: maxConcurrentReconcilesForDNSClassReconciler, }).SetupWithManager(mgr); err != nil { setupLog.Error(err, "unable to create controller", "controller", "DNSClass") @@ -211,8 +212,9 @@ func main() { mgr.GetWebhookServer().Register("/mutate-v1-pod", &webhook.Admission{ Handler: &webhook_controller.PodMutator{ - Client: mgr.GetClient(), - Decoder: admission.NewDecoder(mgr.GetScheme()), + Client: mgr.GetClient(), + Decoder: admission.NewDecoder(mgr.GetScheme()), + EventRecorder: mgr.GetEventRecorderFor("pod-mutator-webhook-controller"), }, }) //+kubebuilder:scaffold:builder diff --git a/internal/controller/dnsclass_controller.go b/internal/controller/dnsclass_controller.go index 733eb20..110ccde 100644 --- a/internal/controller/dnsclass_controller.go +++ b/internal/controller/dnsclass_controller.go @@ -32,6 +32,7 @@ import ( "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" + "k8s.io/client-go/tools/record" "k8s.io/client-go/util/retry" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" @@ -53,6 +54,7 @@ type Workload struct { // DNSClassReconciler reconciles a DNSClass object type DNSClassReconciler struct { client.Client + record.EventRecorder MaxConcurrentReconcilesForDNSClassReconciler int } @@ -70,7 +72,7 @@ type DNSClassReconciler struct { // Reconcile reconciles a DNSClass object func (r *DNSClassReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { logger := log.FromContext(ctx) - logger.Info("Reconciling for DNSClass") + logger.Info("Reconciling") dnsclass := &configv1alpha1.DNSClass{} err := r.Get(ctx, req.NamespacedName, dnsclass) @@ -83,6 +85,8 @@ func (r *DNSClassReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c return ctrl.Result{}, err } + r.EventRecorder.Event(dnsclass, corev1.EventTypeNormal, "Reconciling", "Reconciliation started") + // Set status to Init if it is not set if dnsclass.Status.State == "" { dnsclass.Status.State = configv1alpha1.StateInit @@ -107,6 +111,7 @@ func (r *DNSClassReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c for _, key := range dnsclass.ExtractTemplateKeysRegex() { if !slices.Contains(undiscoveredFields, key) { err = errors.Errorf("failed to discover template keys: %v", strings.Join(undiscoveredFields, ", ")) + r.EventRecorder.Event(dnsclass, corev1.EventTypeWarning, "Failed", "DNSClass will be unavailable due to missing required template keys") return ctrl.Result{}, err } } @@ -120,7 +125,8 @@ func (r *DNSClassReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c dnsclass.Status.State = configv1alpha1.StateReady statusMessage = "Ready" - logger.Info("Reconciling completed for DNSClass") + logger.Info("Reconciling successfully completed") + r.EventRecorder.Event(dnsclass, corev1.EventTypeNormal, "Successful", "Reconciliation successfully completed") return ctrl.Result{}, nil } diff --git a/internal/controller/suite_test.go b/internal/controller/suite_test.go index 69e21fc..dd36c8b 100644 --- a/internal/controller/suite_test.go +++ b/internal/controller/suite_test.go @@ -94,7 +94,8 @@ var _ = BeforeSuite(func() { Expect(err).ToNot(HaveOccurred()) err = (&DNSClassReconciler{ - Client: k8sManager.GetClient(), + Client: k8sManager.GetClient(), + EventRecorder: k8sManager.GetEventRecorderFor("dnsclass-controller"), }).SetupWithManager(k8sManager) Expect(err).ToNot(HaveOccurred()) diff --git a/internal/webhook/pods_webhook.go b/internal/webhook/pods_webhook.go index 01cec18..162cd7f 100644 --- a/internal/webhook/pods_webhook.go +++ b/internal/webhook/pods_webhook.go @@ -26,6 +26,7 @@ import ( "text/template" corev1 "k8s.io/api/core/v1" + "k8s.io/client-go/tools/record" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/webhook/admission" @@ -35,9 +36,18 @@ import ( const DNSClassName = "kubedns-shepherd.io/dns-class-name" +var ( + ErrPodDecode = errors.New("failed to decode pod object") + ErrNoDNSClass = errors.New("no matching DNSClass found") + ErrNotAvailableDNSClass = errors.New("%s DNSClass not currently available") + ErrDNSConfig = errors.New("failed to configure %s DNSClass") + ErrMarshal = errors.New("json marshal failed") +) + type PodMutator struct { client.Client admission.Decoder + record.EventRecorder } // +kubebuilder:webhook:path=/mutate-v1-pod,mutating=true,failurePolicy=ignore,sideEffects=None,groups="",resources=pods,verbs=create;update,versions=v1,name=mpod.kb.io,admissionReviewVersions=v1 @@ -45,52 +55,90 @@ type PodMutator struct { func (p *PodMutator) Handle(ctx context.Context, req admission.Request) admission.Response { logger := log.FromContext(ctx) - skipMsg := fmt.Sprintf("Skipping DNS configuration for %s/%s", req.Namespace, req.Name) pod := &corev1.Pod{} err := p.Decode(req, pod) if err != nil { - msg := fmt.Sprintf("Failed to decode the pod. %s", skipMsg) - logger.Error(err, msg) - return admission.Allowed(msg) + logger.Error(err, ErrPodDecode.Error()) + p.Event(pod, corev1.EventTypeWarning, "FailedDNSClassMutation", ErrPodDecode.Error()) + return admission.Allowed(ErrPodDecode.Error()) } - podName := pod.GetName() - if pod.Name == "" { - podName = fmt.Sprintf("%s... (name pending generation)", pod.GetGenerateName()) + // Create a deep copy of the pod to safely modify its fields for events reference + copyPod := pod.DeepCopy() + + // Set the pod name in the copy if it hasn't been generated yet + // This ensures a name is available for events even if the original pod is pending name generation + if pod.GetName() == "" { + copyPod.SetName(fmt.Sprintf("%s", pod.GetGenerateName())) } - dnsclass, err := getDNSClass(ctx, p.Client, pod.Namespace, pod.Spec.DNSPolicy) + dnsclass, err := p.getDNSClass(ctx, pod) if err != nil { - msg := fmt.Sprintf("Failed to detect a DNSClass. %s", skipMsg) - logger.Info(msg, "error", err) - return admission.Allowed(msg) + logger.Info(err.Error()) + p.Event(copyPod, corev1.EventTypeNormal, "SkippedDNSClassMutation", err.Error()) + return admission.Allowed(err.Error()) } + dnsclassName := dnsclass.GetName() + if dnsclass.Status.State != configv1alpha1.StateReady { - msg := fmt.Sprintf("DNSClass %s is not available at the moment. %s", dnsclass.Name, skipMsg) - logger.Info(msg) - return admission.Allowed(msg) + logger.Info(ErrNotAvailableDNSClass.Error(), "dnsclass", dnsclassName) + p.Eventf(copyPod, corev1.EventTypeWarning, "FailedDNSClassMutation", ErrNotAvailableDNSClass.Error(), dnsclassName) + return admission.Allowed(fmt.Sprintf(ErrNotAvailableDNSClass.Error(), dnsclassName)) } err = configureDNSForPod(pod, dnsclass) if err != nil { - msg := fmt.Sprintf("Failed to configure DNS for pod. %s", skipMsg) - logger.Error(err, msg) - return admission.Allowed(msg) + logger.Error(err, ErrDNSConfig.Error()) + p.Eventf(copyPod, corev1.EventTypeWarning, "FailedDNSClassMutation", ErrDNSConfig.Error(), dnsclassName) + return admission.Allowed(fmt.Sprintf(ErrDNSConfig.Error(), dnsclassName)) } marshaledPod, err := json.Marshal(pod) if err != nil { - msg := fmt.Sprintf("Failed to marshal pod after DNS configuration. %s", skipMsg) - logger.Error(err, msg) - return admission.Allowed(msg) + logger.Error(err, ErrMarshal.Error()) + p.Event(copyPod, corev1.EventTypeWarning, "FailedDNSClassMutation", ErrMarshal.Error()) + return admission.Allowed(ErrMarshal.Error()) } - logger.Info(fmt.Sprintf("Successfully configured DNS for pod %s/%s with DNSClass %s", pod.Namespace, podName, dnsclass.Name)) + logger.Info(fmt.Sprintf("Successfully configured DNS for pod %s/%s with DNSClass %s", pod.GetNamespace(), pod.GetName(), dnsclassName)) + p.Event(copyPod, corev1.EventTypeNormal, "SuccessfulDNSClassMutation", fmt.Sprintf("DNS configuration successfully applied from DNSClass resource '%s'", dnsclassName)) return admission.PatchResponseFromRaw(req.Object.Raw, marshaledPod) } +// GetDNSClass finds the DNSClass for the pod object with its namespace +func (p *PodMutator) getDNSClass(ctx context.Context, pod *corev1.Pod) (configv1alpha1.DNSClass, error) { + var dnsClassList configv1alpha1.DNSClassList + var dnsClass configv1alpha1.DNSClass + + err := p.List(ctx, &dnsClassList) + if err != nil { + return configv1alpha1.DNSClass{}, err + } + + podNamespace := pod.GetNamespace() + + for _, dnsClassItem := range dnsClassList.Items { + // Skip DNSClass if the namespace is disabled + if slices.Contains(dnsClassItem.Spec.DisabledNamespaces, podNamespace) { + continue + } + + // Ensure the DNS policy is allowed + if !slices.Contains(dnsClassItem.Spec.AllowedDNSPolicies, pod.Spec.DNSPolicy) { + continue + } + + // Select the DNSClass if it's allowed for the namespace or no specific namespaces are defined + if len(dnsClassItem.Spec.AllowedNamespaces) == 0 || slices.Contains(dnsClassItem.Spec.AllowedNamespaces, podNamespace) { + return dnsClassItem, nil + } + } + + return dnsClass, ErrNoDNSClass +} + func configureDNSForPod(pod *corev1.Pod, dnsClass configv1alpha1.DNSClass) error { if dnsClass.Spec.DNSPolicy == corev1.DNSNone { searches := []string{} @@ -162,33 +210,3 @@ func updateAnnotation(obj client.Object, key, value string) { annotations[key] = value obj.SetAnnotations(annotations) } - -// GetDNSClass finds the DNSClass for the pod object with its namespace -func getDNSClass(ctx context.Context, c client.Client, podNamespace string, podDNSPolicy corev1.DNSPolicy) (configv1alpha1.DNSClass, error) { - var dnsClassList configv1alpha1.DNSClassList - var dnsClass configv1alpha1.DNSClass - - err := c.List(ctx, &dnsClassList) - if err != nil { - return configv1alpha1.DNSClass{}, err - } - - for _, dnsClassItem := range dnsClassList.Items { - // Skip DNSClass if the namespace is disabled - if slices.Contains(dnsClassItem.Spec.DisabledNamespaces, podNamespace) { - continue - } - - // Ensure the DNS policy is allowed - if !slices.Contains(dnsClassItem.Spec.AllowedDNSPolicies, podDNSPolicy) { - continue - } - - // Select the DNSClass if it's allowed for the namespace or no specific namespaces are defined - if len(dnsClassItem.Spec.AllowedNamespaces) == 0 || slices.Contains(dnsClassItem.Spec.AllowedNamespaces, podNamespace) { - return dnsClassItem, nil - } - } - - return dnsClass, errors.New("no matching DNSClass found for namespace: " + podNamespace) -} diff --git a/internal/webhook/pods_webhook_test.go b/internal/webhook/pods_webhook_test.go index 9449114..569e046 100644 --- a/internal/webhook/pods_webhook_test.go +++ b/internal/webhook/pods_webhook_test.go @@ -17,6 +17,7 @@ limitations under the License. package webhook_controller import ( + "context" "fmt" "time" @@ -24,10 +25,15 @@ import ( "github.com/eminaktas/kubedns-shepherd/test/utils" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" + admissionv1 "k8s.io/api/admission/v1" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" + "k8s.io/client-go/kubernetes/scheme" utilptr "k8s.io/utils/ptr" ) @@ -195,7 +201,108 @@ var _ = Describe("Pods Webhook Controller", Ordered, func() { Expect(utils.CreateKubeletConfigMap(ctx, k8sClient)).Should(Succeed()) }) - It("should fail to apply DNSClass to pod with templating due to missing kubelet", func() { + It("should not apply DNSClass to pod when nameservers field is missing", func() { + By("Deleting kubelet config map") + Expect(utils.DeleteKubeletConfigMap(ctx, k8sClient)).Should(Succeed()) + + dnsconfig := &corev1.PodDNSConfig{} + createAndValidateDNSClass(dnsconfig, corev1.DNSNone, []string{ns.Name}, nil, nil, configv1alpha1.StateReady) + + createPodAndValidate(corev1.DNSClusterFirst, corev1.DNSClusterFirst) + + By("Restoring kubelet config map") + Expect(utils.CreateKubeletConfigMap(ctx, k8sClient)).Should(Succeed()) + }) + + It("should not apply DNSClass to pod when pod namespace is disabled", func() { + dnsconfig := &corev1.PodDNSConfig{} + createAndValidateDNSClass(dnsconfig, corev1.DNSNone, nil, []string{"kube-system", ns.Name}, nil, configv1alpha1.StateReady) + + createPodAndValidate(corev1.DNSClusterFirst, corev1.DNSClusterFirst) + }) + + It("should not apply DNSClass to pod when a namespace is not in allowed list", func() { + dnsconfig := &corev1.PodDNSConfig{} + createAndValidateDNSClass(dnsconfig, corev1.DNSNone, []string{"dummy-namespace"}, nil, nil, configv1alpha1.StateReady) + + createPodAndValidate(corev1.DNSDefault, corev1.DNSDefault) + }) + + It("should not apply DNSClass to pod when a DNSPolicy is not allowed", func() { + dnsconfig := &corev1.PodDNSConfig{} + createAndValidateDNSClass(dnsconfig, corev1.DNSNone, nil, nil, []corev1.DNSPolicy{corev1.DNSNone}, configv1alpha1.StateReady) + + createPodAndValidate(corev1.DNSDefault, corev1.DNSDefault) + }) + + It("should apply DNSClass to pod when DNSConfig is nil", func() { + createAndValidateDNSClass(nil, corev1.DNSClusterFirst, nil, nil, nil, configv1alpha1.StateReady) + + createPodAndValidate(corev1.DNSClusterFirstWithHostNet, corev1.DNSClusterFirst) + + By("Validating Pod annotations") + Expect(pod.GetAnnotations()[DNSClassName]).Should(Equal(dnsclass.Name)) + }) + + It("should apply DNSClass to pod when DNSConfig is nil and DNSPolicy is None", func() { + createAndValidateDNSClass(nil, corev1.DNSNone, nil, nil, nil, configv1alpha1.StateReady) + + createPodAndValidate(corev1.DNSClusterFirstWithHostNet, corev1.DNSNone) + + By("Validating Pod annotations") + Expect(pod.GetAnnotations()[DNSClassName]).Should(Equal(dnsclass.Name)) + }) + }) + + Context("When calling functions directly", func() { + It("should fail to decode pod object", func() { + k8sManager, err := ctrl.NewManager(cfg, ctrl.Options{ + Scheme: scheme.Scheme, + }) + Expect(err).NotTo(HaveOccurred()) + + podMutator := &PodMutator{ + Client: k8sManager.GetClient(), + Decoder: admission.NewDecoder(k8sManager.GetScheme()), + EventRecorder: k8sManager.GetEventRecorderFor("pod-mutator-webhook-controller"), + } + + podReq := admission.Request{ + AdmissionRequest: admissionv1.AdmissionRequest{ + Kind: metav1.GroupVersionKind{ + Kind: "Pod", + Version: "v1", + Group: "", + }, + Name: "test-pod-dummy", + Namespace: "test-ns-dummy", + Object: runtime.RawExtension{ + Raw: nil, + }, + }, + } + admissionResponse := podMutator.Handle(context.TODO(), podReq) + Expect(admissionResponse.AdmissionResponse.Allowed).Should(BeTrue()) + }) + + It("should fail to list dnsclass", func() { + k8sManager, err := ctrl.NewManager(cfg, ctrl.Options{ + Scheme: scheme.Scheme, + }) + Expect(err).NotTo(HaveOccurred()) + + podMutator := &PodMutator{ + Client: k8sManager.GetClient(), + } + + pod := &corev1.Pod{} + + dnsClass, err := podMutator.getDNSClass(context.TODO(), pod) + Expect(dnsClass).Should(Equal(configv1alpha1.DNSClass{})) + Expect(err).Should(HaveOccurred()) + }) + + It("should fail at template execute due to missing key", func() { dnsconfig := &corev1.PodDNSConfig{ Searches: []string{ "svc.{{ .clusterDomain }}", @@ -214,8 +321,8 @@ var _ = Describe("Pods Webhook Controller", Ordered, func() { } localPod := &corev1.Pod{ ObjectMeta: metav1.ObjectMeta{ - Name: "test-pod-templating", - Namespace: ns.Name, + Name: "test-pod-dummy", + Namespace: "test-ns-dummy", }, Spec: corev1.PodSpec{ Containers: []corev1.Container{ @@ -231,7 +338,7 @@ var _ = Describe("Pods Webhook Controller", Ordered, func() { Expect(err).Should(HaveOccurred()) }) - It("should fail to apply DNSClass to pod with templating due to wrong delimiter", func() { + It("should fail at template execute", func() { dnsconfig := &corev1.PodDNSConfig{ Searches: []string{"svc.[[ .clusterDomain ]]"}, } @@ -247,8 +354,8 @@ var _ = Describe("Pods Webhook Controller", Ordered, func() { } localPod := &corev1.Pod{ ObjectMeta: metav1.ObjectMeta{ - Name: "test-pod-templating", - Namespace: ns.Name, + Name: "test-pod-dummy", + Namespace: "test-ns-dummy", }, Spec: corev1.PodSpec{ Containers: []corev1.Container{ @@ -264,56 +371,22 @@ var _ = Describe("Pods Webhook Controller", Ordered, func() { Expect(err).Should(HaveOccurred()) }) - It("should not apply DNSClass to pod when nameservers field is missing", func() { - By("Deleting kubelet config map") - Expect(utils.DeleteKubeletConfigMap(ctx, k8sClient)).Should(Succeed()) - - dnsconfig := &corev1.PodDNSConfig{} - createAndValidateDNSClass(dnsconfig, corev1.DNSNone, []string{ns.Name}, nil, nil, configv1alpha1.StateReady) - - createPodAndValidate(corev1.DNSClusterFirst, corev1.DNSClusterFirst) - - By("Restoring kubelet config map") - Expect(utils.CreateKubeletConfigMap(ctx, k8sClient)).Should(Succeed()) - }) - - It("should not apply DNSClass to pod when pod namespace is disabled", func() { - dnsconfig := &corev1.PodDNSConfig{} - createAndValidateDNSClass(dnsconfig, corev1.DNSNone, nil, []string{"kube-system", ns.Name}, nil, configv1alpha1.StateReady) - - createPodAndValidate(corev1.DNSClusterFirst, corev1.DNSClusterFirst) - }) - - It("should not apply DNSClass to pod when a namespace is not in allowed list", func() { - dnsconfig := &corev1.PodDNSConfig{} - createAndValidateDNSClass(dnsconfig, corev1.DNSNone, []string{"dummy-namespace"}, nil, nil, configv1alpha1.StateReady) - - createPodAndValidate(corev1.DNSDefault, corev1.DNSDefault) - }) - - It("should not apply DNSClass to pod when a DNSPolicy is not allowed", func() { - dnsconfig := &corev1.PodDNSConfig{} - createAndValidateDNSClass(dnsconfig, corev1.DNSNone, nil, nil, []corev1.DNSPolicy{corev1.DNSNone}, configv1alpha1.StateReady) - - createPodAndValidate(corev1.DNSDefault, corev1.DNSDefault) - }) - - It("should apply DNSClass to pod when DNSConfig is nil", func() { - createAndValidateDNSClass(nil, corev1.DNSClusterFirst, nil, nil, nil, configv1alpha1.StateReady) - - createPodAndValidate(corev1.DNSClusterFirstWithHostNet, corev1.DNSClusterFirst) - - By("Validating Pod annotations") - Expect(pod.GetAnnotations()[DNSClassName]).Should(Equal(dnsclass.Name)) - }) - - It("should apply DNSClass to pod when DNSConfig is nil and DNSPolicy is None", func() { - createAndValidateDNSClass(nil, corev1.DNSNone, nil, nil, nil, configv1alpha1.StateReady) - - createPodAndValidate(corev1.DNSClusterFirstWithHostNet, corev1.DNSNone) + It("should fail at template parse", func() { + pod := &corev1.Pod{} + dnsclass := configv1alpha1.DNSClass{ + Spec: configv1alpha1.DNSClassSpec{ + DNSConfig: &corev1.PodDNSConfig{ + Searches: []string{"{{}}"}, + }, + DNSPolicy: corev1.DNSNone, + }, + Status: configv1alpha1.DNSClassStatus{ + DiscoveredFields: &configv1alpha1.DiscoveredFields{}, + }, + } - By("Validating Pod annotations") - Expect(pod.GetAnnotations()[DNSClassName]).Should(Equal(dnsclass.Name)) + err := configureDNSForPod(pod, dnsclass) + Expect(err).Should(HaveOccurred()) }) }) }) diff --git a/internal/webhook/suite_test.go b/internal/webhook/suite_test.go index 7878bdc..11ac953 100644 --- a/internal/webhook/suite_test.go +++ b/internal/webhook/suite_test.go @@ -116,7 +116,8 @@ var _ = BeforeSuite(func() { Expect(err).NotTo(HaveOccurred()) err = (&controller.DNSClassReconciler{ - Client: k8sManager.GetClient(), + Client: k8sManager.GetClient(), + EventRecorder: k8sManager.GetEventRecorderFor("dnsclass-controller"), }).SetupWithManager(k8sManager) Expect(err).ToNot(HaveOccurred()) @@ -125,8 +126,9 @@ var _ = BeforeSuite(func() { // Set up pod webhook k8sManager.GetWebhookServer().Register("/mutate-v1-pod", &webhook.Admission{ Handler: &PodMutator{ - Client: k8sManager.GetClient(), - Decoder: admission.NewDecoder(k8sManager.GetScheme()), + Client: k8sManager.GetClient(), + Decoder: admission.NewDecoder(k8sManager.GetScheme()), + EventRecorder: k8sManager.GetEventRecorderFor("pod-mutator-webhook-controller"), }, })