apisix-ingress-controller icon indicating copy to clipboard operation
apisix-ingress-controller copied to clipboard

proposal: Add IngressClass support for custom resources

Open tao12345666333 opened this issue 2 years ago • 13 comments

Currently our custom resources are all namespace scoped.

If you want to use multiple apisix-ingress-controllers in the namespace at the same time, this is not allowed.

If we can add ingress class to custom resources, then we can let apisix-ingress-controller handle them separately.

Although this is a very low frequency scene. It is currently being discussed #578

I think we may need to reach a consensus and add more complete information

tao12345666333 avatar Jul 14 '21 17:07 tao12345666333

@tao12345666333 That would be a good way to support the soft isolation, but shall we still use the name ingressClass?

tokers avatar Jul 15 '21 01:07 tokers

IngressClass is a general concept, I think it is more acceptable to take it.

tao12345666333 avatar Jul 15 '21 01:07 tao12345666333

First: Ingress Class has a default value ("apisix"). If you continue to use ingressClass, then only ApisixRoute containing "apisix" will be listened by default. And if you want to listen to all ApisixRoute CRD by default, it seems impossible to achieve? Or put IngressClass To an empty string? Second: Does ApisixRoute CRD need to add the Spec.IngressClass field?

Donghui0 avatar Jul 30 '21 07:07 Donghui0

First: Ingress Class has a default value ("apisix"). If you continue to use ingressClass, then only ApisixRoute containing "apisix" will be listened by default. And if you want to listen to all ApisixRoute CRD by default, it seems impossible to achieve? Or put IngressClass To an empty string? Second: Does ApisixRoute CRD need to add the Spec.IngressClass field?

A tricky way to support listen all ApisixRoute CRs is reserving a wildcard *, when the configured ingressClass is *, we listen all ApisixRoutes.

I think add spec.IngressClass is a good way.

tokers avatar Jul 30 '21 08:07 tokers

First: Ingress Class has a default value ("apisix"). If you continue to use ingressClass, then only ApisixRoute containing "apisix" will be listened by default. And if you want to listen to all ApisixRoute CRD by default, it seems impossible to achieve? Or put IngressClass To an empty string?

The IngressClass can use ingressclass.kubernetes.io/is-default-class to set the default IngressClass.

Second: Does ApisixRoute CRD need to add the Spec.IngressClass field?

Yes, we need to add a field spec.IngressClass.

gxthrj avatar Jul 30 '21 08:07 gxthrj

+1 for add spec.IngressClass field

tao12345666333 avatar Jul 30 '21 08:07 tao12345666333

First: Ingress Class has a default value ("apisix"). If you continue to use ingressClass, then only ApisixRoute containing "apisix" will be listened by default. And if you want to listen to all ApisixRoute CRD by default, it seems impossible to achieve? Or put IngressClass To an empty string? Second: Does ApisixRoute CRD need to add the Spec.IngressClass field?

A tricky way to support listen all ApisixRoute CRs is reserving a wildcard *, when the configured ingressClass is *, we listen all ApisixRoutes.

I think add spec.IngressClass is a good way.

Consider this scenario: At present, some people have used ApisixRoute CRD, there is no ingressClass for isolation, so the default is to listen to all ApisixRoute CRDs. But when they upgraded to the new ingress-controller version, they found that by default Controller only listen to CRDs containing "apisix". Is this situation unreasonable? Or change the default value of ingressClass to "*"? But this will affect Ingress CRD.

Donghui0 avatar Jul 30 '21 08:07 Donghui0

First: Ingress Class has a default value ("apisix"). If you continue to use ingressClass, then only ApisixRoute containing "apisix" will be listened by default. And if you want to listen to all ApisixRoute CRD by default, it seems impossible to achieve? Or put IngressClass To an empty string? Second: Does ApisixRoute CRD need to add the Spec.IngressClass field?

A tricky way to support listen all ApisixRoute CRs is reserving a wildcard *, when the configured ingressClass is *, we listen all ApisixRoutes. I think add spec.IngressClass is a good way.

Consider this scenario: At present, some people have used ApisixRoute CRD, there is no ingressClass for isolation, so the default is to listen to all ApisixRoute CRDs. But when they upgraded to the new ingress-controller version, they found that by default Controller only listen to CRDs containing "apisix". Is this situation unreasonable? Or change the default value of ingressClass to "*"? But this will affect Ingress CRD.

As we already have similar logic about IngressClass resource, we may respect it in other APISIX Ingress Controller CRDs, i.e. the default value of spec.ingrssClass will be decided by the default IngressClass resource.

tokers avatar Aug 01 '21 11:08 tokers

This issue has been marked as stale due to 90 days of inactivity. It will be closed in 30 days if no further activity occurs. If this issue is still relevant, please simply write any comment. Even if closed, you can still revive the issue at any time or discuss it on the [email protected] list. Thank you for your contributions.

github-actions[bot] avatar Jun 29 '22 01:06 github-actions[bot]

Looking forward to supporting ingressClass, our scenario: we will have a lot of domain names (several thousand), these domain names belong to different businesses, in order to avoid mutual influence between businesses, we use ingressClass to group domain names, so we are looking forward to CRD ingressClass support

tangzhenhuang avatar Aug 11 '22 06:08 tangzhenhuang

got it. Once we complete this function, it is also very convenient for users to achieve multi-tenant isolation

tao12345666333 avatar Aug 11 '22 08:08 tao12345666333

#593 Can it be merged ? Support ingressClass in ApisixRoute resource.

Donghui0 avatar Aug 11 '22 10:08 Donghui0

@Donghui0 thanks for your contributions! Yes, I'd love to merge it!

But there are some conflicts, and I want the feature to just be added to the v2 version of the ApisixRoute

tao12345666333 avatar Aug 19 '22 19:08 tao12345666333

After discussing with @Donghui0 and @Gallardot , it has been decided that @Donghui0 will be responsible for implementing this feature in ApisixRoute. I will provide a more detailed technical proposal base on https://github.com/apache/apisix-ingress-controller/pull/1523#issuecomment-1429344801

Firstly, we will use the IngressClass field for configuration. All resources will be filtered through the IngressClass field. This is an existing configuration item, so we will need to add this field to custom resources and implement processing logic for this field in custom resources.

Secondly, we need to avoid making breaking changes to users. The specific scenario is as follows:

  • If users deploy the APISIX Ingress project with default configuration, the apisix-ingress-controller will use a configuration with a value of apisix by default. In this scenario, the IngressClass field has not been configured in the custom resources, so all custom resources will be processed.
  • If users modify the value of ingress-class in the apisix-ingress-controller, they may have created corresponding IngressClass resources.

To handle these custom resources properly, we need to add a mechanism to detect whether the IngressClass field has been set in the custom resource or not. If it has been set, the controller will use that value to filter the resources. If not, it will fall back to the default value set in the apisix-ingress-controller configuration.

In this way, we can ensure that both the default configuration and custom configurations with a specified IngressClass value can be processed properly, without causing any destructive changes for the user.

Based on the above description, I believe we can modify the default value to apisix-and-default or apisix-and-all. If a user uses this value, we can assume it is the default installation. Then, apisix-ingress-controller will handle all resources with apisix IngressClass and resources without an IngressClass field.

In addition, we also need to consider the scenario where there are multiple resources with the same IngressClass configuration.

If two controllers have the same ingress-class configuration value, we need to distinguish them based on the controller name. However, I believe this feature can be implemented independently.

tao12345666333 avatar Mar 01 '23 03:03 tao12345666333

This looks to be mostly done, except for watching (and adding to helm) the actual IngressClass (networking.k8s.io/v1) object for defaulting and possibly claiming by controllername instead of just matching the ingressClassName, no?

Adding the resource might also be desirable for discovery.

acuteaura avatar Aug 17 '23 11:08 acuteaura

https://github.com/apache/apisix-ingress-controller/releases/tag/v1.7.0

V1.7.0 has been published, I will close this one, thanks all!!!

tao12345666333 avatar Sep 11 '23 13:09 tao12345666333