New resource: aws_network_interface_sg_attachment (#860)

* New resource: aws_security_group_attachment

This is a transfer of work from hashicorp/terraform#15167.

This adds the aws_security_group_attachment resource, allowing one to
attach security groups to ENIs outside of an aws_instance or
aws_network_interface resource.

Use cases for this would include more granular management of security
groups, or attachment of security groups to instances that are managed
out-of-band from Terraform.

* aws_security_group_attachment -> aws_network_interface_sg_attachment

Renamed as pre review comments in #860. This should help differentiate
it between the other kinds of security groups available in the AWS
provider.

* resource: network_interface_sg_attachment: Require network_interface_id

This attribute was set to optional back when this resource allowed
either an instance or network interface specified. Now that this is no
longer the case, there's no reason to keep it this way.

* resource/aws_network_interface_sg_attachment: refactor tests

Make the test configs a bit easier to understand. Each case (via
resource or data source) now has its own config, but we still
parameterize on enabling/disabling the security group resource for the
removal check.

* resource/aws_network_interface_sg_attachment: Add locks, fix races

The resource was actually racing when there was multiple attachments
trying to work with the same network interface. This is fixed now with
locks added in create and delete.

The added test checks the race in a couple of steps, switching up the
resource names on the second run for the security groups and security
group attachments to get a good mix of creation and deletion events to
really test the effectiveness of the serialization.

Also a small cosmetic re-refactoring of test names and configuration
generation functions.

* resource/aws_network_interface_sg_attachment: Simplify interface check

Simplified the interface check function so that the test case directly
takes the attribute that we are deriving the interface ID from, rather
than taking a bool. This actually uncovered the fact my attribute logic
was reversed (the bool logic was giving primary_network_interface_id for
a false value passed to checkPrimaryInterfaceAttr instead of a true
value, and this error was propagated to the test cases). So this is
fixed now as well.

* resource/aws_network_interface_sg_attachment: Simplify race check

Race check needed simplifying as well, in addition to being reduced from
two steps to one.

Reason for the latter is once security groups were modified so that the
were operating off the same set of groups in both steps (so step1 ->
step2), it was discovered that there was no way we could reasonably
expect the deletion/creation order would never favour a situation where
the new SG attachment would be ordered after the old SG attachment was
removed (as both step1 destroys and step2 creations would be happening
at the same level in the graph and without any dependencies).

* resource/aws_network_interface_sg_attachment: Flatten some code

Removed a bunch of the single-use functions, which has moved most logic
to the main CRUD functions. The old functions served a purpose when this
resource was designed to support both instance IDs and network interface
IDs, but just adds more cruft now that only network interface IDs are
supported.

Also moved all the messages to the DEBUG level as TF_LOG=info does not
mean anything at a provider level, currently.

aws/provider.go

@@ -424,6 +424,7 @@ func Provider() terraform.ResourceProvider {
  "aws_s3_bucket_object": resourceAwsS3BucketObject(),
  "aws_s3_bucket_notification": resourceAwsS3BucketNotification(),
  "aws_security_group": resourceAwsSecurityGroup(),
+ "aws_network_interface_sg_attachment": resourceAwsNetworkInterfaceSGAttachment(),
  "aws_default_security_group": resourceAwsDefaultSecurityGroup(),
  "aws_security_group_rule": resourceAwsSecurityGroupRule(),
  "aws_simpledb_domain": resourceAwsSimpleDBDomain(),

aws/resource_aws_network_interface_sg_attachment.go

@@ -0,0 +1,183 @@
+package aws
+
+import (
+ "fmt"
+ "log"
+ "reflect"
+
+ "github.com/aws/aws-sdk-go/aws"
+ "github.com/aws/aws-sdk-go/service/ec2"
+ "github.com/hashicorp/terraform/helper/schema"
+)
+
+func resourceAwsNetworkInterfaceSGAttachment() *schema.Resource {
+ return &schema.Resource{
+ Create: resourceAwsNetworkInterfaceSGAttachmentCreate,
+ Read: resourceAwsNetworkInterfaceSGAttachmentRead,
+ Delete: resourceAwsNetworkInterfaceSGAttachmentDelete,
+ Schema: map[string]*schema.Schema{
+ "security_group_id": {
+ Type: schema.TypeString,
+ Required: true,
+ ForceNew: true,
+ },
+ "network_interface_id": {
+ Type: schema.TypeString,
+ Required: true,
+ ForceNew: true,
+ },
+ },
+ }
+}
+
+func resourceAwsNetworkInterfaceSGAttachmentCreate(d *schema.ResourceData, meta interface{}) error {
+ // Get a lock to prevent races on other SG attachments/detatchments on this
+ // interface ID. This lock is released when the function exits, regardless of
+ // success or failure.
+ //
+ // The lock here - in the create function - deliberately covers the
+ // post-creation read as well, which is normally not covered as Read is
+ // otherwise only performed on refresh. Locking on it here prevents
+ // inconsistencies that could be caused by other attachments that will be
+ // operating on the interface, ensuring that Create gets a full lay of the
+ // land before moving on.
+ mk := "network_interface_sg_attachment_" + d.Get("network_interface_id").(string)
+ awsMutexKV.Lock(mk)
+ defer awsMutexKV.Unlock(mk)
+
+ sgID := d.Get("security_group_id").(string)
+ interfaceID := d.Get("network_interface_id").(string)
+
+ conn := meta.(*AWSClient).ec2conn
+
+ // Fetch the network interface we will be working with.
+ iface, err := fetchNetworkInterface(conn, interfaceID)
+ if err != nil {
+ return err
+ }
+
+ // Add the security group to the network interface.
+ log.Printf("[DEBUG] Attaching security group %s to network interface ID %s", sgID, interfaceID)
+
+ if sgExistsInENI(sgID, iface) {
+ return fmt.Errorf("security group %s already attached to interface ID %s", sgID, *iface.NetworkInterfaceId)
+ }
+ var groupIDs []string
+ for _, v := range iface.Groups {
+ groupIDs = append(groupIDs, *v.GroupId)
+ }
+ groupIDs = append(groupIDs, sgID)
+ params := &ec2.ModifyNetworkInterfaceAttributeInput{
+ NetworkInterfaceId: iface.NetworkInterfaceId,
+ Groups: aws.StringSlice(groupIDs),
+ }
+
+ _, err = conn.ModifyNetworkInterfaceAttribute(params)
+ if err != nil {
+ return err
+ }
+
+ log.Printf("[DEBUG] Successful attachment of security group %s to network interface ID %s", sgID, interfaceID)
+
+ return resourceAwsNetworkInterfaceSGAttachmentRead(d, meta)
+}
+
+func resourceAwsNetworkInterfaceSGAttachmentRead(d *schema.ResourceData, meta interface{}) error {
+ sgID := d.Get("security_group_id").(string)
+ interfaceID := d.Get("network_interface_id").(string)
+
+ log.Printf("[DEBUG] Checking association of security group %s to network interface ID %s", sgID, interfaceID)
+
+ conn := meta.(*AWSClient).ec2conn
+
+ iface, err := fetchNetworkInterface(conn, interfaceID)
+ if err != nil {
+ return err
+ }
+
+ if sgExistsInENI(sgID, iface) {
+ d.SetId(fmt.Sprintf("%s_%s", sgID, interfaceID))
+ } else {
+ // The assocation does not exist when it should, taint this resource.
+ log.Printf("[WARN] Security group %s not associated with network interface ID %s, tainting", sgID, interfaceID)
+ d.SetId("")
+ }
+ return nil
+}
+
+func resourceAwsNetworkInterfaceSGAttachmentDelete(d *schema.ResourceData, meta interface{}) error {
+ // Get a lock to prevent races on other SG attachments/detatchments on this
+ // interface ID. This lock is released when the function exits, regardless of
+ // success or failure.
+ mk := "network_interface_sg_attachment_" + d.Get("network_interface_id").(string)
+ awsMutexKV.Lock(mk)
+ defer awsMutexKV.Unlock(mk)
+
+ sgID := d.Get("security_group_id").(string)
+ interfaceID := d.Get("network_interface_id").(string)
+
+ log.Printf("[DEBUG] Removing security group %s from interface ID %s", sgID, interfaceID)
+
+ conn := meta.(*AWSClient).ec2conn
+
+ iface, err := fetchNetworkInterface(conn, interfaceID)
+ if err != nil {
+ return err
+ }
+
+ if err := delSGFromENI(conn, sgID, iface); err != nil {
+ return err
+ }
+
+ d.SetId("")
+ return nil
+}
+
+// fetchNetworkInterface is a utility function used by Read and Delete to fetch
+// the full ENI details for a specific interface ID.
+func fetchNetworkInterface(conn *ec2.EC2, ifaceID string) (*ec2.NetworkInterface, error) {
+ log.Printf("[DEBUG] Fetching information for interface ID %s", ifaceID)
+ dniParams := &ec2.DescribeNetworkInterfacesInput{
+ NetworkInterfaceIds: aws.StringSlice([]string{ifaceID}),
+ }
+
+ dniResp, err := conn.DescribeNetworkInterfaces(dniParams)
+ if err != nil {
+ return nil, err
+ }
+ return dniResp.NetworkInterfaces[0], nil
+}
+
+func delSGFromENI(conn *ec2.EC2, sgID string, iface *ec2.NetworkInterface) error {
+ old := iface.Groups
+ var new []*string
+ for _, v := range iface.Groups {
+ if *v.GroupId == sgID {
+ continue
+ }
+ new = append(new, v.GroupId)
+ }
+ if reflect.DeepEqual(old, new) {
+ // The interface already didn't have the security group, nothing to do
+ return nil
+ }
+
+ params := &ec2.ModifyNetworkInterfaceAttributeInput{
+ NetworkInterfaceId: iface.NetworkInterfaceId,
+ Groups: new,
+ }
+
+ _, err := conn.ModifyNetworkInterfaceAttribute(params)
+ return err
+}
+
+// sgExistsInENI is a utility function that can be used to quickly check to
+// see if a security group exists in an *ec2.NetworkInterface.
+func sgExistsInENI(sgID string, iface *ec2.NetworkInterface) bool {
+ for _, v := range iface.Groups {
+ if *v.GroupId == sgID {
+ return true
+ }
+ }
+ return false
+}