[BUG] Crash when providing an int tag directly to a cfngin stack definition
jake-skipper opened this issue · 5 comments
Bug Description
Providing a tag such as CostCenter: 1234
and passing it directly in to a cfngin stack causes a crash due to Boto doing type validation on tag values (and only allowing str
). This does not occur when using the top level tags
field.
For instance, this works without issues:
namespace: ${namespace}
cfngin_bucket: ""
tags:
CostCenter: 1234
stacks:
example-stack:
template_path: template.yaml
And this will crash with a type error:
namespace: ${namespace}
cfngin_bucket: ""
stacks:
example-stack:
template_path: template.yaml
tags:
CostCenter: 1234
We should probably determine what type of int
to str
conversion is happening to the top level tags
field and apply the same to the stack level tags
field.
In the meantime, a workaround is to quote the offending tag:
namespace: ${namespace}
cfngin_bucket: ""
stacks:
example-stack:
template_path: template.yaml
tags:
CostCenter: "1234"
Expected Behavior
Tag value handling at the stack level should be the same as the top level tags
field.
Steps To Reproduce
Already provided in description.
Runway version
2.6.7
Installation Type
pypi (pip, pipenv, poetry, etc)
OS / Environment
N/A
Anything else?
N/A
Further information needed about error message produced.
I have been able to reproduce the different behavior between defining tags at the root level versus the stack level. They do both fail now after the pydantic conversion.
The root level error is now:
[runway] 1 validation error for CFNgin Config File
tags.CostCenter
Input should be a valid string [type=string_type, input_value=1234, input_type=int]
For further information visit https://errors.pydantic.dev/2.8/v/string_type
and the stack level error is:
[runway] Parameter validation failed:
Invalid type for parameter Tags[1].Value, value: 1234, type: <class 'int'>, valid types: <class 'str'>
Even though the errors are different, they do now at least both indicate the same action to be taken, which is use a string value. I think this is sufficient to indicate what needs to be done to address the issue.
Do you agree @jake-skipper ?
Looks good to me. Most of the time I ran into this was during Runway upgrades, and it was very confusing when some would work and others fail. If it fails consistently now (and has a nice readable error) I think that's plenty resolved.
An enhancement might be to convert it to a string rather than have the user worry about it, but I don't know how big that lift is.
This is Jake btw, personal GitHub account lol.
After further discussion, auto-conversion as proposed may make troubleshooting more difficult in certain cases and is best to let this return an error if input is not a string.