Mirror backend URL

[naseemkullah]

This PR intends to add nginx.ingress.kubernetes.io/mirror-backend-host nginx.ingress.kubernetes.io/mirror-backend-url annotation which would be useful in conjunction with nginx.ingress.kubernetes.io/mirror-uri.

The annotation, if used, would create a location in nginx.conf.

So far no code has been written per se but I would like feedback as to if this makes sense, while I work on the code.

What this PR does / why we need it:

The mirror feature seems incomplete. When we add a mirror uri, we will surely want to configure a mirror backend, by adding a mirror backend annotation, we could have that create the required location for the mirrored traffic.

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

Which issue/s this PR fixes

fixes #4901

How Has This Been Tested?

Not yet and will surely need some advice on local testing.
For CI testing I will mimic other tests.

Checklist:

• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I've read the CONTRIBUTION guide
• I have added tests to cover my changes.
• All new and existing tests passed.

[aledbf]

Why do you ask?

Because is not possible to specify a different destination for each path.

[naseemkullah]

Why do you ask?

Because is not possible to specify a different destination for each path.

no problem

In my use case, and i believe this would be a common one, we will mirror all paths of a certain ingress, to an equivalent ingress of a test cluster.

[naseemkullah]

@naseemkullah some comments about the PR:

You are adding a new annotation and not removing URI mirror-uri but you changed the docs.

I just changed by applying the below: to applying: to match the other annotations. I just moved the new mirror-backend-url between the mirror-uri and mirror-request-body to make less changes. Is that ok?

With that, and from your comments, you can only define one mirror without using mirror-uri. For this reason I suggest you remove the mirror-uri annotation and auto-generate the default /mirror location.

One question remains: suppose you have prod cluster (prod.env.com) and test cluster (test.env.com) each have ingresses foobar and bazqaz. You decide you want to mirror traffic in prod to test for all foobar traffic, setting the backend-url to https://test.env.com$request_uri yields :

    location = /mirror {
        internal;
        proxy_pass https://test.env.com$request_uri;
    }

as well as, somewhere in the server config for prod.env.com we will have

  location /foobar {
[...]
        mirror /mirror;
[...]
    }

Then we decide we want to add a mirror for bazqaz... will we want to reuse /mirror ? or would it be better to have different mirror-uri's for each e.g. foobar_mirror and bazqaz_mirror ?

please let me know what you think.

Also, please add an a validation for mirror-backend-url (invalid URL).

OK I will work on this soon.

[naseemkullah]

furthermore regarding using /mirror always.

What if we want to mirror 2 ingresses, send 1 to test.env.com and another to test2.env.com ... they surely will need to send to different mirror uri's no?

[naseemkullah]

PS this is the best doc, apart from nginx docs, regarding mirroring: https://alex.dzyoba.com/blog/nginx-mirror/ but it does not talk about having multiple mirrors so I'm not too sure.

[aledbf]

@naseemkullah friendly ping

[aledbf]

@naseemkullah just to be clear about the PR

This PR should remove nginx.ingress.kubernetes.io/mirror-uri, adding nginx.ingress.kubernetes.io/mirror-backend-url, generating an internal location with a random name (like in the auth location annotation), to allow that multiple mirror annotation, defined in different ingresses, but for the same hostname can work.

[naseemkullah]

merged master... @aledbf anything else pending to get this in?

[naseemkullah]

@naseemkullah just to be clear about the PR

This PR should remove nginx.ingress.kubernetes.io/mirror-uri, adding nginx.ingress.kubernetes.io/mirror-backend-url, generating an internal location with a random name (like in the auth location annotation), to allow that multiple mirror annotation, defined in different ingresses, but for the same hostname can work.

In other words, the mirror-uri should always be randomly generated is that it?

[aledbf]

merged master... @aledbf anything else pending to get this in?

squash the commits

[aledbf]

In other words, the mirror-uri should always be randomly generated is that it?

randomly, but should always generate the same url for the same ingress. Please check

ingress-nginx/internal/ingress/controller/template/template.go

Line 406 in 7ff49b2

func buildAuthLocation(input interface{}, globalExternalAuthURL string) string {

[aledbf]

Also, add a lint rule here

ingress-nginx/cmd/plugin/lints/ingress.go

Line 68 in 771fc9f

removedAnnotation("session-cookie-hash", 3743, "0.24.0"),

about the removal

[naseemkullah]

merged master... @aledbf anything else pending to get this in?

squash the commits

Can this be done when you merge? (e.g. squash commit and merge) Im confused by the git log now. there are many commits that came from merging master since my initial commits.

[aledbf]

Can this be done when you merge?

No.

You can download your PR patch https://patch-diff.githubusercontent.com/raw/kubernetes/ingress-nginx/pull/4903.patch to a file, and recreate your mirror-backend branch from master and then applying the patch.

[k8s-ci-robot]

@naseemkullah: PR needs rebase.

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]

@naseemkullah: The following tests failed, say /retest to rerun all failed tests:

Test name	Commit	Details	Rerun command
pull-ingress-nginx-e2e-1-15	b90b7cf	link	/test pull-ingress-nginx-e2e-1-15
pull-ingress-nginx-e2e-1-17	b90b7cf	link	/test pull-ingress-nginx-e2e-1-17
pull-ingress-nginx-test-lua	b90b7cf	link	/test pull-ingress-nginx-test-lua
pull-ingress-nginx-e2e-1-16	b90b7cf	link	/test pull-ingress-nginx-e2e-1-16
pull-ingress-nginx-lualint	b90b7cf	link	/test pull-ingress-nginx-lualint
pull-ingress-nginx-codegen	b90b7cf	link	/test pull-ingress-nginx-codegen
pull-ingress-nginx-test	b90b7cf	link	/test pull-ingress-nginx-test
pull-ingress-nginx-golint	b90b7cf	link	/test pull-ingress-nginx-golint
pull-ingress-nginx-gofmt	b90b7cf	link	/test pull-ingress-nginx-gofmt
pull-ingress-nginx-boilerplate	b90b7cf	link	/test pull-ingress-nginx-boilerplate

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

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. I understand the commands that are listed here.