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

Support "UserName" property for AWS::IAM::User #529

Merged
merged 1 commit into from Aug 14, 2016

Conversation

hypertext418
Copy link
Contributor

@markpeek
Copy link
Member

markpeek commented Aug 1, 2016

This looks good. It would be good to validate the length not being the empty string explicitly. Also, some positive/negative tests for the length and regex would be a welcome addition.

@hypertext418
Copy link
Contributor Author

@markpeek is troposphere moving towards doing its own validation? In most places it is 'dumb' and has AWS do the validation. I was hesitant to even add validation as a result — what's the pattern? Is there a plan to gradually add validators for everything? If not I think it would be better to leave them out entirely

@markpeek
Copy link
Member

markpeek commented Aug 2, 2016

Besides being able to write templates in Python, the original intent for troposphere was to do as much validation at template generation time as possible to prevent having CF rollbacks from occurring. You're right, not everything is validated but there are both base and custom validators available and I'd welcome people contributing more validation. To the point of my comment, since you had some validation (length of string, regex) I thought it would be good to catch an empty string and also add tests to ensure the correct behavior was occurring.

@markpeek markpeek merged commit 530d459 into cloudtools:master Aug 14, 2016
@markpeek
Copy link
Member

Thanks. I added the empty string check and some unit tests in the above commit.

@hypertext418
Copy link
Contributor Author

@markpeek Thanks! Sorry I didn't get to this before I went on vacation

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants