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.

Sign up for GitHub

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

Jump to bottom

Deprecate securityGroups instance argument #852

Closed
clstokes opened this issue Jan 17, 2020 · 4 comments · Fixed by #902
Closed

Deprecate securityGroups instance argument #852

clstokes opened this issue Jan 17, 2020 · 4 comments · Fixed by #902
Assignees
@stack72
Milestone
0.33

Comments

@clstokes
Copy link

securityGroups is a legacy setting from EC2's "classic" days and is almost always the wrong argument for users to use. When securityGroups is used, any changes to the list of security groups will result in replacing the instance (this is an EC2 limitation). Instead users should use vpcSecurityGroupIds which allows for changes.

Recommendation: can we deprecate securityGroups and/or log a warning when it is used to discourage its use?
@stack72
Copy link
Contributor

stack72 commented Jan 18, 2020

@clstokes this doesn't feel like the correct fix IMO - this parameter is allowed because of EC2 classic - terraform still supports that.

If we mark as deprecated then we may be adding confusion if they try and use EC2-Classic

Thoughts?

@clstokes
Copy link
Author

vpcSecurityGroupIds works for EC2-Classic too, so it should always be used. Using securityGroups is the wrong argument for nearly all users.

Given this, I think deprecating securityGroups is the right move.

@stack72
Copy link
Contributor

stack72 commented Jan 21, 2020

ok, can you tell me what message you would like the deprecation notice to have? I will be able to do it once pulumi/pulumi-terraform-bridge#96 is merged

@stack72 stack72 added this to the 0.31 milestone Jan 21, 2020
@stack72 stack72 self-assigned this Jan 21, 2020
@clstokes
Copy link
Author

How about "Use of securityGroups is discouraged as it does not allow for changes and will force your instance to be replaced if changes are made. To avoid this, use vpcSecurityGroupIds which allows for updates." ?

Can we "parameterize" the attribute names vpcSecurityGroupIds for the different languages?

@lukehoban lukehoban modified the milestones: 0.31, 0.33 Feb 1, 2020
@lukehoban lukehoban mentioned this issue Mar 10, 2020
Closed
clstokes added a commit to pulumi/examples that referenced this issue Mar 10, 2020
@clstokes
Update aws-go-webserver to use VpcSecurityGroupIds, not `Security…
9b1d5b5 
…Groups`.

`SecurityGroups` doesn't allow for updates while `VpcSecurityGroupIds` does.

See pulumi/pulumi-aws#852 for more context.
@clstokes clstokes mentioned this issue Mar 10, 2020
Merged
clstokes pushed a commit to pulumi/examples that referenced this issue Mar 10, 2020
Update aws-go-webserver to use VpcSecurityGroupIds, not `Security…
8e6afac 
…Groups`. (#596)

`SecurityGroups` doesn't allow for updates while `VpcSecurityGroupIds` does.

See pulumi/pulumi-aws#852 for more context.
clstokes added a commit to pulumi/examples that referenced this issue Mar 10, 2020
@clstokes
Use vpc_security_group_ids instead of security_groups.
9bb30cb 
`security_groups` doesn't allow for updates while `vpc_security_group_ids` does. See pulumi/pulumi-aws#852 for more context.

Also, updated `Buffer` usage based on https://nodejs.org/fr/docs/guides/buffer-constructor-deprecation/.
@clstokes clstokes mentioned this issue Mar 10, 2020
Merged
stack72 pushed a commit to pulumi/examples that referenced this issue Mar 11, 2020
Use vpc_security_group_ids instead of security_groups. (#600)
9df507d 
`security_groups` doesn't allow for updates while `vpc_security_group_ids` does. See pulumi/pulumi-aws#852 for more context.

Also, updated `Buffer` usage based on https://nodejs.org/fr/docs/guides/buffer-constructor-deprecation/.
stack72 added a commit that referenced this issue Mar 20, 2020
@stack72
Add deprecation message to ec2.Instance securityGroups
065bcbe 
Fixes: #852
stack72 added a commit that referenced this issue Mar 20, 2020
@stack72
Add deprecation message to ec2.Instance securityGroups
084eda5 
Fixes: #852
@stack72 stack72 mentioned this issue Mar 20, 2020
Merged
stack72 added a commit that referenced this issue Mar 20, 2020
@stack72
Add deprecation message to ec2.Instance securityGroups
53891c1 
Fixes: #852
stack72 added a commit that referenced this issue Mar 20, 2020
@stack72
Add deprecation message to ec2.Instance securityGroups (#902)
170eba6 
Fixes: #852
This was referenced May 30, 2020
Closed
Closed
dixler pushed a commit to pulumi/examples that referenced this issue Jan 21, 2022
Update aws-go-webserver to use VpcSecurityGroupIds, not `Security…
49b5b37 
…Groups`. (#596)

`SecurityGroups` doesn't allow for updates while `VpcSecurityGroupIds` does.

See pulumi/pulumi-aws#852 for more context.
dixler pushed a commit to pulumi/examples that referenced this issue Jan 21, 2022
Use vpc_security_group_ids instead of security_groups. (#600)
b4538bb 
`security_groups` doesn't allow for updates while `vpc_security_group_ids` does. See pulumi/pulumi-aws#852 for more context.

Also, updated `Buffer` usage based on https://nodejs.org/fr/docs/guides/buffer-constructor-deprecation/.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@stack72 stack72

Labels
None yet
Projects
None yet
Milestone
0.33
Development

Successfully merging a pull request may close this issue.

Add deprecation message to ec2.Instance securityGroups pulumi/pulumi-aws
3 participants
@lukehoban @stack72 @clstokes