From dc8fd3de625e91121dc71418d93464527507607c Mon Sep 17 00:00:00 2001 From: Dennis Seah Date: Fri, 3 Apr 2020 01:21:12 -0700 Subject: [PATCH 1/2] [FEATURE] Simplify inheritance of option vals for project create variable group cmd --- .../create-variable-group.decorator.json | 12 +- .../project/create-variable-group.test.ts | 56 ++++---- src/commands/project/create-variable-group.ts | 133 +++++++----------- 3 files changed, 84 insertions(+), 117 deletions(-) diff --git a/src/commands/project/create-variable-group.decorator.json b/src/commands/project/create-variable-group.decorator.json index 4fa71583f..283ba4ca8 100644 --- a/src/commands/project/create-variable-group.decorator.json +++ b/src/commands/project/create-variable-group.decorator.json @@ -11,7 +11,8 @@ { "arg": "-U, --hld-repo-url ", "description": "The high level definition (HLD) git repo url; falls back to azure_devops.org in spk config.", - "required": true + "required": true, + "inherit": "azure_devops.hld_repository" }, { "arg": "-u, --service-principal-id ", @@ -31,17 +32,20 @@ { "arg": "-o, --org-name ", "description": "Azure DevOps organization name; falls back to azure_devops.org in spk config.", - "required": true + "required": true, + "inherit": "azure_devops.org" }, { "arg": "-d, --devops-project ", "description": "Azure DevOps project name; falls back to azure_devops.project in spk config.", - "required": true + "required": true, + "inherit": "azure_devops.project" }, { "arg": "-a, --personal-access-token ", "description": "Azure DevOps Personal access token; falls back to azure_devops.access_token in spk config.", - "required": true + "required": true, + "inherit": "azure_devops.access_token" } ] } diff --git a/src/commands/project/create-variable-group.test.ts b/src/commands/project/create-variable-group.test.ts index 14b4e1e5e..77a8c2c06 100644 --- a/src/commands/project/create-variable-group.test.ts +++ b/src/commands/project/create-variable-group.test.ts @@ -15,7 +15,6 @@ import { PROJECT_PIPELINE_FILENAME, VERSION_MESSAGE, } from "../../lib/constants"; -import { AzureDevOpsOpts } from "../../lib/git"; import { createTempDir } from "../../lib/ioUtil"; import * as pipelineVariableGroup from "../../lib/pipelines/variableGroup"; import { @@ -29,6 +28,7 @@ import { } from "../../test/mockFactory"; import { AzurePipelinesYaml, BedrockFile } from "../../types"; import { + CommandOptions, create, execute, setVariableGroupInBedrockFile, @@ -37,6 +37,7 @@ import { } from "./create-variable-group"; import * as createVariableGrp from "./create-variable-group"; import * as fileutils from "../../lib/fileutils"; +import { deepClone } from "../../lib/util"; beforeAll(() => { enableVerboseLogging(); @@ -140,22 +141,6 @@ describe("test execute function", () => { }); describe("create", () => { - it("Should fail with empty variable group arguments", async () => { - const accessOpts: AzureDevOpsOpts = { - orgName, - personalAccessToken, - project: devopsProject, - }; - - // for some reasons, cannot get the await expect(...).rejects.toThrow() to work - try { - await create("", "", "", "", "", "", accessOpts); - expect(true).toBeFalsy(); - } catch (e) { - expect(e.message).toBe("Required values were missing"); - } - }); - test("Should pass with variable group arguments", async () => { // mock the function that calls the Azdo project's Task API // because unit test is unable to reach this API. @@ -169,23 +154,18 @@ describe("create", () => { return Promise.resolve({}); }); - const accessOpts: AzureDevOpsOpts = { - orgName, - personalAccessToken, - project: devopsProject, - }; - try { logger.info("calling create"); - await create( - variableGroupName, + await create(variableGroupName, { registryName, hldRepoUrl, servicePrincipalId, servicePrincipalPassword, tenant, - accessOpts - ); + orgName, + devopsProject, + personalAccessToken, + }); } catch (err) { // should not reach here expect(true).toBe(false); @@ -397,18 +377,34 @@ describe("updateLifeCyclePipeline", () => { }); }); +const mockConfigValues: CommandOptions = { + hldRepoUrl, + orgName, + personalAccessToken, + devopsProject, + registryName, + servicePrincipalId, + servicePrincipalPassword, + tenant, +}; + describe("test validateValues function", () => { it("valid org and project name", () => { - validateValues(devopsProject, orgName); + const data = deepClone(mockConfigValues); + validateValues(data); }); it("invalid project name", () => { + const data = deepClone(mockConfigValues); + data.devopsProject = "project\\abc"; expect(() => { - validateValues("project\\abc", orgName); + validateValues(data); }).toThrow(); }); it("invalid org name", () => { + const data = deepClone(mockConfigValues); + data.orgName = "org name"; expect(() => { - validateValues(devopsProject, "org name"); + validateValues(data); }).toThrow(); }); }); diff --git a/src/commands/project/create-variable-group.ts b/src/commands/project/create-variable-group.ts index 8df593a64..00f16d9d6 100644 --- a/src/commands/project/create-variable-group.ts +++ b/src/commands/project/create-variable-group.ts @@ -7,13 +7,13 @@ import { fileInfo as bedrockFileInfo } from "../../lib/bedrockYaml"; import { build as buildCmd, exit as exitCmd, + populateInheritValueFromConfig, validateForRequiredValues, } from "../../lib/commandBuilder"; import { PROJECT_INIT_DEPENDENCY_ERROR_MESSAGE, PROJECT_PIPELINE_FILENAME, } from "../../lib/constants"; -import { AzureDevOpsOpts } from "../../lib/git"; import { addVariableGroup } from "../../lib/pipelines/variableGroup"; import { hasValue, @@ -30,7 +30,7 @@ import { import decorator from "./create-variable-group.decorator.json"; // values that we need to pull out from command operator -interface CommandOptions { +export interface CommandOptions { registryName: string | undefined; servicePrincipalId: string | undefined; servicePrincipalPassword: string | undefined; @@ -41,6 +41,17 @@ interface CommandOptions { devopsProject: string | undefined; } +interface ConfigValues { + hldRepoUrl: string; + orgName: string; + personalAccessToken: string; + devopsProject: string; + registryName: string; + servicePrincipalId: string; + servicePrincipalPassword: string; + tenant: string; +} + export const checkDependencies = (projectPath: string): void => { const fileInfo: BedrockFileInfo = bedrockFileInfo(projectPath); if (fileInfo.exist === false) { @@ -48,11 +59,6 @@ export const checkDependencies = (projectPath: string): void => { } }; -export const validateValues = (projectName: string, orgName: string): void => { - validateProjectNameThrowable(projectName); - validateOrgNameThrowable(orgName); -}; - /** * Creates a Azure DevOps variable group * @@ -66,49 +72,34 @@ export const validateValues = (projectName: string, orgName: string): void => { */ export const create = ( variableGroupName: string, - registryName: string | undefined, - hldRepoUrl: string | undefined, - servicePrincipalId: string | undefined, - servicePrincipalPassword: string | undefined, - tenantId: string | undefined, - accessOpts: AzureDevOpsOpts + values: ConfigValues ): Promise => { logger.info( `Creating Variable Group from group definition '${variableGroupName}'` ); - if ( - !registryName || - !hldRepoUrl || - !servicePrincipalId || - !servicePrincipalPassword || - !tenantId - ) { - throw Error("Required values were missing"); - } - const vars: VariableGroupDataVariable = { ACR_NAME: { - value: registryName, + value: values.registryName, }, HLD_REPO: { - value: hldRepoUrl, + value: values.hldRepoUrl, }, PAT: { isSecret: true, - value: accessOpts.personalAccessToken, + value: values.personalAccessToken, }, SP_APP_ID: { isSecret: true, - value: servicePrincipalId, + value: values.servicePrincipalId, }, SP_PASS: { isSecret: true, - value: servicePrincipalPassword, + value: values.servicePrincipalPassword, }, SP_TENANT: { isSecret: true, - value: tenantId, + value: values.tenant, }, }; const variableGroupData: VariableGroupData = { @@ -117,7 +108,11 @@ export const create = ( type: "Vsts", variables: vars, }; - return addVariableGroup(variableGroupData, accessOpts); + return addVariableGroup(variableGroupData, { + orgName: values.orgName, + personalAccessToken: values.personalAccessToken, + project: values.devopsProject, + }); }; /** @@ -143,7 +138,7 @@ export const setVariableGroupInBedrockFile = ( // Get bedrock.yaml const bedrockFile = Bedrock(rootProjectPath); - if (typeof bedrockFile === "undefined") { + if (!bedrockFile) { throw Error(`Bedrock file does not exist.`); } @@ -182,7 +177,7 @@ export const updateLifeCyclePipeline = (rootProjectPath: string): void => { path.join(absProjectRoot, fileName) ) as AzurePipelinesYaml; - if (typeof pipelineFile === "undefined") { + if (!pipelineFile) { throw new Error("${fileName} file does not exist in ${absProjectRoot}."); } @@ -206,6 +201,27 @@ export const updateLifeCyclePipeline = (rootProjectPath: string): void => { write(pipelineFile, absProjectRoot, fileName); }; +export const validateValues = (opts: CommandOptions): ConfigValues => { + populateInheritValueFromConfig(decorator, Config(), opts); + validateForRequiredValues(decorator, opts, true); + + // validateForRequiredValues already check required values + // || "" is just to satisfy eslint rule. + validateProjectNameThrowable(opts.devopsProject || ""); + validateOrgNameThrowable(opts.orgName || ""); + + return { + hldRepoUrl: opts.hldRepoUrl || "", + orgName: opts.orgName || "", + personalAccessToken: opts.personalAccessToken || "", + devopsProject: opts.devopsProject || "", + registryName: opts.registryName || "", + servicePrincipalId: opts.servicePrincipalId || "", + servicePrincipalPassword: opts.servicePrincipalPassword || "", + tenant: opts.tenant || "", + }; +}; + /** * Executes the command. * @@ -227,58 +243,9 @@ export const execute = async ( logger.verbose(`project path: ${projectPath}`); checkDependencies(projectPath); + const values = validateValues(opts); - const { azure_devops } = Config(); - - const { - registryName, - servicePrincipalId, - servicePrincipalPassword, - tenant, - hldRepoUrl = azure_devops?.hld_repository, - orgName = azure_devops?.org, - personalAccessToken = azure_devops?.access_token, - devopsProject = azure_devops?.project, - } = opts; - - const accessOpts: AzureDevOpsOpts = { - orgName, - personalAccessToken, - project: devopsProject, - }; - - logger.debug(`access options: ${JSON.stringify(accessOpts)}`); - - const errors = validateForRequiredValues(decorator, { - devopsProject, - hldRepoUrl, - orgName, - personalAccessToken, - registryName, - servicePrincipalId, - servicePrincipalPassword, - tenant, - }); - - if (errors.length !== 0) { - await exitFn(1); - return; - } - - // validateForRequiredValues assure that devopsProject - // and orgName are not empty string - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - validateValues(devopsProject!, orgName!); - - const variableGroup = await create( - variableGroupName, - registryName, - hldRepoUrl, - servicePrincipalId, - servicePrincipalPassword, - tenant, - accessOpts - ); + const variableGroup = await create(variableGroupName, values); // set the variable group name // variableGroup.name is set at this point that's it should have value From 421cfe0e1d86d9f636cec2306fc12061f292cee0 Mon Sep 17 00:00:00 2001 From: Dennis Seah Date: Mon, 6 Apr 2020 17:00:44 -0700 Subject: [PATCH 2/2] Update i18n.json --- docs/commands/data.json | 12 ++++++++---- src/lib/i18n.json | 2 +- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/docs/commands/data.json b/docs/commands/data.json index 0b25ba6a0..9a808f354 100644 --- a/docs/commands/data.json +++ b/docs/commands/data.json @@ -399,7 +399,8 @@ { "arg": "-U, --hld-repo-url ", "description": "The high level definition (HLD) git repo url; falls back to azure_devops.org in spk config.", - "required": true + "required": true, + "inherit": "azure_devops.hld_repository" }, { "arg": "-u, --service-principal-id ", @@ -419,17 +420,20 @@ { "arg": "-o, --org-name ", "description": "Azure DevOps organization name; falls back to azure_devops.org in spk config.", - "required": true + "required": true, + "inherit": "azure_devops.org" }, { "arg": "-d, --devops-project ", "description": "Azure DevOps project name; falls back to azure_devops.project in spk config.", - "required": true + "required": true, + "inherit": "azure_devops.project" }, { "arg": "-a, --personal-access-token ", "description": "Azure DevOps Personal access token; falls back to azure_devops.access_token in spk config.", - "required": true + "required": true, + "inherit": "azure_devops.access_token" } ], "markdown": "## Description\n\nCreate new variable group in Azure DevOps project\n\n## Command Prerequisites\n\nIn addition to an existing\n[Azure DevOps project](https://azure.microsoft.com/en-us/services/devops/), to\nlink secrets from an Azure key vault as variables in Variable Group, you will\nneed an existing key vault containing your secrets and the Service Principal for\nauthorization with Azure Key Vault.\n\n1. Use existng or\n [create a service principal either in Azure Portal](https://docs.microsoft.com/en-us/azure/active-directory/develop/howto-create-service-principal-portal)\n or\n [with Azure CLI](https://docs.microsoft.com/en-us/cli/azure/create-an-azure-service-principal-azure-cli?view=azure-cli-latest).\n2. Use existing or\n [create a Azure Container Registry in Azure Portal](https://docs.microsoft.com/en-us/azure/container-registry/container-registry-get-started-portal)\n or\n [with Azure CLI](https://docs.microsoft.com/en-us/azure/container-registry/container-registry-get-started-azure-cli).\n" diff --git a/src/lib/i18n.json b/src/lib/i18n.json index fa25f8ed4..dade3410d 100644 --- a/src/lib/i18n.json +++ b/src/lib/i18n.json @@ -229,7 +229,7 @@ "project-create-variable-group-cmd-err-bedrock-file-missing": "Bedrock file does not exist. Check that the project directory has been initialized.", "project-create-variable-group-cmd-err-file-missing": "The file '{0}' does not exist in project '{1}'.", "project-create-variable-group-cmd-err-dependency": "Please run `spk project init` command before running this command to initialize the project.", - "project-create-variable-group-cmd-err-values-missing": "Required values were missing. Provide values for registry-name, hld-repo-url, service-principal-id, service-principal-password, tenant.", + "project-init-cmd-failed": "Project init was not successfully executed.", "validation-err-missing-vals": "These mandatory options were missing:\n {0}. Provide them.",