Skip to content
This repository has been archived by the owner on Feb 5, 2020. It is now read-only.

*: fix cert and env binding for k8s-node-boostrapper #2757

Closed
wants to merge 1 commit into from
Closed

*: fix cert and env binding for k8s-node-boostrapper #2757

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 3 additions & 1 deletion modules/ignition/resources/services/k8s-node-bootstrap.service
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,12 @@ RestartSec=10
TimeoutStartSec=1h
ExecStartPre=/usr/bin/mkdir -p /etc/kubernetes
${kubeconfig_fetch_cmd}
ExecStartPre=-/bin/touch /etc/profile.env
ExecStartPre=/usr/bin/docker run --rm \
--env-file /etc/profile.env \
Copy link
Contributor

@euank euank Jan 23, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

Would it be bettre to use --env HTTP_PROXY --env NO_PROXY (and any other proxy-related variables; those are the ones the go stdlib understands iirc)?

The profile.env file is meant for full shell parsing and for interactive user shells, so it's perfectly possible a user will, for valid reasons, have set it to include more complex statements than docker's --env-file may understand.

Minimizing the set of environment variables getting passed through is also good from a security perspective.

With the --env suggestion above, using a dropin for specifically this service or globally via /etc/systemd/system.conf.d/ should work, and it won't require touching anything in the default case since docker doesn't error if an --env value doesn't exist.

Copy link
Contributor

Choose a reason for hiding this comment

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

the problem tho is that the docker container needs the env vars. I am fine with pointing at a diff file. In most cases I am blindly expecting the env vars in /etc/profile.env to apply cleanly here.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think this makes sense. Basically extend the k8s-node-bootstrapper.service with the proxy vars and feed them in with --env This is probably cleaner cause the --env-file doesn't have to exist.

--tmpfs /tmp \
-v /usr/share:/usr/share:ro \
-v /usr/lib/os-release:/usr/lib/os-release:ro \
-v /usr/share/ca-certificates/ca-certificates.crt:/etc/ssl/certs/ca-certificates.crt:ro \
-v /var/lib/torcx:/var/lib/torcx \
-v /var/run/dbus:/var/run/dbus \
-v /run/metadata:/run/metadata:ro \
Expand All @@ -24,6 +25,7 @@ ExecStartPre=/usr/bin/docker run --rm \
-v /etc/coreos:/etc/coreos:ro \
-v /etc/torcx:/etc/torcx \
-v /etc/kubernetes:/etc/kubernetes \
-v /etc/ssl/certs/ca-certificates.crt:/etc/ssl/certs/ca-certificates.crt \
${tectonic_torcx_image_url}:${tectonic_torcx_image_tag} \
/tectonic-torcx-bootstrap \
--upgrade-os=${bootstrap_upgrade_cl} \
Expand Down