Azure / Azure/apiops-cli

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

Open
#147 2 comments 0 reactions 0 assignees View on GitHub
Enhancement P2
Dominant language
TypeScript
Stars
26
Forks
9
Avg merge
1d 1h
Merged PRs (30d)
19

Description

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

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.