fix(controllers) populate TypeMeta consistently (#4767)

Add a type-agnostic helper to populate TypeMeta using a scheme with
registered types.

Populate TypeMeta throughout all controllers and reference filter
predicates.

Remove feature gate limitations on types added to the manager scheme to
simplify its use where config is not readily available.

CHANGELOG.md

@@ -120,6 +120,8 @@ Adding a new version? You'll need three changes:
 
 - No more "log.SetLogger(...) was never called..." log entry during shutdown of KIC
  [#4738](https:/Kong/kubernetes-ingress-controller/pull/4738)
+- Changes to referenced Secrets are now tracked independent of their referent.
+ [#4758](https:/Kong/kubernetes-ingress-controller/pull/4758)
 - When Kong returns a flattened error related to a Kong entity, the entity's type and name
  will be included in the message reported in `KongConfigurationApplyFailed` Kubernetes event
  generated for it.

hack/generators/controllers/networking/main.go

@@ -397,7 +397,6 @@ import (
  discoveryv1 "k8s.io/api/discovery/v1"
  netv1 "k8s.io/api/networking/v1"
  apierrors "k8s.io/apimachinery/pkg/api/errors"
- metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
  "k8s.io/apimachinery/pkg/runtime"
  "k8s.io/apimachinery/pkg/runtime/schema"
  k8stypes "k8s.io/apimachinery/pkg/types"
@@ -550,11 +549,6 @@ func (r *{{.PackageAlias}}{{.Kind}}Reconciler) Reconcile(ctx context.Context, re
 
  // get the relevant object
  obj := new({{.PackageImportAlias}}.{{.Kind}})
- // set type meta to the object
- obj.TypeMeta = metav1.TypeMeta{
- APIVersion: {{.PackageImportAlias}}.SchemeGroupVersion.String(),
- Kind: "{{.Kind}}",
- }
 
  if err := r.Get(ctx, req.NamespacedName, obj); err != nil {
  if apierrors.IsNotFound(err) {

internal/controllers/configuration/secret_controller.go

@@ -113,6 +113,7 @@ func (r *CoreV1SecretReconciler) Reconcile(ctx context.Context, req ctrl.Request
  }
  return ctrl.Result{}, err
  }
+
  log.V(util.DebugLevel).Info("reconciling resource", "namespace", req.Namespace, "name", req.Name)
 
  // clean the object up if it's being deleted

internal/controllers/gateway/gateway_controller.go

@@ -37,13 +37,7 @@ import (
 // Vars & Consts
 // -----------------------------------------------------------------------------
 
-var (
- gatewayV1beta1Group = gatewayapi.Group(gatewayv1beta1.GroupName)
- gatewayTypeMeta = metav1.TypeMeta{
- APIVersion: gatewayv1beta1.GroupVersion.String(),
- Kind: "Gateway",
- }
-)
+var gatewayV1beta1Group = gatewayapi.Group(gatewayv1beta1.GroupName)
 
 // -----------------------------------------------------------------------------
 // Gateway Controller - GatewayReconciler
@@ -341,7 +335,6 @@ func (r *GatewayReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct
  // gather the gateway object based on the reconciliation trigger. It's possible for the object
  // to be gone at this point in which case it will be ignored.
  gateway := new(gatewayapi.Gateway)
- gateway.TypeMeta = gatewayTypeMeta
  if err := r.Get(ctx, req.NamespacedName, gateway); err != nil {
  if apierrors.IsNotFound(err) {
  gateway.Namespace = req.Namespace
@@ -356,6 +349,7 @@ func (r *GatewayReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct
  }
  return ctrl.Result{Requeue: true}, err
  }
+
  debug(log, gateway, "processing gateway")
 
  // though our watch configuration eliminates reconciliation of unsupported gateways it's

internal/controllers/gateway/gatewayclass_controller.go

@@ -120,6 +120,7 @@ func (r *GatewayClassReconciler) Reconcile(ctx context.Context, req ctrl.Request
  }
  return ctrl.Result{}, err
  }
+
  log.V(util.DebugLevel).Info("processing gatewayclass", "name", req.Name)
 
  if isGatewayClassControlledAndUnmanaged(gwc) {

internal/controllers/gateway/grpcroute_controller.go

@@ -272,6 +272,7 @@ func (r *GRPCRouteReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
  // for any error other than 404, requeue
  return ctrl.Result{}, err
  }
+
  debug(log, grpcroute, "processing grpcroute")
 
  // if there's a present deletion timestamp then we need to update the proxy cache

internal/controllers/gateway/httproute_controller.go

@@ -346,6 +346,7 @@ func (r *HTTPRouteReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
  // for any error other than 404, requeue
  return ctrl.Result{}, err
  }
+
  debug(log, httproute, "processing httproute")
 
  // if there's a present deletion timestamp then we need to update the proxy cache

internal/controllers/gateway/referencegrant_controller.go

@@ -84,6 +84,7 @@ func (r *ReferenceGrantReconciler) Reconcile(ctx context.Context, req ctrl.Reque
  // for any error other than 404, requeue
  return ctrl.Result{}, err
  }
+
  debug(log, grant, "processing referencegrant")
 
  debug(log, grant, "checking deletion timestamp")

internal/controllers/gateway/tcproute_controller.go

@@ -269,6 +269,7 @@ func (r *TCPRouteReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c
  // for any error other than 404, requeue
  return ctrl.Result{}, err
  }
+
  debug(log, tcproute, "processing tcproute")
 
  // if there's a present deletion timestamp then we need to update the proxy cache

internal/controllers/gateway/tlsroute_controller.go

@@ -268,6 +268,7 @@ func (r *TLSRouteReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c
  // for any error other than 404, requeue
  return ctrl.Result{}, err
  }
+
  debug(log, tlsroute, "processing tlsroute")
 
  // if there's a present deletion timestamp then we need to update the proxy cache

internal/controllers/gateway/udproute_controller.go

@@ -268,6 +268,7 @@ func (r *UDPRouteReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c
  // for any error other than 404, requeue
  return ctrl.Result{}, err
  }
+
  debug(log, udproute, "processing udproute")
 
  // if there's a present deletion timestamp then we need to update the proxy cache

internal/controllers/reference/indexer.go

@@ -8,6 +8,7 @@ import (
  "sigs.k8s.io/controller-runtime/pkg/client"
 
  "github.com/kong/kubernetes-ingress-controller/v2/internal/controllers"
+ "github.com/kong/kubernetes-ingress-controller/v2/internal/manager/scheme"
  "github.com/kong/kubernetes-ingress-controller/v2/internal/util"
 )
 
@@ -29,9 +30,25 @@ type ObjectReference struct {
 // objectKeyFunc returns a k8s object key in the following format:
 // group/version,Kind=kind/namespace/name.
 // the combination is unique inside a kubernetes cluster.
-func objectKeyFunc(obj client.Object) string {
- return obj.GetObjectKind().GroupVersionKind().String() + "/" +
- obj.GetNamespace() + "/" + obj.GetName()
+func objectKeyFunc(obj client.Object) (string, error) {
+ // TypeMeta is necessary to generate the correct key for references, but we can't use the original object.
+ // controller-runtime's client provides the same object to both predicates and the admission webhook, and can result
+ // in a race condition if this uses the original
+ o := obj.DeepCopyObject()
+ metaObj, ok := o.(client.Object)
+ if !ok {
+ return "", fmt.Errorf("could not convert %s/%s back to client Object", obj.GetNamespace(), obj.GetName())
+ }
+ s, err := scheme.Get()
+ if err != nil {
+ return "", fmt.Errorf("could not get scheme for %s/%s metadata: %w", obj.GetNamespace(), obj.GetName(), err)
+ }
+ err = util.PopulateTypeMeta(o, s)
+ if err != nil {
+ return "", fmt.Errorf("could not populate %s/%s metadata: %w", obj.GetNamespace(), obj.GetName(), err)
+ }
+ return metaObj.GetObjectKind().GroupVersionKind().String() + "/" +
+ metaObj.GetNamespace() + "/" + metaObj.GetName(), nil
 }
 
 // CacheIndexers implements a reference cache to store reference relationship between k8s objects
@@ -62,8 +79,14 @@ func ObjectReferenceKeyFunc(obj interface{}) (string, error) {
  if !ok {
  return "", ErrTypeNotObjectReference
  }
- referrerKey := objectKeyFunc(ref.Referrer)
- referentKey := objectKeyFunc(ref.Referent)
+ referrerKey, err := objectKeyFunc(ref.Referrer)
+ if err != nil {
+ return "", err
+ }
+ referentKey, err := objectKeyFunc(ref.Referent)
+ if err != nil {
+ return "", err
+ }
 
  return referrerKey + ":" + referentKey, nil
 }
@@ -76,7 +99,11 @@ func ObjectReferenceIndexerReferrer(obj interface{}) ([]string, error) {
  if !ok {
  return nil, ErrTypeNotObjectReference
  }
- return []string{objectKeyFunc(ref.Referrer)}, nil
+ referrerKey, err := objectKeyFunc(ref.Referrer)
+ if err != nil {
+ return []string{}, err
+ }
+ return []string{referrerKey}, nil
 }
 
 // ObjectReferenceIndexerReferent is the index function to index by the referent,
@@ -87,7 +114,11 @@ func ObjectReferenceIndexerReferent(obj interface{}) ([]string, error) {
  if !ok {
  return nil, ErrTypeNotObjectReference
  }
- return []string{objectKeyFunc(ref.Referent)}, nil
+ referentKey, err := objectKeyFunc(ref.Referent)
+ if err != nil {
+ return []string{}, err
+ }
+ return []string{referentKey}, nil
 }
 
 // SetObjectReference adds or updates a reference record between referrer and referent in reference cache.
@@ -127,7 +158,11 @@ func (c CacheIndexers) DeleteObjectReference(referrer client.Object, referent cl
 // ObjectReferred returns true if an object is referenced (being the referent)
 // in at least one reference record.
 func (c CacheIndexers) ObjectReferred(obj client.Object) (bool, error) {
- refs, err := c.indexer.ByIndex(IndexNameReferent, objectKeyFunc(obj))
+ objKey, err := objectKeyFunc(obj)
+ if err != nil {
+ return false, err
+ }
+ refs, err := c.indexer.ByIndex(IndexNameReferent, objKey)
  if err != nil {
  return false, err
  }
@@ -138,7 +173,11 @@ func (c CacheIndexers) ObjectReferred(obj client.Object) (bool, error) {
 // ListReferencesByReferrer lists all reference records where referrer has the same key
 // (GroupVersionKind+NamespacedName, that means the same k8s object).
 func (c CacheIndexers) ListReferencesByReferrer(referrer client.Object) ([]*ObjectReference, error) {
- refList, err := c.indexer.ByIndex(IndexNameReferrer, objectKeyFunc(referrer))
+ referrerKey, err := objectKeyFunc(referrer)
+ if err != nil {
+ return nil, err
+ }
+ refList, err := c.indexer.ByIndex(IndexNameReferrer, referrerKey)
  if err != nil {
  return nil, err
  }
@@ -157,7 +196,10 @@ func (c CacheIndexers) ListReferencesByReferrer(referrer client.Object) ([]*Obje
 // (GroupVersionKind+NamespacedName, that means the same k8s object).
 // called when a k8s object deleted in cluster, or when we do not care about it anymore.
 func (c CacheIndexers) DeleteReferencesByReferrer(referrer client.Object) error {
- key := objectKeyFunc(referrer)
+ key, err := objectKeyFunc(referrer)
+ if err != nil {
+ return err
+ }
  refs, err := c.indexer.ByIndex(IndexNameReferrer, key)
  if err != nil {
  return err