Azure key vault task #3933
Azure key vault task #3933
Conversation
|
@Lovakumar, |
| "loc.messages.ARG_UnsupportedAzurePSVersion": "未安裝 Azure Powershell Cmdlet 要求的最低版本 {0}。您可以遵循下列網址中的指示取得最新 Azure powershell: {1}", | ||
| "loc.messages.ARG_AzureRMModuleNotFound": "未安裝必要的 AzureRM Powershell 模組。您可以遵循下列網址中的指示取得最新 Azure Powershell: {0}", | ||
| "loc.messages.ARG_DeploymentFailed": "資源群組部署 '{0}' 失敗", | ||
| "loc.messages.ARG_ValidationFailed": "資源群組部署範本驗證失敗，錯誤碼為 {0}，錯誤訊息為: {1}", |
There was a problem hiding this comment.
We do not need to check-in other languages resources. Hence deleted these files.
Tasks/AzureKeyVault/icon.svg
Outdated
| version="1.1" | ||
| inkscape:version="0.91 r13725" | ||
| sodipodi:docname="icon.svg" | ||
| inkscape:export-filename="C:\Users\Jamie\Sources\vsts-tasks\Tasks\DeployAzureResourceGroup\icon.png" |
There was a problem hiding this comment.
Is this path correct? Should it have DeployARG name in it?
There was a problem hiding this comment.
icon svg is not needed. icon.png is sufficient. Hence deleted this file.
Tasks/AzureKeyVault/main.ts
Outdated
| tl.debug("Setting resource path to " + taskManifestPath); | ||
| tl.setResourcePath(taskManifestPath); | ||
|
|
||
| run().then((result) => |
| @@ -0,0 +1,45 @@ | |||
| /// <reference path="../../../definitions/node.d.ts" /> | |||
There was a problem hiding this comment.
each task needs it's own typings. see shellscript task. this causes updates needing all teams to revalidate all tasks.
| this.keyVaultUrl = util.format("https://%s.%s", this.keyVaultName, azureKeyVaultDnsSuffix); | ||
| this.vaultCredentials = this.getVaultCredentials(connectedService, azureKeyVaultDnsSuffix); | ||
| } | ||
| catch (error) { |
There was a problem hiding this comment.
you shouldn't catch in a contructor. The consumer can catch this. You're just masking what the root issue is.
| "displayName": "Azure key vault details", | ||
| "isExpanded": true | ||
| } | ||
| ], |
There was a problem hiding this comment.
Is this group required. I can see all the inputs below to see and hence doubt?
There was a problem hiding this comment.
Checked with PM - it's ok to have it.
Tasks/AzureKeyVault/task.json
Outdated
| "dataSourceName": "AzureKeyVaults" | ||
| } | ||
| ], | ||
| "sourceDefinitions": [], |
| } | ||
| }, | ||
| { | ||
| "name": "SecretsFilter", |
There was a problem hiding this comment.
For users using this from Task catelogue, there has to be help on how to access the downloaded values.
| @@ -0,0 +1,5 @@ | |||
| # Azure Key Vault Task | |||
There was a problem hiding this comment.
You would want to add L0 tests.
| }, | ||
| "demands": [], | ||
| "minimumAgentVersion": "2.0.0", | ||
| "groups": [ |
There was a problem hiding this comment.
Can we validate this in both the Agent phase and Deployment group phase.
| tl.debug(util.format("Downloading secret value for: %s", secretName)); | ||
|
|
||
| return new Promise<void>((resolve, reject) => { | ||
| // if (tl.getVariable(secretName) !== undefined) { |
There was a problem hiding this comment.
You can remove unwanted code.
| "label": "Key vault", | ||
| "required": true, | ||
| "groupName": "AzureKeyVaultDetails", | ||
| "helpMarkDown": "Provide the name of a key vault.", |
There was a problem hiding this comment.
Is there any allowed set of characters and restrictions for name. Please call that out in help. It might also be good idea to validate that in the script.
There was a problem hiding this comment.
comma is not allowed since we use that as delimiter. I will update the help text for the same.
| return reject(tl.loc("GetSecretValueFailed", secretName, this.getError(error))); | ||
| } | ||
|
|
||
| console.log("##vso[task.setvariable variable=" + secretName + ";issecret=true;]" + secretValue); |
There was a problem hiding this comment.
If secretName and secretValue can support special characters, please test if you are able to set names with those characters in environment variable.
Tasks/AzureKeyVault/package.json
Outdated
| "name": "AzureKeyVault", | ||
| "main":"main.js", | ||
| "dependencies": { | ||
| "vsts-task-lib" : "^0.9.20", |
There was a problem hiding this comment.
Tie to specific version. It will help you in controlling nested dependencies that might come along.
| var downloadAllSecrets = false; | ||
| if (this.taskParameters.secretsFilter && this.taskParameters.secretsFilter.length > 0) | ||
| { | ||
| if (this.taskParameters.secretsFilter.length === 1 && this.taskParameters.secretsFilter[0] === "*") { |
There was a problem hiding this comment.
do we need to do any trimming etc?
There was a problem hiding this comment.
input reading functions in task lib takes care of it.
| this.credentials = credentials; | ||
| this.subscriptionId = subscriptionId | ||
| this.baseUri = this.credentials.armUrl; | ||
| this.longRunningOperationRetryTimeout = 60; // In minutes |
There was a problem hiding this comment.
copied code and Sachin's team will do the refactoring. I will file a bug on them
No description provided.