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

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

Merged
merged 6 commits into from
Mar 7, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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
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
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

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
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
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
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
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
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
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
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
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
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
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

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
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
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 <[email protected]>"]
license = "Apache-2.0 OR MIT"
edition = "2018"
publish = false

[dependencies]
migration-helpers = { path = "../../../migration-helpers", version = "0.1.0"}
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);
}
}
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 <[email protected]>"]
license = "Apache-2.0 OR MIT"
edition = "2018"
publish = false

[dependencies]
migration-helpers = { path = "../../../migration-helpers", version = "0.1.0"}
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