builder: check Org/OU ARNs during prepare

When a builder accepts organisation or organisational unit ARNs as part
of its config, we don't actually validate that the format matches what
AWS accepts, leading to confusion among our users.

This commit checks that the provided ARNs match the expected format
before running the build, and tries to provide information on what to
change in the configs so they succeed later.

builder/common/ami_config.go

@@ -16,6 +16,12 @@ import (
 	"github.com/hashicorp/packer-plugin-sdk/template/interpolate"
 )
 
+// OrgARNRegexp validates organisation ARNs
+var OrgARNRegexp = regexp.MustCompile(`^arn:aws:organizations::\d{12}:organization\/o-[a-z0-9]{10,32}`)
+
+// ARNRegexp validates organisation unit (OU) ARNs
+var OUARNRegexp = regexp.MustCompile(`^arn:aws:organizations::\d{12}:ou\/o-[a-z0-9]{10,32}\/ou-[0-9a-z]{4,32}-[0-9a-z]{8,32}`)
+
 // AMIConfig is for common configuration related to creating AMIs.
 type AMIConfig struct {
 	// The name of the resulting AMI that will appear when managing AMIs in the
@@ -195,6 +201,34 @@ func (c *AMIConfig) Prepare(accessConfig *AccessConfig, ctx *interpolate.Context
 
 	errs = append(errs, c.prepareRegions(accessConfig)...)
 
+	for _, orgARN := range c.AMIOrgArns {
+		if !OrgARNRegexp.MatchString(orgARN) {
+			if OUARNRegexp.MatchString(orgARN) {
+				errs = append(errs,
+					fmt.Errorf("Organisation ARN %q looks like a OU ARN, "+
+						"it should be part of the `ami_ou_arns` collection instead", orgARN))
+				continue
+			}
+
+			errs = append(errs, fmt.Errorf("Invalid Organisation ARN %q, expect something that matches the "+
+				"following: ^arn:aws:organizations::\\d{12}:organization\\/o-[a-z0-9]{10,32}", orgARN))
+		}
+	}
+
+	for _, ouARN := range c.AMIOuArns {
+		if !OUARNRegexp.MatchString(ouARN) {
+			if OrgARNRegexp.MatchString(ouARN) {
+				errs = append(errs,
+					fmt.Errorf("Organisation Unit ARN %q looks like a Organisation ARN, "+
+						"it should be part of the `ami_org_arns` collection instead", ouARN))
+				continue
+			}
+
+			errs = append(errs, fmt.Errorf("Invalid Organisation Unit ARN %q, expect something that matches the "+
+				"following: ^arn:aws:organizations::\\d{12}:ou\\/o-[a-z0-9]{10,32}\\/ou-[0-9a-z]{4,32}-[0-9a-z]{8,32}", ouARN))
+		}
+	}
+
 	// Prevent sharing of default KMS key encrypted volumes with other aws users
 	if len(c.AMIUsers) > 0 || len(c.AMIOrgArns) > 0 || len(c.AMIOuArns) > 0 {
 		if len(c.AMIKmsKeyId) == 0 && len(c.AMIRegionKMSKeyIDs) == 0 && c.AMIEncryptBootVolume.True() {

builder/common/ami_config_test.go

@@ -283,3 +283,78 @@ func TestAMINameValidation(t *testing.T) {
 	}
 
 }
+
+// Make sure only valid ARNs are accepted
+func TestAMIARNValidation(t *testing.T) {
+	tests := []struct {
+		name      string
+		OrgARN    []string
+		OUARN     []string
+		expectErr bool
+	}{
+		{
+			"Nothing defined, should succeed",
+			nil,
+			nil,
+			false,
+		},
+		{
+			"Invalid OrgARN defined, should fail",
+			[]string{"arn:aws:organizations::1234"},
+			nil,
+			true,
+		},
+		{
+			"Invalid OUARN defined, should fail",
+			nil,
+			[]string{"arn:aws:organizations::1234:ou/o-1234567890/ou-aced"},
+			true,
+		},
+		{
+			"Valid OrgARN, invalid OUARN defined, should fail",
+			[]string{"arn:aws:organizations::123456789012:organization/o-1234567890000"},
+			[]string{"arn:aws:organizations::1234:ou/o-1234567890/ou-aced"},
+			true,
+		},
+		{
+			"Invalid OrgARN, valid OUARN defined, should fail",
+			[]string{"arn:aws:organizations::1234:organization"},
+			[]string{"arn:aws:organizations::123456789012:ou/o-12345abcdef/ou-ab12-12345678"},
+			true,
+		},
+		{
+			"Valid OrgARN, valid OUARN defined, should succeed",
+			[]string{"arn:aws:organizations::123456789012:organization/o-1234567890000"},
+			[]string{"arn:aws:organizations::123456789012:ou/o-12345abcdef/ou-ab12-12345678"},
+			false,
+		},
+		{
+			"Valid OrgARN as OU ARN, should fail",
+			[]string{"arn:aws:organizations::123456789012:ou/o-12345abcdef/ou-ab12-12345678"},
+			nil,
+			true,
+		},
+		{
+			"Valid OU ARN as Org ARN, should fail",
+			nil,
+			[]string{"arn:aws:organizations::123456789012:organization/o-1234567890000"},
+			true,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			c := testAMIConfig()
+			c.AMIOrgArns = tt.OrgARN
+			c.AMIOuArns = tt.OUARN
+
+			errs := c.Prepare(FakeAccessConfig(), nil)
+			if len(errs) != 0 && !tt.expectErr {
+				t.Errorf("Unexpected error %v; expected none.", errs)
+			}
+			if len(errs) == 0 && tt.expectErr {
+				t.Errorf("Expected error, got none.")
+			}
+		})
+	}
+}