fix: fixing a bug on label comparision

openmfp · Apr 12, 2024 · cf9cc4e · cf9cc4e
1 parent e56f2d3
commit cf9cc4e
Show file tree

Hide file tree

Showing 2 changed files with 30 additions and 10 deletions.
diff --git a/internal/subroutines/namespace.go b/internal/subroutines/namespace.go
@@ -82,13 +82,12 @@ var NamespaceOwnedByAnotherAccountErr = errors.New("Namespace already owned by a
 var NamespaceOwnedByAnAccountInAnotherNamespaceErr = errors.New("Namespace already owned by another account in another namespace")
 
 func setNamespaceLabels(ns *v1.Namespace, instance *corev1alpha1.Account) error {
-	hasOwnerLabel := verifyLabel(NamespaceAccountOwnerLabel, instance.GetName(), ns.Labels)
-	hasOwnerNamespaceLabel := verifyLabel(NamespaceAccountOwnerNamespaceLabel, instance.GetNamespace(), ns.Labels)
-
-	if hasOwnerLabel && instance.Labels[NamespaceAccountOwnerLabel] != instance.GetName() {
+	hasOwnerLabel := hasLabel(NamespaceAccountOwnerLabel, ns.Labels)
+	hasOwnerNamespaceLabel := hasLabel(NamespaceAccountOwnerNamespaceLabel, ns.Labels)
+	if hasOwnerLabel && ns.Labels[NamespaceAccountOwnerLabel] != instance.GetName() {
 		return NamespaceOwnedByAnotherAccountErr
 	}
-	if hasOwnerNamespaceLabel && instance.Labels[NamespaceAccountOwnerNamespaceLabel] != instance.GetNamespace() {
+	if hasOwnerNamespaceLabel && ns.Labels[NamespaceAccountOwnerNamespaceLabel] != instance.GetNamespace() {
 		return NamespaceOwnedByAnAccountInAnotherNamespaceErr
 	}
 
@@ -102,11 +101,9 @@ func setNamespaceLabels(ns *v1.Namespace, instance *corev1alpha1.Account) error
 	return nil
 }
 
-func verifyLabel(key string, value string, labels map[string]string) bool {
-	if val, ok := labels[key]; ok {
-		return val == value
-	}
-	return false
+func hasLabel(key string, labels map[string]string) bool {
+	_, ok := labels[key]
+	return ok
 }
 
 func generateNamespace(instance *corev1alpha1.Account) *v1.Namespace {

diff --git a/internal/subroutines/namespace_test.go b/internal/subroutines/namespace_test.go
@@ -8,6 +8,7 @@ import (
 	"github.com/stretchr/testify/suite"
 	v1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/errors"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/utils/ptr"
@@ -298,6 +299,28 @@ func (suite *NamespaceSubroutineTestSuite) TestFinalizeNamespace_OK() {
 	suite.Nil(err)
 }
 
+// Test an account with a namspace in the spec, where the already existing namespace has different owner labels
+func (suite *NamespaceSubroutineTestSuite) TestProcessingWithDeclaredNamespaceMismatchedOwnerLabels() {
+	// Given
+	namespaceName := "a-names-space"
+	testAccount := &corev1alpha1.Account{
+		ObjectMeta: metav1.ObjectMeta{Name: "test-account"},
+		Spec: corev1alpha1.AccountSpec{
+			Namespace: &namespaceName,
+		},
+	}
+	mockGetNamespaceCallWithLabels(suite, namespaceName, map[string]string{
+		NamespaceAccountOwnerLabel: "different-owner",
+	})
+
+	// When
+	_, err := suite.testObj.Process(context.Background(), testAccount)
+
+	// Then
+	suite.Require().Nil(testAccount.Status.Namespace)
+	suite.NotNil(err)
+}
+
 func TestNamespaceSubroutineTestSuite(t *testing.T) {
 	suite.Run(t, new(NamespaceSubroutineTestSuite))
 }