From 71b3781cb347df99be2b1f8bdb0143660dfebd43 Mon Sep 17 00:00:00 2001 From: Vladimir Vagaytsev Date: Tue, 14 Nov 2023 12:08:06 +0100 Subject: [PATCH] fix(sync-mode): use the same source path schemas for all action types (#5363) * refactor(container): extract named function for sync source path schema * fix(sync-mode): use the same source path schemas for all action types * docs(reference): re-generated docs * test(windows): always test sync mode config validation on Windows * docs: update source path description --- .circleci/continue-config.yml | 18 ++++++++++- core/src/plugins/container/config.ts | 32 +++++++++++++------ core/src/plugins/kubernetes/sync.ts | 15 ++------- .../action-types/Deploy/container.md | 5 ++- docs/reference/action-types/Deploy/helm.md | 22 ++++++++++--- .../action-types/Deploy/kubernetes.md | 22 ++++++++++--- docs/reference/module-types/container.md | 14 ++++++-- docs/reference/module-types/helm.md | 14 ++++++-- docs/reference/module-types/jib-container.md | 14 ++++++-- docs/reference/module-types/kubernetes.md | 14 ++++++-- 10 files changed, 124 insertions(+), 46 deletions(-) diff --git a/.circleci/continue-config.yml b/.circleci/continue-config.yml index 4fdf5d040d..31cbf1bf56 100644 --- a/.circleci/continue-config.yml +++ b/.circleci/continue-config.yml @@ -1084,9 +1084,25 @@ jobs: command: | .\dist\windows-amd64\garden.exe deploy --root .\examples\demo-project\ --logger-type basic --log-level silly --env remote --force-build --var userId=$env:CIRCLE_BUILD_NUM-win - run: - name: Cleanup + name: Cleanup demo-project command: (kubectl delete namespace --wait=false demo-project-testing-$env:CIRCLE_BUILD_NUM-win) -or $true when: always + - run: + name: Validate vote-helm project # to validate the sync mode paths on Windows with action-based configs + command: | + .\dist\windows-amd64\garden.exe validate --root .\e2e\projects\vote-helm\ --logger-type basic --log-level silly --env testing --var userId=$env:CIRCLE_BUILD_NUM-win + - run: + name: Cleanup vote-helm project + command: (kubectl delete namespace --wait=false vote-helm-testing-$env:CIRCLE_BUILD_NUM-win) -or $true + when: always + - run: + name: Validate vote-helm-modules project # to validate the sync mode paths on Windows with module-based configs + command: | + .\dist\windows-amd64\garden.exe validate --root .\e2e\projects\vote-helm-modules\ --logger-type basic --log-level silly --env testing --var userId=$env:CIRCLE_BUILD_NUM-win + - run: + name: Cleanup vote-helm-modules project + command: (kubectl delete namespace --wait=false vote-helm-modules-testing-$env:CIRCLE_BUILD_NUM-win) -or $true + when: always - fail_fast workflows: diff --git a/core/src/plugins/container/config.ts b/core/src/plugins/container/config.ts index 7eeaba8ae0..41a8685099 100644 --- a/core/src/plugins/container/config.ts +++ b/core/src/plugins/container/config.ts @@ -31,6 +31,7 @@ import type { RunAction, RunActionConfig } from "../../actions/run.js" import { memoize } from "lodash-es" import type Joi from "@hapi/joi" import type { OctalPermissionMask } from "../kubernetes/types.js" +import { templateStringLiteral } from "../../docs/common.js" export const defaultDockerfileName = "Dockerfile" @@ -237,6 +238,27 @@ export const syncDefaultGroupSchema = memoize(() => .description("Set the default group on files and directories at the target. " + ownerDocs) ) +const exampleActionRef = templateStringLiteral("actions.build.my-container-image.sourcePath") +const backSlash = "`\\`" +const forwardSlash = "`/`" + +export const syncSourcePathSchema = memoize(() => + joi + .string() + .default(".") + .description( + deline` + Path to a local directory to be synchronized with the target. + + This should generally be a templated path to another action's source path (e.g. ${exampleActionRef}), or a relative path. + + If a path is hard-coded, we recommend sticking with relative paths here, and using forward slashes (${forwardSlash}) as a delimiter, as Windows-style paths with back slashes (${backSlash}) and absolute paths will work on some platforms, but they are not portable and will not work for users on other platforms. + + Defaults to the Deploy action's config's directory if no value is provided. + ` + ) + .example("src") +) export const syncTargetPathSchema = memoize(() => joi .posixPath() @@ -254,15 +276,7 @@ export const syncTargetPathSchema = memoize(() => const containerSyncSchema = createSchema({ name: "container-sync", keys: () => ({ - source: joi - .string() - .default(".") - .description( - deline` - POSIX-style or Windows path of the directory to sync to the target. Defaults to the config's directory if no value is provided. - ` - ) - .example("src"), + source: syncSourcePathSchema(), target: syncTargetPathSchema(), exclude: syncExcludeSchema(), mode: syncModeSchema(), diff --git a/core/src/plugins/kubernetes/sync.ts b/core/src/plugins/kubernetes/sync.ts index ca332eecd4..283a32196b 100644 --- a/core/src/plugins/kubernetes/sync.ts +++ b/core/src/plugins/kubernetes/sync.ts @@ -24,6 +24,7 @@ import { syncDefaultOwnerSchema, syncExcludeSchema, syncModeSchema, + syncSourcePathSchema, syncTargetPathSchema, } from "../container/moduleConfig.js" import { dedent, gardenAnnotationKey } from "../../util/string.js" @@ -58,7 +59,6 @@ import type { PluginContext } from "../../plugin-context.js" import type { SyncConfig, SyncSession } from "../../mutagen.js" import { mutagenAgentPath, Mutagen, haltedStatuses, mutagenStatusDescriptions } from "../../mutagen.js" import { k8sSyncUtilImageName } from "./constants.js" -import { templateStringLiteral } from "../../docs/common.js" import { relative, resolve } from "path" import type { Resolved } from "../../actions/types.js" import { isAbsolute } from "path" @@ -136,8 +136,6 @@ export interface KubernetesDeployDevModeSyncSpec extends DevModeSyncOptions { containerName?: string } -const exampleActionRef = templateStringLiteral("actions.build.my-container-image.sourcePath") - export const kubernetesDeploySyncPathSchema = () => joi .object() @@ -145,16 +143,7 @@ export const kubernetesDeploySyncPathSchema = () => target: targetResourceSpecSchema().description( "The Kubernetes resource to sync to. If specified, this is used instead of `spec.defaultTarget`." ), - sourcePath: joi - .posixPath() - .default(".") - .description( - dedent` - The local path to sync from, either absolute or relative to the source directory where the Deploy action is defined. - - This should generally be a templated path to another action's source path (e.g. ${exampleActionRef}), or a relative path. If a path is hard-coded, you must make sure the path exists, and that it is reliably the correct path for every user. - ` - ), + sourcePath: syncSourcePathSchema(), containerPath: syncTargetPathSchema(), exclude: syncExcludeSchema(), diff --git a/docs/reference/action-types/Deploy/container.md b/docs/reference/action-types/Deploy/container.md index c71b2e2d7d..ae88702838 100644 --- a/docs/reference/action-types/Deploy/container.md +++ b/docs/reference/action-types/Deploy/container.md @@ -556,7 +556,10 @@ Specify one or more source files or directories to automatically sync with the r [spec](#spec) > [sync](#specsync) > [paths](#specsyncpaths) > source -POSIX-style or Windows path of the directory to sync to the target. Defaults to the config's directory if no value is provided. +Path to a local directory to be synchronized with the target. +This should generally be a templated path to another action's source path (e.g. `${actions.build.my-container-image.sourcePath}`), or a relative path. +If a path is hard-coded, we recommend sticking with relative paths here, and using forward slashes (`/`) as a delimiter, as Windows-style paths with back slashes (`\`) and absolute paths will work on some platforms, but they are not portable and will not work for users on other platforms. +Defaults to the Deploy action's config's directory if no value is provided. | Type | Default | Required | | -------- | ------- | -------- | diff --git a/docs/reference/action-types/Deploy/helm.md b/docs/reference/action-types/Deploy/helm.md index 82cf96668e..2c1a82a702 100644 --- a/docs/reference/action-types/Deploy/helm.md +++ b/docs/reference/action-types/Deploy/helm.md @@ -647,13 +647,25 @@ The name of a container in the target. Specify this if the target contains more [spec](#spec) > [sync](#specsync) > [paths](#specsyncpaths) > sourcePath -The local path to sync from, either absolute or relative to the source directory where the Deploy action is defined. +Path to a local directory to be synchronized with the target. +This should generally be a templated path to another action's source path (e.g. `${actions.build.my-container-image.sourcePath}`), or a relative path. +If a path is hard-coded, we recommend sticking with relative paths here, and using forward slashes (`/`) as a delimiter, as Windows-style paths with back slashes (`\`) and absolute paths will work on some platforms, but they are not portable and will not work for users on other platforms. +Defaults to the Deploy action's config's directory if no value is provided. -This should generally be a templated path to another action's source path (e.g. `${actions.build.my-container-image.sourcePath}`), or a relative path. If a path is hard-coded, you must make sure the path exists, and that it is reliably the correct path for every user. +| Type | Default | Required | +| -------- | ------- | -------- | +| `string` | `"."` | No | + +Example: -| Type | Default | Required | -| ----------- | ------- | -------- | -| `posixPath` | `"."` | No | +```yaml +spec: + ... + sync: + ... + paths: + - sourcePath: "src" +``` ### `spec.sync.paths[].containerPath` diff --git a/docs/reference/action-types/Deploy/kubernetes.md b/docs/reference/action-types/Deploy/kubernetes.md index 2ee5c49fe9..ab7ee9225d 100644 --- a/docs/reference/action-types/Deploy/kubernetes.md +++ b/docs/reference/action-types/Deploy/kubernetes.md @@ -684,13 +684,25 @@ The name of a container in the target. Specify this if the target contains more [spec](#spec) > [sync](#specsync) > [paths](#specsyncpaths) > sourcePath -The local path to sync from, either absolute or relative to the source directory where the Deploy action is defined. +Path to a local directory to be synchronized with the target. +This should generally be a templated path to another action's source path (e.g. `${actions.build.my-container-image.sourcePath}`), or a relative path. +If a path is hard-coded, we recommend sticking with relative paths here, and using forward slashes (`/`) as a delimiter, as Windows-style paths with back slashes (`\`) and absolute paths will work on some platforms, but they are not portable and will not work for users on other platforms. +Defaults to the Deploy action's config's directory if no value is provided. -This should generally be a templated path to another action's source path (e.g. `${actions.build.my-container-image.sourcePath}`), or a relative path. If a path is hard-coded, you must make sure the path exists, and that it is reliably the correct path for every user. +| Type | Default | Required | +| -------- | ------- | -------- | +| `string` | `"."` | No | + +Example: -| Type | Default | Required | -| ----------- | ------- | -------- | -| `posixPath` | `"."` | No | +```yaml +spec: + ... + sync: + ... + paths: + - sourcePath: "src" +``` ### `spec.sync.paths[].containerPath` diff --git a/docs/reference/module-types/container.md b/docs/reference/module-types/container.md index aaabcc04fc..55dfd2f19c 100644 --- a/docs/reference/module-types/container.md +++ b/docs/reference/module-types/container.md @@ -312,8 +312,13 @@ services: # Specify one or more source files or directories to automatically sync with the running container. paths: - - # POSIX-style or Windows path of the directory to sync to the target. Defaults to the config's directory if - # no value is provided. + - # Path to a local directory to be synchronized with the target. + # This should generally be a templated path to another action's source path (e.g. + # `${actions.build.my-container-image.sourcePath}`), or a relative path. + # If a path is hard-coded, we recommend sticking with relative paths here, and using forward slashes (`/`) + # as a delimiter, as Windows-style paths with back slashes (`\`) and absolute paths will work on some + # platforms, but they are not portable and will not work for users on other platforms. + # Defaults to the Deploy action's config's directory if no value is provided. source: . # POSIX-style absolute path to sync to inside the container. The root path (i.e. "/") is not allowed. @@ -1409,7 +1414,10 @@ Specify one or more source files or directories to automatically sync with the r [services](#services) > [sync](#servicessync) > [paths](#servicessyncpaths) > source -POSIX-style or Windows path of the directory to sync to the target. Defaults to the config's directory if no value is provided. +Path to a local directory to be synchronized with the target. +This should generally be a templated path to another action's source path (e.g. `${actions.build.my-container-image.sourcePath}`), or a relative path. +If a path is hard-coded, we recommend sticking with relative paths here, and using forward slashes (`/`) as a delimiter, as Windows-style paths with back slashes (`\`) and absolute paths will work on some platforms, but they are not portable and will not work for users on other platforms. +Defaults to the Deploy action's config's directory if no value is provided. | Type | Default | Required | | -------- | ------- | -------- | diff --git a/docs/reference/module-types/helm.md b/docs/reference/module-types/helm.md index 1bcb4b284b..09f6335226 100644 --- a/docs/reference/module-types/helm.md +++ b/docs/reference/module-types/helm.md @@ -243,8 +243,13 @@ sync: # Specify one or more source files or directories to automatically sync with the running container. paths: - - # POSIX-style or Windows path of the directory to sync to the target. Defaults to the config's directory if no - # value is provided. + - # Path to a local directory to be synchronized with the target. + # This should generally be a templated path to another action's source path (e.g. + # `${actions.build.my-container-image.sourcePath}`), or a relative path. + # If a path is hard-coded, we recommend sticking with relative paths here, and using forward slashes (`/`) as a + # delimiter, as Windows-style paths with back slashes (`\`) and absolute paths will work on some platforms, but + # they are not portable and will not work for users on other platforms. + # Defaults to the Deploy action's config's directory if no value is provided. source: . # POSIX-style absolute path to sync to inside the container. The root path (i.e. "/") is not allowed. @@ -1099,7 +1104,10 @@ Specify one or more source files or directories to automatically sync with the r [sync](#sync) > [paths](#syncpaths) > source -POSIX-style or Windows path of the directory to sync to the target. Defaults to the config's directory if no value is provided. +Path to a local directory to be synchronized with the target. +This should generally be a templated path to another action's source path (e.g. `${actions.build.my-container-image.sourcePath}`), or a relative path. +If a path is hard-coded, we recommend sticking with relative paths here, and using forward slashes (`/`) as a delimiter, as Windows-style paths with back slashes (`\`) and absolute paths will work on some platforms, but they are not portable and will not work for users on other platforms. +Defaults to the Deploy action's config's directory if no value is provided. | Type | Default | Required | | -------- | ------- | -------- | diff --git a/docs/reference/module-types/jib-container.md b/docs/reference/module-types/jib-container.md index 2c4dbcc499..e5262ebc43 100644 --- a/docs/reference/module-types/jib-container.md +++ b/docs/reference/module-types/jib-container.md @@ -380,8 +380,13 @@ services: # Specify one or more source files or directories to automatically sync with the running container. paths: - - # POSIX-style or Windows path of the directory to sync to the target. Defaults to the config's directory if - # no value is provided. + - # Path to a local directory to be synchronized with the target. + # This should generally be a templated path to another action's source path (e.g. + # `${actions.build.my-container-image.sourcePath}`), or a relative path. + # If a path is hard-coded, we recommend sticking with relative paths here, and using forward slashes (`/`) + # as a delimiter, as Windows-style paths with back slashes (`\`) and absolute paths will work on some + # platforms, but they are not portable and will not work for users on other platforms. + # Defaults to the Deploy action's config's directory if no value is provided. source: . # POSIX-style absolute path to sync to inside the container. The root path (i.e. "/") is not allowed. @@ -1624,7 +1629,10 @@ Specify one or more source files or directories to automatically sync with the r [services](#services) > [sync](#servicessync) > [paths](#servicessyncpaths) > source -POSIX-style or Windows path of the directory to sync to the target. Defaults to the config's directory if no value is provided. +Path to a local directory to be synchronized with the target. +This should generally be a templated path to another action's source path (e.g. `${actions.build.my-container-image.sourcePath}`), or a relative path. +If a path is hard-coded, we recommend sticking with relative paths here, and using forward slashes (`/`) as a delimiter, as Windows-style paths with back slashes (`\`) and absolute paths will work on some platforms, but they are not portable and will not work for users on other platforms. +Defaults to the Deploy action's config's directory if no value is provided. | Type | Default | Required | | -------- | ------- | -------- | diff --git a/docs/reference/module-types/kubernetes.md b/docs/reference/module-types/kubernetes.md index f54072272d..569a83bd65 100644 --- a/docs/reference/module-types/kubernetes.md +++ b/docs/reference/module-types/kubernetes.md @@ -269,8 +269,13 @@ sync: # Specify one or more source files or directories to automatically sync with the running container. paths: - - # POSIX-style or Windows path of the directory to sync to the target. Defaults to the config's directory if no - # value is provided. + - # Path to a local directory to be synchronized with the target. + # This should generally be a templated path to another action's source path (e.g. + # `${actions.build.my-container-image.sourcePath}`), or a relative path. + # If a path is hard-coded, we recommend sticking with relative paths here, and using forward slashes (`/`) as a + # delimiter, as Windows-style paths with back slashes (`\`) and absolute paths will work on some platforms, but + # they are not portable and will not work for users on other platforms. + # Defaults to the Deploy action's config's directory if no value is provided. source: . # POSIX-style absolute path to sync to inside the container. The root path (i.e. "/") is not allowed. @@ -1150,7 +1155,10 @@ Specify one or more source files or directories to automatically sync with the r [sync](#sync) > [paths](#syncpaths) > source -POSIX-style or Windows path of the directory to sync to the target. Defaults to the config's directory if no value is provided. +Path to a local directory to be synchronized with the target. +This should generally be a templated path to another action's source path (e.g. `${actions.build.my-container-image.sourcePath}`), or a relative path. +If a path is hard-coded, we recommend sticking with relative paths here, and using forward slashes (`/`) as a delimiter, as Windows-style paths with back slashes (`\`) and absolute paths will work on some platforms, but they are not portable and will not work for users on other platforms. +Defaults to the Deploy action's config's directory if no value is provided. | Type | Default | Required | | -------- | ------- | -------- |