fix(): remove validation for existing project ns Merge pull request #229

 from kubeslice/hotfix-project-namespace-validations

fix(): remove validation for existing project ns

config/events/controller.yaml

@@ -133,6 +133,18 @@ events:
     type: Warning
     reportingController: controller
     message: Namespace creation failed.
+  - name: NamespaceUpdated
+    reason: NamespaceUpdated
+    action: UpdateNamespace
+    type: Normal
+    reportingController: controller
+    message: Namespace updated.
+  - name: NamespaceUpdateFailed
+    reason: NamespaceUpdateFailed
+    action: UpdateNamespace
+    type: Warning
+    reportingController: controller
+    message: Namespace update failed.
   - name: NamespaceDeleted
     reason: NamespaceDeleted
     action: DeleteNamespace

config/events/events_config_map.yaml

@@ -30,6 +30,8 @@ data:
       - SecretDeletionFailed
       - NamespaceCreated
       - NamespaceCreationFailed
+      - NamespaceUpdated
+      - NamespaceUpdateFailed
       - NamespaceDeleted
       - NamespaceDeletionFailed
       - WorkerClusterRoleCreated

events/events_generated.go

@@ -198,6 +198,22 @@ var EventsMap = map[events.EventName]*events.EventSchema{
 		ReportingController: "controller",
 		Message:             "Namespace creation failed.",
 	},
+	"NamespaceUpdated": {
+		Name:                "NamespaceUpdated",
+		Reason:              "NamespaceUpdated",
+		Action:              "UpdateNamespace",
+		Type:                events.EventTypeNormal,
+		ReportingController: "controller",
+		Message:             "Namespace updated.",
+	},
+	"NamespaceUpdateFailed": {
+		Name:                "NamespaceUpdateFailed",
+		Reason:              "NamespaceUpdateFailed",
+		Action:              "UpdateNamespace",
+		Type:                events.EventTypeWarning,
+		ReportingController: "controller",
+		Message:             "Namespace update failed.",
+	},
 	"NamespaceDeleted": {
 		Name:                "NamespaceDeleted",
 		Reason:              "NamespaceDeleted",
@@ -743,6 +759,8 @@ var (
 	EventSecretDeletionFailed                 events.EventName = "SecretDeletionFailed"
 	EventNamespaceCreated                     events.EventName = "NamespaceCreated"
 	EventNamespaceCreationFailed              events.EventName = "NamespaceCreationFailed"
+	EventNamespaceUpdated                     events.EventName = "NamespaceUpdated"
+	EventNamespaceUpdateFailed                events.EventName = "NamespaceUpdateFailed"
 	EventNamespaceDeleted                     events.EventName = "NamespaceDeleted"
 	EventNamespaceDeletionFailed              events.EventName = "NamespaceDeletionFailed"
 	EventWorkerClusterRoleCreated             events.EventName = "WorkerClusterRoleCreated"

service/namespace_service.go

@@ -19,6 +19,7 @@ package service
 import (
 	"context"
 	"fmt"
+
 	"github.com/kubeslice/kubeslice-controller/metrics"
 
 	"github.com/kubeslice/kubeslice-controller/events"
@@ -84,6 +85,39 @@ func (n *NamespaceService) ReconcileProjectNamespace(ctx context.Context, namesp
 				"object_kind": metricKindNamespace,
 			},
 		)
+	} else {
+		// check if the namespace has the correct labels
+		if !util.CompareLabels(nsResource.Labels, n.getResourceLabel(namespace, owner)) {
+			// append missing labels
+			for key, value := range n.getResourceLabel(namespace, owner) {
+				if nsResource.Labels[key] != value {
+					nsResource.Labels[key] = value
+				}
+			}
+			err := util.UpdateResource(ctx, nsResource)
+			nsResource.Namespace = ControllerNamespace
+			if err != nil {
+				util.RecordEvent(ctx, eventRecorder, nsResource, nil, events.EventNamespaceUpdateFailed)
+				n.mf.RecordCounterMetric(metrics.KubeSliceEventsCounter,
+					map[string]string{
+						"action":      "update_failed",
+						"event":       string(events.EventNamespaceUpdateFailed),
+						"object_name": nsResource.Name,
+						"object_kind": metricKindNamespace,
+					},
+				)
+				return ctrl.Result{}, err
+			}
+			util.RecordEvent(ctx, eventRecorder, nsResource, nil, events.EventNamespaceUpdated)
+			n.mf.RecordCounterMetric(metrics.KubeSliceEventsCounter,
+				map[string]string{
+					"action":      "updated",
+					"event":       string(events.EventNamespaceUpdated),
+					"object_name": nsResource.Name,
+					"object_kind": metricKindNamespace,
+				},
+			)
+		}
 	}
 	return ctrl.Result{}, nil
 }

service/namespace_service_test.go

@@ -18,9 +18,10 @@ package service
 
 import (
 	"context"
+	"testing"
+
 	"github.com/kubeslice/kubeslice-controller/metrics"
 	metricMock "github.com/kubeslice/kubeslice-controller/metrics/mocks"
-	"testing"
 
 	ossEvents "github.com/kubeslice/kubeslice-controller/events"
 	"github.com/kubeslice/kubeslice-monitoring/pkg/events"
@@ -52,9 +53,9 @@ func TestNamespaceSuite(t *testing.T) {
 
 var NamespaceTestbed = map[string]func(*testing.T){
 	"TestReconcileProjectNamespace_NamespaceGetsCreatedWithOwnerLabelAndReturnsReconciliationComplete_Happypath": TestReconcileProjectNamespace_NamespaceGetsCreatedWithOwnerLabelAndReturnsReconciliationComplete_Happypath,
-	"TestReconcileProjectNamespace_DoesNothingIfNamespaceExistAlready":                                           TestReconcileProjectNamespace_DoesNothingIfNamespaceExistAlready,
-	"TestDeleteNamespace_DeletesObjectWithReconciliationComplete":                                                TestDeleteNamespace_DeletesObjectWithReconciliationComplete,
-	"TestDeleteNamespace_DoesNothingIfNamespaceDoNotExist":                                                       TestDeleteNamespace_DoesNothingIfNamespaceDoNotExist,
+	// "TestReconcileProjectNamespace_DoesNothingIfNamespaceExistAlready":                                           TestReconcileProjectNamespace_DoesNothingIfNamespaceExistAlready,
+	"TestDeleteNamespace_DeletesObjectWithReconciliationComplete": TestDeleteNamespace_DeletesObjectWithReconciliationComplete,
+	"TestDeleteNamespace_DoesNothingIfNamespaceDoNotExist":        TestDeleteNamespace_DoesNothingIfNamespaceDoNotExist,
 }
 
 func TestReconcileProjectNamespace_NamespaceGetsCreatedWithOwnerLabelAndReturnsReconciliationComplete_Happypath(t *testing.T) {
@@ -96,32 +97,32 @@ func TestReconcileProjectNamespace_NamespaceGetsCreatedWithOwnerLabelAndReturnsR
 	mMock.AssertExpectations(t)
 }
 
-func TestReconcileProjectNamespace_DoesNothingIfNamespaceExistAlready(t *testing.T) {
-	namespaceName := "cisco"
-	mMock := &metricMock.IMetricRecorder{}
-	namespaceService := NamespaceService{
-		mf: mMock,
-	}
-
-	namespace := &corev1.Namespace{}
-	namespaceObject := client.ObjectKey{
-		Name: namespaceName,
-	}
-	clientMock := &utilMock.Client{}
-	scheme := runtime.NewScheme()
-	controllerv1alpha1.AddToScheme(scheme)
-	ctx := prepareNamespaceTestContext(context.Background(), clientMock, scheme)
-	mMock.On("WithProject", mock.AnythingOfType("string")).Return(&metrics.MetricRecorder{}).Once()
-	clientMock.On("Get", ctx, namespaceObject, namespace).Return(nil)
-	project := &controllerv1alpha1.Project{}
-	project.ObjectMeta.Labels = map[string]string{"testLabel": "testValue"}
-	result, err := namespaceService.ReconcileProjectNamespace(ctx, namespaceName, project)
-	expectedResult := ctrl.Result{}
-	require.Equal(t, result, expectedResult)
-	require.Nil(t, err)
-	clientMock.AssertExpectations(t)
-	mMock.AssertExpectations(t)
-}
+// func TestReconcileProjectNamespace_DoesNothingIfNamespaceExistAlready(t *testing.T) {
+// 	namespaceName := "cisco"
+// 	mMock := &metricMock.IMetricRecorder{}
+// 	namespaceService := NamespaceService{
+// 		mf: mMock,
+// 	}
+
+// 	namespace := &corev1.Namespace{}
+// 	namespaceObject := client.ObjectKey{
+// 		Name: namespaceName,
+// 	}
+// 	clientMock := &utilMock.Client{}
+// 	scheme := runtime.NewScheme()
+// 	controllerv1alpha1.AddToScheme(scheme)
+// 	ctx := prepareNamespaceTestContext(context.Background(), clientMock, scheme)
+// 	mMock.On("WithProject", mock.AnythingOfType("string")).Return(&metrics.MetricRecorder{}).Once()
+// 	clientMock.On("Get", ctx, namespaceObject, namespace).Return(nil)
+// 	project := &controllerv1alpha1.Project{}
+// 	project.ObjectMeta.Labels = map[string]string{"testLabel": "testValue"}
+// 	result, err := namespaceService.ReconcileProjectNamespace(ctx, namespaceName, project)
+// 	expectedResult := ctrl.Result{}
+// 	require.Equal(t, result, expectedResult)
+// 	require.Nil(t, err)
+// 	clientMock.AssertExpectations(t)
+// 	mMock.AssertExpectations(t)
+// }
 
 func TestDeleteNamespace_DeletesObjectWithReconciliationComplete(t *testing.T) {
 	namespaceName := "cisco"

service/project_webhook_validation.go

@@ -19,11 +19,12 @@ package service
 import (
 	"context"
 	"fmt"
-	apierrors "k8s.io/apimachinery/pkg/api/errors"
-	"k8s.io/apimachinery/pkg/runtime/schema"
 	"os"
 	"strings"
 
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
+	"k8s.io/apimachinery/pkg/runtime/schema"
+
 	controllerv1alpha1 "github.com/kubeslice/kubeslice-controller/apis/controller/v1alpha1"
 	"github.com/kubeslice/kubeslice-controller/util"
 	corev1 "k8s.io/api/core/v1"
@@ -40,9 +41,11 @@ func ValidateProjectCreate(ctx context.Context, project *controllerv1alpha1.Proj
 	if err := validateProjectName(project.Name); err != nil {
 		return apierrors.NewInvalid(schema.GroupKind{Group: apiGroupKubeSliceControllers, Kind: "Project"}, project.Name, field.ErrorList{err})
 	}
-	if err := validateProjectNamespaceIfAlreadyExists(ctx, project.Name); err != nil {
-		return apierrors.NewInvalid(schema.GroupKind{Group: apiGroupKubeSliceControllers, Kind: "Project"}, project.Name, field.ErrorList{err})
-	}
+	// FIXME: Remove the comment after testing.
+	// Validation for existing project namespace is not required. User may want to use an existing namespace.
+	// if err := validateProjectNamespaceIfAlreadyExists(ctx, project.Name); err != nil {
+	// 	return apierrors.NewInvalid(schema.GroupKind{Group: apiGroupKubeSliceControllers, Kind: "Project"}, project.Name, field.ErrorList{err})
+	// }
 	if err := validateDNSCompliantSANames(ctx, project); err != nil {
 		return apierrors.NewInvalid(schema.GroupKind{Group: apiGroupKubeSliceControllers, Kind: "Project"}, project.Name, field.ErrorList{err})
 	}
@@ -99,14 +102,14 @@ func validateAppliedInControllerNamespace(ctx context.Context, project *controll
 	return nil
 }
 
-// validateProjectNamespaceIfAlreadyExists is a function validate the whether the project namespace already exists or not
-func validateProjectNamespaceIfAlreadyExists(ctx context.Context, projectName string) *field.Error {
-	projectNamespace := fmt.Sprintf(ProjectNamespacePrefix, projectName)
-	if exist, _ := util.GetResourceIfExist(ctx, client.ObjectKey{Name: projectNamespace}, &corev1.Namespace{}); exist {
-		return field.Invalid(field.NewPath("project namespace"), projectNamespace, "already exists")
-	}
-	return nil
-}
+// // validateProjectNamespaceIfAlreadyExists is a function validate the whether the project namespace already exists or not
+// func validateProjectNamespaceIfAlreadyExists(ctx context.Context, projectName string) *field.Error {
+// 	projectNamespace := fmt.Sprintf(ProjectNamespacePrefix, projectName)
+// 	if exist, _ := util.GetResourceIfExist(ctx, client.ObjectKey{Name: projectNamespace}, &corev1.Namespace{}); exist {
+// 		return field.Invalid(field.NewPath("project namespace"), projectNamespace, "already exists")
+// 	}
+// 	return nil
+// }
 
 // validateDNSCompliantSANames is a function to validate the service account name whether it is DNS compliant
 func validateDNSCompliantSANames(ctx context.Context, project *controllerv1alpha1.Project) *field.Error {

service/project_webhook_validation_test.go

@@ -43,8 +43,8 @@ func TestProjectWebhookSuite(t *testing.T) {
 }
 
 var ProjectWebhookTestbed = map[string]func(*testing.T){
-	"TestValidateProjectCreate_Applied_Namespace_Error":                                  TestValidateProjectCreate_Applied_Namespace_Error,
-	"TestValidateProjectCreate_FailsIfNamespaceAlreadyExists":                            TestValidateProjectCreate_FailsIfNamespaceAlreadyExists,
+	"TestValidateProjectCreate_Applied_Namespace_Error": TestValidateProjectCreate_Applied_Namespace_Error,
+	// "TestValidateProjectCreate_FailsIfNamespaceAlreadyExists":                            TestValidateProjectCreate_FailsIfNamespaceAlreadyExists,
 	"TestValidateProjectCreate_FailsIf_Sa_Name_Not_DNS_Compliant":                        TestValidateProjectCreate_FailsIf_Sa_Name_Not_DNS_Compliant,
 	"TestValidateProjectCreate_FailsIfNameContainsDot":                                   TestValidateProjectCreate_FailsIfNameContainsDot,
 	"TestValidateProjectCreate_FailsIfNameContainsGreaterThan30Characters":               TestValidateProjectCreate_FailsIfNameContainsGreaterThan30Characters,
@@ -101,21 +101,21 @@ func TestValidateProjectCreate_FailsIfNameContainsGreaterThan30Characters(t *tes
 	clientMock.AssertExpectations(t)
 }
 
-func TestValidateProjectCreate_FailsIfNamespaceAlreadyExists(t *testing.T) { //todo
-	project := &controllerv1alpha1.Project{}
-	clientMock := &utilMock.Client{}
-	name := "testProject"
-	project.ObjectMeta.Name = name
-	namespace := "avesha-controller"
-	project.ObjectMeta.Namespace = namespace
-	os.Setenv("KUBESLICE_CONTROLLER_MANAGER_NAMESPACE", namespace)
-	ctx := prepareProjectWebhookTestContext(context.Background(), clientMock, nil)
-	clientMock.On("Get", ctx, mock.Anything, mock.Anything).Return(nil).Once()
-	err := ValidateProjectCreate(ctx, project)
-	require.Error(t, err)
-	require.Contains(t, err.Error(), "already exists", namespace)
-	clientMock.AssertExpectations(t)
-}
+// func TestValidateProjectCreate_FailsIfNamespaceAlreadyExists(t *testing.T) { //todo
+// 	project := &controllerv1alpha1.Project{}
+// 	clientMock := &utilMock.Client{}
+// 	name := "testProject"
+// 	project.ObjectMeta.Name = name
+// 	namespace := "avesha-controller"
+// 	project.ObjectMeta.Namespace = namespace
+// 	os.Setenv("KUBESLICE_CONTROLLER_MANAGER_NAMESPACE", namespace)
+// 	ctx := prepareProjectWebhookTestContext(context.Background(), clientMock, nil)
+// 	clientMock.On("Get", ctx, mock.Anything, mock.Anything).Return(nil).Once()
+// 	err := ValidateProjectCreate(ctx, project)
+// 	require.Error(t, err)
+// 	require.Contains(t, err.Error(), "already exists", namespace)
+// 	clientMock.AssertExpectations(t)
+// }
 
 func TestValidateProjectCreate_FailsIf_Sa_Name_Not_DNS_Compliant(t *testing.T) { //todo
 	project := &controllerv1alpha1.Project{}
@@ -131,9 +131,9 @@ func TestValidateProjectCreate_FailsIf_Sa_Name_Not_DNS_Compliant(t *testing.T) {
 	project.Spec.ServiceAccount.ReadOnly = []string{invalidName1, invalidName2}
 	project.Spec.ServiceAccount.ReadWrite = []string{invalidNameRw1, invalidNameRw2}
 	os.Setenv("KUBESLICE_CONTROLLER_MANAGER_NAMESPACE", namespace)
-	notFoundError := k8sError.NewNotFound(util.Resource("projecttest"), "isnotFound")
+	// notFoundError := k8sError.NewNotFound(util.Resource("projecttest"), "isnotFound")
 	ctx := prepareProjectWebhookTestContext(context.Background(), clientMock, nil)
-	clientMock.On("Get", ctx, mock.Anything, mock.Anything).Return(notFoundError).Once()
+	// clientMock.On("Get", ctx, mock.Anything, mock.Anything).Return(notFoundError).Once()
 	err := ValidateProjectCreate(ctx, project)
 	require.Error(t, err)
 	require.Contains(t, err.Error(), "Invalid value", invalidName1, invalidName2)
@@ -155,9 +155,9 @@ func TestValidateProjectCreate_HappyPath(t *testing.T) { //todo
 	project.Spec.ServiceAccount.ReadOnly = []string{validName1, validName2}
 	project.Spec.ServiceAccount.ReadWrite = []string{validNameRw1, validNameRw2}
 	os.Setenv("KUBESLICE_CONTROLLER_MANAGER_NAMESPACE", namespace)
-	notFoundError := k8sError.NewNotFound(util.Resource("projecttest"), "isnotFound")
+	// notFoundError := k8sError.NewNotFound(util.Resource("projecttest"), "isnotFound")
 	ctx := prepareProjectWebhookTestContext(context.Background(), clientMock, nil)
-	clientMock.On("Get", ctx, mock.Anything, mock.Anything).Return(notFoundError).Once()
+	// clientMock.On("Get", ctx, mock.Anything, mock.Anything).Return(notFoundError).Once()
 	err := ValidateProjectCreate(ctx, project)
 	require.Nil(t, err)
 	clientMock.AssertExpectations(t)
@@ -220,8 +220,8 @@ func Test_ValidateProjectUpdate_ThrowsErrorIf_SA_DNS_Invalid_throws_error(t *tes
 	project.Spec.ServiceAccount.ReadWrite = []string{invalidNameRw1, invalidNameRw2}
 	os.Setenv("KUBESLICE_CONTROLLER_MANAGER_NAMESPACE", namespace)
 	ctx := prepareProjectWebhookTestContext(context.Background(), clientMock, nil)
-	notFoundError := k8sError.NewNotFound(util.Resource("projecttest"), "isnotFound")
-	clientMock.On("Get", ctx, mock.Anything, mock.Anything).Return(notFoundError).Times(4)
+	// notFoundError := k8sError.NewNotFound(util.Resource("projecttest"), "isnotFound")
+	// clientMock.On("Get", ctx, mock.Anything, mock.Anything).Return(notFoundError).Times(4)
 	//calls rolebinding next
 	clientMock.On("Get", ctx, mock.Anything, mock.Anything).Return(nil).Times(1)
 	err := ValidateProjectUpdate(ctx, project)

util/reconciliation_utility.go

@@ -22,11 +22,12 @@ import (
 	"encoding/base64"
 	"encoding/json"
 	"fmt"
-	"github.com/kubeslice/kubeslice-monitoring/pkg/events"
-	corev1 "k8s.io/api/core/v1"
 	"reflect"
 	"strings"
 
+	"github.com/kubeslice/kubeslice-monitoring/pkg/events"
+	corev1 "k8s.io/api/core/v1"
+
 	"k8s.io/apimachinery/pkg/runtime"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
@@ -268,6 +269,19 @@ func CheckForProjectNamespace(namespace *corev1.Namespace) bool {
 	return namespace.Labels[LabelName] == fmt.Sprintf(LabelValue, "Project", namespace.Name)
 }
 
+// CompareLabels is a function to compare the labels
+func CompareLabels(label1 map[string]string, label2 map[string]string) bool {
+	if len(label1) != len(label2) {
+		return false
+	}
+	for key, value := range label1 {
+		if label2[key] != value {
+			return false
+		}
+	}
+	return true
+}
+
 // GetProjectName is function to get the project name from the namespace
 func GetProjectName(namespace string) string {
 	d := strings.Split(namespace, NamespacePrefix)