Merge pull request #2308 from mitchellh/b-amazon-ami-sharing-fix

builder/amazon: Fix issue with sharing AMIs with ami_users

builder/amazon/common/step_modify_ami_attributes.go

@@ -54,11 +54,16 @@ func (s *StepModifyAMIAttributes) Run(state multistep.StateBag) multistep.StepAc
 
 	if len(s.Users) > 0 {
 		users := make([]*string, len(s.Users))
+		adds := make([]*ec2.LaunchPermission, len(s.Users))
 		for i, u := range s.Users {
 			users[i] = &u
+			adds[i] = &ec2.LaunchPermission{UserID: &u}
 		}
 		options["users"] = &ec2.ModifyImageAttributeInput{
 			UserIDs: users,
+			LaunchPermission: &ec2.LaunchPermissionModifications{
+				Add: adds,
+			},
 		}
 	}
 

builder/amazon/ebs/builder_acc_test.go

@@ -5,6 +5,7 @@ import (
 	"os"
 	"testing"
 
+	"github.com/aws/aws-sdk-go/aws"
 	"github.com/aws/aws-sdk-go/service/ec2"
 	"github.com/mitchellh/packer/builder/amazon/common"
 	builderT "github.com/mitchellh/packer/helper/builder/testing"
@@ -44,6 +45,60 @@ func TestBuilderAcc_forceDeregister(t *testing.T) {
 	})
 }
 
+func TestBuilderAcc_amiSharing(t *testing.T) {
+	builderT.Test(t, builderT.TestCase{
+		PreCheck: func() { testAccPreCheck(t) },
+		Builder:  &Builder{},
+		Template: testBuilderAccSharing,
+		Check:    checkAMISharing(1, "932021504756"),
+	})
+}
+
+func checkAMISharing(count int, uid string) builderT.TestCheckFunc {
+	return func(artifacts []packer.Artifact) error {
+		if len(artifacts) > 1 {
+			return fmt.Errorf("more than 1 artifact")
+		}
+
+		// Get the actual *Artifact pointer so we can access the AMIs directly
+		artifactRaw := artifacts[0]
+		artifact, ok := artifactRaw.(*common.Artifact)
+		if !ok {
+			return fmt.Errorf("unknown artifact: %#v", artifactRaw)
+		}
+
+		// describe the image, get block devices with a snapshot
+		ec2conn, _ := testEC2Conn()
+		imageResp, err := ec2conn.DescribeImageAttribute(&ec2.DescribeImageAttributeInput{
+			Attribute: aws.String("launchPermission"),
+			ImageID:   aws.String(artifact.Amis["us-east-1"]),
+		})
+
+		if err != nil {
+			return fmt.Errorf("Error retrieving Image Attributes for AMI Artifact (%#v) in AMI Sharing Test: %s", artifact, err)
+		}
+
+		// Launch Permissions are in addition to the userid that created it, so if
+		// you add 3 additional ami_users, you expect 2 Launch Permissions here
+		if len(imageResp.LaunchPermissions) != count {
+			return fmt.Errorf("Error in Image Attributes, expected (%d) Launch Permissions, got (%d)", count, len(imageResp.LaunchPermissions))
+		}
+
+		found := false
+		for _, lp := range imageResp.LaunchPermissions {
+			if uid == *lp.UserID {
+				found = true
+			}
+		}
+
+		if !found {
+			return fmt.Errorf("Error in Image Attributes, expected User ID (%s) to have Launch Permissions, but was not found", uid)
+		}
+
+		return nil
+	}
+}
+
 func checkRegionCopy(regions []string) builderT.TestCheckFunc {
 	return func(artifacts []packer.Artifact) error {
 		if len(artifacts) > 1 {
@@ -138,6 +193,21 @@ const testBuilderAccForceDeregister = `
 }
 `
 
+// share with catsby
+const testBuilderAccSharing = `
+{
+	"builders": [{
+		"type": "test",
+		"region": "us-east-1",
+		"instance_type": "m3.medium",
+		"source_ami": "ami-76b2a71e",
+		"ssh_username": "ubuntu",
+		"ami_name": "packer-test {{timestamp}}",
+		"ami_users":["932021504756"]
+	}]
+}
+`
+
 func buildForceDeregisterConfig(name, flag string) string {
 	return fmt.Sprintf(testBuilderAccForceDeregister, name, flag)
 }