fix: rename ExtractPlugins to ExtractPluginsWithNamespace and add Ext…

…ractPlugins that does not include namespace (#649)
Kong · Sep 26, 2024 · 281dc6d · 281dc6d
1 parent ee76d6d
commit 281dc6d
Show file tree

Hide file tree

Showing 4 changed files with 117 additions and 5 deletions.
diff --git a/controller/konnect/index_kongroute.go b/controller/konnect/index_kongroute.go
@@ -29,5 +29,5 @@ func kongRouteUsesPlugins(object client.Object) []string {
 	if !ok {
 		return nil
 	}
-	return annotations.ExtractPlugins(route)
+	return annotations.ExtractPluginsWithNamespaces(route)
 }
diff --git a/controller/konnect/index_kongservice.go b/controller/konnect/index_kongservice.go
@@ -30,5 +30,5 @@ func kongServiceUsesPlugins(object client.Object) []string {
 		return nil
 	}
 
-	return annotations.ExtractPlugins(svc)
+	return annotations.ExtractPluginsWithNamespaces(svc)
 }
diff --git a/pkg/annotations/plugins.go b/pkg/annotations/plugins.go
@@ -6,9 +6,9 @@ import (
 	"github.com/kong/gateway-operator/pkg/consts"
 )
 
-// ExtractPlugins extracts plugin names from the given object's
+// ExtractPluginsWithNamespaces extracts plugin namespaced names from the given object's
 // konghq.com/plugins annotation.
-func ExtractPlugins(obj interface {
+func ExtractPluginsWithNamespaces(obj interface {
 	GetAnnotations() map[string]string
 	GetNamespace() string
 },
@@ -22,7 +22,32 @@ func ExtractPlugins(obj interface {
 	split := strings.Split(ann, ",")
 	ret := make([]string, 0, len(split))
 	for _, p := range split {
+		if p == "" {
+			continue
+		}
 		ret = append(ret, namespace+"/"+strings.TrimSpace(p))
 	}
 	return ret
 }
+
+// ExtractPlugins extracts plugin names from the given object's
+// konghq.com/plugins annotation.
+func ExtractPlugins(obj interface {
+	GetAnnotations() map[string]string
+},
+) []string {
+	ann, ok := obj.GetAnnotations()[consts.PluginsAnnotationKey]
+	if !ok {
+		return nil
+	}
+
+	split := strings.Split(ann, ",")
+	ret := make([]string, 0, len(split))
+	for _, p := range split {
+		if p == "" {
+			continue
+		}
+		ret = append(ret, strings.TrimSpace(p))
+	}
+	return ret
+}
diff --git a/pkg/annotations/plugins_test.go b/pkg/annotations/plugins_test.go
@@ -21,7 +21,7 @@ func (m *mockObject) GetNamespace() string {
 	return m.namespace
 }
 
-func TestExtractPlugins(t *testing.T) {
+func TestExtractPluginsWithNamespace(t *testing.T) {
 	tests := []struct {
 		name     string
 		obj      *mockObject
@@ -55,6 +55,16 @@ func TestExtractPlugins(t *testing.T) {
 			},
 			expected: []string{"default/plugin1", "default/plugin2", "default/plugin3"},
 		},
+		{
+			name: "empty plugin name gets filtered out",
+			obj: &mockObject{
+				annotations: map[string]string{
+					consts.PluginsAnnotationKey: "plugin1,,plugin3",
+				},
+				namespace: "default",
+			},
+			expected: []string{"default/plugin1", "default/plugin3"},
+		},
 		{
 			name: "plugins with spaces",
 			obj: &mockObject{
@@ -77,6 +87,83 @@ func TestExtractPlugins(t *testing.T) {
 		},
 	}
 
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			result := ExtractPluginsWithNamespaces(tt.obj)
+			assert.Equal(t, tt.expected, result)
+		})
+	}
+}
+
+func TestExtractPlugins(t *testing.T) {
+	tests := []struct {
+		name     string
+		obj      *mockObject
+		expected []string
+	}{
+		{
+			name: "no annotations",
+			obj: &mockObject{
+				annotations: map[string]string{},
+			},
+			expected: nil,
+		},
+		{
+			name: "single plugin",
+			obj: &mockObject{
+				annotations: map[string]string{
+					consts.PluginsAnnotationKey: "plugin1",
+				},
+			},
+			expected: []string{"plugin1"},
+		},
+		{
+			name: "multiple plugins",
+			obj: &mockObject{
+				annotations: map[string]string{
+					consts.PluginsAnnotationKey: "plugin1,plugin2,plugin3",
+				},
+			},
+			expected: []string{"plugin1", "plugin2", "plugin3"},
+		},
+		{
+			name: "plugins with spaces",
+			obj: &mockObject{
+				annotations: map[string]string{
+					consts.PluginsAnnotationKey: " plugin1 , plugin2 , plugin3 ",
+				},
+			},
+			expected: []string{"plugin1", "plugin2", "plugin3"},
+		},
+		{
+			name: "empty plugin names",
+			obj: &mockObject{
+				annotations: map[string]string{
+					consts.PluginsAnnotationKey: "plugin1,,plugin3",
+				},
+			},
+			expected: []string{"plugin1", "plugin3"},
+		},
+		{
+			name: "trailing comma",
+			obj: &mockObject{
+				annotations: map[string]string{
+					consts.PluginsAnnotationKey: "plugin1,plugin2,",
+				},
+			},
+			expected: []string{"plugin1", "plugin2"},
+		},
+		{
+			name: "leading comma",
+			obj: &mockObject{
+				annotations: map[string]string{
+					consts.PluginsAnnotationKey: ",plugin1,plugin2",
+				},
+			},
+			expected: []string{"plugin1", "plugin2"},
+		},
+	}
+
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			result := ExtractPlugins(tt.obj)