Merge pull request #145 from Azure/fix/publish-overrides

Resolve named value reference, schema ref, and override issues in publish

Author: petehauge

docs/guides/environment-overrides.md

@@ -204,6 +204,89 @@ loggers:
       isBuffered: true
 ```
 
+> **Note:** When you provide a raw `instrumentationKey` value (instead of a `{{namedValue}}` reference), APIM will automatically create a named value to store the credential securely.
+
+### Logger credentials with auto-generated named values
+
+When APIM creates a logger (e.g., for Application Insights), it auto-generates a named value to store the instrumentation key. These auto-generated named values have 24-character hex IDs (e.g., `66f48e1226dab62c0823e4f8`) and are normally skipped during publish because APIM recreates them automatically.
+
+However, when publishing to a **fresh environment**, APIM cannot recreate these named values because the logger doesn't exist yet. To handle this, provide an override for the auto-generated named value:
+
+```yaml
+namedValues:
+  # Override the auto-generated named value with the production instrumentation key.
+  # Use the 24-char hex ID from the extracted artifact filename.
+  - name: 66f48e1226dab62c0823e4f8
+    properties:
+      value: "prod-instrumentation-key-value"
+
+loggers:
+  - name: appinsights-logger
+    properties:
+      loggerType: applicationInsights
+      resourceId: "/subscriptions/<sub-id>/resourceGroups/<rg>/providers/microsoft.insights/components/prod-appinsights"
+      isBuffered: true
+```
+
+Alternatively, you can override the logger credentials directly to bypass the named value reference entirely:
+
+```yaml
+loggers:
+  - name: appinsights-logger
+    properties:
+      loggerType: applicationInsights
+      resourceId: "/subscriptions/<sub-id>/resourceGroups/<rg>/providers/microsoft.insights/components/prod-appinsights"
+      credentials:
+        instrumentationKey: "prod-instrumentation-key-value"
+      isBuffered: true
+```
+
+### Subscriptions
+
+```yaml
+subscriptions:
+  - name: my-subscription
+    properties:
+      displayName: "My Subscription (Production)"
+      scope: "/apis/petstore-api"
+```
+
+> **Note:** The built-in `master` subscription is automatically skipped during publish.
+> Product-scoped subscriptions auto-generated by APIM are also skipped to avoid subscription limit errors.
+
+### Products
+
+```yaml
+products:
+  - name: starter-product
+    properties:
+      displayName: "Starter (Production)"
+      subscriptionRequired: true
+      approvalRequired: false
+      subscriptionsLimit: 10
+```
+
+### Gateways
+
+```yaml
+gateways:
+  - name: on-prem-gateway
+    properties:
+      locationData:
+        name: "Production datacenter"
+        city: "Seattle"
+        countryOrRegion: "US"
+```
+
+### Policy fragments
+
+```yaml
+policyFragments:
+  - name: rate-limit-fragment
+    properties:
+      description: "Production rate limiting policy"
+```
+
 ### Service-level policies
 
 ```yaml
@@ -213,21 +296,26 @@ policies:
       format: rawxml
 ```
 
-### All other resource types
+### Version sets, groups, and tags
 
-Overrides are also supported for: `gateways`, `versionSets`, `groups`, `subscriptions`, `products`, `tags`, `policyFragments`, and `workspaces`. Each uses the same `name` + `properties` format:
+Overrides are also supported for `versionSets`, `groups`, `tags`, and `workspaces`. Each uses the same `name` + `properties` format:
 
 ```yaml
-gateways:
-  - name: on-prem-gateway
+versionSets:
+  - name: petstore-versions
     properties:
-      locationData:
-        name: "On-premises datacenter"
+      displayName: "Petstore API Versions"
+      versioningScheme: Segment
 
-products:
-  - name: starter-product
+groups:
+  - name: partner-developers
     properties:
-      displayName: "Starter (Production)"
+      displayName: "Partner Developers (Production)"
+
+tags:
+  - name: public-api
+    properties:
+      displayName: "Public API"
 ```
 
 ## Override rules
@@ -360,6 +448,17 @@ If you add a new backend in dev but forget to add it to your override files, pub
 
 When using Key Vault references, the APIM managed identity needs access to the Key Vault. A common failure mode: overrides reference a Key Vault but APIM lacks the `Key Vault Secrets User` role on that vault.
 
+### Gotcha: Auto-generated named values for loggers
+
+When you create a logger in APIM (e.g., for Application Insights), APIM auto-generates a named value to store the credential. These have 24-character hex names (e.g., `<24-char-hex-id>`). During extract, these are captured as artifacts. During publish:
+
+- **Same environment:** Auto-generated named values are skipped (APIM already has them).
+- **Fresh environment:** The logger fails because the named value doesn't exist yet. Provide an override with the target environment's credential value, or override the logger's `credentials` directly.
+
+### Gotcha: Redacted secrets
+
+Extracted secret named values have their `value` replaced with `*** REDACTED ***`. If you publish these without providing an override with a real value or Key Vault reference, they will be skipped with a warning. Always provide overrides for secret named values when publishing to a different environment.
+
 ### Dry-run validation
 
 Use `--dry-run` to preview publish behavior with overrides:

src/services/api-publisher.ts

@@ -462,34 +462,46 @@ async function reconcileOperationsAfterSpecImport(
   }
 }
 
-/** Strip source schema refs from request/response representations before PATCH. */
+/**
+ * Strip source schema refs from operation parameters and representations
+ * before PATCH. APIM assigns new schema IDs on spec import, so stale IDs
+ * from the source instance would cause "Operation references schema that
+ * does not exist" validation errors.
+ */
 function stripRepresentationSchemaRefs(patchProps: Record<string, unknown>): void {
   const SCHEMA_REF_FIELDS = ['schemaId', 'typeName'];
 
-  function stripFromRepresentations(representations: unknown): void {
-    if (!Array.isArray(representations)) return;
-    for (const rep of representations) {
-      if (rep && typeof rep === 'object') {
+  function stripFromItems(items: unknown): void {
+    if (!Array.isArray(items)) return;
+    for (const item of items) {
+      if (item && typeof item === 'object') {
         for (const field of SCHEMA_REF_FIELDS) {
-          delete (rep as Record<string, unknown>)[field];
+          delete (item as Record<string, unknown>)[field];
         }
       }
     }
   }
 
-  // Strip schema refs from request.representations.
+  // Strip schema refs from top-level templateParameters.
+  stripFromItems(patchProps.templateParameters);
+
+  // Strip schema refs from request parameters and representations.
   const request = patchProps.request;
   if (request && typeof request === 'object') {
     const req = request as Record<string, unknown>;
-    stripFromRepresentations(req.representations);
+    stripFromItems(req.representations);
+    stripFromItems(req.queryParameters);
+    stripFromItems(req.headers);
   }
 
-  // Strip schema refs from responses[].representations.
+  // Strip schema refs from responses[].representations and headers.
   const responses = patchProps.responses;
   if (Array.isArray(responses)) {
     for (const response of responses) {
       if (response && typeof response === 'object') {
-        stripFromRepresentations((response as Record<string, unknown>).representations);
+        const resp = response as Record<string, unknown>;
+        stripFromItems(resp.representations);
+        stripFromItems(resp.headers);
       }
     }
   }

src/services/publish-service.ts

@@ -8,7 +8,7 @@
 
 import { IApimClient } from '../clients/iapim-client.js';
 import { IArtifactStore } from '../clients/iartifact-store.js';
-import { PublishConfig } from '../models/config.js';
+import { PublishConfig, OverrideConfig } from '../models/config.js';
 import { ApimServiceContext, ResourceDescriptor } from '../models/types.js';
 import { ResourceType } from '../models/resource-types.js';
 import { getResourceTier } from '../lib/dependency-graph.js';
@@ -257,7 +257,7 @@ async function executePuts(
       //   Pool backends reference individual Backend resources and must be
       //   published after the backends they aggregate (see pool backend
       //   ordering comments in splitPoolBackends).
-      const { namedValues, otherTier1 } = splitNamedValues(nonWorkspaceTier1);
+      const { namedValues, otherTier1 } = splitNamedValues(nonWorkspaceTier1, config.overrides);
 
       if (namedValues.length > 0) {
         logger.debug(`Publishing ${namedValues.length} named value(s) first (wave 1 of tier 1)`);
@@ -378,19 +378,22 @@ async function publishAndOutput(
  * Auto-generated NamedValues (24-char hex IDs like Logger credentials) are
  * filtered out because APIM recreates them when the referencing Logger is
  * published. Publishing them would create orphan duplicates.
+ *
+ * However, if the user has provided an explicit override for an auto-generated
+ * named value, it is published normally. This supports publishing to fresh
+ * environments where APIM cannot recreate the named value automatically.
  */
 function splitNamedValues(
-  descriptors: ResourceDescriptor[]
+  descriptors: ResourceDescriptor[],
+  overrides?: OverrideConfig
 ): { namedValues: ResourceDescriptor[]; otherTier1: ResourceDescriptor[] } {
   const namedValues: ResourceDescriptor[] = [];
   const otherTier1: ResourceDescriptor[] = [];
 
   for (const descriptor of descriptors) {
     if (descriptor.type === ResourceType.NamedValue) {
-      // Skip auto-generated named values (24-char hex IDs) - APIM recreates
-      // these when publishing loggers with credential references
       const name = getNamePart(descriptor.nameParts, 0);
-      if (isAutoGeneratedId(name)) {
+      if (isAutoGeneratedId(name) && !hasNamedValueOverride(name, overrides)) {
         logger.debug(`Skipping auto-generated named value: ${name}`);
         continue;
       }
@@ -403,6 +406,18 @@ function splitNamedValues(
   return { namedValues, otherTier1 };
 }
 
+/**
+ * Check whether a named value has an explicit override entry.
+ * Uses case-insensitive matching to align with override-merger behavior.
+ */
+function hasNamedValueOverride(name: string, overrides?: OverrideConfig): boolean {
+  if (!overrides?.namedValues) return false;
+  const lowerName = name.toLowerCase();
+  return Object.keys(overrides.namedValues).some(
+    (key) => key.toLowerCase() === lowerName
+  );
+}
+
 /**
  * Separates Backend descriptors that are pool backends (properties.type === "Pool")
  * from all other descriptors in the supplied list.

src/services/resource-extractor.ts

@@ -88,10 +88,6 @@ export async function extractResourceType(
   };
 
   try {
-    const loggerTokenMap =
-      type === ResourceType.Logger
-        ? await loadNamedValueDisplayNameMap(client, context)
-        : undefined;
 
     const resources = client.listResources(context, type, parent);
 
@@ -124,10 +120,6 @@ export async function extractResourceType(
           }
         }
 
-        if (type === ResourceType.Logger && loggerTokenMap && loggerTokenMap.size > 0) {
-          json = normalizeLoggerCredentialPlaceholders(json, loggerTokenMap);
-        }
-
         // Apply secret redaction
         const safeJson = redactSecrets(descriptor, json);
 
@@ -166,68 +158,6 @@ export async function extractResourceType(
   return result;
 }
 
-async function loadNamedValueDisplayNameMap(
-  client: IApimClient,
-  context: ApimServiceContext
-): Promise<Map<string, string>> {
-  const map = new Map<string, string>();
-
-  for await (const namedValue of client.listResources(context, ResourceType.NamedValue)) {
-    const name = namedValue.name;
-    const properties = namedValue.properties as Record<string, unknown> | undefined;
-    const displayName = properties?.displayName;
-
-    if (typeof name === 'string' && typeof displayName === 'string' && displayName.length > 0) {
-      map.set(displayName, name);
-    }
-  }
-
-  return map;
-}
-
-function normalizeLoggerCredentialPlaceholders(
-  json: Record<string, unknown>,
-  displayNameToName: Map<string, string>
-): Record<string, unknown> {
-  const properties = json.properties as Record<string, unknown> | undefined;
-  const credentials = properties?.credentials;
-
-  if (!properties || credentials === undefined) {
-    return json;
-  }
-
-  const normalizeValue = (value: unknown): unknown => {
-    if (typeof value === 'string') {
-      return value.replace(/\{\{([^}]+)\}\}/g, (match, tokenName: string) => {
-        const mappedName = displayNameToName.get(tokenName);
-        return mappedName ? `{{${mappedName}}}` : match;
-      });
-    }
-
-    if (Array.isArray(value)) {
-      return value.map(normalizeValue);
-    }
-
-    if (value && typeof value === 'object') {
-      const out: Record<string, unknown> = {};
-      for (const [key, child] of Object.entries(value as Record<string, unknown>)) {
-        out[key] = normalizeValue(child);
-      }
-      return out;
-    }
-
-    return value;
-  };
-
-  return {
-    ...json,
-    properties: {
-      ...properties,
-      credentials: normalizeValue(credentials),
-    },
-  };
-}
-
 /**
  * Extract a single resource by descriptor and write to artifact store.
  */