fix(AM-12018): prevent protocol update

Signed-off-by: Mridul <[email protected]>
kubeslice · Nov 22, 2023 · 19ecacd · 19ecacd
1 parent e34420c
commit 19ecacd
Show file tree

Hide file tree

Showing 2 changed files with 40 additions and 22 deletions.
diff --git a/service/slice_config_webhook_validation.go b/service/slice_config_webhook_validation.go
@@ -382,18 +382,20 @@ 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{}
+	// not allowed to switch gw svc types & protocols
 	// create cluster:GwType map from old config
-	for _, i := range sliceConfig.Spec.SliceGatewayProvider.SliceGatewayServiceType {
-		gwSvcType[i.Cluster] = i.Type
-	}
+	gwSvcTypeMap := getSliceGwSvcTypes(sliceConfig)
+
 	// check new config
 	for _, new := range sc.Spec.SliceGatewayProvider.SliceGatewayServiceType {
-		oldType, exists := gwSvcType[new.Cluster]
+		oldType, exists := gwSvcTypeMap[new.Cluster]
 		// allow user to update NodePort to LoadBalancer but not vice versa
-		if exists && oldType != defaultSliceGatewayServiceType && new.Type != oldType {
-			return field.Forbidden(field.NewPath("Spec").Child("SliceGatewayProvider").Child("SliceGatewayServiceType"), "update not allowed")
+		if exists && oldType.Type != defaultSliceGatewayServiceType && new.Type != oldType.Type {
+			return field.Forbidden(field.NewPath("Spec").Child("SliceGatewayProvider").Child("SliceGatewayServiceType"), "updating gateway service type is not allowed")
+		}
+		// don't allow user to update TCP to UDP & vice versa
+		if exists && new.Protocol != oldType.Protocol {
+			return field.Forbidden(field.NewPath("Spec").Child("SliceGatewayProvider").Child("SliceGatewayServiceType"), "updating gateway protocol is not allowed")
 		}
 	}
 	return nil

diff --git a/service/slice_config_webhook_validation_test.go b/service/slice_config_webhook_validation_test.go
@@ -89,7 +89,7 @@ var SliceConfigWebhookValidationTestBed = map[string]func(*testing.T){
 	"SliceConfigWebhookValidation_UpdateValidateSliceConfigWithExternalGatewayConfigHasAsterisksInMoreThanOnePlace":            UpdateValidateSliceConfigWithExternalGatewayConfigHasAsterisksInMoreThanOnePlace,
 	"SliceConfigWebhookValidation_UpdateValidateSliceConfigWithExternalGatewayConfigHasDuplicateClusters":                      UpdateValidateSliceConfigWithExternalGatewayConfigHasDuplicateClusters,
 	"SliceConfigWebhookValidation_UpdateValidateSliceConfigWithoutErrors":                                                      UpdateValidateSliceConfigWithoutErrors,
-	"SliceConfigWebhookValidation_UpdateValidateSliceGatewayServiceType":                                                       UpdateValidateSliceConfig_SliceGatewayServiceType,
+	"SliceConfigWebhookValidation_UpdateValidateSliceGatewayServiceType":                                                       UpdateValidateSliceConfig_PreventUpdate_SliceGatewayServiceType,
 	"SliceConfigWebhookValidation_DeleteValidateSliceConfigWithApplicationNamespacesNotEmpty":                                  DeleteValidateSliceConfigWithApplicationNamespacesAndAllowedNamespacesNotEmpty,
 	"SliceConfigWebhookValidation_DeleteValidateSliceConfigWithOnboardedAppNamespacesNotEmpty":                                 DeleteValidateSliceConfigWithOnboardedAppNamespacesNotEmpty,
 	"SliceConfigWebhookValidation_validateAllowedNamespacesWithDuplicateClusters":                                              ValidateAllowedNamespacesWithDuplicateClusters,
@@ -120,19 +120,24 @@ var SliceConfigWebhookValidationTestBed = map[string]func(*testing.T){
 	"SliceConfigWebhookValidation_UpdateValidateSliceConfigUpdatingVPNCipher":                                                  UpdateValidateSliceConfigUpdatingVPNCipher,
 }
 
-func UpdateValidateSliceConfig_SliceGatewayServiceType(t *testing.T) {
+func UpdateValidateSliceConfig_PreventUpdate_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",
+		Spec: controllerv1alpha1.SliceConfigSpec{
+			SliceGatewayProvider: controllerv1alpha1.WorkerSliceGatewayProvider{
+				SliceGatewayServiceType: []controllerv1alpha1.SliceGatewayServiceType{
+					{
+						Cluster: "c1",
+						Type:    "LoadBalancer",
+					},
+				},
+			},
+			Clusters: []string{"c1"},
 		},
 	}
 	clientMock, newSliceConfig, ctx := setupSliceConfigWebhookValidationTest(name, namespace)
@@ -142,19 +147,30 @@ func UpdateValidateSliceConfig_SliceGatewayServiceType(t *testing.T) {
 			Type:    "NodePort",
 		},
 	}
+	newSliceConfig.Spec.Clusters = []string{"c1"}
 	// loadbalancer to nodeport not allowed
 	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")
+	require.Contains(t, err.Error(), "updating gateway service type is not allowed")
 
-	// 	NodePort to LB allowed
-	oldSliceConfig.Spec.SliceGatewayProvider.SliceGatewayServiceType[0].Type = "NodePort"
-	newSliceConfig.Spec.SliceGatewayProvider.SliceGatewayServiceType[0].Type = "LoadBalancer"
-	err = ValidateSliceConfigUpdate(ctx, newSliceConfig, runtime.Object(&oldSliceConfig))
+	// tcp to udp & vice-versa not allowed
+	oldSliceConfig.Spec.SliceGatewayProvider.SliceGatewayServiceType = []controllerv1alpha1.SliceGatewayServiceType{
+		{
+			Cluster:  "c1",
+			Protocol: "TCP",
+		},
+	}
+	newSliceConfig.Spec.SliceGatewayProvider.SliceGatewayServiceType = []controllerv1alpha1.SliceGatewayServiceType{
+		{
+			Cluster:  "c1",
+			Protocol: "UDP",
+		},
+	}
 	require.NotNil(t, err)
-	require.NotContains(t, err.Error(), "Spec.SliceGatewayProvider.SliceGatewayServiceType: Forbidden:")
-	require.NotContains(t, err.Error(), "update not allowed")
+	err = ValidateSliceConfigUpdate(ctx, newSliceConfig, runtime.Object(&oldSliceConfig))
+	require.Contains(t, err.Error(), "Spec.SliceGatewayProvider.SliceGatewayServiceType: Forbidden:")
+	require.Contains(t, err.Error(), "updating gateway protocol is not allowed")
 
 	clientMock.AssertExpectations(t)
 }