fix: include API stage variables in the deployment hash

[warren830]

Which issue(s) does this change fix?

Fixes #3703

Why is this change necessary?

ApiGatewayDeployment.make_auto_deployable builds its hash from the swagger body, the OpenAPI version, the custom domain and the associated function names — but not from the stage's Variables. Since AWS::ApiGateway::Stage does carry Variables, a template change that only touches stage variables produces the same deployment logical id, so CloudFormation creates no new AWS::ApiGateway::Deployment.

That has two effects, both described in the issue:

1. The stage-variable change is never actually deployed.
2. The UpdateStage call points the stage back at the deployment it already references. If any deployment was made outside SAM since the last SAM deployment, the stage silently reverts to the SAM-known deployment id — losing the out-of-band changes.

How does it address the issue?

Stage variables are appended to hash_input, following the existing if openapi_version: / if domain: pattern.

They are appended only when set, which is the important part for backwards compatibility: templates that define no stage variables keep their current deployment logical id and see no churn. sort_keys=True keeps the hash stable regardless of key order in the template.

What side effects does this change have?

Templates that do set stage variables get a new deployment logical id once, so the next deploy creates a new deployment. That is the intended behaviour — it is exactly the deployment that was previously being skipped — but it is a one-time redeploy for those stacks, so calling it out explicitly.

The 24 updated files under tests/translator/output/ are that same effect in the checked-in expected outputs: only the deployment logical id and its RestApi deployment id: <sha> description changed. Eight testcases are affected (explicit_api, explicit_api_openapi_3, api_with_openapi_definition_body_no_flag, api_with_swagger_and_openapi_with_auth, function_with_alias_and_event_sources, global_handle_path_level_parameter, globals_for_api, intrinsic_functions) across the three partition folders. I rewrote just those identifiers rather than regenerating the files, so the diff stays reviewable.

Testing

Verified on develop at d24401d, Python 3.12:

• Added three tests to tests/translator/test_api_resource.py:
  • stage variables are part of the hash input;
  • two stages differing only in variables produce different deployment logical ids (this is the regression test — it fails before the change);
  • a stage with no variables produces the same logical id as one with {}, which is the no-churn guard.
• Before the change: the first two fail. After: all pass.
• tests/translator/ — 2423 passed, 0 failed (42 failed before the fixtures were refreshed).
• Whole suite: 268 failed / 4397 passed with the change versus 268 failed / 4394 passed on an otherwise clean tree — same failures either way (they are pre-existing environment failures in this sandbox, e.g. test_region_configuration, and need network/endpoint data), plus the 3 new passing tests. No regressions.
• ruff check (pinned ruff~=0.15.6) and black --check clean on both changed Python files; git diff --check clean.

One existing test needed a one-line change: test_make_auto_deployable_with_swagger_dict uses stage = MagicMock(), whose .Variables is a truthy MagicMock rather than the unset value a real ApiGatewayStage has, so it now sets stage.Variables = None to express "no stage variables".

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

[aws-sam-tooling-bot]

Code Review Results

Reviewed: d24401d..c9e3efb
Files: 26 (1 source, 1 test, 24 expected-output fixtures)
Comments: 1

[aws-sam-tooling-bot]

[BUG] The hash uses the raw, unresolved stage.Variables, so the fix misses the common case where stage variables are parameter-driven. SamApi.to_cloudformation (samtranslator/model/sam_resources.py:1845-1847) runs resolve_parameter_refs over BinaryMediaTypes, Domain and Auth before constructing the generator, but never over self.Variables — it is passed through verbatim (line 1857) and lands on the stage unchanged (api_generator.py:463, and visible in tests/translator/output/intrinsic_functions.json:265 where the variables stay as Fn::Sub/Fn::Join).

So for a template like:

Parameters:
 EndpointUri:
   Type: String
Resources:
 MyApi:
   Type: AWS::Serverless::Api
   Properties:
     StageName: prod
     Variables:
       EndpointUri: !Ref EndpointUri

the hash input is always {"EndpointUri": {"Ref": "EndpointUri"}} regardless of the deployed parameter value. Deploying with a new value for EndpointUri still yields the same deployment logical id, which is exactly the failure described in #3703: the variable change is not deployed, and UpdateStage points the stage back at the SAM-known deployment. Domain does not have this problem precisely because it is resolved first.

Resolving the variables for hashing purposes would close the gap. Note that Domain is resolved by reassigning self.Domain, which also rewrites the emitted template; for Variables that would inline parameter values into the output AWS::ApiGateway::Stage, so prefer resolving into a separate value used only for the hash rather than mutating self.Variables.

Everything else checks out: stage.Variables is assigned before make_auto_deployable is called (api_generator.py:463 vs 470), that is the only call site, {} and None both fall through the if so no-variable templates keep their logical id, and the 24 fixture updates cover exactly the 8 input templates that set API stage variables across all three partitions, with the logical id, Description digest and Stage.DeploymentId ref updated consistently in each.

[warren830]

Good catch — this was a real gap and I have fixed it in a8fee86e. (Replying here as a top-level comment rather than inline under the review thread, for tooling reasons on my side; it addresses the [BUG] note on samtranslator/model/apigateway.py:139.)

I reproduced it before changing anything: transforming a template whose stage variable is {"Ref": "EndpointUri"} with two different parameter values produced the same deployment logical id (GetHtmlApiDeployment...), so the original fix did nothing for the parameter-driven case, exactly as you described.

What changed:

• SamApi.to_cloudformation now resolves the stage variables for hashing purposes and passes them to make_auto_deployable as a new keyword-only stage_variables argument.
• As you recommended, self.Variables is not reassigned the way Domain is. resolve_parameter_refs mutates its argument and inlines values, so it resolves a copy.deepcopy and the resolved form is used only for the hash; the emitted AWS::ApiGateway::Stage keeps the customer's Ref.
• make_auto_deployable falls back to the stage's own variables when the argument is not supplied, so direct callers keep their current behaviour.

Tests added in tests/translator/test_api_resource.py:

• test_redeploy_when_parameter_driven_stage_variable_changes — two parameter values must give different deployment ids, and the same value must stay stable so an unchanged stack does not churn. It fails on the pre-fix code with assert first != second / "Both sets are equal", and passes after.
• test_parameter_driven_stage_variables_are_not_inlined_into_stage — asserts the emitted Stage still holds {"EndpointUri": {"Ref": "EndpointUri"}}, guarding the mutation hazard you flagged.

Fixtures: your point is already covered by the repo's own corpus — tests/translator/input/explicit_api.yaml sets EndpointUri: {Ref: something}, so the GetHtmlApi deployment id changed in the six explicit_api / explicit_api_openapi_3 expected outputs. I substituted only the logical id, its Ref, and the Description digest in each file rather than regenerating them, so the diff shows only those three lines per fixture — and the Stage's Variables block is visibly untouched, which is the inlining check in fixture form.

Verification: tests/translator/ is 2425/2425. Full suite compared against the branch point is 268 failed / 4399 passed versus 268 failed / 4397 passed at baseline — the failure set is identical line for line (pre-existing, NoRegionFound on a machine with no configured region) and the two extra passes are these tests. ruff (pinned 0.15.6), black and mypy --strict are clean, with mypy reporting the same 15 pre-existing errors as baseline and none in the changed files.

Two notes on judgement calls, in case you would rather they went the other way:

1. make_auto_deployable crossed the PLR0913 argument limit, so it now carries # noqa: PLR0913, matching the 13 existing uses of that idiom in samtranslator/. The alternative was threading the value through the translator-level redeploy_restapi_parameters dict, which is shared across APIs and seemed worse than one more argument.
2. I pinned region resolution inside the new test helper rather than inheriting the ambient session, because the two pre-existing transform-based tests in this same file fail with NoRegionFound when the whole suite runs on a machine without a configured region. The new tests do not have that dependency.

Also worth flagging what I did not change: HttpApi and WebSocketApi go through ApiV2Generator and are untouched, so this stays scoped to the REST path in #3703.

[warren830]

Review nudge — this PR is now fully green and has no open feedback.

• The automated lambda-tooling pr reviewer check went from failure to success: it reports "No issues found. The changes look good." on the current head. Its earlier failure was a genuine [BUG] finding, not infrastructure noise, and I have addressed it (see the comment above) rather than arguing it away.
• Branch is up to date with develop, and mergeable is true.
• Scope: 28 files, +264/-109 — 2 source files, 1 test file, and 25 expected-output fixtures where only the deployment logical id, its Ref and the Description digest changed.

It fixes #3703 (type/bug, stage/bug-repro, maintainer-reproduced): the API deployment hash omitted the stage Variables, so a variables-only change reused the existing deployment — the new variables were never deployed, and UpdateStage pointed the stage back at the deployment SAM already knew about, reverting anything deployed outside SAM since. The follow-up commit extends that to parameter-driven variables, which hashed identically for every parameter value.

Verification: tests/translator/ is 2425/2425. Against the branch point the full suite is 268 failed / 4399 passed versus 268 failed / 4397 passed at baseline — the failure set matches line for line (pre-existing, NoRegionFound without a configured region) and the two extra passes are the new tests. Both new tests fail on the pre-fix code and pass after. ruff (pinned 0.15.6), black and mypy --strict are clean, mypy reporting the same 15 pre-existing errors as baseline and none in the changed files.

Two things worth a maintainer's eye, since they are judgement calls rather than mechanics:

1. Stacks that already set API stage variables will get one deployment on the next sam deploy, because their deployment logical id changes. That is inherent to including the variables in the hash — it is the fix — but it is a visible one-time effect and you may want it called out in release notes. Templates with no stage variables keep their existing logical id, and there is a test pinning that.
2. make_auto_deployable crossed the PLR0913 argument limit and now carries # noqa: PLR0913, matching the 13 existing uses in samtranslator/. The alternative was routing the value through the shared redeploy_restapi_parameters dict; I judged one more argument cleaner, but I will restructure it if you prefer.

Happy to rebase, split the fixture churn into its own commit, or adjust anything else that would make review easier.