-
-
Notifications
You must be signed in to change notification settings - Fork 35
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
chore(deps): update terraform cloudposse/security-group/aws to v2 (main) #64
Merged
max-lobur
merged 18 commits into
main
from
renovate/main-cloudposse-security-group-aws-2.x
Jul 6, 2023
Merged
chore(deps): update terraform cloudposse/security-group/aws to v2 (main) #64
max-lobur
merged 18 commits into
main
from
renovate/main-cloudposse-security-group-aws-2.x
Jul 6, 2023
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Edited/Blocked NotificationRenovate will not automatically rebase this PR, because it does not recognize the last commit author and assumes somebody else may have edited the PR. You can manually request rebase by checking the rebase/retry box above. ⚠ Warning: custom changes will be lost. |
/terratest |
max-lobur
previously approved these changes
Jul 6, 2023
/terratest |
/terratest |
max-lobur
previously approved these changes
Jul 6, 2023
/terratest |
/terratest |
/terratest |
max-lobur
approved these changes
Jul 6, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR contains the following updates:
0.3.1
->2.2.0
Release Notes
cloudposse/terraform-aws-security-group (cloudposse/security-group/aws)
v2.2.0
Compare Source
`.editorconfig` Typo @milldr (#50)
what
fixed intent typo
why
should be spelled "indent"
references
https://cloudposse.slack.com/archives/C01EY65H1PA/p1685638634845009
Sync github @max-lobur (#47)
Rebuild github dir from the template
v2.1.0
Compare Source
v2.0.1
Compare Source
🐛 Bug Fixes
Properly handle enabled = false @Nuru (#45)
what
enabled = false
why
v2.0.0
: Breaking changesCompare Source
For details about migrating from v1 to v2, read the migration documentation.
Version 1 of this module had a flaw in that it tried to create new security group rules before deleting the old ones, which the Terraform provider does not handle properly and caused most attempted changes to fail. Version 2 resolves this issue by also creating a new security group when the rules change, installing the new rules in the new security group, then changing the security group assignments. Read the README and the migration documentation for more details.
Document migration from v1 to v2 @Nuru (#42)
what
why
references
Fixes the link for examples/complete/main.tf @jdmedeiros (#41)
Fixes the link for examples/complete/main.tf on the README.md file.
v1.0.1
Compare Source
🐛 Bug Fixes
Handle `self = false`, add warning about `compact` and `sort` @Nuru (#33)
what
self = false
compact
andsort
why
self = false
in a rule (meaning the rule does not apply to the security group it is being associated with) was causing an empty rule to be created and/or causing conflicts with other rule elements. Now it works likeself = null
works.references
v1.0.0
: Initial release with production Semantic VersioningCompare Source
Initial release with production Semantic Versioning, part of Cloud Posse's general policy to convert to production versioning as we make updates to relatively mature modules.
This version is functionally equivalent to v0.4.3. The only differences are to the support framework (for things like developing and testing the module).
git.io->cloudposse.tools update and test framework update @dylanbannon (#32)
what and why
Change all references to
git.io/build-harness
intocloudposse.tools/build-harness
, sincegit.io
redirects will stop working on April 29th, 2022.References
Rename the exported `security-group-inputs` file to `security-group-variables` @aknysh (#31)
what
security-group-inputs.tf
file tosecurity-group-variables.tf
why
For example, we have
spotinst-variables.tf
for Spot,security-group-variables.tf
for SG, etc.v0.4.3
Compare Source
Update recommended inputs and outputs @Nuru (#26)
what
why
🚀 Enhancements
Rename the exported `security_group_inputs.tf` file to `security-group-inputs.tf` @aknysh (#30)
what
security_group_inputs.tf
file tosecurity-group-inputs.tf
why
kebab-case
for all files. Having a file insnake_case
(after adding it to a repo) together with all the other files inkebab-case
in the same repo does not look correctv0.4.2
Compare Source
🐛 Bug Fixes
Correctly extract security group name for tags @Nuru (#25)
what
var.security_group_name
list when setting tagswhy
v0.4.1
Compare Source
🐛 Bug Fixes
Fix bad markup (unclosed `details` block) in README.yaml @Nuru (#24)
what
details
block) in README.yamlwhy
v0.4.0
: New Standards, Breaking ChangesCompare Source
This release makes no attempt at backward compatibility with earlier versions.
It puts forth some new Cloud Posse standards. See details below.
As a major overhaul, it likely has bugs. It may have breaking changes in the near future as we discover design issues. However, the intention is to get this module stabilized and provide a consistent interface moving forward.
This module requires Terraform version 0.14 or later due to numerous issues in Terraform 0.13.
🚀 Enhancements
Overhaul Module to New Standards @Nuru (#17) (click to see details)
what
use_name_prefix
replaced withcreate_before_destroy
. Previously,create_before_destroy
was always set totrue
but of course that fails if you are not using a name prefix, because the names must be unique. Now the name is automatically a prefix ifcreate_before_destroy
istrue
and not if it is not.security_group_enabled
renamed tocreate_security_group
. Whether the security group is created or not, it will be enabled, and settingsecurity_group_enabled
to false does not disable the entire module, even though the module is named "security-group", which makes the old name terribly confusing. The new name is more descriptive.id
renamed totarget_security_group_id
. Againid
by itself is too vague. Converted to list to conform to new standard pattern that optional inputs which are used in conditionals are passed as list elements. See Hashicorp recommendationsecurity_group_name
input, which, if set, will set the security group name. If not set, name will be derived fromnull-label
. Because the security group name must be unique within an account, we should provide some way for people to set/override it other than forcing them to create a customizednull-label
.rule_matrix
. Many of our modules allow users to simply give a list of security groups to allow access to the new resource, typically calledallowed_security_groups
. This variable allows for easy migration by closely paralleling the existing resource creation code. It allows any number of rules to be applied to any combined list of security groups and CIDRs. See example.allow_all_egress
. AWS by default allows full egress to newly created security groups. Terraform removes this when taking over a security group, but our modules frequently want to restore it. Historically, though, the modules have implemented this slightly differently, and few or none have allowed IPv6 egress. Adding this boolean gives us a way to enable it simply and consistently (as opposed to every module writing its own egress rule).rules
to use infor_each
. Existing generated keys were not guaranteed unique, and keys that were generated and guaranteed to be unique would not be known at plan time and thus could not be used. Instead, provide option for user to provide stable keys and, if not provided, generate keys knowing they might not be stable.why
naming conventions
We want to migrate to a consistent set of name across modules. However, it is also quite painful to be forced to rename, so where possible I would like to maintain existing names but mark them deprecated and feed them into the new names in
main.tf locals{}
. We have also already seen issues with the most recent set of name changes. Therefore I propose these names with these meanings:associated_security_group_ids
associated_security_group_ids
is a list of IDs of Security Group that are "associated" (AWS' term) with the resource being created. In other words, the new resource is placed in or becomes a member of the Security Groups identified by the ID.Most often our modules got this information as
existing_security_groups
and a booleanuse_existing_security_groups
, and the recent change was to call this input simplysecurity_groups
. However, there is no consistency in naming in the AWS provider (redshift_cluster calls itcluster_security_groups
, elasticache_replication_group calls itsecurity_group_ids
, ec2_instance calls it justsecurity_groups
but accepts legacy security group names as well as IDs) and in our modules we typically have several lists of security groups (see below), so just calling thissecurity_groups
is very confusing.Using "associated" makes it clear the purpose, and suffixing with "ids" makes it clear the type. Since AWS in inconsistent and variously uses ARNs, IDs, and Names to identify resources, I think including the type is very helpful.
allowed_security_group_ids
allowed_security_group_ids
are security groups that are allowed ingress to the resource being created. Typically rules allowing this are added to the single created security group, as it should be unnecessary for an existing security group, but where desired, these rules can be added to the first in the list of "associated" security groupsallow_all_egress
AWS by default creates security groups that allow no ingress but allow all egress. When Terraform starts managing rules for the security group, it removes this default egress rule. Modules should include an
allow_all_egress
boolean to restore that rule whentrue
.security_group_description
Our modules have evolved over time (at community request) to provide more useful descriptions of Security Groups. Unfortunately, Terraform cannot change the description of an existing security group; instead it must replace the SG with a new one with the new description. For this reason, changes to the description field, while beneficial for new users, can be too disruptive on existing infrastructure to be worth it. In order to provide users with control over the description and thus mitigate the impact of changes, all modules that create security groups should include a
security_group_description
input which, if set, overrides any other kind of generated or default description.Instance Metadata Services
Although not actually part of the Security Group module, since we are covering consistent naming of inputs, we document here that we are using the following inputs and defaults to configure the AWS Instance Metadata Service. Note that our inputs do not exactly match the Terraform resource inputs because we have chosen to use boolean inputs rather than string inputs to toggle features. Our standard
metadata_options
block looks like thisand defaults are
We picked these defaults so that we default to best security practices with a concession (hop limit 2 instead of 1) to running containerized services. However,
metadata_http_tokens_required = true
may break some existing applications and is a breaking change, so when implemented, it should be noted in the release notes, along with how to preserve the previous settings.Optional Inputs
This module is among the first to implement the new Cloud Posse standard for optional inputs in Terraform. Because of issues like this (just one of many, many examples) we are going to follow Hashicorp's advice and prohibit the conditional creation of resources based on values of inputs. If you want to condition the creation of a resource (e.g.
count = xxx ? 1 : 0
) based on whether the input is supplied or not, the way we are going to do it is to make the optional input a list. A supplied value will be in a list with 1 element. An omitted value will use the default list of 0 elements. It will remain standard practice to depend on the value ofenabled
, but otherwise we should avoid conditional creation of resources based on input values.Unfortunately, this also means we cannot use
for_each
when the values might be generated during apply. This appears to be a consequence of the fact thatfor_each
requires aset
and the cardinality of the set depends on the values generated (adding 2 of the same value to a set only increases the size of the set by 1). So we can only usefor_each
when we can guarantee the user is hard coding the values so they are all known an plan time. Otherwise usecount
.references
Issues with Terraform management of Security Group Rules: drift detection, cyclical dependencies, and competing for control: a post from a Hashicorp engineer
Bug in Terraform AWS provider requires multiple
apply
cycles to updateaws_elasticache_replication_group
security groups:Some problems with the previous version:
v0.3.3
Compare Source
🤖 Automatic Updates
Update context.tf @cloudpossebot (#21)
what
This is an auto-generated PR that updates the
context.tf
file to the latest version fromcloudposse/terraform-null-label
why
To support all the features of the
context
interface.v0.3.2
Compare Source
🚀 Enhancements
add missing required input (vpc_id) in the example @Zaargh (#20)
what
vpc_id
in the exampleConfiguration
📅 Schedule: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).
🚦 Automerge: Disabled by config. Please merge this manually once you are satisfied.
♻ Rebasing: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.
🔕 Ignore: Close this PR and you won't be reminded about this update again.
This PR has been generated by Mend Renovate. View repository job log here.