From b4a5c8fa69089df009262ba534a8e79e775174b2 Mon Sep 17 00:00:00 2001 From: Evan Louie Date: Sun, 29 Mar 2020 22:30:47 -0700 Subject: [PATCH 01/14] bedrock.yaml refactor & service create spec update - Updated bedrock.yaml `services` to contain a list of services instead of a map indexed by path. - Auto-migration of bedrock.yaml is hooked into `read()` function of the `bedrockYaml.ts` file. - `read()` is now the only function used for loading the bedrock.yaml file; `Bedrock()` in `config.ts` is now noted as deprecated and now calls `read()` under the hood. - `path` property for a service now tracks the path to the service. - Documentation references all updated. - `spk service create ` now follows the spec `spk service create ` - Documentation references all updated. - Integration tests updated. --- docs/commands/data.json | 11 +- guides/auth-private-helm-repos.md | 6 +- guides/bedrock-end-to-end-dx.md | 15 +- guides/building-helm-charts-for-spk.md | 12 +- guides/manual-guide-to-rings.md | 4 +- guides/project-service-management-guide.md | 10 +- guides/rings-101.md | 6 +- package.json | 2 +- src/commands/hld/reconcile.md | 2 +- src/commands/hld/reconcile.test.ts | 65 ++-- src/commands/hld/reconcile.ts | 5 +- .../project/create-variable-group.test.ts | 26 +- src/commands/project/create-variable-group.ts | 6 +- src/commands/project/init.test.ts | 10 +- src/commands/project/init.ts | 5 +- src/commands/project/pipeline.test.ts | 2 +- src/commands/ring/create.md | 26 +- src/commands/ring/create.test.ts | 11 +- src/commands/ring/delete.md | 26 +- src/commands/ring/set-default.md | 26 +- src/commands/ring/set-default.test.ts | 2 +- src/commands/service/create-revision.test.ts | 22 +- src/commands/service/create.decorator.json | 7 +- src/commands/service/create.md | 4 +- src/commands/service/create.test.ts | 167 ++++------ src/commands/service/create.ts | 48 +-- src/config.test.ts | 17 +- src/config.ts | 20 +- src/lib/{ => bedrockYaml}/bedrockYaml.test.ts | 77 ++++- src/lib/{ => bedrockYaml}/bedrockYaml.ts | 65 ++-- src/lib/bedrockYaml/index.ts | 1 + .../migrations/service-map-to-list.test.ts | 294 ++++++++++++++++++ .../migrations/service-map-to-list.ts | 94 ++++++ src/lib/fileutils.ts | 8 +- src/lib/setup/scaffold.ts | 3 +- src/test/mockFactory.ts | 13 +- src/types.d.ts | 11 +- tests/functions.sh | 6 +- tests/validations.sh | 6 +- 39 files changed, 772 insertions(+), 369 deletions(-) rename src/lib/{ => bedrockYaml}/bedrockYaml.test.ts (81%) rename src/lib/{ => bedrockYaml}/bedrockYaml.ts (85%) create mode 100644 src/lib/bedrockYaml/index.ts create mode 100644 src/lib/bedrockYaml/migrations/service-map-to-list.test.ts create mode 100644 src/lib/bedrockYaml/migrations/service-map-to-list.ts diff --git a/docs/commands/data.json b/docs/commands/data.json index 1395e2ca6..9a25e537a 100644 --- a/docs/commands/data.json +++ b/docs/commands/data.json @@ -318,7 +318,7 @@ "alias": "r", "description": "Reconcile a HLD with the services tracked in bedrock.yaml.", "options": [], - "markdown": "## Description\n\nThe reconcile feature scaffolds a HLD with the services in the `bedrock.yaml`\nfile at the root level of the application repository. Recall that in a\nmono-repo, `spk service create` will add an entry into the `bedrock.yaml`\ncorresponding to all tracked services. When the service has been merged into\n`master` of the application repository, a pipeline (see `hld-lifecycle.yaml`,\ncreated by `spk project init`) runs `spk hld reconcile` to add any _new_\nservices tracked in `bedrock.yaml` to the HLD.\n\nThis command is _intended_ to be run in a pipeline (see the generated\n`hld-lifecycle.yaml` created from `spk project init`), but can be run by the\nuser in a CLI for verification.\n\nFor a `bedrock.yaml` file that contained within the\n`https://dev.azure.com/foo/bar/_git` repository, that has the following\nstructure:\n\n```yaml\nrings:\n master:\n isDefault: true\nservices:\n ./services/fabrikam:\n displayName: \"fabrikam\"\n k8sBackendPort: 8001\n k8sBackend: \"fabrikam-k8s-svc\"\n pathPrefix: \"fabrikam-service\"\n pathPrefixMajorVersion: \"v1\"\n helm:\n chart:\n branch: master\n git: \"https://dev.azure.com/foo/bar/_git\"\n path: stable/fabrikam-application\n middlewares:\n - \"\"\nvariableGroups:\n - fabrikam-vg\n```\n\nA HLD is produced that resembles the following:\n\n```\n├── component.yaml\n└── fabrikam\n ├── access.yaml\n ├── component.yaml\n ├── config\n │ └── common.yaml\n └── fabrikam\n ├── component.yaml\n ├── config\n │ └── common.yaml\n └── master\n ├── component.yaml\n ├── config\n │ └── common.yaml\n └── static\n ├── ingress-route.yaml\n └── middlewares.yaml\n```\n\nWith the `ingress-route.yaml` representing a\n[Traefik2 Ingress Route](https://docs.traefik.io/routing/providers/kubernetes-crd/#kind-ingressroute)\nbacked by a Kubernetes Service, and the `middlewares.yaml` representing a\n[Traefik2 Middleware](https://docs.traefik.io/routing/providers/kubernetes-crd/#kind-middleware)\nthat strips path prefixes.\n\nFor the `bedrock.yaml` shown above, the `ingress-route.yaml` produced is:\n\n```yaml\napiVersion: traefik.containo.us/v1alpha1\nkind: IngressRoute\nmetadata:\n name: fabrikam-master\nspec:\n routes:\n - kind: Rule\n match: \"PathPrefix(`/v1/fabrikam-service`) && Headers(`Ring`, `master`)\"\n middlewares:\n - name: fabrikam-master\n services:\n - name: fabrikam-k8s-svc-master\n port: 8001\n```\n\nAnd the `middlewares.yaml` produced is:\n\n```yaml\napiVersion: traefik.containo.us/v1alpha1\nkind: Middleware\nmetadata:\n name: fabrikam-master\nspec:\n stripPrefix:\n forceSlash: false\n prefixes:\n - /v1/fabrikam-service\n```\n\nNote that there exists a third generated file, `access.yaml`. For the above\n`bedrock.yaml`, `access.yaml` contains a single line, which represents a\n[Fabrikate access.yaml definition](https://github.com/microsoft/fabrikate/blob/master/docs/auth.md#accessyaml),\nallowing Fabrikate to pull Helm Charts that are contained within the same\napplication repository:\n\n```yaml\n\"https://dev.azure.com/foo/bar/_git\": ACCESS_TOKEN_SECRET\n```\n\nWhen `fabrikate` is invoked in the HLD to Manifest pipeline, it will utilize the\n`ACCESS_TOKEN_SECRET` environment variable injected at pipeline run-time as a\nPersonal Access Token to pull any referenced helm charts from the application\nrepository.\n" + "markdown": "## Description\n\nThe reconcile feature scaffolds a HLD with the services in the `bedrock.yaml`\nfile at the root level of the application repository. Recall that in a\nmono-repo, `spk service create` will add an entry into the `bedrock.yaml`\ncorresponding to all tracked services. When the service has been merged into\n`master` of the application repository, a pipeline (see `hld-lifecycle.yaml`,\ncreated by `spk project init`) runs `spk hld reconcile` to add any _new_\nservices tracked in `bedrock.yaml` to the HLD.\n\nThis command is _intended_ to be run in a pipeline (see the generated\n`hld-lifecycle.yaml` created from `spk project init`), but can be run by the\nuser in a CLI for verification.\n\nFor a `bedrock.yaml` file that contained within the\n`https://dev.azure.com/foo/bar/_git` repository, that has the following\nstructure:\n\n```yaml\nrings:\n master:\n isDefault: true\nservices:\n - path: ./services/fabrikam\n displayName: \"fabrikam\"\n k8sBackendPort: 8001\n k8sBackend: \"fabrikam-k8s-svc\"\n pathPrefix: \"fabrikam-service\"\n pathPrefixMajorVersion: \"v1\"\n helm:\n chart:\n branch: master\n git: \"https://dev.azure.com/foo/bar/_git\"\n path: stable/fabrikam-application\n middlewares:\n - \"\"\nvariableGroups:\n - fabrikam-vg\n```\n\nA HLD is produced that resembles the following:\n\n```\n├── component.yaml\n└── fabrikam\n ├── access.yaml\n ├── component.yaml\n ├── config\n │ └── common.yaml\n └── fabrikam\n ├── component.yaml\n ├── config\n │ └── common.yaml\n └── master\n ├── component.yaml\n ├── config\n │ └── common.yaml\n └── static\n ├── ingress-route.yaml\n └── middlewares.yaml\n```\n\nWith the `ingress-route.yaml` representing a\n[Traefik2 Ingress Route](https://docs.traefik.io/routing/providers/kubernetes-crd/#kind-ingressroute)\nbacked by a Kubernetes Service, and the `middlewares.yaml` representing a\n[Traefik2 Middleware](https://docs.traefik.io/routing/providers/kubernetes-crd/#kind-middleware)\nthat strips path prefixes.\n\nFor the `bedrock.yaml` shown above, the `ingress-route.yaml` produced is:\n\n```yaml\napiVersion: traefik.containo.us/v1alpha1\nkind: IngressRoute\nmetadata:\n name: fabrikam-master\nspec:\n routes:\n - kind: Rule\n match: \"PathPrefix(`/v1/fabrikam-service`) && Headers(`Ring`, `master`)\"\n middlewares:\n - name: fabrikam-master\n services:\n - name: fabrikam-k8s-svc-master\n port: 8001\n```\n\nAnd the `middlewares.yaml` produced is:\n\n```yaml\napiVersion: traefik.containo.us/v1alpha1\nkind: Middleware\nmetadata:\n name: fabrikam-master\nspec:\n stripPrefix:\n forceSlash: false\n prefixes:\n - /v1/fabrikam-service\n```\n\nNote that there exists a third generated file, `access.yaml`. For the above\n`bedrock.yaml`, `access.yaml` contains a single line, which represents a\n[Fabrikate access.yaml definition](https://github.com/microsoft/fabrikate/blob/master/docs/auth.md#accessyaml),\nallowing Fabrikate to pull Helm Charts that are contained within the same\napplication repository:\n\n```yaml\n\"https://dev.azure.com/foo/bar/_git\": ACCESS_TOKEN_SECRET\n```\n\nWhen `fabrikate` is invoked in the HLD to Manifest pipeline, it will utilize the\n`ACCESS_TOKEN_SECRET` environment variable injected at pipeline run-time as a\nPersonal Access Token to pull any referenced helm charts from the application\nrepository.\n" }, "infra generate": { "command": "generate", @@ -517,7 +517,7 @@ "markdown": "## Description\n\nGenerate a PR in Azure DevOps against default ring branches\n" }, "service create": { - "command": "create ", + "command": "create ", "alias": "c", "description": "Add a new service into this initialized spk project repository", "options": [ @@ -556,11 +556,6 @@ "description": "The directory containing the mono-repo packages.", "defaultValue": "" }, - { - "arg": "-n, --display-name ", - "description": "Display name of the service.", - "defaultValue": "" - }, { "arg": "-m, --maintainer-name ", "description": "The name of the primary maintainer for this service.", @@ -602,7 +597,7 @@ "defaultValue": "" } ], - "markdown": "## Description\n\nAdd a new service into this initialized spk project repository.\n\n## Example\n\n```bash\nspk service create . \\\n --display-name $app_name \\\n --helm-config-path $path_to_chart_in_repo \\\n --helm-config-git $helm_repo_url \\ # Needs to start with https and not contain user name\n --helm-config-branch master \\\n --helm-chart-access-token-variable $ENV_VAR_NAME\n```\n\n## Note\n\n- `--helm-chart-*` and `--helm-config-*` settings are mutually-exclusive. **You\n may only use one.**\n - If the git repository referenced in `--helm-config-git` is a private\n repository, you can specify an environment variable in your\n HLD-to-Materialized pipeline containing your a PAT to authenticate with via\n the `--helm-chart-access-token-variable` option.\n- `--middlewares`, `--k8s-backend-port`, `--path-prefix`,\n `--path-prefix-major-version`, and `--k8s-backend` are all used to configure\n the generated Traefik2 IngressRoutes. i.e.\n\n ```sh\n spk service create my-example-documents-service \\\n --middlewares middleware \\\n --k8s-backend-port 3001 \\\n --k8s-backend docs-service \\\n --path-prefix documents \\\n --path-prefix-major-version v2\n ```\n\n will result in an IngressRoute that looks like:\n\n ```yaml\n apiVersion: traefik.containo.us/v1alpha1\n kind: IngressRoute\n metadata:\n name: my-example-documents-service-master\n spec:\n routes:\n - kind: Rule\n match: \"PathPrefix(`/v2/documents`) && Headers(`Ring`, `master`)\"\n middlewares:\n - name: my-example-documents-service-master\n - name: middlewareA\n services:\n - name: docs-service\n port: 3001\n ```\n" + "markdown": "## Description\n\nAdd a new service into this initialized spk project repository.\n\n## Example\n\n```bash\nspk service create my-service . \\\n --display-name $app_name \\\n --helm-config-path $path_to_chart_in_repo \\\n --helm-config-git $helm_repo_url \\ # Needs to start with https and not contain user name\n --helm-config-branch master \\\n --helm-chart-access-token-variable $ENV_VAR_NAME\n```\n\n## Note\n\n- `--helm-chart-*` and `--helm-config-*` settings are mutually-exclusive. **You\n may only use one.**\n - If the git repository referenced in `--helm-config-git` is a private\n repository, you can specify an environment variable in your\n HLD-to-Materialized pipeline containing your a PAT to authenticate with via\n the `--helm-chart-access-token-variable` option.\n- `--middlewares`, `--k8s-backend-port`, `--path-prefix`,\n `--path-prefix-major-version`, and `--k8s-backend` are all used to configure\n the generated Traefik2 IngressRoutes. i.e.\n\n ```sh\n spk service create my-example-documents-service path/to/my/service \\\n --middlewares middleware \\\n --k8s-backend-port 3001 \\\n --k8s-backend docs-service \\\n --path-prefix documents \\\n --path-prefix-major-version v2\n ```\n\n will result in an IngressRoute that looks like:\n\n ```yaml\n apiVersion: traefik.containo.us/v1alpha1\n kind: IngressRoute\n metadata:\n name: my-example-documents-service-master\n spec:\n routes:\n - kind: Rule\n match: \"PathPrefix(`/v2/documents`) && Headers(`Ring`, `master`)\"\n middlewares:\n - name: my-example-documents-service-master\n - name: middlewareA\n services:\n - name: docs-service\n port: 3001\n ```\n" }, "service install-build-pipeline": { "command": "install-build-pipeline ", diff --git a/guides/auth-private-helm-repos.md b/guides/auth-private-helm-repos.md index 91fcf0967..bac5c21b9 100644 --- a/guides/auth-private-helm-repos.md +++ b/guides/auth-private-helm-repos.md @@ -36,7 +36,7 @@ can make it aware of it via the `--helm-config-access-token-variable` flag in the `spk service create` command. ```sh -spk service create my-service \ +spk service create my-service ./path/to/service \ --helm-config-git https://dev.azure.com/my-org/my-project/_git/my-repo \ --helm-config-branch master \ --helm-config-path my-service-helm-chart \ @@ -57,8 +57,8 @@ rings: master: isDefault: true services: - ./my-service: - displayName: "" + - path: ./path/to/service + displayName: my-service helm: chart: accessTokenVariable: MY_ENV_VAR # Note: this is where the environment variable gets tracked diff --git a/guides/bedrock-end-to-end-dx.md b/guides/bedrock-end-to-end-dx.md index 04efe0924..0d2149903 100644 --- a/guides/bedrock-end-to-end-dx.md +++ b/guides/bedrock-end-to-end-dx.md @@ -72,7 +72,7 @@ The core `discovery-service` microservice already exists, so he grandfathers it into the Bedrock workflow with: ```bash -$ spk service create discovery-service -d services +$ spk service create discovery-service ./path/to/discovery/service -d services ``` This updates the bedrock.yaml file to include this service: @@ -80,13 +80,12 @@ This updates the bedrock.yaml file to include this service: ```bash $ cat bedrock.yaml services: - ./services/discovery-service: - helm: - chart: - branch: '' - git: '' - path: '' - + - path: ./services/discovery-service + helm: + chart: + branch: '' + git: '' + path: '' ``` and adds a `build-update-hld.yaml` file in `services/discovery-service` to build diff --git a/guides/building-helm-charts-for-spk.md b/guides/building-helm-charts-for-spk.md index e5053593c..63461f36c 100644 --- a/guides/building-helm-charts-for-spk.md +++ b/guides/building-helm-charts-for-spk.md @@ -38,8 +38,8 @@ rings: dev: isDefault: true services: - ./: - displayName: "fabrikam" + - displayName: "fabrikam" + path: ./ helm: chart: branch: master @@ -58,7 +58,7 @@ The above service `fabrikam` was added to `bedrock.yaml` by invoking `spk service create` with the requisite parameters ie: ```sh -spk service create . \ +spk service create fabrikam . \ --display-name fabrikam \ --helm-config-git https://dev.azure.com/fabrikam/frontend/_git/charts \ --helm-config-path frontend \ @@ -152,9 +152,9 @@ rings: dev: isDefault: true services: - ./ - ... + - path: ./ k8sBackend: 'fabrikam-k8s-svc' + ... ``` The `serviceName` is generated from a combination of the `k8sBackend`, @@ -179,7 +179,7 @@ rings: dev: isDefault: true services: - ./ + - path: ./ ... displayName: 'fabrikam' k8sBackend: 'fabrikam-k8s-svc' diff --git a/guides/manual-guide-to-rings.md b/guides/manual-guide-to-rings.md index 885e131ef..44081e815 100644 --- a/guides/manual-guide-to-rings.md +++ b/guides/manual-guide-to-rings.md @@ -35,7 +35,7 @@ rings: qa: prod: services: - ./: + - path: ./ displayName: "fabrikam" helm: chart: @@ -87,7 +87,7 @@ rings: prod: test-new-homepage: <-- NEW --> services: - ./: + - path: ./ displayName: "fabrikam" helm: chart: diff --git a/guides/project-service-management-guide.md b/guides/project-service-management-guide.md index 9b89c2fdb..095defa47 100644 --- a/guides/project-service-management-guide.md +++ b/guides/project-service-management-guide.md @@ -235,7 +235,7 @@ application repositories flag parameters can be used if `spk` was not intialized) ``` SERVICE_NAME= - spk service create . --display-name $SERVICE_NAME ... + spk service create $SERVICE_NAME . ... git add -A git commit -m "Adding $SERVICE_NAME to the repository." git push -u origin --all @@ -286,8 +286,8 @@ As an `spk` user, if you would like to incorporate helm charts from a well known public repository, you may simply run `spk` the following `helm-chart` arguments: -``` -spk service create --helm-chart-chart stable/nginx --helm-chart-repository github.com/helm/charts +```sh +spk service create nginx my-nginx-service --helm-chart-chart stable/nginx --helm-chart-repository github.com/helm/charts ``` ##### Helm Charts in a distinct Git Repository from Application Sources in the same Azure DevOps Project @@ -296,7 +296,7 @@ If your Helm Charts are in their own distinct Git Repository in the _same_ Azure DevOps project, you can use the `helm-config` arguments to configure `spk`: ``` -spk service create \ +spk service create fabrikam path/to/fabrikam \ --helm-config-git https://dev.azure.com/fabrikam/fabrikam-project/_git/fabrikam-helm-charts --helm-config-branch master \ --helm-path /charts/fabrikam @@ -340,7 +340,7 @@ the environment variable containing the Personal Access Token to access the git repository in `helm-config-git`: ``` -spk service create \ +spk service create fabrikam path/to/fabrikam \ --helm-config-git https://dev.azure.com/fabrikam/fabrikam-helm-charts-project/_git/fabrikam-helm-charts \ --helm-config-branch master \ --helm-path /charts/fabrikam \ diff --git a/guides/rings-101.md b/guides/rings-101.md index 611916b86..58f2d984f 100644 --- a/guides/rings-101.md +++ b/guides/rings-101.md @@ -136,7 +136,7 @@ rings: isDefault: false qa: {} # isDefault not present is same as isDefault: false services: - ./my-service-foo: + - path: my-service-foo displayName: fancy-service helm: chart: @@ -212,7 +212,7 @@ rings: develop: isDefault: false services: - ./foo: + - path: ./foo displayName: "" helm: chart: @@ -225,7 +225,7 @@ services: middlewares: [] pathPrefix: "" pathPrefixMajorVersion: "" - ./bar: + - path: bar displayName: "" helm: chart: diff --git a/package.json b/package.json index 20359968b..75644f064 100644 --- a/package.json +++ b/package.json @@ -13,7 +13,7 @@ "md-lint": "find guides -name \\*.md | xargs -n 1 markdown-link-check", "format": "prettier --write 'src/**/*.ts{,x}'", "test": "jest --rootDir src --reporters=jest-junit --reporters=default --coverage --coverageReporters=cobertura --coverageReporters=html", - "test-watch": "jest --watchAll", + "test-watch": "jest --rootDir src --watchAll", "postinstall": "cd node_modules/azure-devops-node-api && git apply ../../patches/001-azure-devops-node.patch || true", "test-coverage-html": "jest --coverage --coverageReporters=html" }, diff --git a/src/commands/hld/reconcile.md b/src/commands/hld/reconcile.md index 392711c1b..485837ab2 100644 --- a/src/commands/hld/reconcile.md +++ b/src/commands/hld/reconcile.md @@ -21,7 +21,7 @@ rings: master: isDefault: true services: - ./services/fabrikam: + - path: ./services/fabrikam displayName: "fabrikam" k8sBackendPort: 8001 k8sBackend: "fabrikam-k8s-svc" diff --git a/src/commands/hld/reconcile.test.ts b/src/commands/hld/reconcile.test.ts index 4d50ca2cd..06dbfbc31 100644 --- a/src/commands/hld/reconcile.test.ts +++ b/src/commands/hld/reconcile.test.ts @@ -250,6 +250,7 @@ describe("addChartToRing", () => { const chartPath = "/charts/service"; const serviceConfig: BedrockServiceConfig = { + path: "./", helm: { chart: { branch, @@ -278,6 +279,7 @@ describe("addChartToRing", () => { const chartPath = "/charts/service"; const serviceConfig: BedrockServiceConfig = { + path: "./", helm: { chart: { git, @@ -305,6 +307,7 @@ describe("addChartToRing", () => { const chart = "/charts/service"; const serviceConfig: BedrockServiceConfig = { + path: "./", helm: { chart: { chart, @@ -333,6 +336,7 @@ describe("addChartToRing", () => { const chart = "/charts/service"; const serviceConfig: BedrockServiceConfig = { + path: "./", helm: { chart: { chart, @@ -354,6 +358,7 @@ describe("configureChartForRing", () => { const ringName = "myringname"; const normalizedServiceName = "my-great-service"; const serviceConfig: BedrockServiceConfig = { + path: "./", helm: { chart: { git: "foo", @@ -383,6 +388,7 @@ describe("configureChartForRing", () => { it("should invoke the correct command and calculate the k8s service name from the bedrock service name if there is no k8sbackend configured.", async () => { const serviceConfigNoK8sBackend: BedrockServiceConfig = { + path: "./", helm: { chart: { git: "foo", @@ -456,8 +462,9 @@ describe("reconcile tests", () => { }, prod: {}, }, - services: { - "./path/to/a/svc/": { + services: [ + { + path: "./path/to/a/svc/", disableRouteScaffold: false, helm: { chart: { @@ -470,7 +477,7 @@ describe("reconcile tests", () => { k8sBackend: "cool-service", k8sBackendPort: 1337, }, - }, + ], version: "1.0", }; }); @@ -526,8 +533,9 @@ describe("reconcile tests", () => { isDefault: true, }, }, - services: { - "./path/to/svc/": { + services: [ + { + path: "./path/to/svc/", disableRouteScaffold: true, helm: { chart: { @@ -538,7 +546,7 @@ describe("reconcile tests", () => { }, k8sBackendPort: 1337, }, - }, + ], version: "1.0", }; @@ -606,8 +614,9 @@ describe("reconcile tests", () => { }); it("does not create service components if the service path is `.`, and a display name does not exist", async () => { - bedrockYaml.services = { - ".": { + bedrockYaml.services = [ + { + path: ".", disableRouteScaffold: false, helm: { chart: { @@ -618,7 +627,7 @@ describe("reconcile tests", () => { }, k8sBackendPort: 80, }, - }; + ]; await reconcileHld( dependencies, @@ -634,8 +643,9 @@ describe("reconcile tests", () => { it("does create service components if the service path is `.` and a display name does exist", async () => { const displayName = "fabrikam"; - bedrockYaml.services = { - ".": { + bedrockYaml.services = [ + { + path: ".", disableRouteScaffold: false, displayName, helm: { @@ -647,7 +657,7 @@ describe("reconcile tests", () => { }, k8sBackendPort: 80, }, - }; + ]; await reconcileHld( dependencies, @@ -667,8 +677,9 @@ describe("reconcile tests", () => { it("uses display name over the service path for creating service components", async () => { const displayName = "fabrikam"; - bedrockYaml.services = { - "/my/service/path": { + bedrockYaml.services = [ + { + path: "/my/service/path", disableRouteScaffold: false, displayName, helm: { @@ -680,7 +691,7 @@ describe("reconcile tests", () => { }, k8sBackendPort: 80, }, - }; + ]; await reconcileHld( dependencies, @@ -700,18 +711,22 @@ describe("reconcile tests", () => { it("properly updates access.yaml", async () => { const anotherGit = "github.com/foobar/baz"; const anotherToken = "MY_FANCY_ENV_VAR"; - bedrockYaml.services["another/service"] = { - disableRouteScaffold: false, - helm: { - chart: { - accessTokenVariable: anotherToken, - git: anotherGit, - path: "path/to/chart", - sha: "12345", + bedrockYaml.services = [ + ...bedrockYaml.services, + { + path: "another/service", + disableRouteScaffold: false, + helm: { + chart: { + accessTokenVariable: anotherToken, + git: anotherGit, + path: "path/to/chart", + sha: "12345", + }, }, + k8sBackendPort: 8888, }, - k8sBackendPort: 8888, - }; + ]; const pathToHLD = "./the/path/to/hld"; const service = "service"; await reconcileHld( diff --git a/src/commands/hld/reconcile.ts b/src/commands/hld/reconcile.ts index e715f97ca..696168a89 100644 --- a/src/commands/hld/reconcile.ts +++ b/src/commands/hld/reconcile.ts @@ -476,9 +476,8 @@ export const reconcileHld = async ( normalizedAbsRepositoryInHldPath ); - for (const [serviceRelPath, serviceConfig] of Object.entries( - managedServices - )) { + for (const serviceConfig of managedServices) { + const serviceRelPath = serviceConfig.path; const serviceName = serviceConfig.displayName || path.basename(serviceRelPath); diff --git a/src/commands/project/create-variable-group.test.ts b/src/commands/project/create-variable-group.test.ts index 14b4e1e5e..ccbbb2a43 100644 --- a/src/commands/project/create-variable-group.test.ts +++ b/src/commands/project/create-variable-group.test.ts @@ -3,13 +3,9 @@ import yaml from "js-yaml"; import mockFs from "mock-fs"; import path from "path"; import uuid from "uuid/v4"; -import { readYaml, write } from "../../config"; +import { readYaml } from "../../config"; import * as config from "../../config"; -import { - create as createBedrockYaml, - isExists as isBedrockFileExists, - read as readBedrockFile, -} from "../../lib/bedrockYaml"; +import * as bedrockYaml from "../../lib/bedrockYaml"; import * as commandBuilder from "../../lib/commandBuilder"; import { PROJECT_PIPELINE_FILENAME, @@ -243,12 +239,12 @@ describe("setVariableGroupInBedrockFile", () => { test("Should pass adding a valid variable group name when bedrock file exists with empty variableGroups", async () => { // Create random directory to initialize - const randomTmpDir = createBedrockYaml(); + const randomTmpDir = bedrockYaml.create(); await setVariableGroupInBedrockFile(randomTmpDir, variableGroupName); - expect(isBedrockFileExists(randomTmpDir)).toBe(true); - const bedrockFile = readBedrockFile(randomTmpDir); + expect(bedrockYaml.isExists(randomTmpDir)).toBe(true); + const bedrockFile = bedrockYaml.read(randomTmpDir); logger.info(`filejson: ${JSON.stringify(bedrockFile)}`); expect(bedrockFile.variableGroups).toBeDefined(); @@ -264,16 +260,16 @@ describe("setVariableGroupInBedrockFile", () => { logger.info(`prevariableGroupName: ${prevariableGroupName}`); const bedrockFileData: BedrockFile = { 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 + services: [], // service property is not optional so set it to null variableGroups: [prevariableGroupName], version: "", }; - const randomTmpDir = createBedrockYaml("", bedrockFileData); + const randomTmpDir = bedrockYaml.create("", bedrockFileData); await setVariableGroupInBedrockFile(randomTmpDir, variableGroupName); - expect(isBedrockFileExists(randomTmpDir)).toBe(true); + expect(bedrockYaml.isExists(randomTmpDir)).toBe(true); - const bedrockFile = readBedrockFile(randomTmpDir); + const bedrockFile = bedrockYaml.read(randomTmpDir); logger.info(`filejson: ${JSON.stringify(bedrockFile)}`); expect(bedrockFile.variableGroups).toBeDefined(); if (bedrockFile.variableGroups) { @@ -330,7 +326,7 @@ describe("updateLifeCyclePipeline", () => { false ) as BedrockFile; - write(defaultBedrockFileObject, randomTmpDir); + bedrockYaml.create(randomTmpDir, defaultBedrockFileObject); const hldFilePath = path.join(randomTmpDir, PROJECT_PIPELINE_FILENAME); @@ -369,7 +365,7 @@ describe("updateLifeCyclePipeline", () => { variableGroupName, ]; - write(defaultBedrockFileObject, randomTmpDir); + bedrockYaml.create(randomTmpDir, defaultBedrockFileObject); const hldFilePath = path.join(randomTmpDir, PROJECT_PIPELINE_FILENAME); diff --git a/src/commands/project/create-variable-group.ts b/src/commands/project/create-variable-group.ts index 8df593a64..f68cabedc 100644 --- a/src/commands/project/create-variable-group.ts +++ b/src/commands/project/create-variable-group.ts @@ -3,7 +3,7 @@ import commander from "commander"; import path from "path"; import { echo } from "shelljs"; import { Bedrock, Config, readYaml, write } from "../../config"; -import { fileInfo as bedrockFileInfo } from "../../lib/bedrockYaml"; +import * as bedrockYaml from "../../lib/bedrockYaml"; import { build as buildCmd, exit as exitCmd, @@ -42,7 +42,7 @@ interface CommandOptions { } export const checkDependencies = (projectPath: string): void => { - const fileInfo: BedrockFileInfo = bedrockFileInfo(projectPath); + const fileInfo: BedrockFileInfo = bedrockYaml.fileInfo(projectPath); if (fileInfo.exist === false) { throw new Error(PROJECT_INIT_DEPENDENCY_ERROR_MESSAGE); } @@ -160,7 +160,7 @@ export const setVariableGroupInBedrockFile = ( ]; // Write out - write(bedrockFile, absProjectRoot); + bedrockYaml.create(absProjectRoot, bedrockFile); }; /** diff --git a/src/commands/project/init.test.ts b/src/commands/project/init.test.ts index 604c6facb..36b5a8525 100644 --- a/src/commands/project/init.test.ts +++ b/src/commands/project/init.test.ts @@ -2,6 +2,7 @@ import fs from "fs"; import path from "path"; import uuid from "uuid/v4"; import { Bedrock, Maintainers, write } from "../../config"; +import * as bedrockYaml from "../../lib/bedrockYaml"; import { PROJECT_PIPELINE_FILENAME, SERVICE_PIPELINE_FILENAME, @@ -58,8 +59,9 @@ describe("initializing an existing file does not modify it", () => { const randomDir = createTempDir(); const bedrockFile: BedrockFile = { rings: { master: { isDefault: true } }, - services: { - "some/random/dir": { + services: [ + { + path: "some/random/dir", helm: { chart: { git: "foo", @@ -69,10 +71,10 @@ describe("initializing an existing file does not modify it", () => { }, k8sBackendPort: 1337, }, - }, + ], version: "1.0", }; - write(bedrockFile, randomDir); + bedrockYaml.create(randomDir, bedrockFile); await initialize(randomDir); // bedrock file should not have been modified diff --git a/src/commands/project/init.ts b/src/commands/project/init.ts index 76bae6b5e..fc7e6a1a2 100644 --- a/src/commands/project/init.ts +++ b/src/commands/project/init.ts @@ -2,6 +2,7 @@ import commander from "commander"; import fs from "fs"; import path from "path"; import { Bedrock, write } from "../../config"; +import * as bedrockYaml from "../../lib/bedrockYaml"; import { build as buildCmd, exit as exitCmd } from "../../lib/commandBuilder"; import { generateGitIgnoreFile, @@ -38,7 +39,7 @@ const generateBedrockFile = ( }, {} ), - services: {}, + services: [], version: getVersion(), }; @@ -50,7 +51,7 @@ const generateBedrockFile = ( ); } else { // Write out - write(baseBedrockFile, absProjectPath); + bedrockYaml.create(absProjectPath, baseBedrockFile); } }; diff --git a/src/commands/project/pipeline.test.ts b/src/commands/project/pipeline.test.ts index c5fd81321..667eaa130 100644 --- a/src/commands/project/pipeline.test.ts +++ b/src/commands/project/pipeline.test.ts @@ -140,7 +140,7 @@ describe("installLifecyclePipeline and execute tests", () => { const tmpDir = createTempDir(); createBedrockYaml(tmpDir, { rings: {}, - services: {}, + services: [], variableGroups: ["test"], version: "1.0", }); diff --git a/src/commands/ring/create.md b/src/commands/ring/create.md index 042425fdb..50db0dc44 100644 --- a/src/commands/ring/create.md +++ b/src/commands/ring/create.md @@ -13,7 +13,7 @@ rings: qa: prod: services: - ./: + - path: ./ displayName: "fabrikam" helm: chart: @@ -40,18 +40,18 @@ running `spk ring create stage` will result in a few changes: prod: stage: services: - ./: - displayName: "fabrikam" - helm: - chart: - branch: master - git: "https://dev.azure.com/fabrikam/frontend/_git/charts" - path: frontend - k8sBackend: "fabrikam-k8s-svc" - k8sBackendPort: 80 - middlewares: [] - pathPrefix: "fabrikam-service" - pathPrefixMajorVersion: "v1" + - path: ./ + displayName: "fabrikam" + helm: + chart: + branch: master + git: "https://dev.azure.com/fabrikam/frontend/_git/charts" + path: frontend + k8sBackend: "fabrikam-k8s-svc" + k8sBackendPort: 80 + middlewares: [] + pathPrefix: "fabrikam-service" + pathPrefixMajorVersion: "v1" variableGroups: - fabrikam-vg ``` diff --git a/src/commands/ring/create.test.ts b/src/commands/ring/create.test.ts index 7239f1752..70d919ca3 100644 --- a/src/commands/ring/create.test.ts +++ b/src/commands/ring/create.test.ts @@ -32,7 +32,7 @@ describe("checkDependencies", () => { isDefault: true, }, }, - services: {}, + services: [], variableGroups: ["testvg"], version: "1.0", }); @@ -46,7 +46,7 @@ describe("checkDependencies", () => { isDefault: true, }, }, - services: {}, + services: [], variableGroups: ["testvg"], version: "1.0", }); @@ -94,8 +94,9 @@ describe("test execute function and logic", () => { isDefault: true, }, }, - services: { - "./my-service": { + services: [ + { + path: "./my-service", helm: { chart: { branch: "master", @@ -105,7 +106,7 @@ describe("test execute function and logic", () => { }, k8sBackendPort: 80, }, - }, + ], variableGroups: ["testvg"], version: "1.0", }); diff --git a/src/commands/ring/delete.md b/src/commands/ring/delete.md index 7117a3cd3..ee4166b3c 100644 --- a/src/commands/ring/delete.md +++ b/src/commands/ring/delete.md @@ -16,7 +16,7 @@ rings: qa: prod: services: - ./: + - path: ./ displayName: "fabrikam" helm: chart: @@ -41,18 +41,18 @@ running `spk ring delete prod` will result in a few changes: isDefault: true qa: services: - ./: - displayName: "fabrikam" - helm: - chart: - branch: master - git: "https://dev.azure.com/fabrikam/frontend/_git/charts" - path: frontend - k8sBackend: "fabrikam-k8s-svc" - k8sBackendPort: 80 - middlewares: [] - pathPrefix: "fabrikam-service" - pathPrefixMajorVersion: "v1" + - path: ./ + displayName: "fabrikam" + helm: + chart: + branch: master + git: "https://dev.azure.com/fabrikam/frontend/_git/charts" + path: frontend + k8sBackend: "fabrikam-k8s-svc" + k8sBackendPort: 80 + middlewares: [] + pathPrefix: "fabrikam-service" + pathPrefixMajorVersion: "v1" variableGroups: - fabrikam-vg ``` diff --git a/src/commands/ring/set-default.md b/src/commands/ring/set-default.md index 44408920c..e25015c4d 100644 --- a/src/commands/ring/set-default.md +++ b/src/commands/ring/set-default.md @@ -13,7 +13,7 @@ rings: qa: prod: services: - ./: + - path: ./ displayName: "fabrikam" helm: chart: @@ -40,18 +40,18 @@ running `spk ring set-default prod` will result in: prod: isDefault: true services: - ./: - displayName: "fabrikam" - helm: - chart: - branch: master - git: "https://dev.azure.com/fabrikam/frontend/_git/charts" - path: frontend - k8sBackend: "fabrikam-k8s-svc" - k8sBackendPort: 80 - middlewares: [] - pathPrefix: "fabrikam-service" - pathPrefixMajorVersion: "v1" + - path: ./ + displayName: "fabrikam" + helm: + chart: + branch: master + git: "https://dev.azure.com/fabrikam/frontend/_git/charts" + path: frontend + k8sBackend: "fabrikam-k8s-svc" + k8sBackendPort: 80 + middlewares: [] + pathPrefix: "fabrikam-service" + pathPrefixMajorVersion: "v1" variableGroups: - fabrikam-vg ``` diff --git a/src/commands/ring/set-default.test.ts b/src/commands/ring/set-default.test.ts index d12d61607..0ecb95935 100644 --- a/src/commands/ring/set-default.test.ts +++ b/src/commands/ring/set-default.test.ts @@ -41,7 +41,7 @@ describe("test execute function and logic", () => { }, prod: {}, }, - services: {}, + services: [], variableGroups: ["testvg"], version: "1.0", }); diff --git a/src/commands/service/create-revision.test.ts b/src/commands/service/create-revision.test.ts index 74b9175b3..70c8fde05 100644 --- a/src/commands/service/create-revision.test.ts +++ b/src/commands/service/create-revision.test.ts @@ -1,6 +1,6 @@ import { write } from "../../config"; import * as config from "../../config"; -import { DEFAULT_CONTENT as BedrockMockedContent } from "../../lib/bedrockYaml"; +import * as bedrockYaml from "../../lib/bedrockYaml"; import * as azure from "../../lib/git/azure"; import * as gitutils from "../../lib/gitutils"; import { createTempDir } from "../../lib/ioUtil"; @@ -19,7 +19,7 @@ jest .mockReturnValueOnce(Promise.resolve("prod")) .mockReturnValue(Promise.resolve("")); jest.spyOn(config, "Config").mockReturnValue({}); -jest.spyOn(config, "Bedrock").mockReturnValue(BedrockMockedContent); +jest.spyOn(config, "Bedrock").mockReturnValue(bedrockYaml.DEFAULT_CONTENT()); describe("test makePullRequest function", () => { it("sanity test", async (done) => { @@ -52,8 +52,9 @@ describe("Default rings", () => { prod: { isDefault: false }, westus: { isDefault: true }, }, - services: { - "foo/a": { + services: [ + { + path: "foo/a", helm: { chart: { chart: "elastic", @@ -65,11 +66,11 @@ describe("Default rings", () => { pathPrefix: "servicepath", pathPrefixMajorVersion: "v1", }, - }, + ], version: "1.0", }; - write(validBedrockYaml, randomTmpDir); + bedrockYaml.create(randomTmpDir, validBedrockYaml); const defaultRings = getDefaultRings(undefined, validBedrockYaml); expect(defaultRings.length).toBe(2); expect(defaultRings[0]).toBe("master"); @@ -84,8 +85,9 @@ describe("Default rings", () => { prod: { isDefault: false }, westus: { isDefault: false }, }, - services: { - "foo/a": { + services: [ + { + path: "foo/a", helm: { chart: { chart: "elastic", @@ -97,11 +99,11 @@ describe("Default rings", () => { pathPrefix: "servicepath", pathPrefixMajorVersion: "v1", }, - }, + ], version: "1.0", }; - write(validBedrockYaml, randomTmpDir); + bedrockYaml.create(randomTmpDir, validBedrockYaml); let hasError = false; try { diff --git a/src/commands/service/create.decorator.json b/src/commands/service/create.decorator.json index 439621fdc..4608eb952 100644 --- a/src/commands/service/create.decorator.json +++ b/src/commands/service/create.decorator.json @@ -1,5 +1,5 @@ { - "command": "create ", + "command": "create ", "alias": "c", "description": "Add a new service into this initialized spk project repository", "options": [ @@ -38,11 +38,6 @@ "description": "The directory containing the mono-repo packages.", "defaultValue": "" }, - { - "arg": "-n, --display-name ", - "description": "Display name of the service.", - "defaultValue": "" - }, { "arg": "-m, --maintainer-name ", "description": "The name of the primary maintainer for this service.", diff --git a/src/commands/service/create.md b/src/commands/service/create.md index 7e3a41a7e..3b4590ff2 100644 --- a/src/commands/service/create.md +++ b/src/commands/service/create.md @@ -5,7 +5,7 @@ Add a new service into this initialized spk project repository. ## Example ```bash -spk service create . \ +spk service create my-service . \ --display-name $app_name \ --helm-config-path $path_to_chart_in_repo \ --helm-config-git $helm_repo_url \ # Needs to start with https and not contain user name @@ -26,7 +26,7 @@ spk service create . \ the generated Traefik2 IngressRoutes. i.e. ```sh - spk service create my-example-documents-service \ + spk service create my-example-documents-service path/to/my/service \ --middlewares middleware \ --k8s-backend-port 3001 \ --k8s-backend docs-service \ diff --git a/src/commands/service/create.test.ts b/src/commands/service/create.test.ts index c765d5e81..11f0c5347 100644 --- a/src/commands/service/create.test.ts +++ b/src/commands/service/create.test.ts @@ -7,7 +7,6 @@ import uuid = require("uuid/v4"); import { Bedrock } from "../../config"; import * as config from "../../config"; import * as bedrockYaml from "../../lib/bedrockYaml"; -import { DEFAULT_CONTENT as BedrockMockedContent } from "../../lib/bedrockYaml"; import { SERVICE_PIPELINE_FILENAME } from "../../lib/constants"; import { checkoutCommitPushCreatePRLink } from "../../lib/gitutils"; import { createTempDir, removeDir } from "../../lib/ioUtil"; @@ -30,6 +29,7 @@ import { CommandValues, validateGitUrl, } from "./create"; +import { BedrockServiceConfig, HelmConfig } from "../../types"; jest.mock("../../lib/gitutils"); @@ -46,7 +46,6 @@ beforeEach(() => { }); const mockValues: CommandValues = { - displayName: "", gitPush: false, helmChartChart: "", helmChartRepository: "", @@ -74,14 +73,14 @@ const getMockValues = (): CommandValues => { const validateDirNFiles = ( dir: string, - serviceName: string, + servicePath: string, values: CommandValues ): void => { // Check temp test directory exists expect(fs.existsSync(dir)).toBe(true); // Check service directory exists - const serviceDirPath = path.join(dir, values.packagesDir, serviceName); + const serviceDirPath = path.join(dir, values.packagesDir, servicePath); expect(fs.existsSync(serviceDirPath)).toBe(true); // Verify new azure-pipelines created @@ -107,7 +106,9 @@ describe("Test fetchValues function", () => { }); it("Positive test: with middlewares value", () => { - jest.spyOn(config, "Bedrock").mockReturnValueOnce(BedrockMockedContent); + jest + .spyOn(config, "Bedrock") + .mockReturnValueOnce(bedrockYaml.DEFAULT_CONTENT()); const mocked = getMockValues(); mocked.middlewares = "mid1, mid2"; // space after comma is intentional, expecting trimming to happen const result = fetchValues(mocked); @@ -115,7 +116,7 @@ describe("Test fetchValues function", () => { }); it("Positive test: with bedrock rings", () => { - const mockedBedrockFileConfig = { ...BedrockMockedContent }; + const mockedBedrockFileConfig = { ...bedrockYaml.DEFAULT_CONTENT() }; mockedBedrockFileConfig.rings = { master: {}, qa: {}, @@ -129,7 +130,9 @@ describe("Test fetchValues function", () => { it("Positive test", () => { const mocked = getMockValues(); - jest.spyOn(config, "Bedrock").mockReturnValueOnce(BedrockMockedContent); + jest + .spyOn(config, "Bedrock") + .mockReturnValueOnce(bedrockYaml.DEFAULT_CONTENT()); const result = fetchValues(mocked); expect(result).toEqual(mocked); }); @@ -139,7 +142,6 @@ describe("isValidDnsInputs", () => { test("valid inputs does not throw", () => { expect(() => assertValidDnsInputs({ - displayName: "bar", k8sBackend: "my-service", pathPrefix: "service", pathPrefixMajorVersion: "v1", @@ -150,16 +152,6 @@ describe("isValidDnsInputs", () => { test("invalid inputs throws", () => { expect(() => assertValidDnsInputs({ - displayName: "-not_dns_compliant", - k8sBackend: "", - pathPrefix: "", - pathPrefixMajorVersion: "", - }) - ).toThrow(); - - expect(() => - assertValidDnsInputs({ - displayName: "", k8sBackend: "-not_dns_compliant", pathPrefix: "", pathPrefixMajorVersion: "", @@ -168,7 +160,6 @@ describe("isValidDnsInputs", () => { expect(() => assertValidDnsInputs({ - displayName: "", k8sBackend: "", pathPrefix: "-not_dns_compliant", pathPrefixMajorVersion: "", @@ -177,7 +168,6 @@ describe("isValidDnsInputs", () => { expect(() => assertValidDnsInputs({ - displayName: "", k8sBackend: "", pathPrefix: "", pathPrefixMajorVersion: "-not_dns_compliant", @@ -190,30 +180,20 @@ describe("Test execute function", () => { it("Negative test: with non-dns compliant values", async () => { const exitFn = jest.fn(); jest.spyOn(dns, "assertIsValid"); - await execute( - "foo", - { ...getMockValues(), displayName: "-non_dns@compliant" }, - exitFn - ); + await execute("-non_dns@compliant", "foo", { ...getMockValues() }, exitFn); expect(dns.assertIsValid).toHaveBeenCalledTimes(1); }); it("Negative test: without service name", async () => { const exitFn = jest.fn(); - await execute("", getMockValues(), exitFn); - expect(exitFn).toBeCalledTimes(1); - expect(exitFn.mock.calls).toEqual([[1]]); - }); - - it("Negative test: service name is cwd and missing display name", async () => { - const exitFn = jest.fn(); - await execute(".", getMockValues(), exitFn); + await execute("", "my/service/path", getMockValues(), exitFn); expect(exitFn).toBeCalledTimes(1); expect(exitFn.mock.calls).toEqual([[1]]); }); it("Negative test: missing bedrock file", async () => { const testServiceName = uuid(); + const testServicePath = "my/service/path"; const exitFn = jest.fn(); jest.spyOn(bedrockYaml, "fileInfo").mockImplementation(() => ({ @@ -222,7 +202,7 @@ describe("Test execute function", () => { })); try { - await execute(testServiceName, getMockValues(), exitFn); + await execute(testServiceName, testServicePath, getMockValues(), exitFn); expect(exitFn).toBeCalledTimes(1); expect(exitFn.mock.calls).toEqual([[1]]); } finally { @@ -232,6 +212,7 @@ describe("Test execute function", () => { it("Negative test: missing bedrock variable groups", async () => { const testServiceName = uuid(); + const testServicePath = "my/service/path"; const exitFn = jest.fn(); jest.spyOn(bedrockYaml, "fileInfo").mockImplementation(() => ({ @@ -240,7 +221,7 @@ describe("Test execute function", () => { })); try { - await execute(testServiceName, getMockValues(), exitFn); + await execute(testServiceName, testServicePath, getMockValues(), exitFn); expect(exitFn).toBeCalledTimes(1); expect(exitFn.mock.calls).toEqual([[1]]); } finally { @@ -299,33 +280,6 @@ describe("Adding a service to a repo directory", () => { randomTmpDir = createTempDir(); }); - test("New service is created in project root directory. No display name given, so this should throw an error.", async () => { - await writeSampleMaintainersFileToDir( - path.join(randomTmpDir, "maintainers.yaml") - ); - await writeSampleBedrockFileToDir(path.join(randomTmpDir, "bedrock.yaml")); - - const values = getMockValues(); - values.packagesDir = ""; - values.k8sPort = 1337; - const serviceName = "."; - - logger.info( - `creating randomTmpDir ${randomTmpDir} and service ${serviceName}` - ); - - let hasError = false; - try { - await createService(randomTmpDir, serviceName, values); - } catch (err) { - hasError = true; - expect(err.message).toBe( - "Cannot create service pipeline due to serviceName being '.'. Please include a displayName if you are trying to create a service in your project root directory." - ); - } - expect(hasError).toBe(true); - }); - test("New service is created in project root directory. With display name given, so this works fine.", async () => { await writeSampleMaintainersFileToDir( path.join(randomTmpDir, "maintainers.yaml") @@ -335,34 +289,34 @@ describe("Adding a service to a repo directory", () => { const values = getMockValues(); values.packagesDir = ""; values.k8sPort = 1337; - values.displayName = "my-service-name"; values.helmConfigAccessTokenVariable = "SOME_ENV_VAR"; - const serviceName = "."; + const serviceName = "my-service-name"; + const servicePath = "."; logger.info( `creating randomTmpDir ${randomTmpDir} and service ${serviceName}` ); - let hasError = false; - try { - await createService(randomTmpDir, serviceName, values); - } catch (err) { - hasError = true; - } - expect(hasError).toBe(false); - - await createService(randomTmpDir, serviceName, values); - validateDirNFiles(randomTmpDir, serviceName, values); + expect(createService(randomTmpDir, serviceName, servicePath, values)) + .resolves; + validateDirNFiles(randomTmpDir, servicePath, values); // TODO: Verify root project bedrock.yaml and maintainers.yaml has been changed too. const bedrock = Bedrock(randomTmpDir); - const newService = bedrock.services["./"]; - expect(newService).toBeDefined(); - expect(newService.k8sBackendPort).toBe(values.k8sPort); - expect(newService.displayName).toBe(values.displayName); - // eslint-disable-next-line @typescript-eslint/no-explicit-any - expect((newService.helm.chart as any).accessTokenVariable).toBe( - values.helmConfigAccessTokenVariable + const newService = bedrock.services.find( + (s) => s.displayName === "my-service-name" + ) as BedrockServiceConfig; + expect(newService).toStrictEqual( + expect.objectContaining({ + path: "./", + k8sBackendPort: values.k8sPort, + displayName: serviceName, + helm: { + chart: expect.objectContaining({ + accessTokenVariable: "SOME_ENV_VAR", + }), + }, + }) ); }); @@ -376,19 +330,27 @@ describe("Adding a service to a repo directory", () => { values.packagesDir = ""; values.k8sPort = 1337; const serviceName = uuid(); + const servicePath = uuid(); logger.info( `creating randomTmpDir ${randomTmpDir} and service ${serviceName}` ); - await createService(randomTmpDir, serviceName, values); - validateDirNFiles(randomTmpDir, serviceName, values); + await createService(randomTmpDir, serviceName, servicePath, values); + validateDirNFiles(randomTmpDir, servicePath, values); // TODO: Verify root project bedrock.yaml and maintainers.yaml has been changed too. const bedrock = Bedrock(randomTmpDir); - const newService = bedrock.services["./" + serviceName]; - expect(newService).toBeDefined(); - expect(newService.k8sBackendPort).toBe(values.k8sPort); + const newService = bedrock.services.find( + (s) => s.displayName === serviceName + ) as BedrockServiceConfig; + expect(newService).toStrictEqual( + expect.objectContaining({ + path: "./" + servicePath, + k8sBackendPort: values.k8sPort, + displayName: serviceName, + }) + ); }); test("New directory is created under '/packages' directory with required service files.", async () => { @@ -401,14 +363,15 @@ describe("Adding a service to a repo directory", () => { values.packagesDir = "packages"; values.k8sPort = 1337; const serviceName = uuid(); + const servicePath = uuid(); logger.info( `creating randomTmpDir ${randomTmpDir} and service ${serviceName}` ); // addService call - await createService(randomTmpDir, serviceName, values); - validateDirNFiles(randomTmpDir, serviceName, values); + await createService(randomTmpDir, serviceName, servicePath, values); + validateDirNFiles(randomTmpDir, servicePath, values); // TODO: Verify root project bedrock.yaml and maintainers.yaml has been changed too. }); @@ -424,13 +387,14 @@ describe("Adding a service to a repo directory", () => { values.gitPush = true; values.k8sPort = 1337; const serviceName = uuid(); + const servicePath = uuid(); logger.info( `creating randomTmpDir ${randomTmpDir} and service ${serviceName}` ); - await createService(randomTmpDir, serviceName, values); - validateDirNFiles(randomTmpDir, serviceName, values); + await createService(randomTmpDir, serviceName, servicePath, values); + validateDirNFiles(randomTmpDir, servicePath, values); expect(checkoutCommitPushCreatePRLink).toHaveBeenCalled(); }); @@ -444,21 +408,21 @@ describe("Adding a service to a repo directory", () => { const values = getMockValues(); values.k8sPort = 1337; const serviceName = uuid(); + const servicePath = uuid(); + logger.info( `creating randomTmpDir ${randomTmpDir} and service ${serviceName}` ); // create service with no middleware - await createService(randomTmpDir, serviceName, values); - validateDirNFiles(randomTmpDir, serviceName, values); + await createService(randomTmpDir, serviceName, servicePath, values); + validateDirNFiles(randomTmpDir, servicePath, values); const bedrockConfig = Bedrock(randomTmpDir); // check the added service has an empty list for middlewares - for (const [servicePath, service] of Object.entries( - bedrockConfig.services - )) { - if (servicePath.includes(serviceName)) { + for (const service of bedrockConfig.services) { + if (service.displayName === serviceName) { expect(service.middlewares).toBeDefined(); expect(Array.isArray(service.middlewares)).toBe(true); expect(service.middlewares!.length).toBe(0); @@ -477,20 +441,19 @@ describe("Adding a service to a repo directory", () => { values.middlewaresArray = ["foo", "bar", "baz"]; const serviceName = uuid(); + const servicePath = uuid(); logger.info( `creating randomTmpDir ${randomTmpDir} and service ${serviceName}` ); - await createService(randomTmpDir, serviceName, values); - validateDirNFiles(randomTmpDir, serviceName, values); + await createService(randomTmpDir, serviceName, servicePath, values); + validateDirNFiles(randomTmpDir, servicePath, values); const bedrockConfig = Bedrock(randomTmpDir); // check that the added service has the expected middlewares - for (const [servicePath, service] of Object.entries( - bedrockConfig.services - )) { - if (servicePath.includes(serviceName)) { + for (const service of bedrockConfig.services) { + if (service.displayName === serviceName) { expect(service.middlewares).toBeDefined(); expect(Array.isArray(service.middlewares)).toBe(true); expect(service.middlewares?.length).toBe( diff --git a/src/commands/service/create.ts b/src/commands/service/create.ts index 1e5b981f6..5ab07f251 100644 --- a/src/commands/service/create.ts +++ b/src/commands/service/create.ts @@ -29,7 +29,6 @@ import { BedrockFileInfo, HelmConfig, User } from "../../types"; import decorator from "./create.decorator.json"; export interface CommandOptions { - displayName: string; gitPush: boolean; helmChartChart: string; helmChartRepository: string; @@ -69,7 +68,6 @@ export const fetchValues = (opts: CommandOptions): CommandValues => { } const values: CommandValues = { - displayName: opts.displayName, gitPush: opts.gitPush, helmChartChart: opts.helmChartChart, helmChartRepository: opts.helmChartRepository, @@ -113,9 +111,6 @@ export const checkDependencies = (projectPath: string): void => { * pathPrefixMajorVersion are provided and are non-DNS compliant */ export const assertValidDnsInputs = (opts: Partial): void => { - if (opts.displayName) { - dns.assertIsValid("--display-name", opts.displayName); - } if (opts.k8sBackend) { dns.assertIsValid("--k8s-backend", opts.k8sBackend); } @@ -132,6 +127,7 @@ export const assertValidDnsInputs = (opts: Partial): void => { export const execute = async ( serviceName: string, + servicePath: string, opts: CommandOptions, exitFn: (status: number) => Promise ): Promise => { @@ -141,16 +137,15 @@ export const execute = async ( return; } - if (serviceName === "." && opts.displayName === "") { - logger.error( - `If specifying the current directory as service name, please include a display name using '-n'` - ); + if (!servicePath) { + logger.error("Service path is missing"); await exitFn(1); return; } // validate user inputs are DNS compliant try { + dns.assertIsValid("", serviceName); assertValidDnsInputs(opts); } catch (err) { logger.error(err); @@ -166,7 +161,7 @@ export const execute = async ( try { checkDependencies(projectPath); const values = fetchValues(opts); - await createService(projectPath, serviceName, values); + await createService(projectPath, serviceName, servicePath, values); await exitFn(0); } catch (err) { logger.error( @@ -228,8 +223,8 @@ export const validateGitUrl = async ( */ export const commandDecorator = (command: commander.Command): void => { buildCmd(command, decorator).action( - async (serviceName: string, opts: CommandOptions) => { - await execute(serviceName, opts, async (status: number) => { + async (serviceName: string, servicePath: string, opts: CommandOptions) => { + await execute(serviceName, servicePath, opts, async (status: number) => { await exitCmd(logger, process.exit, status); }); } @@ -246,46 +241,27 @@ export const commandDecorator = (command: commander.Command): void => { export const createService = async ( rootProjectPath: string, serviceName: string, + servicePath: string, values: CommandValues ): Promise => { logger.info( - `Adding Service: ${serviceName}, to Project: ${rootProjectPath} under directory: ${values.packagesDir}` - ); - logger.info( - `DisplayName: ${values.displayName}, MaintainerName: ${values.maintainerName}, MaintainerEmail: ${values.maintainerEmail}` + `Adding Service: ${serviceName}, located at ${servicePath}, to Project: ${rootProjectPath} under directory: ${values.packagesDir}` ); const newServiceDir = path.join( rootProjectPath, values.packagesDir, - serviceName + servicePath ); logger.info(`servicePath: ${newServiceDir}`); shelljs.mkdir("-p", newServiceDir); - const pipelineServiceName = values.displayName - ? values.displayName - : serviceName; // displayName takes priority over serviceName (which could be '.') - // Sanity check - if ( - pipelineServiceName === "." || - pipelineServiceName === "./" || - pipelineServiceName === "./." - ) { - logger.error( - `Cannot create service pipeline due to serviceName being '.'. Please include a displayName if you are trying to create a service in your project root directory.` - ); - throw Error( - "Cannot create service pipeline due to serviceName being '.'. Please include a displayName if you are trying to create a service in your project root directory." - ); - } - // Create azure pipelines yaml in directory generateServiceBuildAndUpdatePipelineYaml( rootProjectPath, values.ringNames, - pipelineServiceName, + serviceName, newServiceDir, values.variableGroups ); @@ -333,7 +309,7 @@ export const createService = async ( addNewServiceToBedrockFile( rootProjectPath, newServiceRelativeDir, - values.displayName, + serviceName, helmConfig, values.middlewaresArray, values.k8sPort, diff --git a/src/config.test.ts b/src/config.test.ts index 5b91b520b..50274f7e7 100644 --- a/src/config.test.ts +++ b/src/config.test.ts @@ -10,12 +10,12 @@ import { loadConfiguration, saveConfiguration, updateVariableWithLocalEnv, - write, } from "./config"; import { createTempDir } from "./lib/ioUtil"; import { disableVerboseLogging, enableVerboseLogging } from "./logger"; import { BedrockFile } from "./types"; import { getVersionMessage } from "./lib/fileutils"; +import * as bedrockYaml from "./lib/bedrockYaml"; beforeAll(() => { enableVerboseLogging(); @@ -59,8 +59,9 @@ describe("Bedrock", () => { shell.mkdir("-p", randomTmpDir); const validBedrockYaml: BedrockFile = { rings: {}, - services: { - "foo/a": { + services: [ + { + path: "foo/a", helm: { chart: { chart: "elastic", @@ -72,7 +73,8 @@ describe("Bedrock", () => { pathPrefix: "servicepath", pathPrefixMajorVersion: "v1", }, - "foo/b": { + { + path: "foo/b", helm: { chart: { git: "foo", @@ -85,10 +87,11 @@ describe("Bedrock", () => { pathPrefix: "servicepath", pathPrefixMajorVersion: "v1", }, - }, + ], version: "1.0", }; - write(validBedrockYaml, randomTmpDir); + + bedrockYaml.create(randomTmpDir, validBedrockYaml); const expectedFilePath = path.join(randomTmpDir, "bedrock.yaml"); expect(writeSpy).toBeCalledWith( expectedFilePath, @@ -127,7 +130,7 @@ describe("Bedrock", () => { version: "1.0", // eslint-disable-next-line @typescript-eslint/no-explicit-any } as any; - write(validBedrockYaml, randomTmpDir); + bedrockYaml.create(randomTmpDir, validBedrockYaml); const expectedFilePath = path.join(randomTmpDir, "bedrock.yaml"); expect(writeSpy).toBeCalledWith( expectedFilePath, diff --git a/src/config.ts b/src/config.ts index 6d51900b9..bec5bff1b 100644 --- a/src/config.ts +++ b/src/config.ts @@ -7,10 +7,11 @@ import { writeVersion } from "./lib/fileutils"; import { logger } from "./logger"; import { AzurePipelinesYaml, - BedrockFile, ConfigYaml, MaintainersFile, + BedrockFile, } from "./types"; +import * as bedrockYaml from "./lib/bedrockYaml"; //////////////////////////////////////////////////////////////////////////////// // State @@ -148,6 +149,8 @@ export const Config = (): ConfigYaml => { }; /** + * **DEPRECATED**: Use `read()` from bedrockYaml lib + * * Returns the current bedrock.yaml file for the project * * Does some validations against the file; if errors occur, an Exception is @@ -158,7 +161,7 @@ export const Config = (): ConfigYaml => { */ export const Bedrock = (fileDirectory = process.cwd()): BedrockFile => { const bedrockYamlPath = path.join(fileDirectory, "bedrock.yaml"); - const bedrock = readYaml(bedrockYamlPath); + const bedrock = bedrockYaml.read(fileDirectory); const { services } = bedrock; // validate service helm configurations @@ -195,6 +198,8 @@ export const Bedrock = (fileDirectory = process.cwd()): BedrockFile => { }; /** + * **DEPRECATED**: Use `read()` from bedrockYaml lib + * * Async wrapper for the Bedrock() function * Use this if preferring to use Promise based control flow over try/catch as * Bedrock() can throw and Error @@ -225,22 +230,17 @@ export const MaintainersAsync = async ( ): Promise => Maintainers(fileDirectory); /** - * Helper to write out a bedrock.yaml or maintainers.yaml file to the project root + * Helper to write out a maintainers.yaml or azure-pipelines.yaml file to the project root * * @param file config file object to serialize and write out */ export const write = ( - file: BedrockFile | MaintainersFile | AzurePipelinesYaml, + file: MaintainersFile | AzurePipelinesYaml, targetDirectory = process.cwd(), fileName?: string ): void => { const asYaml = yaml.safeDump(file, { lineWidth: Number.MAX_SAFE_INTEGER }); - if ("rings" in file) { - // Is bedrock.yaml - fileName = "bedrock.yaml"; - writeVersion(path.join(targetDirectory, fileName)); - return fs.appendFileSync(path.join(targetDirectory, fileName), asYaml); - } else if ("services" in file) { + if ("services" in file) { // Is maintainers file fileName = "maintainers.yaml"; writeVersion(path.join(targetDirectory, fileName)); diff --git a/src/lib/bedrockYaml.test.ts b/src/lib/bedrockYaml/bedrockYaml.test.ts similarity index 81% rename from src/lib/bedrockYaml.test.ts rename to src/lib/bedrockYaml/bedrockYaml.test.ts index 486d8b53f..314310cc9 100644 --- a/src/lib/bedrockYaml.test.ts +++ b/src/lib/bedrockYaml/bedrockYaml.test.ts @@ -1,7 +1,7 @@ import uuid = require("uuid/v4"); -import { createTempDir } from "../lib/ioUtil"; -import { createTestBedrockYaml } from "../test/mockFactory"; -import { BedrockFile, HelmConfig } from "../types"; +import { createTestBedrockYaml } from "../../test/mockFactory"; +import { BedrockFile, HelmConfig, RingConfig } from "../../types"; +import { createTempDir } from "../ioUtil"; import { addNewRing, addNewService, @@ -13,25 +13,26 @@ import { removeRing, setDefaultRing, validateRings, + getRings, } from "./bedrockYaml"; describe("Creation and Existence test on bedrock.yaml", () => { it("without folder name in creation function call", () => { const dir = create(); expect(isExists(dir)).toBe(true); - expect(read(dir)).toEqual(DEFAULT_CONTENT); + expect(read(dir)).toEqual(DEFAULT_CONTENT()); }); it("with folder name in creation function call", () => { const dir = createTempDir(); create(dir); expect(isExists(dir)).toBe(true); - expect(read(dir)).toEqual(DEFAULT_CONTENT); + expect(read(dir)).toEqual(DEFAULT_CONTENT()); }); it("withContent", () => { const dir = createTempDir(); const data = { rings: {}, - services: {}, + services: [], version: "1.0", }; create(dir, data); @@ -80,9 +81,10 @@ describe("Adding a new service to a Bedrock file", () => { const expected: BedrockFile = { ...defaultBedrockFileObject, - services: { + services: [ ...(defaultBedrockFileObject as BedrockFile).services, - ["./" + servicePath]: { + { + path: "./" + servicePath, displayName: svcDisplayName, helm: helmConfig, k8sBackend, @@ -91,7 +93,7 @@ describe("Adding a new service to a Bedrock file", () => { pathPrefix, pathPrefixMajorVersion, }, - }, + ], variableGroups: [], version: defaultBedrockFileObject.version, }; @@ -118,9 +120,7 @@ describe("Adding a new ring to an existing bedrock.yaml", () => { ...(defaultBedrockFileObject as BedrockFile).rings, [ringName]: {}, }, - services: { - ...(defaultBedrockFileObject as BedrockFile).services, - }, + services: [...(defaultBedrockFileObject as BedrockFile).services], variableGroups: [], version: defaultBedrockFileObject.version, }; @@ -149,7 +149,7 @@ describe("Bedrock file info", () => { const dir = createTempDir(); const data = { rings: {}, - services: {}, + services: [], variableGroups: [uuid()], version: "1.0", }; @@ -168,7 +168,7 @@ describe("Set default ring", () => { master: { isDefault: false }, prod: {}, }, - services: {}, + services: [], variableGroups: [uuid()], version: "1.0", }; @@ -185,7 +185,7 @@ describe("Set default ring", () => { master: { isDefault: false }, prod: { isDefault: true }, }, - services: {}, + services: [], variableGroups: [uuid()], version: "1.0", }; @@ -274,3 +274,50 @@ describe("validateRings", () => { }); } }); + +describe("getRings", () => { + const tests: { + name: string; + actual: unknown; + expected: Array; + }[] = [ + { + name: "empty rings", + actual: getRings({ rings: {}, services: [], version: "" }), + expected: [], + }, + { + name: "one ring -- isDefault is automatically added to output", + actual: getRings({ + rings: { master: {} }, + services: [], + version: "", + }), + expected: [{ name: "master", isDefault: false }], + }, + + { + name: "multiple rings -- isDefault is automatically added to output", + actual: getRings({ + rings: { + master: {}, + qa: { isDefault: false }, + dev: { isDefault: true }, + }, + services: [], + version: "", + }), + expected: [ + { name: "master", isDefault: false }, + { name: "qa", isDefault: false }, + { name: "dev", isDefault: true }, + ], + }, + ]; + + for (const { name, actual, expected } of tests) { + it(name, () => { + expect(actual).toStrictEqual(expected); + }); + } +}); diff --git a/src/lib/bedrockYaml.ts b/src/lib/bedrockYaml/bedrockYaml.ts similarity index 85% rename from src/lib/bedrockYaml.ts rename to src/lib/bedrockYaml/bedrockYaml.ts index 361ee36e2..6077de66c 100644 --- a/src/lib/bedrockYaml.ts +++ b/src/lib/bedrockYaml/bedrockYaml.ts @@ -1,25 +1,26 @@ import fs from "fs"; import yaml from "js-yaml"; import path from "path"; -import { createTempDir } from "../lib/ioUtil"; -import { logger } from "../logger"; +import { logger } from "../../logger"; import { BedrockFile, BedrockFileInfo, HelmConfig, - Rings, RingConfig, -} from "../types"; -import { writeVersion, getVersion } from "./fileutils"; + Rings, +} from "../../types"; +import { getVersion, writeVersion } from "../fileutils"; +import { createTempDir } from "../ioUtil"; +import * as indexByNameMigration from "./migrations/service-map-to-list"; export const YAML_NAME = "bedrock.yaml"; -export const DEFAULT_CONTENT: BedrockFile = { +export const DEFAULT_CONTENT = (): BedrockFile => ({ rings: {}, - services: {}, + services: [], variableGroups: [], version: getVersion(), -}; +}); /** * Creates a Bedrock.yaml file. @@ -34,7 +35,7 @@ export const DEFAULT_CONTENT: BedrockFile = { export const create = (dir?: string, data?: BedrockFile): string => { dir = dir || createTempDir(); const absPath = path.resolve(dir); - data = data || DEFAULT_CONTENT; + data = data || DEFAULT_CONTENT(); const asYaml = yaml.safeDump(data, { lineWidth: Number.MAX_SAFE_INTEGER, }); @@ -68,7 +69,8 @@ export const isExists = (dir: string): boolean => { export const read = (dir: string): BedrockFile => { const absPath = path.resolve(dir); const file = path.join(absPath, YAML_NAME); - return yaml.safeLoad(fs.readFileSync(file, "utf8")); + const b = yaml.safeLoad(fs.readFileSync(file, "utf8")); + return indexByNameMigration.migrate(b, dir); }; /** @@ -98,15 +100,19 @@ export const addNewService = ( const absPath = path.resolve(dir); const data = read(absPath); - data.services["./" + newServicePath] = { - displayName: svcDisplayName, - helm: helmConfig, - k8sBackend, - k8sBackendPort, - middlewares, - pathPrefix, - pathPrefixMajorVersion, - }; + data.services = [ + ...data.services.filter((s) => s.displayName !== svcDisplayName), + { + displayName: svcDisplayName, + path: "./" + newServicePath, + helm: helmConfig, + k8sBackend, + k8sBackendPort, + middlewares, + pathPrefix, + pathPrefixMajorVersion, + }, + ]; const asYaml = yaml.safeDump(data, { lineWidth: Number.MAX_SAFE_INTEGER, @@ -240,6 +246,22 @@ export const removeRing = ( return bedrockWithoutRing; }; +/** + * Returns a list of Ring configurations with an added `name` parameter which + * it is indexed on. + * + * @param b bedrock config file + */ +export const getRings = ( + b: BedrockFile +): Array => { + return Object.entries(b.rings).map(([name, config]) => ({ + ...config, + name, + isDefault: !!config.isDefault, + })); +}; + /** * Validates that the rings in `bedrock` are valid and throws an Error if not. * @@ -249,10 +271,7 @@ export const removeRing = ( * @param bedrock file to validate the rings of */ export const validateRings = (bedrock: BedrockFile): void => { - const rings = Object.entries(bedrock.rings).reduce( - (carry, [name, config]) => carry.concat({ ...config, name }), - [] as (RingConfig & { name: string })[] - ); + const rings = getRings(bedrock); const defaultRings = rings.filter((ring) => ring.isDefault); if (defaultRings.length > 1) { const defaultRingsNames = defaultRings.map((ring) => ring.name).join(", "); diff --git a/src/lib/bedrockYaml/index.ts b/src/lib/bedrockYaml/index.ts new file mode 100644 index 000000000..56d6f8835 --- /dev/null +++ b/src/lib/bedrockYaml/index.ts @@ -0,0 +1 @@ +export * from "./bedrockYaml"; diff --git a/src/lib/bedrockYaml/migrations/service-map-to-list.test.ts b/src/lib/bedrockYaml/migrations/service-map-to-list.test.ts new file mode 100644 index 000000000..3330a67ae --- /dev/null +++ b/src/lib/bedrockYaml/migrations/service-map-to-list.test.ts @@ -0,0 +1,294 @@ +import { BedrockFile } from "../../../types"; +import * as migration from "./service-map-to-list"; +import { LegacyBedrockFile } from "./service-map-to-list"; +import uuid from "uuid/v4"; +import * as fs from "fs"; +import * as bedrockYaml from "../bedrockYaml"; +import * as os from "os"; +import * as path from "path"; +import yaml from "js-yaml"; +import { getVersion } from "../../fileutils"; + +describe("isLegacySchema", () => { + type test = { + name: string; + actual: () => unknown; + expected: unknown; + effects?: (() => void)[]; + }; + const createTest = ( + b: LegacyBedrockFile | BedrockFile, + legacy: boolean + ): test => { + return { + name: `${JSON.stringify(b.services)} == ${legacy}`, + actual: (): unknown => migration.isLegacySchema(b), + expected: legacy, + }; + }; + const tests: test[] = [ + createTest( + { + rings: {}, + services: {}, + version: "0.5.8", + }, + true + ), + createTest( + { + rings: {}, + services: [], + version: "0.5.8", + }, + false + ), + ]; + + for (const { name, actual, expected, effects } of tests) { + it(name, () => { + expect(actual()).toStrictEqual(expected); + for (const effect of effects ?? []) { + effect(); + } + }); + } +}); + +describe("convertToNewSchema", () => { + const tests: { + name: string; + actual: () => unknown; + expected: BedrockFile; + effects?: (() => void)[]; + }[] = [ + { + name: "converts empty service map to empty list", + actual: (): BedrockFile => + migration.convertToNewSchema({ + rings: {}, + services: {}, + version: "0.5.8", + }), + expected: { + rings: {}, + services: [], + version: "0.5.8", + }, + }, + + { + name: + "service index is swapped with displayName, correct path is injected, displayName is removed", + actual: (): BedrockFile => + migration.convertToNewSchema({ + rings: { + master: { isDefault: true }, + preProd: { isDefault: false }, + qa: {}, + }, + services: { + "./foo": { + displayName: "foo-display-name", + helm: { + chart: { + git: "foo.bar.git", + path: "foo", + branch: "master", + }, + }, + k8sBackendPort: 80, + }, + "./src/bar": { + displayName: "bar-display-name", + helm: { + chart: { + git: "foo.bar.git", + path: "bar", + branch: "master", + }, + }, + k8sBackendPort: 80, + }, + }, + version: "0.5.8", + }), + expected: { + rings: { + master: { isDefault: true }, + preProd: { isDefault: false }, + qa: {}, + }, + services: [ + { + displayName: "foo-display-name", + path: "./foo", + helm: { + chart: { + git: "foo.bar.git", + path: "foo", + branch: "master", + }, + }, + k8sBackendPort: 80, + }, + { + displayName: "bar-display-name", + path: "./src/bar", + helm: { + chart: { + git: "foo.bar.git", + path: "bar", + branch: "master", + }, + }, + k8sBackendPort: 80, + }, + ], + version: "0.5.8", + }, + }, + ]; + + for (const { name, actual, expected, effects } of tests) { + it(name, () => { + expect(actual()).toStrictEqual(expected); + for (const effect of effects ?? []) { + effect(); + } + }); + } +}); + +describe("migrate", () => { + let tempDir: string; + let tempBed: LegacyBedrockFile | BedrockFile; + + const tests: { + name: string; + actual: () => unknown; + expected: unknown; + effects?: (() => void)[]; + }[] = [ + { + name: "Non-legacy is noop", + actual: (): unknown => { + // scaffold a "current" schema bedrock + tempDir = bedrockYaml.create(); + tempBed = yaml.load( + fs.readFileSync(path.join(tempDir, bedrockYaml.YAML_NAME), "utf8") + ); + // migrate non-legacy yaml + return migration.migrate(tempBed, tempDir); + }, + expected: { + rings: {}, + services: [], + variableGroups: [], + version: getVersion(), + }, + }, + + { + name: "Legacy gets converted - empty service map", + actual: (): unknown => { + // scaffold a legacy style bedrock.yaml + tempDir = path.join(os.tmpdir(), uuid()); + const bedPath = path.join(tempDir, bedrockYaml.YAML_NAME); + fs.mkdirSync(tempDir); + fs.writeFileSync( + bedPath, + yaml.safeDump({ rings: {}, services: {}, version: "" }) + ); + tempBed = yaml.safeLoad(fs.readFileSync(bedPath, "utf8")); + // migrate the legacy yaml + return migration.migrate(tempBed, tempDir); + }, + expected: { rings: {}, services: [], version: "" }, + effects: [ + (): void => { + // load the modified yaml via filesystem + const modified = yaml.load( + fs.readFileSync(path.join(tempDir, bedrockYaml.YAML_NAME), "utf8") + ); + // it should match the new schema + expect(modified).toStrictEqual({ + rings: {}, + services: [], + version: "", + }); + }, + ], + }, + + { + name: "Legacy gets converted - non-empty service map", + actual: (): unknown => { + // scaffold legacy bedrock.yaml with services + tempDir = path.join(os.tmpdir(), uuid()); + const bedPath = path.join(tempDir, bedrockYaml.YAML_NAME); + fs.mkdirSync(tempDir); + fs.writeFileSync( + bedPath, + yaml.safeDump({ + rings: {}, + services: { + "./foo": { displayName: "foo-service" }, + "./bar": { displayName: "bar-service" }, + }, + version: "", + }) + ); + tempBed = yaml.safeLoad(fs.readFileSync(bedPath, "utf8")); + // migrate the yaml + return migration.migrate(tempBed, tempDir); + }, + expected: { + rings: {}, + services: expect.arrayContaining([ + expect.objectContaining({ + path: "./foo", + displayName: "foo-service", + }), + expect.objectContaining({ + path: "./bar", + displayName: "bar-service", + }), + ]), + version: "", + }, + effects: [ + (): void => { + // load the modified yaml from system + const modified = yaml.load( + fs.readFileSync(path.join(tempDir, bedrockYaml.YAML_NAME), "utf8") + ); + // updated yaml should match new schema + expect(modified).toStrictEqual({ + rings: {}, + services: expect.arrayContaining([ + expect.objectContaining({ + path: "./foo", + displayName: "foo-service", + }), + expect.objectContaining({ + path: "./bar", + displayName: "bar-service", + }), + ]), + version: "", + }); + }, + ], + }, + ]; + + for (const { name, actual, expected, effects } of tests) { + it(name, () => { + expect(actual()).toStrictEqual(expected); + for (const effect of effects ?? []) { + effect(); + } + }); + } +}); diff --git a/src/lib/bedrockYaml/migrations/service-map-to-list.ts b/src/lib/bedrockYaml/migrations/service-map-to-list.ts new file mode 100644 index 000000000..c9c8ffc43 --- /dev/null +++ b/src/lib/bedrockYaml/migrations/service-map-to-list.ts @@ -0,0 +1,94 @@ +import * as bedrock from ".."; +import { BedrockFile, Rings, HelmConfig } from "../../../types"; +import * as path from "path"; +import * as fs from "fs"; +import { logger } from "../../../logger"; +import yaml from "js-yaml"; + +export interface LegacyBedrockService { + displayName?: string; + disableRouteScaffold?: boolean; + helm: HelmConfig; + k8sBackend?: string; // k8s service backend name for ingress routing + k8sBackendPort: number; // the service port for the k8s service Traefik2 IngressRoutes will point to + middlewares?: string[]; + pathPrefix?: string; // pathprefix for ingress route, ie. document-service + pathPrefixMajorVersion?: string; // api version, will prefix path prefix if provided. ie. 'v1' will result in the endpoint: /v1/document-service +} + +/** + * Bedrock config file + * Used to capture service meta-information regarding how to deploy + */ +export interface LegacyBedrockFile { + rings: Rings; + services: { + [path: string]: LegacyBedrockService; + }; + variableGroups?: string[]; + version: string; +} + +/** + * Converts a legacy bedrock.yaml file to the new v0.6.0 syntax + * + * - Changes the index of the services to be the displayName, uses + * `getServiceName` to compute it. + * - Removes displayName from all services if found + * - Adds `path` to the service -- using the value of the existing index for the + * service + * + * @param legacy bedrockYaml to convert + */ +export const convertToNewSchema = (legacy: LegacyBedrockFile): BedrockFile => { + const services = Object.entries(legacy.services).map(([svcPath, config]) => ({ + ...config, + path: svcPath, + })); + + return { + ...legacy, + services, + }; +}; + +/** + * Checks if the provided bedrock yaml file is the legacy schema. + * It is considered legacy if the value of `b.services` is not an array + * + * @param b config to check if legacy or not + */ +export function isLegacySchema( + b: BedrockFile | LegacyBedrockFile +): b is LegacyBedrockFile { + return !Array.isArray(b.services); +} + +/** + * Run the migration to new Bedrock schema. + * + * - Attempts to load the bedrock.yaml file and check the version it legacy + * - If it is legacy, update it and overwrite + */ +export const migrate = ( + b: BedrockFile | LegacyBedrockFile, + bedrockDir: string +): BedrockFile => { + if (isLegacySchema(b)) { + logger.info( + "Legacy bedrock.yaml schema found -- Migrating to new schema..." + ); + // delete the original + const bedrockPath = path.resolve(path.join(bedrockDir, bedrock.YAML_NAME)); + fs.unlinkSync(path.join(bedrockDir, bedrock.YAML_NAME)); + + // write out the new one + logger.info(`Writing updated bedrock.yaml file to ${bedrockPath}`); + const migrated = convertToNewSchema(b); + bedrock.create(bedrockDir, migrated); + logger.info(`Successfully updated Bedrock config`); + return yaml.safeLoad(fs.readFileSync(bedrockPath, "utf8")); + } else { + return b; + } +}; diff --git a/src/lib/fileutils.ts b/src/lib/fileutils.ts index c8f76b683..326124042 100644 --- a/src/lib/fileutils.ts +++ b/src/lib/fileutils.ts @@ -1,8 +1,10 @@ import fs from "fs"; import yaml from "js-yaml"; import path from "path"; +import { readYaml, write } from "../config"; import { ACCESS_FILENAME, + BEDROCK_FILENAME, HELM_VERSION, HLD_COMPONENT_FILENAME, PROJECT_PIPELINE_FILENAME, @@ -10,8 +12,9 @@ import { SERVICE_PIPELINE_FILENAME, VERSION_MESSAGE, VM_IMAGE, - BEDROCK_FILENAME, } from "../lib/constants"; +import { build as buildError } from "../lib/errorBuilder"; +import { errorStatusCode } from "../lib/errorStatusCode"; import { logger } from "../logger"; import { AccessYaml, @@ -20,9 +23,6 @@ import { MaintainersFile, User, } from "../types"; -import { readYaml, write } from "../config"; -import { build as buildError } from "../lib/errorBuilder"; -import { errorStatusCode } from "../lib/errorStatusCode"; /** * Read given pipeline file as json object. diff --git a/src/lib/setup/scaffold.ts b/src/lib/setup/scaffold.ts index dd4da43ea..37f89dc29 100644 --- a/src/lib/setup/scaffold.ts +++ b/src/lib/setup/scaffold.ts @@ -206,8 +206,7 @@ export const initService = async ( rc: RequestContext, repoName: string ): Promise => { - await createService(".", ".", { - displayName: repoName, + await createService(".", repoName, ".", { gitPush: false, helmChartChart: "", helmChartRepository: "", diff --git a/src/test/mockFactory.ts b/src/test/mockFactory.ts index aa65d73e2..eff9c355b 100644 --- a/src/test/mockFactory.ts +++ b/src/test/mockFactory.ts @@ -286,20 +286,23 @@ export const createTestBedrockYaml = ( qa: {}, testing: {}, }, - services: { - "./": { + services: [ + { + path: "./", helm: service1HelmConfig, k8sBackendPort: 80, }, - "./packages/service1": { + { + path: "./packages/service1", helm: service2HelmConfig, k8sBackendPort: 80, }, - "./zookeeper": { + { + path: "./zookeeper", helm: zookeeperHelmConfig, k8sBackendPort: 80, }, - }, + ], variableGroups: [], version: "1.0", }; diff --git a/src/types.d.ts b/src/types.d.ts index b918e39eb..67854b909 100644 --- a/src/types.d.ts +++ b/src/types.d.ts @@ -53,22 +53,21 @@ export interface RingConfig { */ export interface BedrockFile { rings: Rings; - services: { - [relativeDirectory: string]: BedrockServiceConfig; - }; + services: BedrockServiceConfig[]; variableGroups?: string[]; version: string; } export interface BedrockServiceConfig { displayName?: string; - middlewares?: string[]; - helm: HelmConfig; + path: string; // a **new** required path to the service disableRouteScaffold?: boolean; + helm: HelmConfig; + k8sBackend?: string; // k8s service backend name for ingress routing k8sBackendPort: number; // the service port for the k8s service Traefik2 IngressRoutes will point to + middlewares?: string[]; pathPrefix?: string; // pathprefix for ingress route, ie. document-service pathPrefixMajorVersion?: string; // api version, will prefix path prefix if provided. ie. 'v1' will result in the endpoint: /v1/document-service - k8sBackend?: string; // k8s service backend name for ingress routing } /**@see https://docs.microsoft.com/en-us/azure/devops/pipelines/yaml-schema?view=azure-devops&tabs=schema%2Cparameter-schema#triggers */ diff --git a/tests/functions.sh b/tests/functions.sh index 8ee1df5a6..e66808855 100644 --- a/tests/functions.sh +++ b/tests/functions.sh @@ -267,7 +267,7 @@ function verify_pipeline_with_poll () { # We expect only 1 build right now build_count=$(tr '"\""' '"\\"' <<< "$pipeline_builds" | jq '. | length') - if [[ -z "$expected_build_count" ]]; then + if [[ -z "$expected_build_count" ]]; then expected_build_count=1 fi if [ "$build_count" != "$expected_build_count" ]; then @@ -341,8 +341,8 @@ function create_spk_project_and_service () { variable_group_exists $AZDO_ORG_URL $AZDO_PROJECT $var_group_name "fail" # $spk service create . -n "$repo_dir_name-service" -p chart -g "$1/$2/_git/$repo_dir_name" -b master >> $TEST_WORKSPACE/log.txt - echo "$spk service create . -n "$repo_dir_name-service" -p "$repo_dir_name/chart" -g $helm_repo_url -b master" - $spk service create . -n "$repo_dir_name-service" -p "$repo_dir_name/chart" -g $helm_repo_url -b master >> $TEST_WORKSPACE/log.txt + echo "$spk service create "$repo_dir_name-service" . -p "$repo_dir_name/chart" -g $helm_repo_url -b master" + $spk service create "$repo_dir_name-service" . -p "$repo_dir_name/chart" -g $helm_repo_url -b master >> $TEST_WORKSPACE/log.txt directory_to_check="$TEST_WORKSPACE/$repo_dir_name" file_we_expect=(".gitignore" "build-update-hld.yaml" "Dockerfile" "maintainers.yaml" "hld-lifecycle.yaml" "spk.log" "bedrock.yaml") validate_directory $directory_to_check "${file_we_expect[@]}" diff --git a/tests/validations.sh b/tests/validations.sh index 5604b648a..ff45355b8 100644 --- a/tests/validations.sh +++ b/tests/validations.sh @@ -224,14 +224,14 @@ cd "$TEST_WORKSPACE/$mono_repo_dir" # helm_repo_url="$AZDO_ORG_URL/$AZDO_PROJECT/_git/$helm_charts_dir" local_repo_url="$AZDO_ORG_URL/$AZDO_PROJECT/_git/$mono_repo_dir" -spk service create $FrontEnd -d $services_dir -p "chart" -g $local_repo_url -b master >> $TEST_WORKSPACE/log.txt -# spk service create $FrontEnd -d $services_dir -p "$FrontEnd/chart" -g $helm_repo_url -b master >> $TEST_WORKSPACE/log.txt +spk service create $FrontEnd $FrontEnd -d $services_dir -p "chart" -g $local_repo_url -b master >> $TEST_WORKSPACE/log.txt +# spk service create $FrontEnd $FrontEnd -d $services_dir -p "$FrontEnd/chart" -g $helm_repo_url -b master >> $TEST_WORKSPACE/log.txt directory_to_check="$services_full_dir/$FrontEnd" file_we_expect=(".gitignore" "build-update-hld.yaml" "Dockerfile" ) validate_directory $directory_to_check "${file_we_expect[@]}" # TODO uncomment this when helm chart fixed -# spk service create $BackEnd -d $services_dir -p "$BackEnd/chart" -g $helm_repo_url -b master >> $TEST_WORKSPACE/log.txt +# spk service create $BackEnd $BackEnd -d $services_dir -p "$BackEnd/chart" -g $helm_repo_url -b master >> $TEST_WORKSPACE/log.txt # validate_directory "$services_full_dir/$BackEnd" "${file_we_expect[@]}" git add -A From 919f23afc89897971c6514ac3613c500f179cd8c Mon Sep 17 00:00:00 2001 From: Evan Louie Date: Sun, 29 Mar 2020 22:30:47 -0700 Subject: [PATCH 02/14] bedrock.yaml refactor & service create spec update - Updated bedrock.yaml `services` to contain a list of services instead of a map indexed by path. - Auto-migration of bedrock.yaml is hooked into `read()` function of the `bedrockYaml.ts` file. - `read()` is now the only function used for loading the bedrock.yaml file; `Bedrock()` in `config.ts` is now noted as deprecated and now calls `read()` under the hood. - `path` property for a service now tracks the path to the service. - Documentation references all updated. - `spk service create ` now follows the spec `spk service create ` - Documentation references all updated. - Integration tests updated. --- docs/commands/data.json | 11 +- guides/auth-private-helm-repos.md | 6 +- guides/bedrock-end-to-end-dx.md | 15 +- guides/building-helm-charts-for-spk.md | 12 +- guides/manual-guide-to-rings.md | 4 +- guides/project-service-management-guide.md | 10 +- guides/rings-101.md | 6 +- package.json | 2 +- src/commands/hld/reconcile.md | 2 +- src/commands/hld/reconcile.test.ts | 65 ++-- src/commands/hld/reconcile.ts | 5 +- .../project/create-variable-group.test.ts | 26 +- src/commands/project/create-variable-group.ts | 6 +- src/commands/project/init.test.ts | 10 +- src/commands/project/init.ts | 5 +- src/commands/project/pipeline.test.ts | 2 +- src/commands/ring/create.md | 26 +- src/commands/ring/create.test.ts | 11 +- src/commands/ring/delete.md | 26 +- src/commands/ring/set-default.md | 26 +- src/commands/ring/set-default.test.ts | 2 +- src/commands/service/create-revision.test.ts | 22 +- src/commands/service/create.decorator.json | 7 +- src/commands/service/create.md | 4 +- src/commands/service/create.test.ts | 167 ++++------ src/commands/service/create.ts | 48 +-- src/config.test.ts | 17 +- src/config.ts | 20 +- src/lib/{ => bedrockYaml}/bedrockYaml.test.ts | 77 ++++- src/lib/{ => bedrockYaml}/bedrockYaml.ts | 65 ++-- src/lib/bedrockYaml/index.ts | 1 + .../migrations/service-map-to-list.test.ts | 294 ++++++++++++++++++ .../migrations/service-map-to-list.ts | 94 ++++++ src/lib/fileutils.ts | 8 +- src/lib/setup/scaffold.ts | 3 +- src/test/mockFactory.ts | 13 +- src/types.d.ts | 11 +- tests/functions.sh | 6 +- tests/validations.sh | 6 +- 39 files changed, 772 insertions(+), 369 deletions(-) rename src/lib/{ => bedrockYaml}/bedrockYaml.test.ts (81%) rename src/lib/{ => bedrockYaml}/bedrockYaml.ts (85%) create mode 100644 src/lib/bedrockYaml/index.ts create mode 100644 src/lib/bedrockYaml/migrations/service-map-to-list.test.ts create mode 100644 src/lib/bedrockYaml/migrations/service-map-to-list.ts diff --git a/docs/commands/data.json b/docs/commands/data.json index 930059c2f..ab918ae5e 100644 --- a/docs/commands/data.json +++ b/docs/commands/data.json @@ -334,7 +334,7 @@ "alias": "r", "description": "Reconcile a HLD with the services tracked in bedrock.yaml.", "options": [], - "markdown": "## Description\n\nThe reconcile feature scaffolds a HLD with the services in the `bedrock.yaml`\nfile at the root level of the application repository. Recall that in a\nmono-repo, `spk service create` will add an entry into the `bedrock.yaml`\ncorresponding to all tracked services. When the service has been merged into\n`master` of the application repository, a pipeline (see `hld-lifecycle.yaml`,\ncreated by `spk project init`) runs `spk hld reconcile` to add any _new_\nservices tracked in `bedrock.yaml` to the HLD.\n\nThis command is _intended_ to be run in a pipeline (see the generated\n`hld-lifecycle.yaml` created from `spk project init`), but can be run by the\nuser in a CLI for verification.\n\nFor a `bedrock.yaml` file that contained within the\n`https://dev.azure.com/foo/bar/_git` repository, that has the following\nstructure:\n\n```yaml\nrings:\n master:\n isDefault: true\nservices:\n ./services/fabrikam:\n displayName: \"fabrikam\"\n k8sBackendPort: 8001\n k8sBackend: \"fabrikam-k8s-svc\"\n pathPrefix: \"fabrikam-service\"\n pathPrefixMajorVersion: \"v1\"\n helm:\n chart:\n branch: master\n git: \"https://dev.azure.com/foo/bar/_git\"\n path: stable/fabrikam-application\n middlewares:\n - \"\"\nvariableGroups:\n - fabrikam-vg\n```\n\nA HLD is produced that resembles the following:\n\n```\n├── component.yaml\n└── fabrikam\n ├── access.yaml\n ├── component.yaml\n ├── config\n │ └── common.yaml\n └── fabrikam\n ├── component.yaml\n ├── config\n │ └── common.yaml\n └── master\n ├── component.yaml\n ├── config\n │ └── common.yaml\n └── static\n ├── ingress-route.yaml\n └── middlewares.yaml\n```\n\nWith the `ingress-route.yaml` representing a\n[Traefik2 Ingress Route](https://docs.traefik.io/routing/providers/kubernetes-crd/#kind-ingressroute)\nbacked by a Kubernetes Service, and the `middlewares.yaml` representing a\n[Traefik2 Middleware](https://docs.traefik.io/routing/providers/kubernetes-crd/#kind-middleware)\nthat strips path prefixes.\n\nFor the `bedrock.yaml` shown above, the `ingress-route.yaml` produced is:\n\n```yaml\napiVersion: traefik.containo.us/v1alpha1\nkind: IngressRoute\nmetadata:\n name: fabrikam-master\nspec:\n routes:\n - kind: Rule\n match: \"PathPrefix(`/v1/fabrikam-service`) && Headers(`Ring`, `master`)\"\n middlewares:\n - name: fabrikam-master\n services:\n - name: fabrikam-k8s-svc-master\n port: 8001\n```\n\nAnd the `middlewares.yaml` produced is:\n\n```yaml\napiVersion: traefik.containo.us/v1alpha1\nkind: Middleware\nmetadata:\n name: fabrikam-master\nspec:\n stripPrefix:\n forceSlash: false\n prefixes:\n - /v1/fabrikam-service\n```\n\nNote that there exists a third generated file, `access.yaml`. For the above\n`bedrock.yaml`, `access.yaml` contains a single line, which represents a\n[Fabrikate access.yaml definition](https://github.com/microsoft/fabrikate/blob/master/docs/auth.md#accessyaml),\nallowing Fabrikate to pull Helm Charts that are contained within the same\napplication repository:\n\n```yaml\n\"https://dev.azure.com/foo/bar/_git\": ACCESS_TOKEN_SECRET\n```\n\nWhen `fabrikate` is invoked in the HLD to Manifest pipeline, it will utilize the\n`ACCESS_TOKEN_SECRET` environment variable injected at pipeline run-time as a\nPersonal Access Token to pull any referenced helm charts from the application\nrepository.\n" + "markdown": "## Description\n\nThe reconcile feature scaffolds a HLD with the services in the `bedrock.yaml`\nfile at the root level of the application repository. Recall that in a\nmono-repo, `spk service create` will add an entry into the `bedrock.yaml`\ncorresponding to all tracked services. When the service has been merged into\n`master` of the application repository, a pipeline (see `hld-lifecycle.yaml`,\ncreated by `spk project init`) runs `spk hld reconcile` to add any _new_\nservices tracked in `bedrock.yaml` to the HLD.\n\nThis command is _intended_ to be run in a pipeline (see the generated\n`hld-lifecycle.yaml` created from `spk project init`), but can be run by the\nuser in a CLI for verification.\n\nFor a `bedrock.yaml` file that contained within the\n`https://dev.azure.com/foo/bar/_git` repository, that has the following\nstructure:\n\n```yaml\nrings:\n master:\n isDefault: true\nservices:\n - path: ./services/fabrikam\n displayName: \"fabrikam\"\n k8sBackendPort: 8001\n k8sBackend: \"fabrikam-k8s-svc\"\n pathPrefix: \"fabrikam-service\"\n pathPrefixMajorVersion: \"v1\"\n helm:\n chart:\n branch: master\n git: \"https://dev.azure.com/foo/bar/_git\"\n path: stable/fabrikam-application\n middlewares:\n - \"\"\nvariableGroups:\n - fabrikam-vg\n```\n\nA HLD is produced that resembles the following:\n\n```\n├── component.yaml\n└── fabrikam\n ├── access.yaml\n ├── component.yaml\n ├── config\n │ └── common.yaml\n └── fabrikam\n ├── component.yaml\n ├── config\n │ └── common.yaml\n └── master\n ├── component.yaml\n ├── config\n │ └── common.yaml\n └── static\n ├── ingress-route.yaml\n └── middlewares.yaml\n```\n\nWith the `ingress-route.yaml` representing a\n[Traefik2 Ingress Route](https://docs.traefik.io/routing/providers/kubernetes-crd/#kind-ingressroute)\nbacked by a Kubernetes Service, and the `middlewares.yaml` representing a\n[Traefik2 Middleware](https://docs.traefik.io/routing/providers/kubernetes-crd/#kind-middleware)\nthat strips path prefixes.\n\nFor the `bedrock.yaml` shown above, the `ingress-route.yaml` produced is:\n\n```yaml\napiVersion: traefik.containo.us/v1alpha1\nkind: IngressRoute\nmetadata:\n name: fabrikam-master\nspec:\n routes:\n - kind: Rule\n match: \"PathPrefix(`/v1/fabrikam-service`) && Headers(`Ring`, `master`)\"\n middlewares:\n - name: fabrikam-master\n services:\n - name: fabrikam-k8s-svc-master\n port: 8001\n```\n\nAnd the `middlewares.yaml` produced is:\n\n```yaml\napiVersion: traefik.containo.us/v1alpha1\nkind: Middleware\nmetadata:\n name: fabrikam-master\nspec:\n stripPrefix:\n forceSlash: false\n prefixes:\n - /v1/fabrikam-service\n```\n\nNote that there exists a third generated file, `access.yaml`. For the above\n`bedrock.yaml`, `access.yaml` contains a single line, which represents a\n[Fabrikate access.yaml definition](https://github.com/microsoft/fabrikate/blob/master/docs/auth.md#accessyaml),\nallowing Fabrikate to pull Helm Charts that are contained within the same\napplication repository:\n\n```yaml\n\"https://dev.azure.com/foo/bar/_git\": ACCESS_TOKEN_SECRET\n```\n\nWhen `fabrikate` is invoked in the HLD to Manifest pipeline, it will utilize the\n`ACCESS_TOKEN_SECRET` environment variable injected at pipeline run-time as a\nPersonal Access Token to pull any referenced helm charts from the application\nrepository.\n" }, "infra generate": { "command": "generate", @@ -555,7 +555,7 @@ "markdown": "## Description\n\nGenerate a PR in Azure DevOps against default ring branches\n" }, "service create": { - "command": "create ", + "command": "create ", "alias": "c", "description": "Add a new service into this initialized spk project repository", "options": [ @@ -594,11 +594,6 @@ "description": "The directory containing the mono-repo packages.", "defaultValue": "" }, - { - "arg": "-n, --display-name ", - "description": "Display name of the service.", - "defaultValue": "" - }, { "arg": "-m, --maintainer-name ", "description": "The name of the primary maintainer for this service.", @@ -640,7 +635,7 @@ "defaultValue": "" } ], - "markdown": "## Description\n\nAdd a new service into this initialized spk project repository.\n\n## Example\n\n```bash\nspk service create . \\\n --display-name $app_name \\\n --helm-config-path $path_to_chart_in_repo \\\n --helm-config-git $helm_repo_url \\ # Needs to start with https and not contain user name\n --helm-config-branch master \\\n --helm-chart-access-token-variable $ENV_VAR_NAME\n```\n\n## Note\n\n- `--helm-chart-*` and `--helm-config-*` settings are mutually-exclusive. **You\n may only use one.**\n - If the git repository referenced in `--helm-config-git` is a private\n repository, you can specify an environment variable in your\n HLD-to-Materialized pipeline containing your a PAT to authenticate with via\n the `--helm-chart-access-token-variable` option.\n- `--middlewares`, `--k8s-backend-port`, `--path-prefix`,\n `--path-prefix-major-version`, and `--k8s-backend` are all used to configure\n the generated Traefik2 IngressRoutes. i.e.\n\n ```sh\n spk service create my-example-documents-service \\\n --middlewares middleware \\\n --k8s-backend-port 3001 \\\n --k8s-backend docs-service \\\n --path-prefix documents \\\n --path-prefix-major-version v2\n ```\n\n will result in an IngressRoute that looks like:\n\n ```yaml\n apiVersion: traefik.containo.us/v1alpha1\n kind: IngressRoute\n metadata:\n name: my-example-documents-service-master\n spec:\n routes:\n - kind: Rule\n match: \"PathPrefix(`/v2/documents`) && Headers(`Ring`, `master`)\"\n middlewares:\n - name: my-example-documents-service-master\n - name: middlewareA\n services:\n - name: docs-service\n port: 3001\n ```\n" + "markdown": "## Description\n\nAdd a new service into this initialized spk project repository.\n\n## Example\n\n```bash\nspk service create my-service . \\\n --display-name $app_name \\\n --helm-config-path $path_to_chart_in_repo \\\n --helm-config-git $helm_repo_url \\ # Needs to start with https and not contain user name\n --helm-config-branch master \\\n --helm-chart-access-token-variable $ENV_VAR_NAME\n```\n\n## Note\n\n- `--helm-chart-*` and `--helm-config-*` settings are mutually-exclusive. **You\n may only use one.**\n - If the git repository referenced in `--helm-config-git` is a private\n repository, you can specify an environment variable in your\n HLD-to-Materialized pipeline containing your a PAT to authenticate with via\n the `--helm-chart-access-token-variable` option.\n- `--middlewares`, `--k8s-backend-port`, `--path-prefix`,\n `--path-prefix-major-version`, and `--k8s-backend` are all used to configure\n the generated Traefik2 IngressRoutes. i.e.\n\n ```sh\n spk service create my-example-documents-service path/to/my/service \\\n --middlewares middleware \\\n --k8s-backend-port 3001 \\\n --k8s-backend docs-service \\\n --path-prefix documents \\\n --path-prefix-major-version v2\n ```\n\n will result in an IngressRoute that looks like:\n\n ```yaml\n apiVersion: traefik.containo.us/v1alpha1\n kind: IngressRoute\n metadata:\n name: my-example-documents-service-master\n spec:\n routes:\n - kind: Rule\n match: \"PathPrefix(`/v2/documents`) && Headers(`Ring`, `master`)\"\n middlewares:\n - name: my-example-documents-service-master\n - name: middlewareA\n services:\n - name: docs-service\n port: 3001\n ```\n" }, "service install-build-pipeline": { "command": "install-build-pipeline ", diff --git a/guides/auth-private-helm-repos.md b/guides/auth-private-helm-repos.md index 91fcf0967..bac5c21b9 100644 --- a/guides/auth-private-helm-repos.md +++ b/guides/auth-private-helm-repos.md @@ -36,7 +36,7 @@ can make it aware of it via the `--helm-config-access-token-variable` flag in the `spk service create` command. ```sh -spk service create my-service \ +spk service create my-service ./path/to/service \ --helm-config-git https://dev.azure.com/my-org/my-project/_git/my-repo \ --helm-config-branch master \ --helm-config-path my-service-helm-chart \ @@ -57,8 +57,8 @@ rings: master: isDefault: true services: - ./my-service: - displayName: "" + - path: ./path/to/service + displayName: my-service helm: chart: accessTokenVariable: MY_ENV_VAR # Note: this is where the environment variable gets tracked diff --git a/guides/bedrock-end-to-end-dx.md b/guides/bedrock-end-to-end-dx.md index 04efe0924..0d2149903 100644 --- a/guides/bedrock-end-to-end-dx.md +++ b/guides/bedrock-end-to-end-dx.md @@ -72,7 +72,7 @@ The core `discovery-service` microservice already exists, so he grandfathers it into the Bedrock workflow with: ```bash -$ spk service create discovery-service -d services +$ spk service create discovery-service ./path/to/discovery/service -d services ``` This updates the bedrock.yaml file to include this service: @@ -80,13 +80,12 @@ This updates the bedrock.yaml file to include this service: ```bash $ cat bedrock.yaml services: - ./services/discovery-service: - helm: - chart: - branch: '' - git: '' - path: '' - + - path: ./services/discovery-service + helm: + chart: + branch: '' + git: '' + path: '' ``` and adds a `build-update-hld.yaml` file in `services/discovery-service` to build diff --git a/guides/building-helm-charts-for-spk.md b/guides/building-helm-charts-for-spk.md index e5053593c..63461f36c 100644 --- a/guides/building-helm-charts-for-spk.md +++ b/guides/building-helm-charts-for-spk.md @@ -38,8 +38,8 @@ rings: dev: isDefault: true services: - ./: - displayName: "fabrikam" + - displayName: "fabrikam" + path: ./ helm: chart: branch: master @@ -58,7 +58,7 @@ The above service `fabrikam` was added to `bedrock.yaml` by invoking `spk service create` with the requisite parameters ie: ```sh -spk service create . \ +spk service create fabrikam . \ --display-name fabrikam \ --helm-config-git https://dev.azure.com/fabrikam/frontend/_git/charts \ --helm-config-path frontend \ @@ -152,9 +152,9 @@ rings: dev: isDefault: true services: - ./ - ... + - path: ./ k8sBackend: 'fabrikam-k8s-svc' + ... ``` The `serviceName` is generated from a combination of the `k8sBackend`, @@ -179,7 +179,7 @@ rings: dev: isDefault: true services: - ./ + - path: ./ ... displayName: 'fabrikam' k8sBackend: 'fabrikam-k8s-svc' diff --git a/guides/manual-guide-to-rings.md b/guides/manual-guide-to-rings.md index 885e131ef..44081e815 100644 --- a/guides/manual-guide-to-rings.md +++ b/guides/manual-guide-to-rings.md @@ -35,7 +35,7 @@ rings: qa: prod: services: - ./: + - path: ./ displayName: "fabrikam" helm: chart: @@ -87,7 +87,7 @@ rings: prod: test-new-homepage: <-- NEW --> services: - ./: + - path: ./ displayName: "fabrikam" helm: chart: diff --git a/guides/project-service-management-guide.md b/guides/project-service-management-guide.md index 9b89c2fdb..095defa47 100644 --- a/guides/project-service-management-guide.md +++ b/guides/project-service-management-guide.md @@ -235,7 +235,7 @@ application repositories flag parameters can be used if `spk` was not intialized) ``` SERVICE_NAME= - spk service create . --display-name $SERVICE_NAME ... + spk service create $SERVICE_NAME . ... git add -A git commit -m "Adding $SERVICE_NAME to the repository." git push -u origin --all @@ -286,8 +286,8 @@ As an `spk` user, if you would like to incorporate helm charts from a well known public repository, you may simply run `spk` the following `helm-chart` arguments: -``` -spk service create --helm-chart-chart stable/nginx --helm-chart-repository github.com/helm/charts +```sh +spk service create nginx my-nginx-service --helm-chart-chart stable/nginx --helm-chart-repository github.com/helm/charts ``` ##### Helm Charts in a distinct Git Repository from Application Sources in the same Azure DevOps Project @@ -296,7 +296,7 @@ If your Helm Charts are in their own distinct Git Repository in the _same_ Azure DevOps project, you can use the `helm-config` arguments to configure `spk`: ``` -spk service create \ +spk service create fabrikam path/to/fabrikam \ --helm-config-git https://dev.azure.com/fabrikam/fabrikam-project/_git/fabrikam-helm-charts --helm-config-branch master \ --helm-path /charts/fabrikam @@ -340,7 +340,7 @@ the environment variable containing the Personal Access Token to access the git repository in `helm-config-git`: ``` -spk service create \ +spk service create fabrikam path/to/fabrikam \ --helm-config-git https://dev.azure.com/fabrikam/fabrikam-helm-charts-project/_git/fabrikam-helm-charts \ --helm-config-branch master \ --helm-path /charts/fabrikam \ diff --git a/guides/rings-101.md b/guides/rings-101.md index 611916b86..58f2d984f 100644 --- a/guides/rings-101.md +++ b/guides/rings-101.md @@ -136,7 +136,7 @@ rings: isDefault: false qa: {} # isDefault not present is same as isDefault: false services: - ./my-service-foo: + - path: my-service-foo displayName: fancy-service helm: chart: @@ -212,7 +212,7 @@ rings: develop: isDefault: false services: - ./foo: + - path: ./foo displayName: "" helm: chart: @@ -225,7 +225,7 @@ services: middlewares: [] pathPrefix: "" pathPrefixMajorVersion: "" - ./bar: + - path: bar displayName: "" helm: chart: diff --git a/package.json b/package.json index 20359968b..75644f064 100644 --- a/package.json +++ b/package.json @@ -13,7 +13,7 @@ "md-lint": "find guides -name \\*.md | xargs -n 1 markdown-link-check", "format": "prettier --write 'src/**/*.ts{,x}'", "test": "jest --rootDir src --reporters=jest-junit --reporters=default --coverage --coverageReporters=cobertura --coverageReporters=html", - "test-watch": "jest --watchAll", + "test-watch": "jest --rootDir src --watchAll", "postinstall": "cd node_modules/azure-devops-node-api && git apply ../../patches/001-azure-devops-node.patch || true", "test-coverage-html": "jest --coverage --coverageReporters=html" }, diff --git a/src/commands/hld/reconcile.md b/src/commands/hld/reconcile.md index 392711c1b..485837ab2 100644 --- a/src/commands/hld/reconcile.md +++ b/src/commands/hld/reconcile.md @@ -21,7 +21,7 @@ rings: master: isDefault: true services: - ./services/fabrikam: + - path: ./services/fabrikam displayName: "fabrikam" k8sBackendPort: 8001 k8sBackend: "fabrikam-k8s-svc" diff --git a/src/commands/hld/reconcile.test.ts b/src/commands/hld/reconcile.test.ts index 4d50ca2cd..06dbfbc31 100644 --- a/src/commands/hld/reconcile.test.ts +++ b/src/commands/hld/reconcile.test.ts @@ -250,6 +250,7 @@ describe("addChartToRing", () => { const chartPath = "/charts/service"; const serviceConfig: BedrockServiceConfig = { + path: "./", helm: { chart: { branch, @@ -278,6 +279,7 @@ describe("addChartToRing", () => { const chartPath = "/charts/service"; const serviceConfig: BedrockServiceConfig = { + path: "./", helm: { chart: { git, @@ -305,6 +307,7 @@ describe("addChartToRing", () => { const chart = "/charts/service"; const serviceConfig: BedrockServiceConfig = { + path: "./", helm: { chart: { chart, @@ -333,6 +336,7 @@ describe("addChartToRing", () => { const chart = "/charts/service"; const serviceConfig: BedrockServiceConfig = { + path: "./", helm: { chart: { chart, @@ -354,6 +358,7 @@ describe("configureChartForRing", () => { const ringName = "myringname"; const normalizedServiceName = "my-great-service"; const serviceConfig: BedrockServiceConfig = { + path: "./", helm: { chart: { git: "foo", @@ -383,6 +388,7 @@ describe("configureChartForRing", () => { it("should invoke the correct command and calculate the k8s service name from the bedrock service name if there is no k8sbackend configured.", async () => { const serviceConfigNoK8sBackend: BedrockServiceConfig = { + path: "./", helm: { chart: { git: "foo", @@ -456,8 +462,9 @@ describe("reconcile tests", () => { }, prod: {}, }, - services: { - "./path/to/a/svc/": { + services: [ + { + path: "./path/to/a/svc/", disableRouteScaffold: false, helm: { chart: { @@ -470,7 +477,7 @@ describe("reconcile tests", () => { k8sBackend: "cool-service", k8sBackendPort: 1337, }, - }, + ], version: "1.0", }; }); @@ -526,8 +533,9 @@ describe("reconcile tests", () => { isDefault: true, }, }, - services: { - "./path/to/svc/": { + services: [ + { + path: "./path/to/svc/", disableRouteScaffold: true, helm: { chart: { @@ -538,7 +546,7 @@ describe("reconcile tests", () => { }, k8sBackendPort: 1337, }, - }, + ], version: "1.0", }; @@ -606,8 +614,9 @@ describe("reconcile tests", () => { }); it("does not create service components if the service path is `.`, and a display name does not exist", async () => { - bedrockYaml.services = { - ".": { + bedrockYaml.services = [ + { + path: ".", disableRouteScaffold: false, helm: { chart: { @@ -618,7 +627,7 @@ describe("reconcile tests", () => { }, k8sBackendPort: 80, }, - }; + ]; await reconcileHld( dependencies, @@ -634,8 +643,9 @@ describe("reconcile tests", () => { it("does create service components if the service path is `.` and a display name does exist", async () => { const displayName = "fabrikam"; - bedrockYaml.services = { - ".": { + bedrockYaml.services = [ + { + path: ".", disableRouteScaffold: false, displayName, helm: { @@ -647,7 +657,7 @@ describe("reconcile tests", () => { }, k8sBackendPort: 80, }, - }; + ]; await reconcileHld( dependencies, @@ -667,8 +677,9 @@ describe("reconcile tests", () => { it("uses display name over the service path for creating service components", async () => { const displayName = "fabrikam"; - bedrockYaml.services = { - "/my/service/path": { + bedrockYaml.services = [ + { + path: "/my/service/path", disableRouteScaffold: false, displayName, helm: { @@ -680,7 +691,7 @@ describe("reconcile tests", () => { }, k8sBackendPort: 80, }, - }; + ]; await reconcileHld( dependencies, @@ -700,18 +711,22 @@ describe("reconcile tests", () => { it("properly updates access.yaml", async () => { const anotherGit = "github.com/foobar/baz"; const anotherToken = "MY_FANCY_ENV_VAR"; - bedrockYaml.services["another/service"] = { - disableRouteScaffold: false, - helm: { - chart: { - accessTokenVariable: anotherToken, - git: anotherGit, - path: "path/to/chart", - sha: "12345", + bedrockYaml.services = [ + ...bedrockYaml.services, + { + path: "another/service", + disableRouteScaffold: false, + helm: { + chart: { + accessTokenVariable: anotherToken, + git: anotherGit, + path: "path/to/chart", + sha: "12345", + }, }, + k8sBackendPort: 8888, }, - k8sBackendPort: 8888, - }; + ]; const pathToHLD = "./the/path/to/hld"; const service = "service"; await reconcileHld( diff --git a/src/commands/hld/reconcile.ts b/src/commands/hld/reconcile.ts index e715f97ca..696168a89 100644 --- a/src/commands/hld/reconcile.ts +++ b/src/commands/hld/reconcile.ts @@ -476,9 +476,8 @@ export const reconcileHld = async ( normalizedAbsRepositoryInHldPath ); - for (const [serviceRelPath, serviceConfig] of Object.entries( - managedServices - )) { + for (const serviceConfig of managedServices) { + const serviceRelPath = serviceConfig.path; const serviceName = serviceConfig.displayName || path.basename(serviceRelPath); diff --git a/src/commands/project/create-variable-group.test.ts b/src/commands/project/create-variable-group.test.ts index 14b4e1e5e..ccbbb2a43 100644 --- a/src/commands/project/create-variable-group.test.ts +++ b/src/commands/project/create-variable-group.test.ts @@ -3,13 +3,9 @@ import yaml from "js-yaml"; import mockFs from "mock-fs"; import path from "path"; import uuid from "uuid/v4"; -import { readYaml, write } from "../../config"; +import { readYaml } from "../../config"; import * as config from "../../config"; -import { - create as createBedrockYaml, - isExists as isBedrockFileExists, - read as readBedrockFile, -} from "../../lib/bedrockYaml"; +import * as bedrockYaml from "../../lib/bedrockYaml"; import * as commandBuilder from "../../lib/commandBuilder"; import { PROJECT_PIPELINE_FILENAME, @@ -243,12 +239,12 @@ describe("setVariableGroupInBedrockFile", () => { test("Should pass adding a valid variable group name when bedrock file exists with empty variableGroups", async () => { // Create random directory to initialize - const randomTmpDir = createBedrockYaml(); + const randomTmpDir = bedrockYaml.create(); await setVariableGroupInBedrockFile(randomTmpDir, variableGroupName); - expect(isBedrockFileExists(randomTmpDir)).toBe(true); - const bedrockFile = readBedrockFile(randomTmpDir); + expect(bedrockYaml.isExists(randomTmpDir)).toBe(true); + const bedrockFile = bedrockYaml.read(randomTmpDir); logger.info(`filejson: ${JSON.stringify(bedrockFile)}`); expect(bedrockFile.variableGroups).toBeDefined(); @@ -264,16 +260,16 @@ describe("setVariableGroupInBedrockFile", () => { logger.info(`prevariableGroupName: ${prevariableGroupName}`); const bedrockFileData: BedrockFile = { 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 + services: [], // service property is not optional so set it to null variableGroups: [prevariableGroupName], version: "", }; - const randomTmpDir = createBedrockYaml("", bedrockFileData); + const randomTmpDir = bedrockYaml.create("", bedrockFileData); await setVariableGroupInBedrockFile(randomTmpDir, variableGroupName); - expect(isBedrockFileExists(randomTmpDir)).toBe(true); + expect(bedrockYaml.isExists(randomTmpDir)).toBe(true); - const bedrockFile = readBedrockFile(randomTmpDir); + const bedrockFile = bedrockYaml.read(randomTmpDir); logger.info(`filejson: ${JSON.stringify(bedrockFile)}`); expect(bedrockFile.variableGroups).toBeDefined(); if (bedrockFile.variableGroups) { @@ -330,7 +326,7 @@ describe("updateLifeCyclePipeline", () => { false ) as BedrockFile; - write(defaultBedrockFileObject, randomTmpDir); + bedrockYaml.create(randomTmpDir, defaultBedrockFileObject); const hldFilePath = path.join(randomTmpDir, PROJECT_PIPELINE_FILENAME); @@ -369,7 +365,7 @@ describe("updateLifeCyclePipeline", () => { variableGroupName, ]; - write(defaultBedrockFileObject, randomTmpDir); + bedrockYaml.create(randomTmpDir, defaultBedrockFileObject); const hldFilePath = path.join(randomTmpDir, PROJECT_PIPELINE_FILENAME); diff --git a/src/commands/project/create-variable-group.ts b/src/commands/project/create-variable-group.ts index 8df593a64..f68cabedc 100644 --- a/src/commands/project/create-variable-group.ts +++ b/src/commands/project/create-variable-group.ts @@ -3,7 +3,7 @@ import commander from "commander"; import path from "path"; import { echo } from "shelljs"; import { Bedrock, Config, readYaml, write } from "../../config"; -import { fileInfo as bedrockFileInfo } from "../../lib/bedrockYaml"; +import * as bedrockYaml from "../../lib/bedrockYaml"; import { build as buildCmd, exit as exitCmd, @@ -42,7 +42,7 @@ interface CommandOptions { } export const checkDependencies = (projectPath: string): void => { - const fileInfo: BedrockFileInfo = bedrockFileInfo(projectPath); + const fileInfo: BedrockFileInfo = bedrockYaml.fileInfo(projectPath); if (fileInfo.exist === false) { throw new Error(PROJECT_INIT_DEPENDENCY_ERROR_MESSAGE); } @@ -160,7 +160,7 @@ export const setVariableGroupInBedrockFile = ( ]; // Write out - write(bedrockFile, absProjectRoot); + bedrockYaml.create(absProjectRoot, bedrockFile); }; /** diff --git a/src/commands/project/init.test.ts b/src/commands/project/init.test.ts index 604c6facb..36b5a8525 100644 --- a/src/commands/project/init.test.ts +++ b/src/commands/project/init.test.ts @@ -2,6 +2,7 @@ import fs from "fs"; import path from "path"; import uuid from "uuid/v4"; import { Bedrock, Maintainers, write } from "../../config"; +import * as bedrockYaml from "../../lib/bedrockYaml"; import { PROJECT_PIPELINE_FILENAME, SERVICE_PIPELINE_FILENAME, @@ -58,8 +59,9 @@ describe("initializing an existing file does not modify it", () => { const randomDir = createTempDir(); const bedrockFile: BedrockFile = { rings: { master: { isDefault: true } }, - services: { - "some/random/dir": { + services: [ + { + path: "some/random/dir", helm: { chart: { git: "foo", @@ -69,10 +71,10 @@ describe("initializing an existing file does not modify it", () => { }, k8sBackendPort: 1337, }, - }, + ], version: "1.0", }; - write(bedrockFile, randomDir); + bedrockYaml.create(randomDir, bedrockFile); await initialize(randomDir); // bedrock file should not have been modified diff --git a/src/commands/project/init.ts b/src/commands/project/init.ts index 76bae6b5e..fc7e6a1a2 100644 --- a/src/commands/project/init.ts +++ b/src/commands/project/init.ts @@ -2,6 +2,7 @@ import commander from "commander"; import fs from "fs"; import path from "path"; import { Bedrock, write } from "../../config"; +import * as bedrockYaml from "../../lib/bedrockYaml"; import { build as buildCmd, exit as exitCmd } from "../../lib/commandBuilder"; import { generateGitIgnoreFile, @@ -38,7 +39,7 @@ const generateBedrockFile = ( }, {} ), - services: {}, + services: [], version: getVersion(), }; @@ -50,7 +51,7 @@ const generateBedrockFile = ( ); } else { // Write out - write(baseBedrockFile, absProjectPath); + bedrockYaml.create(absProjectPath, baseBedrockFile); } }; diff --git a/src/commands/project/pipeline.test.ts b/src/commands/project/pipeline.test.ts index 212f97544..6b67283e4 100644 --- a/src/commands/project/pipeline.test.ts +++ b/src/commands/project/pipeline.test.ts @@ -140,7 +140,7 @@ describe("installLifecyclePipeline and execute tests", () => { const tmpDir = createTempDir(); createBedrockYaml(tmpDir, { rings: {}, - services: {}, + services: [], variableGroups: ["test"], version: "1.0", }); diff --git a/src/commands/ring/create.md b/src/commands/ring/create.md index 042425fdb..50db0dc44 100644 --- a/src/commands/ring/create.md +++ b/src/commands/ring/create.md @@ -13,7 +13,7 @@ rings: qa: prod: services: - ./: + - path: ./ displayName: "fabrikam" helm: chart: @@ -40,18 +40,18 @@ running `spk ring create stage` will result in a few changes: prod: stage: services: - ./: - displayName: "fabrikam" - helm: - chart: - branch: master - git: "https://dev.azure.com/fabrikam/frontend/_git/charts" - path: frontend - k8sBackend: "fabrikam-k8s-svc" - k8sBackendPort: 80 - middlewares: [] - pathPrefix: "fabrikam-service" - pathPrefixMajorVersion: "v1" + - path: ./ + displayName: "fabrikam" + helm: + chart: + branch: master + git: "https://dev.azure.com/fabrikam/frontend/_git/charts" + path: frontend + k8sBackend: "fabrikam-k8s-svc" + k8sBackendPort: 80 + middlewares: [] + pathPrefix: "fabrikam-service" + pathPrefixMajorVersion: "v1" variableGroups: - fabrikam-vg ``` diff --git a/src/commands/ring/create.test.ts b/src/commands/ring/create.test.ts index 7239f1752..70d919ca3 100644 --- a/src/commands/ring/create.test.ts +++ b/src/commands/ring/create.test.ts @@ -32,7 +32,7 @@ describe("checkDependencies", () => { isDefault: true, }, }, - services: {}, + services: [], variableGroups: ["testvg"], version: "1.0", }); @@ -46,7 +46,7 @@ describe("checkDependencies", () => { isDefault: true, }, }, - services: {}, + services: [], variableGroups: ["testvg"], version: "1.0", }); @@ -94,8 +94,9 @@ describe("test execute function and logic", () => { isDefault: true, }, }, - services: { - "./my-service": { + services: [ + { + path: "./my-service", helm: { chart: { branch: "master", @@ -105,7 +106,7 @@ describe("test execute function and logic", () => { }, k8sBackendPort: 80, }, - }, + ], variableGroups: ["testvg"], version: "1.0", }); diff --git a/src/commands/ring/delete.md b/src/commands/ring/delete.md index 7117a3cd3..ee4166b3c 100644 --- a/src/commands/ring/delete.md +++ b/src/commands/ring/delete.md @@ -16,7 +16,7 @@ rings: qa: prod: services: - ./: + - path: ./ displayName: "fabrikam" helm: chart: @@ -41,18 +41,18 @@ running `spk ring delete prod` will result in a few changes: isDefault: true qa: services: - ./: - displayName: "fabrikam" - helm: - chart: - branch: master - git: "https://dev.azure.com/fabrikam/frontend/_git/charts" - path: frontend - k8sBackend: "fabrikam-k8s-svc" - k8sBackendPort: 80 - middlewares: [] - pathPrefix: "fabrikam-service" - pathPrefixMajorVersion: "v1" + - path: ./ + displayName: "fabrikam" + helm: + chart: + branch: master + git: "https://dev.azure.com/fabrikam/frontend/_git/charts" + path: frontend + k8sBackend: "fabrikam-k8s-svc" + k8sBackendPort: 80 + middlewares: [] + pathPrefix: "fabrikam-service" + pathPrefixMajorVersion: "v1" variableGroups: - fabrikam-vg ``` diff --git a/src/commands/ring/set-default.md b/src/commands/ring/set-default.md index 44408920c..e25015c4d 100644 --- a/src/commands/ring/set-default.md +++ b/src/commands/ring/set-default.md @@ -13,7 +13,7 @@ rings: qa: prod: services: - ./: + - path: ./ displayName: "fabrikam" helm: chart: @@ -40,18 +40,18 @@ running `spk ring set-default prod` will result in: prod: isDefault: true services: - ./: - displayName: "fabrikam" - helm: - chart: - branch: master - git: "https://dev.azure.com/fabrikam/frontend/_git/charts" - path: frontend - k8sBackend: "fabrikam-k8s-svc" - k8sBackendPort: 80 - middlewares: [] - pathPrefix: "fabrikam-service" - pathPrefixMajorVersion: "v1" + - path: ./ + displayName: "fabrikam" + helm: + chart: + branch: master + git: "https://dev.azure.com/fabrikam/frontend/_git/charts" + path: frontend + k8sBackend: "fabrikam-k8s-svc" + k8sBackendPort: 80 + middlewares: [] + pathPrefix: "fabrikam-service" + pathPrefixMajorVersion: "v1" variableGroups: - fabrikam-vg ``` diff --git a/src/commands/ring/set-default.test.ts b/src/commands/ring/set-default.test.ts index d12d61607..0ecb95935 100644 --- a/src/commands/ring/set-default.test.ts +++ b/src/commands/ring/set-default.test.ts @@ -41,7 +41,7 @@ describe("test execute function and logic", () => { }, prod: {}, }, - services: {}, + services: [], variableGroups: ["testvg"], version: "1.0", }); diff --git a/src/commands/service/create-revision.test.ts b/src/commands/service/create-revision.test.ts index 74b9175b3..70c8fde05 100644 --- a/src/commands/service/create-revision.test.ts +++ b/src/commands/service/create-revision.test.ts @@ -1,6 +1,6 @@ import { write } from "../../config"; import * as config from "../../config"; -import { DEFAULT_CONTENT as BedrockMockedContent } from "../../lib/bedrockYaml"; +import * as bedrockYaml from "../../lib/bedrockYaml"; import * as azure from "../../lib/git/azure"; import * as gitutils from "../../lib/gitutils"; import { createTempDir } from "../../lib/ioUtil"; @@ -19,7 +19,7 @@ jest .mockReturnValueOnce(Promise.resolve("prod")) .mockReturnValue(Promise.resolve("")); jest.spyOn(config, "Config").mockReturnValue({}); -jest.spyOn(config, "Bedrock").mockReturnValue(BedrockMockedContent); +jest.spyOn(config, "Bedrock").mockReturnValue(bedrockYaml.DEFAULT_CONTENT()); describe("test makePullRequest function", () => { it("sanity test", async (done) => { @@ -52,8 +52,9 @@ describe("Default rings", () => { prod: { isDefault: false }, westus: { isDefault: true }, }, - services: { - "foo/a": { + services: [ + { + path: "foo/a", helm: { chart: { chart: "elastic", @@ -65,11 +66,11 @@ describe("Default rings", () => { pathPrefix: "servicepath", pathPrefixMajorVersion: "v1", }, - }, + ], version: "1.0", }; - write(validBedrockYaml, randomTmpDir); + bedrockYaml.create(randomTmpDir, validBedrockYaml); const defaultRings = getDefaultRings(undefined, validBedrockYaml); expect(defaultRings.length).toBe(2); expect(defaultRings[0]).toBe("master"); @@ -84,8 +85,9 @@ describe("Default rings", () => { prod: { isDefault: false }, westus: { isDefault: false }, }, - services: { - "foo/a": { + services: [ + { + path: "foo/a", helm: { chart: { chart: "elastic", @@ -97,11 +99,11 @@ describe("Default rings", () => { pathPrefix: "servicepath", pathPrefixMajorVersion: "v1", }, - }, + ], version: "1.0", }; - write(validBedrockYaml, randomTmpDir); + bedrockYaml.create(randomTmpDir, validBedrockYaml); let hasError = false; try { diff --git a/src/commands/service/create.decorator.json b/src/commands/service/create.decorator.json index 439621fdc..4608eb952 100644 --- a/src/commands/service/create.decorator.json +++ b/src/commands/service/create.decorator.json @@ -1,5 +1,5 @@ { - "command": "create ", + "command": "create ", "alias": "c", "description": "Add a new service into this initialized spk project repository", "options": [ @@ -38,11 +38,6 @@ "description": "The directory containing the mono-repo packages.", "defaultValue": "" }, - { - "arg": "-n, --display-name ", - "description": "Display name of the service.", - "defaultValue": "" - }, { "arg": "-m, --maintainer-name ", "description": "The name of the primary maintainer for this service.", diff --git a/src/commands/service/create.md b/src/commands/service/create.md index 7e3a41a7e..3b4590ff2 100644 --- a/src/commands/service/create.md +++ b/src/commands/service/create.md @@ -5,7 +5,7 @@ Add a new service into this initialized spk project repository. ## Example ```bash -spk service create . \ +spk service create my-service . \ --display-name $app_name \ --helm-config-path $path_to_chart_in_repo \ --helm-config-git $helm_repo_url \ # Needs to start with https and not contain user name @@ -26,7 +26,7 @@ spk service create . \ the generated Traefik2 IngressRoutes. i.e. ```sh - spk service create my-example-documents-service \ + spk service create my-example-documents-service path/to/my/service \ --middlewares middleware \ --k8s-backend-port 3001 \ --k8s-backend docs-service \ diff --git a/src/commands/service/create.test.ts b/src/commands/service/create.test.ts index c765d5e81..11f0c5347 100644 --- a/src/commands/service/create.test.ts +++ b/src/commands/service/create.test.ts @@ -7,7 +7,6 @@ import uuid = require("uuid/v4"); import { Bedrock } from "../../config"; import * as config from "../../config"; import * as bedrockYaml from "../../lib/bedrockYaml"; -import { DEFAULT_CONTENT as BedrockMockedContent } from "../../lib/bedrockYaml"; import { SERVICE_PIPELINE_FILENAME } from "../../lib/constants"; import { checkoutCommitPushCreatePRLink } from "../../lib/gitutils"; import { createTempDir, removeDir } from "../../lib/ioUtil"; @@ -30,6 +29,7 @@ import { CommandValues, validateGitUrl, } from "./create"; +import { BedrockServiceConfig, HelmConfig } from "../../types"; jest.mock("../../lib/gitutils"); @@ -46,7 +46,6 @@ beforeEach(() => { }); const mockValues: CommandValues = { - displayName: "", gitPush: false, helmChartChart: "", helmChartRepository: "", @@ -74,14 +73,14 @@ const getMockValues = (): CommandValues => { const validateDirNFiles = ( dir: string, - serviceName: string, + servicePath: string, values: CommandValues ): void => { // Check temp test directory exists expect(fs.existsSync(dir)).toBe(true); // Check service directory exists - const serviceDirPath = path.join(dir, values.packagesDir, serviceName); + const serviceDirPath = path.join(dir, values.packagesDir, servicePath); expect(fs.existsSync(serviceDirPath)).toBe(true); // Verify new azure-pipelines created @@ -107,7 +106,9 @@ describe("Test fetchValues function", () => { }); it("Positive test: with middlewares value", () => { - jest.spyOn(config, "Bedrock").mockReturnValueOnce(BedrockMockedContent); + jest + .spyOn(config, "Bedrock") + .mockReturnValueOnce(bedrockYaml.DEFAULT_CONTENT()); const mocked = getMockValues(); mocked.middlewares = "mid1, mid2"; // space after comma is intentional, expecting trimming to happen const result = fetchValues(mocked); @@ -115,7 +116,7 @@ describe("Test fetchValues function", () => { }); it("Positive test: with bedrock rings", () => { - const mockedBedrockFileConfig = { ...BedrockMockedContent }; + const mockedBedrockFileConfig = { ...bedrockYaml.DEFAULT_CONTENT() }; mockedBedrockFileConfig.rings = { master: {}, qa: {}, @@ -129,7 +130,9 @@ describe("Test fetchValues function", () => { it("Positive test", () => { const mocked = getMockValues(); - jest.spyOn(config, "Bedrock").mockReturnValueOnce(BedrockMockedContent); + jest + .spyOn(config, "Bedrock") + .mockReturnValueOnce(bedrockYaml.DEFAULT_CONTENT()); const result = fetchValues(mocked); expect(result).toEqual(mocked); }); @@ -139,7 +142,6 @@ describe("isValidDnsInputs", () => { test("valid inputs does not throw", () => { expect(() => assertValidDnsInputs({ - displayName: "bar", k8sBackend: "my-service", pathPrefix: "service", pathPrefixMajorVersion: "v1", @@ -150,16 +152,6 @@ describe("isValidDnsInputs", () => { test("invalid inputs throws", () => { expect(() => assertValidDnsInputs({ - displayName: "-not_dns_compliant", - k8sBackend: "", - pathPrefix: "", - pathPrefixMajorVersion: "", - }) - ).toThrow(); - - expect(() => - assertValidDnsInputs({ - displayName: "", k8sBackend: "-not_dns_compliant", pathPrefix: "", pathPrefixMajorVersion: "", @@ -168,7 +160,6 @@ describe("isValidDnsInputs", () => { expect(() => assertValidDnsInputs({ - displayName: "", k8sBackend: "", pathPrefix: "-not_dns_compliant", pathPrefixMajorVersion: "", @@ -177,7 +168,6 @@ describe("isValidDnsInputs", () => { expect(() => assertValidDnsInputs({ - displayName: "", k8sBackend: "", pathPrefix: "", pathPrefixMajorVersion: "-not_dns_compliant", @@ -190,30 +180,20 @@ describe("Test execute function", () => { it("Negative test: with non-dns compliant values", async () => { const exitFn = jest.fn(); jest.spyOn(dns, "assertIsValid"); - await execute( - "foo", - { ...getMockValues(), displayName: "-non_dns@compliant" }, - exitFn - ); + await execute("-non_dns@compliant", "foo", { ...getMockValues() }, exitFn); expect(dns.assertIsValid).toHaveBeenCalledTimes(1); }); it("Negative test: without service name", async () => { const exitFn = jest.fn(); - await execute("", getMockValues(), exitFn); - expect(exitFn).toBeCalledTimes(1); - expect(exitFn.mock.calls).toEqual([[1]]); - }); - - it("Negative test: service name is cwd and missing display name", async () => { - const exitFn = jest.fn(); - await execute(".", getMockValues(), exitFn); + await execute("", "my/service/path", getMockValues(), exitFn); expect(exitFn).toBeCalledTimes(1); expect(exitFn.mock.calls).toEqual([[1]]); }); it("Negative test: missing bedrock file", async () => { const testServiceName = uuid(); + const testServicePath = "my/service/path"; const exitFn = jest.fn(); jest.spyOn(bedrockYaml, "fileInfo").mockImplementation(() => ({ @@ -222,7 +202,7 @@ describe("Test execute function", () => { })); try { - await execute(testServiceName, getMockValues(), exitFn); + await execute(testServiceName, testServicePath, getMockValues(), exitFn); expect(exitFn).toBeCalledTimes(1); expect(exitFn.mock.calls).toEqual([[1]]); } finally { @@ -232,6 +212,7 @@ describe("Test execute function", () => { it("Negative test: missing bedrock variable groups", async () => { const testServiceName = uuid(); + const testServicePath = "my/service/path"; const exitFn = jest.fn(); jest.spyOn(bedrockYaml, "fileInfo").mockImplementation(() => ({ @@ -240,7 +221,7 @@ describe("Test execute function", () => { })); try { - await execute(testServiceName, getMockValues(), exitFn); + await execute(testServiceName, testServicePath, getMockValues(), exitFn); expect(exitFn).toBeCalledTimes(1); expect(exitFn.mock.calls).toEqual([[1]]); } finally { @@ -299,33 +280,6 @@ describe("Adding a service to a repo directory", () => { randomTmpDir = createTempDir(); }); - test("New service is created in project root directory. No display name given, so this should throw an error.", async () => { - await writeSampleMaintainersFileToDir( - path.join(randomTmpDir, "maintainers.yaml") - ); - await writeSampleBedrockFileToDir(path.join(randomTmpDir, "bedrock.yaml")); - - const values = getMockValues(); - values.packagesDir = ""; - values.k8sPort = 1337; - const serviceName = "."; - - logger.info( - `creating randomTmpDir ${randomTmpDir} and service ${serviceName}` - ); - - let hasError = false; - try { - await createService(randomTmpDir, serviceName, values); - } catch (err) { - hasError = true; - expect(err.message).toBe( - "Cannot create service pipeline due to serviceName being '.'. Please include a displayName if you are trying to create a service in your project root directory." - ); - } - expect(hasError).toBe(true); - }); - test("New service is created in project root directory. With display name given, so this works fine.", async () => { await writeSampleMaintainersFileToDir( path.join(randomTmpDir, "maintainers.yaml") @@ -335,34 +289,34 @@ describe("Adding a service to a repo directory", () => { const values = getMockValues(); values.packagesDir = ""; values.k8sPort = 1337; - values.displayName = "my-service-name"; values.helmConfigAccessTokenVariable = "SOME_ENV_VAR"; - const serviceName = "."; + const serviceName = "my-service-name"; + const servicePath = "."; logger.info( `creating randomTmpDir ${randomTmpDir} and service ${serviceName}` ); - let hasError = false; - try { - await createService(randomTmpDir, serviceName, values); - } catch (err) { - hasError = true; - } - expect(hasError).toBe(false); - - await createService(randomTmpDir, serviceName, values); - validateDirNFiles(randomTmpDir, serviceName, values); + expect(createService(randomTmpDir, serviceName, servicePath, values)) + .resolves; + validateDirNFiles(randomTmpDir, servicePath, values); // TODO: Verify root project bedrock.yaml and maintainers.yaml has been changed too. const bedrock = Bedrock(randomTmpDir); - const newService = bedrock.services["./"]; - expect(newService).toBeDefined(); - expect(newService.k8sBackendPort).toBe(values.k8sPort); - expect(newService.displayName).toBe(values.displayName); - // eslint-disable-next-line @typescript-eslint/no-explicit-any - expect((newService.helm.chart as any).accessTokenVariable).toBe( - values.helmConfigAccessTokenVariable + const newService = bedrock.services.find( + (s) => s.displayName === "my-service-name" + ) as BedrockServiceConfig; + expect(newService).toStrictEqual( + expect.objectContaining({ + path: "./", + k8sBackendPort: values.k8sPort, + displayName: serviceName, + helm: { + chart: expect.objectContaining({ + accessTokenVariable: "SOME_ENV_VAR", + }), + }, + }) ); }); @@ -376,19 +330,27 @@ describe("Adding a service to a repo directory", () => { values.packagesDir = ""; values.k8sPort = 1337; const serviceName = uuid(); + const servicePath = uuid(); logger.info( `creating randomTmpDir ${randomTmpDir} and service ${serviceName}` ); - await createService(randomTmpDir, serviceName, values); - validateDirNFiles(randomTmpDir, serviceName, values); + await createService(randomTmpDir, serviceName, servicePath, values); + validateDirNFiles(randomTmpDir, servicePath, values); // TODO: Verify root project bedrock.yaml and maintainers.yaml has been changed too. const bedrock = Bedrock(randomTmpDir); - const newService = bedrock.services["./" + serviceName]; - expect(newService).toBeDefined(); - expect(newService.k8sBackendPort).toBe(values.k8sPort); + const newService = bedrock.services.find( + (s) => s.displayName === serviceName + ) as BedrockServiceConfig; + expect(newService).toStrictEqual( + expect.objectContaining({ + path: "./" + servicePath, + k8sBackendPort: values.k8sPort, + displayName: serviceName, + }) + ); }); test("New directory is created under '/packages' directory with required service files.", async () => { @@ -401,14 +363,15 @@ describe("Adding a service to a repo directory", () => { values.packagesDir = "packages"; values.k8sPort = 1337; const serviceName = uuid(); + const servicePath = uuid(); logger.info( `creating randomTmpDir ${randomTmpDir} and service ${serviceName}` ); // addService call - await createService(randomTmpDir, serviceName, values); - validateDirNFiles(randomTmpDir, serviceName, values); + await createService(randomTmpDir, serviceName, servicePath, values); + validateDirNFiles(randomTmpDir, servicePath, values); // TODO: Verify root project bedrock.yaml and maintainers.yaml has been changed too. }); @@ -424,13 +387,14 @@ describe("Adding a service to a repo directory", () => { values.gitPush = true; values.k8sPort = 1337; const serviceName = uuid(); + const servicePath = uuid(); logger.info( `creating randomTmpDir ${randomTmpDir} and service ${serviceName}` ); - await createService(randomTmpDir, serviceName, values); - validateDirNFiles(randomTmpDir, serviceName, values); + await createService(randomTmpDir, serviceName, servicePath, values); + validateDirNFiles(randomTmpDir, servicePath, values); expect(checkoutCommitPushCreatePRLink).toHaveBeenCalled(); }); @@ -444,21 +408,21 @@ describe("Adding a service to a repo directory", () => { const values = getMockValues(); values.k8sPort = 1337; const serviceName = uuid(); + const servicePath = uuid(); + logger.info( `creating randomTmpDir ${randomTmpDir} and service ${serviceName}` ); // create service with no middleware - await createService(randomTmpDir, serviceName, values); - validateDirNFiles(randomTmpDir, serviceName, values); + await createService(randomTmpDir, serviceName, servicePath, values); + validateDirNFiles(randomTmpDir, servicePath, values); const bedrockConfig = Bedrock(randomTmpDir); // check the added service has an empty list for middlewares - for (const [servicePath, service] of Object.entries( - bedrockConfig.services - )) { - if (servicePath.includes(serviceName)) { + for (const service of bedrockConfig.services) { + if (service.displayName === serviceName) { expect(service.middlewares).toBeDefined(); expect(Array.isArray(service.middlewares)).toBe(true); expect(service.middlewares!.length).toBe(0); @@ -477,20 +441,19 @@ describe("Adding a service to a repo directory", () => { values.middlewaresArray = ["foo", "bar", "baz"]; const serviceName = uuid(); + const servicePath = uuid(); logger.info( `creating randomTmpDir ${randomTmpDir} and service ${serviceName}` ); - await createService(randomTmpDir, serviceName, values); - validateDirNFiles(randomTmpDir, serviceName, values); + await createService(randomTmpDir, serviceName, servicePath, values); + validateDirNFiles(randomTmpDir, servicePath, values); const bedrockConfig = Bedrock(randomTmpDir); // check that the added service has the expected middlewares - for (const [servicePath, service] of Object.entries( - bedrockConfig.services - )) { - if (servicePath.includes(serviceName)) { + for (const service of bedrockConfig.services) { + if (service.displayName === serviceName) { expect(service.middlewares).toBeDefined(); expect(Array.isArray(service.middlewares)).toBe(true); expect(service.middlewares?.length).toBe( diff --git a/src/commands/service/create.ts b/src/commands/service/create.ts index 1e5b981f6..5ab07f251 100644 --- a/src/commands/service/create.ts +++ b/src/commands/service/create.ts @@ -29,7 +29,6 @@ import { BedrockFileInfo, HelmConfig, User } from "../../types"; import decorator from "./create.decorator.json"; export interface CommandOptions { - displayName: string; gitPush: boolean; helmChartChart: string; helmChartRepository: string; @@ -69,7 +68,6 @@ export const fetchValues = (opts: CommandOptions): CommandValues => { } const values: CommandValues = { - displayName: opts.displayName, gitPush: opts.gitPush, helmChartChart: opts.helmChartChart, helmChartRepository: opts.helmChartRepository, @@ -113,9 +111,6 @@ export const checkDependencies = (projectPath: string): void => { * pathPrefixMajorVersion are provided and are non-DNS compliant */ export const assertValidDnsInputs = (opts: Partial): void => { - if (opts.displayName) { - dns.assertIsValid("--display-name", opts.displayName); - } if (opts.k8sBackend) { dns.assertIsValid("--k8s-backend", opts.k8sBackend); } @@ -132,6 +127,7 @@ export const assertValidDnsInputs = (opts: Partial): void => { export const execute = async ( serviceName: string, + servicePath: string, opts: CommandOptions, exitFn: (status: number) => Promise ): Promise => { @@ -141,16 +137,15 @@ export const execute = async ( return; } - if (serviceName === "." && opts.displayName === "") { - logger.error( - `If specifying the current directory as service name, please include a display name using '-n'` - ); + if (!servicePath) { + logger.error("Service path is missing"); await exitFn(1); return; } // validate user inputs are DNS compliant try { + dns.assertIsValid("", serviceName); assertValidDnsInputs(opts); } catch (err) { logger.error(err); @@ -166,7 +161,7 @@ export const execute = async ( try { checkDependencies(projectPath); const values = fetchValues(opts); - await createService(projectPath, serviceName, values); + await createService(projectPath, serviceName, servicePath, values); await exitFn(0); } catch (err) { logger.error( @@ -228,8 +223,8 @@ export const validateGitUrl = async ( */ export const commandDecorator = (command: commander.Command): void => { buildCmd(command, decorator).action( - async (serviceName: string, opts: CommandOptions) => { - await execute(serviceName, opts, async (status: number) => { + async (serviceName: string, servicePath: string, opts: CommandOptions) => { + await execute(serviceName, servicePath, opts, async (status: number) => { await exitCmd(logger, process.exit, status); }); } @@ -246,46 +241,27 @@ export const commandDecorator = (command: commander.Command): void => { export const createService = async ( rootProjectPath: string, serviceName: string, + servicePath: string, values: CommandValues ): Promise => { logger.info( - `Adding Service: ${serviceName}, to Project: ${rootProjectPath} under directory: ${values.packagesDir}` - ); - logger.info( - `DisplayName: ${values.displayName}, MaintainerName: ${values.maintainerName}, MaintainerEmail: ${values.maintainerEmail}` + `Adding Service: ${serviceName}, located at ${servicePath}, to Project: ${rootProjectPath} under directory: ${values.packagesDir}` ); const newServiceDir = path.join( rootProjectPath, values.packagesDir, - serviceName + servicePath ); logger.info(`servicePath: ${newServiceDir}`); shelljs.mkdir("-p", newServiceDir); - const pipelineServiceName = values.displayName - ? values.displayName - : serviceName; // displayName takes priority over serviceName (which could be '.') - // Sanity check - if ( - pipelineServiceName === "." || - pipelineServiceName === "./" || - pipelineServiceName === "./." - ) { - logger.error( - `Cannot create service pipeline due to serviceName being '.'. Please include a displayName if you are trying to create a service in your project root directory.` - ); - throw Error( - "Cannot create service pipeline due to serviceName being '.'. Please include a displayName if you are trying to create a service in your project root directory." - ); - } - // Create azure pipelines yaml in directory generateServiceBuildAndUpdatePipelineYaml( rootProjectPath, values.ringNames, - pipelineServiceName, + serviceName, newServiceDir, values.variableGroups ); @@ -333,7 +309,7 @@ export const createService = async ( addNewServiceToBedrockFile( rootProjectPath, newServiceRelativeDir, - values.displayName, + serviceName, helmConfig, values.middlewaresArray, values.k8sPort, diff --git a/src/config.test.ts b/src/config.test.ts index 5b91b520b..50274f7e7 100644 --- a/src/config.test.ts +++ b/src/config.test.ts @@ -10,12 +10,12 @@ import { loadConfiguration, saveConfiguration, updateVariableWithLocalEnv, - write, } from "./config"; import { createTempDir } from "./lib/ioUtil"; import { disableVerboseLogging, enableVerboseLogging } from "./logger"; import { BedrockFile } from "./types"; import { getVersionMessage } from "./lib/fileutils"; +import * as bedrockYaml from "./lib/bedrockYaml"; beforeAll(() => { enableVerboseLogging(); @@ -59,8 +59,9 @@ describe("Bedrock", () => { shell.mkdir("-p", randomTmpDir); const validBedrockYaml: BedrockFile = { rings: {}, - services: { - "foo/a": { + services: [ + { + path: "foo/a", helm: { chart: { chart: "elastic", @@ -72,7 +73,8 @@ describe("Bedrock", () => { pathPrefix: "servicepath", pathPrefixMajorVersion: "v1", }, - "foo/b": { + { + path: "foo/b", helm: { chart: { git: "foo", @@ -85,10 +87,11 @@ describe("Bedrock", () => { pathPrefix: "servicepath", pathPrefixMajorVersion: "v1", }, - }, + ], version: "1.0", }; - write(validBedrockYaml, randomTmpDir); + + bedrockYaml.create(randomTmpDir, validBedrockYaml); const expectedFilePath = path.join(randomTmpDir, "bedrock.yaml"); expect(writeSpy).toBeCalledWith( expectedFilePath, @@ -127,7 +130,7 @@ describe("Bedrock", () => { version: "1.0", // eslint-disable-next-line @typescript-eslint/no-explicit-any } as any; - write(validBedrockYaml, randomTmpDir); + bedrockYaml.create(randomTmpDir, validBedrockYaml); const expectedFilePath = path.join(randomTmpDir, "bedrock.yaml"); expect(writeSpy).toBeCalledWith( expectedFilePath, diff --git a/src/config.ts b/src/config.ts index 6d51900b9..bec5bff1b 100644 --- a/src/config.ts +++ b/src/config.ts @@ -7,10 +7,11 @@ import { writeVersion } from "./lib/fileutils"; import { logger } from "./logger"; import { AzurePipelinesYaml, - BedrockFile, ConfigYaml, MaintainersFile, + BedrockFile, } from "./types"; +import * as bedrockYaml from "./lib/bedrockYaml"; //////////////////////////////////////////////////////////////////////////////// // State @@ -148,6 +149,8 @@ export const Config = (): ConfigYaml => { }; /** + * **DEPRECATED**: Use `read()` from bedrockYaml lib + * * Returns the current bedrock.yaml file for the project * * Does some validations against the file; if errors occur, an Exception is @@ -158,7 +161,7 @@ export const Config = (): ConfigYaml => { */ export const Bedrock = (fileDirectory = process.cwd()): BedrockFile => { const bedrockYamlPath = path.join(fileDirectory, "bedrock.yaml"); - const bedrock = readYaml(bedrockYamlPath); + const bedrock = bedrockYaml.read(fileDirectory); const { services } = bedrock; // validate service helm configurations @@ -195,6 +198,8 @@ export const Bedrock = (fileDirectory = process.cwd()): BedrockFile => { }; /** + * **DEPRECATED**: Use `read()` from bedrockYaml lib + * * Async wrapper for the Bedrock() function * Use this if preferring to use Promise based control flow over try/catch as * Bedrock() can throw and Error @@ -225,22 +230,17 @@ export const MaintainersAsync = async ( ): Promise => Maintainers(fileDirectory); /** - * Helper to write out a bedrock.yaml or maintainers.yaml file to the project root + * Helper to write out a maintainers.yaml or azure-pipelines.yaml file to the project root * * @param file config file object to serialize and write out */ export const write = ( - file: BedrockFile | MaintainersFile | AzurePipelinesYaml, + file: MaintainersFile | AzurePipelinesYaml, targetDirectory = process.cwd(), fileName?: string ): void => { const asYaml = yaml.safeDump(file, { lineWidth: Number.MAX_SAFE_INTEGER }); - if ("rings" in file) { - // Is bedrock.yaml - fileName = "bedrock.yaml"; - writeVersion(path.join(targetDirectory, fileName)); - return fs.appendFileSync(path.join(targetDirectory, fileName), asYaml); - } else if ("services" in file) { + if ("services" in file) { // Is maintainers file fileName = "maintainers.yaml"; writeVersion(path.join(targetDirectory, fileName)); diff --git a/src/lib/bedrockYaml.test.ts b/src/lib/bedrockYaml/bedrockYaml.test.ts similarity index 81% rename from src/lib/bedrockYaml.test.ts rename to src/lib/bedrockYaml/bedrockYaml.test.ts index 486d8b53f..314310cc9 100644 --- a/src/lib/bedrockYaml.test.ts +++ b/src/lib/bedrockYaml/bedrockYaml.test.ts @@ -1,7 +1,7 @@ import uuid = require("uuid/v4"); -import { createTempDir } from "../lib/ioUtil"; -import { createTestBedrockYaml } from "../test/mockFactory"; -import { BedrockFile, HelmConfig } from "../types"; +import { createTestBedrockYaml } from "../../test/mockFactory"; +import { BedrockFile, HelmConfig, RingConfig } from "../../types"; +import { createTempDir } from "../ioUtil"; import { addNewRing, addNewService, @@ -13,25 +13,26 @@ import { removeRing, setDefaultRing, validateRings, + getRings, } from "./bedrockYaml"; describe("Creation and Existence test on bedrock.yaml", () => { it("without folder name in creation function call", () => { const dir = create(); expect(isExists(dir)).toBe(true); - expect(read(dir)).toEqual(DEFAULT_CONTENT); + expect(read(dir)).toEqual(DEFAULT_CONTENT()); }); it("with folder name in creation function call", () => { const dir = createTempDir(); create(dir); expect(isExists(dir)).toBe(true); - expect(read(dir)).toEqual(DEFAULT_CONTENT); + expect(read(dir)).toEqual(DEFAULT_CONTENT()); }); it("withContent", () => { const dir = createTempDir(); const data = { rings: {}, - services: {}, + services: [], version: "1.0", }; create(dir, data); @@ -80,9 +81,10 @@ describe("Adding a new service to a Bedrock file", () => { const expected: BedrockFile = { ...defaultBedrockFileObject, - services: { + services: [ ...(defaultBedrockFileObject as BedrockFile).services, - ["./" + servicePath]: { + { + path: "./" + servicePath, displayName: svcDisplayName, helm: helmConfig, k8sBackend, @@ -91,7 +93,7 @@ describe("Adding a new service to a Bedrock file", () => { pathPrefix, pathPrefixMajorVersion, }, - }, + ], variableGroups: [], version: defaultBedrockFileObject.version, }; @@ -118,9 +120,7 @@ describe("Adding a new ring to an existing bedrock.yaml", () => { ...(defaultBedrockFileObject as BedrockFile).rings, [ringName]: {}, }, - services: { - ...(defaultBedrockFileObject as BedrockFile).services, - }, + services: [...(defaultBedrockFileObject as BedrockFile).services], variableGroups: [], version: defaultBedrockFileObject.version, }; @@ -149,7 +149,7 @@ describe("Bedrock file info", () => { const dir = createTempDir(); const data = { rings: {}, - services: {}, + services: [], variableGroups: [uuid()], version: "1.0", }; @@ -168,7 +168,7 @@ describe("Set default ring", () => { master: { isDefault: false }, prod: {}, }, - services: {}, + services: [], variableGroups: [uuid()], version: "1.0", }; @@ -185,7 +185,7 @@ describe("Set default ring", () => { master: { isDefault: false }, prod: { isDefault: true }, }, - services: {}, + services: [], variableGroups: [uuid()], version: "1.0", }; @@ -274,3 +274,50 @@ describe("validateRings", () => { }); } }); + +describe("getRings", () => { + const tests: { + name: string; + actual: unknown; + expected: Array; + }[] = [ + { + name: "empty rings", + actual: getRings({ rings: {}, services: [], version: "" }), + expected: [], + }, + { + name: "one ring -- isDefault is automatically added to output", + actual: getRings({ + rings: { master: {} }, + services: [], + version: "", + }), + expected: [{ name: "master", isDefault: false }], + }, + + { + name: "multiple rings -- isDefault is automatically added to output", + actual: getRings({ + rings: { + master: {}, + qa: { isDefault: false }, + dev: { isDefault: true }, + }, + services: [], + version: "", + }), + expected: [ + { name: "master", isDefault: false }, + { name: "qa", isDefault: false }, + { name: "dev", isDefault: true }, + ], + }, + ]; + + for (const { name, actual, expected } of tests) { + it(name, () => { + expect(actual).toStrictEqual(expected); + }); + } +}); diff --git a/src/lib/bedrockYaml.ts b/src/lib/bedrockYaml/bedrockYaml.ts similarity index 85% rename from src/lib/bedrockYaml.ts rename to src/lib/bedrockYaml/bedrockYaml.ts index 361ee36e2..6077de66c 100644 --- a/src/lib/bedrockYaml.ts +++ b/src/lib/bedrockYaml/bedrockYaml.ts @@ -1,25 +1,26 @@ import fs from "fs"; import yaml from "js-yaml"; import path from "path"; -import { createTempDir } from "../lib/ioUtil"; -import { logger } from "../logger"; +import { logger } from "../../logger"; import { BedrockFile, BedrockFileInfo, HelmConfig, - Rings, RingConfig, -} from "../types"; -import { writeVersion, getVersion } from "./fileutils"; + Rings, +} from "../../types"; +import { getVersion, writeVersion } from "../fileutils"; +import { createTempDir } from "../ioUtil"; +import * as indexByNameMigration from "./migrations/service-map-to-list"; export const YAML_NAME = "bedrock.yaml"; -export const DEFAULT_CONTENT: BedrockFile = { +export const DEFAULT_CONTENT = (): BedrockFile => ({ rings: {}, - services: {}, + services: [], variableGroups: [], version: getVersion(), -}; +}); /** * Creates a Bedrock.yaml file. @@ -34,7 +35,7 @@ export const DEFAULT_CONTENT: BedrockFile = { export const create = (dir?: string, data?: BedrockFile): string => { dir = dir || createTempDir(); const absPath = path.resolve(dir); - data = data || DEFAULT_CONTENT; + data = data || DEFAULT_CONTENT(); const asYaml = yaml.safeDump(data, { lineWidth: Number.MAX_SAFE_INTEGER, }); @@ -68,7 +69,8 @@ export const isExists = (dir: string): boolean => { export const read = (dir: string): BedrockFile => { const absPath = path.resolve(dir); const file = path.join(absPath, YAML_NAME); - return yaml.safeLoad(fs.readFileSync(file, "utf8")); + const b = yaml.safeLoad(fs.readFileSync(file, "utf8")); + return indexByNameMigration.migrate(b, dir); }; /** @@ -98,15 +100,19 @@ export const addNewService = ( const absPath = path.resolve(dir); const data = read(absPath); - data.services["./" + newServicePath] = { - displayName: svcDisplayName, - helm: helmConfig, - k8sBackend, - k8sBackendPort, - middlewares, - pathPrefix, - pathPrefixMajorVersion, - }; + data.services = [ + ...data.services.filter((s) => s.displayName !== svcDisplayName), + { + displayName: svcDisplayName, + path: "./" + newServicePath, + helm: helmConfig, + k8sBackend, + k8sBackendPort, + middlewares, + pathPrefix, + pathPrefixMajorVersion, + }, + ]; const asYaml = yaml.safeDump(data, { lineWidth: Number.MAX_SAFE_INTEGER, @@ -240,6 +246,22 @@ export const removeRing = ( return bedrockWithoutRing; }; +/** + * Returns a list of Ring configurations with an added `name` parameter which + * it is indexed on. + * + * @param b bedrock config file + */ +export const getRings = ( + b: BedrockFile +): Array => { + return Object.entries(b.rings).map(([name, config]) => ({ + ...config, + name, + isDefault: !!config.isDefault, + })); +}; + /** * Validates that the rings in `bedrock` are valid and throws an Error if not. * @@ -249,10 +271,7 @@ export const removeRing = ( * @param bedrock file to validate the rings of */ export const validateRings = (bedrock: BedrockFile): void => { - const rings = Object.entries(bedrock.rings).reduce( - (carry, [name, config]) => carry.concat({ ...config, name }), - [] as (RingConfig & { name: string })[] - ); + const rings = getRings(bedrock); const defaultRings = rings.filter((ring) => ring.isDefault); if (defaultRings.length > 1) { const defaultRingsNames = defaultRings.map((ring) => ring.name).join(", "); diff --git a/src/lib/bedrockYaml/index.ts b/src/lib/bedrockYaml/index.ts new file mode 100644 index 000000000..56d6f8835 --- /dev/null +++ b/src/lib/bedrockYaml/index.ts @@ -0,0 +1 @@ +export * from "./bedrockYaml"; diff --git a/src/lib/bedrockYaml/migrations/service-map-to-list.test.ts b/src/lib/bedrockYaml/migrations/service-map-to-list.test.ts new file mode 100644 index 000000000..3330a67ae --- /dev/null +++ b/src/lib/bedrockYaml/migrations/service-map-to-list.test.ts @@ -0,0 +1,294 @@ +import { BedrockFile } from "../../../types"; +import * as migration from "./service-map-to-list"; +import { LegacyBedrockFile } from "./service-map-to-list"; +import uuid from "uuid/v4"; +import * as fs from "fs"; +import * as bedrockYaml from "../bedrockYaml"; +import * as os from "os"; +import * as path from "path"; +import yaml from "js-yaml"; +import { getVersion } from "../../fileutils"; + +describe("isLegacySchema", () => { + type test = { + name: string; + actual: () => unknown; + expected: unknown; + effects?: (() => void)[]; + }; + const createTest = ( + b: LegacyBedrockFile | BedrockFile, + legacy: boolean + ): test => { + return { + name: `${JSON.stringify(b.services)} == ${legacy}`, + actual: (): unknown => migration.isLegacySchema(b), + expected: legacy, + }; + }; + const tests: test[] = [ + createTest( + { + rings: {}, + services: {}, + version: "0.5.8", + }, + true + ), + createTest( + { + rings: {}, + services: [], + version: "0.5.8", + }, + false + ), + ]; + + for (const { name, actual, expected, effects } of tests) { + it(name, () => { + expect(actual()).toStrictEqual(expected); + for (const effect of effects ?? []) { + effect(); + } + }); + } +}); + +describe("convertToNewSchema", () => { + const tests: { + name: string; + actual: () => unknown; + expected: BedrockFile; + effects?: (() => void)[]; + }[] = [ + { + name: "converts empty service map to empty list", + actual: (): BedrockFile => + migration.convertToNewSchema({ + rings: {}, + services: {}, + version: "0.5.8", + }), + expected: { + rings: {}, + services: [], + version: "0.5.8", + }, + }, + + { + name: + "service index is swapped with displayName, correct path is injected, displayName is removed", + actual: (): BedrockFile => + migration.convertToNewSchema({ + rings: { + master: { isDefault: true }, + preProd: { isDefault: false }, + qa: {}, + }, + services: { + "./foo": { + displayName: "foo-display-name", + helm: { + chart: { + git: "foo.bar.git", + path: "foo", + branch: "master", + }, + }, + k8sBackendPort: 80, + }, + "./src/bar": { + displayName: "bar-display-name", + helm: { + chart: { + git: "foo.bar.git", + path: "bar", + branch: "master", + }, + }, + k8sBackendPort: 80, + }, + }, + version: "0.5.8", + }), + expected: { + rings: { + master: { isDefault: true }, + preProd: { isDefault: false }, + qa: {}, + }, + services: [ + { + displayName: "foo-display-name", + path: "./foo", + helm: { + chart: { + git: "foo.bar.git", + path: "foo", + branch: "master", + }, + }, + k8sBackendPort: 80, + }, + { + displayName: "bar-display-name", + path: "./src/bar", + helm: { + chart: { + git: "foo.bar.git", + path: "bar", + branch: "master", + }, + }, + k8sBackendPort: 80, + }, + ], + version: "0.5.8", + }, + }, + ]; + + for (const { name, actual, expected, effects } of tests) { + it(name, () => { + expect(actual()).toStrictEqual(expected); + for (const effect of effects ?? []) { + effect(); + } + }); + } +}); + +describe("migrate", () => { + let tempDir: string; + let tempBed: LegacyBedrockFile | BedrockFile; + + const tests: { + name: string; + actual: () => unknown; + expected: unknown; + effects?: (() => void)[]; + }[] = [ + { + name: "Non-legacy is noop", + actual: (): unknown => { + // scaffold a "current" schema bedrock + tempDir = bedrockYaml.create(); + tempBed = yaml.load( + fs.readFileSync(path.join(tempDir, bedrockYaml.YAML_NAME), "utf8") + ); + // migrate non-legacy yaml + return migration.migrate(tempBed, tempDir); + }, + expected: { + rings: {}, + services: [], + variableGroups: [], + version: getVersion(), + }, + }, + + { + name: "Legacy gets converted - empty service map", + actual: (): unknown => { + // scaffold a legacy style bedrock.yaml + tempDir = path.join(os.tmpdir(), uuid()); + const bedPath = path.join(tempDir, bedrockYaml.YAML_NAME); + fs.mkdirSync(tempDir); + fs.writeFileSync( + bedPath, + yaml.safeDump({ rings: {}, services: {}, version: "" }) + ); + tempBed = yaml.safeLoad(fs.readFileSync(bedPath, "utf8")); + // migrate the legacy yaml + return migration.migrate(tempBed, tempDir); + }, + expected: { rings: {}, services: [], version: "" }, + effects: [ + (): void => { + // load the modified yaml via filesystem + const modified = yaml.load( + fs.readFileSync(path.join(tempDir, bedrockYaml.YAML_NAME), "utf8") + ); + // it should match the new schema + expect(modified).toStrictEqual({ + rings: {}, + services: [], + version: "", + }); + }, + ], + }, + + { + name: "Legacy gets converted - non-empty service map", + actual: (): unknown => { + // scaffold legacy bedrock.yaml with services + tempDir = path.join(os.tmpdir(), uuid()); + const bedPath = path.join(tempDir, bedrockYaml.YAML_NAME); + fs.mkdirSync(tempDir); + fs.writeFileSync( + bedPath, + yaml.safeDump({ + rings: {}, + services: { + "./foo": { displayName: "foo-service" }, + "./bar": { displayName: "bar-service" }, + }, + version: "", + }) + ); + tempBed = yaml.safeLoad(fs.readFileSync(bedPath, "utf8")); + // migrate the yaml + return migration.migrate(tempBed, tempDir); + }, + expected: { + rings: {}, + services: expect.arrayContaining([ + expect.objectContaining({ + path: "./foo", + displayName: "foo-service", + }), + expect.objectContaining({ + path: "./bar", + displayName: "bar-service", + }), + ]), + version: "", + }, + effects: [ + (): void => { + // load the modified yaml from system + const modified = yaml.load( + fs.readFileSync(path.join(tempDir, bedrockYaml.YAML_NAME), "utf8") + ); + // updated yaml should match new schema + expect(modified).toStrictEqual({ + rings: {}, + services: expect.arrayContaining([ + expect.objectContaining({ + path: "./foo", + displayName: "foo-service", + }), + expect.objectContaining({ + path: "./bar", + displayName: "bar-service", + }), + ]), + version: "", + }); + }, + ], + }, + ]; + + for (const { name, actual, expected, effects } of tests) { + it(name, () => { + expect(actual()).toStrictEqual(expected); + for (const effect of effects ?? []) { + effect(); + } + }); + } +}); diff --git a/src/lib/bedrockYaml/migrations/service-map-to-list.ts b/src/lib/bedrockYaml/migrations/service-map-to-list.ts new file mode 100644 index 000000000..c9c8ffc43 --- /dev/null +++ b/src/lib/bedrockYaml/migrations/service-map-to-list.ts @@ -0,0 +1,94 @@ +import * as bedrock from ".."; +import { BedrockFile, Rings, HelmConfig } from "../../../types"; +import * as path from "path"; +import * as fs from "fs"; +import { logger } from "../../../logger"; +import yaml from "js-yaml"; + +export interface LegacyBedrockService { + displayName?: string; + disableRouteScaffold?: boolean; + helm: HelmConfig; + k8sBackend?: string; // k8s service backend name for ingress routing + k8sBackendPort: number; // the service port for the k8s service Traefik2 IngressRoutes will point to + middlewares?: string[]; + pathPrefix?: string; // pathprefix for ingress route, ie. document-service + pathPrefixMajorVersion?: string; // api version, will prefix path prefix if provided. ie. 'v1' will result in the endpoint: /v1/document-service +} + +/** + * Bedrock config file + * Used to capture service meta-information regarding how to deploy + */ +export interface LegacyBedrockFile { + rings: Rings; + services: { + [path: string]: LegacyBedrockService; + }; + variableGroups?: string[]; + version: string; +} + +/** + * Converts a legacy bedrock.yaml file to the new v0.6.0 syntax + * + * - Changes the index of the services to be the displayName, uses + * `getServiceName` to compute it. + * - Removes displayName from all services if found + * - Adds `path` to the service -- using the value of the existing index for the + * service + * + * @param legacy bedrockYaml to convert + */ +export const convertToNewSchema = (legacy: LegacyBedrockFile): BedrockFile => { + const services = Object.entries(legacy.services).map(([svcPath, config]) => ({ + ...config, + path: svcPath, + })); + + return { + ...legacy, + services, + }; +}; + +/** + * Checks if the provided bedrock yaml file is the legacy schema. + * It is considered legacy if the value of `b.services` is not an array + * + * @param b config to check if legacy or not + */ +export function isLegacySchema( + b: BedrockFile | LegacyBedrockFile +): b is LegacyBedrockFile { + return !Array.isArray(b.services); +} + +/** + * Run the migration to new Bedrock schema. + * + * - Attempts to load the bedrock.yaml file and check the version it legacy + * - If it is legacy, update it and overwrite + */ +export const migrate = ( + b: BedrockFile | LegacyBedrockFile, + bedrockDir: string +): BedrockFile => { + if (isLegacySchema(b)) { + logger.info( + "Legacy bedrock.yaml schema found -- Migrating to new schema..." + ); + // delete the original + const bedrockPath = path.resolve(path.join(bedrockDir, bedrock.YAML_NAME)); + fs.unlinkSync(path.join(bedrockDir, bedrock.YAML_NAME)); + + // write out the new one + logger.info(`Writing updated bedrock.yaml file to ${bedrockPath}`); + const migrated = convertToNewSchema(b); + bedrock.create(bedrockDir, migrated); + logger.info(`Successfully updated Bedrock config`); + return yaml.safeLoad(fs.readFileSync(bedrockPath, "utf8")); + } else { + return b; + } +}; diff --git a/src/lib/fileutils.ts b/src/lib/fileutils.ts index c8f76b683..326124042 100644 --- a/src/lib/fileutils.ts +++ b/src/lib/fileutils.ts @@ -1,8 +1,10 @@ import fs from "fs"; import yaml from "js-yaml"; import path from "path"; +import { readYaml, write } from "../config"; import { ACCESS_FILENAME, + BEDROCK_FILENAME, HELM_VERSION, HLD_COMPONENT_FILENAME, PROJECT_PIPELINE_FILENAME, @@ -10,8 +12,9 @@ import { SERVICE_PIPELINE_FILENAME, VERSION_MESSAGE, VM_IMAGE, - BEDROCK_FILENAME, } from "../lib/constants"; +import { build as buildError } from "../lib/errorBuilder"; +import { errorStatusCode } from "../lib/errorStatusCode"; import { logger } from "../logger"; import { AccessYaml, @@ -20,9 +23,6 @@ import { MaintainersFile, User, } from "../types"; -import { readYaml, write } from "../config"; -import { build as buildError } from "../lib/errorBuilder"; -import { errorStatusCode } from "../lib/errorStatusCode"; /** * Read given pipeline file as json object. diff --git a/src/lib/setup/scaffold.ts b/src/lib/setup/scaffold.ts index dd4da43ea..37f89dc29 100644 --- a/src/lib/setup/scaffold.ts +++ b/src/lib/setup/scaffold.ts @@ -206,8 +206,7 @@ export const initService = async ( rc: RequestContext, repoName: string ): Promise => { - await createService(".", ".", { - displayName: repoName, + await createService(".", repoName, ".", { gitPush: false, helmChartChart: "", helmChartRepository: "", diff --git a/src/test/mockFactory.ts b/src/test/mockFactory.ts index aa65d73e2..eff9c355b 100644 --- a/src/test/mockFactory.ts +++ b/src/test/mockFactory.ts @@ -286,20 +286,23 @@ export const createTestBedrockYaml = ( qa: {}, testing: {}, }, - services: { - "./": { + services: [ + { + path: "./", helm: service1HelmConfig, k8sBackendPort: 80, }, - "./packages/service1": { + { + path: "./packages/service1", helm: service2HelmConfig, k8sBackendPort: 80, }, - "./zookeeper": { + { + path: "./zookeeper", helm: zookeeperHelmConfig, k8sBackendPort: 80, }, - }, + ], variableGroups: [], version: "1.0", }; diff --git a/src/types.d.ts b/src/types.d.ts index b918e39eb..67854b909 100644 --- a/src/types.d.ts +++ b/src/types.d.ts @@ -53,22 +53,21 @@ export interface RingConfig { */ export interface BedrockFile { rings: Rings; - services: { - [relativeDirectory: string]: BedrockServiceConfig; - }; + services: BedrockServiceConfig[]; variableGroups?: string[]; version: string; } export interface BedrockServiceConfig { displayName?: string; - middlewares?: string[]; - helm: HelmConfig; + path: string; // a **new** required path to the service disableRouteScaffold?: boolean; + helm: HelmConfig; + k8sBackend?: string; // k8s service backend name for ingress routing k8sBackendPort: number; // the service port for the k8s service Traefik2 IngressRoutes will point to + middlewares?: string[]; pathPrefix?: string; // pathprefix for ingress route, ie. document-service pathPrefixMajorVersion?: string; // api version, will prefix path prefix if provided. ie. 'v1' will result in the endpoint: /v1/document-service - k8sBackend?: string; // k8s service backend name for ingress routing } /**@see https://docs.microsoft.com/en-us/azure/devops/pipelines/yaml-schema?view=azure-devops&tabs=schema%2Cparameter-schema#triggers */ diff --git a/tests/functions.sh b/tests/functions.sh index 8ee1df5a6..e66808855 100644 --- a/tests/functions.sh +++ b/tests/functions.sh @@ -267,7 +267,7 @@ function verify_pipeline_with_poll () { # We expect only 1 build right now build_count=$(tr '"\""' '"\\"' <<< "$pipeline_builds" | jq '. | length') - if [[ -z "$expected_build_count" ]]; then + if [[ -z "$expected_build_count" ]]; then expected_build_count=1 fi if [ "$build_count" != "$expected_build_count" ]; then @@ -341,8 +341,8 @@ function create_spk_project_and_service () { variable_group_exists $AZDO_ORG_URL $AZDO_PROJECT $var_group_name "fail" # $spk service create . -n "$repo_dir_name-service" -p chart -g "$1/$2/_git/$repo_dir_name" -b master >> $TEST_WORKSPACE/log.txt - echo "$spk service create . -n "$repo_dir_name-service" -p "$repo_dir_name/chart" -g $helm_repo_url -b master" - $spk service create . -n "$repo_dir_name-service" -p "$repo_dir_name/chart" -g $helm_repo_url -b master >> $TEST_WORKSPACE/log.txt + echo "$spk service create "$repo_dir_name-service" . -p "$repo_dir_name/chart" -g $helm_repo_url -b master" + $spk service create "$repo_dir_name-service" . -p "$repo_dir_name/chart" -g $helm_repo_url -b master >> $TEST_WORKSPACE/log.txt directory_to_check="$TEST_WORKSPACE/$repo_dir_name" file_we_expect=(".gitignore" "build-update-hld.yaml" "Dockerfile" "maintainers.yaml" "hld-lifecycle.yaml" "spk.log" "bedrock.yaml") validate_directory $directory_to_check "${file_we_expect[@]}" diff --git a/tests/validations.sh b/tests/validations.sh index c6bd57e2d..ea8a4f657 100644 --- a/tests/validations.sh +++ b/tests/validations.sh @@ -224,14 +224,14 @@ cd "$TEST_WORKSPACE/$mono_repo_dir" # helm_repo_url="$AZDO_ORG_URL/$AZDO_PROJECT/_git/$helm_charts_dir" local_repo_url="$AZDO_ORG_URL/$AZDO_PROJECT/_git/$mono_repo_dir" -spk service create $FrontEnd -d $services_dir -p "chart" -g $local_repo_url -b master >> $TEST_WORKSPACE/log.txt -# spk service create $FrontEnd -d $services_dir -p "$FrontEnd/chart" -g $helm_repo_url -b master >> $TEST_WORKSPACE/log.txt +spk service create $FrontEnd $FrontEnd -d $services_dir -p "chart" -g $local_repo_url -b master >> $TEST_WORKSPACE/log.txt +# spk service create $FrontEnd $FrontEnd -d $services_dir -p "$FrontEnd/chart" -g $helm_repo_url -b master >> $TEST_WORKSPACE/log.txt directory_to_check="$services_full_dir/$FrontEnd" file_we_expect=(".gitignore" "build-update-hld.yaml" "Dockerfile" ) validate_directory $directory_to_check "${file_we_expect[@]}" # TODO uncomment this when helm chart fixed -# spk service create $BackEnd -d $services_dir -p "$BackEnd/chart" -g $helm_repo_url -b master >> $TEST_WORKSPACE/log.txt +# spk service create $BackEnd $BackEnd -d $services_dir -p "$BackEnd/chart" -g $helm_repo_url -b master >> $TEST_WORKSPACE/log.txt # validate_directory "$services_full_dir/$BackEnd" "${file_we_expect[@]}" git add -A From 8a0bcf67b7a501f7d7e6c79980efd7a002578788 Mon Sep 17 00:00:00 2001 From: Samiya Akhtar Date: Fri, 3 Apr 2020 14:15:12 -0700 Subject: [PATCH 03/14] First pass to get display name for service from bedrock.yaml --- docs/commands/data.json | 6 ++ .../project/get-display-name.decorator.json | 6 ++ src/commands/project/get-display-name.ts | 59 +++++++++++++++++++ src/commands/project/index.ts | 7 ++- src/lib/fileutils.ts | 6 +- src/test/mockFactory.ts | 6 +- 6 files changed, 85 insertions(+), 5 deletions(-) create mode 100644 src/commands/project/get-display-name.decorator.json create mode 100644 src/commands/project/get-display-name.ts diff --git a/docs/commands/data.json b/docs/commands/data.json index 9a25e537a..115d020d6 100644 --- a/docs/commands/data.json +++ b/docs/commands/data.json @@ -418,6 +418,12 @@ ], "markdown": "## Description\n\nCreate new variable group in Azure DevOps project\n\n## Command Prerequisites\n\nIn addition to an existing\n[Azure DevOps project](https://azure.microsoft.com/en-us/services/devops/), to\nlink secrets from an Azure key vault as variables in Variable Group, you will\nneed an existing key vault containing your secrets and the Service Principal for\nauthorization with Azure Key Vault.\n\n1. Use existng or\n [create a service principal either in Azure Portal](https://docs.microsoft.com/en-us/azure/active-directory/develop/howto-create-service-principal-portal)\n or\n [with Azure CLI](https://docs.microsoft.com/en-us/cli/azure/create-an-azure-service-principal-azure-cli?view=azure-cli-latest).\n2. Use existing or\n [create a Azure Container Registry in Azure Portal](https://docs.microsoft.com/en-us/azure/container-registry/container-registry-get-started-portal)\n or\n [with Azure CLI](https://docs.microsoft.com/en-us/azure/container-registry/container-registry-get-started-azure-cli).\n" }, + "project get-display-name": { + "command": "get-display-name", + "alias": "gdn", + "description": "Gets display name for a service", + "options": [] + }, "project init": { "command": "init", "alias": "i", diff --git a/src/commands/project/get-display-name.decorator.json b/src/commands/project/get-display-name.decorator.json new file mode 100644 index 000000000..84a980820 --- /dev/null +++ b/src/commands/project/get-display-name.decorator.json @@ -0,0 +1,6 @@ +{ + "command": "get-display-name", + "alias": "gdn", + "description": "Gets display name for a service", + "options": [] +} diff --git a/src/commands/project/get-display-name.ts b/src/commands/project/get-display-name.ts new file mode 100644 index 000000000..c1fc7b53b --- /dev/null +++ b/src/commands/project/get-display-name.ts @@ -0,0 +1,59 @@ +/* eslint-disable @typescript-eslint/no-use-before-define */ +/* eslint-disable @typescript-eslint/camelcase */ +import commander, { CommandOptions } from "commander"; +import * as fs from "fs"; +import { read as readBedrockYaml } from "../../lib/bedrockYaml"; +import { build as buildCmd, exit as exitCmd } from "../../lib/commandBuilder"; +import { logger } from "../../logger"; +import decorator from "./get-display-name.decorator.json"; + +/** + * Executes the command, can all exit function with 0 or 1 + * when command completed successfully or failed respectively. + * + * @param opts validated option values + * @param exitFn exit function + */ +export const execute = async ( + opts: CommandOptions, + exitFn: (status: number) => Promise +): Promise => { + let currentDirectory = process.cwd() + "/bedrock.yaml"; + try { + // First, find a bedrock.yaml file in this directory and if not, iterate by + // one directory up. + while (!fs.existsSync(currentDirectory)) { + const split = currentDirectory.split("/"); + currentDirectory = currentDirectory.replace( + split[split.length - 2] + "/", + "" + ); + } + currentDirectory = currentDirectory.replace("/bedrock.yaml", ""); + const bedrockFile = readBedrockYaml(currentDirectory); + const servicePath = process.cwd().replace(currentDirectory, "."); + for (const serviceIndex in bedrockFile.services) { + if (servicePath === bedrockFile.services[serviceIndex].path) { + console.log(bedrockFile.services[serviceIndex].displayName); + await exitFn(0); + } + } + // Ideally, we should not be getting here, so throw an error. + throw new Error("Display name for current directory could not be found."); + } catch (err) { + logger.error(err); + await exitFn(1); + } +}; + +/** + * Adds the validate command to the commander command object + * @param command Commander command object to decorate + */ +export const commandDecorator = (command: commander.Command): void => { + buildCmd(command, decorator).action(async (opts: CommandOptions) => { + await execute(opts, async (status: number) => { + await exitCmd(logger, process.exit, status); + }); + }); +}; diff --git a/src/commands/project/index.ts b/src/commands/project/index.ts index e624fd278..1d0771c60 100644 --- a/src/commands/project/index.ts +++ b/src/commands/project/index.ts @@ -1,6 +1,11 @@ import { Command } from "../command"; -const subfolders = ["create-variable-group", "init", "pipeline"]; +const subfolders = [ + "create-variable-group", + "init", + "pipeline", + "get-display-name", +]; export const commandDecorator = Command( "project", diff --git a/src/lib/fileutils.ts b/src/lib/fileutils.ts index 326124042..a3e725fec 100644 --- a/src/lib/fileutils.ts +++ b/src/lib/fileutils.ts @@ -199,8 +199,10 @@ export const serviceBuildAndUpdatePipeline = ( `tag_name="$BUILD_REPO_NAME:${IMAGE_TAG}"`, `commitId=$(Build.SourceVersion)`, `commitId=$(echo "\${commitId:0:7}")`, - `service=$(Build.Repository.Name)`, - `service=\${service##*/}`, + `currentDirectory=$(pwd)`, + `cd ${relativeServiceForDockerfile}`, + `service=$(spk project get-display-name)`, + `cd $currentDirectory`, `url=$(git remote --verbose | grep origin | grep fetch | cut -f2 | cut -d' ' -f1)`, `repourl=\${url##*@}`, `. ./build.sh --source-only`, diff --git a/src/test/mockFactory.ts b/src/test/mockFactory.ts index eff9c355b..b08fe834e 100644 --- a/src/test/mockFactory.ts +++ b/src/test/mockFactory.ts @@ -79,8 +79,10 @@ export const createTestServiceBuildAndUpdatePipelineYaml = ( `tag_name="$BUILD_REPO_NAME:${IMAGE_TAG}"`, `commitId=$(Build.SourceVersion)`, `commitId=$(echo "\${commitId:0:7}")`, - `service=$(Build.Repository.Name)`, - `service=\${service##*/}`, + `currentDirectory=$(pwd)`, + `cd ${relativeServiceForDockerfile}`, + `service=$(spk project get-display-name)`, + `cd $currentDirectory`, `url=$(git remote --verbose | grep origin | grep fetch | cut -f2 | cut -d' ' -f1)`, `repourl=\${url##*@}`, `. ./build.sh --source-only`, From 9f9a26f01c9d517fde707cb4904cb02dc9ce93e1 Mon Sep 17 00:00:00 2001 From: Samiya Akhtar Date: Fri, 3 Apr 2020 15:26:52 -0700 Subject: [PATCH 04/14] Adding basic unit tests --- src/commands/project/get-display-name.test.ts | 34 +++++++++++++++++++ src/commands/project/get-display-name.ts | 3 ++ 2 files changed, 37 insertions(+) create mode 100644 src/commands/project/get-display-name.test.ts diff --git a/src/commands/project/get-display-name.test.ts b/src/commands/project/get-display-name.test.ts new file mode 100644 index 000000000..6c44cfaa4 --- /dev/null +++ b/src/commands/project/get-display-name.test.ts @@ -0,0 +1,34 @@ +import { + disableVerboseLogging, + enableVerboseLogging, + logger, +} from "../../logger"; +import { execute } from "./get-display-name"; +import * as fs from "fs"; +import { createTestBedrockYaml } from "../../test/mockFactory"; +import { BedrockFile } from "../../types"; +import * as bedrockYaml from "../../lib/bedrockYaml"; + +beforeAll(() => { + enableVerboseLogging(); +}); + +afterAll(() => { + disableVerboseLogging(); +}); + +describe("get display name", () => { + it("positive test", async () => { + const exitFn = jest.fn(); + execute({}, exitFn); + expect(exitFn).toBeCalledTimes(1); + const defaultBedrockFileObject = createTestBedrockYaml( + false + ) as BedrockFile; + jest.spyOn(fs, "existsSync").mockReturnValue(true); + jest.spyOn(bedrockYaml, "read").mockReturnValue(defaultBedrockFileObject); + jest.spyOn(process, "cwd").mockReturnValue("bedrock.yaml/"); + execute({}, exitFn); + expect(exitFn).toBeCalledTimes(2); + }); +}); diff --git a/src/commands/project/get-display-name.ts b/src/commands/project/get-display-name.ts index c1fc7b53b..17ef2a6a2 100644 --- a/src/commands/project/get-display-name.ts +++ b/src/commands/project/get-display-name.ts @@ -24,6 +24,9 @@ export const execute = async ( // one directory up. while (!fs.existsSync(currentDirectory)) { const split = currentDirectory.split("/"); + if (split.length < 2) { + break; + } currentDirectory = currentDirectory.replace( split[split.length - 2] + "/", "" From f32f58cb728e80f91a9eaafb1edd51ecca8a2594 Mon Sep 17 00:00:00 2001 From: Samiya Akhtar Date: Fri, 3 Apr 2020 17:12:27 -0700 Subject: [PATCH 05/14] Fixing generated yaml file --- src/lib/fileutils.ts | 8 ++++---- src/test/mockFactory.ts | 8 ++++---- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/lib/fileutils.ts b/src/lib/fileutils.ts index a3e725fec..42a3b33dc 100644 --- a/src/lib/fileutils.ts +++ b/src/lib/fileutils.ts @@ -195,19 +195,19 @@ export const serviceBuildAndUpdatePipeline = ( }, { script: generateYamlScript([ + `. ./build.sh --source-only`, + `get_spk_version`, + `download_spk`, `export BUILD_REPO_NAME=${BUILD_REPO_NAME(serviceName)}`, `tag_name="$BUILD_REPO_NAME:${IMAGE_TAG}"`, `commitId=$(Build.SourceVersion)`, `commitId=$(echo "\${commitId:0:7}")`, `currentDirectory=$(pwd)`, `cd ${relativeServiceForDockerfile}`, - `service=$(spk project get-display-name)`, + `service=$($currentDirectory/spk/spk project get-display-name)`, `cd $currentDirectory`, `url=$(git remote --verbose | grep origin | grep fetch | cut -f2 | cut -d' ' -f1)`, `repourl=\${url##*@}`, - `. ./build.sh --source-only`, - `get_spk_version`, - `download_spk`, `./spk/spk deployment create -n $(INTROSPECTION_ACCOUNT_NAME) -k $(INTROSPECTION_ACCOUNT_KEY) -t $(INTROSPECTION_TABLE_NAME) -p $(INTROSPECTION_PARTITION_KEY) --p1 $(Build.BuildId) --image-tag $tag_name --commit-id $commitId --service $service --repository $repourl`, ]), displayName: diff --git a/src/test/mockFactory.ts b/src/test/mockFactory.ts index b08fe834e..995aa2f9c 100644 --- a/src/test/mockFactory.ts +++ b/src/test/mockFactory.ts @@ -75,19 +75,19 @@ export const createTestServiceBuildAndUpdatePipelineYaml = ( }, { script: generateYamlScript([ + `. ./build.sh --source-only`, + `get_spk_version`, + `download_spk`, `export BUILD_REPO_NAME=${BUILD_REPO_NAME(serviceName)}`, `tag_name="$BUILD_REPO_NAME:${IMAGE_TAG}"`, `commitId=$(Build.SourceVersion)`, `commitId=$(echo "\${commitId:0:7}")`, `currentDirectory=$(pwd)`, `cd ${relativeServiceForDockerfile}`, - `service=$(spk project get-display-name)`, + `service=$($currentDirectory/spk/spk project get-display-name)`, `cd $currentDirectory`, `url=$(git remote --verbose | grep origin | grep fetch | cut -f2 | cut -d' ' -f1)`, `repourl=\${url##*@}`, - `. ./build.sh --source-only`, - `get_spk_version`, - `download_spk`, `./spk/spk deployment create -n $(INTROSPECTION_ACCOUNT_NAME) -k $(INTROSPECTION_ACCOUNT_KEY) -t $(INTROSPECTION_TABLE_NAME) -p $(INTROSPECTION_PARTITION_KEY) --p1 $(Build.BuildId) --image-tag $tag_name --commit-id $commitId --service $service --repository $repourl`, ]), displayName: From 7238fb87a913ecad7ee32c035a698073d1277fab Mon Sep 17 00:00:00 2001 From: Samiya Akhtar Date: Mon, 6 Apr 2020 13:11:00 -0700 Subject: [PATCH 06/14] Removing the changes that will come after spk release is ready --- docs/commands/data.json | 6 +++--- src/lib/fileutils.ts | 12 +++++------- src/test/mockFactory.ts | 25 ++++++++++--------------- 3 files changed, 18 insertions(+), 25 deletions(-) diff --git a/docs/commands/data.json b/docs/commands/data.json index 89457fa4a..655cca5c0 100644 --- a/docs/commands/data.json +++ b/docs/commands/data.json @@ -502,19 +502,19 @@ "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.", - "markdown": "## Description\n\nSPK 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 ./:\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 `spk 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 ./:\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" + "markdown": "## Description\n\nSPK 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 `spk 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": { "command": "delete ", "alias": "d", "description": "Delete a ring from the current working directory project repository. This will affect all services within the project repository. The default ring cannot be deleted.", - "markdown": "## Description\n\nSPK command to remove a ring from an initialized bedrock project.\n\n_Note:_ A default ring cannot be removed. First set another ring as the default\nvia `spk ring set-default` before deleting.\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 ./:\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 `spk ring delete prod` will result in a few changes:\n\n1. `prod` will be removed from `bedrock.yaml`:\n ```yaml\n rings:\n dev:\n isDefault: true\n qa:\n services:\n ./:\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 remove the ring, `prod` in their branch\n triggers:\n\n ```yaml\n trigger:\n branches:\n include:\n - dev\n - qa\n # - prod <-- THIS WILL BE DELETED! -->\n variables:\n - group: fabrikam-vg\n …\n ```\n\n3. Commiting these changes will trigger the project's lifecycle pipeline, which\n will then remove the ring from linked services in this project,\n [pending this epic](https://github.com/microsoft/bedrock/issues/858).\n" + "markdown": "## Description\n\nSPK command to remove a ring from an initialized bedrock project.\n\n_Note:_ A default ring cannot be removed. First set another ring as the default\nvia `spk ring set-default` before deleting.\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 `spk ring delete prod` will result in a few changes:\n\n1. `prod` will be removed from `bedrock.yaml`:\n ```yaml\n rings:\n dev:\n isDefault: true\n qa:\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 remove the ring, `prod` in their branch\n triggers:\n\n ```yaml\n trigger:\n branches:\n include:\n - dev\n - qa\n # - prod <-- THIS WILL BE DELETED! -->\n variables:\n - group: fabrikam-vg\n …\n ```\n\n3. Commiting these changes will trigger the project's lifecycle pipeline, which\n will then remove the ring from linked services in this project,\n [pending this epic](https://github.com/microsoft/bedrock/issues/858).\n" }, "ring set-default": { "command": "set-default ", "alias": "s", "description": "Set a ring as the default for the current working directory project repository. This will affect all services within the project repository.", - "markdown": "## Description\n\nSPK command to set a ring as the default for 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 ./:\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 `spk ring set-default prod` will result in:\n\n1. `prod` will be set as the default in `bedrock.yaml`:\n\n ```yaml\n rings:\n dev:\n qa:\n prod:\n isDefault: true\n services:\n ./:\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 ```\n\n2. Commiting these changes will trigger the project's lifecycle pipeline, which\n may update the default \"no-ring\" IngressRoutes to route to the `prod` service\n in the linked HLD repository,\n [pending the work in this issue](https://github.com/microsoft/bedrock/issues/1084).\n" + "markdown": "## Description\n\nSPK command to set a ring as the default for 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 `spk ring set-default prod` will result in:\n\n1. `prod` will be set as the default in `bedrock.yaml`:\n\n ```yaml\n rings:\n dev:\n qa:\n prod:\n isDefault: true\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 ```\n\n2. Commiting these changes will trigger the project's lifecycle pipeline, which\n may update the default \"no-ring\" IngressRoutes to route to the `prod` service\n in the linked HLD repository,\n [pending the work in this issue](https://github.com/microsoft/bedrock/issues/1084).\n" }, "service create-revision": { "command": "create-revision", diff --git a/src/lib/fileutils.ts b/src/lib/fileutils.ts index 42a3b33dc..326124042 100644 --- a/src/lib/fileutils.ts +++ b/src/lib/fileutils.ts @@ -195,19 +195,17 @@ export const serviceBuildAndUpdatePipeline = ( }, { script: generateYamlScript([ - `. ./build.sh --source-only`, - `get_spk_version`, - `download_spk`, `export BUILD_REPO_NAME=${BUILD_REPO_NAME(serviceName)}`, `tag_name="$BUILD_REPO_NAME:${IMAGE_TAG}"`, `commitId=$(Build.SourceVersion)`, `commitId=$(echo "\${commitId:0:7}")`, - `currentDirectory=$(pwd)`, - `cd ${relativeServiceForDockerfile}`, - `service=$($currentDirectory/spk/spk project get-display-name)`, - `cd $currentDirectory`, + `service=$(Build.Repository.Name)`, + `service=\${service##*/}`, `url=$(git remote --verbose | grep origin | grep fetch | cut -f2 | cut -d' ' -f1)`, `repourl=\${url##*@}`, + `. ./build.sh --source-only`, + `get_spk_version`, + `download_spk`, `./spk/spk deployment create -n $(INTROSPECTION_ACCOUNT_NAME) -k $(INTROSPECTION_ACCOUNT_KEY) -t $(INTROSPECTION_TABLE_NAME) -p $(INTROSPECTION_PARTITION_KEY) --p1 $(Build.BuildId) --image-tag $tag_name --commit-id $commitId --service $service --repository $repourl`, ]), displayName: diff --git a/src/test/mockFactory.ts b/src/test/mockFactory.ts index 995aa2f9c..aa65d73e2 100644 --- a/src/test/mockFactory.ts +++ b/src/test/mockFactory.ts @@ -75,19 +75,17 @@ export const createTestServiceBuildAndUpdatePipelineYaml = ( }, { script: generateYamlScript([ - `. ./build.sh --source-only`, - `get_spk_version`, - `download_spk`, `export BUILD_REPO_NAME=${BUILD_REPO_NAME(serviceName)}`, `tag_name="$BUILD_REPO_NAME:${IMAGE_TAG}"`, `commitId=$(Build.SourceVersion)`, `commitId=$(echo "\${commitId:0:7}")`, - `currentDirectory=$(pwd)`, - `cd ${relativeServiceForDockerfile}`, - `service=$($currentDirectory/spk/spk project get-display-name)`, - `cd $currentDirectory`, + `service=$(Build.Repository.Name)`, + `service=\${service##*/}`, `url=$(git remote --verbose | grep origin | grep fetch | cut -f2 | cut -d' ' -f1)`, `repourl=\${url##*@}`, + `. ./build.sh --source-only`, + `get_spk_version`, + `download_spk`, `./spk/spk deployment create -n $(INTROSPECTION_ACCOUNT_NAME) -k $(INTROSPECTION_ACCOUNT_KEY) -t $(INTROSPECTION_TABLE_NAME) -p $(INTROSPECTION_PARTITION_KEY) --p1 $(Build.BuildId) --image-tag $tag_name --commit-id $commitId --service $service --repository $repourl`, ]), displayName: @@ -288,23 +286,20 @@ export const createTestBedrockYaml = ( qa: {}, testing: {}, }, - services: [ - { - path: "./", + services: { + "./": { helm: service1HelmConfig, k8sBackendPort: 80, }, - { - path: "./packages/service1", + "./packages/service1": { helm: service2HelmConfig, k8sBackendPort: 80, }, - { - path: "./zookeeper", + "./zookeeper": { helm: zookeeperHelmConfig, k8sBackendPort: 80, }, - ], + }, variableGroups: [], version: "1.0", }; From 2db61ce6257bce30e233a14385ad12fb1a5ccdae Mon Sep 17 00:00:00 2001 From: Samiya Akhtar Date: Mon, 6 Apr 2020 13:13:32 -0700 Subject: [PATCH 07/14] Updating files --- src/test/mockFactory.ts | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/test/mockFactory.ts b/src/test/mockFactory.ts index aa65d73e2..eff9c355b 100644 --- a/src/test/mockFactory.ts +++ b/src/test/mockFactory.ts @@ -286,20 +286,23 @@ export const createTestBedrockYaml = ( qa: {}, testing: {}, }, - services: { - "./": { + services: [ + { + path: "./", helm: service1HelmConfig, k8sBackendPort: 80, }, - "./packages/service1": { + { + path: "./packages/service1", helm: service2HelmConfig, k8sBackendPort: 80, }, - "./zookeeper": { + { + path: "./zookeeper", helm: zookeeperHelmConfig, k8sBackendPort: 80, }, - }, + ], variableGroups: [], version: "1.0", }; From 7b8346c8a20b24b27c5716edd92cda3ebd117478 Mon Sep 17 00:00:00 2001 From: Samiya Akhtar Date: Tue, 7 Apr 2020 09:29:06 -0700 Subject: [PATCH 08/14] Command should be run from bedrock/yaml directory and move under --- docs/commands/data.json | 18 +++++--- .../project/get-display-name.decorator.json | 6 --- src/commands/project/index.ts | 7 +-- .../service/get-display-name.decorator.json | 12 +++++ .../get-display-name.test.ts | 8 ++-- .../{project => service}/get-display-name.ts | 44 +++++++++++-------- src/commands/service/index.ts | 7 ++- src/lib/i18n.json | 3 ++ 8 files changed, 65 insertions(+), 40 deletions(-) delete mode 100644 src/commands/project/get-display-name.decorator.json create mode 100644 src/commands/service/get-display-name.decorator.json rename src/commands/{project => service}/get-display-name.test.ts (83%) rename src/commands/{project => service}/get-display-name.ts (58%) diff --git a/docs/commands/data.json b/docs/commands/data.json index f0dcb1c8a..70d40e223 100644 --- a/docs/commands/data.json +++ b/docs/commands/data.json @@ -434,12 +434,6 @@ ], "markdown": "## Description\n\nCreate new variable group in Azure DevOps project\n\n## Command Prerequisites\n\nIn addition to an existing\n[Azure DevOps project](https://azure.microsoft.com/en-us/services/devops/), to\nlink secrets from an Azure key vault as variables in Variable Group, you will\nneed an existing key vault containing your secrets and the Service Principal for\nauthorization with Azure Key Vault.\n\n1. Use existng or\n [create a service principal either in Azure Portal](https://docs.microsoft.com/en-us/azure/active-directory/develop/howto-create-service-principal-portal)\n or\n [with Azure CLI](https://docs.microsoft.com/en-us/cli/azure/create-an-azure-service-principal-azure-cli?view=azure-cli-latest).\n2. Use existing or\n [create a Azure Container Registry in Azure Portal](https://docs.microsoft.com/en-us/azure/container-registry/container-registry-get-started-portal)\n or\n [with Azure CLI](https://docs.microsoft.com/en-us/azure/container-registry/container-registry-get-started-azure-cli).\n" }, - "project get-display-name": { - "command": "get-display-name", - "alias": "gdn", - "description": "Gets display name for a service", - "options": [] - }, "project init": { "command": "init", "alias": "i", @@ -644,6 +638,18 @@ ], "markdown": "## Description\n\nAdd a new service into this initialized spk project repository.\n\n## Example\n\n```bash\nspk service create my-service . \\\n --display-name $app_name \\\n --helm-config-path $path_to_chart_in_repo \\\n --helm-config-git $helm_repo_url \\ # Needs to start with https and not contain user name\n --helm-config-branch master \\\n --helm-chart-access-token-variable $ENV_VAR_NAME\n```\n\n## Note\n\n- `--helm-chart-*` and `--helm-config-*` settings are mutually-exclusive. **You\n may only use one.**\n - If the git repository referenced in `--helm-config-git` is a private\n repository, you can specify an environment variable in your\n HLD-to-Materialized pipeline containing your a PAT to authenticate with via\n the `--helm-chart-access-token-variable` option.\n- `--middlewares`, `--k8s-backend-port`, `--path-prefix`,\n `--path-prefix-major-version`, and `--k8s-backend` are all used to configure\n the generated Traefik2 IngressRoutes. i.e.\n\n ```sh\n spk service create my-example-documents-service path/to/my/service \\\n --middlewares middleware \\\n --k8s-backend-port 3001 \\\n --k8s-backend docs-service \\\n --path-prefix documents \\\n --path-prefix-major-version v2\n ```\n\n will result in an IngressRoute that looks like:\n\n ```yaml\n apiVersion: traefik.containo.us/v1alpha1\n kind: IngressRoute\n metadata:\n name: my-example-documents-service-master\n spec:\n routes:\n - kind: Rule\n match: \"PathPrefix(`/v2/documents`) && Headers(`Ring`, `master`)\"\n middlewares:\n - name: my-example-documents-service-master\n - name: middlewareA\n services:\n - name: docs-service\n port: 3001\n ```\n" }, + "service get-display-name": { + "command": "get-display-name", + "alias": "gdn", + "description": "Gets display name for a service", + "options": [ + { + "arg": "-p, --path ", + "description": "Path to the service folder for which display name is to be extracted", + "required": true + } + ] + }, "service install-build-pipeline": { "command": "install-build-pipeline ", "alias": "p", diff --git a/src/commands/project/get-display-name.decorator.json b/src/commands/project/get-display-name.decorator.json deleted file mode 100644 index 84a980820..000000000 --- a/src/commands/project/get-display-name.decorator.json +++ /dev/null @@ -1,6 +0,0 @@ -{ - "command": "get-display-name", - "alias": "gdn", - "description": "Gets display name for a service", - "options": [] -} diff --git a/src/commands/project/index.ts b/src/commands/project/index.ts index 1d0771c60..e624fd278 100644 --- a/src/commands/project/index.ts +++ b/src/commands/project/index.ts @@ -1,11 +1,6 @@ import { Command } from "../command"; -const subfolders = [ - "create-variable-group", - "init", - "pipeline", - "get-display-name", -]; +const subfolders = ["create-variable-group", "init", "pipeline"]; export const commandDecorator = Command( "project", diff --git a/src/commands/service/get-display-name.decorator.json b/src/commands/service/get-display-name.decorator.json new file mode 100644 index 000000000..cfa735585 --- /dev/null +++ b/src/commands/service/get-display-name.decorator.json @@ -0,0 +1,12 @@ +{ + "command": "get-display-name", + "alias": "gdn", + "description": "Gets display name for a service", + "options": [ + { + "arg": "-p, --path ", + "description": "Path to the service folder for which display name is to be extracted", + "required": true + } + ] +} diff --git a/src/commands/project/get-display-name.test.ts b/src/commands/service/get-display-name.test.ts similarity index 83% rename from src/commands/project/get-display-name.test.ts rename to src/commands/service/get-display-name.test.ts index 6c44cfaa4..59cd722c3 100644 --- a/src/commands/project/get-display-name.test.ts +++ b/src/commands/service/get-display-name.test.ts @@ -20,15 +20,17 @@ afterAll(() => { describe("get display name", () => { it("positive test", async () => { const exitFn = jest.fn(); - execute({}, exitFn); + execute({ path: "test" }, exitFn); expect(exitFn).toBeCalledTimes(1); + execute({ path: undefined }, exitFn); + expect(exitFn).toBeCalledTimes(2); const defaultBedrockFileObject = createTestBedrockYaml( false ) as BedrockFile; jest.spyOn(fs, "existsSync").mockReturnValue(true); jest.spyOn(bedrockYaml, "read").mockReturnValue(defaultBedrockFileObject); jest.spyOn(process, "cwd").mockReturnValue("bedrock.yaml/"); - execute({}, exitFn); - expect(exitFn).toBeCalledTimes(2); + execute({ path: "./packages/service1" }, exitFn); + expect(exitFn).toBeCalledTimes(3); }); }); diff --git a/src/commands/project/get-display-name.ts b/src/commands/service/get-display-name.ts similarity index 58% rename from src/commands/project/get-display-name.ts rename to src/commands/service/get-display-name.ts index 17ef2a6a2..22c974b58 100644 --- a/src/commands/project/get-display-name.ts +++ b/src/commands/service/get-display-name.ts @@ -1,11 +1,17 @@ /* eslint-disable @typescript-eslint/no-use-before-define */ /* eslint-disable @typescript-eslint/camelcase */ -import commander, { CommandOptions } from "commander"; +import commander from "commander"; import * as fs from "fs"; import { read as readBedrockYaml } from "../../lib/bedrockYaml"; import { build as buildCmd, exit as exitCmd } from "../../lib/commandBuilder"; import { logger } from "../../logger"; import decorator from "./get-display-name.decorator.json"; +import { build as buildError, log as logError } from "../../lib/errorBuilder"; +import { errorStatusCode } from "../../lib/errorStatusCode"; + +export interface CommandOptions { + path: string | undefined; +} /** * Executes the command, can all exit function with 0 or 1 @@ -18,31 +24,33 @@ export const execute = async ( opts: CommandOptions, exitFn: (status: number) => Promise ): Promise => { - let currentDirectory = process.cwd() + "/bedrock.yaml"; + // The assumption is that this command should be ran from the directory where bedrock.yaml exists try { - // First, find a bedrock.yaml file in this directory and if not, iterate by - // one directory up. - while (!fs.existsSync(currentDirectory)) { - const split = currentDirectory.split("/"); - if (split.length < 2) { - break; - } - currentDirectory = currentDirectory.replace( - split[split.length - 2] + "/", - "" + if (!opts.path) { + throw buildError( + errorStatusCode.EXE_FLOW_ERR, + "service-get-display-name-path-missing-param-err" + ); + } + const bedrockFile = readBedrockYaml(process.cwd()); + if (!bedrockFile) { + throw buildError( + errorStatusCode.CMD_EXE_ERR, + "service-get-display-name-bedrock-yaml-missing-err" ); } - currentDirectory = currentDirectory.replace("/bedrock.yaml", ""); - const bedrockFile = readBedrockYaml(currentDirectory); - const servicePath = process.cwd().replace(currentDirectory, "."); + logger.info(JSON.stringify(bedrockFile, null, 4)); for (const serviceIndex in bedrockFile.services) { - if (servicePath === bedrockFile.services[serviceIndex].path) { + if (opts.path === bedrockFile.services[serviceIndex].path) { console.log(bedrockFile.services[serviceIndex].displayName); await exitFn(0); } } - // Ideally, we should not be getting here, so throw an error. - throw new Error("Display name for current directory could not be found."); + + throw buildError(errorStatusCode.CMD_EXE_ERR, { + errorKey: "service-get-display-name-err", + values: [opts.path], + }); } catch (err) { logger.error(err); await exitFn(1); diff --git a/src/commands/service/index.ts b/src/commands/service/index.ts index 226b44536..3d3de297d 100644 --- a/src/commands/service/index.ts +++ b/src/commands/service/index.ts @@ -1,6 +1,11 @@ import { Command } from "../command"; -const subfolders = ["create", "create-revision", "pipeline"]; +const subfolders = [ + "create", + "create-revision", + "pipeline", + "get-display-name", +]; export const commandDecorator = Command( "service", diff --git a/src/lib/i18n.json b/src/lib/i18n.json index fa25f8ed4..cb5fc6f4e 100644 --- a/src/lib/i18n.json +++ b/src/lib/i18n.json @@ -142,6 +142,9 @@ "introspect-dashboard-cmd-launch-pre-req-err": "Requirements to launch dashboard were not met.", "introspect-dashboard-cmd-launch-err": "Could not launch dashboard docker container.", + "service-get-display-name-path-missing-param-err": "Value for path parameter was missing. This is required for running get-display-command. Provide it.", + "service-get-display-name-bedrock-yaml-missing-err": "Could not find bedrock.yaml in current directory. Make sure to run this from a directory that contains bedrock.yaml.", + "service-get-display-name-err": "Could not find a service for path {0}. Make sure that the specified path is valid.", "az-cli-login-err": "Could not login through azure cli.", "az-cli-create-sp-err": "Could not create service principal with azure cli", From 2a65a575132ab48b1077abd0d44f1d7803178959 Mon Sep 17 00:00:00 2001 From: Samiya Akhtar Date: Tue, 7 Apr 2020 09:32:58 -0700 Subject: [PATCH 09/14] Minor changes --- src/commands/service/get-display-name.test.ts | 6 +----- src/commands/service/get-display-name.ts | 8 ++------ 2 files changed, 3 insertions(+), 11 deletions(-) diff --git a/src/commands/service/get-display-name.test.ts b/src/commands/service/get-display-name.test.ts index 59cd722c3..014287052 100644 --- a/src/commands/service/get-display-name.test.ts +++ b/src/commands/service/get-display-name.test.ts @@ -1,8 +1,4 @@ -import { - disableVerboseLogging, - enableVerboseLogging, - logger, -} from "../../logger"; +import { disableVerboseLogging, enableVerboseLogging } from "../../logger"; import { execute } from "./get-display-name"; import * as fs from "fs"; import { createTestBedrockYaml } from "../../test/mockFactory"; diff --git a/src/commands/service/get-display-name.ts b/src/commands/service/get-display-name.ts index 22c974b58..57f037123 100644 --- a/src/commands/service/get-display-name.ts +++ b/src/commands/service/get-display-name.ts @@ -1,12 +1,9 @@ -/* eslint-disable @typescript-eslint/no-use-before-define */ -/* eslint-disable @typescript-eslint/camelcase */ import commander from "commander"; -import * as fs from "fs"; import { read as readBedrockYaml } from "../../lib/bedrockYaml"; import { build as buildCmd, exit as exitCmd } from "../../lib/commandBuilder"; import { logger } from "../../logger"; import decorator from "./get-display-name.decorator.json"; -import { build as buildError, log as logError } from "../../lib/errorBuilder"; +import { build as buildError } from "../../lib/errorBuilder"; import { errorStatusCode } from "../../lib/errorStatusCode"; export interface CommandOptions { @@ -39,7 +36,6 @@ export const execute = async ( "service-get-display-name-bedrock-yaml-missing-err" ); } - logger.info(JSON.stringify(bedrockFile, null, 4)); for (const serviceIndex in bedrockFile.services) { if (opts.path === bedrockFile.services[serviceIndex].path) { console.log(bedrockFile.services[serviceIndex].displayName); @@ -58,7 +54,7 @@ export const execute = async ( }; /** - * Adds the validate command to the commander command object + * Adds the get-display-name command to the commander command object * @param command Commander command object to decorate */ export const commandDecorator = (command: commander.Command): void => { From 64e4bcca6e3e26a4d6b783bb0fb5c3038a45b5fc Mon Sep 17 00:00:00 2001 From: Samiya Akhtar Date: Tue, 7 Apr 2020 09:37:49 -0700 Subject: [PATCH 10/14] Use error chain --- src/commands/service/get-display-name.ts | 12 +++++++++--- src/lib/i18n.json | 1 + 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/src/commands/service/get-display-name.ts b/src/commands/service/get-display-name.ts index 57f037123..0123027c1 100644 --- a/src/commands/service/get-display-name.ts +++ b/src/commands/service/get-display-name.ts @@ -3,7 +3,7 @@ import { read as readBedrockYaml } from "../../lib/bedrockYaml"; import { build as buildCmd, exit as exitCmd } from "../../lib/commandBuilder"; import { logger } from "../../logger"; import decorator from "./get-display-name.decorator.json"; -import { build as buildError } from "../../lib/errorBuilder"; +import { build as buildError, log as logError } from "../../lib/errorBuilder"; import { errorStatusCode } from "../../lib/errorStatusCode"; export interface CommandOptions { @@ -25,7 +25,7 @@ export const execute = async ( try { if (!opts.path) { throw buildError( - errorStatusCode.EXE_FLOW_ERR, + errorStatusCode.CMD_EXE_ERR, "service-get-display-name-path-missing-param-err" ); } @@ -48,7 +48,13 @@ export const execute = async ( values: [opts.path], }); } catch (err) { - logger.error(err); + logError( + buildError( + errorStatusCode.CMD_EXE_ERR, + "service-get-display-name-generic-err", + err + ) + ); await exitFn(1); } }; diff --git a/src/lib/i18n.json b/src/lib/i18n.json index cb5fc6f4e..5e46d977b 100644 --- a/src/lib/i18n.json +++ b/src/lib/i18n.json @@ -145,6 +145,7 @@ "service-get-display-name-path-missing-param-err": "Value for path parameter was missing. This is required for running get-display-command. Provide it.", "service-get-display-name-bedrock-yaml-missing-err": "Could not find bedrock.yaml in current directory. Make sure to run this from a directory that contains bedrock.yaml.", "service-get-display-name-err": "Could not find a service for path {0}. Make sure that the specified path is valid.", + "service-get-display-name-generic-err": "Error occurred while getting display name.", "az-cli-login-err": "Could not login through azure cli.", "az-cli-create-sp-err": "Could not create service principal with azure cli", From c319ce75c12bf553d9f2d0b4dd0faa7cbd59917c Mon Sep 17 00:00:00 2001 From: Samiya Akhtar Date: Tue, 7 Apr 2020 11:10:55 -0700 Subject: [PATCH 11/14] More feedback --- docs/commands/data.json | 3 ++- src/commands/service/get-display-name.md | 4 ++++ src/commands/service/get-display-name.ts | 8 ++++---- 3 files changed, 10 insertions(+), 5 deletions(-) create mode 100644 src/commands/service/get-display-name.md diff --git a/docs/commands/data.json b/docs/commands/data.json index e49cb5228..63424f6ae 100644 --- a/docs/commands/data.json +++ b/docs/commands/data.json @@ -649,7 +649,8 @@ "description": "Path to the service folder for which display name is to be extracted", "required": true } - ] + ], + "markdown": "## Description\n\nGets display name of a service based on the provided path by extracting this\ninformation from bedrrock.yaml\n" }, "service install-build-pipeline": { "command": "install-build-pipeline ", diff --git a/src/commands/service/get-display-name.md b/src/commands/service/get-display-name.md new file mode 100644 index 000000000..095f6edfa --- /dev/null +++ b/src/commands/service/get-display-name.md @@ -0,0 +1,4 @@ +## Description + +Gets display name of a service based on the provided path by extracting this +information from bedrrock.yaml diff --git a/src/commands/service/get-display-name.ts b/src/commands/service/get-display-name.ts index 0123027c1..3ab87865a 100644 --- a/src/commands/service/get-display-name.ts +++ b/src/commands/service/get-display-name.ts @@ -25,14 +25,14 @@ export const execute = async ( try { if (!opts.path) { throw buildError( - errorStatusCode.CMD_EXE_ERR, + errorStatusCode.VALIDATION_ERR, "service-get-display-name-path-missing-param-err" ); } const bedrockFile = readBedrockYaml(process.cwd()); if (!bedrockFile) { throw buildError( - errorStatusCode.CMD_EXE_ERR, + errorStatusCode.FILE_IO_ERR, "service-get-display-name-bedrock-yaml-missing-err" ); } @@ -43,14 +43,14 @@ export const execute = async ( } } - throw buildError(errorStatusCode.CMD_EXE_ERR, { + throw buildError(errorStatusCode.ENV_SETTING_ERR, { errorKey: "service-get-display-name-err", values: [opts.path], }); } catch (err) { logError( buildError( - errorStatusCode.CMD_EXE_ERR, + errorStatusCode.VALIDATION_ERR, "service-get-display-name-generic-err", err ) From 5792cd33bd3fcc61b8b6e40b173aaa6ca6811f87 Mon Sep 17 00:00:00 2001 From: Samiya Akhtar Date: Tue, 7 Apr 2020 12:44:41 -0700 Subject: [PATCH 12/14] Improve doc --- docs/commands/data.json | 2 +- src/commands/service/get-display-name.md | 10 +++++++++- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/docs/commands/data.json b/docs/commands/data.json index 63424f6ae..d85b22f7a 100644 --- a/docs/commands/data.json +++ b/docs/commands/data.json @@ -650,7 +650,7 @@ "required": true } ], - "markdown": "## Description\n\nGets display name of a service based on the provided path by extracting this\ninformation from bedrrock.yaml\n" + "markdown": "## Description\n\nGets display name of a service based on the provided path by extracting this\ninformation from bedrock.yaml. This command tries to locate bedrock.yaml in the\ncurrent directory.\n\nIf bedrock.yaml is not found in current directory, the command will fail to\nextract display name.\n\nIf the specified path is not found in any services listed in bedrock.yaml, the\ndisplay name will not be extracted. Make sure that specified path matches the\npath in bedrock.yaml exactly.\n" }, "service install-build-pipeline": { "command": "install-build-pipeline ", diff --git a/src/commands/service/get-display-name.md b/src/commands/service/get-display-name.md index 095f6edfa..eeaeae0fc 100644 --- a/src/commands/service/get-display-name.md +++ b/src/commands/service/get-display-name.md @@ -1,4 +1,12 @@ ## Description Gets display name of a service based on the provided path by extracting this -information from bedrrock.yaml +information from bedrock.yaml. This command tries to locate bedrock.yaml in the +current directory. + +If bedrock.yaml is not found in current directory, the command will fail to +extract display name. + +If the specified path is not found in any services listed in bedrock.yaml, the +display name will not be extracted. Make sure that specified path matches the +path in bedrock.yaml exactly. From 2b6eb4577d028e03d233800ae370af2b4da522da Mon Sep 17 00:00:00 2001 From: Samiya Akhtar Date: Tue, 7 Apr 2020 13:45:22 -0700 Subject: [PATCH 13/14] Improve unit test --- src/commands/service/get-display-name.test.ts | 2 ++ src/commands/service/get-display-name.ts | 13 ++++++++----- src/test/mockFactory.ts | 3 +++ 3 files changed, 13 insertions(+), 5 deletions(-) diff --git a/src/commands/service/get-display-name.test.ts b/src/commands/service/get-display-name.test.ts index 014287052..3c192b8d4 100644 --- a/src/commands/service/get-display-name.test.ts +++ b/src/commands/service/get-display-name.test.ts @@ -26,7 +26,9 @@ describe("get display name", () => { jest.spyOn(fs, "existsSync").mockReturnValue(true); jest.spyOn(bedrockYaml, "read").mockReturnValue(defaultBedrockFileObject); jest.spyOn(process, "cwd").mockReturnValue("bedrock.yaml/"); + const consoleSpy = jest.spyOn(console, "log"); execute({ path: "./packages/service1" }, exitFn); + expect(consoleSpy).toHaveBeenCalledWith("service1"); expect(exitFn).toBeCalledTimes(3); }); }); diff --git a/src/commands/service/get-display-name.ts b/src/commands/service/get-display-name.ts index 3ab87865a..3ae8645bb 100644 --- a/src/commands/service/get-display-name.ts +++ b/src/commands/service/get-display-name.ts @@ -36,11 +36,14 @@ export const execute = async ( "service-get-display-name-bedrock-yaml-missing-err" ); } - for (const serviceIndex in bedrockFile.services) { - if (opts.path === bedrockFile.services[serviceIndex].path) { - console.log(bedrockFile.services[serviceIndex].displayName); - await exitFn(0); - } + + const serviceIndex = Object.keys(bedrockFile.services).find( + (index) => opts.path === bedrockFile.services[+index].path + ); + + if (serviceIndex) { + console.log(bedrockFile.services[+serviceIndex].displayName); + await exitFn(0); } throw buildError(errorStatusCode.ENV_SETTING_ERR, { diff --git a/src/test/mockFactory.ts b/src/test/mockFactory.ts index 6469a4602..aa0a883ab 100644 --- a/src/test/mockFactory.ts +++ b/src/test/mockFactory.ts @@ -291,16 +291,19 @@ export const createTestBedrockYaml = ( path: "./", helm: service1HelmConfig, k8sBackendPort: 80, + displayName: "root", }, { path: "./packages/service1", helm: service2HelmConfig, k8sBackendPort: 80, + displayName: "service1", }, { path: "./zookeeper", helm: zookeeperHelmConfig, k8sBackendPort: 80, + displayName: "zookeeper", }, ], variableGroups: [], From 253ae29d5751a94c465a177d97db2b8bf91d53e5 Mon Sep 17 00:00:00 2001 From: Samiya Akhtar Date: Wed, 8 Apr 2020 13:43:08 -0700 Subject: [PATCH 14/14] Separated out negative tests --- src/commands/service/get-display-name.test.ts | 21 +++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/src/commands/service/get-display-name.test.ts b/src/commands/service/get-display-name.test.ts index 3c192b8d4..8bb471a59 100644 --- a/src/commands/service/get-display-name.test.ts +++ b/src/commands/service/get-display-name.test.ts @@ -16,10 +16,6 @@ afterAll(() => { describe("get display name", () => { it("positive test", async () => { const exitFn = jest.fn(); - execute({ path: "test" }, exitFn); - expect(exitFn).toBeCalledTimes(1); - execute({ path: undefined }, exitFn); - expect(exitFn).toBeCalledTimes(2); const defaultBedrockFileObject = createTestBedrockYaml( false ) as BedrockFile; @@ -29,6 +25,23 @@ describe("get display name", () => { const consoleSpy = jest.spyOn(console, "log"); execute({ path: "./packages/service1" }, exitFn); expect(consoleSpy).toHaveBeenCalledWith("service1"); + expect(exitFn).toBeCalledTimes(1); + expect(exitFn).toBeCalledWith(0); + }); + it("negative test", async () => { + const exitFn = jest.fn(); + execute({ path: "" }, exitFn); + expect(exitFn).toBeCalledTimes(1); + execute({ path: undefined }, exitFn); + expect(exitFn).toBeCalledWith(1); + const defaultBedrockFileObject = createTestBedrockYaml( + false + ) as BedrockFile; + jest.spyOn(fs, "existsSync").mockReturnValue(true); + jest.spyOn(bedrockYaml, "read").mockReturnValue(defaultBedrockFileObject); + jest.spyOn(process, "cwd").mockReturnValue("bedrock.yaml/"); + execute({ path: "./packages/service" }, exitFn); // should not exist expect(exitFn).toBeCalledTimes(3); + expect(exitFn).toBeCalledWith(1); }); });