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

AutoScalingGroupProps.AssociatePublicIpAddress has no effect #21576

Closed
icelava opened this issue Aug 12, 2022 · 6 comments · Fixed by #21714
Closed

AutoScalingGroupProps.AssociatePublicIpAddress has no effect #21576

icelava opened this issue Aug 12, 2022 · 6 comments · Fixed by #21714
Assignees
Labels
@aws-cdk/aws-autoscaling Related to Amazon EC2 Auto Scaling bug This issue is a bug. effort/small Small work item – less than a day of effort in-progress This issue is being actively worked on. p2

Comments

@icelava
Copy link

icelava commented Aug 12, 2022

Describe the bug

https://docs.aws.amazon.com/cdk/api/v2/dotnet/api/Amazon.CDK.AWS.AutoScaling.AutoScalingGroupProps.html#Amazon_CDK_AWS_AutoScaling_AutoScalingGroupProps_AssociatePublicIpAddress

Setting property to false still results in instances launched with public IP address.

Expected Behavior

Launched instances should not assign public IP address.

Current Behavior

Launched instances get public IP address.

Reproduction Steps

this.asg = new AutoScalingGroup(this, this.asgName, new AutoScalingGroupProps
{
	AutoScalingGroupName = this.asgName,
	LaunchTemplate = lt,
	Vpc = this._props.Vpc,
	VpcSubnets = this._props.AppSubnets,
	AssociatePublicIpAddress = false,
	MinCapacity = this._props.MinCapacity,
	MaxCapacity = this._props.MaxCapacity,
	DesiredCapacity = this._props.DesiredCapacity
});

Possible Solution

That property should cause a change in the resultant CloudFormation template.

Additional Information/Context

No response

CDK CLI Version

2.35.0

Framework Version

.NET Core 3.1

Node.js Version

NA

OS

Windows

Language

.NET

Language Version

C# 8

Other information

No response

@icelava icelava added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Aug 12, 2022
@github-actions github-actions bot added the @aws-cdk/aws-autoscaling Related to Amazon EC2 Auto Scaling label Aug 12, 2022
@peterwoodworth
Copy link
Contributor

We're passing this prop directly to the LaunchTemplate resource - You can see a more full description of the underlying behavior of this prop here

That property should cause a change in the resultant CloudFormation template.

Does this mean that if you alternate this value between true and false that you aren't seeing a difference in the resulting template?

@peterwoodworth peterwoodworth added response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. and removed needs-triage This issue or PR still needs to be triaged. labels Aug 12, 2022
@peterwoodworth peterwoodworth added p2 effort/small Small work item – less than a day of effort labels Aug 12, 2022
@peterwoodworth peterwoodworth self-assigned this Aug 12, 2022
@github-actions
Copy link

This issue has not received a response in a while. If you want to keep this issue open, please leave a comment below and auto-close will be canceled.

@github-actions github-actions bot added closing-soon This issue will automatically close in 4 days unless further comments are made. closed-for-staleness This issue was automatically closed because it hadn't received any attention in a while. and removed closing-soon This issue will automatically close in 4 days unless further comments are made. labels Aug 15, 2022
@icelava
Copy link
Author

icelava commented Aug 22, 2022

Yeap it was originally not set, so defaulted to true. When I set it to false, no change in the resulting CloudFormation template.

@mrgrain mrgrain reopened this Aug 22, 2022
@github-actions github-actions bot removed the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. label Aug 22, 2022
@peterwoodworth
Copy link
Contributor

This property isn't supposed to be used when passing in your own launch template to a new ASG. We are supposed to throw an error here, but there's a bug in the logic which prevents that. I've made a PR which updates the documentation for all props which can't be used when specifying a launch template, as well as ensures an error gets thrown when this prop is set to false

@peterwoodworth peterwoodworth added in-progress This issue is being actively worked on. and removed closed-for-staleness This issue was automatically closed because it hadn't received any attention in a while. labels Aug 22, 2022
@icelava
Copy link
Author

icelava commented Aug 23, 2022

Hmmm but the LaunchTemplate does not feature such a property?

@mergify mergify bot closed this as completed in #21714 Aug 25, 2022
mergify bot pushed a commit that referenced this issue Aug 25, 2022
…et to false when specifying launchTemplate (#21714)

Setting `associatePublicIpAddress` prop when also setting `launchTemplate` prop on a new ASG is supposed to throw an error, but doesn't due to `associatePublicIpAddress` being a boolean, and the error message not triggering if a falsy value is used for the prop. This PR will ensure an error is thrown when `associatePublicIpAddress` is set, even when the value passed is falsy. It also updates the documentation on all props which conflict with `launchTemplate` or `mixedInstancesPolicy`

fixes: #21576

----

### All Submissions:

* [ ] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md)

### Adding new Unconventional Dependencies:

* [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies)

### New Features

* [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)?
	* [ ] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)?

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@github-actions
Copy link

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

josephedward pushed a commit to josephedward/aws-cdk that referenced this issue Aug 30, 2022
…et to false when specifying launchTemplate (aws#21714)

Setting `associatePublicIpAddress` prop when also setting `launchTemplate` prop on a new ASG is supposed to throw an error, but doesn't due to `associatePublicIpAddress` being a boolean, and the error message not triggering if a falsy value is used for the prop. This PR will ensure an error is thrown when `associatePublicIpAddress` is set, even when the value passed is falsy. It also updates the documentation on all props which conflict with `launchTemplate` or `mixedInstancesPolicy`

fixes: aws#21576

----

### All Submissions:

* [ ] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md)

### Adding new Unconventional Dependencies:

* [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies)

### New Features

* [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)?
	* [ ] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)?

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-autoscaling Related to Amazon EC2 Auto Scaling bug This issue is a bug. effort/small Small work item – less than a day of effort in-progress This issue is being actively worked on. p2
Projects
None yet
4 participants