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

models, containerd, ecs-agent, host-ctr: support registry credentials #1955

Merged
merged 6 commits into from
Mar 7, 2022
Merged

models, containerd, ecs-agent, host-ctr: support registry credentials #1955

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
a185b30
models, containerd, ecs-agent: add 'container-registry.credentials'
etungsten Feb 15, 2022
068bb47
host-ctr: move registryHosts to the registry pkg
etungsten Feb 15, 2022
3d7b770
host-ctr: configure resolver's registry auth based on registry config
etungsten Feb 15, 2022
b70f945
ecs-agent, host-ctr, containerd: add mount units for configuration di…
etungsten Feb 22, 2022
20a7ddd
README: add info about "settings.container-registry.credentials"
etungsten Mar 1, 2022
6f59a59
migrations: add migrations for 'settings.container-registry.credentials'
etungsten Mar 1, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -453,6 +453,30 @@ When pulling an image from a registry, the container runtime will try the endpoi

For [host-container](#host-containers-settings) and [bootstrap-container](#bootstrap-containers-settings) images from Amazon ECR private repositories, registry mirrors are currently unsupported.

The following setting is optional and allows you to configure image registry credentials.
* `settings.container-registry.credentials`: An array of container images registry credential settings. Each element specifies the registry and the credential information for said registry.
The credential fields map to [containerd's registry credential fields](/~https://github.com/containerd/containerd/blob/v1.6.0/docs/cri/registry.md#configure-registry-credentials), which in turn map to the fields in `.docker/config.json`.
It is recommended to programmatically set these settings via `apiclient` through the Bottlerocket control container and/or custom host-containers.
* An example `apiclient` call to set registry credentials for `gcr.io` and `docker.io` looks like this:
```bash
apiclient set --json '{
"container-registry": {
"credentials": [
{
"registry": "gcr.io",
"username": "example_username",
"password": "example_password"
},
{
"registry": "docker.io",
"auth": "example_base64_encoded_auth_string"
}
]
}
}'
```
In addition to the container runtime daemons, these credential settings will also apply to [host-container](#host-containers-settings) and [bootstrap-container](#bootstrap-containers-settings) image pulls as well.

#### Updates settings

* `settings.updates.metadata-base-url`: The common portion of all URIs used to download update metadata.
Expand Down
2 changes: 2 additions & 0 deletions Release.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,4 +103,6 @@ version = "1.6.1"
"(1.6.0, 1.6.1)" = []
"(1.6.1, 1.6.2)" = [
"migrate_v1.6.2_add-cfsignal.lz4",
"migrate_v1.6.2_container-registry-credentials.lz4",
"migrate_v1.6.2_container-registry-credentials-metadata.lz4",
]
22 changes: 22 additions & 0 deletions packages/containerd/containerd-config-toml_k8s
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,3 +37,25 @@ conf_dir = "/etc/cni/net.d"
endpoint = [{{join_array ", " endpoint }}]
{{/each}}
{{/if}}

{{#if settings.container-registry.credentials}}
{{#each settings.container-registry.credentials}}
{{#if (eq registry "docker.io" )}}
[plugins."io.containerd.grpc.v1.cri".registry.configs."registry-1.docker.io".auth]
{{else}}
[plugins."io.containerd.grpc.v1.cri".registry.configs."{{registry}}".auth]
{{/if}}
Comment on lines +43 to +47
Copy link
Contributor

Choose a reason for hiding this comment

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

There's a potential edge case if someone specifies docker.io and registry-1.docker.io, where we'd end up with two entries. Not sure if that's worth handling though, especially if it doesn't cause an error in the client. I'd be OK with undefined or non-deterministic behavior.

Copy link
Contributor Author

@etungsten etungsten Mar 4, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Good call out. I tested this and containerd does complain about duplicate entries and then refuse to start:

Mar 04 00:27:32 containerd[124031]: containerd: failed to load TOML: /etc/containerd/config.toml: (37, 2): duplicated tables
Mar 04 00:27:32 systemd[1]: containerd.service: Main process exited, code=exited, status=1/FAILURE
Mar 04 00:27:32 systemd[1]: containerd.service: Failed with result 'exit-code'.
Mar 04 00:27:32 systemd[1]: Failed to start containerd container runtime.

But I kinda see setting both docker.io and registry-1.docker.io akin to setting the same registry twice which would also make containerd fail.

We do not enforce uniqueness of the registries in the container-registry.credentials settings model. We would essentially need to wrap Vec<RegistryCredential> into its own settings model type that would special case docker.io and enforce uniqueness of the registry field. Lemme know what you think!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Alternatively, I can remove this conditional and we can just make users specify registry-1.docker.io when they want to set up credentials for dockerhub. And for ECS variants, https://index.docker.io/v1/.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm good with your current approach, after further thought and based on our discussion out of band.

Without the conditional, on aws-ecs-1 to override registry credentials for Docker Hub, you'd need to specify both so that host-ctr and ecs-agent both get the form they expect (and ignore the other form). That's awkward and confusing enough that it's worth the extra bit implementation effort to paper over it.

{{#if username}}
username = "{{{username}}}"
{{/if}}
{{#if password}}
password = "{{{password}}}"
{{/if}}
{{#if auth}}
auth = "{{{auth}}}"
{{/if}}
{{#if identitytoken}}
identitytoken = "{{{identitytoken}}}"
{{/if}}
{{/each}}
{{/if}}
22 changes: 22 additions & 0 deletions packages/containerd/containerd-config-toml_k8s_nvidia
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,3 +37,25 @@ conf_dir = "/etc/cni/net.d"
endpoint = [{{join_array ", " endpoint }}]
{{/each}}
{{/if}}

{{#if settings.container-registry.credentials}}
{{#each settings.container-registry.credentials}}
{{#if (eq registry "docker.io" )~}}
[plugins."io.containerd.grpc.v1.cri".registry.configs."registry-1.docker.io".auth]
{{else}}
[plugins."io.containerd.grpc.v1.cri".registry.configs."{{registry}}".auth]
{{/if}}
{{#if username}}
username = "{{{username}}}"
{{/if}}
{{#if password}}
password = "{{{password}}}"
{{/if}}
{{#if auth}}
auth = "{{{auth}}}"
{{/if}}
{{#if identitytoken}}
identitytoken = "{{{identitytoken}}}"
{{/if}}
{{/each}}
{{/if}}
7 changes: 6 additions & 1 deletion packages/containerd/containerd.spec
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,10 @@ Source2: containerd-config-toml_k8s
Source3: containerd-config-toml_basic
Source4: containerd-config-toml_k8s_nvidia
Source5: containerd-tmpfiles.conf

# Mount for writing containerd configuration
Source100: etc-containerd.mount

Source1000: clarify.toml

# TODO: submit this upstream, including a unit test.
Expand Down Expand Up @@ -72,7 +76,7 @@ do
done

install -d %{buildroot}%{_cross_unitdir}
install -p -m 0644 %{S:1} %{buildroot}%{_cross_unitdir}/containerd.service
install -p -m 0644 %{S:1} %{S:100} %{buildroot}%{_cross_unitdir}

install -d %{buildroot}%{_cross_templatedir}
install -d %{buildroot}%{_cross_factorydir}%{_cross_sysconfdir}/containerd
Expand All @@ -93,6 +97,7 @@ install -p -m 0644 %{S:5} %{buildroot}%{_cross_tmpfilesdir}/containerd.conf
%{_cross_bindir}/containerd-shim-runc-v2
%{_cross_bindir}/ctr
%{_cross_unitdir}/containerd.service
%{_cross_unitdir}/etc-containerd.mount
%dir %{_cross_factorydir}%{_cross_sysconfdir}/containerd
%{_cross_templatedir}/containerd-config-toml*
%{_cross_tmpfilesdir}/containerd.conf
Expand Down
16 changes: 16 additions & 0 deletions packages/containerd/etc-containerd.mount
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
[Unit]
Description=Containerd Configuration Directory (/etc/containerd)
DefaultDependencies=no
Conflicts=umount.target
Before=local-fs.target umount.target
After=selinux-policy-files.service
Wants=selinux-policy-files.service

[Mount]
What=tmpfs
Where=/etc/containerd
Type=tmpfs
Options=nosuid,nodev,noexec,noatime,context=system_u:object_r:secret_t:s0

[Install]
WantedBy=preconfigured.target
8 changes: 7 additions & 1 deletion packages/ecs-agent/ecs-agent.spec
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,9 @@ Source108: pause-repositories
# Bottlerocket-specific - version data can be set with linker options
Source109: version.go

# Mount for writing ECS agent configuration
Source200: etc-ecs.mount

# Patches are numbered according to which source they apply to
# Patches 0000 - 0999 apply to Source0
# Patches 1000 - 1999 apply to Source1
Expand Down Expand Up @@ -241,7 +244,9 @@ install -D -p -m 0755 %{ecscni_gorepo}-%{ecscni_gitrev}/ecs-eni %{buildroot}%{_c
install -D -p -m 0755 %{ecscni_gorepo}-%{ecscni_gitrev}/ecs-ipam %{buildroot}%{_cross_libexecdir}/amazon-ecs-agent/ecs-ipam
install -D -p -m 0755 %{vpccni_gorepo}-%{vpccni_gitrev}/vpc-branch-eni %{buildroot}%{_cross_libexecdir}/amazon-ecs-agent/vpc-branch-eni

install -D -p -m 0644 %{S:101} %{buildroot}%{_cross_unitdir}/ecs.service
install -d %{buildroot}%{_cross_unitdir}
install -D -p -m 0644 %{S:101} %{S:200} %{buildroot}%{_cross_unitdir}

install -D -p -m 0644 %{S:102} %{buildroot}%{_cross_tmpfilesdir}/ecs.conf
install -D -p -m 0644 %{S:103} %{buildroot}%{_cross_sysctldir}/90-ecs.conf
install -D -p -m 0644 %{S:104} %{buildroot}%{_cross_templatedir}/ecs.config
Expand Down Expand Up @@ -288,6 +293,7 @@ mv %{vpccni_gorepo}-%{vpccni_gitrev}/vendor go-vendor/%{vpccni_gorepo}
%{_cross_libexecdir}/amazon-ecs-agent/ecs-ipam
%{_cross_libexecdir}/amazon-ecs-agent/vpc-branch-eni
%{_cross_unitdir}/ecs.service
%{_cross_unitdir}/etc-ecs.mount
%{_cross_tmpfilesdir}/ecs.conf
%{_cross_sysctldir}/90-ecs.conf
%{_cross_templatedir}/ecs.config
Expand Down
16 changes: 16 additions & 0 deletions packages/ecs-agent/ecs.config
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,2 +1,18 @@
ECS_LOGFILE=/var/log/ecs/ecs-agent.log
ECS_LOGLEVEL="{{settings.ecs.loglevel}}"
{{#if settings.container-registry.credentials~}}
ECS_ENGINE_AUTH_TYPE=dockercfg
ECS_ENGINE_AUTH_DATA='{
{{~#each settings.container-registry.credentials~}}
{{~#unless @first~}},{{~/unless~}}
{{~#if (eq registry "docker.io" )~}}
"https://index.docker.io/v1/":
{{~else~}}
"{{registry}}":
{{~/if~}}
{"email": "."
{{~#if auth~}},"auth": "{{{auth}}}"{{/if}}
{{~#if username~}},"username": "{{{username}}}"{{/if}}
{{~#if password~}},"password": "{{{password}}}"}{{/if}}
{{~/each~}}}}'
{{/if}}
16 changes: 16 additions & 0 deletions packages/ecs-agent/etc-ecs.mount
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
[Unit]
Description=ECS agent Configuration Directory (/etc/ecs)
DefaultDependencies=no
Conflicts=umount.target
Before=local-fs.target umount.target
After=selinux-policy-files.service
Wants=selinux-policy-files.service

[Mount]
What=tmpfs
Where=/etc/ecs
Type=tmpfs
Options=nosuid,nodev,noexec,noatime,context=system_u:object_r:secret_t:s0

[Install]
WantedBy=preconfigured.target
5 changes: 5 additions & 0 deletions packages/host-ctr/clarify.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
[clarify."sigs.k8s.io/yaml"]
expression = "MIT AND BSD-3-Clause"
license-files = [
{ path = "LICENSE", hash = 0xcdf3ae00 },
]
16 changes: 16 additions & 0 deletions packages/host-ctr/etc-host-containers.mount.in
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
[Unit]
Description=Host containers Configuration Directory (/etc/host-containers)
DefaultDependencies=no
Conflicts=umount.target
Before=local-fs.target umount.target
After=selinux-policy-files.service
Wants=selinux-policy-files.service

[Mount]
What=tmpfs
Where=/etc/host-containers
Type=tmpfs
Options=nosuid,nodev,noexec,noatime,context=system_u:object_r:secret_t:s0

[Install]
WantedBy=preconfigured.target
10 changes: 9 additions & 1 deletion packages/host-ctr/host-ctr.spec
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,11 @@ Source10: host-containerd.service
Source11: host-containerd-tmpfiles.conf
Source12: host-containerd-config.toml

# Mount for writing host-ctr configuration
Source100: etc-host-containers.mount.in

Source1000: clarify.toml

%description
%{summary}.

Expand All @@ -30,19 +35,22 @@ install -p -m 0755 host-ctr %{buildroot}%{_cross_bindir}

install -d %{buildroot}%{_cross_unitdir}
install -p -m 0644 %{S:10} %{buildroot}%{_cross_unitdir}
ETC_HOST_CONTAINERS=$(systemd-escape --path /etc/host-containers)
install -p -m 0644 %{S:100} %{buildroot}%{_cross_unitdir}/${ETC_HOST_CONTAINERS}.mount

install -d %{buildroot}%{_cross_tmpfilesdir}
install -p -m 0644 %{S:11} %{buildroot}%{_cross_tmpfilesdir}/host-containerd.conf

install -d %{buildroot}%{_cross_factorydir}%{_cross_sysconfdir}/host-containerd
install -p -m 0644 %{S:12} %{buildroot}%{_cross_factorydir}%{_cross_sysconfdir}/host-containerd/config.toml

%cross_scan_attribution go-vendor vendor
%cross_scan_attribution --clarify %{S:1000} go-vendor vendor

%files
%{_cross_attribution_vendor_dir}
%{_cross_bindir}/host-ctr
%{_cross_unitdir}/host-containerd.service
%{_cross_unitdir}/*.mount
%{_cross_tmpfilesdir}/host-containerd.conf
%{_cross_factorydir}%{_cross_sysconfdir}/host-containerd/config.toml

Expand Down
22 changes: 22 additions & 0 deletions packages/os/host-ctr-toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,25 @@
endpoints = [{{join_array ", " endpoint }}]
{{/each}}
{{/if}}

{{#if settings.container-registry.credentials}}
{{#each settings.container-registry.credentials}}
{{#if (eq registry "docker.io" )~}}
[creds."registry-1.docker.io"]
{{else}}
[creds."{{registry}}"]
{{/if}}
{{#if username}}
username = "{{{username}}}"
{{/if}}
{{#if password}}
password = "{{{password}}}"
{{/if}}
{{#if auth}}
auth = "{{{auth}}}"
{{/if}}
{{#if identitytoken}}
identitytoken = "{{{identitytoken}}}"
{{/if}}
{{/each}}
{{/if}}
14 changes: 14 additions & 0 deletions sources/Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions sources/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@ members = [
"api/migration/migrations/v1.6.0/public-admin-container-v0-7-4",
"api/migration/migrations/v1.6.0/public-control-container-v0-5-5",
"api/migration/migrations/v1.6.2/add-cfsignal",
"api/migration/migrations/v1.6.2/container-registry-credentials",
"api/migration/migrations/v1.6.2/container-registry-credentials-metadata",

"bottlerocket-release",

Expand Down
10 changes: 10 additions & 0 deletions sources/api/migration/migrations/v1.6.2/container-registry-credentials-metadata/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
[package]
name = "container-registry-credentials-metadata"
version = "0.1.0"
authors = ["Erikson Tung <etung@amazon.com>"]
license = "Apache-2.0 OR MIT"
edition = "2018"
publish = false

[dependencies]
migration-helpers = { path = "../../../migration-helpers", version = "0.1.0"}
31 changes: 31 additions & 0 deletions sources/api/migration/migrations/v1.6.2/container-registry-credentials-metadata/src/main.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
#![deny(rust_2018_idioms)]

use migration_helpers::common_migrations::{AddMetadataMigration, SettingMetadata};
use migration_helpers::{migrate, Result};
use std::process;

/// We added a new setting and `affected-services` metadata for `container-registry.credentials`
/// We subdivided metadata for `container-registry` into `container-registry.mirrors` and `container-registry.credentials`
/// This is for the docker variants where don't want to restart the docker daemon when credentials settings change.
fn run() -> Result<()> {
migrate(AddMetadataMigration(&[
SettingMetadata {
metadata: &["affected-services"],
setting: "settings.container-registry.credentials",
},
SettingMetadata {
metadata: &["affected-services"],
setting: "settings.container-registry.mirrors",
},
]))
}

// Returning a Result from main makes it print a Debug representation of the error, but with Snafu
// we have nice Display representations of the error, so we wrap "main" (run) and print any error.
// /~https://github.com/shepmaster/snafu/issues/110
fn main() {
if let Err(e) = run() {
eprintln!("{}", e);
process::exit(1);
}
}
10 changes: 10 additions & 0 deletions sources/api/migration/migrations/v1.6.2/container-registry-credentials/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
[package]
name = "container-registry-credentials"
version = "0.1.0"
authors = ["Erikson Tung <etung@amazon.com>"]
license = "Apache-2.0 OR MIT"
edition = "2018"
publish = false

[dependencies]
migration-helpers = { path = "../../../migration-helpers", version = "0.1.0"}
22 changes: 22 additions & 0 deletions sources/api/migration/migrations/v1.6.2/container-registry-credentials/src/main.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
#![deny(rust_2018_idioms)]

use migration_helpers::common_migrations::AddPrefixesMigration;
use migration_helpers::{migrate, Result};
use std::process;

/// We added a new setting for configuring image credentials, `settings.container-registry.credentials`
fn run() -> Result<()> {
migrate(AddPrefixesMigration(vec![
"settings.container-registry.credentials",
]))
}

// Returning a Result from main makes it print a Debug representation of the error, but with Snafu
// we have nice Display representations of the error, so we wrap "main" (run) and print any error.
// /~https://github.com/shepmaster/snafu/issues/110
fn main() {
if let Err(e) = run() {
eprintln!("{}", e);
process::exit(1);
}
}
Loading