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

settings.network: add new proxy settings #1204

Merged
merged 2 commits into from
Jan 5, 2021
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
21 changes: 21 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -344,6 +344,27 @@ These settings can be changed at any time.
* `settings.updates.version-lock`: Controls the version that will be selected when you issue an update request. Can be locked to a specific version like `v1.0.0`, or `latest` to take the latest available version. Defaults to `latest`.
* `settings.updates.ignore-waves`: Updates are rolled out in waves to reduce the impact of issues. For testing purposes, you can set this to `true` to ignore those waves and update immediately.

#### Network settings
tjkirch marked this conversation as resolved.
Show resolved Hide resolved

##### Proxy settings

These settings will configure the proxying behavior of the following services:
* For all variants:
* [containerd.service](packages/containerd/containerd.service)
* [host-containerd.service](packages/host-ctr/host-containerd.service)
* For Kubernetes variants:
* [kubelet.service](packages/kubernetes-1.18/kubelet.service)
* For the ECS variant:
* [docker.service](packages/docker-engine/docker.service)
* [ecs.service](packages/ecs-agent/ecs.service)

* `settings.network.https-proxy`: The HTTPS proxy server to be used by services listed above.
* `settings.network.no-proxy`: A list of hosts that are excluded from proxying.
tjkirch marked this conversation as resolved.
Show resolved Hide resolved

The no-proxy list will automatically include entries for localhost.

If you're running a Kubernetes variant, the no-proxy list will automatically include the Kubernetes API server endpoint and other commonly used Kubernetes DNS suffixes to facilitate intra-cluster networking.

#### Time settings

* `settings.ntp.time-servers`: A list of NTP servers used to set and verify the system time.
Expand Down
1 change: 1 addition & 0 deletions packages/containerd/containerd.service
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ After=network-online.target configured.target
Wants=network-online.target configured.target

[Service]
EnvironmentFile=/etc/network/proxy.env
ExecStart=/usr/bin/containerd
Type=notify
Delegate=yes
Expand Down
1 change: 1 addition & 0 deletions packages/docker-engine/docker.service
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ StartLimitIntervalSec=60s

[Service]
Type=notify
EnvironmentFile=/etc/network/proxy.env
Copy link
Contributor

Choose a reason for hiding this comment

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

I just realized that if docker is now depending on settings, we need to restart it on changes, i.e. make a docker service in the model.

I guess the service has to be defined separately in the aws-dev and aws-ecs-1 defaults-overrides, since we probably shouldn't have it in the defaults..?

It should probably be restarted before ecs, in the ecs model, though, and we don't have dependencies between services. Maybe in the ecs model, docker is just a restart-command in the ecs service, rather than being its own?

Copy link
Contributor

Choose a reason for hiding this comment

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

It should probably be restarted before ecs, in the ecs model, though, and we don't have dependencies between services.

There's maybe a race while you're configuring/restarting, but it shouldn't really matter? The ECS agent should tolerate Docker going down and coming back up already. There's also a dependency listed in the unit itself, though I know that our model isn't reading that.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ugh, this also applies to containerd, host-containerd, and kubelet, I believe. We haven't dynamically affected the configuration of {host-,}containerd before. kubelet we technically could, and it seems like it wouldn't have worked without a reboot, but they're mostly dynamic settings generated at boot that you wouldn't change. (Maybe labels/taints are the worst offender?)

Copy link
Contributor

Choose a reason for hiding this comment

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

@samuelkarp would containerd be similarly safe to restart? No lost containers, logs, etc.?

Copy link
Contributor

Choose a reason for hiding this comment

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

@bcressey found that restarting host-containerd kills SSH sessions, implying that the admin container is stopped. Not sure if it's a necessary kill, or if our setup is wrong.

Copy link
Contributor

Choose a reason for hiding this comment

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

would containerd be similarly safe to restart? No lost containers, logs, etc.?

Containerd when used either directly through its API or with Docker does no networking and wouldn't need to be restarted.

Containerd when used with Kubernetes (through cri-containerd) does networking to pull images, and would need to be restarted.

By default, containerd will not stop containers when it exits, but systemd might be killing them.

ExecStart=/usr/bin/dockerd -H fd:// --containerd=/run/containerd/containerd.sock
ExecReload=/bin/kill -s HUP $MAINPID
Delegate=yes
Expand Down
1 change: 1 addition & 0 deletions packages/ecs-agent/ecs.service
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ Restart=on-failure
RestartPreventExitStatus=5
RestartSec=1s
EnvironmentFile=-/etc/ecs/ecs.config
EnvironmentFile=/etc/network/proxy.env
Environment=ECS_CHECKPOINT=true
ExecStartPre=/sbin/iptables -t nat -A PREROUTING -d 169.254.170.2/32 \
-p tcp -m tcp --dport 80 -j DNAT --to-destination 127.0.0.1:51679
Expand Down
1 change: 1 addition & 0 deletions packages/host-ctr/host-containerd.service
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ After=network-online.target configured.target
Wants=network-online.target configured.target

[Service]
EnvironmentFile=/etc/network/proxy.env
ExecStart=/usr/bin/containerd --config /etc/host-containerd/config.toml
Type=notify
Delegate=yes
Expand Down
1 change: 1 addition & 0 deletions packages/kubernetes-1.15/kubelet.service
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ BindsTo=containerd.service

[Service]
Type=notify
EnvironmentFile=/etc/network/proxy.env
EnvironmentFile=/etc/kubernetes/kubelet/env
ExecStartPre=/sbin/iptables -P FORWARD ACCEPT
# Pull the pause container image before starting `kubelet` so `containerd/cri` wouldn't have to
Expand Down
1 change: 1 addition & 0 deletions packages/kubernetes-1.16/kubelet.service
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ BindsTo=containerd.service

[Service]
Type=notify
EnvironmentFile=/etc/network/proxy.env
EnvironmentFile=/etc/kubernetes/kubelet/env
ExecStartPre=/sbin/iptables -P FORWARD ACCEPT
# Pull the pause container image before starting `kubelet` so `containerd/cri` wouldn't have to
Expand Down
1 change: 1 addition & 0 deletions packages/kubernetes-1.17/kubelet.service
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ BindsTo=containerd.service

[Service]
Type=notify
EnvironmentFile=/etc/network/proxy.env
EnvironmentFile=/etc/kubernetes/kubelet/env
ExecStartPre=/sbin/iptables -P FORWARD ACCEPT
# Pull the pause container image before starting `kubelet` so `containerd/cri` wouldn't have to
Expand Down
1 change: 1 addition & 0 deletions packages/kubernetes-1.18/kubelet.service
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ BindsTo=containerd.service

[Service]
Type=notify
EnvironmentFile=/etc/network/proxy.env
EnvironmentFile=/etc/kubernetes/kubelet/env
ExecStartPre=/sbin/iptables -P FORWARD ACCEPT
# Pull the pause container image before starting `kubelet` so `containerd/cri` wouldn't have to
Expand Down
6 changes: 6 additions & 0 deletions packages/release/proxy-env
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{{#if settings.network.https-proxy}}
HTTPS_PROXY={{settings.network.https-proxy}}
https_proxy={{settings.network.https-proxy}}
{{/if}}
NO_PROXY={{#each settings.network.no-proxy}}{{this}},{{/each}}localhost,127.0.0.1{{#if settings.kubernetes.api-server}},{{host settings.kubernetes.api-server}}{{/if}}{{#if settings.kubernetes.cluster-domain}},.{{settings.kubernetes.cluster-domain}}{{/if}}
no_proxy={{#each settings.network.no-proxy}}{{this}},{{/each}}localhost,127.0.0.1{{#if settings.kubernetes.api-server}},{{host settings.kubernetes.api-server}}{{/if}}{{#if settings.kubernetes.cluster-domain}},.{{settings.kubernetes.cluster-domain}}{{/if}}
3 changes: 3 additions & 0 deletions packages/release/release.spec
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ Source98: release-systemd-system.conf
Source99: release-tmpfiles.conf

Source200: motd.template
Source201: proxy-env

Source1000: eth0.xml
Source1002: configured.target
Expand Down Expand Up @@ -122,6 +123,7 @@ install -p -m 0644 ${LICENSEPATH}.mount %{buildroot}%{_cross_unitdir}

install -d %{buildroot}%{_cross_templatedir}
install -p -m 0644 %{S:200} %{buildroot}%{_cross_templatedir}/motd
install -p -m 0644 %{S:201} %{buildroot}%{_cross_templatedir}/proxy-env

%files
%{_cross_factorydir}%{_cross_sysconfdir}/hosts
Expand All @@ -142,5 +144,6 @@ install -p -m 0644 %{S:200} %{buildroot}%{_cross_templatedir}/motd
%{_cross_unitdir}/var-lib-bottlerocket.mount
%dir %{_cross_templatedir}
%{_cross_templatedir}/motd
%{_cross_templatedir}/proxy-env

%changelog
16 changes: 14 additions & 2 deletions sources/Cargo.lock

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

1 change: 1 addition & 0 deletions sources/api/schnauzer/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ serde = { version = "1.0", features = ["derive"] }
serde_json = "1"
snafu = "0.6"
tokio = { version = "0.2", default-features = false, features = ["macros", "rt-threaded"] }
url = "2.1"
# When hyper updates to tokio 0.3:
#tokio = { version = "0.3", default-features = false, features = ["macros", "rt-multi-thread"] }

Expand Down
109 changes: 109 additions & 0 deletions sources/api/schnauzer/src/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ use serde_json::value::Value;
use snafu::{OptionExt, ResultExt};
use std::borrow::Borrow;
use std::collections::HashMap;
use url::Url;

lazy_static! {
/// A map to tell us which registry to pull ECR images from for a given region.
Expand Down Expand Up @@ -132,6 +133,21 @@ mod error {
template: String,
source: std::io::Error,
},

#[snafu(display(
"Expected an absolute URL, got '{}' in template '{}': '{}'",
tjkirch marked this conversation as resolved.
Show resolved Hide resolved
url_str,
template,
source
))]
UrlParse {
url_str: String,
template: String,
source: url::ParseError,
},

#[snafu(display("URL '{}' is missing host component", url_str))]
UrlHost { url_str: String },
}

// Handlebars helpers are required to return a RenderError.
Expand Down Expand Up @@ -463,6 +479,40 @@ pub fn ecr_prefix(
Ok(())
}

/// `host` takes an absolute URL and trims it down and returns its host.
pub fn host(
helper: &Helper<'_, '_>,
_: &Handlebars,
_: &Context,
renderctx: &mut RenderContext<'_, '_>,
out: &mut dyn Output,
) -> Result<(), RenderError> {
trace!("Starting host helper");
let template_name = template_name(renderctx);
check_param_count(helper, template_name, 1)?;

let url_val = get_param(helper, 0)?;
let url_str = url_val
.as_str()
.with_context(|| error::InvalidTemplateValue {
expected: "string",
value: url_val.to_owned(),
template: template_name.to_owned(),
})?;
let url = Url::parse(url_str).context(error::UrlParse {
url_str,
template: template_name,
})?;
let url_host = url.host_str().context(error::UrlHost { url_str })?;

// write it to the template
out.write(&url_host).with_context(|| error::TemplateWrite {
template: template_name.to_owned(),
})?;

Ok(())
}

// =^..^= =^..^= =^..^= =^..^= =^..^= =^..^= =^..^= =^..^= =^..^= =^..^= =^..^=
// helpers to the helpers

Expand Down Expand Up @@ -823,3 +873,62 @@ mod test_ecr_registry {
assert_eq!(result, EXPECTED_URL_XY_ZTOWN_1);
}
}

#[cfg(test)]
mod test_host {
use super::*;
use handlebars::TemplateRenderError;
use serde::Serialize;
use serde_json::json;

// A thin wrapper around the handlebars render_template method that includes
// setup and registration of helpers
fn setup_and_render_template<T>(tmpl: &str, data: &T) -> Result<String, TemplateRenderError>
where
T: Serialize,
{
let mut registry = Handlebars::new();
registry.register_helper("host", Box::new(host));

registry.render_template(tmpl, data)
}

#[test]
fn not_absolute_url() {
tjkirch marked this conversation as resolved.
Show resolved Hide resolved
assert!(setup_and_render_template(
"{{ host url_setting }}",
&json!({"url_setting": "example.com"}),
)
.is_err());
}

#[test]
fn https() {
let result = setup_and_render_template(
"{{ host url_setting }}",
&json!({"url_setting": "https://example.example.com/example/example"}),
)
.unwrap();
assert_eq!(result, "example.example.com");
}

#[test]
fn http() {
let result = setup_and_render_template(
"{{ host url_setting }}",
&json!({"url_setting": "http://example.com"}),
)
.unwrap();
assert_eq!(result, "example.com");
}

#[test]
fn unknown_scheme() {
let result = setup_and_render_template(
"{{ host url_setting }}",
&json!({"url_setting": "foo://example.com"}),
)
.unwrap();
assert_eq!(result, "example.com");
}
}
1 change: 1 addition & 0 deletions sources/api/schnauzer/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,7 @@ pub fn build_template_registry() -> Result<handlebars::Handlebars<'static>> {
template_registry.register_helper("join_map", Box::new(helpers::join_map));
template_registry.register_helper("default", Box::new(helpers::default));
template_registry.register_helper("ecr-prefix", Box::new(helpers::ecr_prefix));
template_registry.register_helper("host", Box::new(helpers::host));

Ok(template_registry)
}
13 changes: 11 additions & 2 deletions sources/models/defaults.toml
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ template-path = "/usr/share/templates/motd"
# Container runtime.

[services.containerd]
configuration-files = ["containerd-config-toml"]
configuration-files = ["containerd-config-toml", "proxy-env"]
restart-commands = ["/bin/systemctl try-restart containerd.service"]

[configuration-files.containerd-config-toml]
Expand All @@ -32,7 +32,7 @@ template-path = "/usr/share/templates/containerd-config-toml"
# Host-container runtime

[services.host-containerd]
configuration-files = []
configuration-files = ["proxy-env"]
restart-commands = ["/bin/systemctl try-restart host-containerd.service"]

# Updates.
Expand Down Expand Up @@ -83,6 +83,15 @@ restart-commands = ["/usr/bin/host-containers"]
[metadata.settings.host-containers]
affected-services = ["host-containers"]

# Network

[configuration-files.proxy-env]
path = "/etc/network/proxy.env"
template-path = "/usr/share/templates/proxy-env"

[metadata.settings.network]
affected-services = ["containerd", "host-containerd"]

# NTP

[settings.ntp]
Expand Down
5 changes: 4 additions & 1 deletion sources/models/src/aws-dev/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@ use serde::{Deserialize, Serialize};
use std::collections::HashMap;

use crate::modeled_types::Identifier;
use crate::{AwsSettings, ContainerImage, KernelSettings, NtpSettings, UpdatesSettings};
use crate::{
AwsSettings, ContainerImage, KernelSettings, NetworkSettings, NtpSettings, UpdatesSettings,
};

// Note: we have to use 'rename' here because the top-level Settings structure is the only one
// that uses its name in serialization; internal structures use the field name that points to it
Expand All @@ -13,6 +15,7 @@ struct Settings {
updates: UpdatesSettings,
host_containers: HashMap<Identifier, ContainerImage>,
ntp: NtpSettings,
network: NetworkSettings,
kernel: KernelSettings,
aws: AwsSettings,
}
6 changes: 5 additions & 1 deletion sources/models/src/aws-dev/override-defaults.toml
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,8 @@ template-path = "/usr/share/templates/containerd-config-toml_aws-dev"
# Docker
[services.docker]
restart-commands = ["/bin/systemctl try-restart docker.service"]
configuration-files = []
configuration-files = ["proxy-env"]

# Network
[metadata.settings.network]
affected-services = ["containerd", "docker", "host-containerd"]
3 changes: 2 additions & 1 deletion sources/models/src/aws-ecs-1/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use std::collections::HashMap;

use crate::modeled_types::Identifier;
use crate::{
AwsSettings, ContainerImage, ECSSettings, KernelSettings, NtpSettings, UpdatesSettings,
AwsSettings, ContainerImage, ECSSettings, KernelSettings, NetworkSettings, NtpSettings, UpdatesSettings,
};

// Note: we have to use 'rename' here because the top-level Settings structure is the only one
Expand All @@ -15,6 +15,7 @@ struct Settings {
updates: UpdatesSettings,
host_containers: HashMap<Identifier, ContainerImage>,
ntp: NtpSettings,
network: NetworkSettings,
kernel: KernelSettings,
aws: AwsSettings,
ecs: ECSSettings,
Expand Down
Loading