Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[sam] 'version' field should be optional but is mandatory #3841

Closed
nsstensoriot opened this issue Aug 28, 2019 · 5 comments
Closed

[sam] 'version' field should be optional but is mandatory #3841

nsstensoriot opened this issue Aug 28, 2019 · 5 comments
Assignees
Labels
@aws-cdk/aws-sam Related to AWS Serverless Application Model bug This issue is a bug. in-progress This issue is being actively worked on. package/cfn Related to the CFN layer (L1)

Comments

@nsstensoriot
Copy link

🐛 Bug Report

What is the problem?

When using the Python aws_sam module, and trying to declare a CfnApi object, the definition_uri is requiring 'version', when the documentation says it should be an optional field.

Reproduction Steps

Try to create a Python CfnApi object instance, like so:

        self.serverlessAPI = sam.CfnApi(
            scope=self.scope,
            id=apiName,
            name=apiName,
            stage_name=self.scope.config['ApiGatewayStageName'],
            definition_uri={'bucket': mySwaggerS3File.s3_bucket_name, 'key': mySwaggerS3File.s3_object_key}
        )

If you run 'cdk synth', it'll error out. The log is provided below.

Verbose Log

Traceback (most recent call last):
  File "stack.py", line 72, in <module>
    myStack = BaseStack(app)
  File "/Users/harmony/project/cdkstack/env/lib/python3.7/site-packages/jsii/_runtime.py", line 66, in __call__
    inst = super().__call__(*args, **kwargs)
  File "stack.py", line 33, in __init__
    self.stackApiGateway = stackApiGateway.APIGatewayResources(self, './swagger/swagger.yaml')
  File "/Users/harmony/project/cdkstack/stackModules/apiGateway.py", line 18, in __init__
    definition_uri={'bucket': mySwaggerS3File.s3_bucket_name, 'key': mySwaggerS3File.s3_object_key}
  File "/Users/harmony/project/cdkstack/env/lib/python3.7/site-packages/jsii/_runtime.py", line 66, in __call__
    inst = super().__call__(*args, **kwargs)
  File "/Users/harmony/project/cdkstack/env/lib/python3.7/site-packages/aws_cdk/aws_sam/__init__.py", line 45, in __init__
    jsii.create(CfnApi, self, [scope, id, props])
  File "/Users/harmony/project/cdkstack/env/lib/python3.7/site-packages/jsii/_kernel/__init__.py", line 208, in create
    overrides=overrides,
  File "/Users/harmony/project/cdkstack/env/lib/python3.7/site-packages/jsii/_kernel/providers/process.py", line 331, in create
    return self._process.send(request, CreateResponse)
  File "/Users/harmony/project/cdkstack/env/lib/python3.7/site-packages/jsii/_kernel/providers/process.py", line 316, in send
    raise JSIIError(resp.error) from JavaScriptError(resp.stack)
jsii.errors.JSIIError: Value did not match any type in union: Expected Scalar, got {"bucket":"${Token[TOKEN.71]}","key":"${Token[TOKEN.74]}${Token[TOKEN.76]}"},Expected object reference, got {"bucket":"${Token[TOKEN.71]}","key":"${Token[TOKEN.74]}${Token[TOKEN.76]}"},Missing required properties for @aws-cdk/aws-sam.CfnApi.S3LocationProperty: version

Environment

  • CDK CLI Version: 1.6.0 (build 3a0cde0)
  • Module Version: aws-cdk.aws-sam==1.6.0
  • OS: OSX Mojave
  • Language: Python

Other information

Here's the documentation that says the 'version' for the bucket should be optional:
https://github.com/awslabs/serverless-application-model/blob/master/versions/2016-10-31.md#s3-location-object

@nsstensoriot nsstensoriot added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Aug 28, 2019
@SomayaB SomayaB added language/python Related to Python bindings @aws-cdk/aws-sam Related to AWS Serverless Application Model labels Sep 3, 2019
@SomayaB SomayaB added the needs-reproduction This issue needs reproduction. label Sep 11, 2019
@nija-at nija-at removed language/python Related to Python bindings needs-reproduction This issue needs reproduction. needs-triage This issue or PR still needs to be triaged. labels Sep 24, 2019
@nija-at
Copy link
Contributor

nija-at commented Sep 24, 2019

Confirmed that this is an issue and is not isolated to Python.

It looks like SAM schema does state that the version field of the AWS::Serverless::Api.S3Location is not mandatory. However, CDK enforces the presence of this field.

The issue is coming from the SAM schema defined in the Goformation repo. CDK uses this as the SAM schema to generate its language bindings.

@nija-at
Copy link
Contributor

nija-at commented Sep 24, 2019

There's an existing discussion on this in the Goformation repo - awslabs/goformation#87

@nija-at nija-at added package/cfn Related to the CFN layer (L1) cfn-bug and removed cfn-bug labels Sep 24, 2019
@nija-at nija-at changed the title aws_sam.CfnApi definition_uri requires 'version' when the documentation says it should be optional [sam] 'version' field should be optional but is mandatory Oct 18, 2019
@nija-at
Copy link
Contributor

nija-at commented Oct 18, 2019

This has fixed in goformation. Awaiting propagation to CDK.

@NGL321 NGL321 added in-progress This issue is being actively worked on. and removed cfn-bug labels Nov 5, 2019
@nija-at
Copy link
Contributor

nija-at commented Nov 7, 2019

Change is now in the CDK - fd167c3#diff-2a4aee8c725dbbe91207a037eecbb2dd

Closing.

@nija-at nija-at closed this as completed Nov 7, 2019
@prestomation
Copy link

This was never fixed. The original issue is about AWS::Serverless::Api.S3Location, but the fix was for AWS::Serverless::Function.S3Location.

Can we get this same fix for the Api resource?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-sam Related to AWS Serverless Application Model bug This issue is a bug. in-progress This issue is being actively worked on. package/cfn Related to the CFN layer (L1)
Projects
None yet
Development

No branches or pull requests

5 participants