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

provider/aws: Default Network ACL resource #6165

Merged
merged 5 commits into from
Apr 18, 2016
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions builtin/providers/aws/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,7 @@ func Provider() terraform.ResourceProvider {
"aws_main_route_table_association": resourceAwsMainRouteTableAssociation(),
"aws_nat_gateway": resourceAwsNatGateway(),
"aws_network_acl": resourceAwsNetworkAcl(),
"aws_default_network_acl": resourceAwsDefaultNetworkAcl(),
"aws_network_acl_rule": resourceAwsNetworkAclRule(),
"aws_network_interface": resourceAwsNetworkInterface(),
"aws_opsworks_stack": resourceAwsOpsworksStack(),
Expand Down
287 changes: 287 additions & 0 deletions builtin/providers/aws/resource_aws_default_network_acl.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,287 @@
package aws

import (
"fmt"
"log"

"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/service/ec2"
"github.com/hashicorp/terraform/helper/schema"
)

func resourceAwsDefaultNetworkAcl() *schema.Resource {
return &schema.Resource{
Create: resourceAwsDefaultNetworkAclCreate,
Read: resourceAwsNetworkAclRead,
Delete: resourceAwsDefaultNetworkAclDelete,
Update: resourceAwsDefaultNetworkAclUpdate,

Schema: map[string]*schema.Schema{
"vpc_id": &schema.Schema{
Type: schema.TypeString,
Computed: true,
},
"default_network_acl_id": &schema.Schema{
Type: schema.TypeString,
Required: true,
ForceNew: true,
Computed: false,
},
"subnet_id": &schema.Schema{
Type: schema.TypeString,
Computed: true,
},
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a new resource - why are we starting w/ a deprecated field? Perhaps just a leftover from basing this on the network_acl resource?

Copy link
Contributor Author

@catsby catsby Apr 15, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A bit of a leftover. It was Computed when I was "inheriting" the schema. I missed re-adding that change when I went to fully copying it over.

We're re-using resourceAwsNetworkAclRead though, so it needs to be present. I'll mark it as Computed

Copy link
Contributor

@phinze phinze Apr 18, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just took a read through Read (ha!) on network ACL and didn't see the subnet_id field mentioned there. Looks like it's only referenced in Delete and Update, nether of which we borrow. So I think that means we can drop this without consequence?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Smote in 78cd903

// We want explicit managment of Subnets here, so we do not allow them to be
// computed. Instead, an empty config will enforce just that; removal of the
// any Subnets that have been assigned to the Default Network ACL. Because we
// can't actually remove them, this will be a continual plan
"subnet_ids": &schema.Schema{
Type: schema.TypeSet,
Optional: true,
Elem: &schema.Schema{Type: schema.TypeString},
Set: schema.HashString,
},
// We want explicit managment of Rules here, so we do not allow them to be
// computed. Instead, an empty config will enforce just that; removal of the
// rules
"ingress": &schema.Schema{
Type: schema.TypeSet,
Required: false,
Optional: true,
Elem: &schema.Resource{
Schema: map[string]*schema.Schema{
"from_port": &schema.Schema{
Type: schema.TypeInt,
Required: true,
},
"to_port": &schema.Schema{
Type: schema.TypeInt,
Required: true,
},
"rule_no": &schema.Schema{
Type: schema.TypeInt,
Required: true,
},
"action": &schema.Schema{
Type: schema.TypeString,
Required: true,
},
"protocol": &schema.Schema{
Type: schema.TypeString,
Required: true,
},
"cidr_block": &schema.Schema{
Type: schema.TypeString,
Optional: true,
},
"icmp_type": &schema.Schema{
Type: schema.TypeInt,
Optional: true,
},
"icmp_code": &schema.Schema{
Type: schema.TypeInt,
Optional: true,
},
},
},
Set: resourceAwsNetworkAclEntryHash,
},
"egress": &schema.Schema{
Type: schema.TypeSet,
Required: false,
Optional: true,
Elem: &schema.Resource{
Schema: map[string]*schema.Schema{
"from_port": &schema.Schema{
Type: schema.TypeInt,
Required: true,
},
"to_port": &schema.Schema{
Type: schema.TypeInt,
Required: true,
},
"rule_no": &schema.Schema{
Type: schema.TypeInt,
Required: true,
},
"action": &schema.Schema{
Type: schema.TypeString,
Required: true,
},
"protocol": &schema.Schema{
Type: schema.TypeString,
Required: true,
},
"cidr_block": &schema.Schema{
Type: schema.TypeString,
Optional: true,
},
"icmp_type": &schema.Schema{
Type: schema.TypeInt,
Optional: true,
},
"icmp_code": &schema.Schema{
Type: schema.TypeInt,
Optional: true,
},
},
},
Set: resourceAwsNetworkAclEntryHash,
},

"tags": tagsSchema(),
},
}
}

func resourceAwsDefaultNetworkAclCreate(d *schema.ResourceData, meta interface{}) error {
d.SetId(d.Get("default_network_acl_id").(string))
log.Printf("[DEBUG] Revoking ingress rules for Default Network ACL for %s", d.Id())
err := revokeRulesForType(d.Id(), "ingress", meta)
if err != nil {
return err
}

log.Printf("[DEBUG] Revoking egress rules for Default Network ACL for %s", d.Id())
err = revokeRulesForType(d.Id(), "egress", meta)
if err != nil {
return err
}

return resourceAwsDefaultNetworkAclUpdate(d, meta)
}

func resourceAwsDefaultNetworkAclUpdate(d *schema.ResourceData, meta interface{}) error {
conn := meta.(*AWSClient).ec2conn
d.Partial(true)

if d.HasChange("ingress") {
err := updateNetworkAclEntries(d, "ingress", conn)
if err != nil {
return err
}
}

if d.HasChange("egress") {
err := updateNetworkAclEntries(d, "egress", conn)
if err != nil {
return err
}
}

if d.HasChange("subnet_ids") {
o, n := d.GetChange("subnet_ids")
if o == nil {
o = new(schema.Set)
}
if n == nil {
n = new(schema.Set)
}

os := o.(*schema.Set)
ns := n.(*schema.Set)

remove := os.Difference(ns).List()
add := ns.Difference(os).List()

if len(remove) > 0 {
//
// NO-OP
//
// Subnets *must* belong to a Network ACL. Subnets are not "remove" from
// Network ACLs, instead their association is replaced. In a normal
// Network ACL, any removal of a Subnet is done by replacing the
// Subnet/ACL association with an association between the Subnet and the
// Default Network ACL. Because we're managing the default here, we cannot
// do that, so we simply log a NO-OP. In order to remove the Subnet here,
// it must be destroyed, or assigned to different Network ACL. Those
// operations are not handled here
log.Printf("[WARN] Cannot remove subnets from the Default Network ACL. They must be re-assigned or destroyed")
}

if len(add) > 0 {
for _, a := range add {
association, err := findNetworkAclAssociation(a.(string), conn)
if err != nil {
return fmt.Errorf("Failed to find acl association: acl %s with subnet %s: %s", d.Id(), a, err)
}
log.Printf("[DEBUG] Updating Network Association for Default Network ACL (%s) and Subnet (%s)", d.Id(), a.(string))
_, err = conn.ReplaceNetworkAclAssociation(&ec2.ReplaceNetworkAclAssociationInput{
AssociationId: association.NetworkAclAssociationId,
NetworkAclId: aws.String(d.Id()),
})
if err != nil {
return err
}
}
}
}

if err := setTags(conn, d); err != nil {
return err
} else {
d.SetPartial("tags")
}

d.Partial(false)
// Re-use the exiting Network ACL Resources READ method
return resourceAwsNetworkAclRead(d, meta)
}

func resourceAwsDefaultNetworkAclDelete(d *schema.ResourceData, meta interface{}) error {
log.Printf("[WARN] Cannot destroy Default Network ACL. Terraform will remove this resource from the state file, however resources may remain.")
d.SetId("")
return nil
}

// revokeRulesForType will query the Network ACL for it's entries, and revoke
// any rule of the matching type.
func revokeRulesForType(netaclId, rType string, meta interface{}) error {
conn := meta.(*AWSClient).ec2conn

resp, err := conn.DescribeNetworkAcls(&ec2.DescribeNetworkAclsInput{
NetworkAclIds: []*string{aws.String(netaclId)},
})

if err != nil {
log.Printf("[DEBUG] Error looking up Network ACL: %s", err)
return err
}

if resp == nil {
return fmt.Errorf("[ERR] Error looking up Default Network ACL Entries: No results")
}

networkAcl := resp.NetworkAcls[0]
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 {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you lift this number up into a constant.

continue
}

// networkAcl.Entries contains a list of ACL Entries, with an Egress boolean
// to indicate if they are ingress or egress. Match on that bool to make
// sure we're removing the right kind of rule, instead of just all rules
rt := "ingress"
if *e.Egress == true {
rt = "egress"
}

if rType != rt {
continue
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we're only using this helper to remove both types of rules... could we simplify the function by dropping rType as an arg entirely and removing this whole extra check? Then you'd just call this helper once to clear out everything.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At one point we were only optionally cleaning up the default rules, and by consequence maybe only cleaning up ingress, maybe only cleaning up egress. Hasty omission during my refactoring on my part, fixing


log.Printf("[DEBUG] Destroying Network ACL Entry number (%d) for type (%s)", int(*e.RuleNumber), rt)
_, err := conn.DeleteNetworkAclEntry(&ec2.DeleteNetworkAclEntryInput{
NetworkAclId: aws.String(netaclId),
RuleNumber: e.RuleNumber,
Egress: e.Egress,
})
if err != nil {
return fmt.Errorf("Error deleting entry (%s): %s", e, err)
}
}

return nil
}
Loading