From 9db7ab74a7749f5865db85e48a5baa4634c39174 Mon Sep 17 00:00:00 2001 From: "Brandon Waterloo [MSFT]" <36966225+bwateratmsft@users.noreply.github.com> Date: Mon, 27 Jun 2022 11:47:43 -0400 Subject: [PATCH 1/5] Deprecate and migrate old environment variable settings --- package.json | 47 ++------ package.nls.json | 6 +- src/docker/ContextManager.ts | 15 +-- src/extension.ts | 29 ++++- src/utils/addDockerSettingsToEnv.ts | 30 ++---- .../migrateOldEnvironmentSettingsIfNeeded.ts | 101 ++++++++++++++++++ 6 files changed, 154 insertions(+), 74 deletions(-) create mode 100644 src/utils/migrateOldEnvironmentSettingsIfNeeded.ts diff --git a/package.json b/package.json index 18d7053b43..9d8477faf7 100644 --- a/package.json +++ b/package.json @@ -2231,39 +2231,17 @@ "default": 10, "description": "%vscode-docker.config.docker.truncateMaxLength%" }, - "docker.dockerodeOptions": { + "docker.environment": { "type": "object", - "description": "%vscode-docker.config.docker.dockerodeOptions%" - }, - "docker.host": { - "type": "string", - "default": "", - "description": "%vscode-docker.config.docker.host%", - "scope": "machine-overridable" - }, - "docker.context": { - "type": "string", - "default": "", - "description": "%vscode-docker.config.docker.context%", - "scope": "machine-overridable" - }, - "docker.certPath": { - "type": "string", - "default": "", - "description": "%vscode-docker.config.docker.certPath%", - "scope": "machine-overridable" - }, - "docker.tlsVerify": { - "type": "string", - "default": "", - "description": "%vscode-docker.config.docker.tlsVerify%", + "additionalProperties": { + "type": "string" + }, + "description": "%vscode-docker.config.docker.environment%", "scope": "machine-overridable" }, - "docker.machineName": { - "type": "string", - "default": "", - "description": "%vscode-docker.config.docker.machineName%", - "scope": "machine-overridable" + "docker.dockerodeOptions": { + "type": "object", + "description": "%vscode-docker.config.docker.dockerodeOptions%" }, "docker.languageserver.diagnostics.deprecatedMaintainer": { "scope": "resource", @@ -3081,14 +3059,11 @@ "docker.commands.logs", "docker.commands.composeUp", "docker.commands.composeDown", + "docker.environment", "docker.dockerodeOptions", - "docker.host", - "docker.context", - "docker.certPath", - "docker.tlsVerify", - "docker.machineName", "docker.scaffolding.templatePath", - "docker.dockerPath" + "docker.dockerPath", + "docker.composeCommand" ] } }, diff --git a/package.nls.json b/package.nls.json index 359730b150..6ed435504c 100644 --- a/package.nls.json +++ b/package.nls.json @@ -177,12 +177,8 @@ "vscode-docker.config.docker.imageBuildContextPath": "Build context PATH to pass to Docker build command.", "vscode-docker.config.docker.truncateLongRegistryPaths": "Set to true to truncate long image and container registry paths in Docker view", "vscode-docker.config.docker.truncateMaxLength": "Maximum length of a registry paths displayed in Docker view, including ellipsis. The truncateLongRegistryPaths setting must be set to true for truncateMaxLength setting to be effective.", + "vscode-docker.config.docker.environment": "Environment variables that will be applied to all VS Code terminals and to all background processes started by the Docker extension. Use for variables like `DOCKER_HOST`, etc.", "vscode-docker.config.docker.dockerodeOptions": "If specified, this object will be passed to the Dockerode constructor. Takes precedence over DOCKER_HOST, the Docker Host setting, and any existing Docker contexts.", - "vscode-docker.config.docker.host": "Equivalent to setting the DOCKER_HOST environment variable, for example, ssh://myuser@mymachine or tcp://1.2.3.4.", - "vscode-docker.config.docker.context": "Equivalent to setting the DOCKER_CONTEXT environment variable.", - "vscode-docker.config.docker.certPath": "Equivalent to setting the DOCKER_CERT_PATH environment variable.", - "vscode-docker.config.docker.tlsVerify": "Equivalent to setting the DOCKER_TLS_VERIFY environment variable.", - "vscode-docker.config.docker.machineName": "Equivalent to setting the DOCKER_MACHINE_NAME environment variable.", "vscode-docker.config.docker.languageserver.diagnostics.deprecatedMaintainer": "Controls the diagnostic severity for the deprecated MAINTAINER instruction", "vscode-docker.config.docker.languageserver.diagnostics.emptyContinuationLine": "Controls the diagnostic severity for flagging empty continuation lines found in instructions that span multiple lines", "vscode-docker.config.docker.languageserver.diagnostics.directiveCasing": "Controls the diagnostic severity for parser directives that are not written in lowercase", diff --git a/src/docker/ContextManager.ts b/src/docker/ContextManager.ts index 8dd68cce7a..bbda31f39d 100644 --- a/src/docker/ContextManager.ts +++ b/src/docker/ContextManager.ts @@ -274,7 +274,7 @@ export class DockerContextManager implements ContextManager, Disposable { this.tryGetContextFromEnvironment(actionContext) || this.tryGetContextFromFilesystemClues(actionContext); - // A result from any of these three implies that there is only one context, or it is fixed by `docker.host` / `DOCKER_HOST`, or `docker.context` / `DOCKER_CONTEXT` + // A result from any of these three implies that there is only one context, or it is fixed by `[docker.environment.]DOCKER_HOST`, or `[docker.environment.]DOCKER_CONTEXT` // As such, we will lock to the current context // Otherwise, unlock in case we were previously locked if (fixedContext) { @@ -314,17 +314,18 @@ export class DockerContextManager implements ContextManager, Disposable { const config = workspace.getConfiguration('docker'); let dockerHost: string | undefined; let dockerContext: string | undefined; + const environment = config.get('environment', {}); - if ((dockerHost = config.get('host'))) { // Assignment + check is intentional - actionContext.telemetry.properties.hostSource = 'docker.host'; + if ((dockerHost = environment['DOCKER_HOST'])) { // Assignment + check is intentional + actionContext.telemetry.properties.hostSource = 'docker.environment.host'; return { ...defaultContext, Current: true, DockerEndpoint: dockerHost, } as DockerContext; - } else if ((dockerContext = config.get('context'))) { // Assignment + check is intentional - actionContext.telemetry.properties.hostSource = 'docker.context'; + } else if ((dockerContext = environment['DOCKER_CONTEXT'])) { // Assignment + check is intentional + actionContext.telemetry.properties.hostSource = 'docker.environment.context'; return dockerContext; } @@ -442,13 +443,13 @@ export class DockerContextManager implements ContextManager, Disposable { // If URL parsing fails, let's catch it and give a better error message to help users from a common mistake actionContext.telemetry.properties.hostProtocol = 'unknown'; const message = - localize('vscode-docker.docker.contextManager.invalidHostSetting', 'The value provided for the setting `docker.host` or environment variable `DOCKER_HOST` is invalid. It must include the protocol, for example, ssh://myuser@mymachine or tcp://1.2.3.4.'); + localize('vscode-docker.docker.contextManager.invalidHostSetting', 'The value provided for the setting `docker.environment.DOCKER_HOST` or environment variable `DOCKER_HOST` is invalid. It must include the protocol, for example, ssh://myuser@mymachine or tcp://1.2.3.4.'); const button = localize('vscode-docker.docker.contextManager.openSettings', 'Open Settings'); void window.showErrorMessage(message, button) .then((result: string) => { if (result === button) { - void commands.executeCommand('workbench.action.openSettings', 'docker.host'); + void commands.executeCommand('workbench.action.openSettings', 'docker.environment'); } }); diff --git a/src/extension.ts b/src/extension.ts index 26e0a63137..8ce47902bb 100644 --- a/src/extension.ts +++ b/src/extension.ts @@ -24,6 +24,7 @@ import { registerTrees } from './tree/registerTrees'; import { AzureAccountExtensionListener } from './utils/AzureAccountExtensionListener'; import { cryptoUtils } from './utils/cryptoUtils'; import { DocumentSettingsClientFeature } from './utils/DocumentSettingsClientFeature'; +import { migrateOldEnvironmentSettingsIfNeeded } from './utils/migrateOldEnvironmentSettingsIfNeeded'; import { isLinux, isMac, isWindows } from './utils/osUtils'; export type KeyInfo = { [keyName: string]: string }; @@ -60,6 +61,9 @@ export async function activateInternal(ctx: vscode.ExtensionContext, perfStats: activateContext.telemetry.measurements.mainFileLoad = (perfStats.loadEndTime - perfStats.loadStartTime) / 1000; activateContext.telemetry.properties.dockerInstallationIDHash = await getDockerInstallationIDHash(); + // Set up environment variables + setEnvironmentVariableContributions(ctx); + // All of these internally handle telemetry opt-in ext.activityMeasurementService = new ActivityMeasurementService(ctx.globalState); ext.experimentationService = await createExperimentationService( @@ -111,6 +115,9 @@ export async function activateInternal(ctx: vscode.ExtensionContext, perfStats: registerListeners(); }); + // If this call results in changes to the values, the settings listener set up below will automatically re-update + void migrateOldEnvironmentSettingsIfNeeded(); + // If the magic VSCODE_DOCKER_TEAM environment variable is set to 1, export the mementos for use by the Memento Explorer extension if (process.env.VSCODE_DOCKER_TEAM === '1') { return { @@ -190,13 +197,14 @@ namespace Configuration { settings: null }); + // Reset extension environment variables contribution if needed + if (e.affectsConfiguration('docker.environment')) { + setEnvironmentVariableContributions(ext.context); + } + // These settings will result in a need to change context that doesn't actually change the docker context // So, force a manual refresh so the settings get picked up - if (e.affectsConfiguration('docker.host') || - e.affectsConfiguration('docker.context') || - e.affectsConfiguration('docker.certPath') || - e.affectsConfiguration('docker.tlsVerify') || - e.affectsConfiguration('docker.machineName') || + if (e.affectsConfiguration('docker.environment') || e.affectsConfiguration('docker.dockerodeOptions') || e.affectsConfiguration('docker.dockerPath') || e.affectsConfiguration('docker.composeCommand')) { @@ -208,6 +216,17 @@ namespace Configuration { } /* eslint-enable @typescript-eslint/no-namespace, no-inner-declarations */ +function setEnvironmentVariableContributions(ctx: vscode.ExtensionContext): void { + const settingValue: NodeJS.ProcessEnv = vscode.workspace.getConfiguration('docker').get('environment', {}); + + ctx.environmentVariableCollection.clear(); + ctx.environmentVariableCollection.persistent = true; + + for (const key of Object.keys(settingValue)) { + ctx.environmentVariableCollection.replace(key, settingValue[key]); + } +} + function activateDockerfileLanguageClient(ctx: vscode.ExtensionContext): void { // Don't wait void callWithTelemetryAndErrorHandling('docker.languageclient.activate', async (context: IActionContext) => { diff --git a/src/utils/addDockerSettingsToEnv.ts b/src/utils/addDockerSettingsToEnv.ts index f511857f3e..9f2e9c4478 100644 --- a/src/utils/addDockerSettingsToEnv.ts +++ b/src/utils/addDockerSettingsToEnv.ts @@ -4,30 +4,18 @@ *--------------------------------------------------------------------------------------------*/ import { workspace } from 'vscode'; -import { configPrefix } from '../constants'; import { ext } from '../extensionVariables'; import { localize } from '../localize'; -export function addDockerSettingsToEnv(env: NodeJS.ProcessEnv, oldEnv: NodeJS.ProcessEnv): void { - addDockerSettingToEnv("host", 'DOCKER_HOST', env, oldEnv); - addDockerSettingToEnv("context", 'DOCKER_CONTEXT', env, oldEnv); - addDockerSettingToEnv("certPath", 'DOCKER_CERT_PATH', env, oldEnv); - addDockerSettingToEnv("tlsVerify", 'DOCKER_TLS_VERIFY', env, oldEnv); - addDockerSettingToEnv("machineName", 'DOCKER_MACHINE_NAME', env, oldEnv); -} - -function addDockerSettingToEnv(settingKey: string, envVar: string, env: NodeJS.ProcessEnv, oldEnv: NodeJS.ProcessEnv): void { - const value = workspace.getConfiguration(configPrefix).get(settingKey, ''); +export function addDockerSettingsToEnv(newEnv: NodeJS.ProcessEnv, oldEnv: NodeJS.ProcessEnv): void { + const environmentSettings: NodeJS.ProcessEnv = workspace.getConfiguration('docker').get('environment', {}); - const expectedType = "string"; - const actualType = typeof value; - if (expectedType !== actualType) { - ext.outputChannel.appendLine(localize('vscode-docker.utils.env.ignoring', 'WARNING: Ignoring setting "{0}.{1}" because type "{2}" does not match expected type "{3}".', configPrefix, settingKey, actualType, expectedType)); - } else if (value) { - if (oldEnv[envVar] && oldEnv[envVar] !== value) { - ext.outputChannel.appendLine(localize('vscode-docker.utils.env.overwriting', 'WARNING: Overwriting environment variable "{0}" with VS Code setting "{1}.{2}".', envVar, configPrefix, settingKey)); - } - - env[envVar] = value; + for (const key of Object.keys(environmentSettings)) { + ext.outputChannel.appendLine(localize('vscode-docker.utils.env.overwriting', 'WARNING: Overwriting environment variable "{0}" from VS Code setting "docker.environment".', key)); } + + newEnv = { + ...oldEnv, + ...environmentSettings, + }; } diff --git a/src/utils/migrateOldEnvironmentSettingsIfNeeded.ts b/src/utils/migrateOldEnvironmentSettingsIfNeeded.ts new file mode 100644 index 0000000000..5cb6992f89 --- /dev/null +++ b/src/utils/migrateOldEnvironmentSettingsIfNeeded.ts @@ -0,0 +1,101 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See LICENSE.md in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import { DialogResponses } from '@microsoft/vscode-azext-utils'; +import * as vscode from 'vscode'; +import { localize } from '../localize'; +import { cloneObject } from './cloneObject'; + +const oldSettingsMap = { + 'host': 'DOCKER_HOST', + 'context': 'DOCKER_CONTEXT', + 'certPath': 'DOCKER_CERT_PATH', + 'tlsVerify': 'DOCKER_TLS_VERIFY', + 'machineName': 'DOCKER_MACHINE_NAME', +}; + +export async function migrateOldEnvironmentSettingsIfNeeded(): Promise { + const config = vscode.workspace.getConfiguration('docker'); + + let alreadyPrompted = false; + for (const oldSetting of Object.keys(oldSettingsMap)) { + const settingValue: string | undefined = config.get(oldSetting); + + // If any config target has a value, we'll attempt to migrate all three as necessary + if (settingValue) { + // Prompt if we haven't already + if (!alreadyPrompted) { + const response = await vscode.window.showWarningMessage( + localize('vscode-docker.checkForOldEnvironmentSettings.prompt', 'Some of your Docker extension settings have been renamed. Would you like us to migrate them for you?'), + DialogResponses.yes, + DialogResponses.no + ); + + if (response === DialogResponses.yes) { + alreadyPrompted = true; + } else { + return; + } + } + + await migrateOldEnvironmentSetting(config, oldSetting); + } + } +} + +async function migrateOldEnvironmentSetting(config: vscode.WorkspaceConfiguration, oldSetting: string): Promise { + const settingInspection = config.inspect(oldSetting); + const currentEnvironmentSettings = config.inspect('environment'); + + // Migrate the global AKA user setting + await migrateOldEnvironmentSettingForTarget( + config, + oldSetting, + settingInspection.globalValue, + currentEnvironmentSettings.globalValue, + vscode.ConfigurationTarget.Global + ); + + // Migrate the workspace setting + await migrateOldEnvironmentSettingForTarget( + config, + oldSetting, + settingInspection.workspaceValue, + currentEnvironmentSettings.workspaceValue, + vscode.ConfigurationTarget.Workspace + ); + + // Migrate the workspace folder setting + await migrateOldEnvironmentSettingForTarget( + config, + oldSetting, + settingInspection.workspaceFolderValue, + currentEnvironmentSettings.workspaceFolderValue, + vscode.ConfigurationTarget.WorkspaceFolder + ); +} + +async function migrateOldEnvironmentSettingForTarget( + config: vscode.WorkspaceConfiguration, + oldSetting: string, + oldValueForTarget: string | undefined, + currentEnvValue: NodeJS.ProcessEnv | undefined, + target: vscode.ConfigurationTarget +): Promise { + // If no value was set in this particular target, skip + if (!oldValueForTarget) { + return; + } + + // Remove the old setting from this target + await config.update(oldSetting, undefined, target); + + // Append the old value to the current environment object for this target + const newValue = cloneObject(currentEnvValue ?? {}); + newValue[oldSettingsMap[oldSetting]] = oldValueForTarget; + + // Update the new setting for this target + await config.update('environment', newValue, target); +} From 5ecdef284bc9a3aa17c8fd0708b5f2440e4e392d Mon Sep 17 00:00:00 2001 From: "Brandon Waterloo [MSFT]" <36966225+bwateratmsft@users.noreply.github.com> Date: Tue, 28 Jun 2022 12:07:28 -0400 Subject: [PATCH 2/5] Fix bug --- src/utils/addDockerSettingsToEnv.ts | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/utils/addDockerSettingsToEnv.ts b/src/utils/addDockerSettingsToEnv.ts index 9f2e9c4478..47b5460473 100644 --- a/src/utils/addDockerSettingsToEnv.ts +++ b/src/utils/addDockerSettingsToEnv.ts @@ -12,10 +12,6 @@ export function addDockerSettingsToEnv(newEnv: NodeJS.ProcessEnv, oldEnv: NodeJS for (const key of Object.keys(environmentSettings)) { ext.outputChannel.appendLine(localize('vscode-docker.utils.env.overwriting', 'WARNING: Overwriting environment variable "{0}" from VS Code setting "docker.environment".', key)); + newEnv[key] = environmentSettings[key]; } - - newEnv = { - ...oldEnv, - ...environmentSettings, - }; } From 80a2a4bcbc2d2251919b74eba437be55c117800f Mon Sep 17 00:00:00 2001 From: "Brandon Waterloo [MSFT]" <36966225+bwateratmsft@users.noreply.github.com> Date: Tue, 5 Jul 2022 11:07:24 -0400 Subject: [PATCH 3/5] Refactoring to `containers.environment` --- package.json | 6 +- package.nls.json | 2 +- src/docker/ContextManager.ts | 12 ++-- src/extension.ts | 6 +- src/utils/addDockerSettingsToEnv.ts | 2 +- .../migrateOldEnvironmentSettingsIfNeeded.ts | 58 +++++++++++-------- 6 files changed, 48 insertions(+), 38 deletions(-) diff --git a/package.json b/package.json index 9d8477faf7..8a1ad4170b 100644 --- a/package.json +++ b/package.json @@ -2231,12 +2231,12 @@ "default": 10, "description": "%vscode-docker.config.docker.truncateMaxLength%" }, - "docker.environment": { + "containers.environment": { "type": "object", "additionalProperties": { "type": "string" }, - "description": "%vscode-docker.config.docker.environment%", + "description": "%vscode-docker.config.containers.environment%", "scope": "machine-overridable" }, "docker.dockerodeOptions": { @@ -3059,7 +3059,7 @@ "docker.commands.logs", "docker.commands.composeUp", "docker.commands.composeDown", - "docker.environment", + "containers.environment", "docker.dockerodeOptions", "docker.scaffolding.templatePath", "docker.dockerPath", diff --git a/package.nls.json b/package.nls.json index 6ed435504c..bc3bb7b866 100644 --- a/package.nls.json +++ b/package.nls.json @@ -177,7 +177,7 @@ "vscode-docker.config.docker.imageBuildContextPath": "Build context PATH to pass to Docker build command.", "vscode-docker.config.docker.truncateLongRegistryPaths": "Set to true to truncate long image and container registry paths in Docker view", "vscode-docker.config.docker.truncateMaxLength": "Maximum length of a registry paths displayed in Docker view, including ellipsis. The truncateLongRegistryPaths setting must be set to true for truncateMaxLength setting to be effective.", - "vscode-docker.config.docker.environment": "Environment variables that will be applied to all VS Code terminals and to all background processes started by the Docker extension. Use for variables like `DOCKER_HOST`, etc.", + "vscode-docker.config.containers.environment": "Environment variables that will be applied to all VS Code terminals and to all background processes started by the Docker extension. Use for variables like `DOCKER_HOST`, etc.", "vscode-docker.config.docker.dockerodeOptions": "If specified, this object will be passed to the Dockerode constructor. Takes precedence over DOCKER_HOST, the Docker Host setting, and any existing Docker contexts.", "vscode-docker.config.docker.languageserver.diagnostics.deprecatedMaintainer": "Controls the diagnostic severity for the deprecated MAINTAINER instruction", "vscode-docker.config.docker.languageserver.diagnostics.emptyContinuationLine": "Controls the diagnostic severity for flagging empty continuation lines found in instructions that span multiple lines", diff --git a/src/docker/ContextManager.ts b/src/docker/ContextManager.ts index bbda31f39d..21fb4a4441 100644 --- a/src/docker/ContextManager.ts +++ b/src/docker/ContextManager.ts @@ -274,7 +274,7 @@ export class DockerContextManager implements ContextManager, Disposable { this.tryGetContextFromEnvironment(actionContext) || this.tryGetContextFromFilesystemClues(actionContext); - // A result from any of these three implies that there is only one context, or it is fixed by `[docker.environment.]DOCKER_HOST`, or `[docker.environment.]DOCKER_CONTEXT` + // A result from any of these three implies that there is only one context, or it is fixed by `[containers.environment.]DOCKER_HOST`, or `[containers.environment.]DOCKER_CONTEXT` // As such, we will lock to the current context // Otherwise, unlock in case we were previously locked if (fixedContext) { @@ -311,13 +311,13 @@ export class DockerContextManager implements ContextManager, Disposable { } private tryGetContextFromSettings(actionContext: IActionContext): DockerContext | undefined | string { - const config = workspace.getConfiguration('docker'); + const config = workspace.getConfiguration('containers'); let dockerHost: string | undefined; let dockerContext: string | undefined; const environment = config.get('environment', {}); if ((dockerHost = environment['DOCKER_HOST'])) { // Assignment + check is intentional - actionContext.telemetry.properties.hostSource = 'docker.environment.host'; + actionContext.telemetry.properties.hostSource = 'containers.environment.host'; return { ...defaultContext, @@ -325,7 +325,7 @@ export class DockerContextManager implements ContextManager, Disposable { DockerEndpoint: dockerHost, } as DockerContext; } else if ((dockerContext = environment['DOCKER_CONTEXT'])) { // Assignment + check is intentional - actionContext.telemetry.properties.hostSource = 'docker.environment.context'; + actionContext.telemetry.properties.hostSource = 'containers.environment.context'; return dockerContext; } @@ -443,13 +443,13 @@ export class DockerContextManager implements ContextManager, Disposable { // If URL parsing fails, let's catch it and give a better error message to help users from a common mistake actionContext.telemetry.properties.hostProtocol = 'unknown'; const message = - localize('vscode-docker.docker.contextManager.invalidHostSetting', 'The value provided for the setting `docker.environment.DOCKER_HOST` or environment variable `DOCKER_HOST` is invalid. It must include the protocol, for example, ssh://myuser@mymachine or tcp://1.2.3.4.'); + localize('vscode-docker.docker.contextManager.invalidHostSetting', 'The value provided for the setting `containers.environment.DOCKER_HOST` or environment variable `DOCKER_HOST` is invalid. It must include the protocol, for example, ssh://myuser@mymachine or tcp://1.2.3.4.'); const button = localize('vscode-docker.docker.contextManager.openSettings', 'Open Settings'); void window.showErrorMessage(message, button) .then((result: string) => { if (result === button) { - void commands.executeCommand('workbench.action.openSettings', 'docker.environment'); + void commands.executeCommand('workbench.action.openSettings', 'containers.environment'); } }); diff --git a/src/extension.ts b/src/extension.ts index 8ce47902bb..d2d3ddda82 100644 --- a/src/extension.ts +++ b/src/extension.ts @@ -198,13 +198,13 @@ namespace Configuration { }); // Reset extension environment variables contribution if needed - if (e.affectsConfiguration('docker.environment')) { + if (e.affectsConfiguration('containers.environment')) { setEnvironmentVariableContributions(ext.context); } // These settings will result in a need to change context that doesn't actually change the docker context // So, force a manual refresh so the settings get picked up - if (e.affectsConfiguration('docker.environment') || + if (e.affectsConfiguration('containers.environment') || e.affectsConfiguration('docker.dockerodeOptions') || e.affectsConfiguration('docker.dockerPath') || e.affectsConfiguration('docker.composeCommand')) { @@ -217,7 +217,7 @@ namespace Configuration { /* eslint-enable @typescript-eslint/no-namespace, no-inner-declarations */ function setEnvironmentVariableContributions(ctx: vscode.ExtensionContext): void { - const settingValue: NodeJS.ProcessEnv = vscode.workspace.getConfiguration('docker').get('environment', {}); + const settingValue: NodeJS.ProcessEnv = vscode.workspace.getConfiguration('containers').get('environment', {}); ctx.environmentVariableCollection.clear(); ctx.environmentVariableCollection.persistent = true; diff --git a/src/utils/addDockerSettingsToEnv.ts b/src/utils/addDockerSettingsToEnv.ts index 47b5460473..ca3c7879dc 100644 --- a/src/utils/addDockerSettingsToEnv.ts +++ b/src/utils/addDockerSettingsToEnv.ts @@ -8,7 +8,7 @@ import { ext } from '../extensionVariables'; import { localize } from '../localize'; export function addDockerSettingsToEnv(newEnv: NodeJS.ProcessEnv, oldEnv: NodeJS.ProcessEnv): void { - const environmentSettings: NodeJS.ProcessEnv = workspace.getConfiguration('docker').get('environment', {}); + const environmentSettings: NodeJS.ProcessEnv = workspace.getConfiguration('containers').get('environment', {}); for (const key of Object.keys(environmentSettings)) { ext.outputChannel.appendLine(localize('vscode-docker.utils.env.overwriting', 'WARNING: Overwriting environment variable "{0}" from VS Code setting "docker.environment".', key)); diff --git a/src/utils/migrateOldEnvironmentSettingsIfNeeded.ts b/src/utils/migrateOldEnvironmentSettingsIfNeeded.ts index 5cb6992f89..d7fa064d73 100644 --- a/src/utils/migrateOldEnvironmentSettingsIfNeeded.ts +++ b/src/utils/migrateOldEnvironmentSettingsIfNeeded.ts @@ -17,13 +17,14 @@ const oldSettingsMap = { }; export async function migrateOldEnvironmentSettingsIfNeeded(): Promise { - const config = vscode.workspace.getConfiguration('docker'); + const oldConfig = vscode.workspace.getConfiguration('docker'); + const newConfig = vscode.workspace.getConfiguration('containers'); let alreadyPrompted = false; for (const oldSetting of Object.keys(oldSettingsMap)) { - const settingValue: string | undefined = config.get(oldSetting); + const settingValue: string | undefined = oldConfig.get(oldSetting); - // If any config target has a value, we'll attempt to migrate all three as necessary + // If any config target has a value, we'll attempt to migrate all three config sections as needed if (settingValue) { // Prompt if we haven't already if (!alreadyPrompted) { @@ -40,62 +41,71 @@ export async function migrateOldEnvironmentSettingsIfNeeded(): Promise { } } - await migrateOldEnvironmentSetting(config, oldSetting); + const newSetting = oldSettingsMap[oldSetting]; + await migrateOldEnvironmentSetting(oldConfig, oldSetting, newConfig, newSetting); } } } -async function migrateOldEnvironmentSetting(config: vscode.WorkspaceConfiguration, oldSetting: string): Promise { - const settingInspection = config.inspect(oldSetting); - const currentEnvironmentSettings = config.inspect('environment'); +async function migrateOldEnvironmentSetting(oldConfig: vscode.WorkspaceConfiguration, oldSetting: string, newConfig: vscode.WorkspaceConfiguration, newSetting: string): Promise { + const oldValueInspection = oldConfig.inspect(oldSetting); + const newValueInspection = newConfig.inspect('environment'); // Migrate the global AKA user setting await migrateOldEnvironmentSettingForTarget( - config, + oldConfig, oldSetting, - settingInspection.globalValue, - currentEnvironmentSettings.globalValue, + oldValueInspection.globalValue, + newConfig, + newSetting, + newValueInspection.globalValue, vscode.ConfigurationTarget.Global ); // Migrate the workspace setting await migrateOldEnvironmentSettingForTarget( - config, + oldConfig, oldSetting, - settingInspection.workspaceValue, - currentEnvironmentSettings.workspaceValue, + oldValueInspection.workspaceValue, + newConfig, + newSetting, + newValueInspection.workspaceValue, vscode.ConfigurationTarget.Workspace ); // Migrate the workspace folder setting await migrateOldEnvironmentSettingForTarget( - config, + oldConfig, oldSetting, - settingInspection.workspaceFolderValue, - currentEnvironmentSettings.workspaceFolderValue, + oldValueInspection.workspaceFolderValue, + newConfig, + newSetting, + newValueInspection.workspaceFolderValue, vscode.ConfigurationTarget.WorkspaceFolder ); } async function migrateOldEnvironmentSettingForTarget( - config: vscode.WorkspaceConfiguration, + oldConfig: vscode.WorkspaceConfiguration, oldSetting: string, - oldValueForTarget: string | undefined, - currentEnvValue: NodeJS.ProcessEnv | undefined, + oldValue: string | undefined, + newConfig: vscode.WorkspaceConfiguration, + newSetting: string, + newValue: NodeJS.ProcessEnv | undefined, target: vscode.ConfigurationTarget ): Promise { // If no value was set in this particular target, skip - if (!oldValueForTarget) { + if (!oldValue) { return; } // Remove the old setting from this target - await config.update(oldSetting, undefined, target); + await oldConfig.update(oldSetting, undefined, target); // Append the old value to the current environment object for this target - const newValue = cloneObject(currentEnvValue ?? {}); - newValue[oldSettingsMap[oldSetting]] = oldValueForTarget; + newValue = cloneObject(newValue ?? {}); + newValue[newSetting] = oldValue; // Update the new setting for this target - await config.update('environment', newValue, target); + await newConfig.update('environment', newValue, target); } From 4631f44e95a82bf05c2fa7f20cabc737d4128af6 Mon Sep 17 00:00:00 2001 From: "Brandon Waterloo [MSFT]" <36966225+bwateratmsft@users.noreply.github.com> Date: Wed, 20 Jul 2022 11:05:59 -0400 Subject: [PATCH 4/5] Update message --- src/utils/addDockerSettingsToEnv.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/utils/addDockerSettingsToEnv.ts b/src/utils/addDockerSettingsToEnv.ts index ca3c7879dc..3d104d8416 100644 --- a/src/utils/addDockerSettingsToEnv.ts +++ b/src/utils/addDockerSettingsToEnv.ts @@ -11,7 +11,7 @@ export function addDockerSettingsToEnv(newEnv: NodeJS.ProcessEnv, oldEnv: NodeJS const environmentSettings: NodeJS.ProcessEnv = workspace.getConfiguration('containers').get('environment', {}); for (const key of Object.keys(environmentSettings)) { - ext.outputChannel.appendLine(localize('vscode-docker.utils.env.overwriting', 'WARNING: Overwriting environment variable "{0}" from VS Code setting "docker.environment".', key)); + ext.outputChannel.appendLine(localize('vscode-docker.utils.env.overwriting', 'WARNING: Overwriting environment variable "{0}" from VS Code setting "containers.environment".', key)); newEnv[key] = environmentSettings[key]; } } From 256202413b7ba4db90bb04714e0da288de259bc1 Mon Sep 17 00:00:00 2001 From: "Brandon Waterloo [MSFT]" <36966225+bwateratmsft@users.noreply.github.com> Date: Wed, 20 Jul 2022 11:09:19 -0400 Subject: [PATCH 5/5] Eye candy --- src/utils/migrateOldEnvironmentSettingsIfNeeded.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/utils/migrateOldEnvironmentSettingsIfNeeded.ts b/src/utils/migrateOldEnvironmentSettingsIfNeeded.ts index d7fa064d73..51002e123b 100644 --- a/src/utils/migrateOldEnvironmentSettingsIfNeeded.ts +++ b/src/utils/migrateOldEnvironmentSettingsIfNeeded.ts @@ -47,7 +47,12 @@ export async function migrateOldEnvironmentSettingsIfNeeded(): Promise { } } -async function migrateOldEnvironmentSetting(oldConfig: vscode.WorkspaceConfiguration, oldSetting: string, newConfig: vscode.WorkspaceConfiguration, newSetting: string): Promise { +async function migrateOldEnvironmentSetting( + oldConfig: vscode.WorkspaceConfiguration, + oldSetting: string, + newConfig: vscode.WorkspaceConfiguration, + newSetting: string +): Promise { const oldValueInspection = oldConfig.inspect(oldSetting); const newValueInspection = newConfig.inspect('environment');