Repository: Kong/kubernetes-ingress-controller
Pull request #6297 (2024-07-05 10:03:40)

perf: don't deep copy the annotations in FromK8sObject
**What this PR does / why we need it**:

No need to deep copy the annotations:

```
goos: darwin
goarch: arm64
pkg: github.com/kong/kubernetes-ingress-controller/v3/internal/util
                 │   old.txt    │              new.txt               │
                 │    sec/op    │   sec/op     vs base               │
FromK8sObject-10   202.30n ± 3%   44.08n ± 0%  -78.21% (p=0.008 n=5)

                 │  old.txt   │             new.txt              │
                 │    B/op    │   B/op    vs base                │
FromK8sObject-10   384.0 ± 0%   0.0 ± 0%  -100.00% (p=0.008 n=5)

                 │  old.txt   │              new.txt               │
                 │ allocs/op  │ allocs/op   vs base                │
FromK8sObject-10   3.000 ± 0%   0.000 ± 0%  -100.00% (p=0.008 n=5)
```


randmonkey: Can we guarantee that we never write the `annotations`? I think there is no need to write to it but we need to notice that.
pmalek: > Can we guarantee that we never write the `annotations`? I think there is no need to write to it but we need to notice that.

Added a comment stating that explicitly. PTAL.

Patch:
diff --git a/go.mod b/go.mod
index 773d39783..60b7dc577 100644
--- a/go.mod
+++ b/go.mod
@@ -215,7 +215,7 @@ require (
 	gomodules.xyz/jsonpatch/v2 v2.4.0 // indirect
 	google.golang.org/genproto/googleapis/api v0.0.0-20240617180043-68d350f18fd4 // indirect
 	google.golang.org/genproto/googleapis/rpc v0.0.0-20240624140628-dc46fd24d27d // indirect
-	google.golang.org/grpc v1.64.0
+	google.golang.org/grpc v1.65.0
 	google.golang.org/protobuf v1.34.2 // indirect
 	gopkg.in/evanphx/json-patch.v4 v4.12.0 // indirect
 	gopkg.in/inf.v0 v0.9.1 // indirect
diff --git a/go.sum b/go.sum
index 324ac6315..8a33f453f 100644
--- a/go.sum
+++ b/go.sum
@@ -581,8 +581,8 @@ google.golang.org/grpc v1.23.0/go.mod h1:Y5yQAOtifL1yxbo5wqy6BxZv8vAUGQwXBOALyac
 google.golang.org/grpc v1.25.1/go.mod h1:c3i+UQWmh7LiEpx4sFZnkU36qjEYZ0imhYfXVyQciAY=
 google.golang.org/grpc v1.27.0/go.mod h1:qbnxyOmOxrQa7FizSgH+ReBfzJrCY1pSN7KXBS8abTk=
 google.golang.org/grpc v1.33.2/go.mod h1:JMHMWHQWaTccqQQlmk3MJZS+GWXOdAesneDmEnv2fbc=
-google.golang.org/grpc v1.64.0 h1:KH3VH9y/MgNQg1dE7b3XfVK0GsPSIzJwdF617gUSbvY=
-google.golang.org/grpc v1.64.0/go.mod h1:oxjF8E3FBnjp+/gVFYdWacaLDx9na1aqy9oovLpxQYg=
+google.golang.org/grpc v1.65.0 h1:bs/cUb4lp1G5iImFFd3u5ixQzweKizoZJAwBNLR42lc=
+google.golang.org/grpc v1.65.0/go.mod h1:WgYC2ypjlB0EiQi6wdKixMqukr6lBc0Vo+oOgjrM5ZQ=
 google.golang.org/protobuf v0.0.0-20200109180630-ec00e32a8dfd/go.mod h1:DFci5gLYBciE7Vtevhsrf46CRTquxDuWsQurQQe4oz8=
 google.golang.org/protobuf v0.0.0-20200221191635-4d8936d0db64/go.mod h1:kwYJMbMJ01Woi6D6+Kah6886xMZcty6N08ah7+eCXa0=
 google.golang.org/protobuf v0.0.0-20200228230310-ab0ca4ff8a60/go.mod h1:cfTl7dwQJ+fmap5saPgwCLgHXTUD7jkjRqWcaiX5VyM=
diff --git a/internal/dataplane/kongstate/util.go b/internal/dataplane/kongstate/util.go
index f5ef152a7..7362994b8 100644
--- a/internal/dataplane/kongstate/util.go
+++ b/internal/dataplane/kongstate/util.go
@@ -7,10 +7,10 @@ import (
 
 	"github.com/kong/go-kong/kong"
 	corev1 "k8s.io/api/core/v1"
+	"sigs.k8s.io/controller-runtime/pkg/client"
 
 	"github.com/kong/kubernetes-ingress-controller/v3/internal/annotations"
 	"github.com/kong/kubernetes-ingress-controller/v3/internal/store"
-	"github.com/kong/kubernetes-ingress-controller/v3/internal/util"
 	kongv1 "github.com/kong/kubernetes-ingress-controller/v3/pkg/apis/configuration/v1"
 )
 
@@ -48,7 +48,7 @@ func getKongIngressForServices(
 
 func getKongIngressFromObjectMeta(
 	s store.Storer,
-	obj util.K8sObjectInfo,
+	obj client.Object,
 ) (
 	*kongv1.KongIngress, error,
 ) {
@@ -57,19 +57,19 @@ func getKongIngressFromObjectMeta(
 
 func getKongIngressFromObjAnnotations(
 	s store.Storer,
-	obj util.K8sObjectInfo,
+	obj client.Object,
 ) (
 	*kongv1.KongIngress, error,
 ) {
-	confName := annotations.ExtractConfigurationName(obj.Annotations)
+	confName := annotations.ExtractConfigurationName(obj.GetAnnotations())
 	if confName != "" {
-		ki, err := s.GetKongIngress(obj.Namespace, confName)
+		ki, err := s.GetKongIngress(obj.GetNamespace(), confName)
 		if err == nil {
 			return ki, nil
 		}
 	}
 
-	ki, err := s.GetKongIngress(obj.Namespace, obj.Name)
+	ki, err := s.GetKongIngress(obj.GetNamespace(), obj.GetName())
 	if err == nil {
 		return ki, nil
 	}
diff --git a/internal/util/objectmeta.go b/internal/util/objectmeta.go
index 1792b2443..333348d7b 100644
--- a/internal/util/objectmeta.go
+++ b/internal/util/objectmeta.go
@@ -1,8 +1,6 @@
 package util
 
 import (
-	"maps"
-
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 )
@@ -15,14 +13,14 @@ type K8sObjectInfo struct {
 	GroupVersionKind schema.GroupVersionKind
 }
 
+// FromK8sObject extracts information from a Kubernetes object.
+// It performs a shallow copy of object annotations so any modifications after
+// calling FromK8sObject will have an effect on the original object.
 func FromK8sObject(obj client.Object) K8sObjectInfo {
-	ret := K8sObjectInfo{
-		Name:        obj.GetName(),
-		Namespace:   obj.GetNamespace(),
-		Annotations: maps.Clone(obj.GetAnnotations()),
-	}
-	if gvk := obj.GetObjectKind().GroupVersionKind(); gvk.String() != "" {
-		ret.GroupVersionKind = gvk
+	return K8sObjectInfo{
+		Name:             obj.GetName(),
+		Namespace:        obj.GetNamespace(),
+		Annotations:      obj.GetAnnotations(),
+		GroupVersionKind: obj.GetObjectKind().GroupVersionKind(),
 	}
-	return ret
 }


Test patch:
diff --git a/internal/dataplane/kongstate/util_test.go b/internal/dataplane/kongstate/util_test.go
index 5c24252ad..263a22568 100644
--- a/internal/dataplane/kongstate/util_test.go
+++ b/internal/dataplane/kongstate/util_test.go
@@ -12,7 +12,6 @@ import (
 	"github.com/kong/kubernetes-ingress-controller/v3/internal/annotations"
 	"github.com/kong/kubernetes-ingress-controller/v3/internal/gatewayapi"
 	"github.com/kong/kubernetes-ingress-controller/v3/internal/store"
-	"github.com/kong/kubernetes-ingress-controller/v3/internal/util"
 	kongv1 "github.com/kong/kubernetes-ingress-controller/v3/pkg/apis/configuration/v1"
 )
 
@@ -281,8 +280,7 @@ func TestGetKongIngressFromObjectMeta(t *testing.T) {
 			})
 			require.NoError(t, err)
 
-			obj := util.FromK8sObject(tt.route)
-			kongIngress, err := getKongIngressFromObjectMeta(storer, obj)
+			kongIngress, err := getKongIngressFromObjectMeta(storer, tt.route)
 
 			if tt.expectedError == nil {
 				require.NoError(t, err)
diff --git a/internal/dataplane/translator/translate_httproute_test.go b/internal/dataplane/translator/translate_httproute_test.go
index 02ead543c..5156bca3c 100644
--- a/internal/dataplane/translator/translate_httproute_test.go
+++ b/internal/dataplane/translator/translate_httproute_test.go
@@ -239,7 +239,7 @@ func TestIngressRulesFromHTTPRoutes(t *testing.T) {
 										kong.String("k8s-version:v1beta1"),
 									},
 								},
-								Ingress: k8sObjectInfoOfHTTPRoute(routes[0]),
+								Ingress: util.FromK8sObject(routes[0]),
 							}},
 							Parent: routes[0],
 						},
@@ -312,7 +312,7 @@ func TestIngressRulesFromHTTPRoutes(t *testing.T) {
 										kong.String("k8s-version:v1beta1"),
 									},
 								},
-								Ingress: k8sObjectInfoOfHTTPRoute(routes[0]),
+								Ingress: util.FromK8sObject(routes[0]),
 							}},
 							Parent: routes[0],
 						},
@@ -391,7 +391,7 @@ func TestIngressRulesFromHTTPRoutes(t *testing.T) {
 										kong.String("k8s-version:v1beta1"),
 									},
 								},
-								Ingress: k8sObjectInfoOfHTTPRoute(routes[0]),
+								Ingress: util.FromK8sObject(routes[0]),
 							}},
 							Parent: routes[0],
 						},
@@ -469,7 +469,7 @@ func TestIngressRulesFromHTTPRoutes(t *testing.T) {
 										kong.String("k8s-version:v1beta1"),
 									},
 								},
-								Ingress: k8sObjectInfoOfHTTPRoute(routes[0]),
+								Ingress: util.FromK8sObject(routes[0]),
 							}},
 							Parent: routes[0],
 						},
@@ -549,7 +549,7 @@ func TestIngressRulesFromHTTPRoutes(t *testing.T) {
 										kong.String("k8s-version:v1beta1"),
 									},
 								},
-								Ingress: k8sObjectInfoOfHTTPRoute(routes[0]),
+								Ingress: util.FromK8sObject(routes[0]),
 							}},
 							Parent: routes[0],
 						},
@@ -639,7 +639,7 @@ func TestIngressRulesFromHTTPRoutes(t *testing.T) {
 											kong.String("k8s-version:v1beta1"),
 										},
 									},
-									Ingress: k8sObjectInfoOfHTTPRoute(routes[0]),
+									Ingress: util.FromK8sObject(routes[0]),
 								},
 							},
 							Parent: routes[0],
@@ -733,7 +733,7 @@ func TestIngressRulesFromHTTPRoutes(t *testing.T) {
 										kong.String("k8s-version:v1beta1"),
 									},
 								},
-								Ingress: k8sObjectInfoOfHTTPRoute(routes[0]),
+								Ingress: util.FromK8sObject(routes[0]),
 							}},
 							Parent: routes[0],
 						},
@@ -773,7 +773,7 @@ func TestIngressRulesFromHTTPRoutes(t *testing.T) {
 										kong.String("k8s-version:v1beta1"),
 									},
 								},
-								Ingress: k8sObjectInfoOfHTTPRoute(routes[0]),
+								Ingress: util.FromK8sObject(routes[0]),
 							}},
 							Parent: routes[0],
 						},
@@ -893,7 +893,7 @@ func TestIngressRulesFromHTTPRoutes(t *testing.T) {
 											kong.String("k8s-version:v1beta1"),
 										},
 									},
-									Ingress: k8sObjectInfoOfHTTPRoute(routes[0]),
+									Ingress: util.FromK8sObject(routes[0]),
 								},
 							},
 							Parent: routes[0],
@@ -936,7 +936,7 @@ func TestIngressRulesFromHTTPRoutes(t *testing.T) {
 											kong.String("k8s-version:v1beta1"),
 										},
 									},
-									Ingress: k8sObjectInfoOfHTTPRoute(routes[0]),
+									Ingress: util.FromK8sObject(routes[0]),
 								},
 							},
 							Parent: routes[0],
@@ -1050,7 +1050,7 @@ func TestIngressRulesFromHTTPRoutes(t *testing.T) {
 											kong.String("k8s-version:v1beta1"),
 										},
 									},
-									Ingress: k8sObjectInfoOfHTTPRoute(routes[0]),
+									Ingress: util.FromK8sObject(routes[0]),
 									Plugins: []kong.Plugin{
 										{
 											Name: kong.String("request-transformer"),
@@ -1083,7 +1083,7 @@ func TestIngressRulesFromHTTPRoutes(t *testing.T) {
 											kong.String("k8s-version:v1beta1"),
 										},
 									},
-									Ingress: k8sObjectInfoOfHTTPRoute(routes[0]),
+									Ingress: util.FromK8sObject(routes[0]),
 									Plugins: []kong.Plugin{
 										{
 											Name: kong.String("request-transformer"),
@@ -1205,7 +1205,7 @@ func TestIngressRulesFromHTTPRoutes(t *testing.T) {
 											kong.String("k8s-version:v1beta1"),
 										},
 									},
-									Ingress: k8sObjectInfoOfHTTPRoute(routes[0]),
+									Ingress: util.FromK8sObject(routes[0]),
 								},
 								// Second two matches consolidated into a single route
 								{
@@ -1232,7 +1232,7 @@ func TestIngressRulesFromHTTPRoutes(t *testing.T) {
 											kong.String("k8s-version:v1beta1"),
 										},
 									},
-									Ingress: k8sObjectInfoOfHTTPRoute(routes[0]),
+									Ingress: util.FromK8sObject(routes[0]),
 								},
 								// Third two matches consolidated into a single route
 								{
@@ -1262,7 +1262,7 @@ func TestIngressRulesFromHTTPRoutes(t *testing.T) {
 											kong.String("k8s-version:v1beta1"),
 										},
 									},
-									Ingress: k8sObjectInfoOfHTTPRoute(routes[0]),
+									Ingress: util.FromK8sObject(routes[0]),
 								},
 							},
 							Parent: routes[0],
@@ -1406,7 +1406,7 @@ func TestIngressRulesFromHTTPRoutes(t *testing.T) {
 											kong.String("k8s-version:v1beta1"),
 										},
 									},
-									Ingress: k8sObjectInfoOfHTTPRoute(routes[0]),
+									Ingress: util.FromK8sObject(routes[0]),
 								},
 								// Second two matches consolidated into a single route
 								{
@@ -1433,7 +1433,7 @@ func TestIngressRulesFromHTTPRoutes(t *testing.T) {
 											kong.String("k8s-version:v1beta1"),
 										},
 									},
-									Ingress: k8sObjectInfoOfHTTPRoute(routes[0]),
+									Ingress: util.FromK8sObject(routes[0]),
 								},
 
 								// Matches from rule 3, that has different filter, are not consolidated
@@ -1460,7 +1460,7 @@ func TestIngressRulesFromHTTPRoutes(t *testing.T) {
 											kong.String("k8s-version:v1beta1"),
 										},
 									},
-									Ingress: k8sObjectInfoOfHTTPRoute(routes[0]),
+									Ingress: util.FromK8sObject(routes[0]),
 									Plugins: []kong.Plugin{
 										{
 											Name: kong.String("request-transformer"),
@@ -1556,7 +1556,7 @@ func TestIngressRulesFromHTTPRoutes(t *testing.T) {
 										kong.String("k8s-version:v1beta1"),
 									},
 								},
-								Ingress: k8sObjectInfoOfHTTPRoute(routes[0]),
+								Ingress: util.FromK8sObject(routes[0]),
 							}},
 							Parent: routes[0],
 						},
@@ -1725,7 +1725,7 @@ func TestIngressRulesFromHTTPRoutes_RegexPrefix(t *testing.T) {
 										kong.String("k8s-version:v1beta1"),
 									},
 								},
-								Ingress: k8sObjectInfoOfHTTPRoute(routes[0]),
+								Ingress: util.FromK8sObject(routes[0]),
 							}},
 							Parent: routes[0],
 						},
@@ -2537,21 +2537,6 @@ func TestIngressRulesFromSplitHTTPRouteMatchWithPriority(t *testing.T) {
 	}
 }
 
-func k8sObjectInfoOfHTTPRoute(route *gatewayapi.HTTPRoute) util.K8sObjectInfo {
-	anotations := route.Annotations
-
-	return util.K8sObjectInfo{
-		Name:        route.Name,
-		Namespace:   route.Namespace,
-		Annotations: anotations,
-		GroupVersionKind: schema.GroupVersionKind{
-			Group:   "gateway.networking.k8s.io",
-			Version: "v1beta1",
-			Kind:    "HTTPRoute",
-		},
-	}
-}
-
 func commonRouteSpecMock(parentReferentName string) gatewayapi.CommonRouteSpec {
 	return gatewayapi.CommonRouteSpec{
 		ParentRefs: []gatewayapi.ParentReference{{
diff --git a/internal/util/objectmeta_test.go b/internal/util/objectmeta_test.go
index 0419d85f6..2694382ca 100644
--- a/internal/util/objectmeta_test.go
+++ b/internal/util/objectmeta_test.go
@@ -6,6 +6,7 @@ import (
 	"github.com/stretchr/testify/assert"
 	netv1 "k8s.io/api/networking/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime/schema"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 )
 
@@ -43,10 +44,53 @@ func TestFromK8sObject(t *testing.T) {
 				Annotations: map[string]string{"a": "1", "b": "2"},
 			},
 		},
+		{
+			name: "with group version kind",
+			in: &netv1.Ingress{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:        "name",
+					Namespace:   "namespace",
+					Annotations: map[string]string{"a": "1", "b": "2"},
+				},
+				TypeMeta: metav1.TypeMeta{
+					Kind:       "Ingress",
+					APIVersion: "networking.k8s.io/v1",
+				},
+			},
+			want: K8sObjectInfo{
+				Name:        "name",
+				Namespace:   "namespace",
+				Annotations: map[string]string{"a": "1", "b": "2"},
+				GroupVersionKind: schema.GroupVersionKind{
+					Group:   "networking.k8s.io",
+					Version: "v1",
+					Kind:    "Ingress",
+				},
+			},
+		},
 	} {
 		t.Run(tt.name, func(t *testing.T) {
 			got := FromK8sObject(tt.in)
-			assert.EqualValues(t, tt.want, got)
+			assert.Equal(t, tt.want, got)
 		})
 	}
 }
+
+func BenchmarkFromK8sObject(b *testing.B) {
+	in := &netv1.Ingress{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:        "name",
+			Namespace:   "namespace",
+			Annotations: map[string]string{"a": "1", "b": "2"},
+		},
+		TypeMeta: metav1.TypeMeta{
+			Kind:       "Ingress",
+			APIVersion: "networking.k8s.io/v1",
+		},
+	}
+	b.ResetTimer()
+	for i := 0; i < b.N; i++ {
+		out := FromK8sObject(in)
+		_ = out
+	}
+}