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.

Sign up for GitHub

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

Jump to bottom

vendor: github.com/docker/distribution v2.8.3 #46542

Merged
merged 1 commit into from
Oct 12, 2023
Merged

vendor: github.com/docker/distribution v2.8.3 #46542

merged 1 commit into from
Oct 12, 2023

Conversation

thaJeztah
Copy link
Member

@thaJeztah thaJeztah commented Sep 26, 2023 •
edited
Loading

opening as draft, as it's not tagged / released yet

  • Fix storageDriver gcs not registered in binaries
  • reference: replace uses of deprecated function SplitHostname
  • Dont parse errors as JSON unless Content-Type is set to JSON
  • update to go1.20.8
  • Set Content-Type header in registry client ReadFrom
  • deprecate reference package, migrate to github.com/distribution/reference
  • digestset: deprecate package in favor of go-digest/digestset
  • Do not close HTTP request body in HTTP handler

full diff: distribution/distribution@v2.8.2...v2.8.3

- What I did

- How I did it

- How to verify it

- Description for the changelog

- A picture of a cute animal (not mandatory but encouraged)

@thaJeztah thaJeztah added this to the 25.0.0 milestone Sep 26, 2023
@thaJeztah thaJeztah force-pushed the update_distribution_2.8.3 branch from 151116c to 55bbdd6 Compare September 26, 2023 17:50
@thaJeztah
Copy link
Member Author

Wondering if this could be a legit failure

=== FAIL: github.com/docker/docker/distribution TestPullSchema2Config/unauthorized (0.00s)
    pull_v2_test.go:282: HTTP GET /v2/
    pull_v2_test.go:282: HTTP GET /v2/docker.io/library/testremotename/blobs/sha256:66ad98165d38f53ee73868f82bd4eed60556ddfee824810a4062c4f777b20a5b
    pull_v2_test.go:312: expected error="download failed after attempts=1: unauthorized: " to contain "unauthorized: authentication required"

@thaJeztah
Copy link
Member Author

Test looks to be failing indeed;

=== FAIL: github.com/docker/docker/distribution TestPullSchema2Config/unauthorized (0.00s)
    pull_v2_test.go:282: HTTP GET /v2/
    pull_v2_test.go:282: HTTP GET /v2/docker.io/library/testremotename/blobs/sha256:66ad98165d38f53ee73868f82bd4eed60556ddfee824810a4062c4f777b20a5b
    pull_v2_test.go:312: expected error="download failed after attempts=1: unauthorized: " to contain "unauthorized: authentication required"

I wonder though where the authentication required part came from before, because the tests is just setting a status-header;

moby/distribution/pull_v2_test.go

Lines 268 to 273 in 605c8fb

name: "unauthorized",
handler: func(callCount int, w http.ResponseWriter) {
w.WriteHeader(http.StatusUnauthorized)
},
expectError: "unauthorized: authentication required",
expectAttempts: 1,

That would probably come from

moby/vendor/github.com/docker/distribution/registry/api/errcode/register.go

Lines 36 to 46 in 605c8fb

// ErrorCodeUnauthorized is returned if a request requires
// authentication.
ErrorCodeUnauthorized = Register("errcode", ErrorDescriptor{
Value: "UNAUTHORIZED",
Message: "authentication required",
Description: `The access controller was unable to authenticate
the client. Often this will be accompanied by a
Www-Authenticate HTTP response header indicating how to
authenticate.`,
HTTPStatusCode: http.StatusUnauthorized,
})
, but not sure how that got into the response in this test (perhaps client mapping?)

@thaJeztah
Copy link
Member Author

Fixed that one, but need to look at the integration-cli ones;

=== Failed
=== FAIL: amd64.integration-cli TestDockerRegistryAuthTokenSuite/TestPushMisconfiguredTokenServiceResponseError (5.17s)
    docker_cli_push_test.go:288: assertion failed: toomanyrequests: {"errors": [{"code":"TOOMANYREQUESTS","message":"out of tokens"}]} (split[len(split)-2] string) != toomanyrequests: out of tokens (string)
    check_test.go:559: [d8a98b9190c8a] daemon is not started
    --- FAIL: TestDockerRegistryAuthTokenSuite/TestPushMisconfiguredTokenServiceResponseError (5.17s)

=== FAIL: amd64.integration-cli TestDockerRegistryAuthTokenSuite/TestPushMisconfiguredTokenServiceResponseUnauthorized (0.16s)
    docker_cli_push_test.go:273: assertion failed: unauthorized: {"error": "unauthorized"} (split[len(split)-2] string) != unauthorized: authentication required (string)
    check_test.go:559: [df745bdd35c0a] daemon is not started
    --- FAIL: TestDockerRegistryAuthTokenSuite/TestPushMisconfiguredTokenServiceResponseUnauthorized (0.16s)

=== FAIL: amd64.integration-cli TestDockerRegistryAuthTokenSuite/TestPushMisconfiguredTokenServiceResponseUnparsable (0.16s)
    docker_cli_push_test.go:301: assertion failed: expression is false: strings.Contains(split[len(split)-2], "error parsing HTTP 403 response body: ")
    check_test.go:559: [d971a6903e6c6] daemon is not started
    --- FAIL: TestDockerRegistryAuthTokenSuite/TestPushMisconfiguredTokenServiceResponseUnparsable (0.16s)

=== FAIL: amd64.integration-cli TestDockerRegistryAuthTokenSuite/TestPushTokenServiceUnauthResponse (0.16s)
    docker_cli_push_test.go:260: assertion failed: expression is false: strings.Contains(out, "unauthorized: a message")
    check_test.go:559: [d78bb30ecd086] daemon is not started
    --- FAIL: TestDockerRegistryAuthTokenSuite/TestPushTokenServiceUnauthResponse (0.16s)

@thaJeztah thaJeztah mentioned this pull request Sep 27, 2023
Merged
@thaJeztah thaJeztah force-pushed the update_distribution_2.8.3 branch 4 times, most recently from 3f866cc to 65eded2 Compare October 2, 2023 18:29
@thaJeztah thaJeztah marked this pull request as ready for review October 2, 2023 18:30
@thaJeztah thaJeztah mentioned this pull request Oct 2, 2023
Merged
@thaJeztah thaJeztah force-pushed the update_distribution_2.8.3 branch from 65eded2 to aad1ee7 Compare October 10, 2023 07:31
@thaJeztah
vendor: github.com/docker/distribution v2.8.3
e1d0aac 
- Fix storageDriver gcs not registered in binaries
- reference: replace uses of deprecated function SplitHostname
- Dont parse errors as JSON unless Content-Type is set to JSON
- update to go1.20.8
- Set Content-Type header in registry client ReadFrom
- deprecate reference package, migrate to github.com/distribution/reference
- digestset: deprecate package in favor of go-digest/digestset
- Do not close HTTP request body in HTTP handler

full diff: distribution/distribution@v2.8.2...v2.8.3

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thaJeztah thaJeztah force-pushed the update_distribution_2.8.3 branch from aad1ee7 to e1d0aac Compare October 11, 2023 15:11
@thaJeztah
Copy link
Member Author

@rumpl @vvoland ptal :)

@vvoland vvoland merged commit c538935 into moby:master Oct 12, 2023
@thaJeztah thaJeztah deleted the update_distribution_2.8.3 branch October 12, 2023 10:37
@thaJeztah thaJeztah mentioned this pull request Oct 18, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vvoland vvoland vvoland approved these changes

@rumpl rumpl rumpl approved these changes

Assignees
No one assigned
Labels
area/distribution kind/bugfix PR's that fix bugs process/cherry-picked status/2-code-review
Projects
None yet
Milestone
25.0.0
Development

Successfully merging this pull request may close these issues.

Pushing blobs doesn't send content-type
3 participants
@thaJeztah @rumpl @vvoland