Azure / Azure/apiops-cli

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

Aberta
#147 2 comentários 0 reações 0 responsáveis Ver no GitHub
Enhancement P2
Linguagem predominante
TypeScript
Estrelas
26
Forks
9
Merge médio
1d 3h
PRs com merge (30d)
20

Descrição

## 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

Guia de contribuição

Abrir o guia de contribuição

Direção de pesquisa

Comece lendo src/services/dry-run-reporter.ts, src/services/override-merger.ts e src/lib/config-loader.ts e, em seguida, compare tests/test-overrides/extract-dryrun.log com extract.log. Rastreie o payload final após a mesclagem dos overrides e identifique como o dry-run relata as operações PUT. Considera-se concluído quando as verificações no lado do cliente emitirem linhas [DRY RUN] WARN informativas sem bloquear o relatório nem alterar seu código de saída.

Escrita pelo modelo de indexação a partir do texto da issue.

Avaliação

Stack de tecnologia
typescript
Domínio
api, cli
Tipo de issue
Funcionalidade
Dificuldade
5/5
Tempo estimado
Mais de uma semana
Status de atividade
Pouca atividade
Clareza
Razoavelmente clara
Facilidade para iniciantes
45/100

Receba novas issues na sua caixa de entrada

Um resumo curto de issues do GitHub para quem está começando.