-
Notifications
You must be signed in to change notification settings - Fork 9.5k
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
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
8b223ba
provider/aws: Default Network ACL resource
catsby d104bbd
Remove subnet_id update, mark as computed value. Remove extra tag update
catsby c8e29f1
refactor default rule number to be a constant
catsby af367a4
refactor revokeRulesForType to be revokeAllNetworkACLEntries
catsby 78cd903
smite subnet_id, improve docs
catsby File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
283 changes: 283 additions & 0 deletions
283
builtin/providers/aws/resource_aws_default_network_acl.go
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,283 @@ | ||
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" | ||
) | ||
|
||
// ACL Network ACLs all contain an explicit deny-all rule that cannot be | ||
// destroyed or changed by users. This rule is numbered very high to be a | ||
// catch-all. | ||
// See http://docs.aws.amazon.com/AmazonVPC/latest/UserGuide/VPC_ACLs.html#default-network-acl | ||
const awsDefaultAclRuleNumber = 32767 | ||
|
||
func resourceAwsDefaultNetworkAcl() *schema.Resource { | ||
return &schema.Resource{ | ||
Create: resourceAwsDefaultNetworkAclCreate, | ||
// We reuse aws_network_acl's read method, the operations are the same | ||
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, | ||
}, | ||
// We want explicit management 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 until the | ||
// Subnets are themselves destroyed or reassigned to a different Network | ||
// ACL | ||
"subnet_ids": &schema.Schema{ | ||
Type: schema.TypeSet, | ||
Optional: true, | ||
Elem: &schema.Schema{Type: schema.TypeString}, | ||
Set: schema.HashString, | ||
}, | ||
// We want explicit management 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)) | ||
|
||
// revoke all default and pre-existing rules on the default network acl. | ||
// In the UPDATE method, we'll apply only the rules in the configuration. | ||
log.Printf("[DEBUG] Revoking default ingress and egress rules for Default Network ACL for %s", d.Id()) | ||
err := revokeAllNetworkACLEntries(d.Id(), 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 "removed" 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 | ||
} | ||
|
||
// revokeAllNetworkACLEntries revoke all ingress and egress rules that the Default | ||
// Network ACL currently has | ||
func revokeAllNetworkACLEntries(netaclId 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. See http://docs.aws.amazon.com/AmazonVPC/latest/UserGuide/VPC_ACLs.html#default-network-acl | ||
if *e.RuleNumber == awsDefaultAclRuleNumber { | ||
continue | ||
} | ||
|
||
// track if this is an egress or ingress rule, for logging purposes | ||
rt := "ingress" | ||
if *e.Egress == true { | ||
rt = "egress" | ||
} | ||
|
||
log.Printf("[DEBUG] Destroying Network ACL (%s) Entry number (%d)", rt, int(*e.RuleNumber)) | ||
_, 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 | ||
} |
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we check here if the given ACL is actually already the default for some VPC? Otherwise this resource feels more like "adopt some arbitrary Network ACL".
Alternatively, could the argument actually be
vpc_id
rather thandefault_network_acl_id
, and then we look up the actual ACL id from the VPC, thus ensuring that we'll always get the right one. IMO this interface seems more intuitive, as it makes the relationship to the VPC clearer:I suppose this model might be trickier since a user might choose a different Network ACL to be the default via the AWS console. In that case, I expect the id of this resource would be the VPC id rather than the ACL id, and
Read
would re-read the default ACL from the VPC each time.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @apparentlymart these are good points. I did consider using the
vpc_id
and looking up the default, but thought that in most cases I would already know this default's id so went with the approach above. I think for now we'll leave as is, and can adopt this suggestion with little trouble in the future if we think it's needed.Do you see this as a blocker?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My concern here was two things, really:
At the very least I think it'd be good to protect against the latter case by failing if the specified route table isn't the default for some VPC. It looks like DescribeNetworkAcls returns enough information answer this question with just one additional API request.
With that said, not a blocker if you're not worried about it. Mostly I was just expressing poorly some frustration of it taking me a while to understand what was going on here.