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

transport: Don't pass default service if unset #1360

Merged
merged 1 commit into from
May 7, 2022

Conversation

imjasonh
Copy link
Collaborator

@imjasonh imjasonh commented May 6, 2022

Fixes #1359

@imjasonh imjasonh requested a review from jonjohnsonjr May 6, 2022 14:13
@codecov-commenter
Copy link

Codecov Report

Merging #1360 (f698316) into main (9d1ceb8) will decrease coverage by 0.00%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #1360      +/-   ##
==========================================
- Coverage   74.20%   74.19%   -0.01%     
==========================================
  Files         113      113              
  Lines        8439     8436       -3     
==========================================
- Hits         6262     6259       -3     
  Misses       1571     1571              
  Partials      606      606              
Impacted Files Coverage Δ
pkg/v1/remote/transport/bearer.go 78.10% <100.00%> (+0.21%) ⬆️
pkg/v1/remote/transport/transport.go 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9d1ceb8...f698316. Read the comment docs.

@Andrei-Stepanov
Copy link

@imjasonh Hi, thank you for super quick fix.
Could you please give a hint how this issue can be propagated to kaniko image?
https://github.com/GoogleContainerTools/kaniko

@imjasonh
Copy link
Collaborator Author

imjasonh commented May 6, 2022

@imjasonh Hi, thank you for super quick fix. Could you please give a hint how this issue can be propagated to kaniko image? https://github.com/GoogleContainerTools/kaniko

When this PR is merged I'll make sure to update Kaniko's dependency to pick it up. Then Kaniko will publish a commit-tagged image with that change that you can try out.

@imjasonh imjasonh merged commit 82405e5 into google:main May 7, 2022
imjasonh added a commit to imjasonh/kaniko that referenced this pull request May 7, 2022
imjasonh added a commit to GoogleContainerTools/kaniko that referenced this pull request May 8, 2022
@Andrei-Stepanov
Copy link

@imjasonh Hello,

Thank you for the merging the PR.

Could you please point to kaniko image, with merged fix?

I see something is related to is:
https://github.com/GoogleContainerTools/kaniko/runs/6342142557?check_suite_focus=true
But I am not sure where it stores built images.

If possible to -debug version: executor:debug

Thank you in advance.

Regards.

@imjasonh
Copy link
Collaborator Author

imjasonh commented May 9, 2022

Could you please point to kaniko image, with merged fix?

The Kaniko commit that bumped the dep to include this change was 816e6d52b4dd9e1909868f8ddc9c9a8294cf0919,

This means there's an image, gcr.io/kaniko-project/executor:816e6d52b4dd9e1909868f8ddc9c9a8294cf0919, which includes this change, and a debug variant at :816e6d52b4dd9e1909868f8ddc9c9a8294cf0919-debug. If you try those out, please let me know if they fixed your issue.

@Andrei-Stepanov
Copy link

Hi,

I am so sorry, but kaniko executor from 816e6d52b4dd9e1909868f8ddc9c9a8294cf0919-debug fails with the same error:

error building image: GET https://registry-proxy.domain.com/v2/rh-osbs/ubi9/manifests/sha256:2ae256a7be38e1d0de200de4f6757da841d0d5fc00048cb4125c8fdb32391e08: unexpected status code 401 Unauthorized: {"error": "Invalid audience"}

Kaniko doesn't log HTTP traffic. Is there crane (can be in container-image) binary that I can use to track HTTP traffic?

Thank you.

@imjasonh
Copy link
Collaborator Author

imjasonh commented May 9, 2022

If the issue is pulling that image, you could definitely do a crane pull $img /dev/null --verbose to see what kind of auth is happening and why that's failing. crane is available in a container image at gcr.io/go-containerregistry/crane, but it won't contain the ?service= change in this PR. It might still be useful for getting more debug information though.

Invalid audience doesn't sound particularly related to the ?service= change, but I admit I might not understand how the registry proxy you're using works.

@Andrei-Stepanov
Copy link

Andrei-Stepanov commented May 9, 2022

Hi, @imjasonh .

Example with curl how JWT bearer token defines reply from registry proxy:

TOKEN1=`curl -X GET -sH 'Accept-encoding: gzip' 'https://registry-proxy.domain.com/v2/auth?scope=repository:rh-osbs/ubi9:pull&service=registry-proxy.domain.com' | jq -r '.token'`
TOKEN2=`curl -X GET -sH 'Accept-encoding: gzip' 'https://registry-proxy.domain.com/v2/auth?scope=repository:rh-osbs/ubi9:pull' | jq -r '.token'`

# TOKEN1 - with &service=registry-proxy.domain.com fails
curl -H "Authorization: Bearer $TOKEN1" -H "Accept: application/vnd.docker.distribution.manifest.list.v2+json" 'https://registry-proxy.domain.com/v2/rh-osbs/ubi9/manifests/latest'
{"error": "Invalid audience"}

^^^^^ exactly in the same way how kaniko fails.

# TOKEN1 - without &service:
curl -H "Authorization: Bearer $TOKEN2" -H "Accept: application/vnd.docker.distribution.manifest.list.v2+json" 'https://registry-proxy.domain.com/v2/rh-osbs/ubi9/manifests/latest'
{
    "manifests": [
...

Decoded TOKEN1 payload:

{
  "iss": "quay",
  "aud": "registry-proxy.domain.com", <-------------------------- with &service=registry-proxy.domain.com
  "nbf": 1652113788,
  "iat": 1652113788,
  "exp": 1652117388,
  "sub": "rh-osbs+internal_registry_proxy_3",
  "access": [
    {
      "type": "repository",
      "name": "rh-osbs/ubi9",
      "actions": [
        "pull"
      ]
    }
  ],
  "context": {
    "version": 2,
    "entity_kind": "robot",
    "entity_reference": "rh-osbs+internal_registry_proxy_3",
    "kind": "user",
    "user": "rh-osbs+internal_registry_proxy_3",
    "com.apostille.roots": {
      "rh-osbs/ubi9": "$disabled"
    },
    "com.apostille.root": "$disabled"
  }
}

Decoded TOKEN2 payload:

{
  "iss": "quay",
  "aud": "quay.io", <---------------------------- correct audience
  "nbf": 1652113800,
  "iat": 1652113800,
  "exp": 1652117400,
  "sub": "rh-osbs+internal_registry_proxy_3",
  "access": [
    {
      "type": "repository",
      "name": "rh-osbs/ubi9",
      "actions": [
        "pull"
      ]
    }
  ],
  "context": {
    "version": 2,
    "entity_kind": "robot",
    "entity_reference": "rh-osbs+internal_registry_proxy_3",
    "kind": "user",
    "user": "rh-osbs+internal_registry_proxy_3",
    "com.apostille.roots": {
      "rh-osbs/ubi9": "$disabled"
    },
    "com.apostille.root": "$disabled"
  }
}

With the above, I can assume that kaniko adds: &service=registry-proxy.domain.com
However, the chellange doesn't ask to do this.

Www-Authenticate: Bearer realm="https://registry-proxy.domain.com/v2/auth"

@imjasonh
Copy link
Collaborator Author

imjasonh commented May 9, 2022

That's super helpful! So that makes it seem like Kaniko is still adding the proxy's URL as the service, because the www-authenticate header doesn't provide one. That's unexpected if this PR is included (which I believe it is...)

Can you try running Kaniko with that commit-tagged image, with --verbosity=trace, to see what HTTP requests it's making? That would tell us whether the service is truly being added by go-containerregistry. There may be other ways to intercept/log HTTP requests as well, if Kaniko's --verbosity doesn't get you what you need.

@Andrei-Stepanov
Copy link

I will try to provide the info with --verbosity=trace asap.

@Andrei-Stepanov
Copy link

Hello @imjasonh , unfortunatelly it doesn't show the same verbosity as crane does,

$ /kaniko/executor --verbosity=trace --skip-tls-verify --insecure-pull --skip-tls-verify-pull --context $CI_PROJECT_DIR --dockerfile $CI_PROJECT_DIR/Dockerfile --destination ${CI_REGISTRY}/${CI_REGISTRY_REPO}:ubi9-$CI_COMMIT_SHORT_SHA
DEBU[0000] Copying file /builds/osci/images/universal_base_image/Dockerfile to /kaniko/Dockerfile 
TRAC[0000] Adding /var/run to default ignore list       
DEBU[0000] Skip resolving path /kaniko/Dockerfile       
DEBU[0000] Skip resolving path /builds/osci/images/universal_base_image 
DEBU[0000] Skip resolving path /cache                   
DEBU[0000] Skip resolving path                          
DEBU[0000] Skip resolving path                          
DEBU[0000] Skip resolving path                          
DEBU[0000] Skip resolving path                          
DEBU[0000] Built stage name to index map: map[]         
INFO[0000] Using dockerignore file: /builds/osci/images/universal_base_image/.dockerignore 
INFO[0000] Retrieving image manifest registry-proxy.domain.com/rh-osbs/ubi9:latest 
INFO[0000] Retrieving image registry-proxy.domain.com/rh-osbs/ubi9:latest from registry registry-proxy.domain.com 
error building image: GET https://registry-proxy.domain.com/v2/rh-osbs/ubi9/manifests/latest: unexpected status code 401 Unauthorized: {"error": "Invalid audience"}
 
Cleaning up project directory and file based variables  
  00:00
 
ERROR: Job failed: exit code 1

@sturivny
Copy link

@imjasonh Hello, any updates on this?
Thank you.

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

Successfully merging this pull request may close these issues.

ggcr: bearer transport for OAuth 2.0: sets always: service
5 participants