Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ingress class watching not disabled when fetching ingress class by name #2712

Closed
1 task done
jasiek-zywczak opened this issue Jul 19, 2022 · 1 comment · Fixed by #2724
Closed
1 task done

ingress class watching not disabled when fetching ingress class by name #2712

jasiek-zywczak opened this issue Jul 19, 2022 · 1 comment · Fixed by #2724
Labels
bug Something isn't working priority/medium

Comments

@jasiek-zywczak
Copy link

jasiek-zywczak commented Jul 19, 2022

Is there an existing issue for this?

  • I have searched the existing issues

Current Behavior

When I disable watching of ingress class by KIC by env (of flag) CONTROLLER_ENABLE_CONTROLLER_INGRESS_CLASS_NETWORKINGV1 I expect igressClass was not watched or listed. Unfortunately when using "namespaced" mode of KIC, running it only for a particular namespace with flag --watched-namespace I got error when watching every Ingress:

W0719 05:54:22.523590       1 reflector.go:324] pkg/mod/k8s.io/client-go@v0.24.2/tools/cache/reflector.go:167: failed to list *v1.IngressClass: ingressclasses.networking.k8s.io is forbidden: User "system:serviceaccount:kong:kong-serviceaccount" cannot list resource "ingressclasses" in API group "networking.k8s.io" at the cluster scope
E0719 05:54:22.523618       1 reflector.go:138] pkg/mod/k8s.io/client-go@v0.24.2/tools/cache/reflector.go:167: Failed to watch *v1.IngressClass: failed to list *v1.IngressClass: ingressclasses.networking.k8s.io is forbidden: User "system:serviceaccount:kong:kong-serviceaccount" cannot list resource "ingressclasses" in API group "networking.k8s.io" at the cluster scope

Expected Behavior

Ingress was properly reconciled.

Steps To Reproduce

1. In cluster run KIC for a particular namespace (here name namespace)
2. use kong chart with --set ingressController.watchNamespaces={namespace} 
3. correct template with adding CONTROLLER_ENABLE_CONTROLLER_INGRESS_CLASS_NETWORKINGV1 = 'false' set for KIC
4. create ingress in the manespace with ingressClassName == 'kong' (or different if set)
5. check logs in KIC

Kong Ingress Controller version

2.4.2, 2.5.0

Kubernetes version

No response

Anything else?

It seems that in controllers.configuration.zz_generated_controllers.go in line 390 you forgot to check if watching ingressClass is disabled. It also seems - but I am not expert here - that k8s client (reader) used in multinamespace mode (https://pkg.go.dev/sigs.k8s.io/controller-runtime/pkg/client#Reader) is using list and watch to cache ingressClasses, even for the get method. Because of this in call:

if err := r.Get(ctx, types.NamespacedName{Name: r.IngressClassName}, class); err != nil {
			// we log this without taking action to support legacy configurations that only set ingressClassName or
			// used the class annotation and did not create a corresponding IngressClass. We only need this to determine
			// if the IngressClass is default or to configure default settings, and can assume no/no additional defaults
			// if none exists.
			log.V(util.DebugLevel).Info("could not retrieve IngressClass", "ingressclass", r.IngressClassName)
		}

the further processing is blocked (it never pass this line).
The solution might be simple:

if !r.DisableIngressClassLookups {
		if err := r.Get(ctx, types.NamespacedName{Name: r.IngressClassName}, class); err != nil {
			// we log this without taking action to support legacy configurations that only set ingressClassName or
			// used the class annotation and did not create a corresponding IngressClass. We only need this to determine
			// if the IngressClass is default or to configure default settings, and can assume no/no additional defaults
			// if none exists.
			log.V(util.DebugLevel).Info("could not retrieve IngressClass", "ingressclass", r.IngressClassName)
		}
	}
@mflendrich
Copy link
Contributor

mflendrich commented Jul 19, 2022

Thank you @jasiek-zywczak for this bug report. We're prioritizing this for validation and fixing on our side; since there appears to be a simple workaround for this issue (just create the IngressClass resource), we're assigning priority/low.

edit: misunderstood the request, this should be priority/medium indeed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working priority/medium
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants