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

Adapt the nginx.org/rewrites from the official nginxinc helm chart #11124

Closed
philipp-durrer-jarowa opened this issue Mar 14, 2024 · 10 comments
Closed
Assignees
Labels
kind/feature Categorizes issue or PR as related to a new feature. needs-priority needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one.

Comments

@philipp-durrer-jarowa
Copy link

We have a need to use per-service rewrites like the Nginx Inc. maintained helm chart supports (see https://github.com/nginxinc/kubernetes-ingress/tree/main/examples/ingress-resources/rewrites) however we wouldn't like to switch over to that chart as all our clusters use this chart here, so it would be great if one could adopt the same feature in a nginx.ingress.kubernetes.io/rewrites annotation.

I haven't found any other issues talking about the same feature.

No specific Kubernetes version needed for this feature.

@philipp-durrer-jarowa philipp-durrer-jarowa added the kind/feature Categorizes issue or PR as related to a new feature. label Mar 14, 2024
@k8s-ci-robot
Copy link
Contributor

This issue is currently awaiting triage.

If Ingress contributors determines this is a relevant issue, they will accept it by applying the triage/accepted label and provide further guidance.

The triage/accepted label can be added by org members by writing /triage accepted in a comment.

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.

@k8s-ci-robot k8s-ci-robot added needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. needs-priority labels Mar 14, 2024
@longwuyuan
Copy link
Contributor

And you are already using this https://kubernetes.github.io/ingress-nginx/examples/rewrite/ ?

@philipp-durrer-jarowa
Copy link
Author

philipp-durrer-jarowa commented Mar 14, 2024

Yes but we'd like to have only one ingress resource per domain, but there are multiple services behind it that need different rewrites. Example:

...
    - host: "{{ $tenant }}.{{ .host }}"
      http:
        paths:
          - path: "/"
            pathType: Prefix
            backend:
              service:
                name: {{ $fullName }}-frontend
                port:
                  name: http-fe
          - path: "/api/"     ---> needs a rewrite to /
            pathType: Prefix
            backend:
              service:
                name: {{ $fullName }}-backend
                port:
                  name: http
          - path: "/static"     ---> needs a rewrite to /$tenant/static/
            pathType: Prefix
            backend:
              service:
                name: {{ $fullName }}-data
                port:
                  name: http-fe
...

With the nginx.org/rewrites I could do this:

    nginx.org/rewrites: "serviceName={{ $fullName }}-backend rewrite=/; serviceName={{ $fullName }}-data rewrite=/{{ $tenant }}/static;"

@longwuyuan
Copy link
Contributor

Thanks for clarifying.

@strongjz
Copy link
Member

/assign @Gacko

I will leave this up to @Gacko, but this would be a breaking change for all other uses. The two projects are different, so annotations should reflect that.

@Gacko
Copy link
Member

Gacko commented Mar 14, 2024

Hm, I think we're mixing up things a bit here.

What @philipp-durrer-jarowa asks for is the ability to have just one Ingress resource per domain (IIRC we are kind of enforcing that somewhere during validations anyway) with different backends separated by path.

In their use case each of those backends requires its own rewrite configuration. Some example requests:

http://first-tenant.great-service.com/anything -> http://frontend-service/anything
http://first-tenant.great-service.com/api/anything -> http://service-backend/anything
http://first-tenant.great-service.com/static/anything -> http://data-backend/first-tenant/anything

If that's correct, I do not think it's related to the chart but more to how we're rendering rewrites in locations.

@longwuyuan
Copy link
Contributor

It seems like this controller's one annotation will perform a single rewrite of the target-path to all rules in that ingress.

But what is @philipp-durrer-jarowa is asking is a combo like ;

  • one annotation
  • multiple rewrite operations defined in the value part of that one single annotation (new ability)

And @Gacko's example is showing a 2nd new ability of even having option to pick different backends for different rewrite operations, all in the same annotation

@Gacko
Copy link
Member

Gacko commented Mar 14, 2024

And @Gacko's example is showing a 2nd new ability of even having option to pick different backends for different rewrite operations, all in the same annotation

That is already covered by the rules matching the according paths.

What's not yet covered is the ability to specify a rewrite rule per backend. So in case a requests gets sent to backend A, apply rewrite rule A, if backend b then rewrite rule B and in all the other cases either do nothing or apply a general rewrite rule.

I really don't wanna over-engineer things, but that's how I understand the NGINX Inc. documentation.

@strongjz
Copy link
Member

This will be a feature for support in HTTP routes https://gateway-api.sigs.k8s.io/guides/http-redirect-rewrite/

We are working on gateway API support and cannot implement this issue in ingress.

/close

@k8s-ci-robot
Copy link
Contributor

@strongjz: Closing this issue.

In response to this:

This will be a feature for support in HTTP routes https://gateway-api.sigs.k8s.io/guides/http-redirect-rewrite/

We are working on gateway API support and cannot implement this issue in ingress.

/close

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature Categorizes issue or PR as related to a new feature. needs-priority needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one.
Projects
Development

No branches or pull requests

5 participants