-
Notifications
You must be signed in to change notification settings - Fork 8.1k
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
Support namespaced ingress without accessing the IngressClass #11223
base: main
Are you sure you want to change the base?
Support namespaced ingress without accessing the IngressClass #11223
Conversation
…ect and using the annotation. suggestions: IngressController needn't cluster level permission to access the IngressClass for namespaced Ingress consumer drop annotation "kubernetes.io/ingress.class" from ingress Consumer set the ingressClassName by ingress.Spec.IngressClassName IngressController accept the incoming ingress object when a) IngressController has permission to IngressClass, keep the current implementation. b) IngressController dont' have permission to access the IngressClass but ingress.Spec.IngressClassName is equals to the ingress class name specified by CLI parameter "--ingress-class"
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: yong-jie-gong The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
This issue is currently awaiting triage. If Ingress contributors determines this is a relevant issue, they will accept it by applying the The Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
Hi @yong-jie-gong. Thanks for your PR. I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
✅ Deploy Preview for kubernetes-ingress-nginx canceled.
|
…s else and outdent its block (revive)
This sounds like trying to get around a cluster admin not allowing deployments. |
So it seems this is meant to implement what the docs talk about below
https://kubernetes.io/docs/concepts/services-networking/ingress/#ingressclass-scope |
/kind feature |
Yes, exactly, with current k8s implementation,IngressClass is always cluster visible resource, in most of shared cluster, cluster admin don't allow the namespaced application to access any cluster level resource. |
|
@longwuyuan thanks for quick response.
with statement above, if applications are deployed in different customer environment secenario 1, scenario 2.2, all of Ingress objects in applications have to be created with IngressClassName(ingreess.spec.IngressClassName) for some customers or annotations(ingress.metadata.annotaiton.<ingress-classs>) for some other customer. besides, from k8s 1.24, it continue to print warnings if any ingress contains annnotation "kubernetes.io/ingress.class". Regarding namespace scoped instance, before k8s 1.24, Ingress object is pure the Namespace objects and not assoicate with cluster level resource IngressClass by attribute Ingress.Spec.IngresssClassName. |
@yong-jie-gong I think you completely missed the key aspects I pointed out.
So I don't think its a improvement to change the code of the project itself permanently to namespace scoped controller install. If you are installing this project's ingress-controller and the cluster-admin is not aware that a external-ip address is provisioned to input traffic into the cluster from outside the cluster, then that is a breach of trust and should not be allowed to happen. And then I glanced at your code changes. I am not a developer but from what I understand, your changes do not help other users of this project and your changes are not even implemented with best practices. For example you did not care to even write a test or describe what will happen to |
|
I request that you edit the issue-description in Issue #11222 as per below idea
|
@longwuyuan As requested, I has updated more information in defect #11222 |
@strongjz @tao12345666333 @rikatz @longwuyuan someone can help review? |
/assign I will add this to my list and finish the review this week. Thank you |
…ect and using the annotation.
so it is better support namespaced ingressClass without accessing the IngresClass object and using the annotation.
suggestions:
What this PR does / why we need it:
#11222 #11222
Types of changes
Which issue/s this PR fixes
fix #11222
How Has This Been Tested?
Checklist: