cloudtools / troposphere

troposphere - Python library to create AWS CloudFormation descriptions
BSD 2-Clause "Simplified" License
4.93k stars 1.45k forks source link

Support integers and booleans #371

Open nikolay opened 8 years ago

nikolay commented 8 years ago

A lot of settings in CloudFormation are declared to accept only strings, but they, in fact, accept integers nad booleans. All my templates have numbers as integers, not as strings, and I've never had issues. Unfortunately, now, after I'm finally using troposphere, I need to use strings as you guys are strict about this.

Is it possible to enable a mode where either:

wt commented 8 years ago

Are you talking about Parameters or are you talking about Resources? I see that for my Resources, integers are integers. For Parameters, I do have things like strings that are "True" or "False".

nikolay commented 8 years ago

@wt They should be allowed anywhere - even if they are normalized to strings at the end. I'm using Troposphere on a small project and I have this error only with Elastic Beanstalk parameters, which is the only place I have integers and booleans, so, I haven't tried using them elsewhere, but I think the validator is pretty strict.

markpeek commented 8 years ago

@nikolay is there a small example you can share to show this issue? There are different validators which allow for integers so perhaps this is just an issue with something in Elastic Beanstalk.

nikolay commented 8 years ago

@markpeek @wt Here's some code:

elasticbeanstalk.OptionSettings(
    Namespace='aws:elb:healthcheck',
    OptionName='HealthyThreshold',
    Value=str(healthy_threshold),
)

As you see here, the option healthy_threshold is naturally an integer and I have to explicitly entertain the weirdness of OptionSettings having my arms twisted to convert it to a string.

Again, the CloudFormation validator is not as strict as Troposphere and where in the documentation it says that you need to supply a string, it also takes integers and booleans without giving errors or warnings.

nikolay commented 8 years ago

@wt @markpeek No feedback?

markpeek commented 8 years ago

Sorry, I missed your update to this issue.

troposphere is intentionally strict to catch errors prior to submitting to CF. In this case the documentation states a string value is required. I'd have to contemplate whether coercing all non-string values to strings when basestring is specified would be the right thing across the board. In some ways having user code handle that for this case might be a better approach. I've tried to adhere to the docs where a required integer must be an integer.

nikolay commented 8 years ago

@markpeek I totally understand and CloudFormation converting everything to a string is a big nonsense, but it's an undocumented feature, I agree. I think one day they will be typesafe. And I think having to convert integer values to strings all the time is actually a drawback for using Troposphere as when I use JSON programmatically I don't have to.

phobologic commented 8 years ago

Maybe we should reach out to the CF folks and ask what their suggestion is - it might be that they consider the allowance of integers as a bug that they need to fix, in which case I think sticking with the documentation is the right thing.

Also - are we sure this behavior is consistent across all Cloudformation resources? Seems like it'd be hard to be sure of that without verification from AWS themselves.

nikolay commented 8 years ago

@phobologic I'm not saying we integers should be output - they still can be converted to strings in the JSON if you want to be that strict but, at least, allow them in the API, because it's not Pythonic to have to convert everything to a string, which is not a string internally and shouldn't be. Maybe you guys keep, for example, port numbers as strings, which is bad - I don't, and it's terrible to have to convert them every time I use them! But I agree that we should bring their attention. For example, I have JSON configuration files maintained via cfn-init and because every JSON leaf is force-converted to a string in CloudFormation, those files end up having strings everywhere, which may not be supported by the different software. In general, I don't get why Amazon did this, but it definitely does not make sense and leads to promoting bad practices!

markpeek commented 8 years ago

In general we allow and validate integers for integer objects within CF. Check this with grep -r integer troposphere. The issue with elasticbeanstalk.OptionSettings is the use of opaque information that accepts arbitrary values and we can only rely on validating against basestring. The way to have better validation for this specific attribute would be to create a custom validator function that would have a table of name to type to validate against.