fix(provisioning): suppress CloudFront OriginGroups drift + cover deeper cache-behavior fields (review on #871)

Code + test review (no blockers) on the CloudFront readCurrentState PR:

- convertToCfnFormat unwraps only the TOP-LEVEL OriginGroups list; each origin
  group's inner {Quantity,Items} wrappers (Members, FailoverCriteria.
  StatusCodes) are not unwrapped (no revertOriginGroup), and convertToSdkFormat
  is symmetrically unaware of them. Against the observed baseline both sides go
  through the same readCurrentState so it never false-positives on a normal
  deploy, but against the properties-fallback baseline the raw shape would diff
  the bare-array template form. Rather than guess the OriginGroups inner shape
  with no fixture to verify it (risking a wrong revertOriginGroup), add
  DistributionConfig.OriginGroups to getDriftUnknownPaths so drift on origin
  groups is suppressed until a dedicated fix lands a revertOriginGroup + a real
  OriginGroups integ fixture. Documented in the changelog + a code comment.

- Add a unit test round-tripping the cache-behavior {Quantity,Items} fields the
  main test missed: LambdaFunctionAssociations, FunctionAssociations,
  ForwardedValues.QueryStringCacheKeys, and the 3-level
  ForwardedValues.Cookies.WhitelistedNames (exercises unwrapQuantityAtPath's
  multi-level recursion specifically). Update the getDriftUnknownPaths test for
  the new OriginGroups entry.

Full suite 5924 tests pass. Happy-path re-verified by re-running the
s3-cloudfront integ (deploy + distribution-scoped drift clean/detect + clean
destroy, 0 orphans).

Author: go-to-k

docs/changelog-cdkd.md

@@ -29,7 +29,7 @@ The CLAUDE.md `## Known Limitations` section retains the load-bearing summary
   5. **Glue Crawler create raced IAM trust-policy propagation.** The first real-AWS run of the integ surfaced a 5th bug NOT in `glue-provider.ts`: cdkd's fast SDK path creates the Crawler's IAM role and the Crawler ~1s apart, before the role's trust policy propagates to Glue's assume layer, so `CreateCrawler` was rejected with "Service is unable to assume provided role. Please verify role's TrustPolicy." and the deploy failed at CREATE. CloudFormation never hits this (its deployment latency lets IAM settle) but cdkd does — the same just-created-role propagation race the RDS Enhanced Monitoring (#794) / ECS CapacityProvider (#805) / SNS+SQS resource-policy (#839) fixes handle for other services. **Fix:** add `'is unable to assume provided role'` to `RETRYABLE_ERROR_MESSAGE_PATTERNS` in `src/deployment/retryable-errors.ts` (anchored on the Glue-specific phrase — the existing lower-case `'trust policy'` pattern does NOT match Glue's `"TrustPolicy"`), so cdkd's outer `withRetry` (1s/2s/4s/8s backoff) absorbs the propagation window the way CloudFormation's deployment latency does. 1 unit test added.
   Unit tests cover: string→number coercion on Job create + update + already-numeric pass-through + non-finite/unparseable left-unchanged (no `NaN`); Workflow MAP + list tag shapes + no-tags elision; Crawler stop-and-retry on CrawlerRunningException for both delete + update + the already-stopping-race StopCrawler-rejection tolerance; Trigger restore-on-UpdateTrigger-failure, wait-for-DEACTIVATED ordering, no stop/restart when already DEACTIVATED, stop-before-delete, no-stop-when-not-ACTIVATED; Glue assume-role propagation message is retryable AND a non-matching Glue error is not. New `glue-update-hardening` integ fixture (L1 `CfnJob` with numeric props + idle `CfnCrawler` + ON_DEMAND `CfnTrigger` running the Job + `CfnWorkflow` with MAP-shape tags); `verify.sh` asserts the Job numeric props reach AWS as JSON numbers (`aws glue get-job`), the Workflow MAP-shape tags reach AWS (`aws glue get-tags`), the crawler + trigger exist, and all four are gone after a clean destroy (0 orphans); `CDKD_TEST_UPDATE=true` re-deploys to exercise the update paths. (The string-INPUT coercion path is unit-tested rather than integ-tested because CDK's L1 validator rejects stringly-typed numerics at synth time.)
 
-- ✅ **Feature: `CloudFrontDistributionProvider.readCurrentState` + `getDriftUnknownPaths` (closes the `AWS::CloudFront::Distribution` drift blind spot)** — `src/provisioning/providers/cloudfront-distribution-provider.ts`. **Gap:** the provider was 1 of only 3 (the other two — custom-resource / nested-stack — are by-design N/A) lacking `readCurrentState`, so `cdkd drift` fell back to the generic CC-API `GetResource` path. CloudFront's deeply-nested `DistributionConfig` carries `{ Quantity, Items }` wrappers + AWS-injected defaults whose shape diverges from the CFn shape cdkd state stores, so the generic strip pass surfaced phantom drift on a no-change distribution (and could miss real drift). CloudFront is SDK-provisioned, config-heavy, and frequently console-edited (cache behaviors, TTLs, origins, aliases, certs) — the highest-value drift gap. **Fix:** `readCurrentState` calls the read-only `GetDistributionConfigCommand` and inverts the provider's existing `convertToSdkFormat` via a new `convertToCfnFormat` (unwraps every top-level + nested-cache-behavior + origin `{ Quantity, Items }` wrapper back to bare arrays, hoists the AWS-nested `CachedMethods` out of `AllowedMethods` to the CFn sibling shape, and drops the `CallerReference` idempotency token), then overlays AWS-current tags (`GetDistribution` ARN → `ListTagsForResource`, filtering `aws:cdk:*` auto-tags; best-effort — a tag read failure still returns the config). `getDriftUnknownPaths` declares `DistributionConfig.CallerReference` (cdkd-generated, never templated) + `DistributionConfig.Logging.Bucket` (AWS normalizes to a regional domain on read) as drift-unknown. Because the deploy-time observed baseline is produced by the SAME `readCurrentState`, a fresh distribution compares byte-equal against itself → no false positive. 8 unit tests (non-CloudFront type returns undefined; NoSuchDistribution → undefined; full Quantity-unwrap + CachedMethods hoist + CallerReference drop + tag surface incl. aws:cdk filter; no Tags key when no user tags; config still returned when tag read fails; byte-stable across two reads; getDriftUnknownPaths). Integ: `tests/integration/s3-cloudfront/verify.sh` extended to assert (via `cdkd drift --json` + jq, scoped to the `AWS::CloudFront::Distribution` resource) that the distribution is in the `clean` bucket and NOT in `drifted` on a freshly-deployed stack, then mutates `Comment` out-of-band via `aws cloudfront update-distribution` and asserts the distribution now appears in `drifted` with a `Comment` change, then reverts before the clean destroy. The assertion is distribution-scoped rather than whole-stack-clean because the fixture's S3 BucketPolicy granting the OAI carries a SEPARATE, pre-existing latent false-positive (cdkd stores the OAI principal as the S3 canonical user id at deploy time but `GetBucketPolicy` returns it as `arn:aws:iam::cloudfront:user/CloudFront Origin Access Identity <id>`, so the two equivalent principal forms compare unequal) — an S3 BucketPolicy provider gap unrelated to this PR, tracked as a follow-up. **Lossiness note:** `convertToCfnFormat` is not perfectly lossless (SDK-required defaults `convertToSdkFormat` injects — `Comment: ''`, `Logging.*`, `CustomOriginConfig.HTTP{,S}Port` — are preserved on the inverted side), but this is correct for the comparator: the observed baseline carries the same defaults, and the `properties`-fallback baseline only walks state keys, so the extra defaults can never fire false drift. NOTE: `drift.ts` / `drift-calculator.ts` are unchanged (not in `integ-destroy` scope), but the provider file IS, so the merge still requires a real-AWS deploy+destroy (`/run-integ s3-cloudfront`).
+- ✅ **Feature: `CloudFrontDistributionProvider.readCurrentState` + `getDriftUnknownPaths` (closes the `AWS::CloudFront::Distribution` drift blind spot)** — `src/provisioning/providers/cloudfront-distribution-provider.ts`. **Gap:** the provider was 1 of only 3 (the other two — custom-resource / nested-stack — are by-design N/A) lacking `readCurrentState`, so `cdkd drift` fell back to the generic CC-API `GetResource` path. CloudFront's deeply-nested `DistributionConfig` carries `{ Quantity, Items }` wrappers + AWS-injected defaults whose shape diverges from the CFn shape cdkd state stores, so the generic strip pass surfaced phantom drift on a no-change distribution (and could miss real drift). CloudFront is SDK-provisioned, config-heavy, and frequently console-edited (cache behaviors, TTLs, origins, aliases, certs) — the highest-value drift gap. **Fix:** `readCurrentState` calls the read-only `GetDistributionConfigCommand` and inverts the provider's existing `convertToSdkFormat` via a new `convertToCfnFormat` (unwraps every top-level + nested-cache-behavior + origin `{ Quantity, Items }` wrapper back to bare arrays, hoists the AWS-nested `CachedMethods` out of `AllowedMethods` to the CFn sibling shape, and drops the `CallerReference` idempotency token), then overlays AWS-current tags (`GetDistribution` ARN → `ListTagsForResource`, filtering `aws:cdk:*` auto-tags; best-effort — a tag read failure still returns the config). `getDriftUnknownPaths` declares `DistributionConfig.CallerReference` (cdkd-generated, never templated) + `DistributionConfig.Logging.Bucket` (AWS normalizes to a regional domain on read) + `DistributionConfig.OriginGroups` (the per-origin-group inner `{ Quantity, Items }` wrappers — `Members` / `FailoverCriteria.StatusCodes` — are not yet unwrapped by `convertToCfnFormat`, and `convertToSdkFormat` is symmetrically unaware of them, so OriginGroups is suppressed to avoid a `properties`-fallback-baseline false positive until a dedicated `revertOriginGroup` + a real OriginGroups integ fixture lands; PR #871 review) as drift-unknown. Because the deploy-time observed baseline is produced by the SAME `readCurrentState`, a fresh distribution compares byte-equal against itself → no false positive. 9 unit tests (non-CloudFront type returns undefined; NoSuchDistribution → undefined; full Quantity-unwrap + CachedMethods hoist + CallerReference drop + tag surface incl. aws:cdk filter; no Tags key when no user tags; config still returned when tag read fails; byte-stable across two reads; getDriftUnknownPaths). Integ: `tests/integration/s3-cloudfront/verify.sh` extended to assert (via `cdkd drift --json` + jq, scoped to the `AWS::CloudFront::Distribution` resource) that the distribution is in the `clean` bucket and NOT in `drifted` on a freshly-deployed stack, then mutates `Comment` out-of-band via `aws cloudfront update-distribution` and asserts the distribution now appears in `drifted` with a `Comment` change, then reverts before the clean destroy. The assertion is distribution-scoped rather than whole-stack-clean because the fixture's S3 BucketPolicy granting the OAI carries a SEPARATE, pre-existing latent false-positive (cdkd stores the OAI principal as the S3 canonical user id at deploy time but `GetBucketPolicy` returns it as `arn:aws:iam::cloudfront:user/CloudFront Origin Access Identity <id>`, so the two equivalent principal forms compare unequal) — an S3 BucketPolicy provider gap unrelated to this PR, tracked as a follow-up. **Lossiness note:** `convertToCfnFormat` is not perfectly lossless (SDK-required defaults `convertToSdkFormat` injects — `Comment: ''`, `Logging.*`, `CustomOriginConfig.HTTP{,S}Port` — are preserved on the inverted side), but this is correct for the comparator: the observed baseline carries the same defaults, and the `properties`-fallback baseline only walks state keys, so the extra defaults can never fire false drift. NOTE: `drift.ts` / `drift-calculator.ts` are unchanged (not in `integ-destroy` scope), but the provider file IS, so the merge still requires a real-AWS deploy+destroy (`/run-integ s3-cloudfront`).
 
 - ✅ **Fix: `AWS::S3Vectors::VectorBucket` in-place Tags UPDATE (was a silent no-op) + `S3VectorsProvider.update()` immutable-property guard** — `src/provisioning/providers/s3-vectors-provider.ts`. **Bug:** `update()` was a silent no-op (`return { wasReplaced: false }` with no AWS call), so a `Tags` change was dropped while cdkd recorded the new Tags into state as if applied — the next deploy then saw no diff and the AWS-side tags stayed stale forever (the silent-drift failure mode documented in `feedback_tags_on_update_must_throw`). `Tags` is the ONLY in-place-updatable property of a VectorBucket (the CFn registry schema marks `VectorBucketName` + `EncryptionConfiguration` create-only, so a change to either drives a replacement, not an update). **Fix:** `updateVectorBucket()` diffs old vs new tags and applies the delta via `TagResource` (added / changed keys) + `UntagResource` (removed keys), resolving the bucket ARN via `GetVectorBucket` first (the `update()` contract only hands the physicalId). A tag-API failure THROWS (state is NOT written → next deploy retries) rather than being swallowed. A create-only property change that somehow reaches `update()` is surfaced as `ResourceUpdateNotSupportedError` (`cdkd deploy --replace`) instead of silently leaving AWS unchanged. 5 unit tests (no-delta no-op; TagResource add/change; UntagResource remove; tag-API failure throws; immutable-property change rejects before any AWS call). Validated against real AWS (`/run-integ s3-vectors` — the fixture now re-deploys under `CDKD_TEST_UPDATE=true` to change `env`, add `owner`, remove `team`; `verify.sh` asserts the AWS-side tags reflect all three via `ListTagsForResource`; deploy + update + destroy clean, 0 orphans).
 

src/provisioning/providers/cloudfront-distribution-provider.ts

@@ -437,10 +437,27 @@ export class CloudFrontDistributionProvider implements ResourceProvider {
    *   logging bucket to its `<bucket>.s3.amazonaws.com` regional domain on
    *   read, which never matches the bare bucket domain a template may
    *   carry; treat it as drift-unknown to avoid a guaranteed mismatch.
+   * - `DistributionConfig.OriginGroups`: each origin group carries its own
+   *   inner `{ Quantity, Items }` wrappers (`Members`,
+   *   `FailoverCriteria.StatusCodes`) that `convertToCfnFormat` does not yet
+   *   unwrap symmetrically (only the top-level OriginGroups list is unwrapped;
+   *   there is no `revertOriginGroup` pass, and `convertToSdkFormat` likewise
+   *   does not descend into them). Against the observed baseline both sides go
+   *   through the same `readCurrentState` so this never false-positives on a
+   *   normal deploy, but against the `properties`-fallback baseline (older
+   *   state without `observedProperties`) the raw `{ Quantity, Items }` shape
+   *   would diff the bare-array template form. Suppress it until a dedicated
+   *   fix lands a `revertOriginGroup` + a real OriginGroups integ fixture
+   *   (there is no OriginGroups fixture to verify the inner shape today). See
+   *   the PR #871 review thread.
    */
   getDriftUnknownPaths(resourceType: string): string[] {
     if (resourceType !== 'AWS::CloudFront::Distribution') return [];
-    return ['DistributionConfig.CallerReference', 'DistributionConfig.Logging.Bucket'];
+    return [
+      'DistributionConfig.CallerReference',
+      'DistributionConfig.Logging.Bucket',
+      'DistributionConfig.OriginGroups',
+    ];
   }
 
   /**

tests/unit/provisioning/cloudfront-distribution-provider.test.ts

@@ -872,6 +872,60 @@ describe('CloudFrontDistributionProvider', () => {
       expect(result!['Tags']).toEqual([{ Key: 'Env', Value: 'prod' }]);
     });
 
+    it('unwraps the deeper cache-behavior {Quantity,Items} fields (LambdaFunctionAssociations + 3-level ForwardedValues.Cookies.WhitelistedNames + QueryStringCacheKeys)', async () => {
+      // These fields are in CACHE_BEHAVIOR_QUANTITY_FIELDS but not covered by
+      // the main round-trip above; the 3-level Cookies.WhitelistedNames path
+      // exercises unwrapQuantityAtPath's multi-level recursion specifically.
+      mockSend.mockResolvedValueOnce({
+        DistributionConfig: {
+          CallerReference: 'ref',
+          Enabled: true,
+          Comment: '',
+          DefaultCacheBehavior: {
+            TargetOriginId: 'o',
+            ViewerProtocolPolicy: 'https-only',
+            LambdaFunctionAssociations: {
+              Quantity: 1,
+              Items: [{ EventType: 'origin-request', LambdaFunctionARN: 'arn:aws:lambda:...' }],
+            },
+            FunctionAssociations: { Quantity: 0, Items: [] },
+            ForwardedValues: {
+              QueryString: true,
+              QueryStringCacheKeys: { Quantity: 1, Items: ['lang'] },
+              Cookies: {
+                Forward: 'whitelist',
+                WhitelistedNames: { Quantity: 2, Items: ['sid', 'theme'] },
+              },
+            },
+          },
+        },
+      });
+      mockSend.mockResolvedValueOnce({
+        Distribution: { ARN: 'arn:aws:cloudfront::111122223333:distribution/EDFDVBD6EXAMPLE' },
+      });
+      mockSend.mockResolvedValueOnce({ Tags: { Items: [] } });
+
+      const result = await provider.readCurrentState(
+        'EDFDVBD6EXAMPLE',
+        'MyDistribution',
+        'AWS::CloudFront::Distribution'
+      );
+
+      const dcb = (result!['DistributionConfig'] as Record<string, unknown>)[
+        'DefaultCacheBehavior'
+      ] as Record<string, unknown>;
+      expect(dcb['LambdaFunctionAssociations']).toEqual([
+        { EventType: 'origin-request', LambdaFunctionARN: 'arn:aws:lambda:...' },
+      ]);
+      expect(dcb['FunctionAssociations']).toEqual([]);
+      const fv = dcb['ForwardedValues'] as Record<string, unknown>;
+      expect(fv['QueryStringCacheKeys']).toEqual(['lang']);
+      // The 3-level nested path: ForwardedValues.Cookies.WhitelistedNames.
+      const cookies = fv['Cookies'] as Record<string, unknown>;
+      expect(cookies['WhitelistedNames']).toEqual(['sid', 'theme']);
+      expect(cookies['Forward']).toBe('whitelist');
+    });
+
     it('returns DistributionConfig without Tags key when no user tags exist', async () => {
       mockSend.mockResolvedValueOnce({
         DistributionConfig: { CallerReference: 'ref', Enabled: true, Comment: '' },
@@ -941,10 +995,15 @@ describe('CloudFrontDistributionProvider', () => {
   });
 
   describe('getDriftUnknownPaths', () => {
-    it('excludes CallerReference + Logging.Bucket for CloudFront::Distribution', () => {
+    it('excludes CallerReference + Logging.Bucket + OriginGroups for CloudFront::Distribution', () => {
       expect(provider.getDriftUnknownPaths('AWS::CloudFront::Distribution')).toEqual([
         'DistributionConfig.CallerReference',
         'DistributionConfig.Logging.Bucket',
+        // OriginGroups inner {Quantity,Items} (Members / FailoverCriteria.
+        // StatusCodes) is not unwrapped by convertToCfnFormat yet, so it is
+        // suppressed to avoid a properties-fallback-baseline false positive
+        // until a dedicated revertOriginGroup + fixture lands.
+        'DistributionConfig.OriginGroups',
       ]);
     });