IntenseWebs/opentofu
3
0
Fork 0
mirror of https://github.com/opentofu/opentofu.git synced 2025-02-25 18:45:20 -06:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #1843 from ctiwald/ct/fix-network-acls

Browse Source 
Fix a number of issues in AWS network ACLs
This commit is contained in:
Clint 2015-05-07 16:39:10 -05:00
commit 4874179e9a
5 changed files with 218 additions and 49 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

31
builtin/providers/aws/network_acl_entry.go
View File

@ -2,6 +2,7 @@ package aws
import (
"fmt"
"net"
"strconv"
"github.com/awslabs/aws-sdk-go/aws"
@ -66,3 +67,33 @@ func protocolIntegers() map[string]int {
}
return protocolIntegers
}
// expectedPortPair stores a pair of ports we expect to see together.
type expectedPortPair struct {
to_port int64
from_port int64
}
// validatePorts ensures the ports and protocol match expected
// values.
func validatePorts(to int64, from int64, expected expectedPortPair) bool {
if to != expected.to_port || from != expected.from_port {
return false
}
return true
}
// validateCIDRBlock ensures the passed CIDR block represents an implied
// network, and not an overly-specified IP address.
func validateCIDRBlock(cidr string) error {
_, ipnet, err := net.ParseCIDR(cidr)
if err != nil {
return err
}
if ipnet.String() != cidr {
return fmt.Errorf("%s is not a valid mask; did you mean %s?", cidr, ipnet)
}
return nil
}

37
builtin/providers/aws/network_acl_entry_test.go
View File

@ -135,3 +135,40 @@ func Test_flattenNetworkACLEntry(t *testing.T) {
}
}
func Test_validatePorts(t *testing.T) {
for _, ts := range []struct {
to int64
from int64
expected *expectedPortPair
wanted bool
}{
{0, 0, &expectedPortPair{0, 0}, true},
{0, 1, &expectedPortPair{0, 0}, false},
} {
got := validatePorts(ts.to, ts.from, *ts.expected)
if got != ts.wanted {
t.Fatalf("Got: %t; Expected: %t\n", got, ts.wanted)
}
}
}
func Test_validateCIDRBlock(t *testing.T) {
for _, ts := range []struct {
cidr string
shouldErr bool
}{
{"10.2.2.0/24", false},
{"10.2.2.0/1234", true},
{"10/24", true},
{"10.2.2.2/24", true},
} {
err := validateCIDRBlock(ts.cidr)
if ts.shouldErr && err == nil {
t.Fatalf("Input '%s' should error but didn't!", ts.cidr)
}
if !ts.shouldErr && err != nil {
t.Fatalf("Got unexpected error for '%s' input: %s", ts.cidr, err)
}
}
}

100
builtin/providers/aws/resource_aws_network_acl.go
View File

@ -4,6 +4,7 @@ import (
"bytes"
"fmt"
"log"
"strconv"
"time"
"github.com/awslabs/aws-sdk-go/aws"
@ -151,6 +152,12 @@ func resourceAwsNetworkAclRead(d *schema.ResourceData, meta interface{}) error {
// separate the ingress and egress rules
for _, e := range networkAcl.Entries {
// Skip the default rules added by AWS. They can be neither
// configured or deleted by users.
if *e.RuleNumber == 32767 {
continue
}
if *e.Egress == true {
egressEntries = append(egressEntries, e)
} else {
@ -159,10 +166,15 @@ func resourceAwsNetworkAclRead(d *schema.ResourceData, meta interface{}) error {
}
d.Set("vpc_id", networkAcl.VPCID)
d.Set("ingress", ingressEntries)
d.Set("egress", egressEntries)
d.Set("tags", tagsToMapSDK(networkAcl.Tags))
if err := d.Set("ingress", networkAclEntriesToMapList(ingressEntries)); err != nil {
return err
}
if err := d.Set("egress", networkAclEntriesToMapList(egressEntries)); err != nil {
return err
}
return nil
}
@ -230,6 +242,15 @@ func updateNetworkAclEntries(d *schema.ResourceData, entryType string, conn *ec2
return err
}
for _, remove := range toBeDeleted {
// AWS includes default rules with all network ACLs that can be
// neither modified nor destroyed. They have a custom rule
// number that is out of bounds for any other rule. If we
// encounter it, just continue. There's no work to be done.
if *remove.RuleNumber == 32767 {
continue
}
// Delete old Acl
_, err := conn.DeleteNetworkACLEntry(&ec2.DeleteNetworkACLEntryInput{
NetworkACLID: aws.String(d.Id()),
@ -246,8 +267,34 @@ func updateNetworkAclEntries(d *schema.ResourceData, entryType string, conn *ec2
return err
}
for _, add := range toBeCreated {
// Protocol -1 rules don't store ports in AWS. Thus, they'll always
// hash differently when being read out of the API. Force the user
// to set from_port and to_port to 0 for these rules, to keep the
// hashing consistent.
if *add.Protocol == "-1" {
to := *add.PortRange.To
from := *add.PortRange.From
expected := &expectedPortPair{
to_port: 0,
from_port: 0,
}
if ok := validatePorts(to, from, *expected); !ok {
return fmt.Errorf(
"to_port (%d) and from_port (%d) must both be 0 to use the the 'all' \"-1\" protocol!",
to, from)
}
}
// AWS mutates the CIDR block into a network implied by the IP and
// mask provided. This results in hashing inconsistencies between
// the local config file and the state returned by the API. Error
// if the user provides a CIDR block with an inappropriate mask
if err := validateCIDRBlock(*add.CIDRBlock); err != nil {
return err
}
// Add new Acl entry
_, err := conn.CreateNetworkACLEntry(&ec2.CreateNetworkACLEntryInput{
_, connErr := conn.CreateNetworkACLEntry(&ec2.CreateNetworkACLEntryInput{
NetworkACLID: aws.String(d.Id()),
CIDRBlock: add.CIDRBlock,
Egress: add.Egress,
@ -256,7 +303,7 @@ func updateNetworkAclEntries(d *schema.ResourceData, entryType string, conn *ec2
RuleAction: add.RuleAction,
RuleNumber: add.RuleNumber,
})
if err != nil {
if connErr != nil {
return fmt.Errorf("Error creating %s entry: %s", entryType, err)
}
}
@ -309,7 +356,18 @@ func resourceAwsNetworkAclEntryHash(v interface{}) int {
buf.WriteString(fmt.Sprintf("%d-", m["to_port"].(int)))
buf.WriteString(fmt.Sprintf("%d-", m["rule_no"].(int)))
buf.WriteString(fmt.Sprintf("%s-", m["action"].(string)))
buf.WriteString(fmt.Sprintf("%s-", m["protocol"].(string)))
// The AWS network ACL API only speaks protocol numbers, and that's
// all we store. Never hash a protocol name.
protocol := m["protocol"].(string)
if _, err := strconv.Atoi(m["protocol"].(string)); err != nil {
// We're a protocol name. Look up the number.
buf.WriteString(fmt.Sprintf("%d-", protocolIntegers()[protocol]))
} else {
// We're a protocol number. Pass the value through.
buf.WriteString(fmt.Sprintf("%s-", protocol))
}
buf.WriteString(fmt.Sprintf("%s-", m["cidr_block"].(string)))
if v, ok := m["ssl_certificate_id"]; ok {
@ -361,3 +419,35 @@ func findNetworkAclAssociation(subnetId string, conn *ec2.EC2) (networkAclAssoci
}
return nil, fmt.Errorf("could not find association for subnet %s ", subnetId)
}
// networkAclEntriesToMapList turns ingress/egress rules read from AWS into a list
// of maps.
func networkAclEntriesToMapList(networkAcls []*ec2.NetworkACLEntry) []map[string]interface{} {
result := make([]map[string]interface{}, 0, len(networkAcls))
for _, entry := range networkAcls {
acl := make(map[string]interface{})
acl["rule_no"] = *entry.RuleNumber
acl["action"] = *entry.RuleAction
acl["cidr_block"] = *entry.CIDRBlock
// The AWS network ACL API only speaks protocol numbers, and
// that's all we record.
if _, err := strconv.Atoi(*entry.Protocol); err != nil {
// We're a protocol name. Look up the number.
acl["protocol"] = protocolIntegers()[*entry.Protocol]
} else {
// We're a protocol number. Pass through.
acl["protocol"] = *entry.Protocol
}
acl["protocol"] = *entry.Protocol
if entry.PortRange != nil {
acl["from_port"] = *entry.PortRange.From
acl["to_port"] = *entry.PortRange.To
}
result = append(result, acl)
}
return result
}

89
builtin/providers/aws/resource_aws_network_acl_test.go
View File

@ -23,29 +23,29 @@ func TestAccAWSNetworkAcl_EgressAndIngressRules(t *testing.T) {
Check: resource.ComposeTestCheckFunc(
testAccCheckAWSNetworkAclExists("aws_network_acl.bar", &networkAcl),
resource.TestCheckResourceAttr(
"aws_network_acl.bar", "ingress.3409203205.protocol", "tcp"),
"aws_network_acl.bar", "ingress.1216169466.protocol", "6"),
resource.TestCheckResourceAttr(
"aws_network_acl.bar", "ingress.3409203205.rule_no", "1"),
"aws_network_acl.bar", "ingress.1216169466.rule_no", "1"),
resource.TestCheckResourceAttr(
"aws_network_acl.bar", "ingress.3409203205.from_port", "80"),
"aws_network_acl.bar", "ingress.1216169466.from_port", "80"),
resource.TestCheckResourceAttr(
"aws_network_acl.bar", "ingress.3409203205.to_port", "80"),
"aws_network_acl.bar", "ingress.1216169466.to_port", "80"),
resource.TestCheckResourceAttr(
"aws_network_acl.bar", "ingress.3409203205.action", "allow"),
"aws_network_acl.bar", "ingress.1216169466.action", "allow"),
resource.TestCheckResourceAttr(
"aws_network_acl.bar", "ingress.3409203205.cidr_block", "10.3.10.3/18"),
"aws_network_acl.bar", "ingress.1216169466.cidr_block", "10.3.0.0/18"),
resource.TestCheckResourceAttr(
"aws_network_acl.bar", "egress.2579689292.protocol", "tcp"),
"aws_network_acl.bar", "egress.2634340476.protocol", "6"),
resource.TestCheckResourceAttr(
"aws_network_acl.bar", "egress.2579689292.rule_no", "2"),
"aws_network_acl.bar", "egress.2634340476.rule_no", "2"),
resource.TestCheckResourceAttr(
"aws_network_acl.bar", "egress.2579689292.from_port", "443"),
"aws_network_acl.bar", "egress.2634340476.from_port", "443"),
resource.TestCheckResourceAttr(
"aws_network_acl.bar", "egress.2579689292.to_port", "443"),
"aws_network_acl.bar", "egress.2634340476.to_port", "443"),
resource.TestCheckResourceAttr(
"aws_network_acl.bar", "egress.2579689292.cidr_block", "10.3.2.3/18"),
"aws_network_acl.bar", "egress.2634340476.cidr_block", "10.3.0.0/18"),
resource.TestCheckResourceAttr(
"aws_network_acl.bar", "egress.2579689292.action", "allow"),
"aws_network_acl.bar", "egress.2634340476.action", "allow"),
),
},
},
@ -66,17 +66,17 @@ func TestAccAWSNetworkAcl_OnlyIngressRules(t *testing.T) {
testAccCheckAWSNetworkAclExists("aws_network_acl.foos", &networkAcl),
// testAccCheckSubnetAssociation("aws_network_acl.foos", "aws_subnet.blob"),
resource.TestCheckResourceAttr(
"aws_network_acl.foos", "ingress.2750166237.protocol", "tcp"),
"aws_network_acl.foos", "ingress.3264550475.protocol", "6"),
resource.TestCheckResourceAttr(
"aws_network_acl.foos", "ingress.2750166237.rule_no", "2"),
"aws_network_acl.foos", "ingress.3264550475.rule_no", "2"),
resource.TestCheckResourceAttr(
"aws_network_acl.foos", "ingress.2750166237.from_port", "443"),
"aws_network_acl.foos", "ingress.3264550475.from_port", "443"),
resource.TestCheckResourceAttr(
"aws_network_acl.foos", "ingress.2750166237.to_port", "443"),
"aws_network_acl.foos", "ingress.3264550475.to_port", "443"),
resource.TestCheckResourceAttr(
"aws_network_acl.foos", "ingress.2750166237.action", "deny"),
"aws_network_acl.foos", "ingress.3264550475.action", "deny"),
resource.TestCheckResourceAttr(
"aws_network_acl.foos", "ingress.2750166237.cidr_block", "10.2.2.3/18"),
"aws_network_acl.foos", "ingress.3264550475.cidr_block", "10.2.0.0/18"),
),
},
},
@ -97,21 +97,21 @@ func TestAccAWSNetworkAcl_OnlyIngressRulesChange(t *testing.T) {
testAccCheckAWSNetworkAclExists("aws_network_acl.foos", &networkAcl),
testIngressRuleLength(&networkAcl, 2),
resource.TestCheckResourceAttr(
"aws_network_acl.foos", "ingress.37211640.protocol", "tcp"),
"aws_network_acl.foos", "ingress.2824900805.protocol", "6"),
resource.TestCheckResourceAttr(
"aws_network_acl.foos", "ingress.37211640.rule_no", "1"),
"aws_network_acl.foos", "ingress.2824900805.rule_no", "1"),
resource.TestCheckResourceAttr(
"aws_network_acl.foos", "ingress.37211640.from_port", "0"),
"aws_network_acl.foos", "ingress.2824900805.from_port", "0"),
resource.TestCheckResourceAttr(
"aws_network_acl.foos", "ingress.37211640.to_port", "22"),
"aws_network_acl.foos", "ingress.2824900805.to_port", "22"),
resource.TestCheckResourceAttr(
"aws_network_acl.foos", "ingress.37211640.action", "deny"),
"aws_network_acl.foos", "ingress.2824900805.action", "deny"),
resource.TestCheckResourceAttr(
"aws_network_acl.foos", "ingress.37211640.cidr_block", "10.2.2.3/18"),
"aws_network_acl.foos", "ingress.2824900805.cidr_block", "10.2.0.0/18"),
resource.TestCheckResourceAttr(
"aws_network_acl.foos", "ingress.2750166237.from_port", "443"),
"aws_network_acl.foos", "ingress.3264550475.from_port", "443"),
resource.TestCheckResourceAttr(
"aws_network_acl.foos", "ingress.2750166237.rule_no", "2"),
"aws_network_acl.foos", "ingress.3264550475.rule_no", "2"),
),
},
resource.TestStep{
@ -120,17 +120,17 @@ func TestAccAWSNetworkAcl_OnlyIngressRulesChange(t *testing.T) {
testAccCheckAWSNetworkAclExists("aws_network_acl.foos", &networkAcl),
testIngressRuleLength(&networkAcl, 1),
resource.TestCheckResourceAttr(
"aws_network_acl.foos", "ingress.37211640.protocol", "tcp"),
"aws_network_acl.foos", "ingress.2824900805.protocol", "6"),
resource.TestCheckResourceAttr(
"aws_network_acl.foos", "ingress.37211640.rule_no", "1"),
"aws_network_acl.foos", "ingress.2824900805.rule_no", "1"),
resource.TestCheckResourceAttr(
"aws_network_acl.foos", "ingress.37211640.from_port", "0"),
"aws_network_acl.foos", "ingress.2824900805.from_port", "0"),
resource.TestCheckResourceAttr(
"aws_network_acl.foos", "ingress.37211640.to_port", "22"),
"aws_network_acl.foos", "ingress.2824900805.to_port", "22"),
resource.TestCheckResourceAttr(
"aws_network_acl.foos", "ingress.37211640.action", "deny"),
"aws_network_acl.foos", "ingress.2824900805.action", "deny"),
resource.TestCheckResourceAttr(
"aws_network_acl.foos", "ingress.37211640.cidr_block", "10.2.2.3/18"),
"aws_network_acl.foos", "ingress.2824900805.cidr_block", "10.2.0.0/18"),
),
},
},
@ -330,7 +330,7 @@ resource "aws_network_acl" "foos" {
protocol = "tcp"
rule_no = 1
action = "deny"
cidr_block = "10.2.2.3/18"
cidr_block = "10.2.0.0/18"
from_port = 0
to_port = 22
}
@ -338,7 +338,7 @@ resource "aws_network_acl" "foos" {
protocol = "tcp"
rule_no = 2
action = "deny"
cidr_block = "10.2.2.3/18"
cidr_block = "10.2.0.0/18"
from_port = 443
to_port = 443
}
@ -360,7 +360,7 @@ resource "aws_network_acl" "foos" {
protocol = "tcp"
rule_no = 1
action = "deny"
cidr_block = "10.2.2.3/18"
cidr_block = "10.2.0.0/18"
from_port = 0
to_port = 22
}
@ -383,16 +383,25 @@ resource "aws_network_acl" "bond" {
protocol = "tcp"
rule_no = 2
action = "allow"
cidr_block = "10.2.2.3/18"
cidr_block = "10.2.0.0/18"
from_port = 443
to_port = 443
}
egress = {
protocol = "-1"
rule_no = 4
action = "allow"
cidr_block = "0.0.0.0/0"
from_port = 0
to_port = 0
}
egress = {
protocol = "tcp"
rule_no = 1
action = "allow"
cidr_block = "10.2.10.3/18"
cidr_block = "10.2.0.0/18"
from_port = 80
to_port = 80
}
@ -401,7 +410,7 @@ resource "aws_network_acl" "bond" {
protocol = "tcp"
rule_no = 3
action = "allow"
cidr_block = "10.2.10.3/18"
cidr_block = "10.2.0.0/18"
from_port = 22
to_port = 22
}
@ -427,7 +436,7 @@ resource "aws_network_acl" "bar" {
protocol = "tcp"
rule_no = 2
action = "allow"
cidr_block = "10.3.2.3/18"
cidr_block = "10.3.0.0/18"
from_port = 443
to_port = 443
}
@ -436,7 +445,7 @@ resource "aws_network_acl" "bar" {
protocol = "tcp"
rule_no = 1
action = "allow"
cidr_block = "10.3.10.3/18"
cidr_block = "10.3.0.0/18"
from_port = 80
to_port = 80
}

10
website/source/docs/providers/aws/r/network_acl.html.markdown
View File

@ -20,7 +20,7 @@ resource "aws_network_acl" "main" {
protocol = "tcp"
rule_no = 2
action = "allow"
cidr_block = "10.3.2.3/18"
cidr_block = "10.3.0.0/18"
from_port = 443
to_port = 443
}
@ -29,7 +29,7 @@ resource "aws_network_acl" "main" {
protocol = "tcp"
rule_no = 1
action = "allow"
cidr_block = "10.3.10.3/18"
cidr_block = "10.3.0.0/18"
from_port = 80
to_port = 80
}
@ -56,8 +56,10 @@ Both `egress` and `ingress` support the following keys:
* `to_port` - (Required) The to port to match.
* `rule_no` - (Required) The rule number. Used for ordering.
* `action` - (Required) The action to take.
* `protocol` - (Required) The protocol to match.
* `cidr_block` - (Optional) The CIDR block to match.
* `protocol` - (Required) The protocol to match. If using the -1 'all'
protocol, you must specify a from and to port of 0.
* `cidr_block` - (Optional) The CIDR block to match. This must be a
valid network mask.
## Attributes Reference