fix: local PropagateTags: false is not overridden by global PropagateTags: true

[Adityaj0]

Summary

Fixes #3977.

ImplicitApiPlugin._add_tags_to_implicit_api_if_necessary() decided whether to copy a resource's Tags onto its generated implicit AWS::Serverless::Api/HttpApi resource with:

should_propagate_tags = resource.properties.get("PropagateTags") or globals_var.get("PropagateTags")

PropagateTags is bool | None (see the Pydantic schema in samtranslator/internal/schema_source/aws_serverless_function.py), where None means "not set" and False is a meaningful, explicit opt-out. or treats a local False the same as "not set," so it silently falls through to the global value — a function with an explicit PropagateTags: false still gets tags propagated to its implicit API if a global PropagateTags: true is set.

This checks is not None instead, so an explicit local value (including False) always wins over the global, consistent with the "local wins when present" semantics GlobalProperties._prefer_local already uses for the standard Globals merge (samtranslator/plugins/globals/globals.py).

Test plan

• Added tests/translator/input/function_with_local_propagate_tags_false_overrides_global_true.yaml (local PropagateTags: false + global PropagateTags: true + global Tags) with expected output for all three partitions (aws, aws-cn, aws-us-gov), asserting the implicit API resource has no Tags.
• Verified the new test fails against the pre-fix code (tags leak through) and passes with the fix.
• python -m pytest tests/translator tests/plugins tests/parser — 2581 passed (same 5 pre-existing, unrelated SAR-timing/region failures as on develop).
• ruff check / black --check clean on the changed file.

[aws-sam-tooling-bot]

Code Review Results

Reviewed: bbc6a9a..2fb8a84
Files: 5
Comments: 1

---

Comments on lines outside the diff:

[samtranslator/plugins/api/implicit_api_plugin.py:115] [BUG] The PropagateTags precedence fix on the lines above is right, but the Tags line immediately below has the analogous precedence problem and is left unchanged. or makes a resource-level Tags value fully replace the global one, whereas the documented Globals behavior for dictionaries (see GlobalProperties._merge_dict in samtranslator/plugins/globals/globals.py) is a recursive merge.

Concretely:

Globals:
 Function:
   Tags:
     env: prod
Resources:
 ApiFunction:
   Type: AWS::Serverless::Function
   Properties:
     PropagateTags: true
     Tags:
       app: foo
     Events:
       ApiEvent:
         Type: Api
         Properties: {Path: /, Method: get}

The function and its generated resources end up with {env: prod, app: foo} (Globals merges, then propagate_tags runs), but ServerlessRestApi only receives {app: foo} because resource.properties.get("Tags") is truthy and the global Tags are never consulted. The global tag is silently dropped from the implicit API — the same class of "local silently discards global" bug this PR is fixing for PropagateTags.

Since the implicit API plugin runs before the Globals plugin, the merge has to be done explicitly here:

tags_properties = {**(globals_var.get("Tags") or {}), (resource.properties.get("Tags") or {})}

Note this assumes both are dicts, which the existing code already assumes (see the setdefault("Tags", {}).update(tags_properties) call below and its accompanying comment), so guarding for intrinsics is no worse than today. If you'd rather keep this PR minimal, that's reasonable — but it's worth splitting out rather than leaving, since the two lines now encode inconsistent global/local rules side by side.

[Adityaj0]

Agreed, and fixed rather than split out — the two lines were left inconsistent otherwise. Line 115 now does:

tags_properties = {**(globals_var.get("Tags") or {}), **(resource.properties.get("Tags") or {})}

Verified with an independent repro (Globals.Function.Tags={env: prod}, resource Tags={app: foo}, PropagateTags: true): before this change ServerlessRestApi got only {app: foo}; after, it gets {env: prod, app: foo} — matching the merge the resource itself already gets from Globals. Added tests/translator/input/function_with_local_and_global_tags_merge.yaml with expected output for all three partitions; new tests pass, plus the existing 33 propagate_tags tests and black/ruff both clean. Pushed as f8fe3c7.

[aws-sam-tooling-bot]

Code Review Results

Reviewed: bbc6a9a..f8fe3c7
Files: 9
Comments: 1

[aws-sam-tooling-bot]

[BUG] The new merge unpacks both Tags values as mappings unconditionally, before the if guard:

tags_properties = {**(globals_var.get("Tags") or {}), (resource.properties.get("Tags") or {})}

Two consequences that the previous a or b form did not have:

1. A non-mapping Tags now crashes the transform with a raw TypeError. Tags is only type-checked much later, when the resource is constructed ("Tags": PropertyType(False, IS_DICT) in samtranslator/model/sam_resources.py:181); Parser.validate_datatypes only verifies that Properties is a map. So a CFN-style list, e.g.

Tags:
   - Key: env
    Value: prod

reaches this line and raises TypeError: 'list' object is not a mapping. on_before_transform_template only catches InvalidEventException, so it escapes as an unhandled error instead of the customer-facing InvalidResourceException about the invalid Tags type. Note this now happens even when tags are not propagated at all (PropagateTags unset or false), because the merge is evaluated before the should_propagate_tags check — previously that line was a plain assignment and the guard kept the bad value untouched.

2. A whole-Tags intrinsic gets corrupted. For Tags: {"Fn::If": [...]} the merge yields {"env": "prod", "Fn::If": [...]}, so Fn::If becomes a tag key on the implicit API. This also diverges from the Globals semantics the merge is meant to mirror: GlobalProperties._token_of classifies intrinsic dicts as PRIMITIVE, so _do_merge returns _prefer_local and the function itself ends up with the local intrinsic only.

Guarding the merge keeps both paths behaving as before:

global_tags = globals_var.get("Tags")
local_tags = resource.properties.get("Tags")
if isinstance(global_tags, dict) and isinstance(local_tags, dict) and not is_intrinsics(local_tags):
   tags_properties = {**global_tags, local_tags}
else:
   tags_properties = local_tags if local_tags is not None else global_tags