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

Port user_data length validation to aws_launch_configuration #2973

Merged

Conversation

alewando
Copy link
Contributor

Port user_data length validation from #2765 to aws_launch_configuration

@bflad bflad added enhancement Requests to existing resources that expand the functionality or scope. size/XS Managed by automation to categorize the size of a PR. service/autoscaling Issues and PRs that pertain to the autoscaling service. labels Jan 12, 2018
@bflad
Copy link
Contributor

bflad commented Jan 24, 2018

@alewando interestingly enough, in the AWS Autoscaling API documentation, the limit for the UserData attribute is actually 21847. 🙁

Length Constraints: Maximum length of 21847.

Can you validate or preferably write an acceptance test to prove that the shorter value of 16384 is also correct for launch configuration user data? If not, can you create a separate validation method for this attribute please? Some people might be using values in between the two.

@bflad bflad added the waiting-response Maintainers are waiting on response from community or contributor. label Jan 24, 2018
@alewando
Copy link
Contributor Author

I believe the 21847 byte limit is for base64-encoded data (16384 * 1.3333 = 21846), but will verify manually. Terraform base64-encodes the supplied userdata, so I suspect the limit is actually 16kb of binary data.

@ghost ghost added the size/XS Managed by automation to categorize the size of a PR. label Feb 11, 2018
@alewando
Copy link
Contributor Author

alewando commented Feb 11, 2018

I have verified manually that the maximum allowable size for user_data on a launch configuration is 16384 bytes (same as for EC2 instances themselves). I verified by creating files of controlled sizes and using them as template data sources for a launch configuration's user_data. Files were created via:

awk 'BEGIN {while (c++<__DESIRED_SIZE__) printf "x"}'>user-data.tpl

For any file size larger than 16834, AWS failed with an error message:
* aws_launch_configuration.lc: Error creating launch configuration: ValidationError: 1 validation error detected: Value 'xxx...xxx' at 'userData' failed to satisfy constraint: Member must have length less than or equal to 21847

Writing an acceptance test to prove this would be awkward, since the whole point of the PR is to
apply this validation prior to sending the request to AWS. An acceptance test would need to provide some way of disabling this validation, which seems like it would be complicating the user-facing implementation for the purpose of validating documented (albeit poorly) external system behavior.

@bflad bflad removed the waiting-response Maintainers are waiting on response from community or contributor. label Mar 17, 2018
@bflad bflad added this to the v1.12.0 milestone Mar 17, 2018
Copy link
Contributor

@bflad bflad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks so much for this contribution! Sorry for my delay merging this. 😖

Test failure unrelated

Tests failed: 1, passed: 8
=== RUN   TestAccAWSLaunchConfiguration_withVpcClassicLink
--- FAIL: TestAccAWSLaunchConfiguration_withVpcClassicLink (6.77s)
    testing.go:518: Step 0 error: Error applying: 1 error(s) occurred:
        
        * aws_launch_configuration.foo: 1 error(s) occurred:
        
        * aws_launch_configuration.foo: Error creating launch configuration: AlreadyExists: Launch Configuration by this name already exists - A launch configuration already exists with the name TestAccAWSLaunchConfiguration_withVpcClassicLink
            status code: 400, request id: b59341ba-2a10-11e8-a667-cdd28445ed38
=== RUN   TestAccAWSLaunchConfiguration_withEncryption
--- PASS: TestAccAWSLaunchConfiguration_withEncryption (7.42s)
=== RUN   TestAccAWSLaunchConfiguration_withBlockDevices
--- PASS: TestAccAWSLaunchConfiguration_withBlockDevices (7.76s)
=== RUN   TestAccAWSLaunchConfiguration_withSpotPrice
--- PASS: TestAccAWSLaunchConfiguration_withSpotPrice (7.86s)
=== RUN   TestAccAWSLaunchConfiguration_importBasic
--- PASS: TestAccAWSLaunchConfiguration_importBasic (13.17s)
=== RUN   TestAccAWSLaunchConfiguration_updateRootBlockDevice
--- PASS: TestAccAWSLaunchConfiguration_updateRootBlockDevice (13.60s)
=== RUN   TestAccAWSLaunchConfiguration_updateEbsBlockDevices
--- PASS: TestAccAWSLaunchConfiguration_updateEbsBlockDevices (15.80s)
=== RUN   TestAccAWSLaunchConfiguration_basic
--- PASS: TestAccAWSLaunchConfiguration_basic (17.03s)
=== RUN   TestAccAWSLaunchConfiguration_withIAMProfile
--- PASS: TestAccAWSLaunchConfiguration_withIAMProfile (26.55s)

@bflad bflad merged commit c2c77de into hashicorp:master Mar 17, 2018
bflad added a commit that referenced this pull request Mar 17, 2018
@bflad
Copy link
Contributor

bflad commented Mar 23, 2018

This has been released in version 1.12.0 of the AWS provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

@Zordrak
Copy link

Zordrak commented May 3, 2018

I could do with some clarification of what is going on here.

Every project everywhere is suddently having to change its default user data value from an empty string to a a single space character as a workaround for this validation.

Is this intentional? What prevents an empty string from being a valid way to define no userdata requirement?

@alewando
Copy link
Contributor Author

alewando commented May 3, 2018

No, that wasn't the intent, and this pull request did not introduce that issue. The new validation requiring a minimum length of 1 (when user_data is present) came from #4037 (commit f147424) by @radeksimko .
One potential workaround: if you don't have any user data you can omit the user_data attribute from the launch configuration.

@Zordrak
Copy link

Zordrak commented May 3, 2018

I will ask the question on #4037 . I appreciate that you can omit the user_data attribute in one case, but in every single deployment we have autoscaling groups are part of a microservice module that takes userdata information as a parameter. Some of our microservices have userdata and some don't and as a result of this change, those that don't fail to validate. The workaround of changing the default from "" to " " would work.. but will cause a recreation of all launch configurations that have no userdata.

@ghost
Copy link

ghost commented Apr 6, 2020

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thanks!

@ghost ghost locked and limited conversation to collaborators Apr 6, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement Requests to existing resources that expand the functionality or scope. service/autoscaling Issues and PRs that pertain to the autoscaling service. size/XS Managed by automation to categorize the size of a PR.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants