Skip to content

Fix deprecated registry auth conversion.#4987

Merged
estesp merged 1 commit intocontainerd:masterfrom
Random-Liu:fix-auth-config-conversion
Feb 4, 2021
Merged

Fix deprecated registry auth conversion.#4987
estesp merged 1 commit intocontainerd:masterfrom
Random-Liu:fix-auth-config-conversion

Conversation

@Random-Liu
Copy link
Copy Markdown
Member

@Random-Liu Random-Liu commented Feb 2, 2021

In the old field registry.auths in containerd 1.2, the field has the scheme. https://github.com/containerd/cri/blob/release/1.4/docs/registry.md#configure-registry-credentials

However, in the new field registry.configs introduced in containerd 1.3 and above, the field doesn't need the scheme any more. https://github.com/containerd/cri/blob/master/docs/registry.md#configure-registry-credentials

Although containerd 1.2 is End-Of-Life, the config option field is not removed yet, so we should do the conversion correctly.

This should probably be cherrypicked into 1.3 and 1.4.

@theopenlab-ci
Copy link
Copy Markdown

theopenlab-ci Bot commented Feb 2, 2021

Build succeeded.

@Random-Liu Random-Liu force-pushed the fix-auth-config-conversion branch from a11cbb2 to 19d3f11 Compare February 2, 2021 06:24
@theopenlab-ci
Copy link
Copy Markdown

theopenlab-ci Bot commented Feb 2, 2021

Build succeeded.

Comment thread pkg/cri/config/config.go Outdated
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: errors.Wrapf could be used here

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. Forgot about this. :P

@Random-Liu Random-Liu force-pushed the fix-auth-config-conversion branch from 19d3f11 to b5bf1fd Compare February 4, 2021 03:23
@theopenlab-ci
Copy link
Copy Markdown

theopenlab-ci Bot commented Feb 4, 2021

Build succeeded.

Copy link
Copy Markdown
Member

@estesp estesp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@estesp estesp merged commit ccde82d into containerd:master Feb 4, 2021
@Random-Liu Random-Liu deleted the fix-auth-config-conversion branch February 4, 2021 06:12
@thaJeztah
Copy link
Copy Markdown
Member

Looks like these have to be cherry-picked in the https://github.com/containerd/cri repository, and then vendored in

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cherry-picked/1.4.x PR commits are cherry picked into the release/1.4 branch

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants