Open ryan-ju opened 3 years ago
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 S3Location
struct to use String
, like so:
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:
json: cannot unmarshal number into Go struct field Function_S3Location.Version of type string
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: https://github.com/awslabs/goformation/blob/d56929b269373078275877578e19f2f554ac6eff/cloudformation/serverless/aws-serverless-function_s3location.go#L24
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