[BUG FIX] Variable group is created too late in spk setup command

Author: dennisseah

docs/commands/data.json

@@ -721,4 +721,4 @@
     ],
     "markdown": "## Description\n\nInstall the build and push to acr pipeline for a service to your Azure DevOps\ninstance. The default pipeline generated by `spk service create` is a multistage\npipeline, which is in public preview and must be enabled to use.\nhttps://docs.microsoft.com/en-us/azure/devops/pipelines/process/stages?view=azure-devops&tabs=yaml\n"
   }
-}
+}

src/commands/project/create-variable-group.ts

@@ -11,7 +11,6 @@ import {
   validateForRequiredValues,
 } from "../../lib/commandBuilder";
 import { PROJECT_PIPELINE_FILENAME } from "../../lib/constants";
-import { AzureDevOpsOpts } from "../../lib/git";
 import { addVariableGroup } from "../../lib/pipelines/variableGroup";
 import {
   hasValue,

src/commands/setup.test.ts

@@ -15,6 +15,8 @@ import * as promptInstance from "../lib/setup/prompt";
 import * as scaffold from "../lib/setup/scaffold";
 import * as setupLog from "../lib/setup/setupLog";
 import * as azureStorage from "../lib/setup/azureStorage";
+import * as variableGroup from "../lib/setup/variableGroup";
+import * as shell from "../lib/shell";
 import { deepClone } from "../lib/util";
 import { ConfigYaml } from "../types";
 import {
@@ -23,6 +25,7 @@ import {
   execute,
   getAPIClients,
   getErrorMessage,
+  isAzCLIInstall,
 } from "./setup";
 import * as setup from "./setup";
 
@@ -38,6 +41,8 @@ const mockRequestContext: RequestContext = {
   workspace: WORKSPACE,
 };
 
+jest.spyOn(variableGroup, "setupVariableGroup").mockResolvedValue();
+
 describe("test createSPKConfig function", () => {
   it("positive test", () => {
     const tmpFile = path.join(createTempDir(), "config.yaml");
@@ -420,3 +425,25 @@ describe("test getAPIClients function", () => {
     );
   });
 });
+
+describe("test isAzCLIInstall function", () => {
+  it("positive test", async () => {
+    const version = JSON.stringify({
+      "azure-cli": "2.0.79",
+    });
+    jest.spyOn(shell, "exec").mockResolvedValueOnce(version);
+    await isAzCLIInstall();
+  });
+  it("negative test: version is not returned", async () => {
+    jest.spyOn(shell, "exec").mockResolvedValueOnce("");
+    await expect(isAzCLIInstall()).rejects.toThrow(
+      errorMessage("setup-cmd-az-cli-err")
+    );
+  });
+  it("negative test: az is not installed", async () => {
+    jest.spyOn(shell, "exec").mockRejectedValueOnce(Error("test"));
+    await expect(isAzCLIInstall()).rejects.toThrow(
+      errorMessage("setup-cmd-az-cli-err")
+    );
+  });
+});

src/commands/setup.ts

@@ -38,6 +38,7 @@ import {
   manifestRepo,
 } from "../lib/setup/scaffold";
 import { create as createSetupLog } from "../lib/setup/setupLog";
+import { setupVariableGroup } from "../lib/setup/variableGroup";
 import { logger } from "../logger";
 import decorator from "./setup.decorator.json";
 import { createStorage } from "../lib/setup/azureStorage";
@@ -188,6 +189,7 @@ export const createAppRepoTasks = async (
       rc.acrName,
       RESOURCE_GROUP_LOCATION
     );
+    await setupVariableGroup(rc);
     await helmRepo(gitAPI, rc);
     await appRepo(gitAPI, rc);
     await createLifecyclePipeline(buildAPI, rc);
@@ -266,6 +268,7 @@ export const execute = async (
     const { coreAPI, gitAPI, buildAPI } = await getAPIClients();
 
     await createProjectIfNotExist(coreAPI, rc);
+    await setupVariableGroup(rc);
     await hldRepo(gitAPI, rc);
     await manifestRepo(gitAPI, rc);
     await createHLDtoManifestPipeline(buildAPI, rc);

src/lib/azure/keyvault.test.ts

@@ -1,12 +1,10 @@
-import { PipelineOptions } from "@azure/core-http";
 import { KeyVaultSecret } from "@azure/keyvault-secrets";
 import uuid from "uuid/v4";
 import { disableVerboseLogging, enableVerboseLogging } from "../../logger";
 import { getErrorMessage } from "../errorBuilder";
 import * as azurecredentials from "./azurecredentials";
 import { getClient, getSecret, setSecret } from "./keyvault";
 import * as keyvault from "./keyvault";
-import { TokenCredential } from "azure-storage";
 
 const keyVaultName = uuid();
 const mockedName = uuid();

src/lib/fileutils.test.ts

@@ -24,7 +24,7 @@ import {
   VM_IMAGE,
   BEDROCK_FILENAME,
 } from "../lib/constants";
-import { disableVerboseLogging, enableVerboseLogging, logger } from "../logger";
+import { disableVerboseLogging, enableVerboseLogging } from "../logger";
 import {
   createTestComponentYaml,
   createTestHldAzurePipelinesYaml,

src/lib/i18n.json

@@ -30,6 +30,14 @@
     "setup-cmd-git-api-err": "Could not get Git API client. Check the Azure DevOps credential.",
     "setup-cmd-build-api-err": "Could not get Build API client. Check the Azure DevOps credential.",
     "setup-cmd-cannot-locate-pr-for-approval": "Could not locate pull request for approval",
+    "setup-cmd-create-variable-group-missing-pat": "Could not created variable group because Personal Access Token was missing.",
+    "setup-cmd-create-variable-group-missing-acr-name": "Could not created variable group because ACR name was missing.",
+    "setup-cmd-create-variable-group-missing-sp-id": "Could not created variable group because service principal id was missing.",
+    "setup-cmd-create-variable-group-missing-sp-pwd": "Could not created variable group because service principal password was missing.",
+    "setup-cmd-create-variable-group-missing-tenant-id": "Could not created variable group because service principal tenant id was missing.",
+    "setup-cmd-create-variable-group-missing-storage-access-key": "Could not created variable group because storage account access key was missing.",
+    "setup-cmd-create-variable-group-missing-storage-account-name": "Could not created variable group because storage account name was missing.",
+    "setup-cmd-create-variable-group-missing-storage-table-name": "Could not created variable group because storage table name was missing.",
 
     "spk-config-yaml-err-readyaml": "Could not load file, {0}.",
     "spk-config-yaml-var-undefined": "Environment variable needs to be defined for {0} since it's referenced in the config file.",

src/lib/setup/prompt.test.ts

@@ -14,7 +14,6 @@ import {
   validationServicePrincipalInfoFromFile,
 } from "./prompt";
 import * as promptInstance from "./prompt";
-import * as gitService from "./gitService";
 import * as servicePrincipalService from "../azure/servicePrincipalService";
 import * as subscriptionService from "../azure/subscriptionService";
 

src/lib/setup/prompt.ts

@@ -16,11 +16,9 @@ import {
 import {
   ACR_NAME,
   DEFAULT_PROJECT_NAME,
-  HLD_REPO,
   RequestContext,
   WORKSPACE,
 } from "./constants";
-import { getAzureRepoUrl } from "./gitService";
 import {
   azCLILogin,
   createWithAzCLI,

src/lib/setup/scaffold.test.ts

@@ -2,12 +2,10 @@
 import * as fs from "fs-extra";
 import * as path from "path";
 import simpleGit from "simple-git/promise";
-import * as cmdCreateVariableGroup from "../../commands/project/create-variable-group";
 import * as projectInit from "../../commands/project/init";
+import * as cmdCreateVariableGroup from "../../commands/project/create-variable-group";
 import * as createService from "../../commands/service/create";
 import * as fileutils from "../../lib/fileutils";
-import * as variableGroup from "../../lib/pipelines/variableGroup";
-import * as sVariableGroup from "../setup/variableGroup";
 import { createTempDir } from "../ioUtil";
 import {
   APP_REPO,
@@ -23,7 +21,6 @@ import {
   hldRepo,
   initService,
   manifestRepo,
-  setupVariableGroup,
 } from "./scaffold";
 import * as scaffold from "./scaffold";
 
@@ -160,11 +157,16 @@ describe("test appRepo function", () => {
     const git = simpleGit();
     git.init = jest.fn();
 
-    jest.spyOn(scaffold, "setupVariableGroup").mockResolvedValueOnce();
     jest.spyOn(scaffold, "initService").mockResolvedValueOnce();
     jest.spyOn(projectInit, "initialize").mockImplementationOnce(async () => {
       fs.createFileSync("README.md");
     });
+    jest
+      .spyOn(cmdCreateVariableGroup, "setVariableGroupInBedrockFile")
+      .mockReturnValueOnce();
+    jest
+      .spyOn(cmdCreateVariableGroup, "updateLifeCyclePipeline")
+      .mockReturnValueOnce();
 
     await appRepo({} as any, createRequestContext(tempDir));
     const folder = path.join(tempDir, APP_REPO);
@@ -175,19 +177,6 @@ describe("test appRepo function", () => {
     jest.spyOn(createService, "createService").mockResolvedValueOnce();
     await initService(createRequestContext("test"), "test");
   });
-  it("sanity test on setupVariableGroup", async () => {
-    jest
-      .spyOn(variableGroup, "deleteVariableGroup")
-      .mockResolvedValueOnce(true);
-    jest.spyOn(sVariableGroup, "create").mockResolvedValueOnce();
-    jest
-      .spyOn(cmdCreateVariableGroup, "setVariableGroupInBedrockFile")
-      .mockReturnValueOnce();
-    jest
-      .spyOn(cmdCreateVariableGroup, "updateLifeCyclePipeline")
-      .mockReturnValueOnce();
-    await setupVariableGroup(createRequestContext("/dummy"));
-  });
   it("negative test", async () => {
     const tempDir = createTempDir();
     jest.spyOn(gitService, "createRepoInAzureOrg").mockImplementation(() => {