-
Notifications
You must be signed in to change notification settings - Fork 195
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
Bug: serverless.Function_S3Location's Version field should be string type, not int #396
Comments
Hey Ryan, Thanks for reporting this - you're 100% correct. I've had a look into possible solutions for this, but i'm struggling to find something that works well for all use cases. Because of the large number of SAM/CFN templates that specify as a number, we need to support parsing from both number: MyFunction:
Type: AWS::Serverless::Function
Properties:
Runtime: nodejs
CodeUri:
Bucket: testbucket
Key: testkey.zip
Version: 5 and string: MyFunction:
Type: AWS::Serverless::Function
Properties:
Runtime: nodejs
CodeUri:
Bucket: testbucket
Key: testkey.zip
Version: "5" If we simply change the type Function_S3Location struct {
Bucket string `json:"Bucket,omitempty"`
Key string `json:"Key,omitempty"`
Version string `json:"Version,omitempty"`
} Then any SAM/CFN templates that specify the value as a JSON/YAML number (such as the first example above), will fail to parse: The only solution I can think of would be to create an intermediate type with a custom JSON unmarshaller that can accept both number and string formats. Something like this StackOverflow solution. This has a pretty big downside though - it means that when composing templates in Go code, you can no longer just do: resource := &serverless.Function{
Runtime: "nodejs6.10",
CodeUri: &serverless.Function_CodeUri{
S3Location: &serverless.Function_S3Location{
Bucket: "test-bucket",
Key: "test-key",
Version: "123",
},
},
} Instead for any place in the goformation API where a number is used (which can be specified as a string or number in JSON/YAML due to CFN's relaxed typing), we would need to do something like this: type Function_S3Location struct {
Bucket StringOrNumber `json:"Bucket,omitempty"`
Key StringOrNumber `json:"Key,omitempty"`
Version StringOrNumber `json:"Version,omitempty"`
} resource := &serverless.Function{
Runtime: "nodejs6.10",
CodeUri: &serverless.Function_CodeUri{
S3Location: &serverless.Function_S3Location{
Bucket: cloudformation.String("test-bucket"),
Key: cloudformation.String("test-key"),
Version: cloudformation.Number(123),
},
},
} ... which would be a major change to the API of the library - although not necessarily a bad one, as it would allow us to move to using pointers instead of values, and therefore solve a number of open issues in this library to do with null values. Unless you can think of a smarter way of handling this? |
This code is wrong:
goformation/cloudformation/serverless/aws-serverless-function_s3location.go
Line 24 in d56929b
This is the official doc. It should be
string
, notint
.https://docs.aws.amazon.com/serverless-application-model/latest/developerguide/sam-property-function-functioncode.html#sam-function-functioncode-version
The text was updated successfully, but these errors were encountered: