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

Update values in devcontainer.json are not respected after the container is created #252

Closed
edgonmsft opened this issue Oct 28, 2022 · 2 comments · Fixed by #254
Closed
Assignees
Labels
bug Something isn't working candidate Issue identified as probable candidate for fixing in the next release verified
Milestone

Comments

@edgonmsft
Copy link
Contributor

Version 0.74.0

Updated values, such as remoteEnv in devcontainer.json after the container is created are not respected since the values from the container metadata are taking precedence.

This breaks flows where this values are updated, like secrets, to keep environments working.

The issue appears to be coming from here:

const copy = { ...config };
replaceProperties.forEach(property => delete (copy as any)[property]);
const merged: MergedDevContainerConfig = {
...copy,
init: imageMetadata.some(entry => entry.init),
privileged: imageMetadata.some(entry => entry.privileged),
capAdd: unionOrUndefined(imageMetadata.map(entry => entry.capAdd)),
securityOpt: unionOrUndefined(imageMetadata.map(entry => entry.securityOpt)),
entrypoints: collectOrUndefined(imageMetadata, 'entrypoint'),
mounts: mergeMounts(imageMetadata),
customizations: Object.keys(customizations).length ? customizations : undefined,
onCreateCommands: collectOrUndefined(imageMetadata, 'onCreateCommand'),
updateContentCommands: collectOrUndefined(imageMetadata, 'updateContentCommand'),
postCreateCommands: collectOrUndefined(imageMetadata, 'postCreateCommand'),
postStartCommands: collectOrUndefined(imageMetadata, 'postStartCommand'),
postAttachCommands: collectOrUndefined(imageMetadata, 'postAttachCommand'),
waitFor: reversed.find(entry => entry.waitFor)?.waitFor,
remoteUser: reversed.find(entry => entry.remoteUser)?.remoteUser,
containerUser: reversed.find(entry => entry.containerUser)?.containerUser,
userEnvProbe: reversed.find(entry => entry.userEnvProbe)?.userEnvProbe,
remoteEnv: Object.assign({}, ...imageMetadata.map(entry => entry.remoteEnv)),
containerEnv: Object.assign({}, ...imageMetadata.map(entry => entry.containerEnv)),
overrideCommand: reversed.find(entry => typeof entry.overrideCommand === 'boolean')?.overrideCommand,
portsAttributes: Object.assign({}, ...imageMetadata.map(entry => entry.portsAttributes)),
otherPortsAttributes: reversed.find(entry => entry.otherPortsAttributes)?.otherPortsAttributes,
forwardPorts: mergeForwardPorts(imageMetadata),
shutdownAction: reversed.find(entry => entry.shutdownAction)?.shutdownAction,
updateRemoteUserUID: reversed.find(entry => typeof entry.updateRemoteUserUID === 'boolean')?.updateRemoteUserUID,
hostRequirements: mergeHostRequirements(imageMetadata),
};
return merged;

@chrmarti chrmarti self-assigned this Oct 31, 2022
@chrmarti chrmarti added the candidate Issue identified as probable candidate for fixing in the next release label Oct 31, 2022
@chrmarti chrmarti added this to the October 2022 milestone Oct 31, 2022
@chrmarti chrmarti added the bug Something isn't working label Oct 31, 2022
chrmarti added a commit that referenced this issue Oct 31, 2022
@chrmarti
Copy link
Contributor

The difficulty here is that imageMetadata has the properties from the initial devcontainer.json as its last entry if our CLI created the container as a dev container (e.g., a non-dev container in Compose could still have metadata entries, other tools might do something different).

To make sure we don't replace the wrong metadata entry, I suggest we update only the properties that are straight-forward to merge: remoteEnv, remoteUser and userEnvProbe.

Alternatively we could surface the option --remote-env to allow for adding / overwriting environment variables.

@chrmarti
Copy link
Contributor

Fixed in 0.262.1-pre-release. To verify in the Dev Containers extension:

  • Create a dev container with a variable in "remoteEnv".
  • Change the variable's value and add another variable.
  • Reload the window.
  • Verify the environment variables are updated in a new terminal (restored terminals won't update their environment).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working candidate Issue identified as probable candidate for fixing in the next release verified
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants