address comments

CHANGELOG.md

@@ -112,7 +112,7 @@ Adding a new version? You'll need three changes:
   in translating and applying configurations to Kong.
   [#5296](https://github.com/Kong/kubernetes-ingress-controller/pull/5296)
   [#5299](https://github.com/Kong/kubernetes-ingress-controller/pull/5299)
- - Added validation on `Secret`s to reject the change if it will genereate
+ - Added validation on `Secret`s to reject the change if it will generate
    invalid confiugration of plugins for `KongPlugin`s or `KongClusterPlugin`s
    referencing to the secret.
    [#5203](https://github.com/Kong/kubernetes-ingress-controller/pull/5203)

internal/admission/handler.go

@@ -288,9 +288,7 @@ func (h RequestHandler) checkReferrersOfSecret(ctx context.Context, secret *core
 
 	for _, obj := range referrers {
 		gvk := obj.GetObjectKind().GroupVersionKind()
-		// REVIEW: Should we check version here? Seems that we do not need to support KongPlugin and KongClusterPlugin in other versions.
-		if gvk.Group == kongv1.GroupVersion.Group && gvk.Kind == KindKongPlugin {
-			// REVIEW: run type check here to avoid panic, although it should be unlikely to happen after checking the GVK?
+		if gvk.Group == kongv1.GroupVersion.Group && gvk.Version == kongv1.GroupVersion.Version && gvk.Kind == KindKongPlugin {
 			plugin := obj.(*kongv1.KongPlugin)
 			ok, message, err := h.Validator.ValidatePlugin(ctx, *plugin, []*corev1.Secret{secret})
 			if err != nil {
@@ -305,7 +303,7 @@ func (h RequestHandler) checkReferrersOfSecret(ctx context.Context, secret *core
 					), nil
 			}
 		}
-		if gvk.Group == kongv1.GroupVersion.Group && gvk.Kind == KindKongClusterPlugin {
+		if gvk.Group == kongv1.GroupVersion.Group && gvk.Version == kongv1.GroupVersion.Version && gvk.Kind == KindKongClusterPlugin {
 			plugin := obj.(*kongv1.KongClusterPlugin)
 			ok, message, err := h.Validator.ValidateClusterPlugin(ctx, *plugin, []*corev1.Secret{secret})
 			if err != nil {

internal/admission/validator.go

@@ -56,13 +56,16 @@ type ConsumerGetter interface {
 	ListAllConsumers(ctx context.Context) ([]kongv1.KongConsumer, error)
 }
 
-var _ kongstate.SecretGetter = &SecretGetterWithOverride{}
-
+// SecretGetterWithOverride returns the override secrets in the list if the namespace and name matches,
+// or use the nested secretGetter to fetch the secret otherwise.
+// Used for validating changes of secrets to override existing the one in cache with the one to be updated.
 type SecretGetterWithOverride struct {
 	overrideSecrets map[k8stypes.NamespacedName]*corev1.Secret
 	secretGetter    kongstate.SecretGetter
 }
 
+var _ kongstate.SecretGetter = &SecretGetterWithOverride{}
+
 func (s *SecretGetterWithOverride) GetSecret(namespace, name string) (*corev1.Secret, error) {
 	nsName := k8stypes.NamespacedName{
 		Namespace: namespace,
@@ -76,6 +79,7 @@ func (s *SecretGetterWithOverride) GetSecret(namespace, name string) (*corev1.Se
 	return s.secretGetter.GetSecret(namespace, name)
 }
 
+// NewSecretGetterWithOverride returns a secret getter with given override secrets.
 func NewSecretGetterWithOverride(s kongstate.SecretGetter, overrideSecrets []*corev1.Secret) *SecretGetterWithOverride {
 	overrideSecretMap := lo.SliceToMap(overrideSecrets, func(secret *corev1.Secret) (k8stypes.NamespacedName, *corev1.Secret) {
 		return k8stypes.NamespacedName{

internal/controllers/reference/indexer.go

@@ -267,6 +267,7 @@ func (c CacheIndexers) ListReferredObjects(referrer client.Object) ([]client.Obj
 	return objs, nil
 }
 
+// ListReferrerObjectsByReferent lists all objects that refers to the same referent.
 func (c CacheIndexers) ListReferrerObjectsByReferent(referent client.Object) ([]client.Object, error) {
 	refs, err := c.ListReferencesByReferent(referent)
 	if err != nil {

internal/controllers/reference/indexer_test.go

@@ -263,7 +263,7 @@ func TestDeleteReferencesByReferrer(t *testing.T) {
 	}
 }
 
-func TestListReferencesByReferent(t *testing.T) {
+func TestListReferrerObjectsByReferent(t *testing.T) {
 	testCases := []struct {
 		name          string
 		addReferrer   client.Object

internal/manager/setup.go

@@ -171,7 +171,6 @@ func setupAdmissionServer(
 	ctx context.Context,
 	managerConfig *Config,
 	clientsManager *clients.AdminAPIClientsManager,
-	// REVIEW: where to add it? More general: where to add new args in constructors, all append to the last?
 	referenceIndexers ctrlref.CacheIndexers,
 	managerClient client.Client,
 	logger logr.Logger,