From dc9f08db95ee2b7875b7fc3b4abfca45895094e0 Mon Sep 17 00:00:00 2001 From: Evan Louie Date: Thu, 30 Apr 2020 01:51:05 -0700 Subject: [PATCH] Add branch mapping to rings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - 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 └── ├── config │ └── common.yaml ├── component.yaml └── # 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 https://github.com/microsoft/bedrock/issues/1313 --- docs/commands/data.json | 7 +++ src/commands/hld/reconcile.ts | 18 +++--- src/commands/ring/create.decorator.json | 9 ++- src/commands/ring/create.test.ts | 13 +++-- src/commands/ring/create.ts | 39 +++++++++---- src/commands/ring/delete.ts | 4 +- src/lib/bedrockYaml/bedrockYaml.test.ts | 75 ++++++++++++++++++------- src/lib/bedrockYaml/bedrockYaml.ts | 23 ++++++-- src/lib/fileutils.ts | 2 +- src/types.d.ts | 3 +- tests/validations.sh | 12 ++-- 11 files changed, 148 insertions(+), 57 deletions(-) diff --git a/docs/commands/data.json b/docs/commands/data.json index c2d91183..8375cdc1 100644 --- a/docs/commands/data.json +++ b/docs/commands/data.json @@ -531,6 +531,13 @@ "command": "create ", "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 ", + "description": "The target branch this ring will map to; defaults to 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 index f9048546..c04d14a4 100644 --- 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 => { - // 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 => { 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 index d4fff2fd..843ebc32 100644 --- a/src/commands/ring/create.decorator.json +++ b/src/commands/ring/create.decorator.json @@ -1,5 +1,12 @@ { "command": "create ", "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 ", + "description": "The target branch this ring will map to; defaults to if not provided.", + "defaultValue": "" + } + ] } diff --git a/src/commands/ring/create.test.ts b/src/commands/ring/create.test.ts index bb2244be..565ac7ad 100644 --- a/src/commands/ring/create.test.ts +++ b/src/commands/ring/create.test.ts @@ -59,14 +59,19 @@ 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]]); @@ -74,7 +79,7 @@ describe("test execute function and logic", () => { 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 index 9ebe1336..e268957c 100644 --- 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 ): Promise => { try { @@ -65,23 +70,33 @@ export const execute = async ( logger.info(`Project path: ${projectPath}`); dns.assertIsValid("", 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("", 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 index 01945d02..f06ae011 100644 --- 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(bedrockConfig.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 index 14fc6789..c1f7fa87 100644 --- 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; + 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 index 09e3dff8..c5f07c44 100644 --- 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 bedrock.yaml 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/lib/fileutils.ts b/src/lib/fileutils.ts index 7095dbca..76202675 100644 --- a/src/lib/fileutils.ts +++ b/src/lib/fileutils.ts @@ -486,7 +486,7 @@ export const updateTriggerBranchesForServiceBuildAndUpdatePipeline = ( SERVICE_PIPELINE_FILENAME ); - if (buildPipelineYaml.trigger && buildPipelineYaml.trigger.branches) { + if (buildPipelineYaml.trigger?.branches) { buildPipelineYaml.trigger.branches.include = ringBranches; } diff --git a/src/types.d.ts b/src/types.d.ts index 189fa7e4..7664525e 100644 --- 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 index 443ed69f..b8832ad5 100644 --- a/tests/validations.sh +++ b/tests/validations.sh @@ -389,6 +389,7 @@ 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 @@ -396,9 +397,9 @@ 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." ##################################