DockerV2 - Fixing some issues/feeback:

- Use Rename input from dockerFile to Dockerfile - Ignore arguments input for buildAndPush and create warning instead of failing the task - Fix the code to restore earlier login and some debug statements.
microsoft · Mar 26, 2019 · 1659732 · 1659732
1 parent ab84caf
commit 1659732
Show file tree

Hide file tree

Showing 7 changed files with 45 additions and 33 deletions.
diff --git a/Tasks/Common/docker-common/containerconnection.ts b/Tasks/Common/docker-common/containerconnection.ts
@@ -153,7 +153,7 @@ export default class ContainerConnection {
                     }
                     else {
                         // trying to logout from a registry where no login was done. Nothing to be done here.
-                        tl.debug(tl.loc('RegistryAuthNotPresentInConfig', registry, existingConfigJson));
+                        tl.debug(tl.loc('RegistryAuthNotPresentInConfig', registry));
                     }
                 }
             }
@@ -247,7 +247,7 @@ export default class ContainerConnection {
                             // If auth is already present for the same registry, then cache it so that we can 
                             // preserve it back on logout. 
                             if (existingConfigJson.auths[registryName]) {
-                                this.oldDockerConfigContent = existingConfigJson;
+                                this.oldDockerConfigContent = JSON.stringify(existingConfigJson);
                                 tl.debug(tl.loc('OldDockerConfigContent', this.oldDockerConfigContent));
                             }
 
@@ -277,10 +277,19 @@ export default class ContainerConnection {
         return null;
     }
 
-    private getDockerConfigJson(configurationFilePath : string ): any {
+    private getDockerConfigJson(configurationFilePath : string): any {
+        let configJson: any;
         let dockerConfig = fs.readFileSync(configurationFilePath, "utf-8");
         tl.debug(tl.loc('FoundDockerConfigStoredInTempPath', configurationFilePath, dockerConfig));
-        return JSON.parse(dockerConfig);
+        try {
+            configJson = JSON.parse(dockerConfig);
+        }
+        catch(err) {
+            let errorMessage = tl.loc('ErrorParsingDockerConfig', err);
+            throw new Error(errorMessage);
+        }
+
+        return configJson;
     }
 
     private writeDockerConfigJson(dockerConfigContent: string, configurationFilePath?: string): void {
@@ -325,7 +334,7 @@ export default class ContainerConnection {
                 regUrls = Object.keys(existingConfigJson.auths);
             }
             else {
-                tl.debug(tl.loc('NoAuthInfoFoundInDockerConfig', existingConfigJson));
+                tl.debug(tl.loc('NoAuthInfoFoundInDockerConfig'));
             }
         }
         else {

diff --git a/Tasks/DockerV2/Strings/resources.resjson/en-US/resources.resjson b/Tasks/DockerV2/Strings/resources.resjson/en-US/resources.resjson
@@ -12,16 +12,15 @@
   "loc.input.help.repository": "Name of the repository.",
   "loc.input.label.command": "Command",
   "loc.input.help.command": "Select a Docker command.",
-  "loc.input.label.dockerFile": "Dockerfile",
-  "loc.input.help.dockerFile": "Path to the Dockerfile.",
+  "loc.input.label.Dockerfile": "Dockerfile",
+  "loc.input.help.Dockerfile": "Path to the Dockerfile.",
   "loc.input.label.buildContext": "Build context",
   "loc.input.help.buildContext": "Path to the build context. Pass ** to specify the directory that contains the Dockerfile.",
   "loc.input.label.tags": "Tags",
   "loc.input.help.tags": "A list of tags in separate lines. These tags are used in build, push and buildAndPush commands. Ex:<br><br>beta1.1<br>latest",
   "loc.input.label.arguments": "Arguments",
   "loc.input.help.arguments": "Docker command options. Ex:<br> For build command,<br>--build-arg HTTP_PROXY=http://10.20.30.2:1234 --quiet",
   "loc.messages.AddingNewAuthToExistingConfig": "Adding auth data for registry to Docker config file. Registry: %s.",
-  "loc.messages.ArgumentsNotSupportedWithBuildAndPush": "The input arguments is not supported when the command is buildAndPush",
   "loc.messages.ContainerPatternFound": "Pattern found in Docker filepath parameter",
   "loc.messages.ContainerPatternNotFound": "No pattern found in Docker filepath parameter",
   "loc.messages.ContainerDockerFileNotFound": "No Dockerfile matching  %s  was found.",
@@ -30,20 +29,22 @@
   "loc.messages.DeletingDockerConfigDirectory": "Deleting Docker config directory. Path: %s",
   "loc.messages.DeletingAuthDataFromDockerConfig": "Deleting auth data for registry from Docker config file. Registry: %s, New Docker config: %s",
   "loc.messages.DockerRegistryNotFound": "Docker registry service connection not specified.",
+  "loc.messages.ErrorParsingDockerConfig": "Could not parse the Docker config obtained from the file. Error: %s",
   "loc.messages.FileContentSynced": "Synced the file content to the disk. The content is %s.",
   "loc.messages.FoundDockerConfigStoredInTempPath": "Found the Docker Config stored in the temp path. Docker config path: %s, Docker config: %s",
   "loc.messages.FoundLoginsForOtherRegistries": "Found login info for other registry(s). Trying to remove auth from the Docker config for the registry: %s",
-  "loc.messages.OldDockerConfigContent": "Found an earlier login to the same registry. Saving old auth data and continuing with the new login. Old docker config: %s",
+  "loc.messages.IgnoringArgumentsInput": "The arguments input is not supported when the command is buildAndPush. Ignoring the input.",
   "loc.messages.LoggingOutFromRegistry": "Trying to logout from registry: %s",
   "loc.messages.LoggingOutWithNoRegistrySpecified": "Logging out. Removing all auth data from temp docker config, since no registry is specified.",
-  "loc.messages.NoAuthInfoFoundInDockerConfig": "No auths found in Docker config. Hence returning 0 registry url's. Docker config: %s",
+  "loc.messages.NoAuthInfoFoundInDockerConfig": "No auths found in Docker config. Hence returning 0 registry url's.",
   "loc.messages.NoDataWrittenOnFile": "No data was written into the file %s",
   "loc.messages.NoImagesInImageNamesFile": "At least one image name is expected in file '%s'.",
   "loc.messages.NotAddingAnyTagsToBuild": "Not adding any container registry tags to the build image as no login information was found.",
   "loc.messages.NotPushingAsNoLoginFound": "Not pushing to any registry as no login information was found.",
+  "loc.messages.OldDockerConfigContent": "Found an earlier login to the same registry. Saving old auth data and continuing with the new login. Old docker config: %s",
   "loc.messages.OutputVariableDataSizeExceeded": "Output variable not set as Docker command output exceeded the maximum supported length. Output length: %s, Maximum supported length: %s",
   "loc.messages.PathIsNotInTempDirectory": "The config path is not inside the temp directory. Config path: %s, Temp directory: %s",
-  "loc.messages.RegistryAuthNotPresentInConfig": "Could not find the auth data for registry in the Docker config file. Nothing to be done to logout. Registry: %s, Docker config: %s",
+  "loc.messages.RegistryAuthNotPresentInConfig": "Could not find the auth data for registry in the Docker config file. Nothing to be done to logout. Registry: %s",
   "loc.messages.RestoringOldLoginAuth": "Restoring the previous login auth data for the registry: %s",
   "loc.messages.WritingDockerConfigToTempFile": "Writing Docker config to temp file. File path: %s, Docker config: %s"
 }
diff --git a/Tasks/DockerV2/dockerbuild.ts b/Tasks/DockerV2/dockerbuild.ts
@@ -15,17 +15,17 @@ function useDefaultBuildContext(buildContext: string): boolean {
     return buildContext === defaultPath;
 }
 
-export function run(connection: ContainerConnection, outputUpdate: (data: string) => any): any {
+export function run(connection: ContainerConnection, outputUpdate: (data: string) => any, ignoreArguments?: boolean): any {
     // find dockerfile path
-    let dockerfilepath = tl.getInput("dockerFile", true);
+    let dockerfilepath = tl.getInput("Dockerfile", true);
     let dockerFile = fileUtils.findDockerFile(dockerfilepath);
 
     if(!tl.exist(dockerFile)) {
         throw new Error(tl.loc('ContainerDockerFileNotFound', dockerfilepath));
     }
 
     // get command arguments
-    let commandArguments = tl.getInput("arguments", false);
+    let commandArguments = ignoreArguments ? "" : tl.getInput("arguments", false);
 
     // get qualified image names by combining container registry(s) and repository
     let repositoryName = tl.getInput("repository");

diff --git a/Tasks/DockerV2/dockerbuildandpush.ts b/Tasks/DockerV2/dockerbuildandpush.ts
@@ -6,15 +6,15 @@ import ContainerConnection from "docker-common/containerconnection";
 export function run(connection: ContainerConnection, outputUpdate: (data: string) => any): any {
     let args = tl.getInput("arguments");
     if (args) {
-        throw new Error(tl.loc('ArgumentsNotSupportedWithBuildAndPush'));
+        tl.warning(tl.loc('IgnoringArgumentsInput'));
     }
 
     let dockerbuild = require("./dockerbuild");
     let dockerpush = require("./dockerpush");
 
     let outputPaths = "";
-    let promise = dockerbuild.run(connection, (outputPath) => outputPaths += outputPath).then(() => {
-        return dockerpush.run(connection, (outputPath) => outputPaths += ("\n" + outputPath)).then(() => {
+    let promise = dockerbuild.run(connection, (outputPath) => outputPaths += outputPath, true).then(() => {
+        return dockerpush.run(connection, (outputPath) => outputPaths += ("\n" + outputPath), true).then(() => {
             outputUpdate(outputPaths);
         });
     })

diff --git a/Tasks/DockerV2/dockerpush.ts b/Tasks/DockerV2/dockerpush.ts
@@ -41,8 +41,8 @@ function pushMultipleImages(connection: ContainerConnection, imageNames: string[
     return promise;
 }
 
-export function run(connection: ContainerConnection, outputUpdate: (data: string) => any): any {
-    var commandArguments = tl.getInput("arguments", false); 
+export function run(connection: ContainerConnection, outputUpdate: (data: string) => any, ignoreArguments?: boolean): any {
+    let commandArguments = ignoreArguments ? "" : tl.getInput("arguments");
 
     // get tags input
     let tags = tl.getDelimitedInput("tags", "\n");

diff --git a/Tasks/DockerV2/task.json b/Tasks/DockerV2/task.json
@@ -14,7 +14,7 @@
     "version": {
         "Major": 2,
         "Minor": 150,
-        "Patch": 0
+        "Patch": 1
     },
     "demands": [],
     "releaseNotes": "Simplified the task YAML by:<br/>&nbsp;- Removing the Container registry type input<br/>&nbsp;- Removing complex inputs as they can be passed as arguments to the command.",
@@ -70,7 +70,7 @@
             "helpMarkDown": "Select a Docker command."
         },
         {
-            "name": "dockerFile",
+            "name": "Dockerfile",
             "type": "filePath",
             "label": "Dockerfile",
             "defaultValue": "**/Dockerfile",
@@ -115,7 +115,7 @@
     "outputVariables": [
         {
             "name": "DockerOutput",
-            "description": "The path of the file(s) which contains the output of the command"
+            "description": "The path of the file(s) which contains the output of the command. This contains two file paths (separated by newline characters) in case of buildAndPush command, and one file path for any other command."
         }
     ],
     "instanceNameFormat": "$(command)",
@@ -127,7 +127,6 @@
     },
     "messages": {
         "AddingNewAuthToExistingConfig": "Adding auth data for registry to Docker config file. Registry: %s.",
-        "ArgumentsNotSupportedWithBuildAndPush": "The input arguments is not supported when the command is buildAndPush",
         "ContainerPatternFound": "Pattern found in Docker filepath parameter",
         "ContainerPatternNotFound": "No pattern found in Docker filepath parameter",
         "ContainerDockerFileNotFound": "No Dockerfile matching  %s  was found.",
@@ -136,20 +135,22 @@
         "DeletingDockerConfigDirectory": "Deleting Docker config directory. Path: %s",
         "DeletingAuthDataFromDockerConfig": "Deleting auth data for registry from Docker config file. Registry: %s, New Docker config: %s",
         "DockerRegistryNotFound": "Docker registry service connection not specified.",
+        "ErrorParsingDockerConfig": "Could not parse the Docker config obtained from the file. Error: %s",
         "FileContentSynced": "Synced the file content to the disk. The content is %s.",
         "FoundDockerConfigStoredInTempPath": "Found the Docker Config stored in the temp path. Docker config path: %s, Docker config: %s",
         "FoundLoginsForOtherRegistries": "Found login info for other registry(s). Trying to remove auth from the Docker config for the registry: %s",
-        "OldDockerConfigContent": "Found an earlier login to the same registry. Saving old auth data and continuing with the new login. Old docker config: %s",
+        "IgnoringArgumentsInput": "The arguments input is not supported when the command is buildAndPush. Ignoring the input.",
         "LoggingOutFromRegistry": "Trying to logout from registry: %s",
         "LoggingOutWithNoRegistrySpecified": "Logging out. Removing all auth data from temp docker config, since no registry is specified.",
-        "NoAuthInfoFoundInDockerConfig": "No auths found in Docker config. Hence returning 0 registry url's. Docker config: %s",
+        "NoAuthInfoFoundInDockerConfig": "No auths found in Docker config. Hence returning 0 registry url's.",
         "NoDataWrittenOnFile": "No data was written into the file %s",
         "NoImagesInImageNamesFile": "At least one image name is expected in file '%s'.",
         "NotAddingAnyTagsToBuild": "Not adding any container registry tags to the build image as no login information was found.",
         "NotPushingAsNoLoginFound": "Not pushing to any registry as no login information was found.",
+        "OldDockerConfigContent": "Found an earlier login to the same registry. Saving old auth data and continuing with the new login. Old docker config: %s",
         "OutputVariableDataSizeExceeded": "Output variable not set as Docker command output exceeded the maximum supported length. Output length: %s, Maximum supported length: %s",
         "PathIsNotInTempDirectory": "The config path is not inside the temp directory. Config path: %s, Temp directory: %s",
-        "RegistryAuthNotPresentInConfig": "Could not find the auth data for registry in the Docker config file. Nothing to be done to logout. Registry: %s, Docker config: %s",
+        "RegistryAuthNotPresentInConfig": "Could not find the auth data for registry in the Docker config file. Nothing to be done to logout. Registry: %s",
         "RestoringOldLoginAuth": "Restoring the previous login auth data for the registry: %s",
         "WritingDockerConfigToTempFile": "Writing Docker config to temp file. File path: %s, Docker config: %s"
     }

diff --git a/Tasks/DockerV2/task.loc.json b/Tasks/DockerV2/task.loc.json
@@ -14,7 +14,7 @@
   "version": {
     "Major": 2,
     "Minor": 150,
-    "Patch": 0
+    "Patch": 1
   },
   "demands": [],
   "releaseNotes": "ms-resource:loc.releaseNotes",
@@ -70,14 +70,14 @@
       "helpMarkDown": "ms-resource:loc.input.help.command"
     },
     {
-      "name": "dockerFile",
+      "name": "Dockerfile",
       "type": "filePath",
-      "label": "ms-resource:loc.input.label.dockerFile",
+      "label": "ms-resource:loc.input.label.Dockerfile",
       "defaultValue": "**/Dockerfile",
       "required": true,
       "visibleRule": "command = build || command = buildAndPush",
       "groupName": "commands",
-      "helpMarkDown": "ms-resource:loc.input.help.dockerFile"
+      "helpMarkDown": "ms-resource:loc.input.help.Dockerfile"
     },
     {
       "name": "buildContext",
@@ -114,7 +114,7 @@
   "outputVariables": [
     {
       "name": "DockerOutput",
-      "description": "The path of the file(s) which contains the output of the command"
+      "description": "The path of the file(s) which contains the output of the command. This contains two file paths (separated by newline characters) in case of buildAndPush command, and one file path for any other command."
     }
   ],
   "instanceNameFormat": "ms-resource:loc.instanceNameFormat",
@@ -126,7 +126,6 @@
   },
   "messages": {
     "AddingNewAuthToExistingConfig": "ms-resource:loc.messages.AddingNewAuthToExistingConfig",
-    "ArgumentsNotSupportedWithBuildAndPush": "ms-resource:loc.messages.ArgumentsNotSupportedWithBuildAndPush",
     "ContainerPatternFound": "ms-resource:loc.messages.ContainerPatternFound",
     "ContainerPatternNotFound": "ms-resource:loc.messages.ContainerPatternNotFound",
     "ContainerDockerFileNotFound": "ms-resource:loc.messages.ContainerDockerFileNotFound",
@@ -135,17 +134,19 @@
     "DeletingDockerConfigDirectory": "ms-resource:loc.messages.DeletingDockerConfigDirectory",
     "DeletingAuthDataFromDockerConfig": "ms-resource:loc.messages.DeletingAuthDataFromDockerConfig",
     "DockerRegistryNotFound": "ms-resource:loc.messages.DockerRegistryNotFound",
+    "ErrorParsingDockerConfig": "ms-resource:loc.messages.ErrorParsingDockerConfig",
     "FileContentSynced": "ms-resource:loc.messages.FileContentSynced",
     "FoundDockerConfigStoredInTempPath": "ms-resource:loc.messages.FoundDockerConfigStoredInTempPath",
     "FoundLoginsForOtherRegistries": "ms-resource:loc.messages.FoundLoginsForOtherRegistries",
-    "OldDockerConfigContent": "ms-resource:loc.messages.OldDockerConfigContent",
+    "IgnoringArgumentsInput": "ms-resource:loc.messages.IgnoringArgumentsInput",
     "LoggingOutFromRegistry": "ms-resource:loc.messages.LoggingOutFromRegistry",
     "LoggingOutWithNoRegistrySpecified": "ms-resource:loc.messages.LoggingOutWithNoRegistrySpecified",
     "NoAuthInfoFoundInDockerConfig": "ms-resource:loc.messages.NoAuthInfoFoundInDockerConfig",
     "NoDataWrittenOnFile": "ms-resource:loc.messages.NoDataWrittenOnFile",
     "NoImagesInImageNamesFile": "ms-resource:loc.messages.NoImagesInImageNamesFile",
     "NotAddingAnyTagsToBuild": "ms-resource:loc.messages.NotAddingAnyTagsToBuild",
     "NotPushingAsNoLoginFound": "ms-resource:loc.messages.NotPushingAsNoLoginFound",
+    "OldDockerConfigContent": "ms-resource:loc.messages.OldDockerConfigContent",
     "OutputVariableDataSizeExceeded": "ms-resource:loc.messages.OutputVariableDataSizeExceeded",
     "PathIsNotInTempDirectory": "ms-resource:loc.messages.PathIsNotInTempDirectory",
     "RegistryAuthNotPresentInConfig": "ms-resource:loc.messages.RegistryAuthNotPresentInConfig",