fix(ServiceExport): Reject Conflicting Alias Names for ServiceExport …

…object Added a validating webhook to verify alias names in serviceexport object and reject and if there are conflicting aliases Fixes #315 Signed-off-by: Md Soharab Ansari <[email protected]>
kubeslice · Apr 7, 2024 · cd03978 · cd03978
1 parent 236a5d6
commit cd03978
Show file tree

Hide file tree

Showing 6 changed files with 246 additions and 0 deletions.
diff --git a/config/webhook/manifests.yaml b/config/webhook/manifests.yaml
@@ -28,3 +28,29 @@ webhooks:
     - statefulsets
     - daemonsets
   sideEffects: NoneOnDryRun
+---
+apiVersion: admissionregistration.k8s.io/v1
+kind: ValidatingWebhookConfiguration
+metadata:
+  name: validating-webhook-configuration
+webhooks:
+- admissionReviewVersions:
+  - v1
+  clientConfig:
+    service:
+      name: webhook-service
+      namespace: system
+      path: /validate-webhook
+  failurePolicy: Fail
+  name: webhook.kubeslice.io
+  rules:
+  - apiGroups:
+    - networking.kubeslice.io
+    apiVersions:
+    - v1beta1
+    operations:
+    - CREATE
+    - UPDATE
+    resources:
+    - serviceexports
+  sideEffects: NoneOnDryRun
diff --git a/controllers/slice/reconciler.go b/controllers/slice/reconciler.go
@@ -81,6 +81,7 @@ type SliceReconciler struct {
 //+kubebuilder:rbac:groups=networking.istio.io,resources=serviceentries,verbs=get;list;create;update;watch;delete
 //+kubebuilder:rbac:groups=networking.istio.io,resources=virtualservices,verbs=get;list;create;update;watch;delete
 //+kubebuilder:webhook:path=/mutate-webhook,mutating=true,failurePolicy=fail,groups="";apps,resources=pods;deployments;statefulsets;daemonsets,verbs=create;update,versions=v1,name=webhook.kubeslice.io,admissionReviewVersions=v1,sideEffects=NoneOnDryRun
+//+kubebuilder:webhook:path=/validate-webhook,mutating=false,failurePolicy=fail,groups="networking.kubeslice.io",resources=serviceexports,verbs=create;update,versions=v1beta1,name=webhook.kubeslice.io,admissionReviewVersions=v1,sideEffects=NoneOnDryRun
 //+kubebuilder:rbac:groups=core,resources=nodes,verbs=get;list;watch
 //+kubebuilder:rbac:groups=core,resources=namespaces,verbs=get;list;watch;create;update;patch;delete
 //+kubebuilder:rbac:groups=networking.k8s.io,resources=networkpolicies,verbs=get;list;watch;create;update;patch;delete

diff --git a/main.go b/main.go
@@ -140,6 +140,13 @@ func main() {
 				Decoder:         admission.NewDecoder(mgr.GetScheme()),
 			},
 		})
+		mgr.GetWebhookServer().Register("/validate-webhook", &webhook.Admission{
+			Handler: &podwh.WebhookServer{
+				Client:          mgr.GetClient(),
+				SliceInfoClient: podwh.NewWebhookClient(),
+				Decoder:         admission.NewDecoder(mgr.GetScheme()),
+			},
+		})
 	}
 	if err != nil {
 		setupLog.With("error", err).Error("unable to start manager")

diff --git a/pkg/webhook/pod/webhook.go b/pkg/webhook/pod/webhook.go
@@ -25,6 +25,7 @@ import (
 	"net/http"
 
 	"github.com/kubeslice/apis/pkg/controller/v1alpha1"
+	"github.com/kubeslice/worker-operator/api/v1beta1"
 	"github.com/kubeslice/worker-operator/controllers"
 	"github.com/kubeslice/worker-operator/pkg/logger"
 	v1 "k8s.io/api/admission/v1"
@@ -53,6 +54,7 @@ type SliceInfoProvider interface {
 	SliceAppNamespaceConfigured(ctx context.Context, slice string, namespace string) (bool, error)
 	GetNamespaceLabels(ctx context.Context, client client.Client, namespace string) (map[string]string, error)
 	GetSliceOverlayNetworkType(ctx context.Context, client client.Client, sliceName string) (v1alpha1.NetworkType, error)
+	GetAllServiceExports(ctx context.Context, client client.Client, slice string) (*v1beta1.ServiceExportList, error)
 }
 
 type WebhookServer struct {
@@ -151,6 +153,24 @@ func (wh *WebhookServer) Handle(ctx context.Context, req admission.Request) admi
 			return admission.Errored(http.StatusInternalServerError, err)
 		}
 		return admission.PatchResponseFromRaw(req.Object.Raw, marshaled)
+	} else if req.Kind.Kind == "ServiceExport" {
+		serviceexport := &v1beta1.ServiceExport{}
+		err := wh.Decoder.Decode(req, serviceexport)
+		if err != nil {
+			return admission.Errored(http.StatusBadRequest, err)
+		}
+		log := logger.FromContext(ctx)
+
+		log.Info("validating serviceexport", "serviceexport spec", serviceexport.Spec)
+		validation, conflictingAlias, err := wh.ValidateServiceExport(serviceexport, ctx)
+		if err != nil {
+			return admission.Errored(http.StatusInternalServerError, err)
+		}
+		if !validation {
+			log.Info("serviceexport validation failed: alias already exist", "serviceexport-name", serviceexport.ObjectMeta.Name)
+			return admission.Denied(fmt.Sprintf("Alias %s already exist", conflictingAlias))
+		}
+		return admission.Allowed("")
 	}
 
 	return admission.Response{AdmissionResponse: v1.AdmissionResponse{
@@ -257,6 +277,33 @@ func MutateDaemonSet(ds *appsv1.DaemonSet, sliceName string) *appsv1.DaemonSet {
 	return ds
 }
 
+func (wh *WebhookServer) ValidateServiceExport(svcex *v1beta1.ServiceExport, ctx context.Context) (bool, string, error) {
+
+	log := logger.FromContext(ctx)
+	log.Info("fetching all serviceexport objects belonging to the slice", "slice", svcex.Spec.Slice)
+	serviceExportList, err := wh.SliceInfoClient.GetAllServiceExports(context.Background(), wh.Client, svcex.Spec.Slice)
+	if err != nil {
+		return false, "", err
+	}
+
+	newAliases := svcex.Spec.Aliases
+
+	for _, serviceExport := range serviceExportList.Items {
+		// In case we are updating an existing ServiceExport resource
+		if svcex.ObjectMeta.Name == serviceExport.ObjectMeta.Name &&
+			svcex.ObjectMeta.Namespace == serviceExport.ObjectMeta.Namespace {
+			continue
+		}
+		existingAliases := serviceExport.Spec.Aliases
+		for _, newAlias := range newAliases {
+			if aliasExist(existingAliases, newAlias) {
+				return false, newAlias, nil
+			}
+		}
+	}
+	return true, "", nil
+}
+
 func (wh *WebhookServer) MutationRequired(metadata metav1.ObjectMeta, ctx context.Context, kind string) (bool, string) {
 	log := logger.FromContext(ctx)
 	annotations := metadata.GetAnnotations()

diff --git a/pkg/webhook/pod/webhook_test.go b/pkg/webhook/pod/webhook_test.go
@@ -26,6 +26,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
 	"github.com/kubeslice/apis/pkg/controller/v1alpha1"
+	"github.com/kubeslice/worker-operator/api/v1beta1"
 	"github.com/kubeslice/worker-operator/controllers"
 	"github.com/kubeslice/worker-operator/pkg/webhook/pod"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -45,6 +46,33 @@ func (f fakeWebhookClient) GetNamespaceLabels(ctx context.Context, client client
 	return map[string]string{controllers.ApplicationNamespaceSelectorLabelKey: "green"}, nil
 }
 
+func (f fakeWebhookClient) GetAllServiceExports(ctx context.Context, client client.Client, slice string) (*v1beta1.ServiceExportList, error) {
+	return &v1beta1.ServiceExportList{
+		Items: []v1beta1.ServiceExport{
+			{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "svcex-1",
+					Namespace: "test-ns-1",
+				},
+				Spec: v1beta1.ServiceExportSpec{
+					Slice:   "test-slice",
+					Aliases: []string{"server.com"},
+				},
+			},
+			{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "svcex-2",
+					Namespace: "test-ns-2",
+				},
+				Spec: v1beta1.ServiceExportSpec{
+					Slice:   "test-slice",
+					Aliases: []string{"traffic.com"},
+				},
+			},
+		},
+	}, nil
+}
+
 func (f fakeWebhookClient) GetSliceOverlayNetworkType(ctx context.Context, client client.Client, sliceName string) (v1alpha1.NetworkType, error) {
 	return "", nil
 }
@@ -128,3 +156,111 @@ var _ = Describe("Deploy Webhook", func() {
 		})
 	})
 })
+
+var _ = Describe("Validating Webhook", func() {
+	fakeWhClient := new(fakeWebhookClient)
+	webhookServer := pod.WebhookServer{
+		SliceInfoClient: fakeWhClient,
+	}
+	Describe("ValidateServiceExport", func() {
+		Context("ServiceExport Object with no alias conflict", func() {
+			serviceExportList := &v1beta1.ServiceExportList{
+				Items: []v1beta1.ServiceExport{
+					{
+						ObjectMeta: metav1.ObjectMeta{
+							Name:      "svcex-3",
+							Namespace: "test-ns-1",
+						},
+						Spec: v1beta1.ServiceExportSpec{
+							Slice:   "test-slice",
+							Aliases: []string{"hello.com"},
+						},
+					},
+					{
+						ObjectMeta: metav1.ObjectMeta{
+							Name:      "svcex-4",
+							Namespace: "test-ns-2",
+						},
+						Spec: v1beta1.ServiceExportSpec{
+							Slice:   "test-slice",
+							Aliases: []string{"connect.com"},
+						},
+					},
+				},
+			}
+			It("should be created", func() {
+				for _, serviceExport := range serviceExportList.Items {
+					is, _, _ := webhookServer.ValidateServiceExport(&serviceExport, context.Background())
+					Expect(is).To(BeTrue())
+				}
+			})
+		})
+
+		Context("Alias already exist", func() {
+			serviceExportList := &v1beta1.ServiceExportList{
+				Items: []v1beta1.ServiceExport{
+					{
+						ObjectMeta: metav1.ObjectMeta{
+							Name:      "svcex-3",
+							Namespace: "test-ns-1",
+						},
+						Spec: v1beta1.ServiceExportSpec{
+							Slice:   "test-slice",
+							Aliases: []string{"Server.com"},
+						},
+					},
+					{
+						ObjectMeta: metav1.ObjectMeta{
+							Name:      "svcex-4",
+							Namespace: "test-ns-2",
+						},
+						Spec: v1beta1.ServiceExportSpec{
+							Slice:   "test-slice",
+							Aliases: []string{"traffic.com"},
+						},
+					},
+				},
+			}
+			It("should be rejected", func() {
+				for _, serviceExport := range serviceExportList.Items {
+					is, _, _ := webhookServer.ValidateServiceExport(&serviceExport, context.Background())
+					Expect(is).To(BeFalse())
+				}
+			})
+		})
+
+		Context("Update ServiceExport aliases", func() {
+			serviceExport := &v1beta1.ServiceExport{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "svcex-2",
+					Namespace: "test-ns-2",
+				},
+				Spec: v1beta1.ServiceExportSpec{
+					Slice:   "test-slice",
+					Aliases: []string{"Traffic.com", "connect.com"},
+				},
+			}
+			It("should be updated", func() {
+				is, _, _ := webhookServer.ValidateServiceExport(serviceExport, context.Background())
+				Expect(is).To(BeTrue())
+			})
+		})
+
+		Context("Update ServiceExport with conflicting alias", func() {
+			serviceExport := &v1beta1.ServiceExport{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "svcex-1",
+					Namespace: "test-ns-1",
+				},
+				Spec: v1beta1.ServiceExportSpec{
+					Slice:   "test-slice",
+					Aliases: []string{"traffic.com"},
+				},
+			}
+			It("should be rejected", func() {
+				is, _, _ := webhookServer.ValidateServiceExport(serviceExport, context.Background())
+				Expect(is).To(BeFalse())
+			})
+		})
+	})
+})
diff --git a/pkg/webhook/pod/webhook_utils.go b/pkg/webhook/pod/webhook_utils.go
@@ -2,8 +2,10 @@ package pod
 
 import (
 	"context"
+	"strings"
 
 	"github.com/kubeslice/apis/pkg/controller/v1alpha1"
+	"github.com/kubeslice/worker-operator/api/v1beta1"
 	"github.com/kubeslice/worker-operator/controllers"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/types"
@@ -33,6 +35,33 @@ func (w *webhookClient) GetNamespaceLabels(ctx context.Context, client client.Cl
 	return nsLabels, nil
 }
 
+// Fetch all serviceexport objects belonging to the slice
+func (w *webhookClient) GetAllServiceExports(ctx context.Context, c client.Client, slice string) (*v1beta1.ServiceExportList, error) {
+
+	listOpts := []client.ListOption{
+		client.MatchingLabels(map[string]string{
+			controllers.ApplicationNamespaceSelectorLabelKey: slice,
+		},
+		),
+	}
+
+	serviceExportList := &v1beta1.ServiceExportList{}
+	if err := c.List(ctx, serviceExportList, listOpts...); err != nil {
+		log.Info("Failed to get ServiceExportList", "slice", slice)
+		return nil, err
+	}
+	return serviceExportList, nil
+}
+
+func aliasExist(existingAliases []string, newAlias string) bool {
+	for _, alias := range existingAliases {
+		if strings.EqualFold(alias, newAlias) {
+			return true
+		}
+	}
+	return false
+}
+
 func (w *webhookClient) GetSliceOverlayNetworkType(ctx context.Context, client client.Client, sliceName string) (v1alpha1.NetworkType, error) {
 	return controllers.GetSliceOverlayNetworkType(ctx, client, sliceName)
 }