Skip to content

always use a fresh docker config for each step#756

Merged
chloeruka merged 7 commits into
masterfrom
isolate-docker-config
Oct 26, 2020
Merged

always use a fresh docker config for each step#756
chloeruka merged 7 commits into
masterfrom
isolate-docker-config

Conversation

@yob

@yob yob commented Oct 26, 2020

Copy link
Copy Markdown
Contributor

This improves job isolation, preventing jobs from relying on authentication configured in other jobs.

This is based on PR #678, but we've removed the stack parameter because the v5.0.0 release is pending and we're comfortable making a small breaking change.

Patrick Robinson and others added 2 commits October 26, 2020 16:37
This avoids potential race condition that:
- Allow steps to use credential from other steps
- Causes steps to use incorrect credentials, if they are over-written by another step
This improves job isolation, preventing jobs from relying on
authentication configured in other jobs.

This is based on PR #678, but we've removed the stack parameter because
the v5.0.0 release is pending and we're comfortable making a small
breaking change.

@chloeruka chloeruka left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Straightforward change to improve step isolation. We've discussed this and decided it's better to make this (possibly) breaking change now as part of v5 release, and to bugfix it later if necessary. Original PR (#678) says this has been working well for them for quite some time.

@lox

lox commented Oct 26, 2020

Copy link
Copy Markdown
Contributor

Couldn't we do this in the environment hook, or a bootstrap script?

@yob

yob commented Oct 26, 2020

Copy link
Copy Markdown
Contributor Author

@lox are you suggesting this doesn't need to be baked in for everyone?

yob and others added 3 commits October 26, 2020 17:05
Setting it directly in the agent environment hook means we don't need to
escape the eval. It also means we won't cat it out into build output
with the rest of cfn-env.

While I was there I also made the same change to windows.
This also fixes a shellcheck error SC2155
Instead of setting DOCKER_CONFIG directly, set
BUILDKITE_DOCKER_CONFIG_TEMP_DIRECTORY and hydrate DOCKER_CONFIG for
plugin steps. if this gets overriden by the user, they can manage their
own DOCKER_CONFIG cleanup activities and we will always remove the
temporary directory from the agent instance filesystem.
Comment thread packer/linux/conf/buildkite-agent/hooks/pre-exit
@chloeruka chloeruka self-assigned this Oct 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants