nginx-directory: preserve url encoded chars

[darrenjaneczek]

It was discovered that encoded characters such as %20 were converted through the proxy engine, resulting in 400 "bad request." This would affect urls such as: e.g., baseurl.com/server_name/resource%20with%20space

The previous solution uses a regex location selector, and with that approach the $2 group variable would decode the url, causing the 400 if there were any special %-encoded characters. The example above, resulted in: baseurl.com/server_name/resource with space

After replacing to a non-regex location selector, some additional efforts were required to ensure:

• relative urls work within the proxy subdir
• redirects from the target server are translated correctly
• %-encoded characters proxy without 400 "bad request"

[darrenjaneczek]

Not ready for merge. Will work in this further in a few weeks.

Required outstanding fixes:

• [ ] Handle paths that end in /, e.g., path: 'endwithslash/',
• [ ] Handle requests to legitimate "path" entries, which end in //, e.g., https://site.net/test// where path: 'test'
• [ ] Note potential environment setup errors if there are invalid entries in the configuration; see:
  • https://github.com/grafana/deployment_tools/pull/13996
  • https://github.com/grafana/deployment_tools/pull/13998

    $ kubectl -n default logs nginx-6bc87ddb99-74qd2 
    2021/07/30 13:59:12 [emerg] 1#1: host not found in upstream "[cluster]" in [file:line]
    

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[jdbaldry]

Still relevant

[darrenjaneczek]

Plans to look into this again this quarter. December at the latest.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[jdbaldry]

I believe this is still valid

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[jdbaldry]

@darrenjaneczek Is there still desire to get this merged?

[darrenjaneczek]

Yes. I still need to correct some of the identified issues above.

[CLAassistant]

All committers have signed the CLA.

[gaantunes]

Hello folks, is this still WIP?

[jdbaldry]

I believe so. @darrenjaneczek is this something you want to pick up again?

[darrenjaneczek]

Sorry for this prolonged story. The effort is over a year old now!

The solution as it appears in this PR isn't complete because it creates some errors, itemized here:

• https://github.com/grafana/jsonnet-libs/pull/624#issuecomment-889938857

My difficulty in picking this up again is that I would need to re-learn how to set up my environment to work with these files. I would probably need some help to get it up and running again, and to refresh my mental model of how to work with nginx.

[jdbaldry]

I'm not sure that the cost of revisiting this issue is worth it at this point in time unless we have seen continued issues with the current behavior so I'm in favor of closing this PR and using it as a starting point if we need to revisit the issue at a later point.

[darrenjaneczek]

That sounds pragmatic. It is rare that any of our APIs require spaces in the URLs, and that particular one that triggered my investigation was worked-around long ago.