From 8c0d7756ff23d9622fbc15aeee95ee1e4507bfc6 Mon Sep 17 00:00:00 2001 From: panicboat Date: Sat, 31 Oct 2026 08:45:49 +0900 Subject: [PATCH 1/2] feat!: define one service name rule and remove all A service name had three definitions: `{service}` captured any path element, the scanner and change detection rejected only a leading dot, and labels required [a-zA-Z0-9_-]+. A directory such as apps/a.b was therefore detected as a service and labeled deploy:a.b, services-detected emitted [null,"api"], and the resolver silently skipped the label. Entities::Service.valid_name? is now the only definition. A name that fails it is not a service anywhere: ChangeDetector gives it no label, the directory scanner prunes it during descent, so it may be unreadable, and a `service` exclude condition that fails it is a configuration error. BREAKING CHANGE: Directories whose name does not match [a-zA-Z0-9_-]+ are no longer services, and `service: a.b` in an exclude condition is rejected. `all` is no longer special, because the label is unused and its only other user is being deleted. A directory named all is an ordinary service, deploy:all targets only that directory, and `targets all` looks up a service named all, returning [] when there is none. DeployLabel#deploy_all? and `services: nil` in EnumerateTargets are removed, since nothing lists every service anymore. The candidates output of `targets SERVICE` and the label-resolver action is removed along with the root calculation, because that same check was their only reader. Entities::ServiceTargets, TargetCandidate#root, and ListedTarget#root go with them. The targets and has-targets outputs are unchanged, and so is config-manager test for a valid configuration. Claude-Session: https://claude.ai/code/session_01N1XYRqYewzhQfyJcM9zFjG Signed-off-by: panicboat --- README-ja.md | 2 +- README.md | 2 +- action-scripts/config-manager/README-ja.md | 12 +++- action-scripts/config-manager/README.md | 12 +++- action-scripts/label-dispatcher/README-ja.md | 8 +-- action-scripts/label-dispatcher/README.md | 8 +-- action-scripts/label-resolver/README-ja.md | 18 +++--- action-scripts/label-resolver/README.md | 18 +++--- action-scripts/label-resolver/bin/resolver | 2 +- .../controllers/label_resolver_controller.rb | 7 +-- .../use_cases/generated_matrix.rb | 3 +- .../use_cases/list_service_targets.rb | 10 ++-- .../shared/entities/change_detector.rb | 2 +- .../shared/entities/deploy_label.rb | 10 +--- .../shared/entities/listed_target.rb | 2 +- action-scripts/shared/entities/service.rb | 13 ++-- .../shared/entities/service_targets.rb | 3 - .../shared/entities/target_candidate.rb | 4 +- .../shared/entities/target_candidates.rb | 9 +-- .../shared/entities/workflow_config_parser.rb | 2 +- .../presenters/console_presenter.rb | 4 +- .../presenters/github_actions_presenter.rb | 7 +-- .../interfaces/presenters/text_format.rb | 8 +-- .../shared/use_cases/enumerate_targets.rb | 8 +-- .../shared/use_cases/resolve_environments.rb | 2 +- .../use_cases/test_service_spec.rb | 8 +-- action-scripts/spec/factories.rb | 4 -- .../spec/label-dispatcher/cli_spec.rb | 20 +++++++ .../spec/label-resolver/cli_spec.rb | 27 ++++++--- .../label_resolver_controller_spec.rb | 18 +++--- .../use_cases/generate_matrix_spec.rb | 35 +++++++---- .../use_cases/list_service_targets_spec.rb | 60 ++++++++++--------- .../shared/entities/change_detector_spec.rb | 41 +++++++------ .../spec/shared/entities/deploy_label_spec.rb | 42 +++++-------- .../spec/shared/entities/service_spec.rb | 31 +++++----- .../shared/entities/target_candidate_spec.rb | 5 +- .../shared/entities/target_candidates_spec.rb | 9 --- .../entities/workflow_config_parser_spec.rb | 20 ++++++- .../presenters/console_presenter_spec.rb | 10 ++-- .../github_actions_presenter_spec.rb | 41 +++++-------- .../interfaces/presenters/text_format_spec.rb | 26 ++++---- .../use_cases/enumerate_targets_spec.rb | 50 ++++++---------- .../shared/use_cases/list_candidates_spec.rb | 7 +-- .../use_cases/resolve_environments_spec.rb | 32 +++++----- label-resolver/action.yaml | 5 +- 45 files changed, 331 insertions(+), 336 deletions(-) delete mode 100644 action-scripts/shared/entities/service_targets.rb diff --git a/README-ja.md b/README-ja.md index ddf264e..4047d5b 100644 --- a/README-ja.md +++ b/README-ja.md @@ -29,7 +29,7 @@ services: type: kustomize ``` -サービスは `roots` の下にあるファイルを持ち、`{service}` の位置のディレクトリ名がサービス名になります。stack はサービスの中のデプロイ元ディレクトリです。この設定では、`apps/api/src/main.rb` を変更すると Pull Request に `deploy:api` が付き、`apps/api/kubernetes/overlays/develop` ディレクトリがあれば次のターゲットが作られます。 +サービスは `roots` の下にあるファイルを持ち、`{service}` の位置のディレクトリ名がサービス名になります。[サービス名](action-scripts/config-manager/README-ja.md#service-names)に使えるのは ASCII の英字、数字、`_`、`-` だけで、`a.b` や `.github` のようにそれ以外の文字を含むディレクトリはサービスではありません。stack はサービスの中のデプロイ元ディレクトリです。この設定では、`apps/api/src/main.rb` を変更すると Pull Request に `deploy:api` が付き、`apps/api/kubernetes/overlays/develop` ディレクトリがあれば次のターゲットが作られます。 ```json { diff --git a/README.md b/README.md index 562a5f5..d004118 100644 --- a/README.md +++ b/README.md @@ -29,7 +29,7 @@ services: type: kustomize ``` -A service owns the files under its `roots`, and the directory name at `{service}` becomes the service name. A stack is a directory inside the service to deploy from. With this configuration, changing `apps/api/src/main.rb` labels the pull request `deploy:api`, and an existing `apps/api/kubernetes/overlays/develop` directory produces this target: +A service owns the files under its `roots`, and the directory name at `{service}` becomes the service name. A [service name](action-scripts/config-manager/README.md#service-names) contains only ASCII letters, digits, `_`, and `-`, and a directory with any other name, such as `a.b` or `.github`, is not a service. A stack is a directory inside the service to deploy from. With this configuration, changing `apps/api/src/main.rb` labels the pull request `deploy:api`, and an existing `apps/api/kubernetes/overlays/develop` directory produces this target: ```json { diff --git a/action-scripts/config-manager/README-ja.md b/action-scripts/config-manager/README-ja.md index 5e08429..2cb5fe7 100644 --- a/action-scripts/config-manager/README-ja.md +++ b/action-scripts/config-manager/README-ja.md @@ -22,7 +22,7 @@ bundle exec ruby config-manager/bin/config-manager test demo bundle exec ruby config-manager/bin/config-manager test demo production ``` -環境を省略すると、定義された環境と探索された環境のすべてに、環境をまたいで共有されるターゲットを加えて調べます。環境を指定すると、その環境と共有ターゲットを調べます。サービスの事前登録は不要で、どのディレクトリにも一致しない名前を渡すと結果は空になります。各ターゲットは stack の `id`、展開後の属性値、取得したプレースホルダーとともに表示されます。除外されたターゲットも `excluded: true` を付けて表示されるので、デプロイから外れるものを確認できます。未知の環境、不正な設定、ディレクトリを列挙できない場合、同じターゲットで取得値が食い違う場合はエラーになり、終了コード 1 で終わります。 +環境を省略すると、定義された環境と探索された環境のすべてに、環境をまたいで共有されるターゲットを加えて調べます。環境を指定すると、その環境と共有ターゲットを調べます。サービスの事前登録は不要で、どのディレクトリにも一致しない名前や[正しいサービス名](#service-names)でない名前を渡すと結果は空になります。各ターゲットは stack の `id`、展開後の属性値、取得したプレースホルダーとともに表示されます。除外されたターゲットも `excluded: true` を付けて表示されるので、デプロイから外れるものを確認できます。未知の環境、不正な設定、ディレクトリを列挙できない場合、同じターゲットで取得値が食い違う場合はエラーになり、終了コード 1 で終わります。 元の設定を編集せずに変更を試すときは、`WORKFLOW_CONFIG_PATH` でコピーを指定します。 @@ -118,6 +118,12 @@ services: root には `{project}` のような任意のプレースホルダーを使えます。`envs/{environment}/{service}` のように root に `{environment}` を書くと拒否されます。環境は stack の `path` で表すものだからです。環境ディレクトリがサービスの下にあるなら、`{environment}` は `path` に書き、root はサービスのディレクトリのままにします。こうすると、環境の隣にある共有ディレクトリも root の下に収まります。 +### Service Names + +サービス名は `[a-zA-Z0-9_-]+` に一致する名前で、使えるのは ASCII の英字、数字、`_`、`-` です。`all` も他と同じ普通の名前です。 + +`{service}` の位置のディレクトリのうち、`a.b` や `.github` のようにそれ以外の名前を持つものはサービスではありません。その下を変更してもラベルは付かず、ターゲットも作られず、その下の環境も探索されません。ツールは中を見ないので、読み取れないディレクトリでも構いません。`{team}` など他のプレースホルダーにこの制約はありません。 + ### Stacks stack は各 root に `path` を続けた場所で動きます。root が `aws/{service}`、path が `{environment}` なら、ディレクトリのパターンは `aws/{service}/{environment}` です。複数の stack が同じディレクトリを指しても構いません。 @@ -154,7 +160,7 @@ profile の属性には環境の属性と同じルールが適用されます。 ### Path Patterns -root と path は相対パターンです。絶対パスと `..` セグメントは拒否されます。先頭の `./`、余分な `/`、`.` セグメントは正規化されます。名前がドットで始まるサービスは対象から外れます。 +root と path は相対パターンです。絶対パスと `..` セグメントは拒否されます。先頭の `./`、余分な `/`、`.` セグメントは正規化されます。 任意のプレースホルダーの名前は `[a-z_][a-z0-9_]*` に一致する必要があり、値は 1 つのパスセグメントに収まります。同じプレースホルダーを複数回書くと、すべて同じ値に一致しなければなりません。glob の文字は文字どおりに扱われます。 @@ -166,7 +172,7 @@ root と path は相対パターンです。絶対パスと `..` セグメント 条件は完全一致で判定します。マップ内のキーは AND、配列のエントリは OR で組み合わされ、書かなかったキーは制約になりません。一致した root が取得しない値を要求する条件は、その root では一致しません。たとえば `project: sandbox` が除外するのは、`gcp/{project}/{service}` のような root の下のターゲットだけです。 -条件の値は 1 つのパスセグメントを表す空でない文字列で、`/`、`.`、`..` は使えません。サービス名をドットで始めることはできません。値は文字どおりに扱われ、プレースホルダーは展開されません。 +条件の値は 1 つのパスセグメントを表す空でない文字列で、`/`、`.`、`..` は使えません。`service` の条件は[正しいサービス名](#service-names)でなければならず、`service: a.b` は設定エラーになります。値は文字どおりに扱われ、プレースホルダーは展開されません。 `environment` 条件の扱いは、stack が環境をどう得るかで変わります。`environments` があれば、その stack で定義した環境だけを指定できます。環境を探索する stack では、まだ存在しないディレクトリの値も指定できます。`environments` も `path` の `{environment}` も持たない stack では、`environment` を省略するか `null` にします。それ以外の条件の値に `null` は使えません。 diff --git a/action-scripts/config-manager/README.md b/action-scripts/config-manager/README.md index 7719822..d99b276 100644 --- a/action-scripts/config-manager/README.md +++ b/action-scripts/config-manager/README.md @@ -22,7 +22,7 @@ bundle exec ruby config-manager/bin/config-manager test demo bundle exec ruby config-manager/bin/config-manager test demo production ``` -Without an environment it covers all declared and discovered environments plus the targets shared across environments. With an environment it covers that environment plus the shared targets. Services do not need registering, and a name that matches no directory prints an empty result. Each target is printed with its stack `id`, expanded attribute values, and captured placeholders. Excluded targets are printed too, with `excluded: true`, so you can see what is left out of deployment. An unknown environment, an invalid configuration, a directory that cannot be listed, and conflicting captured values for the same target are errors and exit with status 1. +Without an environment it covers all declared and discovered environments plus the targets shared across environments. With an environment it covers that environment plus the shared targets. Services do not need registering, and a name that matches no directory or is not a [valid service name](#service-names) prints an empty result. Each target is printed with its stack `id`, expanded attribute values, and captured placeholders. Excluded targets are printed too, with `excluded: true`, so you can see what is left out of deployment. An unknown environment, an invalid configuration, a directory that cannot be listed, and conflicting captured values for the same target are errors and exit with status 1. To try a configuration change without editing the original, point `WORKFLOW_CONFIG_PATH` at a copy: @@ -118,6 +118,12 @@ The value captured by `{service}` is the service name in labels and the matrix. Roots can contain arbitrary placeholders such as `{project}`. `{environment}` is rejected in roots, as in `envs/{environment}/{service}`, because the environment belongs to a stack's `path`. When environment directories sit below the service, put `{environment}` in `path` and keep the root at the service directory, so that shared directories next to the environments stay under the root. +### Service Names + +A service name matches `[a-zA-Z0-9_-]+`: ASCII letters, digits, `_`, and `-`. `all` is a name like any other. + +A directory at the `{service}` position with any other name, such as `a.b` or `.github`, is not a service. A change below it labels nothing, it produces no targets, and environments below it are not discovered. The tools do not look inside it, so it may be unreadable. Other placeholders, such as `{team}`, have no such rule. + ### Stacks A stack runs in each root followed by its `path`: root `aws/{service}` with path `{environment}` is the directory pattern `aws/{service}/{environment}`. Different stacks can point at the same directory. @@ -154,7 +160,7 @@ Every placeholder referenced by an attribute must be available for every root of ### Path Patterns -Roots and paths are relative patterns. Absolute paths and `..` segments are rejected. A leading `./`, extra `/` separators, and `.` segments are normalized. Service names starting with a dot are excluded. +Roots and paths are relative patterns. Absolute paths and `..` segments are rejected. A leading `./`, extra `/` separators, and `.` segments are normalized. Names of arbitrary placeholders must match `[a-z_][a-z0-9_]*`, and each value occupies one path segment. Repeated occurrences of a placeholder must match the same value. Glob characters are treated literally. @@ -166,7 +172,7 @@ Each entry in `exclude` is a nonempty condition map. Conditions can use `service Conditions use exact matching: keys within a map are combined with AND, and entries in the array are combined with OR. Omitted keys impose no restriction. A condition requiring a captured value that the matched root does not capture does not match there. For example, `project: sandbox` excludes only the targets under a root such as `gcp/{project}/{service}`. -Condition values must be nonempty strings representing one path segment, so `/`, `.`, and `..` are not allowed. Service names cannot start with a dot. Values are literal and do not expand placeholders. +Condition values must be nonempty strings representing one path segment, so `/`, `.`, and `..` are not allowed. A `service` condition must be a [valid service name](#service-names), so `service: a.b` is a configuration error. Values are literal and do not expand placeholders. The `environment` condition depends on how the stack gets its environments. With `environments`, it can name only the stack's declared environments. A stack that discovers environments can name a directory value that does not exist yet. A stack with neither `environments` nor `{environment}` in `path` can omit `environment` or set it to `null`. Other condition values cannot be `null`. diff --git a/action-scripts/label-dispatcher/README-ja.md b/action-scripts/label-dispatcher/README-ja.md index ab7543b..90560e6 100644 --- a/action-scripts/label-dispatcher/README-ja.md +++ b/action-scripts/label-dispatcher/README-ja.md @@ -40,7 +40,7 @@ action は `repository` を全履歴つきで自分で checkout するので、j 変更ファイルがいずれかの root の下にあれば、そのサービスにラベルが付きます。dispatcher は Pull Request の base と head を three-dot diff で比較し、すべての変更ファイルをすべての root に対して次の順で評価します。 -1. ファイルが root の下にない場合、または `{service}` の取得値がドットで始まる場合はスキップします。 +1. ファイルが root の下にない場合、または `{service}` の取得値が[正しいサービス名](../config-manager/README-ja.md#service-names)でない場合はスキップします。 2. root からの相対パスが `ignore` のパターンに一致する場合はスキップします。 3. ファイルが stack のディレクトリ（root に `path` を続けた場所）の下にある場合は、[Label Resolver と同じ方法](../label-resolver/README-ja.md#how-targets-are-built)で候補ターゲットを作ります。`exclude` が候補をすべて除外しない限り、サービスにラベルを付けます。 4. `modules/` や `Makefile` のようにどの stack のディレクトリの下にもない場合は、サービスにラベルを付けます。こうした共有ファイルに `exclude` は効かないので、スキップするには `ignore` を使います。 @@ -55,7 +55,7 @@ action は `repository` を全履歴つきで自分で checkout するので、j - 別の stack のディレクトリの下にあり、その候補がすべて除外されるファイルには、やはりラベルが付きません。 - `environments` を持つ stack では、`{environment}` の位置にある定義外のディレクトリは stack のディレクトリの外として扱います。 -関係のないトップレベルのディレクトリがサービスとみなされないよう、root は十分に絞ります。たとえば `{service}` だけの root では、トップレベルのディレクトリすべてがサービスになります。 +関係のないトップレベルのディレクトリがサービスとみなされないよう、root は十分に絞ります。たとえば `{service}` だけの root では、名前が正しいサービス名であるトップレベルのディレクトリすべてがサービスになります。 共有ファイルの変更で、解決できるターゲットのないサービスにラベルが付くこともあります。唯一の環境が除外されている場合などです。ラベルは付きますが、Label Resolver が返すターゲットは空です。 @@ -128,9 +128,9 @@ labels: prefix は区切り文字を含めてそのまま使われるので、`:` で終わる prefix は YAML で引用符を付けます。prefix は空でない文字列でなければなりません。ある prefix が別の prefix で始まることは許されません。Label Dispatcher が重なった prefix のラベルまで外してしまうためです。 -`all` は Label Resolver で全サービスを対象にします。dispatcher がこれを作るのは `{service}` の位置に `all` というディレクトリがある場合だけなので、普通は手で付けます。ただし `prefix` のラベルを手で付けても次の push で外れます。`extra_prefixes` で始まるラベルには一切触らないので、`manual/all` や `manual/api` は以後の push でも残ります。 +`prefix` のラベルを手で付けても次の push で外れます。`extra_prefixes` で始まるラベルには一切触らないので、`manual/api` は以後の push でも残ります。 -`{service}` の位置のディレクトリには名前を問わずラベルが付きますが、Label Resolver は `[a-zA-Z0-9_-]+` に一致する名前[だけを読みます](../label-resolver/README-ja.md#from-labels-to-targets)。そのため `a.b` のようなディレクトリには、何にも解決されないラベルが付きます。 +ラベルが付くのは[正しいサービス名](../config-manager/README-ja.md#service-names)だけです。`a.b` のようなディレクトリはサービスではないので、その下を変更してもラベルは付きません。`all` というディレクトリは普通のサービスで `all` が付き、[Label Resolver はそれを](../label-resolver/README-ja.md#from-labels-to-targets)その 1 つのサービスとして読みます。 ## Commands diff --git a/action-scripts/label-dispatcher/README.md b/action-scripts/label-dispatcher/README.md index 4c79c65..247171c 100644 --- a/action-scripts/label-dispatcher/README.md +++ b/action-scripts/label-dispatcher/README.md @@ -40,7 +40,7 @@ Outputs: A service gets a label when a changed file sits below one of its roots. The dispatcher compares the pull request's base and head with a three-dot diff, then evaluates every changed file against every root in this order: -1. The file is skipped when it is not below the root, or when the `{service}` capture starts with a dot. +1. The file is skipped when it is not below the root, or when the `{service}` capture is not a [valid service name](../config-manager/README.md#service-names). 2. The file is skipped when its path relative to the root matches an `ignore` pattern. 3. When the file is below a stack's directory (root followed by `path`), candidate targets are built as [Label Resolver builds them](../label-resolver/README.md#how-targets-are-built). The service is labeled unless `exclude` removes every candidate. 4. When the file is below no stack's directory, such as `modules/` or `Makefile`, the service is labeled. `exclude` does not apply to these shared files, so use `ignore` to skip them. @@ -55,7 +55,7 @@ Three cases change the outcome of rules 3 and 4: - A file below another stack's directory whose candidates are all excluded is still not labeled. - For a stack with `environments`, a directory at the `{environment}` position that is not a declared environment is outside the stack's directory. -Choose roots narrow enough that unrelated top-level directories are not taken as services. A bare `{service}` root, for example, makes every top-level directory a service. +Choose roots narrow enough that unrelated top-level directories are not taken as services. A bare `{service}` root, for example, makes every top-level directory with a valid service name a service. A shared file can label a service that has no target to resolve, for example when its only environment is excluded. The label is added and Label Resolver returns no targets. @@ -128,9 +128,9 @@ labels: Each prefix is used as-is, including its separator, so quote prefixes ending with `:` in YAML. Prefixes must be nonempty strings. No prefix may start with another prefix, because Label Dispatcher would remove the labels of the overlapping prefix. -`all` targets every service in Label Resolver. The dispatcher creates it only for a directory named `all` at the `{service}` position, so it is normally added by hand, and a hand-added label with `prefix` is removed on the next push. Labels starting with an `extra_prefixes` entry are never touched, so `manual/all` or `manual/api` survives later pushes. +A label added by hand with `prefix` is removed on the next push. Labels starting with an `extra_prefixes` entry are never touched, so `manual/api` survives later pushes. -A label is added for any directory name at the `{service}` position, but Label Resolver [reads only names matching](../label-resolver/README.md#from-labels-to-targets) `[a-zA-Z0-9_-]+`. A directory such as `a.b` therefore gets a label that resolves to nothing. +Only a [valid service name](../config-manager/README.md#service-names) gets a label. A directory such as `a.b` is not a service, so a change below it labels nothing. A directory named `all` is an ordinary service and gets `all`, which [Label Resolver reads](../label-resolver/README.md#from-labels-to-targets) as that one service. ## Commands diff --git a/action-scripts/label-resolver/README-ja.md b/action-scripts/label-resolver/README-ja.md index 601bccf..d5a6cd7 100644 --- a/action-scripts/label-resolver/README-ja.md +++ b/action-scripts/label-resolver/README-ja.md @@ -16,7 +16,7 @@ Label Resolver は `deploy:` ラベル、または直接渡したサー environments: "develop,staging" ``` -`service` を指定すると、Pull Request なしでサービスまたは `all` を解決します。release や手動実行の workflow に向いています。このとき `pr-number` は無視され、`ref` でソースリポジトリの checkout 先を選びます。 +`service` を指定すると、Pull Request なしでサービスを解決します。release や手動実行の workflow に向いています。このとき `pr-number` は無視され、`ref` でソースリポジトリの checkout 先を選びます。 ```yaml - uses: panicboat/deploy-actions/label-resolver@v0.1.0 @@ -35,7 +35,7 @@ action はリポジトリを自分で checkout するので、job に checkout |---|---|---| | `repository` | はい | ソースリポジトリ（`owner/repo`） | | `pr-number` | `service` がなければ必須 | ラベルを解決する Pull Request。`service` があれば無視されます | -| `service` | いいえ | サービス名または `all`。Pull Request なしでターゲットを解決します | +| `service` | いいえ | [サービス名](../config-manager/README-ja.md#service-names)。Pull Request なしでターゲットを解決します | | `environments` | いいえ | カンマ区切りの環境。空なら定義された環境と探索された環境のすべて | | `ref` | いいえ | checkout するソースリポジトリの ref。既定は、起動元のリポジトリならイベントの ref、別のリポジトリならデフォルトブランチ | | `github-token` | いいえ | リポジトリの checkout と PR の読み取りに使うトークン。既定は `${{ github.token }}` | @@ -47,11 +47,10 @@ action はリポジトリを自分で checkout するので、job に checkout |---|---| | `targets` | デプロイターゲットの JSON 配列（[Matrix Output](#matrix-output) を参照） | | `has-targets` | `true` または `false` | -| `candidates` | `service` を指定したときの、除外されたものも含む全候補の JSON 配列。各要素は `{"target": , "excluded": , "root": ""}` | ## From Labels to Targets -resolver は Pull Request のラベルのうち、[設定された prefix](../label-dispatcher/README-ja.md#labels)（既定は `deploy:`）のいずれかで始まるものを読み、残りの部分をサービス名として扱います。同じサービスに別の prefix のラベルが付いていても 1 つのサービスとして扱います。どの prefix でも `all` なら、設定されたパスから見つかる全サービスが対象です。残りの部分が正しいサービス名（`[a-zA-Z0-9_-]+`）でないラベルは無視します。 +resolver は Pull Request のラベルのうち、[設定された prefix](../label-dispatcher/README-ja.md#labels)（既定は `deploy:`）のいずれかで始まるものを読み、残りの部分をサービス名として扱います。同じサービスに別の prefix のラベルが付いていても 1 つのサービスとして扱います。残りの部分が[正しいサービス名](../config-manager/README-ja.md#service-names)でないラベルは無視します。`all` も普通の名前なので、`deploy:all` の対象は `all` という名前のディレクトリだけです。 デプロイラベルのない Pull Request では、`targets` は空、`has-targets` は `false` になります。 @@ -59,7 +58,7 @@ resolver は Pull Request のラベルのうち、[設定された prefix](../la ## How Targets Are Built -resolver は、いずれかの root に `path` を続けたパターンに一致する実在のディレクトリをすべて列挙します。存在しないパスからターゲットは作られず、結果が空でも正常です。サービスの事前登録は不要です。stack ごとに、次の順でターゲットを作ります。 +resolver は、いずれかの root に `path` を続けたパターンに一致する実在のディレクトリをすべて列挙します。存在しないパスからターゲットは作られず、結果が空でも正常です。サービスの事前登録は不要です。`{service}` の位置のディレクトリのうち、名前が[正しいサービス名](../config-manager/README-ja.md#service-names)でないものは、中を見ずに読み飛ばします。stack ごとに、次の順でターゲットを作ります。 1. `environments` があれば、`path` に `{environment}` がなくても、定義された環境ごとに 1 つのターゲットを作ります。`path` に `{environment}` があれば定義された名前だけが一致するので、`modules/` のようなディレクトリはターゲットになりません。 2. `environments` がなく `path` に `{environment}` があれば、その位置の実在ディレクトリから環境名を探索します。`modules/` のように環境でないディレクトリも環境として探索されるので、そうしたディレクトリが環境ディレクトリと並ぶなら `environments` を定義します。 @@ -79,7 +78,7 @@ stack の[除外条件](../config-manager/README-ja.md#exclusion-conditions)に | キー | 由来 | |---|---| -| `service` | ラベルまたは `all` で見つかったサービス名。`{service}` で取得した値 | +| `service` | ラベルまたは `service` 入力のサービス名。`{service}` で取得した値 | | `environment` | 定義または探索された環境名。環境をまたいで共有するターゲットでは `null` | | `stack` | stack の `id` | | `working_directory` | 実在するターゲットディレクトリの相対パス | @@ -132,7 +131,7 @@ deploy: | コマンド | 結果 | |---|---| | `bundle exec ruby label-resolver/bin/resolver resolve PR_NUMBER [ENVIRONMENTS]` | PR のラベルをターゲットに解決します。`ENVIRONMENTS` はカンマ区切りで、空ならすべて | -| `bundle exec ruby label-resolver/bin/resolver targets SERVICE [ENVIRONMENTS]` | PR なしで 1 つのサービスまたは `all` のターゲットを解決します。`ENVIRONMENTS` は `resolve` と同じ | +| `bundle exec ruby label-resolver/bin/resolver targets SERVICE [ENVIRONMENTS]` | PR なしで 1 つのサービスのターゲットを解決します。`ENVIRONMENTS` は `resolve` と同じ | `resolve` はラベルを読むために `GITHUB_TOKEN` と `GITHUB_REPOSITORY` を必要とします。 @@ -151,16 +150,15 @@ bundle exec ruby label-resolver/bin/resolver resolve 123 ```bash bundle exec ruby label-resolver/bin/resolver targets demo develop -bundle exec ruby label-resolver/bin/resolver targets all ``` -`targets` に渡せるのは `[a-zA-Z0-9_-]+` に一致する名前か `all` で、それ以外の名前は終了コード 1 で失敗します。正しい名前で一致するディレクトリがないのはエラーにならず、出力の `targets` は `[]`、`has-targets` は `false`、`candidates` は `[]` になります。出力の `targets` と `has-targets` は除外されなかった候補から作られ、`candidates` は全候補を除外状態と root とともに並べます。`resolve` コマンドが書き出すのは `targets` と `has-targets` だけです。 +`targets` に渡せるのは[正しいサービス名](../config-manager/README-ja.md#service-names)で、それ以外の名前は終了コード 1 で失敗します。正しい名前で一致するディレクトリがないのはエラーにならず、出力の `targets` は `[]`、`has-targets` は `false` になります。`all` も他の名前と同じように探します。どちらのコマンドも書き出すのは `targets` と `has-targets` だけで、除外されたターゲットは含みません。除外されたターゲットを確認するには [`config-manager test`](../config-manager/README-ja.md#commands) を使います。 どちらのコマンドも、不正な設定、定義も探索もされていない環境、列挙できないディレクトリ、取得値の食い違いがあると終了コード 1 で失敗します。読む環境変数は [Environment Variables](../config-manager/README-ja.md#environment-variables) を参照してください。 ## Code Layout -2 つのコマンドの入口は `LabelResolverController` です。`resolve` では、`GetLabels` が PR のデプロイラベルを読み、`GenerateMatrix` が `ListCandidates` を呼んで、除外されなかった候補を残します。`ListCandidates` は環境を検証し、ディレクトリ構造から候補を列挙します。`targets` では、`ListServiceTargets` がサービス名を検証して `ListCandidates` を呼び、ターゲットと、除外状態と root を付けた全候補を返します。 +2 つのコマンドの入口は `LabelResolverController` です。`resolve` では、`GetLabels` が PR のデプロイラベルを読み、`GenerateMatrix` が `ListCandidates` を呼んで、除外されなかった候補を残します。`ListCandidates` は環境を検証し、ディレクトリ構造から候補を列挙します。`targets` では、`ListServiceTargets` がサービス名を検証して `ListCandidates` を呼び、除外されなかったターゲットを返します。 ## Testing diff --git a/action-scripts/label-resolver/README.md b/action-scripts/label-resolver/README.md index d675f26..3e2949a 100644 --- a/action-scripts/label-resolver/README.md +++ b/action-scripts/label-resolver/README.md @@ -16,7 +16,7 @@ The usual call resolves the labels of a pull request. `environments` is optional environments: "develop,staging" ``` -Set `service` to resolve a service, or `all`, without a pull request. This suits release or manual workflows. `pr-number` is then ignored, and `ref` selects the ref of the source repository to check out. +Set `service` to resolve a service without a pull request. This suits release or manual workflows. `pr-number` is then ignored, and `ref` selects the ref of the source repository to check out. ```yaml - uses: panicboat/deploy-actions/label-resolver@v0.1.0 @@ -35,7 +35,7 @@ Inputs: |---|---|---| | `repository` | Yes | Source repository (`owner/repo`) | | `pr-number` | Unless `service` is set | Pull request whose labels are resolved; ignored when `service` is set | -| `service` | No | Service name or `all`; resolves its targets without a pull request | +| `service` | No | [Service name](../config-manager/README.md#service-names); resolves its targets without a pull request | | `environments` | No | Comma-separated environments; empty means all declared and discovered environments | | `ref` | No | Ref of the source repository to check out; defaults to the event's ref for the triggering repository and to the default branch for another repository | | `github-token` | No | Token for checking out the repository and reading the PR; defaults to `${{ github.token }}` | @@ -47,11 +47,10 @@ Outputs: |---|---| | `targets` | JSON array of deployment targets (see [Matrix Output](#matrix-output)) | | `has-targets` | `true` or `false` | -| `candidates` | When `service` is set, JSON array of every candidate including the excluded ones, each `{"target": , "excluded": , "root": ""}` | ## From Labels to Targets -The resolver reads the pull request's labels that start with any [configured prefix](../label-dispatcher/README.md#labels), `deploy:` by default, and treats the rest of the label as the service name. The same service labeled with different prefixes yields one service, and `all` with any prefix targets every service found through the configured paths. Labels whose remaining text is not a valid service name (`[a-zA-Z0-9_-]+`) are ignored. +The resolver reads the pull request's labels that start with any [configured prefix](../label-dispatcher/README.md#labels), `deploy:` by default, and treats the rest of the label as the service name. The same service labeled with different prefixes yields one service. Labels whose remaining text is not a [valid service name](../config-manager/README.md#service-names) are ignored. `all` is an ordinary name, so `deploy:all` targets only a directory named `all`. A pull request without deploy labels gives empty `targets` and `has-targets: false`. @@ -59,7 +58,7 @@ A workflow triggered by `labeled` must filter on the prefixes it uses. When you ## How Targets Are Built -The resolver lists all existing directories matching any root followed by `path`. Missing paths generate no targets and are a valid empty result. Services do not require registration. For each stack, targets are built in this order: +The resolver lists all existing directories matching any root followed by `path`. Missing paths generate no targets and are a valid empty result. Services do not require registration, and a directory at `{service}` whose name is not a [valid service name](../config-manager/README.md#service-names) is skipped without looking inside it. For each stack, targets are built in this order: 1. With `environments`, one target is built for each declared environment, even when `path` does not contain `{environment}`. When it does, only the declared names match, so a directory such as `modules/` never becomes a target. 2. Without `environments`, when `path` contains `{environment}`, environment names are discovered from existing directories at that position. Directories at that position that are not environments, such as `modules/`, are also discovered as environments, so declare `environments` when such directories sit beside the environment directories. @@ -79,7 +78,7 @@ The selectable environment list is the union of all declared names and names dis | Key | Source | |---|---| -| `service` | Service name from a label or discovered through `all`; the value captured by `{service}` | +| `service` | Service name from a label or the `service` input; the value captured by `{service}` | | `environment` | Declared or discovered environment name, or `null` for a target shared across environments | | `stack` | Stack `id` | | `working_directory` | Relative path of the existing target directory | @@ -132,7 +131,7 @@ Run these from the `action-scripts` directory. | Command | Result | |---|---| | `bundle exec ruby label-resolver/bin/resolver resolve PR_NUMBER [ENVIRONMENTS]` | Resolves the PR's labels into targets; `ENVIRONMENTS` is comma-separated, and empty means all | -| `bundle exec ruby label-resolver/bin/resolver targets SERVICE [ENVIRONMENTS]` | Resolves the targets of one service, or `all`, without a PR; `ENVIRONMENTS` is the same as for `resolve` | +| `bundle exec ruby label-resolver/bin/resolver targets SERVICE [ENVIRONMENTS]` | Resolves the targets of one service without a PR; `ENVIRONMENTS` is the same as for `resolve` | `resolve` needs `GITHUB_TOKEN` and `GITHUB_REPOSITORY` to read the labels: @@ -151,16 +150,15 @@ bundle exec ruby label-resolver/bin/resolver resolve 123 ```bash bundle exec ruby label-resolver/bin/resolver targets demo develop -bundle exec ruby label-resolver/bin/resolver targets all ``` -`targets` takes a name matching `[a-zA-Z0-9_-]+` or `all`, and any other name fails with status 1. A valid name with no matching directory is not an error: the `targets` output is `[]`, `has-targets` is `false`, and `candidates` is `[]`. The `targets` and `has-targets` outputs come from the candidates that are not excluded, and `candidates` lists them all with their exclusion and root. The `resolve` command writes only `targets` and `has-targets`. +`targets` takes a [valid service name](../config-manager/README.md#service-names), and any other name fails with status 1. A valid name with no matching directory is not an error: the `targets` output is `[]` and `has-targets` is `false`. The `all` name is looked up like any other. Both commands write only `targets` and `has-targets`, which leave out excluded targets. To see the excluded targets, use [`config-manager test`](../config-manager/README.md#commands). Both commands fail with status 1 on an invalid configuration, an environment that is neither declared nor discovered, a directory that cannot be listed, and conflicting captured values. See [Environment Variables](../config-manager/README.md#environment-variables) for the variables they read. ## Code Layout -`LabelResolverController` backs the two commands. For `resolve`, `GetLabels` reads the deploy labels of the PR, and `GenerateMatrix` calls `ListCandidates`, which validates the environments and lists the candidates from the directory structure, then keeps those that are not excluded. For `targets`, `ListServiceTargets` validates the service name, calls `ListCandidates`, and returns both the targets and every candidate with its exclusion and root. +`LabelResolverController` backs the two commands. For `resolve`, `GetLabels` reads the deploy labels of the PR, and `GenerateMatrix` calls `ListCandidates`, which validates the environments and lists the candidates from the directory structure, then keeps those that are not excluded. For `targets`, `ListServiceTargets` validates the service name, calls `ListCandidates`, and returns the targets that are not excluded. ## Testing diff --git a/action-scripts/label-resolver/bin/resolver b/action-scripts/label-resolver/bin/resolver index 596c7f8..d9aaae4 100644 --- a/action-scripts/label-resolver/bin/resolver +++ b/action-scripts/label-resolver/bin/resolver @@ -20,7 +20,7 @@ class LabelResolverCLI < Thor exit 1 if result.failure? end - desc "targets SERVICE [ENVIRONMENTS]", "Resolve the targets of one service, or all, without a pull request (comma-separated environments, empty=all)" + desc "targets SERVICE [ENVIRONMENTS]", "Resolve the targets of one service without a pull request (comma-separated environments, empty=all)" def targets(service, environments = nil) controller = LabelResolverContainer.resolve(:label_resolver_controller) diff --git a/action-scripts/label-resolver/controllers/label_resolver_controller.rb b/action-scripts/label-resolver/controllers/label_resolver_controller.rb index a76eac5..7ae0d6f 100644 --- a/action-scripts/label-resolver/controllers/label_resolver_controller.rb +++ b/action-scripts/label-resolver/controllers/label_resolver_controller.rb @@ -30,12 +30,7 @@ def targets(service:, environments:) targets_result = @list_service_targets.execute(service: service, environments: environments) return failure(targets_result) if targets_result.failure? - @presenter.present_service_targets( - service: service, - target_environments: environments, - targets: targets_result.value.targets, - candidates: targets_result.value.candidates - ) + @presenter.present_service_targets(service: service, target_environments: environments, targets: targets_result.value) targets_result end diff --git a/action-scripts/label-resolver/use_cases/generated_matrix.rb b/action-scripts/label-resolver/use_cases/generated_matrix.rb index 324ba0d..9abfe0a 100644 --- a/action-scripts/label-resolver/use_cases/generated_matrix.rb +++ b/action-scripts/label-resolver/use_cases/generated_matrix.rb @@ -6,8 +6,7 @@ def initialize(list_candidates:) end def execute(deploy_labels:, target_environments:) - labels = deploy_labels.select(&:valid?) - services = labels.any?(&:deploy_all?) ? nil : labels.map(&:service).uniq + services = deploy_labels.select(&:valid?).map(&:service).uniq candidates = @list_candidates.execute(services: services, environments: target_environments) return candidates if candidates.failure? diff --git a/action-scripts/label-resolver/use_cases/list_service_targets.rb b/action-scripts/label-resolver/use_cases/list_service_targets.rb index acf133f..380246c 100644 --- a/action-scripts/label-resolver/use_cases/list_service_targets.rb +++ b/action-scripts/label-resolver/use_cases/list_service_targets.rb @@ -6,15 +6,15 @@ def initialize(list_candidates:) end def execute(service:, environments:) - unless service.match?(Entities::DeployLabel::SERVICE_PATTERN) - return Entities::Result.failure("Invalid service name: #{service.inspect} (use all or a name matching [a-zA-Z0-9_-]+)") + unless Entities::Service.valid_name?(service) + return Entities::Result.failure("Invalid service name: #{service.inspect} (use a name matching #{Entities::Service.name_format})") end - candidates = @list_candidates.execute(services: service == 'all' ? nil : [service], environments: environments) + candidates = @list_candidates.execute(services: [service], environments: environments) return candidates if candidates.failure? - listed = candidates.value.map(&:to_listed_target) - Entities::Result.success(Entities::ServiceTargets.new(targets: listed.reject(&:excluded?).map(&:target), candidates: listed)) + # Building before filtering keeps an unbuildable excluded candidate a failure instead of hiding it. + Entities::Result.success(candidates.value.map(&:to_listed_target).reject(&:excluded?).map(&:target)) rescue => error Entities::Result.failure("Failed to resolve service targets: #{error.message}") end diff --git a/action-scripts/shared/entities/change_detector.rb b/action-scripts/shared/entities/change_detector.rb index 626d185..5fd8960 100644 --- a/action-scripts/shared/entities/change_detector.rb +++ b/action-scripts/shared/entities/change_detector.rb @@ -27,7 +27,7 @@ def service_name(service, root, file) captures = PatternMatcher.extract_prefix(root, file) return unless captures name = captures.fetch('service') - return if Service.hidden_name?(name) + return unless Service.valid_name?(name) relative_path = file.delete_prefix("#{PatternMatcher.expand(root, captures)}/") name unless service.ignored?(relative_path) end diff --git a/action-scripts/shared/entities/deploy_label.rb b/action-scripts/shared/entities/deploy_label.rb index d81ae26..be516fe 100644 --- a/action-scripts/shared/entities/deploy_label.rb +++ b/action-scripts/shared/entities/deploy_label.rb @@ -4,7 +4,6 @@ module Entities class DeployLabel DEFAULT_PREFIX = 'deploy:'.freeze - SERVICE_PATTERN = /\A[a-zA-Z0-9\-_]+\z/ attr_reader :service, :prefix @@ -14,11 +13,6 @@ def initialize(raw_label, prefixes: [DEFAULT_PREFIX]) parse_label! end - # Check if this is deploy:all label - def deploy_all? - service == 'all' - end - # Factory method to create deploy label from service name def self.from_service(service:, prefix: DEFAULT_PREFIX) new("#{prefix}#{service}", prefixes: [prefix]) @@ -35,7 +29,7 @@ def to_s # Check if the deploy label is valid def valid? - !service.nil? && service.match?(SERVICE_PATTERN) + Service.valid_name?(service) end # Equality comparison @@ -59,7 +53,7 @@ def parse_label! return unless @raw_label @prefix = @prefixes.find do |prefix| - @raw_label.start_with?(prefix) && @raw_label.delete_prefix(prefix).match?(SERVICE_PATTERN) + @raw_label.start_with?(prefix) && Service.valid_name?(@raw_label.delete_prefix(prefix)) end @service = @raw_label.delete_prefix(@prefix) if @prefix end diff --git a/action-scripts/shared/entities/listed_target.rb b/action-scripts/shared/entities/listed_target.rb index 88c4eff..b0e8212 100644 --- a/action-scripts/shared/entities/listed_target.rb +++ b/action-scripts/shared/entities/listed_target.rb @@ -1,5 +1,5 @@ module Entities - ListedTarget = Data.define(:target, :excluded, :root) do + ListedTarget = Data.define(:target, :excluded) do alias_method :excluded?, :excluded end end diff --git a/action-scripts/shared/entities/service.rb b/action-scripts/shared/entities/service.rb index 00fc8fe..01dc2b7 100644 --- a/action-scripts/shared/entities/service.rb +++ b/action-scripts/shared/entities/service.rb @@ -1,12 +1,15 @@ module Entities Service = Data.define(:id, :roots, :ignore, :stacks) do - def self.hidden_name?(name) - name.start_with?('.') + def self.name_format + '[a-zA-Z0-9_-]+' end - def self.hidden_captures?(captures) - service = captures['service'] - !service.nil? && hidden_name?(service) + def self.valid_name?(name) + /\A#{name_format}\z/.match?(name) + end + + def self.captures_invalid_name?(captures) + captures.key?('service') && !valid_name?(captures['service']) end def initialize(**fields) diff --git a/action-scripts/shared/entities/service_targets.rb b/action-scripts/shared/entities/service_targets.rb deleted file mode 100644 index b7762c5..0000000 --- a/action-scripts/shared/entities/service_targets.rb +++ /dev/null @@ -1,3 +0,0 @@ -module Entities - ServiceTargets = Data.define(:targets, :candidates) -end diff --git a/action-scripts/shared/entities/target_candidate.rb b/action-scripts/shared/entities/target_candidate.rb index f401679..7c41658 100644 --- a/action-scripts/shared/entities/target_candidate.rb +++ b/action-scripts/shared/entities/target_candidate.rb @@ -1,5 +1,5 @@ module Entities - TargetCandidate = Data.define(:stack, :service, :environment, :root, :working_directory, :captures) do + TargetCandidate = Data.define(:stack, :service, :environment, :working_directory, :captures) do def exclusion_values captures.merge('service' => service, 'environment' => environment) end @@ -9,7 +9,7 @@ def excluded? end def to_listed_target - ListedTarget.new(target: to_deployment_target, excluded: excluded?, root: root) + ListedTarget.new(target: to_deployment_target, excluded: excluded?) end def to_deployment_target diff --git a/action-scripts/shared/entities/target_candidates.rb b/action-scripts/shared/entities/target_candidates.rb index 954435f..d4cce47 100644 --- a/action-scripts/shared/entities/target_candidates.rb +++ b/action-scripts/shared/entities/target_candidates.rb @@ -14,7 +14,7 @@ def add(stack:, service:, environment:, working_directory:, captures:) raise "Conflicting captures for stack '#{stack.id}' at '#{working_directory}'" end @candidates[identity] = TargetCandidate.new( - stack: stack, service: service, environment: environment, root: root_of(stack, working_directory), + stack: stack, service: service, environment: environment, working_directory: working_directory, captures: custom ) end @@ -22,12 +22,5 @@ def add(stack:, service:, environment:, working_directory:, captures:) def to_a @candidates.values end - - private - - def root_of(stack, working_directory) - parts = working_directory.split('/') - parts.first(parts.length - stack.path.split('/').length).join('/') - end end end diff --git a/action-scripts/shared/entities/workflow_config_parser.rb b/action-scripts/shared/entities/workflow_config_parser.rb index e72dda7..12162f2 100644 --- a/action-scripts/shared/entities/workflow_config_parser.rb +++ b/action-scripts/shared/entities/workflow_config_parser.rb @@ -214,7 +214,7 @@ def validate_exclusions!(stack, position, placeholders, common) end else validate_segment!(value, condition_position) - invalid!(condition_position, 'service must not start with a dot') if key == 'service' && Service.hidden_name?(value) + invalid!(condition_position, "service must match #{Service.name_format}") if key == 'service' && !Service.valid_name?(value) end end end diff --git a/action-scripts/shared/interfaces/presenters/console_presenter.rb b/action-scripts/shared/interfaces/presenters/console_presenter.rb index 1aa602f..9fb0a6f 100644 --- a/action-scripts/shared/interfaces/presenters/console_presenter.rb +++ b/action-scripts/shared/interfaces/presenters/console_presenter.rb @@ -11,8 +11,8 @@ def present_deployment_matrix(deployment_targets:, deploy_labels:, target_enviro print_lines(TextFormat.deployment_matrix(deployment_targets: deployment_targets, deploy_labels: deploy_labels, target_environments: target_environments, pr_number: pr_number), heading_color: :green) end - def present_service_targets(service:, target_environments:, targets:, candidates:) - print_lines(TextFormat.service_targets(service: service, target_environments: target_environments, targets: targets, candidates: candidates), heading_color: :green) + def present_service_targets(service:, target_environments:, targets:) + print_lines(TextFormat.service_targets(service: service, target_environments: target_environments, targets: targets), heading_color: :green) end def present_config_validation_result(valid:, errors: [], summary: nil) diff --git a/action-scripts/shared/interfaces/presenters/github_actions_presenter.rb b/action-scripts/shared/interfaces/presenters/github_actions_presenter.rb index 7b813e8..15df51e 100644 --- a/action-scripts/shared/interfaces/presenters/github_actions_presenter.rb +++ b/action-scripts/shared/interfaces/presenters/github_actions_presenter.rb @@ -25,11 +25,10 @@ def present_deployment_matrix(deployment_targets:, deploy_labels:, target_enviro puts TextFormat.deployment_matrix(deployment_targets: deployment_targets, deploy_labels: deploy_labels, target_environments: target_environments, pr_number: pr_number) end - def present_service_targets(service:, target_environments:, targets:, candidates:) - entries = candidates.map { |listed| { target: listed.target.to_matrix_item, excluded: listed.excluded?, root: listed.root } } - write_outputs('targets' => targets.map(&:to_matrix_item).to_json, 'has-targets' => targets.any?.to_s, 'candidates' => entries.to_json) + def present_service_targets(service:, target_environments:, targets:) + write_outputs('targets' => targets.map(&:to_matrix_item).to_json, 'has-targets' => targets.any?.to_s) - puts TextFormat.service_targets(service: service, target_environments: target_environments, targets: targets, candidates: candidates) + puts TextFormat.service_targets(service: service, target_environments: target_environments, targets: targets) end def present_config_validation_result(valid:, errors: [], summary: nil) diff --git a/action-scripts/shared/interfaces/presenters/text_format.rb b/action-scripts/shared/interfaces/presenters/text_format.rb index 8ad05fa..b034e79 100644 --- a/action-scripts/shared/interfaces/presenters/text_format.rb +++ b/action-scripts/shared/interfaces/presenters/text_format.rb @@ -39,19 +39,17 @@ def deployment_matrix(deployment_targets:, deploy_labels:, target_environments: lines << '' end - def service_targets(service:, target_environments:, targets:, candidates:) + def service_targets(service:, target_environments:, targets:) lines = ['🎯 Service Targets', "Service: #{service}"] if target_environments.length > 1 lines << "Target Environments: #{target_environments.join(', ')}" elsif target_environments.length == 1 lines << "Target Environment: #{target_environments.first}" end - lines << "Targets: #{targets.length} of #{candidates.length} candidates" - candidates.each do |listed| - target = listed.target + lines << "Targets: #{targets.length}" + targets.each do |target| environment = target.environment && " (#{target.environment})" lines << '' << " #{target.service}:#{target.stack}#{environment} -> #{target.working_directory}" - lines << " root: #{listed.root}" << " excluded: #{listed.excluded?}" end lines end diff --git a/action-scripts/shared/use_cases/enumerate_targets.rb b/action-scripts/shared/use_cases/enumerate_targets.rb index c92ff09..d2ab391 100644 --- a/action-scripts/shared/use_cases/enumerate_targets.rb +++ b/action-scripts/shared/use_cases/enumerate_targets.rb @@ -6,15 +6,13 @@ def initialize(directory_scanner:) end def execute(config:, services:, environments:) - selected = services.nil? ? [nil] : services candidates = Entities::TargetCandidates.new config.stacks.each do |stack| stack.target_environments(environments).each do |environment| - selected.each do |service| - values = { 'environment' => environment } - values['service'] = service if service + services.each do |service| + values = { 'environment' => environment, 'service' => service } stack.paths.each do |pattern| - @directory_scanner.resolve(pattern: pattern, values: values, skip: Entities::Service.method(:hidden_captures?)).each do |match| + @directory_scanner.resolve(pattern: pattern, values: values, skip: Entities::Service.method(:captures_invalid_name?)).each do |match| captures = match.fetch(:captures) candidates.add(stack: stack, service: captures.fetch('service'), environment: environment, working_directory: match.fetch(:working_directory), captures: captures) end diff --git a/action-scripts/shared/use_cases/resolve_environments.rb b/action-scripts/shared/use_cases/resolve_environments.rb index 37a3844..1bebed8 100644 --- a/action-scripts/shared/use_cases/resolve_environments.rb +++ b/action-scripts/shared/use_cases/resolve_environments.rb @@ -19,7 +19,7 @@ def execute(config:, requested:) def discovered_environment_names(config) config.stacks.select(&:discovers_environments?).flat_map do |stack| stack.paths.flat_map do |pattern| - @directory_scanner.resolve(pattern: pattern, values: {}, skip: Entities::Service.method(:hidden_captures?)).map do |match| + @directory_scanner.resolve(pattern: pattern, values: {}, skip: Entities::Service.method(:captures_invalid_name?)).map do |match| match.fetch(:captures).fetch('environment') end end diff --git a/action-scripts/spec/config-manager/use_cases/test_service_spec.rb b/action-scripts/spec/config-manager/use_cases/test_service_spec.rb index a379b86..80bcaa1 100644 --- a/action-scripts/spec/config-manager/use_cases/test_service_spec.rb +++ b/action-scripts/spec/config-manager/use_cases/test_service_spec.rb @@ -123,14 +123,14 @@ def candidates(service: 'demo', environment: nil) expect(result.error_message).to eq('Failed to test service configuration: Permission denied') end - it 'carries the root of each target' do - directory('dystopia/demo/aws/develop') - expect(candidates.map(&:root)).to all(eq('dystopia/demo')) + it 'lists nothing for a directory whose name is not a service' do + directory('dystopia/a.b/aws/develop') + expect(candidates(service: 'a.b')).to eq([]) end it 'fails for an attribute that references a placeholder with no value' do stack = Entities::Stack.new(id: 'aws', service_id: 'app', path: 'aws', paths: ['{service}/aws'], attributes: { 'repository' => 'ghcr.io/{team}/{service}' }, environments: nil, exclude: []) - unbuildable = Entities::TargetCandidate.new(stack: stack, service: 'demo', environment: nil, root: 'demo', working_directory: 'demo/aws', captures: {}) + unbuildable = Entities::TargetCandidate.new(stack: stack, service: 'demo', environment: nil, working_directory: 'demo/aws', captures: {}) list_candidates = instance_double(UseCases::Shared::ListCandidates, execute: Entities::Result.success([unbuildable])) result = described_class.new(list_candidates: list_candidates).execute(service_name: 'demo', environment: nil) diff --git a/action-scripts/spec/factories.rb b/action-scripts/spec/factories.rb index df3c485..317fe56 100644 --- a/action-scripts/spec/factories.rb +++ b/action-scripts/spec/factories.rb @@ -8,10 +8,6 @@ label_string { "deploy:test-service" } end - trait :valid_all do - label_string { "deploy:all" } - end - trait :invalid do label_string { "invalid-label" } end diff --git a/action-scripts/spec/label-dispatcher/cli_spec.rb b/action-scripts/spec/label-dispatcher/cli_spec.rb index b57e966..f936ce1 100644 --- a/action-scripts/spec/label-dispatcher/cli_spec.rb +++ b/action-scripts/spec/label-dispatcher/cli_spec.rb @@ -2,6 +2,7 @@ require 'thor' require 'open3' require 'tmpdir' +require 'fileutils' RSpec.describe 'LabelDispatcherCLI' do let(:dispatcher_path) { File.expand_path('../../label-dispatcher/bin/dispatcher', __dir__) } @@ -67,6 +68,25 @@ def run_dispatcher(*args, env: {}) expect(stdout).to include('Label Dispatch Results') end + it 'labels a valid service and not a directory whose name is not a service' do + git = ->(*args) { Open3.capture3('git', '-C', @root, '-c', 'user.name=t', '-c', 'user.email=t@example.com', *args) } + %w[apps/a.b apps/api apps/all].each { |path| FileUtils.mkdir_p(File.join(@root, path)) } + %w[apps/a.b/main.rb apps/api/main.rb apps/all/main.rb].each { |path| File.write(File.join(@root, path), "1\n") } + git.call('add', '-A') + git.call('commit', '-q', '-m', 'base') + %w[apps/a.b/main.rb apps/api/main.rb].each { |path| File.write(File.join(@root, path), "2\n") } + git.call('add', '-A') + git.call('commit', '-q', '-m', 'change') + output_path = File.join(@root, 'github-output') + + stdout, stderr, status = run_dispatcher('test', '--base-ref=HEAD~1', '--head-ref=HEAD', env: { 'GITHUB_ACTIONS' => '1', 'GITHUB_OUTPUT' => output_path }) + + expect(status.exitstatus).to eq(0), stderr + expect(stdout).to include('Deploy Labels: deploy:api').and(include('apps/a.b/main.rb')) + expect(stdout).not_to include('deploy:a.b') + expect(File.read(output_path).lines.map(&:strip)).to include('services-detected=["api"]', 'has-changes=true') + end + it 'exits 1 when test cannot diff the base ref' do stdout, _stderr, status = run_dispatcher('test', '--base-ref', 'missing-ref') expect(status.exitstatus).to eq(1) diff --git a/action-scripts/spec/label-resolver/cli_spec.rb b/action-scripts/spec/label-resolver/cli_spec.rb index ed85f6e..b7cad6e 100644 --- a/action-scripts/spec/label-resolver/cli_spec.rb +++ b/action-scripts/spec/label-resolver/cli_spec.rb @@ -131,20 +131,27 @@ def run_resolver(*args, env: {}) outputs = File.read(output_path).lines.map(&:strip) expect(outputs[0]).to eq('targets=[{"service":"demo","environment":null,"stack":"container","working_directory":"apps/demo"},{"service":"demo","environment":"develop","stack":"kustomize","working_directory":"apps/demo/overlays/develop"}]') expect(outputs[1]).to eq('has-targets=true') - expect(outputs[2]).to start_with('candidates=[{"target":{"service":"demo"') + expect(outputs.length).to eq(2) expect(stdout).to include('Service: demo') end - it 'resolves every service for all' do + it 'writes empty outputs for all when no directory is named all' do _stdout, stderr, status = run_resolver('targets', 'all', env: actions_env) expect(status.exitstatus).to eq(0), stderr - expect(JSON.parse(File.read(output_path)[/^targets=(.*)$/, 1]).map { |item| item['service'] }).to eq(%w[api demo demo]) + expect(File.read(output_path).lines.map(&:strip)).to eq(['targets=[]', 'has-targets=false']) + end + + it 'resolves only the directory named all for all' do + FileUtils.mkdir_p(File.join(@root, 'apps/all')) + _stdout, stderr, status = run_resolver('targets', 'all', env: actions_env) + expect(status.exitstatus).to eq(0), stderr + expect(File.read(output_path).lines.map(&:strip)).to eq(['targets=[{"service":"all","environment":null,"stack":"container","working_directory":"apps/all"}]', 'has-targets=true']) end it 'writes empty outputs for an unknown service' do _stdout, stderr, status = run_resolver('targets', 'missing', env: actions_env) expect(status.exitstatus).to eq(0), stderr - expect(File.read(output_path).lines.map(&:strip)).to eq(['targets=[]', 'has-targets=false', 'candidates=[]']) + expect(File.read(output_path).lines.map(&:strip)).to eq(['targets=[]', 'has-targets=false']) end it 'exits 1 for an environment outside the union of declared and discovered environments' do @@ -154,10 +161,14 @@ def run_resolver(*args, env: {}) expect(File.exist?(output_path)).to be(false) end - it 'exits 1 for a service name outside [a-zA-Z0-9_-]+' do - stdout, _stderr, status = run_resolver('targets', 'demo/aws', env: actions_env) - expect(status.exitstatus).to eq(1) - expect(stdout).to include('::error::', 'demo/aws') + ['demo/aws', 'a.b'].each do |name| + it "exits 1 for the service name #{name} with a message naming the format" do + FileUtils.mkdir_p(File.join(@root, 'apps/a.b')) + stdout, _stderr, status = run_resolver('targets', name, env: actions_env) + expect(status.exitstatus).to eq(1) + expect(stdout).to include("::error::Invalid service name: \"#{name}\" (use a name matching [a-zA-Z0-9_-]+)") + expect(File.exist?(output_path)).to be(false) + end end it 'exits 1 when targets is called without a service' do diff --git a/action-scripts/spec/label-resolver/controllers/label_resolver_controller_spec.rb b/action-scripts/spec/label-resolver/controllers/label_resolver_controller_spec.rb index b9986c7..8859baa 100644 --- a/action-scripts/spec/label-resolver/controllers/label_resolver_controller_spec.rb +++ b/action-scripts/spec/label-resolver/controllers/label_resolver_controller_spec.rb @@ -51,18 +51,16 @@ describe '#targets' do let(:stack) { Entities::WorkflowConfigParser.parse('services' => [{ 'id' => 'app', 'roots' => ['{service}'], 'stacks' => [{ 'id' => 'aws', 'path' => 'aws' }] }]).stacks.first } - let(:candidate) { Entities::TargetCandidate.new(stack: stack, service: 'demo', environment: nil, root: 'demo', working_directory: 'demo/aws', captures: {}) } - let(:service_targets) { Entities::ServiceTargets.new(targets: [candidate.to_deployment_target], candidates: [candidate.to_listed_target]) } + let(:candidate) { Entities::TargetCandidate.new(stack: stack, service: 'demo', environment: nil, working_directory: 'demo/aws', captures: {}) } + let(:targets) { [candidate.to_deployment_target] } - it 'presents the targets and candidates the use case lists' do - allow(list_service_targets).to receive(:execute).with(service: 'demo', environments: ['develop']).and_return(Entities::Result.success(service_targets)) + it 'presents the targets the use case lists' do + allow(list_service_targets).to receive(:execute).with(service: 'demo', environments: ['develop']).and_return(Entities::Result.success(targets)) result = controller.targets(service: 'demo', environments: ['develop']) - expect(result.value).to eq(service_targets) - expect(presenter).to have_received(:present_service_targets).with( - service: 'demo', target_environments: ['develop'], targets: service_targets.targets, candidates: service_targets.candidates - ) + expect(result.value).to eq(targets) + expect(presenter).to have_received(:present_service_targets).with(service: 'demo', target_environments: ['develop'], targets: targets) end it 'presents the failure and skips the targets' do @@ -76,14 +74,14 @@ end it 'does not rescue presenter errors' do - allow(list_service_targets).to receive(:execute).and_return(Entities::Result.success(service_targets)) + allow(list_service_targets).to receive(:execute).and_return(Entities::Result.success(targets)) allow(presenter).to receive(:present_service_targets).and_raise(RuntimeError, 'GITHUB_OUTPUT is not set') expect { controller.targets(service: 'demo', environments: []) }.to raise_error(RuntimeError, 'GITHUB_OUTPUT is not set') end it 'does not build the label reader' do - allow(list_service_targets).to receive(:execute).and_return(Entities::Result.success(service_targets)) + allow(list_service_targets).to receive(:execute).and_return(Entities::Result.success(targets)) controller = described_class.new(get_labels: -> { raise 'label reader built' }, generate_matrix: generate_matrix, list_service_targets: list_service_targets, presenter: presenter) expect(controller.targets(service: 'demo', environments: [])).to be_success diff --git a/action-scripts/spec/label-resolver/use_cases/generate_matrix_spec.rb b/action-scripts/spec/label-resolver/use_cases/generate_matrix_spec.rb index a925e43..7ceaf38 100644 --- a/action-scripts/spec/label-resolver/use_cases/generate_matrix_spec.rb +++ b/action-scripts/spec/label-resolver/use_cases/generate_matrix_spec.rb @@ -146,7 +146,7 @@ def items(**options) { 'id' => 'kubernetes', 'path' => 'overlays/{environment}' } ]) %w[dystopia/demo/aws/production dystopia/demo/overlays/production dystopia/api/overlays/preview].each { |path| directory(path) } - rows = items(labels: ['deploy:all']) + rows = items(labels: ['deploy:demo', 'deploy:api']) expect(rows.map { |row| [row[:service], row[:stack], row[:environment]] }).to contain_exactly( ['demo', 'container', nil], ['api', 'container', nil], ['demo', 'kubernetes', 'production'], ['api', 'kubernetes', 'preview'] ) @@ -193,21 +193,36 @@ def items(**options) expect(items(environments: nil)).to eq([{ service: 'demo', environment: nil, stack: 'container', working_directory: 'dystopia/demo', type: 'container', repository: 'registry.example.com/app' }]) end - it 'discovers unregistered services for all labels and deduplicates overlapping labels' do - directory('dystopia/demo/aws/develop'); directory('dystopia/api/aws/develop'); directory('dystopia/.hidden/aws/develop') - rows = items(labels: ['deploy:all', 'deploy:demo', 'deploy:all']) - expect(rows.length).to eq(4) - expect(rows.map { |row| row[:service] }.uniq).to contain_exactly('demo', 'api') + it 'deduplicates overlapping labels and reads no other service' do + directory('dystopia/demo/aws/develop'); directory('dystopia/api/aws/develop') + rows = items(labels: ['deploy:demo', 'deploy:demo']) + expect(rows.length).to eq(2) + expect(rows.map { |row| row[:service] }.uniq).to eq(['demo']) + end + + it 'targets only the directory named all for an all label' do + directory('dystopia/all/aws/develop'); directory('dystopia/demo/aws/develop') + expect(items(labels: ['deploy:all']).map { |row| row[:service] }.uniq).to eq(['all']) + end + + it 'targets nothing for an all label when no directory is named all' do + directory('dystopia/demo/aws/develop') + expect(items(labels: ['deploy:all'])).to eq([]) + end + + it 'gives no targets for a label that fails the service name definition' do + directory('dystopia/a.b/aws/develop'); directory('dystopia/api/aws/develop') + expect(items(labels: ['deploy:a.b', 'deploy:api']).map { |row| row[:service] }.uniq).to eq(['api']) + expect(items(labels: ['deploy:a.b'])).to eq([]) end { %w[deploy:all extra:all] => ['deploy:all'], - %w[deploy:demo extra:demo] => ['deploy:demo'], - %w[deploy:demo extra:all] => ['deploy:all'] + %w[deploy:demo extra:demo] => ['deploy:demo'] }.each do |labels, equivalent| it "deduplicates targets of #{labels.join(' and ')} across prefixes" do config_hash['labels'] = { 'extra_prefixes' => ['extra:'] } - directory('dystopia/demo/aws/develop'); directory('dystopia/api/aws/develop') + directory('dystopia/demo/aws/develop'); directory('dystopia/all/aws/develop'); directory('dystopia/api/aws/develop') expected = items(labels: equivalent) rows = items(labels: labels) expect(rows).to eq(expected) @@ -275,7 +290,7 @@ def items(**options) it 'returns failures without partial targets for enumeration errors' do directory('dystopia/demo/aws/develop') allow(directory_scanner).to receive(:resolve).and_call_original - allow(directory_scanner).to receive(:resolve).with(pattern: 'system-components/{service}/aws/{environment}', values: { 'service' => 'demo', 'environment' => 'develop' }, skip: Entities::Service.method(:hidden_captures?)).and_raise(Errno::EACCES) + allow(directory_scanner).to receive(:resolve).with(pattern: 'system-components/{service}/aws/{environment}', values: { 'service' => 'demo', 'environment' => 'develop' }, skip: Entities::Service.method(:captures_invalid_name?)).and_raise(Errno::EACCES) result = generate expect(result).to be_failure expect(result.error_message).to include('Permission denied') diff --git a/action-scripts/spec/label-resolver/use_cases/list_service_targets_spec.rb b/action-scripts/spec/label-resolver/use_cases/list_service_targets_spec.rb index f54f58a..74e1a1c 100644 --- a/action-scripts/spec/label-resolver/use_cases/list_service_targets_spec.rb +++ b/action-scripts/spec/label-resolver/use_cases/list_service_targets_spec.rb @@ -9,10 +9,10 @@ { 'id' => 'alb', 'path' => 'alb', 'exclude' => [{ 'service' => 'demo' }] } ] }]) end - let(:included) { Entities::TargetCandidate.new(stack: stack_config.stacks[0], service: 'demo', environment: nil, root: 'demo', working_directory: 'demo/aws', captures: {}) } - let(:excluded) { Entities::TargetCandidate.new(stack: stack_config.stacks[1], service: 'demo', environment: nil, root: 'demo', working_directory: 'demo/alb', captures: {}) } + let(:included) { Entities::TargetCandidate.new(stack: stack_config.stacks[0], service: 'demo', environment: nil, working_directory: 'demo/aws', captures: {}) } + let(:excluded) { Entities::TargetCandidate.new(stack: stack_config.stacks[1], service: 'demo', environment: nil, working_directory: 'demo/alb', captures: {}) } let(:unbuildable_stack) { Entities::Stack.new(id: 'aws', service_id: 'app', path: 'aws', paths: ['{service}/aws'], attributes: { 'repository' => 'ghcr.io/{team}/{service}' }, environments: nil, exclude: [{ 'service' => 'demo' }]) } - let(:unbuildable) { Entities::TargetCandidate.new(stack: unbuildable_stack, service: 'demo', environment: nil, root: 'demo', working_directory: 'demo/aws', captures: {}) } + let(:unbuildable) { Entities::TargetCandidate.new(stack: unbuildable_stack, service: 'demo', environment: nil, working_directory: 'demo/aws', captures: {}) } context 'with a stubbed candidate list' do let(:list_candidates) { instance_double(UseCases::Shared::ListCandidates) } @@ -27,42 +27,37 @@ expect(list_candidates).to have_received(:execute).with(services: ['demo'], environments: ['develop']) end - it 'carries the targets that are not excluded and every candidate with its exclusion and root' do + it 'carries the targets that are not excluded' do allow(list_candidates).to receive(:execute).and_return(Entities::Result.success([included, excluded])) - value = use_case.execute(service: 'demo', environments: []).value - - expect(value.targets).to eq([included.to_deployment_target]) - expect(value.candidates).to eq([included.to_listed_target, excluded.to_listed_target]) - expect(value.candidates.map(&:excluded?)).to eq([false, true]) + expect(use_case.execute(service: 'demo', environments: []).value).to eq([included.to_deployment_target]) end it 'carries no targets when every candidate is excluded' do allow(list_candidates).to receive(:execute).and_return(Entities::Result.success([excluded])) - value = use_case.execute(service: 'demo', environments: []).value - - expect(value.targets).to eq([]) - expect(value.candidates).to eq([excluded.to_listed_target]) + expect(use_case.execute(service: 'demo', environments: []).value).to eq([]) end - it 'treats all as every service' do - allow(list_candidates).to receive(:execute).and_return(Entities::Result.success([included])) + it 'looks up all as a service named all' do + allow(list_candidates).to receive(:execute).and_return(Entities::Result.success([])) result = use_case.execute(service: 'all', environments: []) expect(result).to be_success - expect(list_candidates).to have_received(:execute).with(services: nil, environments: []) + expect(list_candidates).to have_received(:execute).with(services: ['all'], environments: []) end - it 'rejects a service name outside [a-zA-Z0-9_-]+ before listing candidates' do - allow(list_candidates).to receive(:execute) + ['demo/aws', 'a.b', '.hidden', "demo\n"].each do |name| + it "rejects the service name #{name.inspect} before listing candidates" do + allow(list_candidates).to receive(:execute) - result = use_case.execute(service: 'demo/aws', environments: []) + result = use_case.execute(service: name, environments: []) - expect(result).to be_failure - expect(result.error_message).to eq('Invalid service name: "demo/aws" (use all or a name matching [a-zA-Z0-9_-]+)') - expect(list_candidates).not_to have_received(:execute) + expect(result).to be_failure + expect(result.error_message).to eq("Invalid service name: #{name.inspect} (use a name matching [a-zA-Z0-9_-]+)") + expect(list_candidates).not_to have_received(:execute) + end end it 'returns the failure of the candidate list as it is' do @@ -125,25 +120,32 @@ it 'lists the targets of a service apart from the excluded candidates' do value = use_case.execute(service: 'demo', environments: ['develop']).value - expect(value.targets.map(&:to_matrix_item)).to eq([ + expect(value.map(&:to_matrix_item)).to eq([ { service: 'demo', environment: nil, stack: 'container', working_directory: 'apps/demo' }, { service: 'demo', environment: 'develop', stack: 'kustomize', working_directory: 'apps/demo/overlays/develop' } ]) - expect(value.candidates.map { |listed| [listed.target.stack, listed.excluded?, listed.root] }).to contain_exactly(['container', false, 'apps/demo'], ['kustomize', false, 'apps/demo'], ['alb', true, 'apps/demo']) end - it 'lists every service for all' do - value = use_case.execute(service: 'all', environments: []).value + it 'lists nothing for all when no directory is named all' do + result = use_case.execute(service: 'all', environments: []) + + expect(result).to be_success + expect(result.value).to eq([]) + end + + it 'lists only the directory named all for all' do + FileUtils.mkdir_p(File.join(@root, 'apps/all')) - expect(value.targets.map(&:service).uniq).to eq(%w[api demo]) + expect(use_case.execute(service: 'all', environments: []).value.map(&:to_matrix_item)).to eq([ + { service: 'all', environment: nil, stack: 'container', working_directory: 'apps/all' } + ]) end it 'lists nothing for an unknown service' do result = use_case.execute(service: 'missing', environments: []) expect(result).to be_success - expect(result.value.targets).to eq([]) - expect(result.value.candidates).to eq([]) + expect(result.value).to eq([]) end it 'fails for an environment outside the union of declared and discovered environments' do diff --git a/action-scripts/spec/shared/entities/change_detector_spec.rb b/action-scripts/spec/shared/entities/change_detector_spec.rb index 9658a8d..75a0334 100644 --- a/action-scripts/spec/shared/entities/change_detector_spec.rb +++ b/action-scripts/spec/shared/entities/change_detector_spec.rb @@ -151,9 +151,26 @@ def services expect(services).to eq(['api']) end - it 'ignores hidden services' do - changed_files.replace(['teams/platform/.hidden/aws/develop/main.tf']) - expect(services).to eq([]) + ['.hidden', 'a.b', 'café', 'a b'].each do |name| + it "gives no service for a directory named #{name.inspect}" do + changed_files.replace(["teams/platform/#{name}/aws/develop/main.tf", "teams/platform/#{name}/README.md"]) + expect(services).to eq([]) + end + end + + it 'detects a valid service beside a directory whose name is not a service' do + changed_files.replace(['teams/platform/a.b/aws/develop/main.tf', 'teams/platform/api/aws/develop/main.tf']) + expect(services).to eq(['api']) + end + + it 'keeps a dotted value of another placeholder' do + changed_files.replace(['teams/.platform.v2/api/aws/develop/main.tf']) + expect(services).to eq(['api']) + end + + it 'treats a directory named all as an ordinary service' do + changed_files.replace(['teams/platform/all/aws/develop/main.tf']) + expect(services).to eq(['all']) end [false, true].each do |reverse| @@ -169,22 +186,4 @@ def services changed_files.replace(['README.md']) expect(services).to eq([]) end - - describe 'candidate roots' do - def candidate_roots - built = nil - allow(Entities::TargetCandidates).to receive(:new).and_wrap_original { |original, *args| built = original.call(*args) } - services - built.to_a.map(&:root) - end - - { 0 => ['.', 'Dockerfile'], 1 => ['aws', 'aws/main.tf'], 2 => ['aws/{environment}', 'aws/develop/main.tf'] }.each do |length, (path, file)| - it "records the matched root for a stack path of length #{length}" do - service['roots'] = ['dystopia/{service}', 'teams/{team}/{service}'] - service['stacks'] = [{ 'id' => 'terragrunt', 'path' => path }] - changed_files.replace(["dystopia/demo/#{file}", "teams/platform/api/#{file}"]) - expect(candidate_roots).to eq(%w[dystopia/demo teams/platform/api]) - end - end - end end diff --git a/action-scripts/spec/shared/entities/deploy_label_spec.rb b/action-scripts/spec/shared/entities/deploy_label_spec.rb index 89a8ec2..d22ab13 100644 --- a/action-scripts/spec/shared/entities/deploy_label_spec.rb +++ b/action-scripts/spec/shared/entities/deploy_label_spec.rb @@ -29,6 +29,20 @@ end end + it 'gives all no special meaning' do + expect(described_class.new('deploy:all')).not_to respond_to(:deploy_all?) + end + + it 'holds no service name pattern of its own' do + expect(described_class.const_defined?(:SERVICE_PATTERN)).to be(false) + end + + it 'accepts exactly the names the service definition accepts' do + ['api', 'all', 'a.b', '.hidden', "demo\n", 'a b'].each do |name| + expect(described_class.new("deploy:#{name}").valid?).to eq(Entities::Service.valid_name?(name)) + end + end + describe '#valid?' do context 'with valid service label' do subject(:label) { described_class.new('deploy:test-service') } @@ -159,32 +173,6 @@ end end - describe '#deploy_all?' do - context 'with deploy:all label' do - subject(:label) { described_class.new('deploy:all') } - - it 'returns true' do - expect(label).to be_deploy_all - end - end - - context 'with service-specific label' do - subject(:label) { described_class.new('deploy:test-service') } - - it 'returns false' do - expect(label).not_to be_deploy_all - end - end - - context 'with invalid label' do - subject(:label) { described_class.new('invalid-label') } - - it 'returns false' do - expect(label).not_to be_deploy_all - end - end - end - describe '#to_s' do context 'with valid label' do subject(:label) { described_class.new('deploy:test-service') } @@ -319,7 +307,7 @@ it 'extracts the service after any configured prefix' do expect(described_class.new('env:prod:api', prefixes: prefixes).service).to eq('api') - expect(described_class.new('manual/all', prefixes: prefixes)).to be_deploy_all + expect(described_class.new('manual/all', prefixes: prefixes).service).to eq('all') end it 'keeps the matched prefix in its string representation' do diff --git a/action-scripts/spec/shared/entities/service_spec.rb b/action-scripts/spec/shared/entities/service_spec.rb index 72a293d..296cdf5 100644 --- a/action-scripts/spec/shared/entities/service_spec.rb +++ b/action-scripts/spec/shared/entities/service_spec.rb @@ -52,29 +52,32 @@ def service(ignore) end end - describe '.hidden_name?' do - it 'treats a name starting with a dot as hidden' do - expect(described_class.hidden_name?('.preview')).to be(true) - expect(described_class.hidden_name?('.')).to be(true) + describe '.valid_name?' do + %w[demo a Demo-1 my_service all 0 - _].each do |name| + it "accepts #{name}" do + expect(described_class.valid_name?(name)).to be(true) + end end - it 'treats other names as visible' do - expect(described_class.hidden_name?('demo')).to be(false) - expect(described_class.hidden_name?('demo.v2')).to be(false) + ['', '.', '..', '.preview', 'a.b', 'demo.v2', 'a/b', 'a b', "demo\n", "\ndemo", 'café', 'deploy:demo', nil].each do |name| + it "rejects #{name.inspect}" do + expect(described_class.valid_name?(name)).to be(false) + end end end - describe '.hidden_captures?' do - it 'treats captures with a hidden service as hidden' do - expect(described_class.hidden_captures?('team' => 'platform', 'service' => '.hidden')).to be(true) + describe '.captures_invalid_name?' do + it 'is true for captures whose service fails the name definition' do + expect(described_class.captures_invalid_name?('team' => 'platform', 'service' => '.hidden')).to be(true) + expect(described_class.captures_invalid_name?('team' => 'platform', 'service' => 'a.b')).to be(true) end - it 'treats captures with a visible service as visible' do - expect(described_class.hidden_captures?('team' => '.platform', 'service' => 'demo')).to be(false) + it 'is false for captures with a valid service' do + expect(described_class.captures_invalid_name?('team' => '.platform', 'service' => 'demo')).to be(false) end - it 'treats captures without a service as visible' do - expect(described_class.hidden_captures?('team' => '.platform')).to be(false) + it 'is false for captures without a service' do + expect(described_class.captures_invalid_name?('team' => '.platform')).to be(false) end end end diff --git a/action-scripts/spec/shared/entities/target_candidate_spec.rb b/action-scripts/spec/shared/entities/target_candidate_spec.rb index ee69d95..3dd848f 100644 --- a/action-scripts/spec/shared/entities/target_candidate_spec.rb +++ b/action-scripts/spec/shared/entities/target_candidate_spec.rb @@ -3,16 +3,15 @@ RSpec.describe Entities::TargetCandidate do def candidate(attributes: {}, exclude: [], captures: {}) stack = Entities::Stack.new(id: 'aws', service_id: 'app', path: 'aws', paths: ['{service}/aws'], attributes: attributes, environments: nil, exclude: exclude) - described_class.new(stack: stack, service: 'demo', environment: nil, root: 'demo', working_directory: 'demo/aws', captures: captures) + described_class.new(stack: stack, service: 'demo', environment: nil, working_directory: 'demo/aws', captures: captures) end describe '#to_listed_target' do - it 'pairs the built target with its exclusion status and root' do + it 'pairs the built target with its exclusion status' do listed = candidate(attributes: { 'repository' => 'ghcr.io/{service}' }, exclude: [{ 'service' => 'demo' }]).to_listed_target expect(listed.target.to_matrix_item).to eq(service: 'demo', environment: nil, stack: 'aws', working_directory: 'demo/aws', repository: 'ghcr.io/demo') expect(listed).to be_excluded - expect(listed.root).to eq('demo') end it 'is not excluded when no condition matches' do diff --git a/action-scripts/spec/shared/entities/target_candidates_spec.rb b/action-scripts/spec/shared/entities/target_candidates_spec.rb index c646f90..377e7eb 100644 --- a/action-scripts/spec/shared/entities/target_candidates_spec.rb +++ b/action-scripts/spec/shared/entities/target_candidates_spec.rb @@ -43,15 +43,6 @@ def add(captures, directory: 'teams/a/demo/aws/develop') expect([sandbox.excluded?, kept.excluded?]).to eq([true, false]) end - it 'records the working directory without the stack path as the root' do - nested = Entities::Stack.new(id: 'nested', service_id: 'app', path: 'aws/{environment}', paths: ['teams/{team}/{service}/aws/{environment}'], attributes: {}, environments: nil, exclude: []) - common = Entities::Stack.new(id: 'common', service_id: 'app', path: '', paths: ['teams/{team}/{service}'], attributes: {}, environments: nil, exclude: []) - candidates.add(stack: stack, service: 'demo', environment: 'develop', working_directory: 'teams/a/demo/develop', captures: { 'team' => 'a' }) - candidates.add(stack: nested, service: 'demo', environment: 'develop', working_directory: 'teams/a/demo/aws/develop', captures: { 'team' => 'a' }) - candidates.add(stack: common, service: 'demo', environment: nil, working_directory: 'teams/a/demo', captures: { 'team' => 'a' }) - expect(candidates.to_a.map(&:root)).to eq(%w[teams/a/demo teams/a/demo teams/a/demo]) - end - it 'builds a deployment target with the attributes of its environment' do add({ 'team' => 'a' }) expect(candidates.to_a.first.to_deployment_target.to_matrix_item).to eq( diff --git a/action-scripts/spec/shared/entities/workflow_config_parser_spec.rb b/action-scripts/spec/shared/entities/workflow_config_parser_spec.rb index e9ae4d0..7cbae63 100644 --- a/action-scripts/spec/shared/entities/workflow_config_parser_spec.rb +++ b/action-scripts/spec/shared/entities/workflow_config_parser_spec.rb @@ -466,13 +466,31 @@ end end - [nil, 1, [], '', 'a/b', '.', '..', '.hidden'].each do |value| + [nil, 1, [], '', 'a/b', '.', '..', '.hidden', 'a.b', 'a b', "demo\n"].each do |value| it "rejects invalid service conditions #{value.inspect}" do stack['exclude'] = [{ 'service' => value }] expect { config }.to raise_error(ArgumentError, /services\[0\]\.stacks\[0\]\.exclude\[0\]\.service/) end end + it 'names the service name format for a service condition that fails it' do + stack['exclude'] = [{ 'service' => 'a.b' }] + expect { config }.to raise_error(ArgumentError, 'Configuration validation failed: services[0].stacks[0].exclude[0].service: service must match [a-zA-Z0-9_-]+') + end + + %w[demo all my_service Demo-1].each do |value| + it "accepts the service condition #{value}" do + stack['exclude'] = [{ 'service' => value }] + expect { config }.not_to raise_error + end + end + + it 'keeps dotted values for other placeholder conditions' do + service['roots'] = ['teams/{team}/{service}'] + stack['exclude'] = [{ 'team' => 'a.b' }] + expect { config }.not_to raise_error + end + [nil, 'preview', 1].each do |value| it "rejects undeclared environment conditions #{value.inspect}" do stack['exclude'] = [{ 'environment' => value }] diff --git a/action-scripts/spec/shared/interfaces/presenters/console_presenter_spec.rb b/action-scripts/spec/shared/interfaces/presenters/console_presenter_spec.rb index a710f1e..4bcf3c6 100644 --- a/action-scripts/spec/shared/interfaces/presenters/console_presenter_spec.rb +++ b/action-scripts/spec/shared/interfaces/presenters/console_presenter_spec.rb @@ -38,15 +38,15 @@ it 'colors the heading of the service targets text' do stack = Entities::WorkflowConfigParser.parse('services' => [{ 'id' => 'app', 'roots' => ['{service}'], 'stacks' => [{ 'id' => 'aws', 'path' => 'aws' }] }]).stacks.first - candidate = Entities::TargetCandidate.new(stack: stack, service: 'demo', environment: nil, root: 'demo', working_directory: 'demo/aws', captures: {}).to_listed_target - lines = Interfaces::Presenters::TextFormat.service_targets(service: 'demo', target_environments: [], targets: [candidate.target], candidates: [candidate]) + target = Entities::TargetCandidate.new(stack: stack, service: 'demo', environment: nil, working_directory: 'demo/aws', captures: {}).to_deployment_target + lines = Interfaces::Presenters::TextFormat.service_targets(service: 'demo', target_environments: [], targets: [target]) expect do - described_class.new.present_service_targets(service: 'demo', target_environments: [], targets: [candidate.target], candidates: [candidate]) + described_class.new.present_service_targets(service: 'demo', target_environments: [], targets: [target]) end.to output("#{lines.first.colorize(:green)}\n#{lines.drop(1).map { |line| "#{line}\n" }.join}").to_stdout end it 'accepts only the service target fields' do - expect(described_class.instance_method(:present_service_targets).parameters.map(&:last)).to eq(%i[service target_environments targets candidates]) + expect(described_class.instance_method(:present_service_targets).parameters.map(&:last)).to eq(%i[service target_environments targets]) end it 'colors the validation heading by the result' do @@ -64,7 +64,7 @@ it 'shows complete diagnostic targets and their exclusion status' do stack = Entities::WorkflowConfigParser.parse('services' => [{ 'id' => 'app', 'roots' => ['teams/{team}/{service}'], 'stacks' => [{ 'id' => 'aws', 'path' => 'aws', 'attributes' => { 'token' => nil }, 'exclude' => [{ 'team' => 'platform' }] }] }]).stacks.first - candidate = Entities::TargetCandidate.new(stack: stack, service: 'demo', environment: nil, root: 'teams/platform/demo', working_directory: 'teams/platform/demo/aws', captures: { 'team' => 'platform' }).to_listed_target + candidate = Entities::TargetCandidate.new(stack: stack, service: 'demo', environment: nil, working_directory: 'teams/platform/demo/aws', captures: { 'team' => 'platform' }).to_listed_target expect do described_class.new.present_service_test_result(service_name: 'demo', candidates: [candidate]) end.to output(/stack: "aws".*working_directory: "teams\/platform\/demo\/aws".*token: nil.*team: "platform".*excluded: true/m).to_stdout diff --git a/action-scripts/spec/shared/interfaces/presenters/github_actions_presenter_spec.rb b/action-scripts/spec/shared/interfaces/presenters/github_actions_presenter_spec.rb index 8332764..5f766de 100644 --- a/action-scripts/spec/shared/interfaces/presenters/github_actions_presenter_spec.rb +++ b/action-scripts/spec/shared/interfaces/presenters/github_actions_presenter_spec.rb @@ -48,46 +48,37 @@ def dispatch(presenter) { 'id' => 'alb', 'path' => 'alb', 'exclude' => [{ 'service' => 'demo' }] } ] }]) end - let(:included) { Entities::TargetCandidate.new(stack: stack_config.stacks[0], service: 'demo', environment: nil, root: 'demo', working_directory: 'demo/aws', captures: {}).to_listed_target } - let(:excluded) { Entities::TargetCandidate.new(stack: stack_config.stacks[1], service: 'demo', environment: nil, root: 'aws/alb', working_directory: 'aws/alb/alb', captures: {}).to_listed_target } - let(:included_item) { '{"service":"demo","environment":null,"stack":"aws","working_directory":"demo/aws"}' } + let(:target) { Entities::TargetCandidate.new(stack: stack_config.stacks[0], service: 'demo', environment: nil, working_directory: 'demo/aws', captures: {}).to_deployment_target } + let(:excluded_target) { Entities::TargetCandidate.new(stack: stack_config.stacks[1], service: 'demo', environment: nil, working_directory: 'aws/alb/alb', captures: {}).to_deployment_target } + let(:target_item) { '{"service":"demo","environment":null,"stack":"aws","working_directory":"demo/aws"}' } let(:excluded_item) { '{"service":"demo","environment":null,"stack":"alb","working_directory":"aws/alb/alb"}' } - it 'writes the given targets, their presence, and every candidate with its exclusion and root' do - expect { presenter.present_service_targets(service: 'demo', target_environments: [], targets: [included.target], candidates: [included, excluded]) }.to output.to_stdout - expect(File.read(@output_path).lines.map(&:strip)).to eq([ - "targets=[#{included_item}]", - 'has-targets=true', - "candidates=[{\"target\":#{included_item},\"excluded\":false,\"root\":\"demo\"},{\"target\":#{excluded_item},\"excluded\":true,\"root\":\"aws/alb\"}]" - ]) - end - - it 'writes false when no target is given' do - expect { presenter.present_service_targets(service: 'demo', target_environments: [], targets: [], candidates: [excluded]) }.to output.to_stdout - expect(File.read(@output_path).lines.map(&:strip).first(2)).to eq(['targets=[]', 'has-targets=false']) + it 'writes only the given targets and their presence' do + expect { presenter.present_service_targets(service: 'demo', target_environments: [], targets: [target]) }.to output.to_stdout + expect(File.read(@output_path).lines.map(&:strip)).to eq(["targets=[#{target_item}]", 'has-targets=true']) end it 'does not filter the given targets by exclusion' do - expect { presenter.present_service_targets(service: 'demo', target_environments: [], targets: [excluded.target], candidates: [excluded]) }.to output.to_stdout - expect(File.read(@output_path).lines.map(&:strip).first(2)).to eq(["targets=[#{excluded_item}]", 'has-targets=true']) + expect { presenter.present_service_targets(service: 'demo', target_environments: [], targets: [excluded_target]) }.to output.to_stdout + expect(File.read(@output_path).lines.map(&:strip)).to eq(["targets=[#{excluded_item}]", 'has-targets=true']) end - it 'writes empty outputs when no candidates exist' do - expect { presenter.present_service_targets(service: 'missing', target_environments: [], targets: [], candidates: []) }.to output.to_stdout - expect(File.read(@output_path).lines.map(&:strip)).to eq(['targets=[]', 'has-targets=false', 'candidates=[]']) + it 'writes empty outputs when no target exists' do + expect { presenter.present_service_targets(service: 'missing', target_environments: [], targets: []) }.to output.to_stdout + expect(File.read(@output_path).lines.map(&:strip)).to eq(['targets=[]', 'has-targets=false']) end it 'prints the shared text without color' do - expected = Interfaces::Presenters::TextFormat.service_targets(service: 'demo', target_environments: [], targets: [included.target], candidates: [included]).map { |line| "#{line}\n" }.join - expect { presenter.present_service_targets(service: 'demo', target_environments: [], targets: [included.target], candidates: [included]) }.to output(expected).to_stdout + expected = Interfaces::Presenters::TextFormat.service_targets(service: 'demo', target_environments: [], targets: [target]).map { |line| "#{line}\n" }.join + expect { presenter.present_service_targets(service: 'demo', target_environments: [], targets: [target]) }.to output(expected).to_stdout end it 'accepts only the service target fields' do - expect(described_class.instance_method(:present_service_targets).parameters.map(&:last)).to eq(%i[service target_environments targets candidates]) + expect(described_class.instance_method(:present_service_targets).parameters.map(&:last)).to eq(%i[service target_environments targets]) end it 'raises when GITHUB_OUTPUT is not set' do - expect { described_class.new(output_path: nil).present_service_targets(service: 'demo', target_environments: [], targets: [], candidates: []) }.to raise_error(RuntimeError, 'GITHUB_OUTPUT is not set') + expect { described_class.new(output_path: nil).present_service_targets(service: 'demo', target_environments: [], targets: []) }.to raise_error(RuntimeError, 'GITHUB_OUTPUT is not set') end end @@ -146,7 +137,7 @@ def dispatch(presenter) it 'shows complete diagnostic targets and their exclusion status' do stack = Entities::WorkflowConfigParser.parse('services' => [{ 'id' => 'app', 'roots' => ['teams/{team}/{service}'], 'stacks' => [{ 'id' => 'aws', 'path' => 'aws', 'attributes' => { 'token' => nil }, 'exclude' => [{ 'team' => 'platform' }] }] }]).stacks.first - candidate = Entities::TargetCandidate.new(stack: stack, service: 'demo', environment: nil, root: 'teams/platform/demo', working_directory: 'teams/platform/demo/aws', captures: { 'team' => 'platform' }).to_listed_target + candidate = Entities::TargetCandidate.new(stack: stack, service: 'demo', environment: nil, working_directory: 'teams/platform/demo/aws', captures: { 'team' => 'platform' }).to_listed_target expect do presenter.present_service_test_result(service_name: 'demo', candidates: [candidate]) end.to output(/stack: "aws".*working_directory: "teams\/platform\/demo\/aws".*token: nil.*team: "platform".*excluded: true/m).to_stdout diff --git a/action-scripts/spec/shared/interfaces/presenters/text_format_spec.rb b/action-scripts/spec/shared/interfaces/presenters/text_format_spec.rb index b01d827..96dfaf8 100644 --- a/action-scripts/spec/shared/interfaces/presenters/text_format_spec.rb +++ b/action-scripts/spec/shared/interfaces/presenters/text_format_spec.rb @@ -49,7 +49,7 @@ describe '.service_test_result' do it 'shows every matrix field of each target and its exclusion status' do stack = Entities::WorkflowConfigParser.parse('services' => [{ 'id' => 'app', 'roots' => ['{service}'], 'stacks' => [{ 'id' => 'aws', 'path' => 'aws' }] }]).stacks.first - candidate = Entities::TargetCandidate.new(stack: stack, service: 'demo', environment: nil, root: 'demo', working_directory: 'demo/aws', captures: {}).to_listed_target + candidate = Entities::TargetCandidate.new(stack: stack, service: 'demo', environment: nil, working_directory: 'demo/aws', captures: {}).to_listed_target lines = described_class.service_test_result(service_name: 'demo', candidates: [candidate]) expect(lines).to eq(['Service Configuration Test', 'Service: demo', 'Target:', ' service: "demo"', ' environment: nil', ' stack: "aws"', ' working_directory: "demo/aws"', ' excluded: false']) end @@ -62,27 +62,25 @@ { 'id' => 'container', 'path' => '.' } ] }]) end - let(:candidates) do + let(:targets) do [ - Entities::TargetCandidate.new(stack: config.stacks[0], service: 'demo', environment: 'develop', root: 'demo', working_directory: 'demo/overlays/develop', captures: {}), - Entities::TargetCandidate.new(stack: config.stacks[0], service: 'demo', environment: 'production', root: 'demo', working_directory: 'demo/overlays/production', captures: {}), - Entities::TargetCandidate.new(stack: config.stacks[1], service: 'demo', environment: nil, root: 'demo', working_directory: 'demo', captures: {}) - ].map(&:to_listed_target) + Entities::TargetCandidate.new(stack: config.stacks[0], service: 'demo', environment: 'develop', working_directory: 'demo/overlays/develop', captures: {}), + Entities::TargetCandidate.new(stack: config.stacks[1], service: 'demo', environment: nil, working_directory: 'demo', captures: {}) + ].map(&:to_deployment_target) end - it 'shows the service, the environments, and each target with its root and exclusion status' do - lines = described_class.service_targets(service: 'demo', target_environments: %w[develop production], targets: candidates.reject(&:excluded?).map(&:target), candidates: candidates) + it 'shows the service, the environments, and each target' do + lines = described_class.service_targets(service: 'demo', target_environments: %w[develop production], targets: targets) expect(lines).to eq([ - '🎯 Service Targets', 'Service: demo', 'Target Environments: develop, production', 'Targets: 2 of 3 candidates', - '', ' demo:kustomize (develop) -> demo/overlays/develop', ' root: demo', ' excluded: false', - '', ' demo:kustomize (production) -> demo/overlays/production', ' root: demo', ' excluded: true', - '', ' demo:container -> demo', ' root: demo', ' excluded: false' + '🎯 Service Targets', 'Service: demo', 'Target Environments: develop, production', 'Targets: 2', + '', ' demo:kustomize (develop) -> demo/overlays/develop', + '', ' demo:container -> demo' ]) end it 'names a single target environment and omits the line when none is requested' do - expect(described_class.service_targets(service: 'all', target_environments: ['develop'], targets: [], candidates: [])).to include('Target Environment: develop') - expect(described_class.service_targets(service: 'all', target_environments: [], targets: [], candidates: [])).to eq(['🎯 Service Targets', 'Service: all', 'Targets: 0 of 0 candidates']) + expect(described_class.service_targets(service: 'demo', target_environments: ['develop'], targets: [])).to include('Target Environment: develop') + expect(described_class.service_targets(service: 'demo', target_environments: [], targets: [])).to eq(['🎯 Service Targets', 'Service: demo', 'Targets: 0']) end end end diff --git a/action-scripts/spec/shared/use_cases/enumerate_targets_spec.rb b/action-scripts/spec/shared/use_cases/enumerate_targets_spec.rb index 4ef0d89..2e3b81f 100644 --- a/action-scripts/spec/shared/use_cases/enumerate_targets_spec.rb +++ b/action-scripts/spec/shared/use_cases/enumerate_targets_spec.rb @@ -23,7 +23,7 @@ def directory(path) FileUtils.mkdir_p(File.join(@root, path)) end - def enumerate(services: nil, environments: %w[develop production]) + def enumerate(services: ['demo'], environments: %w[develop production]) result = use_case.execute(config: config, services: services, environments: environments) expect(result).to be_success result.value @@ -43,42 +43,30 @@ def enumerate(services: nil, environments: %w[develop production]) expect(candidates.map(&:service).uniq).to eq(['demo']) end - it 'enumerates every service when services are nil' do - %w[dystopia/demo dystopia/api].each { |path| directory(path) } - expect(enumerate.map(&:service)).to eq(%w[api demo]) - end - - context 'with team roots' do - let(:config) do - Entities::WorkflowConfigParser.parse('services' => [{ 'id' => 'app', 'roots' => ['teams/{team}/{service}'], 'stacks' => [ - { 'id' => 'kubernetes', 'path' => '{environment}' }, - { 'id' => 'terragrunt', 'path' => 'aws/{environment}' }, - { 'id' => 'container', 'path' => '.' } - ] }]) + %w[.hidden a.b].each do |name| + it "gives no candidate for a directory named #{name}" do + directory("dystopia/#{name}/overlays/develop") + expect(enumerate(services: [name])).to eq([]) end - it 'records the root directory of each candidate' do - %w[teams/payments/api/develop teams/payments/api/aws/develop].each { |path| directory(path) } - expect(enumerate(environments: ['develop']).map { |candidate| [candidate.stack.id, candidate.working_directory, candidate.root] }).to contain_exactly( - ['kubernetes', 'teams/payments/api/develop', 'teams/payments/api'], - ['terragrunt', 'teams/payments/api/aws/develop', 'teams/payments/api'], - ['container', 'teams/payments/api', 'teams/payments/api'] - ) + it "does not descend into an unreadable directory named #{name}" do + directory("dystopia/#{name}/overlays/develop") + path = File.join(@root, 'dystopia', name) + File.chmod(0, path) + expect(enumerate(services: [name])).to eq([]) + ensure + File.chmod(0o755, path) if path end end - it 'skips hidden services' do - directory('dystopia/.hidden/overlays/develop') - expect(enumerate).to eq([]) - end - - it 'enumerates targets next to an unreadable hidden service' do + it 'enumerates targets next to a directory whose name is not a service' do directory('dystopia/demo/overlays/develop') - directory('dystopia/.cache/overlays/develop') - path = File.join(@root, 'dystopia/.cache') - File.chmod(0, path) + directory('dystopia/a.b/overlays/develop') expect(enumerate.map(&:working_directory)).to eq(['dystopia/demo/overlays/develop', 'dystopia/demo']) - ensure - File.chmod(0o755, path) if path + end + + it 'enumerates a directory named all as an ordinary service' do + %w[dystopia/all dystopia/demo].each { |path| directory(path) } + expect(enumerate(services: ['all']).map(&:working_directory)).to eq(['dystopia/all']) end end diff --git a/action-scripts/spec/shared/use_cases/list_candidates_spec.rb b/action-scripts/spec/shared/use_cases/list_candidates_spec.rb index 405281a..d1bcc50 100644 --- a/action-scripts/spec/shared/use_cases/list_candidates_spec.rb +++ b/action-scripts/spec/shared/use_cases/list_candidates_spec.rb @@ -30,7 +30,7 @@ def directory(path) FileUtils.mkdir_p(File.join(@root, path)) end - def list(services: nil, environments: nil) + def list(services:, environments: nil) result = use_case.execute(services: services, environments: environments) expect(result).to be_success result.value @@ -43,11 +43,6 @@ def list(services: nil, environments: nil) ]) end - it 'lists every service when services is nil' do - %w[dystopia/demo dystopia/api].each { |path| directory(path) } - expect(list.map(&:service)).to eq(%w[api demo]) - end - it 'limits candidates to the given services and environments' do %w[dystopia/demo/overlays/develop dystopia/demo/overlays/production dystopia/api/overlays/develop].each { |path| directory(path) } candidates = list(services: ['demo'], environments: ['develop']) diff --git a/action-scripts/spec/shared/use_cases/resolve_environments_spec.rb b/action-scripts/spec/shared/use_cases/resolve_environments_spec.rb index af3fc0f..0f7734f 100644 --- a/action-scripts/spec/shared/use_cases/resolve_environments_spec.rb +++ b/action-scripts/spec/shared/use_cases/resolve_environments_spec.rb @@ -71,22 +71,24 @@ def resolve(requested, with: config) expect(resolve(nil, with: combined).value).to eq(%w[production develop .preview sandbox]) end - it 'ignores environments found only under hidden services' do - hidden = Entities::WorkflowConfigParser.parse('services' => [{ 'id' => 'app', 'roots' => ['dystopia/{service}'], 'stacks' => [{ 'id' => 'kustomize', 'path' => 'overlays/{environment}' }] }]) - directory('dystopia/.hidden/overlays/qa') - expect(resolve(nil, with: hidden).value).not_to include('qa') - expect(resolve(['qa'], with: hidden)).to be_failure - end + %w[.hidden a.b].each do |name| + it "ignores environments found only under a directory named #{name}" do + discovered = Entities::WorkflowConfigParser.parse('services' => [{ 'id' => 'app', 'roots' => ['dystopia/{service}'], 'stacks' => [{ 'id' => 'kustomize', 'path' => 'overlays/{environment}' }] }]) + directory("dystopia/#{name}/overlays/qa") + expect(resolve(nil, with: discovered).value).not_to include('qa') + expect(resolve(['qa'], with: discovered)).to be_failure + end - it 'resolves environments next to an unreadable hidden service' do - discovered = Entities::WorkflowConfigParser.parse('services' => [{ 'id' => 'app', 'roots' => ['dystopia/{service}'], 'stacks' => [{ 'id' => 'kustomize', 'path' => 'overlays/{environment}' }] }]) - directory('dystopia/demo/overlays/develop') - directory('dystopia/.cache/overlays/qa') - path = File.join(@root, 'dystopia/.cache') - File.chmod(0, path) - expect(resolve(nil, with: discovered).value).to eq(['develop']) - ensure - File.chmod(0o755, path) if path + it "resolves environments next to an unreadable directory named #{name}" do + discovered = Entities::WorkflowConfigParser.parse('services' => [{ 'id' => 'app', 'roots' => ['dystopia/{service}'], 'stacks' => [{ 'id' => 'kustomize', 'path' => 'overlays/{environment}' }] }]) + directory('dystopia/demo/overlays/develop') + directory("dystopia/#{name}/overlays/qa") + path = File.join(@root, 'dystopia', name) + File.chmod(0, path) + expect(resolve(nil, with: discovered).value).to eq(['develop']) + ensure + File.chmod(0o755, path) if path + end end it 'returns no environments when inferred directories are absent' do diff --git a/label-resolver/action.yaml b/label-resolver/action.yaml index 17ae3d9..9d31a98 100644 --- a/label-resolver/action.yaml +++ b/label-resolver/action.yaml @@ -17,7 +17,7 @@ inputs: description: 'PR number (from prepare job output or event)' required: false service: - description: 'Service name or all; when set, resolves its targets without a pull request and ignores pr-number' + description: 'Service name matching [a-zA-Z0-9_-]+; when set, resolves its targets without a pull request and ignores pr-number' required: false environments: description: 'Target environments (comma-separated: develop,staging or single: develop)' @@ -38,9 +38,6 @@ outputs: has-targets: description: 'Whether targets found' value: ${{ steps.extract.outputs.has-targets }} - candidates: - description: 'Every candidate target including the excluded ones; set only when service is given' - value: ${{ steps.extract.outputs.candidates }} runs: using: 'composite' From 1a29df0ca16f9eb29ccd36fd3300a4ae211b0789 Mon Sep 17 00:00:00 2001 From: panicboat Date: Sat, 31 Oct 2026 08:47:36 +0900 Subject: [PATCH 2/2] fix: reject an invalid service name in config-manager test config-manager test printed an empty result and exited 0 for a name such as a.b, while resolver targets exited 1 for the same name. Both now apply the service name definition and report it with one message, kept on Entities::Service so the two cannot drift apart. A valid name with no matching directory still exits 0 with an empty result, as it does for targets. Claude-Session: https://claude.ai/code/session_01N1XYRqYewzhQfyJcM9zFjG Signed-off-by: panicboat --- action-scripts/config-manager/README-ja.md | 2 +- action-scripts/config-manager/README.md | 2 +- .../config-manager/use_cases/test_service.rb | 2 ++ .../use_cases/list_service_targets.rb | 4 +--- action-scripts/shared/entities/service.rb | 4 ++++ action-scripts/spec/config-manager/cli_spec.rb | 16 ++++++++++++++++ .../use_cases/test_service_spec.rb | 16 +++++++++++++--- .../spec/shared/entities/service_spec.rb | 6 ++++++ 8 files changed, 44 insertions(+), 8 deletions(-) diff --git a/action-scripts/config-manager/README-ja.md b/action-scripts/config-manager/README-ja.md index 2cb5fe7..ee01575 100644 --- a/action-scripts/config-manager/README-ja.md +++ b/action-scripts/config-manager/README-ja.md @@ -22,7 +22,7 @@ bundle exec ruby config-manager/bin/config-manager test demo bundle exec ruby config-manager/bin/config-manager test demo production ``` -環境を省略すると、定義された環境と探索された環境のすべてに、環境をまたいで共有されるターゲットを加えて調べます。環境を指定すると、その環境と共有ターゲットを調べます。サービスの事前登録は不要で、どのディレクトリにも一致しない名前や[正しいサービス名](#service-names)でない名前を渡すと結果は空になります。各ターゲットは stack の `id`、展開後の属性値、取得したプレースホルダーとともに表示されます。除外されたターゲットも `excluded: true` を付けて表示されるので、デプロイから外れるものを確認できます。未知の環境、不正な設定、ディレクトリを列挙できない場合、同じターゲットで取得値が食い違う場合はエラーになり、終了コード 1 で終わります。 +環境を省略すると、定義された環境と探索された環境のすべてに、環境をまたいで共有されるターゲットを加えて調べます。環境を指定すると、その環境と共有ターゲットを調べます。サービスの事前登録は不要で、正しい名前がどのディレクトリにも一致しないときは結果が空になります。各ターゲットは stack の `id`、展開後の属性値、取得したプレースホルダーとともに表示されます。除外されたターゲットも `excluded: true` を付けて表示されるので、デプロイから外れるものを確認できます。[正しいサービス名](#service-names)でない名前、未知の環境、不正な設定、ディレクトリを列挙できない場合、同じターゲットで取得値が食い違う場合はエラーになり、終了コード 1 で終わります。 元の設定を編集せずに変更を試すときは、`WORKFLOW_CONFIG_PATH` でコピーを指定します。 diff --git a/action-scripts/config-manager/README.md b/action-scripts/config-manager/README.md index d99b276..41f4d25 100644 --- a/action-scripts/config-manager/README.md +++ b/action-scripts/config-manager/README.md @@ -22,7 +22,7 @@ bundle exec ruby config-manager/bin/config-manager test demo bundle exec ruby config-manager/bin/config-manager test demo production ``` -Without an environment it covers all declared and discovered environments plus the targets shared across environments. With an environment it covers that environment plus the shared targets. Services do not need registering, and a name that matches no directory or is not a [valid service name](#service-names) prints an empty result. Each target is printed with its stack `id`, expanded attribute values, and captured placeholders. Excluded targets are printed too, with `excluded: true`, so you can see what is left out of deployment. An unknown environment, an invalid configuration, a directory that cannot be listed, and conflicting captured values for the same target are errors and exit with status 1. +Without an environment it covers all declared and discovered environments plus the targets shared across environments. With an environment it covers that environment plus the shared targets. Services do not need registering, and a valid name that matches no directory prints an empty result. Each target is printed with its stack `id`, expanded attribute values, and captured placeholders. Excluded targets are printed too, with `excluded: true`, so you can see what is left out of deployment. A name that is not a [valid service name](#service-names), an unknown environment, an invalid configuration, a directory that cannot be listed, and conflicting captured values for the same target are errors and exit with status 1. To try a configuration change without editing the original, point `WORKFLOW_CONFIG_PATH` at a copy: diff --git a/action-scripts/config-manager/use_cases/test_service.rb b/action-scripts/config-manager/use_cases/test_service.rb index 1520fc4..b1e3864 100644 --- a/action-scripts/config-manager/use_cases/test_service.rb +++ b/action-scripts/config-manager/use_cases/test_service.rb @@ -6,6 +6,8 @@ def initialize(list_candidates:) end def execute(service_name:, environment:) + return Entities::Result.failure(Entities::Service.invalid_name_message(service_name)) unless Entities::Service.valid_name?(service_name) + candidates = @list_candidates.execute(services: [service_name], environments: environment && [environment]) return candidates if candidates.failure? diff --git a/action-scripts/label-resolver/use_cases/list_service_targets.rb b/action-scripts/label-resolver/use_cases/list_service_targets.rb index 380246c..8079baf 100644 --- a/action-scripts/label-resolver/use_cases/list_service_targets.rb +++ b/action-scripts/label-resolver/use_cases/list_service_targets.rb @@ -6,9 +6,7 @@ def initialize(list_candidates:) end def execute(service:, environments:) - unless Entities::Service.valid_name?(service) - return Entities::Result.failure("Invalid service name: #{service.inspect} (use a name matching #{Entities::Service.name_format})") - end + return Entities::Result.failure(Entities::Service.invalid_name_message(service)) unless Entities::Service.valid_name?(service) candidates = @list_candidates.execute(services: [service], environments: environments) return candidates if candidates.failure? diff --git a/action-scripts/shared/entities/service.rb b/action-scripts/shared/entities/service.rb index 01dc2b7..4b3411e 100644 --- a/action-scripts/shared/entities/service.rb +++ b/action-scripts/shared/entities/service.rb @@ -8,6 +8,10 @@ def self.valid_name?(name) /\A#{name_format}\z/.match?(name) end + def self.invalid_name_message(name) + "Invalid service name: #{name.inspect} (use a name matching #{name_format})" + end + def self.captures_invalid_name?(captures) captures.key?('service') && !valid_name?(captures['service']) end diff --git a/action-scripts/spec/config-manager/cli_spec.rb b/action-scripts/spec/config-manager/cli_spec.rb index 4216683..0cf57b0 100644 --- a/action-scripts/spec/config-manager/cli_spec.rb +++ b/action-scripts/spec/config-manager/cli_spec.rb @@ -69,6 +69,22 @@ def run_config_manager(*args, env: {}) let(:valid_config) { { 'services' => [{ 'id' => 'app', 'roots' => ['apps/{service}'], 'stacks' => [{ 'id' => 'container', 'path' => '.' }] }] }.to_yaml } + it 'exits 1 for test with a name that is not a valid service name' do + write_config(valid_config) + FileUtils.mkdir_p(File.join(@root, 'apps/a.b')) + stdout, _stderr, status = run_config_manager('test', 'a.b') + expect(status.exitstatus).to eq(1) + expect(stdout).to include('Invalid service name: "a.b" (use a name matching [a-zA-Z0-9_-]+)') + expect(stdout).not_to include('Service Configuration Test') + end + + it 'exits 0 with an empty result for test with a valid name that matches no directory' do + write_config(valid_config) + stdout, stderr, status = run_config_manager('test', 'missing') + expect(status.exitstatus).to eq(0), stderr + expect(stdout).to eq("Service Configuration Test\nService: missing\n") + end + it 'exits 1 when the configuration is invalid' do write_config("services: 1\n") stdout, _stderr, status = run_config_manager('validate') diff --git a/action-scripts/spec/config-manager/use_cases/test_service_spec.rb b/action-scripts/spec/config-manager/use_cases/test_service_spec.rb index 80bcaa1..0a388b3 100644 --- a/action-scripts/spec/config-manager/use_cases/test_service_spec.rb +++ b/action-scripts/spec/config-manager/use_cases/test_service_spec.rb @@ -123,9 +123,19 @@ def candidates(service: 'demo', environment: nil) expect(result.error_message).to eq('Failed to test service configuration: Permission denied') end - it 'lists nothing for a directory whose name is not a service' do - directory('dystopia/a.b/aws/develop') - expect(candidates(service: 'a.b')).to eq([]) + ['a.b', '.hidden', 'demo/aws', "demo\n"].each do |name| + it "fails for the service name #{name.inspect} before loading the configuration" do + directory('dystopia/a.b/aws/develop') + result = use_case.execute(service_name: name, environment: nil) + expect(result).to be_failure + expect(result.error_message).to eq("Invalid service name: #{name.inspect} (use a name matching [a-zA-Z0-9_-]+)") + expect(config_client).not_to have_received(:load_workflow_config) + end + end + + it 'lists nothing for a valid name with no matching directory' do + directory('dystopia/demo/aws/develop') + expect(candidates(service: 'missing')).to eq([]) end it 'fails for an attribute that references a placeholder with no value' do diff --git a/action-scripts/spec/shared/entities/service_spec.rb b/action-scripts/spec/shared/entities/service_spec.rb index 296cdf5..c13795c 100644 --- a/action-scripts/spec/shared/entities/service_spec.rb +++ b/action-scripts/spec/shared/entities/service_spec.rb @@ -66,6 +66,12 @@ def service(ignore) end end + describe '.invalid_name_message' do + it 'quotes the name and states the format' do + expect(described_class.invalid_name_message('a.b')).to eq('Invalid service name: "a.b" (use a name matching [a-zA-Z0-9_-]+)') + end + end + describe '.captures_invalid_name?' do it 'is true for captures whose service fails the name definition' do expect(described_class.captures_invalid_name?('team' => 'platform', 'service' => '.hidden')).to be(true)