Enforces running SPK Project commands in order and Lifecycle pipeline refactoring to use variable groups from Bedrock.yaml (#199)

* refactored cvg and test cases

* Added validation check in pipeline install command

* Removed variable in build definiiton. Added variable group names in lifecycle piepline from bedrock.yaml. Fixed create variable group bug

* Removed unnecessary check for bedrock file content

* Incorporated feedback

* Reverted changes related to variable group bug.

* Added test cases

Author: sarath-p

docs/project-management.md

@@ -1,6 +1,8 @@
 # Project Management
 
-Create and manage components for a Bedrock project.
+Create and manage components for a Bedrock project. All project management
+commands will need to run in the order as listed below in the
+[commands](#Commands) section due to dependencies.
 
 Usage:
 
@@ -11,13 +13,18 @@ spk project [command] [options]
 Commands:
 
 - [Project Management](#project-management)
+
   - [Prerequisites](#prerequisites)
   - [Commands](#commands)
+
     - [init](#init)
     - [create-variable-group](#create-variable-group)
       - [Command Prerequisites](#command-prerequisites)
     - [install-lifecycle-pipeline](#install-lifecycle-pipeline)
 
+    **Please note all project management commands must run in the order as
+    listed above.**
+
 Global options:
 
 ```

docs/service-introspection-onboarding.md

@@ -148,7 +148,9 @@ by service introspection.
 - Add the task before the crucial steps in your pipeline. This will capture
   details about failures if the important steps fail.
 
-- To specify the `spk` version you want to download, set the `VERSION` environment variable. With this, `get_spk_version` will download that version. If `VERSION` is not set, it will download the latest version.
+- To specify the `spk` version you want to download, set the `VERSION`
+  environment variable. With this, `get_spk_version` will download that version.
+  If `VERSION` is not set, it will download the latest version.
 
 #### 3. CD release pipeline (ACR to HLD) configuration
 
@@ -167,14 +169,13 @@ pipeline in the Azure DevOps portal:
 
 ```yaml
 latest_commit=$(git rev-parse --short HEAD) VERSION_TO_DOWNLOAD=$(curl -s
-"https://api.github.com/repos/CatalystCode/spk/releases/latest" | grep "tag_name" | sed -E 's/.*"([^"]+)".*/\1/')
-echo "Downloading SPK"
-curl https://raw.githubusercontent.com/Microsoft/bedrock/master/gitops/azure-devops/build.sh > build.sh
-chmod +x build.sh
-. ./build.sh --source-only
-get_spk_version
-download_spk
-./spk/spk deployment create  -n $(ACCOUNT_NAME) -k $(ACCOUNT_KEY) -t $(TABLE_NAME) -p $(PARTITION_KEY)  --p2 $(Release.ReleaseId) --hld-commit-id $latest_commit --env $(Release.EnvironmentName) --image-tag $(Build.BuildId)
+"https://api.github.com/repos/CatalystCode/spk/releases/latest" | grep
+"tag_name" | sed -E 's/.*"([^"]+)".*/\1/') echo "Downloading SPK" curl
+https://raw.githubusercontent.com/Microsoft/bedrock/master/gitops/azure-devops/build.sh
+> build.sh chmod +x build.sh . ./build.sh --source-only get_spk_version
+download_spk ./spk/spk deployment create  -n $(ACCOUNT_NAME) -k $(ACCOUNT_KEY)
+-t $(TABLE_NAME) -p $(PARTITION_KEY)  --p2 $(Release.ReleaseId) --hld-commit-id
+$latest_commit --env $(Release.EnvironmentName) --image-tag $(Build.BuildId)
 ```
 
 This task is similar to the one from step 1 but instead passes the information
@@ -191,14 +192,13 @@ your multi-stage `azure-pipelines.yml`:
 ```yaml
 latest_commit=$(git rev-parse --short HEAD) tag_name=$(Build.BuildId)
 VERSION_TO_DOWNLOAD=$(curl -s
-"https://api.github.com/repos/CatalystCode/spk/releases/latest" | grep "tag_name" | sed -E 's/.*"([^"]+)".*/\1/')
-echo "Downloading SPK"
-curl https://raw.githubusercontent.com/Microsoft/bedrock/master/gitops/azure-devops/build.sh > build.sh
-chmod +x build.sh
-. ./build.sh --source-only
-get_spk_version
-download_spk
-./spk/spk deployment create  -n $(ACCOUNT_NAME) -k $(ACCOUNT_KEY) -t $(TABLE_NAME) -p $(PARTITION_KEY)  --p2 $(Build.BuildId) --hld-commit-id $latest_commit --env $(Build.SourceBranchName) --image-tag $tag_name
+"https://api.github.com/repos/CatalystCode/spk/releases/latest" | grep
+"tag_name" | sed -E 's/.*"([^"]+)".*/\1/') echo "Downloading SPK" curl
+https://raw.githubusercontent.com/Microsoft/bedrock/master/gitops/azure-devops/build.sh
+> build.sh chmod +x build.sh . ./build.sh --source-only get_spk_version
+download_spk ./spk/spk deployment create  -n $(ACCOUNT_NAME) -k $(ACCOUNT_KEY)
+-t $(TABLE_NAME) -p $(PARTITION_KEY)  --p2 $(Build.BuildId) --hld-commit-id
+$latest_commit --env $(Build.SourceBranchName) --image-tag $tag_name
 ```
 
 Make sure your variable `tag_name` in this script matches the `tag_name` in the

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

@@ -1,19 +1,27 @@
 import fs from "fs";
+import yaml from "js-yaml";
+import mockFs from "mock-fs";
 import os from "os";
 import path from "path";
 import uuid from "uuid/v4";
-import { readYaml, write } from "../../config";
+import { Bedrock, readYaml, write } from "../../config";
 import { IAzureDevOpsOpts } from "../../lib/git";
 import {
   disableVerboseLogging,
   enableVerboseLogging,
   logger
 } from "../../logger";
-import { IBedrockFile } from "../../types";
+import {
+  createTestBedrockYaml,
+  createTestHldLifecyclePipelineYaml
+} from "../../test/mockFactory";
+import { IAzurePipelinesYaml, IBedrockFile } from "../../types";
 import {
   create,
   execute,
+  isBedrockFileExists,
   setVariableGroupInBedrockFile,
+  updateLifeCyclePipeline,
   validateRequiredArguments
 } from "./create-variable-group";
 
@@ -25,6 +33,10 @@ afterAll(() => {
   disableVerboseLogging();
 });
 
+beforeEach(() => {
+  jest.clearAllMocks();
+});
+
 const registryName = uuid();
 const variableGroupName = uuid();
 const hldRepoUrl = uuid();
@@ -255,58 +267,52 @@ describe("setVariableGroupInBedrockFile", () => {
     expect(invalidGroupNameError).toBeDefined();
   });
 
-  test("Should pass adding a variable group name when no bedrock file exists", async () => {
+  test("Should fail adding a variable group name when no bedrock file exists", async () => {
     // Create random directory to initialize
     const randomTmpDir = path.join(os.tmpdir(), uuid());
     fs.mkdirSync(randomTmpDir);
 
-    await setVariableGroupInBedrockFile(randomTmpDir, variableGroupName);
-
-    const filePath = path.join(randomTmpDir, "bedrock.yaml");
-    expect(fs.existsSync(filePath)).toBe(true);
-
-    const bedrockFileData = readYaml<IBedrockFile>(filePath);
-    logger.info(`filejson: ${JSON.stringify(bedrockFileData)}`);
-    expect(bedrockFileData.variableGroups![0]).toBe(variableGroupName);
-  });
+    let noFileError: Error | undefined;
 
-  test("Should pass adding a valid variable group name when bedrock file exists with undefined variableGroups", async () => {
-    // Create random directory to initialize
-    const randomTmpDir = path.join(os.tmpdir(), uuid());
-    fs.mkdirSync(randomTmpDir);
-
-    await setVariableGroupInBedrockFile(randomTmpDir, variableGroupName);
-
-    const bedrockFilePath = path.join(randomTmpDir, "bedrock.yaml");
-    expect(fs.existsSync(bedrockFilePath)).toBe(true);
-
-    const bedrockFile = readYaml<IBedrockFile>(bedrockFilePath);
-    logger.info(`filejson: ${JSON.stringify(bedrockFile)}`);
-    expect(bedrockFile.variableGroups![0]).toBe(variableGroupName);
+    try {
+      await setVariableGroupInBedrockFile(randomTmpDir, variableGroupName);
+    } catch (err) {
+      logger.info(`${err}`);
+      noFileError = err;
+    }
+    expect(noFileError).toBeDefined();
   });
 
   test("Should pass adding a valid variable group name when bedrock file exists with empty variableGroups", async () => {
     // Create random directory to initialize
     const randomTmpDir = path.join(os.tmpdir(), uuid());
     fs.mkdirSync(randomTmpDir);
 
+    // create bedrock file to simulate the the use case that `spk project init` ran before
     const bedrockFileData: IBedrockFile = {
-      rings: {}, // rings is optional but necessary to create a bedrock file in config.write method
-      services: {}, // service property is not optional so set it to null
+      rings: {},
+      services: {},
       variableGroups: []
     };
 
+    const asYaml = yaml.safeDump(bedrockFileData, {
+      lineWidth: Number.MAX_SAFE_INTEGER
+    });
+
+    fs.writeFileSync(path.join(randomTmpDir, "bedrock.yaml"), asYaml);
+
     await setVariableGroupInBedrockFile(randomTmpDir, variableGroupName);
 
     const bedrockFilePath = path.join(randomTmpDir, "bedrock.yaml");
     expect(fs.existsSync(bedrockFilePath)).toBe(true);
 
     const bedrockFile = readYaml<IBedrockFile>(bedrockFilePath);
+
     logger.info(`filejson: ${JSON.stringify(bedrockFile)}`);
     expect(bedrockFile.variableGroups![0]).toBe(variableGroupName);
   });
 
-  test("Should pass adding a valid variable group name when bedrock file exists when variableGroups length is already 1", async () => {
+  test("Should pass adding a valid variable group name when bedrock file exists when variableGroups length is > 0", async () => {
     // Create random directory to initialize
     const randomTmpDir = path.join(os.tmpdir(), uuid());
     fs.mkdirSync(randomTmpDir);
@@ -332,3 +338,165 @@ describe("setVariableGroupInBedrockFile", () => {
     expect(bedrockFile.variableGroups![1]).toBe(variableGroupName);
   });
 });
+
+describe("updateLifeCyclePipeline", () => {
+  beforeAll(() => {
+    mockFs({
+      "bedrock.yaml": createTestBedrockYaml() as any
+    });
+  });
+
+  afterAll(() => {
+    mockFs.restore();
+  });
+
+  afterEach(() => {
+    jest.clearAllMocks();
+  });
+
+  test("Should fail with empty arguments", async () => {
+    let invalidDirError: Error | undefined;
+    try {
+      await updateLifeCyclePipeline("");
+    } catch (err) {
+      invalidDirError = err;
+    }
+    expect(invalidDirError).toBeDefined();
+  });
+
+  test("Should fail adding a variable group name when no pipeline yaml file exists", async () => {
+    // Create random directory to initialize
+    const randomTmpDir = path.join(os.tmpdir(), uuid());
+    fs.mkdirSync(randomTmpDir);
+
+    let noFileError: Error | undefined;
+
+    try {
+      await updateLifeCyclePipeline(randomTmpDir);
+    } catch (err) {
+      noFileError = err;
+    }
+    expect(noFileError).toBeDefined();
+  });
+
+  test("Should pass adding variable groups when bedrock file exists with empty variableGroups", async () => {
+    // Create random directory to initialize
+    const randomTmpDir = path.join(os.tmpdir(), uuid());
+    fs.mkdirSync(randomTmpDir);
+    const writeSpy = jest.spyOn(fs, "writeFileSync");
+
+    const defaultBedrockFileObject = createTestBedrockYaml(
+      false
+    ) as IBedrockFile;
+
+    write(defaultBedrockFileObject, randomTmpDir);
+
+    const hldFilePath = path.join(randomTmpDir, "hld-lifecycle.yaml");
+
+    const hldLifeCycleFile: IAzurePipelinesYaml = createTestHldLifecyclePipelineYaml(
+      false
+    ) as IAzurePipelinesYaml;
+
+    const asYaml = yaml.safeDump(hldLifeCycleFile, {
+      lineWidth: Number.MAX_SAFE_INTEGER
+    });
+
+    fs.writeFileSync(hldFilePath, asYaml);
+
+    await updateLifeCyclePipeline(randomTmpDir);
+
+    const hldLifeCycleYaml = readYaml<IAzurePipelinesYaml>(hldFilePath);
+    logger.info(`filejson: ${JSON.stringify(hldLifeCycleYaml)}`);
+    expect(hldLifeCycleYaml.variables!.length).toBeLessThanOrEqual(0);
+  });
+
+  test("Should pass adding variable groups when bedrock file exists with one variableGroup", async () => {
+    // Create random directory to initialize
+    const randomTmpDir = path.join(os.tmpdir(), uuid());
+    fs.mkdirSync(randomTmpDir);
+    logger.info(`random dir: ${randomTmpDir})`);
+
+    const defaultBedrockFileObject = createTestBedrockYaml(
+      false
+    ) as IBedrockFile;
+
+    // add new variabe group
+    defaultBedrockFileObject.variableGroups = [
+      ...(defaultBedrockFileObject.variableGroups ?? []),
+      variableGroupName
+    ];
+
+    write(defaultBedrockFileObject, randomTmpDir);
+
+    const hldFilePath = path.join(randomTmpDir, "hld-lifecycle.yaml");
+
+    const hldLifeCycleFile: IAzurePipelinesYaml = createTestHldLifecyclePipelineYaml(
+      false
+    ) as IAzurePipelinesYaml;
+
+    const asYaml = yaml.safeDump(hldLifeCycleFile, {
+      lineWidth: Number.MAX_SAFE_INTEGER
+    });
+
+    fs.writeFileSync(hldFilePath, asYaml);
+
+    await updateLifeCyclePipeline(randomTmpDir);
+
+    const hldLifeCycleYaml = readYaml<IAzurePipelinesYaml>(hldFilePath);
+    logger.info(`filejson: ${JSON.stringify(hldLifeCycleYaml)}`);
+    expect(hldLifeCycleYaml.variables![0]).toEqual({
+      group: variableGroupName
+    });
+  });
+});
+
+describe("isBedrockFileExists", () => {
+  test("Should fail when empty file directory is passed", async () => {
+    let invalidDirError: Error | undefined;
+
+    try {
+      logger.info("calling create");
+      await isBedrockFileExists("");
+    } catch (err) {
+      invalidDirError = err;
+    }
+    expect(invalidDirError).toBeDefined();
+  });
+
+  test("Should return false when bedrock file does not exist", async () => {
+    // Create random directory to initialize
+    const randomTmpDir = path.join(os.tmpdir(), uuid());
+    fs.mkdirSync(randomTmpDir);
+
+    const exists = await isBedrockFileExists(randomTmpDir);
+
+    logger.info(`bedrock.yaml file exists: ${exists}`);
+
+    expect(exists).toBe(false);
+  });
+
+  test("Should return true when bedrock file exists", async () => {
+    // Create random directory to initialize
+    const randomTmpDir = path.join(os.tmpdir(), uuid());
+    fs.mkdirSync(randomTmpDir);
+
+    logger.info(`random temp dir: ${randomTmpDir}`);
+
+    // create bedrock file to simulate the the use case that `spk project init` ran before
+    const bedrockFileData: IBedrockFile = {
+      rings: {},
+      services: {},
+      variableGroups: []
+    };
+
+    const asYaml = yaml.safeDump(bedrockFileData, {
+      lineWidth: Number.MAX_SAFE_INTEGER
+    });
+    fs.writeFileSync(path.join(randomTmpDir, "bedrock.yaml"), asYaml);
+
+    const exists = await isBedrockFileExists(randomTmpDir);
+    logger.info(`bedrock.yaml file exists: ${exists} in ${randomTmpDir}`);
+
+    expect(exists).toBe(true);
+  });
+});