Azure / Azure/apiops-cli

Dry-run should perform client-side validation to catch failures that actual publish would hit

オープン
#147 コメント 2 件 リアクション 0 件 担当者 0 名 GitHub で見る
Enhancement P2
主要言語
TypeScript
スター
26
フォーク
9
平均マージ
1日 3時間
マージ済み PR(30日)
20

説明

## Problem

The current `--dry-run` implementation only checks whether resources exist in the target APIM instance (GET to determine PUT vs create). It does **not** validate the merged payload, so dry-run reports success for resources that will fail during actual publish.

### Evidence

Comparing `tests/test-overrides/extract-dryrun.log` (dry-run) vs `tests/test-overrides/extract.log` (actual publish), dry-run reported all resources as `PUT` while actual publish hit multiple `HTTP 400` validation errors:

| Resource | Publish Error | Dry-run result |
|---|---|---|
| `namedvalue/src-nv-plain` | `ValidationError` — display name contains spaces/parens (invalid characters) | ✅ PUT |
| `namedvalue/src-nv-secret` | `ValidationError` — same display name issue | ✅ PUT |
| `namedvalue/src-nv-keyvault` | Managed identity `clientId` not found on target | ✅ PUT |
| `backend/src-backend-circuit-breaker` | `failureCondition must not be empty` | ✅ PUT |
| `backend/src-backend-function` | `resourceId` — "Value should represent absolute http URL" | ✅ PUT |
| `backend/src-backend-logicapp` | `resourceId` — same URL format issue | ✅ PUT |
| `logger/src-logger-appinsights` | Invalid instrumentation key | ✅ PUT |
| `logger/src-logger-eventhub` | HTTP 502/500 (connection string validation) | ✅ PUT |
| `diagnostic/applicationinsights` | Invalid `loggerId` reference | ✅ PUT |
| `diagnostic/azuremonitor` | Invalid `loggerId` reference | ✅ PUT |

## Proposed Validations

The dry-run reporter should add client-side validation **before** reporting PUT. These checks should produce `[DRY RUN] WARN` lines without blocking the rest of the report. Suggested checks:

### 1. Named Value display name format
ARM rejects display names with spaces, parentheses, and other special characters. Validate against the pattern: `^[A-Za-z0-9._-]+$`.

### 2. Resource ID format validation
`resourceId` fields on backends must be valid ARM resource IDs (starting with `/subscriptions/`). Currently, override values like bare paths pass through unchecked.

### 3. URL format validation
Backend `url` fields and API `serviceUrl` fields should be valid URLs with a scheme (`https://`, `wss://`, etc.).

### 4. Cross-resource reference validation (loggerId)
Diagnostics reference loggers by `loggerId`. Dry-run should verify the referenced logger exists either in the artifact set or in the target APIM instance. Same applies after override merging — if an override changes `loggerId`, validate the new value.

### 5. Circuit breaker rule completeness
ARM requires `failureCondition` when `circuitBreaker.rules` is specified. Validate that overrides don't produce partial circuit breaker configs.

### 6. Key Vault identity pre-check
When a named value references a Key Vault with `identityClientId`, verify that the identity is present on the target APIM service (the service's managed identities are available via the service resource GET).

### 7. Logger credential validation
Warn when logger credentials contain placeholder values (e.g., `production-instrumentation-key-00000000`) that are unlikely to be valid GUIDs or connection strings.

### 8. Override key validity
Warn when override properties don't match any known field in the resource's ARM schema (typo detection). For example, `retryCondition` vs the correct `failureCondition` nesting.

### 9. Dependency ordering warnings
When a resource depends on another resource that is itself invalid or skipped, warn that downstream resources will likely also fail.

## Implementation Notes

- Validations should run **after** override merging, against the final payload that would be sent to ARM.
- Warnings should not change the exit code — they are advisory. Consider a `--strict` flag that promotes warnings to errors.
- These are client-side heuristics; ARM will always be the authoritative validator. The goal is to catch the most common mistakes early.

## Related Files

- `src/services/dry-run-reporter.ts` — main dry-run logic
- `src/services/override-merger.ts` — override merging
- `src/lib/config-loader.ts` — override loading

コントリビューションガイド

コントリビューションガイドを開く

調査の方向性

まず src/services/dry-run-reporter.ts、src/services/override-merger.ts、src/lib/config-loader.ts を読み、次に tests/test-overrides/extract-dryrun.log と extract.log を比較します。override のマージ後の最終 payload を追跡し、dry-run が PUT 操作をどのように報告するかを特定します。完了条件は、クライアント側のチェックが通知用の [DRY RUN] WARN 行を出力し、レポートをブロックせず、終了コードも変更しないことです。

索引モデルが issue の本文から書いたものです。

評価

技術スタック
typescript
領域
api, cli
issue の種類
機能追加
難易度
5/5
見積もり時間
1週間以上
活発さ
静か
明瞭さ
おおむね明確
初心者へのやさしさ
45/100

新しい issue をメールで受け取る

初心者向けの GitHub issue を短くまとめたダイジェスト。