diff --git a/service/slice_config_webhook_validation.go b/service/slice_config_webhook_validation.go index 11b9f376..bc88cfba 100644 --- 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 index 17c71dee..422d588c 100644 --- 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,7 +120,7 @@ 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{ @@ -128,11 +128,16 @@ func UpdateValidateSliceConfig_SliceGatewayServiceType(t *testing.T) { 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) }