From aec07c41cc3da349921374b2dab55fc8549fbfc9 Mon Sep 17 00:00:00 2001 From: Mridul Date: Tue, 17 Oct 2023 13:25:52 +0530 Subject: [PATCH] feat(AM-11526): updating gwConnType is not allowed Signed-off-by: Mridul --- service/slice_config_webhook_validation.go | 17 ++++++++++- .../slice_config_webhook_validation_test.go | 29 +++++++++++++++++++ .../worker_slice_config_webhook_validation.go | 4 +++ ...er_slice_config_webhook_validation_test.go | 22 +++++++++++++- ...worker_slice_gateway_webhook_validation.go | 4 +++ ...r_slice_gateway_webhook_validation_test.go | 22 +++++++++++++- 6 files changed, 95 insertions(+), 3 deletions(-) diff --git a/service/slice_config_webhook_validation.go b/service/slice_config_webhook_validation.go index 84b9683c..e313d3cf 100644 --- a/service/slice_config_webhook_validation.go +++ b/service/slice_config_webhook_validation.go @@ -382,7 +382,22 @@ func preventUpdate(ctx context.Context, sc *controllerv1alpha1.SliceConfig, old return field.Invalid(field.NewPath("Spec").Child("VPNConfig").Child("Cipher"), sc.Spec.VPNConfig.Cipher, "cannot be updated") } } - + // can't switch gw svc types + gwSvcType := map[string]string{} + // create cluster:GwType map from old config + for _, i := range sliceConfig.Spec.SliceGatewayProvider.SliceGatewayServiceType { + gwSvcType[i.Cluster] = i.Type + } + // check new config + for _, new := range sc.Spec.SliceGatewayProvider.SliceGatewayServiceType { + oldType, exists := gwSvcType[new.Cluster] + if exists && new.Type != oldType { + return field.Forbidden(field.NewPath("Spec").Child("SliceGatewayProvider").Child("SliceGatewayServiceType"), "update not allowed") + } + if !exists && new.Type != defaultSliceGatewayServiceType { + return field.Forbidden(field.NewPath("Spec").Child("SliceGatewayProvider").Child("SliceGatewayServiceType"), "update not allowed") + } + } return nil } diff --git a/service/slice_config_webhook_validation_test.go b/service/slice_config_webhook_validation_test.go index 76dffc4a..5ba28282 100644 --- a/service/slice_config_webhook_validation_test.go +++ b/service/slice_config_webhook_validation_test.go @@ -89,6 +89,7 @@ var SliceConfigWebhookValidationTestBed = map[string]func(*testing.T){ "SliceConfigWebhookValidation_UpdateValidateSliceConfigWithExternalGatewayConfigHasAsterisksInMoreThanOnePlace": UpdateValidateSliceConfigWithExternalGatewayConfigHasAsterisksInMoreThanOnePlace, "SliceConfigWebhookValidation_UpdateValidateSliceConfigWithExternalGatewayConfigHasDuplicateClusters": UpdateValidateSliceConfigWithExternalGatewayConfigHasDuplicateClusters, "SliceConfigWebhookValidation_UpdateValidateSliceConfigWithoutErrors": UpdateValidateSliceConfigWithoutErrors, + "SliceConfigWebhookValidation_UpdateValidateSliceGatewayServiceType": UpdateValidateSliceConfig_SliceGatewayServiceType, "SliceConfigWebhookValidation_DeleteValidateSliceConfigWithApplicationNamespacesNotEmpty": DeleteValidateSliceConfigWithApplicationNamespacesAndAllowedNamespacesNotEmpty, "SliceConfigWebhookValidation_DeleteValidateSliceConfigWithOnboardedAppNamespacesNotEmpty": DeleteValidateSliceConfigWithOnboardedAppNamespacesNotEmpty, "SliceConfigWebhookValidation_validateAllowedNamespacesWithDuplicateClusters": ValidateAllowedNamespacesWithDuplicateClusters, @@ -119,6 +120,34 @@ var SliceConfigWebhookValidationTestBed = map[string]func(*testing.T){ "SliceConfigWebhookValidation_UpdateValidateSliceConfigUpdatingVPNCipher": UpdateValidateSliceConfigUpdatingVPNCipher, } +func UpdateValidateSliceConfig_SliceGatewayServiceType(t *testing.T) { + name := "test-slice" + namespace := "demons" + oldSliceConfig := controllerv1alpha1.SliceConfig{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + }, + } + oldSliceConfig.Spec.SliceGatewayProvider.SliceGatewayServiceType = []controllerv1alpha1.SliceGatewayServiceType{ + { + Cluster: "c1", + Type: "LoadBalancer", + }, + } + clientMock, newSliceConfig, ctx := setupSliceConfigWebhookValidationTest(name, namespace) + newSliceConfig.Spec.SliceGatewayProvider.SliceGatewayServiceType = []controllerv1alpha1.SliceGatewayServiceType{ + { + Cluster: "c1", + Type: "NodePort", + }, + } + err := ValidateSliceConfigUpdate(ctx, newSliceConfig, runtime.Object(&oldSliceConfig)) + require.NotNil(t, err) + require.Contains(t, err.Error(), "Spec.SliceGatewayProvider.SliceGatewayServiceType: Forbidden:") + require.Contains(t, err.Error(), "update not allowed") + clientMock.AssertExpectations(t) +} func CreateValidateProjectNamespaceDoesNotExist(t *testing.T) { name := "slice_config" namespace := "namespace" diff --git a/service/worker_slice_config_webhook_validation.go b/service/worker_slice_config_webhook_validation.go index f79ed38a..e2359e81 100644 --- a/service/worker_slice_config_webhook_validation.go +++ b/service/worker_slice_config_webhook_validation.go @@ -18,6 +18,7 @@ package service import ( "context" + "k8s.io/apimachinery/pkg/runtime" workerv1alpha1 "github.com/kubeslice/kubeslice-controller/apis/worker/v1alpha1" @@ -40,5 +41,8 @@ func preventUpdateWorkerSliceConfig(ctx context.Context, ss *workerv1alpha1.Work if workerSliceConfig.Spec.Octet != nil && *workerSliceConfig.Spec.Octet != *ss.Spec.Octet { return field.Invalid(field.NewPath("Spec").Child("Octet"), *ss.Spec.Octet, "cannot be updated") } + if workerSliceConfig.Spec.SliceGatewayProvider.SliceGatewayServiceType != ss.Spec.SliceGatewayProvider.SliceGatewayType { + return field.Forbidden(field.NewPath("Spec").Child("SliceGatewayProvider").Child("SliceGatewayServiceType"), "update not allowed") + } return nil } diff --git a/service/worker_slice_config_webhook_validation_test.go b/service/worker_slice_config_webhook_validation_test.go index 39114ad9..ea899835 100644 --- a/service/worker_slice_config_webhook_validation_test.go +++ b/service/worker_slice_config_webhook_validation_test.go @@ -18,9 +18,10 @@ package service import ( "context" - "k8s.io/apimachinery/pkg/runtime" "testing" + "k8s.io/apimachinery/pkg/runtime" + "github.com/dailymotion/allure-go" workerv1alpha1 "github.com/kubeslice/kubeslice-controller/apis/worker/v1alpha1" "github.com/kubeslice/kubeslice-controller/util" @@ -43,6 +44,25 @@ func TestWorkerSliceConfigWebhookValidationSuite(t *testing.T) { var WorkerSliceConfigWebhookValidationTestBed = map[string]func(*testing.T){ "WorkerSliceConfigWebhookValidation_UpdateValidateWorkerSliceConfigUpdatingOctet": UpdateValidateWorkerSliceConfigUpdatingOctet, "WorkerSliceConfigWebhookValidation_UpdateValidateWorkerSliceConfigWithoutErrors": UpdateValidateWorkerSliceConfigWithoutErrors, + "WorkerSliceConfigWebhookValidation_UpdateSliceGatewayServiceType": UpdateSliceGatewayServiceType, +} + +func UpdateSliceGatewayServiceType(t *testing.T) { + name := "slice-clusterx" + namespace := "demons" + clientMock, newWorkerSliceConfig, ctx := setupWorkerSliceConfigWebhookValidationTest(name, namespace) + existingWorkerSliceConfig := workerv1alpha1.WorkerSliceConfig{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + }, + } + existingWorkerSliceConfig.Spec.SliceGatewayProvider.SliceGatewayServiceType = "LoadBalancer" + newWorkerSliceConfig.Spec.SliceGatewayProvider.SliceGatewayServiceType = "NodePort" + err := ValidateWorkerSliceConfigUpdate(ctx, &existingWorkerSliceConfig, runtime.Object(newWorkerSliceConfig)) + require.NotNil(t, err) + require.Contains(t, err.Error(), "Spec.SliceGatewayProvider.SliceGatewayServiceType: Forbidden:") + clientMock.AssertExpectations(t) } func UpdateValidateWorkerSliceConfigUpdatingOctet(t *testing.T) { diff --git a/service/worker_slice_gateway_webhook_validation.go b/service/worker_slice_gateway_webhook_validation.go index 4e94ccdf..9685159e 100644 --- a/service/worker_slice_gateway_webhook_validation.go +++ b/service/worker_slice_gateway_webhook_validation.go @@ -18,6 +18,7 @@ package service import ( "context" + "k8s.io/apimachinery/pkg/runtime" workerv1alpha1 "github.com/kubeslice/kubeslice-controller/apis/worker/v1alpha1" @@ -40,5 +41,8 @@ func preventUpdateWorkerSliceGateway(workerSliceGatewayCtx context.Context, sg * if workerSliceGateway.Spec.GatewayNumber != sg.Spec.GatewayNumber { return field.Invalid(field.NewPath("Spec").Child("GatewayNumber"), sg.Spec.GatewayNumber, "cannot be updated") } + if workerSliceGateway.Spec.GatewayConnectivityType != sg.Spec.GatewayConnectivityType { + return field.Forbidden(field.NewPath("Spec").Child("GatewayConnectivityType"), "update not allowed") + } return nil } diff --git a/service/worker_slice_gateway_webhook_validation_test.go b/service/worker_slice_gateway_webhook_validation_test.go index 421caead..ddd3dced 100644 --- a/service/worker_slice_gateway_webhook_validation_test.go +++ b/service/worker_slice_gateway_webhook_validation_test.go @@ -18,9 +18,10 @@ package service import ( "context" - "k8s.io/apimachinery/pkg/runtime" "testing" + "k8s.io/apimachinery/pkg/runtime" + "github.com/dailymotion/allure-go" workerv1alpha1 "github.com/kubeslice/kubeslice-controller/apis/worker/v1alpha1" "github.com/kubeslice/kubeslice-controller/util" @@ -43,6 +44,25 @@ func TestWorkerSliceGatewayWebhookValidationSuite(t *testing.T) { var WorkerSliceGatewayWebhookValidationTestBed = map[string]func(*testing.T){ "WorkerSliceGatewayWebhookValidation_UpdateValidateWorkerSliceGatewayUpdatingGatewayNumber": UpdateValidateWorkerSliceGatewayUpdatingGatewayNumber, "WorkerSliceGatewayWebhookValidation_UpdateValidateWorkerSliceGatewayWithoutErrors": UpdateValidateWorkerSliceGatewayWithoutErrors, + "WorkerSliceGatewayWebhookValidation_UpdateGatewayConnectivityType": UpdateGatewayConnectivityType, +} + +func UpdateGatewayConnectivityType(t *testing.T) { + name := "slice-cx-cy" + namespace := "stubns" + clientMock, newWorkerSliceGateway, ctx := setupWorkerSliceGatewayWebhookValidationTest(name, namespace) + existingWorkerSliceGateway := workerv1alpha1.WorkerSliceGateway{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + }, + } + existingWorkerSliceGateway.Spec.GatewayConnectivityType = "LoadBalancer" + newWorkerSliceGateway.Spec.GatewayConnectivityType = "NodePort" + err := ValidateWorkerSliceGatewayUpdate(ctx, &existingWorkerSliceGateway, runtime.Object(newWorkerSliceGateway)) + require.NotNil(t, err) + require.Contains(t, err.Error(), "Spec.GatewayConnectivityType: Forbidden:") + clientMock.AssertExpectations(t) } func UpdateValidateWorkerSliceGatewayUpdatingGatewayNumber(t *testing.T) {