From d651a5b11badd32b8b7701d6c9a6fec51002d518 Mon Sep 17 00:00:00 2001 From: Dennis Seah Date: Thu, 26 Mar 2020 10:23:11 -0700 Subject: [PATCH 1/3] [HOUSEKEEPING] Remove/Comment Out resources related to key vault for introspection --- guides/service-introspection.md | 17 +--- .../deployment/onboard.decorator.json | 4 - src/commands/deployment/onboard.test.ts | 80 +++++++++---------- src/commands/deployment/onboard.ts | 74 +++++++---------- 4 files changed, 72 insertions(+), 103 deletions(-) diff --git a/guides/service-introspection.md b/guides/service-introspection.md index 7f4bf5c1c..4f20c4229 100644 --- a/guides/service-introspection.md +++ b/guides/service-introspection.md @@ -44,22 +44,11 @@ introspection: table_name: "table-name" partition_key: "partition-key" key: "storage-access-key" - service_principal_id: "service-principal-id" - service_principal_secret: "service-principal-secret" - subscription_id: "subscription-id" - tenant_id: "tenant-id" - resource-group: "resource-group-name" ``` -## Prerequisites - -1. Service principal with owner access. - [Create a service principal with owner access.](#service-principal) -2. Optionally, Azure Key Vault can be used to securely store and tightly control - access to tokens, passwords, API keys, and other secrets - [How to create key vault](https://docs.microsoft.com/en-us/azure/key-vault/quick-create-cli). -3. Give the service principal get and list access. Follow step 2 from - [these instructions](https://docs.microsoft.com/en-us/azure/devops/pipelines/library/variable-groups?view=azure-devops&tabs=yaml#link-secrets-from-an-azure-key-vault). +To create storage-account and table, use the `spk deployment onboard` command to +create them where subscription Id, resource group name, service principal Id, +password and tenant Id are required. ### Service Principal diff --git a/src/commands/deployment/onboard.decorator.json b/src/commands/deployment/onboard.decorator.json index c99eecce1..963afabc1 100644 --- a/src/commands/deployment/onboard.decorator.json +++ b/src/commands/deployment/onboard.decorator.json @@ -22,10 +22,6 @@ "description": "Name of the resource group to create new storage account when it does not exist", "required": true }, - { - "arg": "-k, --key-vault-name ", - "description": "Name of the Azure key vault" - }, { "arg": "--service-principal-id ", "description": "Azure service principal id with `contributor` role in Azure Resource Group", diff --git a/src/commands/deployment/onboard.test.ts b/src/commands/deployment/onboard.test.ts index a15b7c059..c4b519f69 100644 --- a/src/commands/deployment/onboard.test.ts +++ b/src/commands/deployment/onboard.test.ts @@ -4,7 +4,7 @@ import yaml from "js-yaml"; import * as path from "path"; import { Config, loadConfiguration } from "../../config"; import * as config from "../../config"; -import * as keyvault from "../../lib/azure/keyvault"; +// import * as keyvault from "../../lib/azure/keyvault"; import * as storage from "../../lib/azure/storage"; import { createTempDir } from "../../lib/ioUtil"; import { deepClone } from "../../lib/util"; @@ -15,7 +15,7 @@ import { } from "../../logger"; import { AzureAccessOpts, ConfigYaml } from "../../types"; import { - createKeyVault, + // createKeyVault, execute, getStorageAccessKey, CommandOptions, @@ -37,7 +37,7 @@ afterAll(() => { }); const MOCKED_VALUES: CommandOptions = { - keyVaultName: "testKeyVault", + // keyVaultName: "testKeyVault", servicePrincipalId: "servicePrincipalId", servicePrincipalPassword: "servicePrincipalPassword", storageAccountName: "testaccount", @@ -49,7 +49,7 @@ const MOCKED_VALUES: CommandOptions = { }; const MOCKED_CONFIG: OnBoardConfig = { - keyVaultName: "testKeyVault", + // keyVaultName: "testKeyVault", servicePrincipalId: "servicePrincipalId", servicePrincipalPassword: "servicePrincipalPassword", storageAccountName: "testaccount", @@ -108,7 +108,7 @@ const testPopulatedVal = ( describe("test populateValues", () => { it("all values are set", () => { const values = populateValues(getMockedValues()); - expect(values.keyVaultName).toBe(MOCKED_VALUES.keyVaultName); + // expect(values.keyVaultName).toBe(MOCKED_VALUES.keyVaultName); expect(values.servicePrincipalId).toBe(MOCKED_VALUES.servicePrincipalId); expect(values.servicePrincipalPassword).toBe( MOCKED_VALUES.servicePrincipalPassword @@ -152,19 +152,19 @@ describe("test populateValues", () => { } ); }); - it("keyVaultName default to config.introspection.azure.key_vault_name", () => { - testPopulatedVal( - (configYaml: ConfigYaml) => { - configYaml.key_vault_name = "KeyVaultName"; - }, - (values: CommandOptions) => { - values.keyVaultName = undefined; - }, - (values) => { - expect(values.keyVaultName).toBe("KeyVaultName"); - } - ); - }); + // it("keyVaultName default to config.introspection.azure.key_vault_name", () => { + // testPopulatedVal( + // (configYaml: ConfigYaml) => { + // configYaml.key_vault_name = "KeyVaultName"; + // }, + // (values: CommandOptions) => { + // values.keyVaultName = undefined; + // }, + // (values) => { + // expect(values.keyVaultName).toBe("KeyVaultName"); + // } + // ); + // }); it("servicePrincipalId default to config.introspection.azure.service_principal_id", () => { testPopulatedVal( (configYaml: ConfigYaml) => { @@ -316,9 +316,7 @@ describe("test validateAndCreateStorageAccount function", () => { describe("test getStorageAccessKey function", () => { it("already exist", async () => { - jest - .spyOn(storage, "getStorageAccountKey") - .mockReturnValueOnce(Promise.resolve("key")); + jest.spyOn(storage, "getStorageAccountKey").mockResolvedValueOnce("key"); const values = getMockedConfig(); const accessOpts = getMockedAccessOpts(values); const storageKey = await getStorageAccessKey(values, accessOpts); @@ -326,27 +324,25 @@ describe("test getStorageAccessKey function", () => { }); }); -describe("test createKeyVault function", () => { - it("[+ve] not key vault value", async () => { - const values = getMockedConfig(); - values.keyVaultName = undefined; - const accessOpts = getMockedAccessOpts(values); - // nothing is done - await createKeyVault(values, accessOpts, "accessString"); - }); - it("[+ve] with key vault value", async () => { - jest.spyOn(keyvault, "setSecret").mockReturnValueOnce(Promise.resolve()); - const values = getMockedConfig(); - const accessOpts = getMockedAccessOpts(values); - await createKeyVault(values, accessOpts, "accessString"); - }); -}); +// describe("test createKeyVault function", () => { +// it("[+ve] not key vault value", async () => { +// const values = getMockedConfig(); +// values.keyVaultName = undefined; +// const accessOpts = getMockedAccessOpts(values); +// // nothing is done +// await createKeyVault(values, accessOpts, "accessString"); +// }); +// it("[+ve] with key vault value", async () => { +// jest.spyOn(keyvault, "setSecret").mockReturnValueOnce(Promise.resolve()); +// const values = getMockedConfig(); +// const accessOpts = getMockedAccessOpts(values); +// await createKeyVault(values, accessOpts, "accessString"); +// }); +// }); describe("onboard", () => { test("empty location", async () => { - jest - .spyOn(storage, "isStorageAccountExist") - .mockReturnValueOnce(Promise.resolve(false)); + jest.spyOn(storage, "isStorageAccountExist").mockResolvedValueOnce(false); try { const values = getMockedConfig(); @@ -392,9 +388,9 @@ describe("onboard", () => { jest .spyOn(storage, "createStorageAccount") .mockReturnValueOnce(Promise.resolve({ location: "test" })); - jest - .spyOn(onboardImpl, "createKeyVault") - .mockReturnValueOnce(Promise.resolve()); + // jest + // .spyOn(onboardImpl, "createKeyVault") + // .mockReturnValueOnce(Promise.resolve()); jest.spyOn(onboardImpl, "setConfiguration").mockReturnValueOnce(true); const data = { diff --git a/src/commands/deployment/onboard.ts b/src/commands/deployment/onboard.ts index 942ef14f6..42cf50b79 100644 --- a/src/commands/deployment/onboard.ts +++ b/src/commands/deployment/onboard.ts @@ -4,7 +4,7 @@ import commander from "commander"; import fs from "fs"; import yaml from "js-yaml"; import { Config, defaultConfigFile, readYaml } from "../../config"; -import { setSecret } from "../../lib/azure/keyvault"; +// import { setSecret } from "../../lib/azure/keyvault"; import { createStorageAccount, createTableIfNotExists, @@ -29,7 +29,6 @@ export interface CommandOptions { storageTableName: string | undefined; storageLocation: string | undefined; storageResourceGroupName: string | undefined; - keyVaultName: string | undefined; servicePrincipalId: string | undefined; servicePrincipalPassword: string | undefined; tenantId: string | undefined; @@ -44,7 +43,6 @@ export interface OnBoardConfig { servicePrincipalPassword: string; subscriptionId: string; tenantId: string; - keyVaultName?: string; storageLocation?: string; } @@ -61,7 +59,6 @@ export const populateValues = (opts: CommandOptions): CommandOptions => { opts.storageAccountName || azure?.account_name || undefined; opts.storageTableName = opts.storageTableName || azure?.table_name || undefined; - opts.keyVaultName = opts.keyVaultName || config.key_vault_name || undefined; opts.servicePrincipalId = opts.servicePrincipalId || azure?.service_principal_id || undefined; opts.servicePrincipalPassword = @@ -111,7 +108,6 @@ export const validateValues = (opts: CommandOptions): OnBoardConfig => { servicePrincipalPassword: opts.servicePrincipalPassword || "", subscriptionId: opts.subscriptionId || "", tenantId: opts.tenantId || "", - keyVaultName: opts.keyVaultName, storageLocation: opts.storageLocation, }; }; @@ -207,45 +203,37 @@ export const getStorageAccessKey = async ( return accessKey; }; -/** - * Creates Key Vault if value from commander has value for `keyVaultName` - * - * @param values values from commander - * @param accessOpts Azure Access Opts - * @param accessKey Access Key - */ -export const createKeyVault = async ( - values: OnBoardConfig, - accessOpts: AzureAccessOpts, - accessKey: string -): Promise => { - // if key vault is not specified, exit without reading storage account - // key and setting it in the key vault - if (values.keyVaultName) { - logger.debug( - `Calling setSecret with storage account primary key *** - and ${values.keyVaultName}` - ); - await setSecret( - values.keyVaultName, - `${values.storageAccountName}Key`, - accessKey, - accessOpts - ); - } else { - // notify the user to set the environment variable with storage access key - logger.info( - `Please set the storage account access key in environment variable - INTROSPECTION_STORAGE_ACCESS_KEY before issuing any deployment commands.` - ); - logger.info(`Storage account ${values.storageAccountName} access key: ***`); - } -}; +// export const createKeyVault = async ( +// values: OnBoardConfig, +// accessOpts: AzureAccessOpts, +// accessKey: string +// ): Promise => { +// // if key vault is not specified, exit without reading storage account +// // key and setting it in the key vault +// if (values.keyVaultName) { +// logger.debug( +// `Calling setSecret with storage account primary key *** +// and ${values.keyVaultName}` +// ); +// await setSecret( +// values.keyVaultName, +// `${values.storageAccountName}Key`, +// accessKey, +// accessOpts +// ); +// } else { +// // notify the user to set the environment variable with storage access key +// logger.info( +// `Please set the storage account access key in environment variable +// INTROSPECTION_STORAGE_ACCESS_KEY before issuing any deployment commands.` +// ); +// logger.info(`Storage account ${values.storageAccountName} access key: ***`); +// } +// }; /** * Creates the Storage account `accountName` in resource group `resourceGroup`, - * sets storage account access key in keyvalut, and updates pipelines - * (acr-hld, hld->manifests) + * and updates pipelines (acr-hld, hld->manifests) * * @param values Values from commander. */ @@ -254,7 +242,7 @@ export const onboard = async ( ): Promise => { logger.debug( `onboard called with ${values.storageAccountName}, ${values.storageTableName}, - ${values.storageResourceGroupName}, ${values.storageLocation}, and ${values.keyVaultName}` + ${values.storageResourceGroupName} and ${values.storageLocation}` ); const accessOpts: AzureAccessOpts = { @@ -288,7 +276,7 @@ export const onboard = async ( table ${values.storageTableName} exist.`); } - await createKeyVault(values, accessOpts, accessKey); + // await createKeyVault(values, accessOpts, accessKey); // save storage account and table names in configuration setConfiguration(values.storageAccountName, values.storageTableName); From 3c945f11f51b54a08337c032d18bc21663e11a33 Mon Sep 17 00:00:00 2001 From: Dennis Seah Date: Thu, 26 Mar 2020 10:40:16 -0700 Subject: [PATCH 2/3] remove commented out code --- src/commands/deployment/onboard.test.ts | 34 ------------------------- src/commands/deployment/onboard.ts | 31 ---------------------- 2 files changed, 65 deletions(-) diff --git a/src/commands/deployment/onboard.test.ts b/src/commands/deployment/onboard.test.ts index c4b519f69..bfcef8361 100644 --- a/src/commands/deployment/onboard.test.ts +++ b/src/commands/deployment/onboard.test.ts @@ -4,7 +4,6 @@ import yaml from "js-yaml"; import * as path from "path"; import { Config, loadConfiguration } from "../../config"; import * as config from "../../config"; -// import * as keyvault from "../../lib/azure/keyvault"; import * as storage from "../../lib/azure/storage"; import { createTempDir } from "../../lib/ioUtil"; import { deepClone } from "../../lib/util"; @@ -15,7 +14,6 @@ import { } from "../../logger"; import { AzureAccessOpts, ConfigYaml } from "../../types"; import { - // createKeyVault, execute, getStorageAccessKey, CommandOptions, @@ -37,7 +35,6 @@ afterAll(() => { }); const MOCKED_VALUES: CommandOptions = { - // keyVaultName: "testKeyVault", servicePrincipalId: "servicePrincipalId", servicePrincipalPassword: "servicePrincipalPassword", storageAccountName: "testaccount", @@ -49,7 +46,6 @@ const MOCKED_VALUES: CommandOptions = { }; const MOCKED_CONFIG: OnBoardConfig = { - // keyVaultName: "testKeyVault", servicePrincipalId: "servicePrincipalId", servicePrincipalPassword: "servicePrincipalPassword", storageAccountName: "testaccount", @@ -108,7 +104,6 @@ const testPopulatedVal = ( describe("test populateValues", () => { it("all values are set", () => { const values = populateValues(getMockedValues()); - // expect(values.keyVaultName).toBe(MOCKED_VALUES.keyVaultName); expect(values.servicePrincipalId).toBe(MOCKED_VALUES.servicePrincipalId); expect(values.servicePrincipalPassword).toBe( MOCKED_VALUES.servicePrincipalPassword @@ -152,19 +147,6 @@ describe("test populateValues", () => { } ); }); - // it("keyVaultName default to config.introspection.azure.key_vault_name", () => { - // testPopulatedVal( - // (configYaml: ConfigYaml) => { - // configYaml.key_vault_name = "KeyVaultName"; - // }, - // (values: CommandOptions) => { - // values.keyVaultName = undefined; - // }, - // (values) => { - // expect(values.keyVaultName).toBe("KeyVaultName"); - // } - // ); - // }); it("servicePrincipalId default to config.introspection.azure.service_principal_id", () => { testPopulatedVal( (configYaml: ConfigYaml) => { @@ -324,22 +306,6 @@ describe("test getStorageAccessKey function", () => { }); }); -// describe("test createKeyVault function", () => { -// it("[+ve] not key vault value", async () => { -// const values = getMockedConfig(); -// values.keyVaultName = undefined; -// const accessOpts = getMockedAccessOpts(values); -// // nothing is done -// await createKeyVault(values, accessOpts, "accessString"); -// }); -// it("[+ve] with key vault value", async () => { -// jest.spyOn(keyvault, "setSecret").mockReturnValueOnce(Promise.resolve()); -// const values = getMockedConfig(); -// const accessOpts = getMockedAccessOpts(values); -// await createKeyVault(values, accessOpts, "accessString"); -// }); -// }); - describe("onboard", () => { test("empty location", async () => { jest.spyOn(storage, "isStorageAccountExist").mockResolvedValueOnce(false); diff --git a/src/commands/deployment/onboard.ts b/src/commands/deployment/onboard.ts index 42cf50b79..16f2ec003 100644 --- a/src/commands/deployment/onboard.ts +++ b/src/commands/deployment/onboard.ts @@ -4,7 +4,6 @@ import commander from "commander"; import fs from "fs"; import yaml from "js-yaml"; import { Config, defaultConfigFile, readYaml } from "../../config"; -// import { setSecret } from "../../lib/azure/keyvault"; import { createStorageAccount, createTableIfNotExists, @@ -203,34 +202,6 @@ export const getStorageAccessKey = async ( return accessKey; }; -// export const createKeyVault = async ( -// values: OnBoardConfig, -// accessOpts: AzureAccessOpts, -// accessKey: string -// ): Promise => { -// // if key vault is not specified, exit without reading storage account -// // key and setting it in the key vault -// if (values.keyVaultName) { -// logger.debug( -// `Calling setSecret with storage account primary key *** -// and ${values.keyVaultName}` -// ); -// await setSecret( -// values.keyVaultName, -// `${values.storageAccountName}Key`, -// accessKey, -// accessOpts -// ); -// } else { -// // notify the user to set the environment variable with storage access key -// logger.info( -// `Please set the storage account access key in environment variable -// INTROSPECTION_STORAGE_ACCESS_KEY before issuing any deployment commands.` -// ); -// logger.info(`Storage account ${values.storageAccountName} access key: ***`); -// } -// }; - /** * Creates the Storage account `accountName` in resource group `resourceGroup`, * and updates pipelines (acr-hld, hld->manifests) @@ -276,8 +247,6 @@ export const onboard = async ( table ${values.storageTableName} exist.`); } - // await createKeyVault(values, accessOpts, accessKey); - // save storage account and table names in configuration setConfiguration(values.storageAccountName, values.storageTableName); return storageAccount; From 674601ef5b4905ac4c6429357f68c7deaac68871 Mon Sep 17 00:00:00 2001 From: Dennis Seah Date: Thu, 26 Mar 2020 10:41:32 -0700 Subject: [PATCH 3/3] Update onboard.test.ts --- src/commands/deployment/onboard.test.ts | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/commands/deployment/onboard.test.ts b/src/commands/deployment/onboard.test.ts index bfcef8361..ab8d2d527 100644 --- a/src/commands/deployment/onboard.test.ts +++ b/src/commands/deployment/onboard.test.ts @@ -354,9 +354,6 @@ describe("onboard", () => { jest .spyOn(storage, "createStorageAccount") .mockReturnValueOnce(Promise.resolve({ location: "test" })); - // jest - // .spyOn(onboardImpl, "createKeyVault") - // .mockReturnValueOnce(Promise.resolve()); jest.spyOn(onboardImpl, "setConfiguration").mockReturnValueOnce(true); const data = {