From ee8af4f619ec988ae1debcea8869db2746227538 Mon Sep 17 00:00:00 2001 From: Matt Burgess <549318+mattburgess@users.noreply.github.com> Date: Wed, 21 Jun 2023 21:56:25 +0100 Subject: [PATCH 1/4] Improve doc accuracy for `aws_vpc_security_group_ingress_rule` and `aws_vpc_security_group_egress_rule` * `ip_protocol` was marked as `Required` in the schema but `Optional` in the docs * `security_group_id` is `Required` but was `Optional` in the schema * Try to clarify the situations where `Optional` arguments are actually `Required` * Make examples less confusing by using a single port; using different ports makes it look like some kind of port mapping --- internal/service/ec2/vpc_security_group_ingress_rule.go | 2 +- .../docs/r/vpc_security_group_egress_rule.html.markdown | 5 ++++- .../docs/r/vpc_security_group_ingress_rule.html.markdown | 7 +++++-- 3 files changed, 10 insertions(+), 4 deletions(-) diff --git a/internal/service/ec2/vpc_security_group_ingress_rule.go b/internal/service/ec2/vpc_security_group_ingress_rule.go index db4d25b0b41..58134bb153a 100644 --- a/internal/service/ec2/vpc_security_group_ingress_rule.go +++ b/internal/service/ec2/vpc_security_group_ingress_rule.go @@ -134,7 +134,7 @@ func (r *resourceSecurityGroupRule) Schema(ctx context.Context, req resource.Sch Optional: true, }, "security_group_id": schema.StringAttribute{ - Optional: true, + Required: true, PlanModifiers: []planmodifier.String{ stringplanmodifier.RequiresReplace(), }, diff --git a/website/docs/r/vpc_security_group_egress_rule.html.markdown b/website/docs/r/vpc_security_group_egress_rule.html.markdown index f3250f0aad0..0ddc87258a5 100644 --- a/website/docs/r/vpc_security_group_egress_rule.html.markdown +++ b/website/docs/r/vpc_security_group_egress_rule.html.markdown @@ -26,12 +26,15 @@ resource "aws_vpc_security_group_egress_rule" "example" { cidr_ipv4 = "10.0.0.0/8" from_port = 80 ip_protocol = "tcp" - to_port = 8080 + to_port = 80 } ``` ## Argument Reference +~> **NOTE on optional/required attributes:** Although `cidr_ipv4`, `cidr_ipv6`, `prefix_list_id`, and `referenced_security_group_id` are all marked as optional, you *must* provide one of them in order to configure the destination of the traffic. +`from_port` and `to_port` are required, unless `ip_protocol` is set to `-1` or `icmpv6`. + The following arguments are supported: * `cidr_ipv4` - (Optional) The destination IPv4 CIDR range. diff --git a/website/docs/r/vpc_security_group_ingress_rule.html.markdown b/website/docs/r/vpc_security_group_ingress_rule.html.markdown index 97ae7a47603..6db678f43af 100644 --- a/website/docs/r/vpc_security_group_ingress_rule.html.markdown +++ b/website/docs/r/vpc_security_group_ingress_rule.html.markdown @@ -26,7 +26,7 @@ resource "aws_vpc_security_group_ingress_rule" "example" { cidr_ipv4 = "10.0.0.0/8" from_port = 80 ip_protocol = "tcp" - to_port = 8080 + to_port = 80 } ``` @@ -34,11 +34,14 @@ resource "aws_vpc_security_group_ingress_rule" "example" { The following arguments are supported: +~> **NOTE on optional/required attributes:** Although `cidr_ipv4`, `cidr_ipv6`, `prefix_list_id`, and `referenced_security_group_id` are all marked as optional, you *must* provide one of them in order to configure the source of the traffic. +`from_port` and `to_port` are required, unless `ip_protocol` is set to `-1` or `icmpv6`. + * `cidr_ipv4` - (Optional) The source IPv4 CIDR range. * `cidr_ipv6` - (Optional) The source IPv6 CIDR range. * `description` - (Optional) The security group rule description. * `from_port` - (Optional) The start of port range for the TCP and UDP protocols, or an ICMP/ICMPv6 type. -* `ip_protocol` - (Optional) The IP protocol name or number. Use `-1` to specify all protocols. Note that if `ip_protocol` is set to `-1`, it translates to all protocols, all port ranges, and `from_port` and `to_port` values should not be defined. +* `ip_protocol` - (Required) The IP protocol name or number. Use `-1` to specify all protocols. Note that if `ip_protocol` is set to `-1`, it translates to all protocols, all port ranges, and `from_port` and `to_port` values should not be defined. * `prefix_list_id` - (Optional) The ID of the source prefix list. * `referenced_security_group_id` - (Optional) The source security group that is referenced in the rule. * `security_group_id` - (Required) The ID of the security group. From a4f6b03a3dc701b2c7afbca7d1c618967da4471e Mon Sep 17 00:00:00 2001 From: Matthew Burgess <549318+mattburgess@users.noreply.github.com> Date: Wed, 21 Jun 2023 23:53:47 +0100 Subject: [PATCH 2/4] Apply suggestions from code review Co-authored-by: Justin Retzolk <44710313+justinretzolk@users.noreply.github.com> --- website/docs/r/vpc_security_group_egress_rule.html.markdown | 3 +-- website/docs/r/vpc_security_group_ingress_rule.html.markdown | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/website/docs/r/vpc_security_group_egress_rule.html.markdown b/website/docs/r/vpc_security_group_egress_rule.html.markdown index 0ddc87258a5..8fea624aaaf 100644 --- a/website/docs/r/vpc_security_group_egress_rule.html.markdown +++ b/website/docs/r/vpc_security_group_egress_rule.html.markdown @@ -32,8 +32,7 @@ resource "aws_vpc_security_group_egress_rule" "example" { ## Argument Reference -~> **NOTE on optional/required attributes:** Although `cidr_ipv4`, `cidr_ipv6`, `prefix_list_id`, and `referenced_security_group_id` are all marked as optional, you *must* provide one of them in order to configure the destination of the traffic. -`from_port` and `to_port` are required, unless `ip_protocol` is set to `-1` or `icmpv6`. +~> **Note** Although `cidr_ipv4`, `cidr_ipv6`, `prefix_list_id`, and `referenced_security_group_id` are all marked as optional, you *must* provide one of them in order to configure the destination of the traffic. The `from_port` and `to_port` arguments are required unless `ip_protocol` is set to `-1` or `icmpv6`. The following arguments are supported: diff --git a/website/docs/r/vpc_security_group_ingress_rule.html.markdown b/website/docs/r/vpc_security_group_ingress_rule.html.markdown index 6db678f43af..75ea178f0ff 100644 --- a/website/docs/r/vpc_security_group_ingress_rule.html.markdown +++ b/website/docs/r/vpc_security_group_ingress_rule.html.markdown @@ -34,8 +34,7 @@ resource "aws_vpc_security_group_ingress_rule" "example" { The following arguments are supported: -~> **NOTE on optional/required attributes:** Although `cidr_ipv4`, `cidr_ipv6`, `prefix_list_id`, and `referenced_security_group_id` are all marked as optional, you *must* provide one of them in order to configure the source of the traffic. -`from_port` and `to_port` are required, unless `ip_protocol` is set to `-1` or `icmpv6`. +~> **Note** Although `cidr_ipv4`, `cidr_ipv6`, `prefix_list_id`, and `referenced_security_group_id` are all marked as optional, you *must* provide one of them in order to configure the destination of the traffic. The `from_port` and `to_port` arguments are required unless `ip_protocol` is set to `-1` or `icmpv6`. * `cidr_ipv4` - (Optional) The source IPv4 CIDR range. * `cidr_ipv6` - (Optional) The source IPv6 CIDR range. From 571a5d89210f6d410ec574bc05edf8a4f3b2ca2b Mon Sep 17 00:00:00 2001 From: Matt Burgess <549318+mattburgess@users.noreply.github.com> Date: Thu, 22 Jun 2023 00:11:15 +0100 Subject: [PATCH 3/4] Update docs for aws_security_group and aws_security_group_rule --- website/docs/r/security_group.html.markdown | 4 ++++ website/docs/r/security_group_rule.html.markdown | 2 ++ 2 files changed, 6 insertions(+) diff --git a/website/docs/r/security_group.html.markdown b/website/docs/r/security_group.html.markdown index 4ed02d408e6..4f1aa59b575 100644 --- a/website/docs/r/security_group.html.markdown +++ b/website/docs/r/security_group.html.markdown @@ -236,6 +236,8 @@ The following arguments are required: The following arguments are optional: +~> **Note** Although `cidr_blocks`, `ipv6_cidr_blocks`, `prefix_list_ids`, and `security_groups` are all marked as optional, you _must_ provide one of them in order to configure the source of the traffic. + * `cidr_blocks` - (Optional) List of CIDR blocks. * `description` - (Optional) Description of this ingress rule. * `ipv6_cidr_blocks` - (Optional) List of IPv6 CIDR blocks. @@ -254,6 +256,8 @@ The following arguments are required: The following arguments are optional: +~> **Note** Although `cidr_blocks`, `ipv6_cidr_blocks`, `prefix_list_ids`, and `security_groups` are all marked as optional, you _must_ provide one of them in order to configure the destination of the traffic. + * `cidr_blocks` - (Optional) List of CIDR blocks. * `description` - (Optional) Description of this egress rule. * `ipv6_cidr_blocks` - (Optional) List of IPv6 CIDR blocks. diff --git a/website/docs/r/security_group_rule.html.markdown b/website/docs/r/security_group_rule.html.markdown index 342283ddab0..ebcfd153467 100644 --- a/website/docs/r/security_group_rule.html.markdown +++ b/website/docs/r/security_group_rule.html.markdown @@ -99,6 +99,8 @@ or `egress` (outbound). The following arguments are optional: +~> **Note** Although `cidr_blocks`, `ipv6_cidr_blocks`, `prefix_list_ids`, and `source_security_group_id` are all marked as optional, you _must_ provide one of them in order to configure the source of the traffic. + * `cidr_blocks` - (Optional) List of CIDR blocks. Cannot be specified with `source_security_group_id` or `self`. * `description` - (Optional) Description of the rule. * `ipv6_cidr_blocks` - (Optional) List of IPv6 CIDR blocks. Cannot be specified with `source_security_group_id` or `self`. From 907a04a7a271e6477b12b1bad5d82e3cf04ff333 Mon Sep 17 00:00:00 2001 From: Matt Burgess <549318+mattburgess@users.noreply.github.com> Date: Sat, 24 Jun 2023 11:10:24 +0100 Subject: [PATCH 4/4] Add Changelog entry --- .changelog/32148.txt | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 .changelog/32148.txt diff --git a/.changelog/32148.txt b/.changelog/32148.txt new file mode 100644 index 00000000000..58285c8859e --- /dev/null +++ b/.changelog/32148.txt @@ -0,0 +1,4 @@ +```release-note:bug +resource/aws_vpc_security_group_egress_rule: Make `security_group_id` a required argument +resource/aws_vpc_security_group_ingress_rule: Make `security_group_id` a required argument +```