Repository navigation
feat: implement apiops compare command - #43
Elizabeth Maher (EMaher) with Copilot wants to merge 69 commits into
Conversation
…82f-4385-8d11-36f74c270370 Adding Microsoft SECURITY.MD
Initial checkin for source code, tests & configuration
Add CodeQL analysis workflow configuration
- Remove unnecessary type assertion in override-merger.ts - Fix unknown type in template literals in product-publisher.ts - Add yield* [] to generator without yield in api-product-extractor.test.ts - Remove unused variable, fix eslint-disable directive, and rename unused param in extract-service.test.ts Agent-Logs-Url: https://github.com/Azure/apiops-cli/sessions/e990f413-47fb-4e99-bddf-9c31ae9b68fe Co-authored-by: EMaher <9244742+EMaher@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…st' of https://github.com/Azure/apiops-cli into enewman/integration-test
- Create src/lib/user-agent.ts — single source of truth for User-Agent string - Set User-Agent header on all HTTP requests in ApimClient.request() - Add unit tests: tests/unit/lib/user-agent.test.ts + apim-client coverage Closes #16 Agent-Logs-Url: https://github.com/Azure/apiops-cli/sessions/7d507051-7b27-4f36-b053-acc897e40ecc Co-authored-by: petehauge <12090616+petehauge@users.noreply.github.com>
Second test now verifies User-Agent is set on unauthenticated blob requests (skipAuth=true) and that Authorization header is absent on those calls. Closes #16 Agent-Logs-Url: https://github.com/Azure/apiops-cli/sessions/7d507051-7b27-4f36-b053-acc897e40ecc Co-authored-by: petehauge <12090616+petehauge@users.noreply.github.com>
- Added TypeScriptDev learning: USER_AGENT constant in src/lib/user-agent.ts, header set in ApimClient.request() at line 108 - Added TestEngineer learning: User-Agent testing patterns for both auth paths - Closes #16 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: petehauge <12090616+petehauge@users.noreply.github.com>
Addresses PR review feedback: the production defaultArmRequest fetch call in keyvault-checker.ts was missing the User-Agent header. Now imports USER_AGENT from user-agent.ts and adds it alongside Authorization/Accept. Adds a unit test that calls checkKeyVaultSecretAccess without an injected armRequest (so defaultArmRequest runs) and verifies every fetch call carries the correct User-Agent header. Closes #16 Agent-Logs-Url: https://github.com/Azure/apiops-cli/sessions/8b196bc2-81c8-4320-9a60-c3655ee4b3bc Co-authored-by: petehauge <12090616+petehauge@users.noreply.github.com>
Agent-Logs-Url: https://github.com/Azure/apiops-cli/sessions/8b196bc2-81c8-4320-9a60-c3655ee4b3bc Co-authored-by: petehauge <12090616+petehauge@users.noreply.github.com>
Integration test updates
…tern Agent-Logs-Url: https://github.com/Azure/apiops-cli/sessions/16641d14-86ab-42af-9e18-34ecbc7c4bd1 Co-authored-by: EMaher <9244742+EMaher@users.noreply.github.com>
feat: add User-Agent header to all REST API calls
Removes [uuid](https://github.com/uuidjs/uuid). It's no longer used after updating ancestor dependency [@azure/msal-node](https://github.com/AzureAD/microsoft-authentication-library-for-js). These dependencies need to be updated together. Removes `uuid` Updates `@azure/msal-node` from 5.1.4 to 5.1.5 - [Release notes](https://github.com/AzureAD/microsoft-authentication-library-for-js/releases) - [Commits](AzureAD/microsoft-authentication-library-for-js@msal-node-v5.1.4...msal-node-v5.1.5) --- updated-dependencies: - dependency-name: uuid dependency-version: dependency-type: indirect - dependency-name: "@azure/msal-node" dependency-version: 5.1.5 dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com>
- README: restructure Authentication section to separate GitHub Actions OIDC path (no client secret) from local CLI / other CI (client secret, managed identity). Removes conflation of managed identity with the --client-secret flag path. - identity-guide-service: replace "Rotate service principal credentials periodically" with an accurate note that OIDC has no stored credentials to rotate. Agent-Logs-Url: https://github.com/Azure/apiops-cli/sessions/6c946379-655e-4831-a343-bbb3030e4fdb Co-authored-by: petehauge <12090616+petehauge@users.noreply.github.com>
NodeJsDev implemented single-source-of-truth version management using
ESM import attributes to read from package.json at runtime.
- Created decisions.md with version management pattern
- Updated NodeJsDev history with implementation details
- Decision: Use 'with { type: 'json' }' import syntax for Node 22+
Closes #21 Adds full implementation of `apiops compare` command for comparing two Azure API Management instances via the ARM REST API. Files added: - src/lib/compare-normalizer.ts — normalization engine (string replacement, auto-ID keying, resource map building) - src/lib/compare-differ.ts — diff engine (deep JSON diff, resource map comparison, secret/credential skip) - src/services/compare-service.ts — orchestrator (hierarchical comparison: top-level, API children, product/gateway/workspace children) - src/cli/compare-command.ts — CLI registration + text/JSON output - tests/unit/lib/compare-normalizer.test.ts - tests/unit/lib/compare-differ.test.ts - tests/unit/services/compare-service.test.ts - tests/unit/cli/compare-command.test.ts Files modified: - src/models/config.ts — adds CompareConfig - src/cli/index.ts — registers compare command - specs/contracts/cli-commands.md — documents compare command - specs/tasks.md — Phase 9 with all compare tasks marked done Agent-Logs-Url: https://github.com/Azure/apiops-cli/sessions/a77d8d45-6ce6-4ca7-a7ea-9191c35edc17 Co-authored-by: EMaher <9244742+EMaher@users.noreply.github.com>
…re-service Agent-Logs-Url: https://github.com/Azure/apiops-cli/sessions/a77d8d45-6ce6-4ca7-a7ea-9191c35edc17 Co-authored-by: EMaher <9244742+EMaher@users.noreply.github.com>
apiops compare command
| ); | ||
| s = replaceAll( | ||
| s, | ||
| `/subscriptions/${tgt.subscriptionId}/resourceGroups/${tgt.resourceGroup}/providers/Microsoft.ApiManagement/service/${tgt.serviceName}`, |
There was a problem hiding this comment.
These resource urls shouldn't have hardcode types. The code should use resource-types.ts model and resource-paths.ts and resource-url.ts in the lib,
There was a problem hiding this comment.
Fixed in c680d66. normalizeString now derives the APIM ARM path by calling getArmPathFromBaseUrl(context.baseUrl) — which extracts the path component from the already-constructed baseUrl (built by buildArmBaseUrl in cloud-config.ts). The subscription+RG prefix is then derived via splitAtProviders(). The hardcoded Microsoft.ApiManagement/service string is gone from the normalizer; the provider path comes from the context instead.
…izer Previously, normalizeString built the full APIM ARM path by assembling subscriptionId + resourceGroup + serviceName with a hardcoded 'Microsoft.ApiManagement/service' provider string. Now it uses getArmPathFromBaseUrl(context.baseUrl) to extract the path component from the already-constructed base URL (built by buildArmBaseUrl in cloud-config.ts), and splitAtProviders() to derive the subscription+RG prefix. This removes the hardcoded provider type from the normalizer. Added tests for the two new exported helpers. Agent-Logs-Url: https://github.com/Azure/apiops-cli/sessions/60174382-313e-441b-b12f-562cf1f89617 Co-authored-by: EMaher <9244742+EMaher@users.noreply.github.com>
| const defaultSubscriptionId = | ||
| globalOpts.subscriptionId ?? process.env.AZURE_SUBSCRIPTION_ID; | ||
|
|
||
| const sourceSubscriptionId = options.sourceSubscriptionId ?? defaultSubscriptionId; |
There was a problem hiding this comment.
sourceSubscriptionId and targetSubscriptionId should only be used if the source and target apim resources are not in the same subscription. User should either specify subscriptionId or (sourceSubscriptionId and targetSubscriptionId). They are mutually exclusive.
There was a problem hiding this comment.
Fixed in a91ee14. The two subscription ID modes are now mutually exclusive:
- Same subscription: use
--subscription-id <id>(global flag) orAZURE_SUBSCRIPTION_IDenv var — applied to both source and target. - Different subscriptions: use
--source-subscription-id <id>and--target-subscription-id <id>together.
Mixing both groups exits with code 2, as does specifying only one of the per-side flags. The validation is extracted into a pure resolveSubscriptionIds() function covered by 9 unit tests. The spec doc has been updated with a usage table.
…ally exclusive in compare --subscription-id (global) is for when both APIM instances are in the same subscription. --source-subscription-id + --target-subscription-id are for the cross-subscription case. Using both groups simultaneously is now rejected with exit code 2. Extracted resolveSubscriptionIds() pure helper for testability. Added 9 unit tests covering: shared sub, env-var fallback, per-side pair, mutual-exclusion violation, single per-side flag, and missing sub ID. Updated specs/contracts/cli-commands.md with a subscription note table. Agent-Logs-Url: https://github.com/Azure/apiops-cli/sessions/7bdc7855-c1c2-4087-a9f8-a5ca471577ab Co-authored-by: EMaher <9244742+EMaher@users.noreply.github.com>
| * T-CMP-05: Compare service orchestrator. | ||
| * | ||
| * Coordinates hierarchical comparison of two APIM instances: | ||
| * 1. Top-level resource types |
There was a problem hiding this comment.
I don't like hard-coded things. Resource comparison should be using same libraries/models and dependency analysis as the extract and publish command.
There was a problem hiding this comment.
Fixed in a706159. compare-service.ts now derives all resource type groupings from the same libraries as extract and publish:
- Top-level types:
[...TIER_1_RESOURCES, ...TIER_2_RESOURCES, ...TIER_3_RESOURCES.filter(t => !isChildType(t))]— mirrors theextract-servicepattern - Child types per parent:
getChildTypesOf(parentType)usesgetDependencies()fromdependency-graph.ts— no explicit lists - Per-type options (exclusions, skip flags): consolidated in a
TYPE_OPTIONSmap keyed byResourceType
New resource types added to the dependency graph are automatically picked up without touching compare-service.ts. As a side effect, McpServer (which was missing from the old hardcoded list) is now correctly compared as an API child type.
| logger.debug('Comparing top-level resources...'); | ||
|
|
||
| allResults.push( | ||
| await compareType(client, ctx, source, target, 'Named Values', ResourceType.NamedValue, { |
There was a problem hiding this comment.
Again, why is the type hard-coded. Isn't there lists of top level resource types already defined in another library?
There was a problem hiding this comment.
Fixed in a706159. The top-level type list is now [...TIER_1_RESOURCES, ...TIER_2_RESOURCES, ...TIER_3_RESOURCES.filter(t => !isChildType(t))] from dependency-graph.ts and resource-path.ts — the same approach extract-service.ts uses for its tier loops. Child type groups per parent (API, Product, Gateway) are derived from getDependencies() via a getChildTypesOf(parentType) helper, so no type-specific constants appear anywhere in the comparison logic.
… and resource-path libs Replace hardcoded resource type lists with derivations from the same model used by extract and publish: - TOP_LEVEL_TYPES = TIER_1 + TIER_2 + non-child TIER_3 (mirrors extract-service) - getChildTypesOf(parent) uses getDependencies() from dependency-graph.ts so newly added resource types are automatically included - TYPE_OPTIONS map centralises per-type skip/exclude flags - RESOURCE_TYPE_LABEL map centralises display labels with enum-name fallback - McpServer now automatically included as an API child (was previously missing) Agent-Logs-Url: https://github.com/Azure/apiops-cli/sessions/685cf23c-6c2a-4672-b8f8-8f7331c04b88 Co-authored-by: EMaher <9244742+EMaher@users.noreply.github.com>
| | Both instances in the **same** subscription | `--subscription-id <id>` (global flag) or `AZURE_SUBSCRIPTION_ID` env var | | ||
| | Instances in **different** subscriptions | `--source-subscription-id <id>` **and** `--target-subscription-id <id>` | | ||
|
|
||
| Combining `--subscription-id` with `--source-subscription-id` or `--target-subscription-id` is not allowed and exits with code `2`. Specifying only one of `--source-subscription-id` / `--target-subscription-id` (without the other) is also not allowed. |
There was a problem hiding this comment.
Actually, I don't like that the subscription-id and source-subscription-id mean the same thing. It's confusing. Remove subscription-id as a global parameter. Only the extract and publish commands should have a subscription-id option. It should be required for both those commands
There was a problem hiding this comment.
Fixed in 6790702. --subscription-id has been removed from global options. It is now a per-command required option on extract and publish (accepting AZURE_SUBSCRIPTION_ID as an env var fallback via Commander's .env()). The compare command never accepted the global flag and now uses only --source-subscription-id and --target-subscription-id (both required).
| */ | ||
| export function createCompareCommand(): Command { | ||
| const compare = new Command('compare') | ||
| .description('Compare two Azure APIM instances and report differences') |
There was a problem hiding this comment.
make source subscription-id and target subscription-id required.
There was a problem hiding this comment.
Fixed in 6790702. Both --source-subscription-id and --target-subscription-id are now declared with requiredOption() on the compare command — Commander enforces them at parse time. The old resolveSubscriptionIds() helper and global subscription-id fallback logic have been removed.
|
|
||
| for (const r of result.typeResults) { | ||
| if (r.skipped) { | ||
| process.stdout.write(` ⚠️ ${r.label}: SKIPPED (${r.skipReason ?? 'unknown'})\n`); |
There was a problem hiding this comment.
Remove emojis from text output
There was a problem hiding this comment.
Fixed in 6790702. All emojis removed from compare text output — ✅ PASS, ❌ FAIL, ⚠️ SKIPPED, 💥 ERROR replaced with plain-text PASS, FAIL, SKIPPED, ERROR prefixes. The emoji-laden diff strings in compare-differ.ts were also cleaned up.
| | `--target-resource-group <rg>` | string | yes | — | Target APIM resource group | | ||
| | `--target-service-name <name>` | string | yes | — | Target APIM service instance name | | ||
| | `--source-subscription-id <id>` | string | no | — | Source subscription ID (see subscription note below) | | ||
| | `--target-subscription-id <id>` | string | no | — | Target subscription ID (see subscription note below) | |
There was a problem hiding this comment.
should also have optional output format option that can be text, table or json. Text calls outputText. Table output show table that is resource (by resource id), Status (missing, extra, different, skipped), Notes which includes any notes about why something was skipped. Json is same information as in table, but in json format.
There was a problem hiding this comment.
Fixed in 6790702. The compare command now has --format text|table|json:
text(default) — existing per-type detailed output, no emojistable— ASCII table with three columns: Resource (label/name), Status (missing,extra,different,skipped), Notes (skip reason or first 3 diff paths)json— same row structure as table:{ exitCode, summary, resources: [{ resource, status, notes }] }
The --format is a compare-level option with enforced choices; the global --format (text/json) continues to apply to extract and publish.
… no emojis
- Remove --subscription-id from global CLI options; add it as a
per-command required option (with AZURE_SUBSCRIPTION_ID env var
support) to extract and publish commands
- Make --source-subscription-id and --target-subscription-id required
options on compare (no global subscription-id fallback)
- Remove resolveSubscriptionIds() helper (no longer needed)
- Add --format text|table|json to compare command; table shows
Resource / Status / Notes columns per resource; json mirrors the
same row structure
- Remove all emojis from compare text output
- Add status field ('missing' | 'extra' | 'different') to ResourceDiff
to make categorization explicit
- Update spec doc, all affected tests
Agent-Logs-Url: https://github.com/Azure/apiops-cli/sessions/5742ddd2-d902-42cb-bc38-c94c3ffe182d
Co-authored-by: EMaher <9244742+EMaher@users.noreply.github.com>
6790702 to
79da078
Compare
The request's own signature was masked only in a fixed list of whole forms (as sent, decoded, percent-encoded once or twice, and two JSON-escaped variants). A page that wrote its + characters as HTML character references (&Azure#43;, +), or escaped only some of them, left the signature readable in the error message and in error.body. The signature is now matched with each +, / and = in any of its forms (as is, percent-encoded once or twice, JSON-escaped or as an HTML character reference), in any mix.
The sig= parameter rule stops at an HTML character reference, so for a link written as ?sig=A&Azure#43;... it masked only the first character. The full-signature pattern then no longer matched, and the rest of the signature stayed in the error message and HttpError.body. The request's own signature is now masked first.
Adds
apiops compare— a new subcommand that compares two APIM instances via the ARM REST API and reports resource-level differences. Designed for deployment verification and environment drift investigation.Normalization logic is ported from
tests/integration/all-resource-types/Compare-ApimInstance.ps1.New modules
src/lib/compare-normalizer.ts— Strips ARM envelope fields, read-only/timestamp properties, and replaces instance-specific values with stable placeholders before comparison:{{sub}},{{rg}},{{apim-name}}{{auto-id}}/{{guid}}{{auto-id-0}},{{auto-id-1}}, … for stable cross-instance matchingcontext.baseUrl(viagetArmPathFromBaseUrl/splitAtProviders) — no hardcoded provider type stringssrc/lib/compare-differ.ts— Deep recursive JSON diff with dot-notation paths; skips.properties.valueon secret named values and.properties.credentialson EventHub/AppInsights loggers. EachResourceDiffcarries an explicitstatusfield (missing|extra|different) for structured output.src/services/compare-service.ts— Hierarchical orchestrator using the same dependency graph and resource-path libraries as extract and publish:TIER_1_RESOURCES,TIER_2_RESOURCES, and non-childTIER_3_RESOURCES(viaisChildType()) — same pattern asextract-service.tsgetDependencies()viagetChildTypesOf()— new resource types added to the dependency graph are automatically includedTYPE_OPTIONSmapecho-api,administrators/developers/guests,starter/unlimited,mastersrc/cli/compare-command.ts— Commander subcommand with--format text|table|jsonoutput modes and plain-text (emoji-free) console output:text(default): per-type summary with field-level diff detailstable: ASCII table with Resource (by resource ID), Status (missing,extra,different,skipped), and Notes columnsjson: same row structure astable, machine-readableCLI interface
All six resource-identity flags are required.
--source-subscription-idand--target-subscription-idare independent — use them when the instances are in different subscriptions or the same one.Exit codes:
0= identical,1= differences found,2= fatal errorSupporting changes
src/cli/index.ts—--subscription-idremoved from global options; it is now a per-command required option onextractandpublish(withAZURE_SUBSCRIPTION_IDenv var fallback)src/cli/extract-command.ts/src/cli/publish-command.ts—--subscription-idadded as a mandatory per-command optionsrc/models/config.ts— addsCompareConfigspecs/contracts/cli-commands.md— updated global options table, extract/publish option tables, and compare command documentationspecs/tasks.md— Phase 9 added with all compare tasks