Add branch mapping to rings

- Rings in `bedrock.yaml` now contain a `targetBranch` (set via `--target-branch` flag on `ring create`). - The `targetBranch` is used as the the trigger branch for the serve-build-and-update pipelines; Added during `ring create` time. - The HLD repository now follows a directory structure of: ``` HLD ├── component.yaml └── my-application ├── access.yaml ├── config │ └── common.yaml ├── component.yaml └── <service-name> ├── config │ └── common.yaml ├── component.yaml └── <branch-name> # Is no longer always the name of the ring -- uses targetBranch if it was provided and the ring name as fallback ├── component.yaml ├── config │ └── common.yaml └── static ├── ingress-route.yaml # still matches on a `Ring` header for the ring name └── middlewares.yaml ``` closes microsoft/bedrock#1313
microsoft · May 4, 2020 · f62c190 · f62c190
1 parent f3612e0
commit f62c190
Show file tree

Hide file tree

Showing 10 changed files with 147 additions and 56 deletions.
diff --git a/docs/commands/data.json b/docs/commands/data.json
@@ -531,6 +531,13 @@
     "command": "create <ring-name>",
     "alias": "c",
     "description": "Create a new ring for the current working directory project repository. This will affect all services within the project repository.",
+    "options": [
+      {
+        "arg": "--target-branch <branch-name>",
+        "description": "The target branch this ring will map to; defaults to <ring-name> if not provided.",
+        "defaultValue": ""
+      }
+    ],
     "markdown": "## Description\n\nBedrock command to create a ring into an initialized bedrock project.\n\n## Example\n\nFor a bedrock.yaml file that looks like this:\n\n```yaml\nrings:\n  dev:\n    isDefault: true\n  qa:\n  prod:\nservices:\n  - path: ./\n    displayName: \"fabrikam\"\n    helm:\n      chart:\n        branch: master\n        git: \"https://dev.azure.com/fabrikam/frontend/_git/charts\"\n        path: frontend\n    k8sBackend: \"fabrikam-k8s-svc\"\n    k8sBackendPort: 80\n    middlewares: []\n    pathPrefix: \"fabrikam-service\"\n    pathPrefixMajorVersion: \"v1\"\nvariableGroups:\n  - fabrikam-vg\n```\n\nrunning `bedrock ring create stage` will result in a few changes:\n\n1. `stage` will be added into `bedrock.yaml` rings component:\n   ```yaml\n   rings:\n   dev:\n     isDefault: true\n   qa:\n   prod:\n   stage:\n   services:\n     - path: ./\n       displayName: \"fabrikam\"\n       helm:\n       chart:\n         branch: master\n         git: \"https://dev.azure.com/fabrikam/frontend/_git/charts\"\n         path: frontend\n       k8sBackend: \"fabrikam-k8s-svc\"\n       k8sBackendPort: 80\n       middlewares: []\n       pathPrefix: \"fabrikam-service\"\n       pathPrefixMajorVersion: \"v1\"\n   variableGroups:\n     - fabrikam-vg\n   ```\n2. Each of the referenced services within `bedrock.yaml` will have their\n   `build-update-hld.yaml` updated to include the new ring, `stage` in their\n   branch triggers:\n\n   ```yaml\n    trigger:\n    branches:\n        include:\n        - dev\n        - qa\n        - prod\n        - stage <-- NEW -->\n    variables:\n    - group: fabrikam-vg\n    …\n   ```\n\n3. Commiting these changes will trigger the project's lifecycle pipeline, which\n   will then scaffold out the newly created ring, along with the appropriate\n   IngressRoutes in the linked HLD repository.\n"
   },
   "ring delete": {

diff --git a/src/commands/hld/reconcile.ts b/src/commands/hld/reconcile.ts
@@ -187,7 +187,7 @@ export const configureChartForRing = async (
   serviceConfig: BedrockServiceConfig,
   normalizedServiceName: string
 ): Promise<ExecResult> => {
-  // Configue the k8s backend svc here along with master
+  // Configure the k8s backend svc here along with master
   // If no specific k8s backend name is provided, use the bedrock service name.
   const k8sBackendName = serviceConfig.k8sBackend || normalizedServiceName;
 
@@ -234,18 +234,18 @@ export const createServiceComponent = async (
 export const createRingComponent = async (
   execCmd: typeof execAndLog,
   svcPathInHld: string,
-  normalizedRingName: string
+  normalizedRingDirectory: string
 ): Promise<ExecResult> => {
   assertIsStringWithContent(svcPathInHld, "service-path");
-  assertIsStringWithContent(normalizedRingName, "ring-name");
-  const createRingInSvcCommand = `cd ${svcPathInHld} && mkdir -p ${normalizedRingName} config && fab add ${normalizedRingName} --path ./${normalizedRingName} --method local --type component && touch ./config/common.yaml`;
+  assertIsStringWithContent(normalizedRingDirectory, "branch-name");
+  const createRingInSvcCommand = `cd ${svcPathInHld} && mkdir -p ${normalizedRingDirectory} config && fab add ${normalizedRingDirectory} --path ./${normalizedRingDirectory} --method local --type component && touch ./config/common.yaml`;
 
   return execCmd(createRingInSvcCommand).catch((err) => {
     throw buildError(
       errorStatusCode.EXE_FLOW_ERR,
       {
         errorKey: "hld-reconcile-err-ring-create",
-        values: [normalizedRingName, svcPathInHld],
+        values: [normalizedRingDirectory, svcPathInHld],
       },
       err
     );
@@ -576,14 +576,16 @@ export const reconcileHld = async (
     );
 
     const ringsToCreate = Object.entries(managedRings).map(
-      ([ring, { isDefault }]) => {
+      ([ring, { isDefault, targetBranch }]) => {
         const normalizedRingName = normalizedName(ring);
+        const normalizedBranchName = normalizedName(targetBranch || ring);
         return {
           isDefault: !!isDefault,
           normalizedRingName,
+          normalizedBranchName,
           normalizedRingPathInHld: path.join(
             normalizedSvcPathInHld,
-            normalizedRingName
+            normalizedBranchName
           ),
         };
       }
@@ -596,7 +598,7 @@ export const reconcileHld = async (
       await dependencies.createRingComponent(
         dependencies.exec,
         normalizedSvcPathInHld,
-        normalizedRingName
+        ring.normalizedBranchName
       );
 
       // Add the helm chart to the ring.

diff --git a/src/commands/ring/create.decorator.json b/src/commands/ring/create.decorator.json
@@ -1,5 +1,12 @@
 {
   "command": "create <ring-name>",
   "alias": "c",
-  "description": "Create a new ring for the current working directory project repository. This will affect all services within the project repository."
+  "description": "Create a new ring for the current working directory project repository. This will affect all services within the project repository.",
+  "options": [
+    {
+      "arg": "--target-branch <branch-name>",
+      "description": "The target branch this ring will map to; defaults to <ring-name> if not provided.",
+      "defaultValue": ""
+    }
+  ]
 }
diff --git a/src/commands/ring/create.test.ts b/src/commands/ring/create.test.ts
@@ -59,22 +59,27 @@ describe("checkDependencies", () => {
 describe("test execute function and logic", () => {
   it("test execute function: missing ring input", async () => {
     const exitFn = jest.fn();
-    await execute("", "someprojectpath", exitFn);
+    await execute("", "someprojectpath", { targetBranch: "" }, exitFn);
     expect(exitFn).toBeCalledTimes(1);
     expect(exitFn.mock.calls).toEqual([[1]]);
   });
   it("test execute function: invalid ring input", async () => {
     const exitFn = jest.fn();
     jest.spyOn(dns, "assertIsValid");
-    await execute("-not!dns@compliant%", "someprojectpath", exitFn);
+    await execute(
+      "-not!dns@compliant%",
+      "someprojectpath",
+      { targetBranch: "" },
+      exitFn
+    );
     expect(dns.assertIsValid).toHaveReturnedTimes(0); // should never return because it throws
     expect(exitFn).toBeCalledTimes(1);
     expect(exitFn.mock.calls).toEqual([[1]]);
   });
   it("test execute function: missing project path", async () => {
     const exitFn = jest.fn();
     jest.spyOn(dns, "assertIsValid");
-    await execute("ring", "", exitFn);
+    await execute("ring", "", { targetBranch: "" }, exitFn);
     expect(dns.assertIsValid).toHaveReturnedTimes(1);
     expect(exitFn).toBeCalledTimes(1);
     expect(exitFn.mock.calls).toEqual([[1]]);
@@ -117,7 +122,7 @@ describe("test execute function and logic", () => {
       Object.entries(oldBedrockFile.rings).map(([ring]) => ring)
     ).not.toContain(newRingName);
 
-    await execute(newRingName, tmpDir, exitFn);
+    await execute(newRingName, tmpDir, { targetBranch: "" }, exitFn);
 
     const updatedBedrockFile: BedrockFile = loadBedrockFile(tmpDir);
     expect(

diff --git a/src/commands/ring/create.ts b/src/commands/ring/create.ts
@@ -15,6 +15,10 @@ import decorator from "./create.decorator.json";
 import { build as buildError, log as logError } from "../../lib/errorBuilder";
 import { errorStatusCode } from "../../lib/errorStatusCode";
 
+export interface CommandOptions {
+  targetBranch: string;
+}
+
 /**
  * Check for bedrock.yaml
  *
@@ -52,6 +56,7 @@ export const checkDependencies = (
 export const execute = async (
   ringName: string,
   projectPath: string,
+  opts: CommandOptions,
   exitFn: (status: number) => Promise<void>
 ): Promise<void> => {
   try {
@@ -65,23 +70,33 @@ export const execute = async (
     logger.info(`Project path: ${projectPath}`);
 
     dns.assertIsValid("<ring-name>", ringName);
+
+    // target-branch falls back to ringName
+    const targetBranch = opts.targetBranch || ringName;
+    // only do assertion on targetBranch if user provided
+    if (opts.targetBranch) {
+      dns.assertIsValid("<target-branch>", targetBranch);
+    }
+
     checkDependencies(projectPath, ringName);
 
     // Add ring to bedrock.yaml
-    addNewRing(projectPath, ringName);
+    addNewRing(projectPath, ringName, { targetBranch });
     // Add ring to all linked service build pipelines' branch triggers
     const bedrockFile: BedrockFile = loadBedrockFile(projectPath);
 
-    const newRings = Object.entries(bedrockFile.rings).map(([ring]) => ring);
-    logger.info(`Updated project rings: ${newRings}`);
+    const ringBranches = Object.entries(bedrockFile.rings).map(
+      ([ring, config]) => config.targetBranch || ring
+    );
+    logger.info(`Updated project rings: ${ringBranches}`);
 
     const servicePathDirectories = bedrockFile.services.map(
       (service) => service.path
     );
 
-    servicePathDirectories.forEach((s) => {
-      updateTriggerBranchesForServiceBuildAndUpdatePipeline(newRings, s);
-    });
+    for (const dir of servicePathDirectories) {
+      updateTriggerBranchesForServiceBuildAndUpdatePipeline(ringBranches, dir);
+    }
 
     logger.info(`Successfully created ring: ${ringName} for this project!`);
     await exitFn(0);
@@ -95,9 +110,11 @@ export const execute = async (
 };
 
 export const commandDecorator = (command: commander.Command): void => {
-  buildCmd(command, decorator).action(async (ringName: string) => {
-    await execute(ringName, process.cwd(), async (status: number) => {
-      await exitCmd(logger, process.exit, status);
-    });
-  });
+  buildCmd(command, decorator).action(
+    async (ringName: string, opts: CommandOptions) => {
+      await execute(ringName, process.cwd(), opts, async (status: number) => {
+        await exitCmd(logger, process.exit, status);
+      });
+    }
+  );
 };
diff --git a/src/commands/ring/delete.ts b/src/commands/ring/delete.ts
@@ -55,7 +55,9 @@ export const execute = async (
     bedrock.create(projectPath, bedrockWithoutRing);
 
     // Delete ring from all linked service build pipelines' branch triggers
-    const ringBranches = Object.keys(bedrockWithoutRing.rings);
+    const ringBranches = Object.entries(bedrockWithoutRing.rings).map(
+      ([ring, config]) => config.targetBranch || ring
+    );
     for (const { path: servicePath } of bedrockConfig.services) {
       updateTriggerBranchesForServiceBuildAndUpdatePipeline(
         ringBranches,

diff --git a/src/lib/bedrockYaml/bedrockYaml.test.ts b/src/lib/bedrockYaml/bedrockYaml.test.ts
@@ -1,3 +1,4 @@
+/* eslint-disable @typescript-eslint/explicit-function-return-type */
 import uuid = require("uuid/v4");
 import { createTestBedrockYaml } from "../../test/mockFactory";
 import { BedrockFile, HelmConfig, RingConfig } from "../../types";
@@ -109,31 +110,65 @@ describe("Adding a new service to a Bedrock file", () => {
   });
 });
 
-describe("Adding a new ring to an existing bedrock.yaml", () => {
-  it("should update existing bedrock.yaml with a new service and its helm chart config", () => {
-    const defaultBedrockFileObject = createTestBedrockYaml(
-      false
-    ) as BedrockFile;
-
-    // "" means that bedrock.yaml is written to a random directory
-    const dir = create("", defaultBedrockFileObject);
+describe("addNewRing", () => {
+  const defaultBedrockFileObject = createTestBedrockYaml(false) as BedrockFile;
+  let bedrockDir: string;
+  beforeEach(() => {
+    bedrockDir = create("", defaultBedrockFileObject);
+  });
 
-    const ringName = "new-ring";
+  const tests: {
+    name: string;
+    input: () => Parameters<typeof addNewRing>;
+    effects: () => void;
+  }[] = [
+    {
+      name:
+        "should update existing bedrock.yaml with a new service and its helm chart config",
+      input: () => [bedrockDir, "test-ring"],
+      effects: (): void => {
+        console.log(bedrockDir);
+        const bedrock = read(bedrockDir);
+        const expected: BedrockFile = {
+          ...bedrock,
+          rings: {
+            ...(defaultBedrockFileObject as BedrockFile).rings,
+            ["test-ring"]: { targetBranch: "test-ring" },
+          },
+        };
+        expect(bedrock).toStrictEqual(expected);
+      },
+    },
 
-    addNewRing(dir, ringName);
+    {
+      name: "should use --target-branch if provided",
+      input: () => [bedrockDir, "test-ring", { targetBranch: "foobar" }],
+      effects: (): void => {
+        const bedrock = read(bedrockDir);
+        const rings = getRings(bedrock);
+        const targetRing = rings.find((r) => r.name === "test-ring");
+        expect(targetRing?.targetBranch).toBe("foobar");
+      },
+    },
 
-    const expected: BedrockFile = {
-      rings: {
-        ...(defaultBedrockFileObject as BedrockFile).rings,
-        [ringName]: {},
+    {
+      name: "should use ringName as targetBranch if targetBranch not provided",
+      input: () => [bedrockDir, "test-ring"],
+      effects: (): void => {
+        const bedrock = read(bedrockDir);
+        const rings = getRings(bedrock);
+        const targetRing = rings.find((r) => r.name === "test-ring");
+        expect(targetRing?.targetBranch).toBe("test-ring");
       },
-      services: [...(defaultBedrockFileObject as BedrockFile).services],
-      variableGroups: [],
-      version: defaultBedrockFileObject.version,
-    };
+    },
+  ];
 
-    expect(read(dir)).toEqual(expected);
-  });
+  for (const test of tests) {
+    it(test.name, () => {
+      addNewRing(...test.input());
+      test.effects();
+    });
+  }
 });
 
 describe("Bedrock file info", () => {

diff --git a/src/lib/bedrockYaml/bedrockYaml.ts b/src/lib/bedrockYaml/bedrockYaml.ts
@@ -162,19 +162,32 @@ export const setDefaultRing = (
 
   create(dir, bedrockFile);
 };
+
 /**
  * Update bedrock.yaml with new ring
  *
  * @param dir Directory where <code>bedrock.yaml</code> file resides.
  * @param ringName ring to be added.
  */
-export const addNewRing = (dir: string, ringName: string): void => {
+export const addNewRing = (
+  dir: string,
+  ringName: string,
+  opts: RingConfig = {}
+): void => {
   const absPath = path.resolve(dir);
-  const data: BedrockFile = read(absPath);
-
-  data.rings[ringName] = {}; // Alternatively, we can set isDefault = false or some passable value.
+  const currentBedrock: BedrockFile = read(absPath);
+  const newBedrock = {
+    ...currentBedrock,
+    rings: {
+      ...currentBedrock.rings,
+      [ringName]: {
+        targetBranch: ringName,
+        ...opts,
+      },
+    },
+  };
 
-  const asYaml = yaml.safeDump(data, {
+  const asYaml = yaml.safeDump(newBedrock, {
     lineWidth: Number.MAX_SAFE_INTEGER,
   });
   fs.writeFileSync(path.join(absPath, YAML_NAME), asYaml);

diff --git a/src/types.d.ts b/src/types.d.ts
@@ -44,7 +44,8 @@ export interface Rings {
 }
 
 export interface RingConfig {
-  isDefault?: boolean; // indicates the branch is a default branch to PR against when creating a service revision
+  isDefault?: boolean; // indicates the branch is a default branch to PR against when creating a service revision,
+  targetBranch?: string; // the branch this ring maps to; if not present defaults to the key which maps to this RingConfig
 }
 
 /**

diff --git a/tests/validations.sh b/tests/validations.sh
@@ -389,16 +389,17 @@ git pull
 hld_repo_commit_id=$(git log --format="%H" -n 1)
 verify_pipeline_with_poll_and_source_version $AZDO_ORG_URL $AZDO_PROJECT $hld_to_manifest_pipeline_name 500 15 $hld_repo_commit_id
 ring_name=qa-ring
+branch_name="qa-target-branch"
 
 cd $TEST_WORKSPACE
 cd $mono_repo_dir
 
 echo "Create ring"
 git checkout master
 git pull origin master
-bedrock ring create $ring_name
+bedrock ring create $ring_name --target-branch $branch_name
 git add -A
-git commit -m "Adding test ring: $ring_name"
+git commit -m "Adding test ring '$ring_name' --target-branch '$branch_name'"
 git push -u origin --all
 
 # Wait for the lifecycle pipeline to finish and approve the pull request
@@ -438,13 +439,13 @@ echo "Successfully created ring: $ring_name."
 echo "Update ring."
 cd $TEST_WORKSPACE
 cd $mono_repo_dir
-git branch $ring_name
-git checkout $ring_name
+git branch $branch_name
+git checkout $branch_name
 cd services/$FrontEnd
 echo "Ring doc" >> ringDoc.md
 git add ringDoc.md
 git commit -m "Adding ring doc file"
-git push --set-upstream origin $ring_name
+git push --set-upstream origin $branch_name
 mono_repo_commit_id=$(git log --format="%H" -n 1)
 
 # Verify frontend service pipeline run was successful
@@ -525,6 +526,7 @@ fi
 echo "Successfully deleted ring: $ring_name."
 
 
+
 echo "Successfully reached the end of the service validations scripts."
 
 ##################################