Skip to content

Allow mounting in DOCKER_CONFIG again #392

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

Jump to bottom
Closed
johnstcn opened this issue Oct 17, 2024 · 4 comments · Fixed by #398
Closed

Allow mounting in DOCKER_CONFIG again #392

johnstcn opened this issue Oct 17, 2024 · 4 comments · Fixed by #398
Assignees
@mafredri
Labels
customer-reported

Comments

@johnstcn
Copy link
Member

Motivation

In #336 we temporarily override DOCKER_CONFIG to set the config specified by ENVBUILDER_DOCKER_CONFIG_BASE64.

This breaks some existing users who were mounting in a docker config and specifying the DOCKER_CONFIG environment variable in their image.

Proposed solution

Only override DOCKER_CONFIG if ENVBUILDER_DOCKER_CONFIG_BASE64 is set. Otherwise, leave it as-is.
@mafredri
Copy link
Member

mafredri commented Oct 28, 2024 •
edited
Loading

Only override DOCKER_CONFIG if ENVBUILDER_DOCKER_CONFIG_BASE64 is set. Otherwise, leave it as-is.

@johnstcn It looks like we do this already:

envbuilder/envbuilder.go

Lines 1633 to 1635 in 08bdb8d

if dockerConfigBase64 == "" {
return noop, nil
}

Is this a problem with the latest version of envbuilder or is the customer looking for some other behavior than mentioned?

If the expectation is to never overwrite DOCKER_CONFIG, then setting both DOCKER_CONFIG and ENVBUILDER_DOCKER_CONFIG_BASE64 should probably result in an error. We can't know which should take precedence.

@johnstcn
Copy link
Member Author

Ah, good point. The difference in behaviour was pointed out between v1.0.0-rc.6 and v1.0.0-rc.7.

@mafredri
Copy link
Member

Ok, so in v1.0.0-rc.7 the behavior was changed so that if you don't set base64, DOCKER_CONFIG doesn't get set either. Meaning if you define DOCKER_CONFIG, that's the one that will be used. However, if you were relying on envbuilder to search for a Docker config at /.envbuilder/config.json, then v1.0.0-rc.7 would break your use-case. Could this be what's happening?

@johnstcn
Copy link
Member Author

Confirmed that this is what's happening.

mafredri added a commit that referenced this issue Oct 28, 2024
@mafredri
fix: always set DOCKER_CONFIG unless manually set and no base64 confi…
b34692e 
…g provided

Fixes #392
@mafredri mafredri mentioned this issue Oct 28, 2024
Merged
mafredri added a commit that referenced this issue Oct 28, 2024
@mafredri
fix: always set DOCKER_CONFIG unless manually set and no base64 confi…
147ba65 
…g provided

Fixes #392
mafredri added a commit that referenced this issue Oct 28, 2024
@mafredri
fix: always set DOCKER_CONFIG unless manually set and no base64 confi…
8c0198f 
…g provided

Fixes #392
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@mafredri mafredri

Labels
customer-reported
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix: search $DOCKER_CONFIG if no base64 config is provided coder/envbuilder
2 participants
@mafredri @johnstcn