Skip to content
This repository has been archived by the owner on Mar 9, 2022. It is now read-only.

Add auth config #838

Merged
merged 3 commits into from
Jul 11, 2018
Merged

Add auth config #838

merged 3 commits into from
Jul 11, 2018

Conversation

Random-Liu
Copy link
Member

@Random-Liu Random-Liu commented Jul 9, 2018

Fixes #723.
Fixes #835.

/cc @dmcgowan for the resolver change.

This PR:

  1. Change cri plugin to use containerd docker resolver instead of our own fork. This gets rid of a lot of code duplication and maintenance overhead.
  2. Make default registry credential configurable in the config file.

I'll mark this 1.11 and cherrypick into 1.11. Containerd 1.2 is our real release, I think it is fine to add new features before that.

Copy link
Member

@mikebrow mikebrow left a comment

Choose a reason for hiding this comment

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

See comments to docs code looks good!

docs/registry.md Outdated
The meaning of each field is the same with the corresponding field in `.docker/config.json`.

Please note that auth config passed by CRI takes precedence. The registry
credential in the config file will only be used when auth config is not specified
Copy link
Member

Choose a reason for hiding this comment

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

/s/the config file/this config/

Copy link
Member Author

Choose a reason for hiding this comment

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

Done.

docs/registry.md Outdated
```
The meaning of each field is the same with the corresponding field in `.docker/config.json`.

Please note that auth config passed by CRI takes precedence. The registry
Copy link
Member

Choose a reason for hiding this comment

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

takes precedence over this config.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done.

docs/registry.md Outdated

Please note that auth config passed by CRI takes precedence. The registry
credential in the config file will only be used when auth config is not specified
by CRI.
Copy link
Member

Choose a reason for hiding this comment

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

/s/by CRI/by kubernetes via CRI/
Follow this link to read about configuring kubernetes pod to use images from private registries.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done.

Signed-off-by: Lantao Liu <lantaol@google.com>
Signed-off-by: Lantao Liu <lantaol@google.com>
Copy link
Member

@mikebrow mikebrow left a comment

Choose a reason for hiding this comment

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

/LGTM

Signed-off-by: Lantao Liu <lantaol@google.com>
@k8s-ci-robot
Copy link

New changes are detected. LGTM label has been removed.

@k8s-ci-robot k8s-ci-robot removed the lgtm label Jul 11, 2018
@Random-Liu
Copy link
Member Author

Just updated containerd version to use #835.

Re-apply LGTM.

Copy link
Member

@mikebrow mikebrow left a comment

Choose a reason for hiding this comment

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

/LGTM

@Random-Liu Random-Liu merged commit ca32566 into containerd:master Jul 11, 2018
@Random-Liu Random-Liu deleted the add-auth-config branch July 11, 2018 06:55
This was referenced Jul 11, 2018
Random-Liu added a commit that referenced this pull request Jul 11, 2018
Random-Liu added a commit that referenced this pull request Jul 11, 2018
Random-Liu added a commit that referenced this pull request Jul 11, 2018
Random-Liu added a commit that referenced this pull request Jul 11, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
4 participants