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 3h
Merged PRs (30d)
20

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

Research direction

Start by reading src/services/dry-run-reporter.ts, src/services/override-merger.ts, and src/lib/config-loader.ts, then compare tests/test-overrides/extract-dryrun.log with extract.log. Trace the final payload after override merging and identify how dry-run reports PUT operations. Done means the proposed client-side checks emit advisory [DRY RUN] WARN lines without blocking the report or changing its exit code.

Written by the indexing model from the issue text.

Assessment

Tech stack
typescript
Domain
api, cli
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.