fix(integ-tests): remove invalid Auth from API templates using external DefinitionUri - #3967
Conversation
There was a problem hiding this comment.
Code Review Results
Reviewed: 22bb387..528ebb2
Files: 11
Comments: 2
Comments on lines outside the diff:
[integration/resources/templates/single/api_with_custom_domain_security_policy_regional.yaml:13] [BUG] The fix is incomplete. Five more templates from #3963 still combine Auth with an external DefinitionUri and no DefinitionBody, so they hit the exact same unconditional failure in ApiGenerator._add_auth (samtranslator/model/api/api_generator.py:1044-1048):
Auth:
DefaultAuthorizer: AWS_IAM
StageName: Prod
DefinitionUri: ${definitionuri}single/api_with_custom_domain_security_policy_regional.yaml:13single/api_with_custom_domain_security_policy_edge.yaml:13single/api_with_endpoint_access_mode.yaml:13single/api_with_ipaddresstype.yaml:10single/api_with_domain_ipaddresstype.yaml:15
Each is exercised by an active test (test_api_with_custom_domain_security_policy.py, test_api_with_endpoint_access_mode.py, test_api_with_ipaddresstype.py, test_api_with_domain_ipaddresstype.py), all of which call create_and_verify_stack, so they will fail changeset creation for the same reason as TestResourceReferences::test_api_with_resource_references. Remove the Auth block from these five as well.
[integration/resources/code/swagger1.json:1] [GENERAL] If #3963 was driven by a requirement that integration-test APIs not be publicly invokable, this PR reverts that for every external-Swagger API rather than satisfying it. Since these templates all point DefinitionUri at the shared swagger1.json/swagger2.json fixtures, IAM auth can be kept without moving to inline DefinitionBody by declaring it in the fixture itself (securityDefinitions with x-amazon-apigateway-authtype: awsSigv4 plus a top-level security entry), which also preserves the intrinsic-function coverage in intrinsics_code_definition_uri and intrinsics_serverless_api. I verified none of the affected tests assert on authenticated requests, so this is not a test-correctness blocker — only worth doing if the auth requirement is binding.
The 11 template edits themselves are consistent and correct: each removes only the Auth block from an AWS::Serverless::Api that has no DefinitionBody, and no test for those templates depends on IAM auth behavior. The one test that does encode auth-specific expectations (test_basic_api_with_mode, expecting 404 rather than 403) uses basic_api_with_mode.yaml, which is inline and untouched.
528ebb2 to
4a27299
Compare
Issue #, if available
N/A — regression from #3963 surfaced by HydraTests (
TestResourceReferences::test_api_with_resource_references).Description of changes
PR #3963 ("Add IAM auth to API Gateway resources in integration tests") added an
Auth: { DefaultAuthorizer: AWS_IAM }block to a number ofAWS::Serverless::Apiresources. Several of those APIs define their API via an external
DefinitionUri(S3/Swagger) rather than an inline
DefinitionBody.SAM only supports
Authwhen the API is defined inline viaDefinitionBody, since itneeds to inject security definitions into the Swagger. Combining
Authwith an externalDefinitionUrifails the transform with:(validation in
samtranslator/model/api/api_generator.py). This broke changeset creationfor the affected integration tests.
This PR removes the erroneously added
Authblock from the templates that use an externalDefinitionUri, restoring each template to a valid state while preserving its original testcoverage (external-Swagger path, intrinsic functions on
DefinitionUri, etc.).DefinitionUriitself is unchanged — it is valid and intentional in these templates.
Note: converting these to inline
DefinitionBodyto retainAuthwas intentionally avoided,because two templates (
intrinsics_code_definition_uri,intrinsics_serverless_api) existspecifically to exercise intrinsic functions on
DefinitionUri, and none of the affected testsassert on authenticated requests.
Templates updated (removed
Auth, keptDefinitionUri):integration/resources/templates/combination/api_with_resource_refs.yamlintegration/resources/templates/combination/api_with_binary_media_types.yamlintegration/resources/templates/combination/api_with_endpoint_configuration.yamlintegration/resources/templates/combination/api_with_endpoint_configuration_dict.yamlintegration/resources/templates/combination/api_with_method_settings.yamlintegration/resources/templates/combination/function_with_api.yamlintegration/resources/templates/combination/function_with_alias_and_event_sources.yamlintegration/resources/templates/combination/intrinsics_code_definition_uri.yamlintegration/resources/templates/combination/intrinsics_serverless_api.yamlintegration/resources/templates/single/basic_api.yamlintegration/resources/templates/single/basic_api_with_tags.yamlDescription of how you validated changes
Ran the SAM translator against all 11 modified templates (with
${...}code-key placeholderssubstituted): all 11 transform successfully with no errors. A repo-wide scan of every
integration template confirms zero remaining "works only with inline Swagger / DefinitionBody"
errors.
test_api_with_resource_referencesno longer fails on changeset creation.Checklist
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.