From aa4cb1248aac6f8615fec9fa217ae04caffeb9cc Mon Sep 17 00:00:00 2001 From: Dennis Seah Date: Thu, 2 Apr 2020 16:23:00 -0700 Subject: [PATCH] [FEATURE] Inherit value from config (#488) * [FEATURE] Automate inheriting values into option values from config.yaml * Update commandBuilder.ts --- docs/commands/data.json | 6 + docs/commands/spk.js | 64 +++++---- docs/commands/styles/spk.css | 4 + .../deployment/onboard.decorator.json | 6 + src/commands/deployment/onboard.ts | 31 +---- src/lib/commandBuilder.test.ts | 127 ++++++++++++++++++ src/lib/commandBuilder.ts | 108 +++++++++++++-- 7 files changed, 278 insertions(+), 68 deletions(-) diff --git a/docs/commands/data.json b/docs/commands/data.json index c1db30894..0d4f3302e 100644 --- a/docs/commands/data.json +++ b/docs/commands/data.json @@ -175,11 +175,13 @@ { "arg": "-s, --storage-account-name ", "description": "Azure storage account name", + "inherit": "introspection.azure.account_name", "required": true }, { "arg": "-t, --storage-table-name ", "description": "Azure storage table name", + "inherit": "introspection.azure.table_name", "required": true }, { @@ -194,21 +196,25 @@ { "arg": "--service-principal-id ", "description": "Azure service principal id with `contributor` role in Azure Resource Group", + "inherit": "introspection.azure.service_principal_id", "required": true }, { "arg": "--service-principal-password ", "description": "The Azure service principal password", + "inherit": "introspection.azure.service_principal_secret", "required": true }, { "arg": "--tenant-id ", "description": "The Azure AD tenant id of service principal", + "inherit": "introspection.azure.tenant_id", "required": true }, { "arg": "--subscription-id ", "description": "The Azure subscription id", + "inherit": "introspection.azure.subscription_id", "required": true } ], diff --git a/docs/commands/spk.js b/docs/commands/spk.js index 4184549cd..1ed970957 100644 --- a/docs/commands/spk.js +++ b/docs/commands/spk.js @@ -1,3 +1,4 @@ +/* eslint-disable */ var data = null; var filter = ""; var converter = new showdown.Converter(); @@ -8,7 +9,9 @@ var sepVersion = "@"; var template = '

@@main-cmd@@

@@cmd-description@@

 

Options:

@@options@@

 

'; var optionTemplate = - '

@@option@@

@@description@@

'; + '

@@option@@

@@description@@

@@inherit@@
'; +var inheritTemplate = + '

inherit @@inherit@@ from spk config.yaml

'; var relTemplate = '
  • @@value@@
    • '; @@ -19,18 +22,18 @@ function sanitize(str) { function getExistingVersions() { $.ajax({ url: "releases.txt", - success: function(result) { - result.split("\n").forEach(function(r) { + success: function (result) { + result.split("\n").forEach(function (r) { var rTrim = r.trim(); if (rTrim && releases.indexOf(rTrim) === -1) { releases.push(rTrim); } }); - releases.sort(function(a, b) { + releases.sort(function (a, b) { return a > b ? -1 : 1; }); }, - async: false + async: false, }); } @@ -56,8 +59,8 @@ function populateVersionList() { return a + relTemplate.replace("@@value@@", c); }, "") ); - oSelect.find("li").each(function(i, elm) { - $(elm).on("click", function(evt) { + oSelect.find("li").each(function (i, elm) { + $(elm).on("click", function (evt) { evt.stopPropagation(); oSelect.css("display", "none"); var ver = $(this).text(); @@ -91,15 +94,27 @@ function showDetails(key) { ); content = content.replace("@@cmd-description@@", cmd.description); - var options = (cmd.options || []).reduce(function(a, c) { - a += optionTemplate + var options = (cmd.options || []).reduce(function (a, c) { + var o = optionTemplate .replace("@@option@@", sanitize(c.arg)) .replace("@@description@@", sanitize(c.description)); + + if (c.inherit) { + o = o.replace( + "@@inherit@@", + inheritTemplate.replace("@@inherit@@", c.inherit) + ); + } else { + o = o.replace("@@inherit@@", ""); + } + + a += o; return a; }, ""); options += optionTemplate .replace("@@option@@", "-h, --help") - .replace("@@description@@", "output usage information"); + .replace("@@description@@", "output usage information") + .replace("@@inherit@@", ""); content = content.replace("@@options@@", options); @@ -121,11 +136,11 @@ function showDetails(key) { function populateListing() { var cmdKeys = Object.keys(data); if (filter) { - cmdKeys = cmdKeys.filter(function(k) { + cmdKeys = cmdKeys.filter(function (k) { return k.indexOf(filter) !== -1; }); } - var listing = cmdKeys.reduce(function(a, c) { + var listing = cmdKeys.reduce(function (a, c) { a += "
  • ", "description": "Azure storage account name", + "inherit": "introspection.azure.account_name", "required": true }, { "arg": "-t, --storage-table-name ", "description": "Azure storage table name", + "inherit": "introspection.azure.table_name", "required": true }, { @@ -25,21 +27,25 @@ { "arg": "--service-principal-id ", "description": "Azure service principal id with `contributor` role in Azure Resource Group", + "inherit": "introspection.azure.service_principal_id", "required": true }, { "arg": "--service-principal-password ", "description": "The Azure service principal password", + "inherit": "introspection.azure.service_principal_secret", "required": true }, { "arg": "--tenant-id ", "description": "The Azure AD tenant id of service principal", + "inherit": "introspection.azure.tenant_id", "required": true }, { "arg": "--subscription-id ", "description": "The Azure subscription id", + "inherit": "introspection.azure.subscription_id", "required": true } ] diff --git a/src/commands/deployment/onboard.ts b/src/commands/deployment/onboard.ts index 974eeb96b..f5ba550fc 100644 --- a/src/commands/deployment/onboard.ts +++ b/src/commands/deployment/onboard.ts @@ -12,6 +12,7 @@ import { import { build as buildCmd, exit as exitCmd, + populateInheritValueFromConfig, validateForRequiredValues, } from "../../lib/commandBuilder"; import { logger } from "../../logger"; @@ -50,22 +51,7 @@ export interface OnBoardConfig { * @param opts values from commander */ export const populateValues = (opts: CommandOptions): CommandOptions => { - const config = Config(); - const azure = config.introspection ? config.introspection.azure : undefined; - - opts.storageAccountName = - opts.storageAccountName || azure?.account_name || undefined; - opts.storageTableName = - opts.storageTableName || azure?.table_name || undefined; - opts.servicePrincipalId = - opts.servicePrincipalId || azure?.service_principal_id || undefined; - opts.servicePrincipalPassword = - opts.servicePrincipalPassword || - azure?.service_principal_secret || - undefined; - opts.tenantId = opts.tenantId || azure?.tenant_id || undefined; - opts.subscriptionId = - opts.subscriptionId || azure?.subscription_id || undefined; + populateInheritValueFromConfig(decorator, Config(), opts); return opts; }; @@ -75,18 +61,7 @@ export const populateValues = (opts: CommandOptions): CommandOptions => { * @param opts values from commander (including populated values from spk config) */ export const validateValues = (opts: CommandOptions): OnBoardConfig => { - const errors = validateForRequiredValues(decorator, { - servicePrincipalId: opts.servicePrincipalId, - servicePrincipalPassword: opts.servicePrincipalPassword, - storageAccountName: opts.storageAccountName, - storageResourceGroupName: opts.storageResourceGroupName, - storageTableName: opts.storageTableName, - subscriptionId: opts.subscriptionId, - tenantId: opts.tenantId, - }); - if (errors.length > 0) { - throw Error("Required values are missing"); - } + validateForRequiredValues(decorator, opts, true); // validateForRequiredValues already check // opts.storageAccountName and opts.storageTableName are not empty string diff --git a/src/lib/commandBuilder.test.ts b/src/lib/commandBuilder.test.ts index 98168f678..5305b87a1 100644 --- a/src/lib/commandBuilder.test.ts +++ b/src/lib/commandBuilder.test.ts @@ -4,9 +4,11 @@ import { createLogger } from "winston"; jest.mock("fs"); import { logger } from "../logger"; import { + argToVariableName, build, exit as exitCmd, CommandBuildElements, + populateInheritValueFromConfig, validateForRequiredValues, } from "./commandBuilder"; @@ -16,6 +18,131 @@ interface CommandOption { defaultValue: string | boolean; } +describe("test argToVariableName function", () => { + it("positive test", () => { + const name = argToVariableName({ + arg: "--test-option", + description: "test", + }); + expect(name).toBe("testOption"); + }); + it("positive test", () => { + expect(() => { + argToVariableName({ + arg: "-test-option", + description: "test", + }); + }).toThrow("Could locate option name -test-option"); + }); +}); + +describe("test populateInheritValueFromConfig function", () => { + it("positive test: value = undefined and no inherit match", () => { + const opts = { + testOption: undefined, + }; + populateInheritValueFromConfig( + { + command: "test", + alias: "t", + description: "description of test", + options: [ + { + arg: "--test-option", + description: "test", + inherit: "introspection.test", + }, + ], + // eslint-disable-next-line @typescript-eslint/no-explicit-any + }, + {} as any, + opts + ); + expect(opts.testOption).toBeUndefined(); + }); + it("positive test: value = undefined and inherit match", () => { + const opts = { + testOption: undefined, + }; + populateInheritValueFromConfig( + { + command: "test", + alias: "t", + description: "description of test", + options: [ + { + arg: "--test-option", + description: "test", + inherit: "introspection.test", + }, + ], + }, + { + introspection: { + test: "test-value", + }, + // eslint-disable-next-line @typescript-eslint/no-explicit-any + } as any, + opts + ); + expect(opts.testOption).toBe("test-value"); + }); + it("negative test: incorrect inherit value", () => { + const opts = { + testOption: undefined, + }; + populateInheritValueFromConfig( + { + command: "test", + alias: "t", + description: "description of test", + options: [ + { + arg: "--test-option", + description: "test", + inherit: "introspection.testx", + }, + ], + }, + { + introspection: { + test: "test-value", + }, + // eslint-disable-next-line @typescript-eslint/no-explicit-any + } as any, + opts + ); + expect(opts.testOption).toBeUndefined(); + }); + it("negative test: incorrect inherit value: object", () => { + const opts = { + testOption: undefined, + }; + populateInheritValueFromConfig( + { + command: "test", + alias: "t", + description: "description of test", + options: [ + { + arg: "--test-option", + description: "test", + inherit: "introspection", + }, + ], + }, + { + introspection: { + test: "test-value", + }, + // eslint-disable-next-line @typescript-eslint/no-explicit-any + } as any, + opts + ); + expect(opts.testOption).toBeUndefined(); + }); +}); + describe("Tests Command Builder's build function", () => { it("Declaration with no options", () => { const descriptor: CommandBuildElements = { diff --git a/src/lib/commandBuilder.ts b/src/lib/commandBuilder.ts index 2172cc2b4..992768407 100644 --- a/src/lib/commandBuilder.ts +++ b/src/lib/commandBuilder.ts @@ -2,6 +2,7 @@ import commander from "commander"; import fs from "fs"; import { Logger, transports } from "winston"; import { logger } from "../logger"; +import { ConfigYaml } from "../types"; import { hasValue } from "./validator"; /** @@ -10,6 +11,7 @@ import { hasValue } from "./validator"; export interface CommandOption { arg: string; description: string; + inherit?: string; required?: boolean; defaultValue?: string | boolean; } @@ -30,6 +32,24 @@ interface CommandVariableName2Opt { variableName: string; } +/** + * Returns variable name associated with an option name. + * e.g. servicePrincipalId for --service-principal-id + * + * @param opt Command option + */ +export const argToVariableName = (opt: CommandOption): string => { + const match = opt.arg.match(/\s?--([-\w]+)\s?/); + if (match) { + return match[1] + .replace(/\.?(-[a-z])/g, (_, y) => { + return y.toUpperCase(); + }) + .replace(/-/g, ""); + } + throw Error(`Could locate option name ${opt.arg}`); +}; + /** * Builds a command * @@ -63,11 +83,13 @@ export const build = ( * * @param decorator Descriptor for command building * @param values Values to be inspected. + * @param toThrow Throw exception if there are validation error. * @return error messages. */ export const validateForRequiredValues = ( decorator: CommandBuildElements, - values: { [key: string]: string | undefined } + opts: unknown, + toThrow = false ): string[] => { // gather the required options const required = (decorator.options || []).filter((opt) => opt.required); @@ -77,23 +99,16 @@ export const validateForRequiredValues = ( return []; } + const values = opts as CommandValues; + // opt name to variable name mapping // example --org-name is orgName const mapVariableName2Opt: CommandVariableName2Opt[] = []; required.forEach((opt) => { - const match = opt.arg.match(/\s?--([-\w]+)\s?/); - - if (match) { - const variableName = match[1] - .replace(/\.?(-[a-z])/g, (_, y) => { - return y.toUpperCase(); - }) - .replace(/-/g, ""); - mapVariableName2Opt.push({ - opt, - variableName, - }); - } + mapVariableName2Opt.push({ + opt, + variableName: argToVariableName(opt), + }); }); // figure out which variables have missing values @@ -104,6 +119,10 @@ export const validateForRequiredValues = ( // gather the option flags (args) for the missing one const errors = missingItems.map((item) => item.opt.arg); + if (toThrow && errors.length !== 0) { + throw `The following arguments are required: ${errors.join("\n ")}`; + } + if (errors.length !== 0) { logger.error(`the following arguments are required: ${errors.join("\n ")}`); } @@ -157,6 +176,14 @@ export const exit = ( }); }; +/** + * Returns the command option that match an variable name. + * e.g. servicePrincipalId, the option that has name + * --service-principal-id shall be returned + * + * @param decorator command decorator + * @param name variable name + */ export const getOption = ( decorator: CommandBuildElements, name: string @@ -166,3 +193,56 @@ export const getOption = ( return match && match[1] === name; }); }; + +interface ConfigValues { + [key: string]: string | ConfigValues | undefined; +} + +interface CommandValues { + [key: string]: string | undefined; +} + +/** + * Populates (inherit) values from config.yaml of user does + * not provide values to inheritable options. + * + * @param decorator command decorator + * @param config config YAML + * @param opts option values provided by user. + */ +export const populateInheritValueFromConfig = ( + decorator: CommandBuildElements, + config: ConfigYaml, + opts: unknown +): void => { + if (config && decorator.options) { + const mapOpts = opts as CommandValues; + decorator.options + .filter((o) => !!o.inherit) + .forEach((option) => { + const name = argToVariableName(option); + + // skip if the option already has value provided. + // that's do not need to inherit from config.yaml + if (!hasValue(mapOpts[name])) { + let cfg: ConfigValues | string | undefined = config as ConfigValues; + + // .filter((o) => !!o.inherit) already check that option.inherit is + // defined + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + const arr = option.inherit!.split("."); + + // search for value in config.yaml + while (cfg && typeof config === "object" && arr.length > 0) { + const k = arr.shift(); + if (k) { + cfg = (cfg as ConfigValues)[k]; + } + } + if (typeof cfg === "string") { + mapOpts[name] = cfg; + } + } + }); + } +};