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

Remove ns query string from URI when submitting requests to upstream. #6

Closed
theodiem opened this issue Jul 13, 2023 · 4 comments
Closed

Comments

@theodiem
Copy link

As mentioned on #5:

While I was writing some regex rules in traefik (which i configured in front of oci-registry), so I could forcefully "support" the ?ns={namespace} query ... I've noticed other manifests failing.
For example:

  • works : curl "https://gcr.io/v2/kaniko-project/executor/manifests/v1.9.1-debug"
  • doesn't work: curl "https://oci-registry:8080/v2/kaniko-project/executor/manifests/v1.9.1-debug?ns=gcr.io"
    Maybe because: curl "https://gcr.io/v2/kaniko-project/executor/manifests/v1.9.1-debug?ns=gcr.io" also returns a 404? ....

Originally posted by @theodiem in #5 (comment)

I can confirm by disabling TLS on upstreams.yaml and sniffing the traffic, that ns query parameter gets sent to upstream and some servers (at least gcr.io) returns a 404 if you query data with it. It works fine without it.

@mcronce
Copy link
Owner

mcronce commented Jul 14, 2023

Interesting find - thanks for the report! I was using this for gcr.io images extensively at my former job, but as of today can't find a single image on gcr.io that pulls successfully. I'll dig in further and see if there appears to be any nuance to it; worst case, I'll exclude ns when hitting gcr.io

@theodiem
Copy link
Author

I haven't used many, just stumbled when submitting my kaniko builds to my (local) cluster instead of doing locally.
Maybe having as a flag that you can set in upstream.yaml? Things like test $upstream = "gcr.io" && mangle_url unsettles my heart ;)

@mcronce
Copy link
Owner

mcronce commented Jul 15, 2023

Just pushed a fix for this with 3c62c24; I had code to handle it, but a dependency update broke the functionality. I'll spin a release today, hoping to fix at least one of the other open issues first.

mcronce added a commit that referenced this issue Jul 15, 2023
mcronce added a commit that referenced this issue Jul 15, 2023
@theodiem
Copy link
Author

I can confirm it works for me. Many thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants