feat(globals): add per-property merge strategy for list properties

[vicheey]

Summary

Fixes #3939

Adds a per-property merge strategy to the Globals merge engine. Today, all list-type properties concatenate when both Globals and resource-level values exist. This is incorrect for properties like Architectures (a function runs on one architecture — local should replace, not append).

Changes

New module: samtranslator/plugins/globals/merge_strategy.py

• MergeOp enum with 4 strategies:
  • DEEP_MERGE — recursive key union, local wins scalars (default for dicts, implicit)
  • CONCATENATE — global + local list (default for lists, implicit)
  • REPLACE — local wins entirely, global discarded
  • PRUNE_AND_MERGE — drop global keys not declared in local, then deep-merge shared keys

Modified: samtranslator/plugins/globals/globals.py

• Path-aware recursion: _do_merge tracks current property path through dict recursion
• Schema lookup at each recursion level via dot-notation paths
• CUSTOM_STRATEGIES dict declares per-property behavior:

  CUSTOM_STRATEGIES: dict[str, MergeRule] = {
      "Function.Architectures": REPLACE,
      "CapacityProvider.InstanceRequirements.Architectures": REPLACE,
      "CapacityProvider.ManagedResourceTags": PRUNE_AND_MERGE,
  }

• _prune_and_merge() method for properties with mutual exclusivity constraints (e.g., ManagedResourceTags where Propagate and Tags cannot coexist)

Updated: docs/globals.rst

• Synced "Supported Resources and Properties" with code (added CapacityProvider, StateMachine, WebSocketApi, ~20 missing Function/Api/HttpApi properties)
• Added "Resource Properties with Custom Merge Strategy" section with examples

Design

The schema uses dot-notation paths (e.g., CapacityProvider.InstanceRequirements.Architectures) to target properties at any nesting depth. Properties not listed in CUSTOM_STRATEGIES retain today's behavior — this makes the change fully backward-compatible.

Why PRUNE_AND_MERGE? ManagedResourceTags has a MUTUALLY_EXCLUSIVE validation rule between Propagate and Tags. Without pruning, DEEP_MERGE would inherit Propagate: true from Globals into a resource that only declares Tags, causing a validation error. PRUNE_AND_MERGE drops undeclared keys before merging, preventing the conflict.

How to extend

Add entries to CUSTOM_STRATEGIES in globals.py:

CUSTOM_STRATEGIES: dict[str, MergeRule] = {
    "Function.Architectures": REPLACE,
    "CapacityProvider.InstanceRequirements.Architectures": REPLACE,
    "CapacityProvider.ManagedResourceTags": PRUNE_AND_MERGE,
    # Future:
    # "Function.VpcConfig.SecurityGroupIds": REPLACE,
}

Tests

• Unit tests for MergeOp enum and MergeRule dataclass (all 4 strategies)
• Integration tests covering REPLACE, PRUNE_AND_MERGE, dot-path depth, nested paths, multi-strategy
• End-to-end translator fixture (globals_merge_strategy_architectures.yaml) proving REPLACE behavior across all 3 partitions
• 71 globals-specific tests pass; full suite backward-compatible
• Coverage: 95%+

[aws-sam-tooling-bot]

Code Review Results

Reviewed: 90ed07e..46a50d4
Files: 8
Comments: 1

[vicheey]

This PR takes inspiration from #3940 (by @allenheltondev) and expands the approach into a general-purpose framework with a strategy pattern that supports:

• REPLACE — local list wins entirely (same outcome as fix Globals.Function Architectures override behavior #3940 for Architectures)
• MERGE_BY_KEY — deduplicate list-of-dicts by a named key field (enables future Tags merge-by-key)
• Nested paths — dot-notation schema keys (e.g. VpcConfig.SecurityGroupIds) work at any depth, not just top-level

The framework is extensible via CUSTOM_STRATEGIES — adding new per-property merge behavior is a one-line dict entry rather than per-resource conditional logic.

cc @allenheltondev — thank you for the original fix and for raising #3939.

[aws-sam-tooling-bot]

Code Review Results

Reviewed: 90ed07e..69940b4
Files: 4 (source + tests; output JSON fixtures and YAML inputs are skipped)
Comments: 1

[licjun]

Consider adding brief docstrings or inline comments to each MergeOp member explaining the behavior. The names MERGE_BY_KEY and REPLACE_KEYS_MERGE_VALUES aren't immediately obvious to someone reading this for the first time — a one-liner per variant would save future readers from having to trace through _merge_by_key() / _replace_keys_merge_values() to understand the semantics.

For example:

class MergeOp(Enum):
    CONCATENATE = "concatenate"  # global_list + local_list (default)
    REPLACE = "replace"  # local wins entirely, global discarded
    MERGE_BY_KEY = "merge_by_key"  # list-of-dicts: deduplicate by key field, local overrides same-key global items
    REPLACE_KEYS_MERGE_VALUES = "replace_keys_merge_values"  # dict: only local's key-set survives, shared keys' values are deep-merged

[licjun]

Question about MERGE_BY_KEY — the primary use case seems to be CFN-style Tags ([{Key: ..., Value: ...}]), but in SAM templates Tags is a dict (key: value), not a list of key-value objects. Dict-on-dict merge already gives you the correct "local overrides same key" behavior natively.

Is there a concrete SAM Globals property today that would use MERGE_BY_KEY, or is this purely forward-looking for a future case?

[bnusunny]

Thanks for this — the path-threading approach is clean and minimal, and the Function.Architectures → REPLACE fix for #3939 looks correct to me. I verified inheritance (resource omits Architectures → global applies), override, and both intrinsic-function directions all behave as expected. Requesting changes on one issue in the ManagedResourceTags half.

PRUNE_AND_MERGE silently drops tag propagation in the reverse direction

PRUNE_AND_MERGE is symmetric, but the PR only reasons about the direction where Propagate sits in Globals. The mirror case — Globals provides Tags, the resource provides Propagate — is valid today (Propagate: false does not trip the ManagedResourceTags.Propagate=True mutual-exclusion rule), and this change silently alters its meaning.

Running both merge paths and feeding the result through _get_propagate_tags (samtranslator/model/capacity_provider/generators.py:229-235):

Globals:
  CapacityProvider:
    ManagedResourceTags:
      Tags: {env: prod}

Resources:
  MyCP:
    Type: AWS::Serverless::CapacityProvider
    Properties:
      ManagedResourceTags:
        Propagate: false

before → {Tags: {env: prod}, Propagate: False} → {"Mode": "Explicit", "ExplicitTags": [env=prod]}
after  → {Propagate: False}                    → {"Mode": "None"}

The global Tags key is pruned because it isn't declared locally, so the tags silently stop being applied — no error, no warning. That's a quiet correctness change rather than the mutual-exclusivity fix the strategy is aiming for.

No fixture or unit test covers this direction; every existing ManagedResourceTags fixture puts Propagate in Globals and Tags locally, so the symmetry is untested.

Ask: add a test for globals-provides-Tags / local-provides-Propagate, and either accept-and-document the pruning semantics for that case in docs/globals.rst, or narrow the strategy so it only prunes the keys that actually participate in the mutual-exclusion rule.

Worth noting this is very cheap to fix right now: ManagedResourceTags isn't in a release yet (it landed in 49a7a8a, after the 1.111.0 tag), so there's no customer-visible blast radius today — but the semantics should be settled before the property ships.

---

The rest of my notes are non-blocking, happy to file separately if you'd prefer:

• Latent leading-dot path bug. _merge_dict builds child paths with .lstrip("."), but _prune_and_merge uses a bare f"{path}.{key}". If a rule is ever registered at a section root (path == ""), children become ".Tags" instead of "Tags" and every nested rule silently misses. Unreachable with today's registry, but the "How to extend" section invites exactly this — worth using .lstrip(".") in both places.
• Typo'd registry keys fail silently. {"Architecture": REPLACE} (missing s) just concatenates with no error. Since correctness depends on strings matching supported_properties, consider validating CUSTOM_STRATEGIES keys against supported_properties in a unit test.
• MergeRule is a single-field wrapper around one MergeOp; CUSTOM_STRATEGIES[path] could hold the enum directly unless more fields are planned.
• Naming: tests/translator/input/capacity_provider_managed_resource_tags.yaml:50 still has a stale # REPLACE_KEYS_MERGE_VALUES: comment; docs/globals.rst:328 says "Selective Merge" while the code says PRUNE_AND_MERGE; and the PR title says "for list properties" though PRUNE_AND_MERGE is dict-only.
• No intrinsic-function fixture. globals_merge_strategy_architectures.yaml uses only literal lists. I checked Fn::If-valued Architectures behaves sanely in both directions, so this is a coverage gap rather than a defect — but Fn::If-selected architectures are a realistic pattern worth one case.
• Scope: the ManagedResourceTags strategy is a behavior change to a different resource than #3939 covers, and it removes an existing error fixture. The deletion is correct (that scenario genuinely stops erroring) and you did add a positive replacement across all three partitions, so this is disclosed and defensible — but splitting it out would let the #3939 fix land on pure REPLACE semantics.

[vicheey]

PRUNE_AND_MERGE silently drops tag propagation in the reverse direction

PRUNE_AND_MERGE is symmetric, but the PR only reasons about the direction where Propagate sits in Globals. The mirror case — Globals provides Tags, the resource provides Propagate — is valid today (Propagate: false does not trip the ManagedResourceTags.Propagate=True mutual-exclusion rule), and this change silently alters its meaning.

Running both merge paths and feeding the result through _get_propagate_tags (samtranslator/model/capacity_provider/generators.py:229-235):

Globals:
  CapacityProvider:
    ManagedResourceTags:
      Tags: {env: prod}

Resources:
  MyCP:
    Type: AWS::Serverless::CapacityProvider
    Properties:
      ManagedResourceTags:
        Propagate: false

before → {Tags: {env: prod}, Propagate: False} → {"Mode": "Explicit", "ExplicitTags": [env=prod]}
after  → {Propagate: False}                    → {"Mode": "None"}

The global Tags key is pruned because it isn't declared locally, so the tags silently stop being applied — no error, no warning. That's a quiet correctness change rather than the mutual-exclusivity fix the strategy is aiming for.

This is intentional — added two test cases covering it. When global has Tags and local declares only Propagate, the resource is explicitly opting out of tag inheritance. Without the new PRUNE_AND_MERGE, we previously only depend on DEEP_MERGE would produce {Propagate: true, Tags: {...}}which violates theMUTUALLY_EXCLUSIVE` validation rule. I needed to added the validation rule because the merging strategy we had was limited. Now with the new merging strategy, we generate correct result without violating the validation rule.

Both directions are now tested and documented in docs/globals.rst.

The rest of my notes are non-blocking, happy to file separately if you'd prefer:

• Latent leading-dot path bug. _merge_dict builds child paths with .lstrip("."), but _prune_and_merge uses a bare f"{path}.{key}". If a rule is ever registered at a section root (path == ""), children become ".Tags" instead of "Tags" and every nested rule silently misses. Unreachable with today's registry, but the "How to extend" section invites exactly this — worth using .lstrip(".") in both places.

• Typo'd registry keys fail silently. {"Architecture": REPLACE} (missing s) just concatenates with no error.
  ...

• Leading-dot path: fixed with .lstrip(".")
• MergeRule wrapper: removed — CUSTOM_STRATEGIES maps to MergeOp directly now
• Naming: "Selective Merge" → "Prune and Merge (PRUNE_AND_MERGE)" in docs; stale comment fixed
• Scope: added comment explaining why ManagedResourceTags is the motivating use case (mutual exclusivity)
• Added multi-level dot-path tests (depth-4, coexisting strategies)

Item 5 (intrinsic-function fixture) — happy to add if you feel strongly, but Fn::If-valued properties hit the is_intrinsics() → primitive path which is already covered by existing intrinsic tests.