Skip to content

Commit 37641d4

Browse files
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.
1 parent 9587b11 commit 37641d4

2 files changed

Lines changed: 109 additions & 0 deletions

File tree

builder/common/ami_config.go

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,12 @@ import (
1616
"github.com/hashicorp/packer-plugin-sdk/template/interpolate"
1717
)
1818

19+
// OrgARNRegexp validates organisation ARNs
20+
var OrgARNRegexp = regexp.MustCompile(`^arn:aws:organizations::\d{12}:organization\/o-[a-z0-9]{10,32}`)
21+
22+
// ARNRegexp validates organisation unit (OU) ARNs
23+
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}`)
24+
1925
// AMIConfig is for common configuration related to creating AMIs.
2026
type AMIConfig struct {
2127
// 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
195201

196202
errs = append(errs, c.prepareRegions(accessConfig)...)
197203

204+
for _, orgARN := range c.AMIOrgArns {
205+
if !OrgARNRegexp.MatchString(orgARN) {
206+
if OUARNRegexp.MatchString(orgARN) {
207+
errs = append(errs,
208+
fmt.Errorf("Organisation ARN %q looks like a OU ARN, "+
209+
"it should be part of the `ami_ou_arns` collection instead", orgARN))
210+
continue
211+
}
212+
213+
errs = append(errs, fmt.Errorf("Invalid Organisation ARN %q, expect something that matches the "+
214+
"following: ^arn:aws:organizations::\\d{12}:organization\\/o-[a-z0-9]{10,32}", orgARN))
215+
}
216+
}
217+
218+
for _, ouARN := range c.AMIOuArns {
219+
if !OUARNRegexp.MatchString(ouARN) {
220+
if OrgARNRegexp.MatchString(ouARN) {
221+
errs = append(errs,
222+
fmt.Errorf("Organisation Unit ARN %q looks like a Organisation ARN, "+
223+
"it should be part of the `ami_org_arns` collection instead", ouARN))
224+
continue
225+
}
226+
227+
errs = append(errs, fmt.Errorf("Invalid Organisation Unit ARN %q, expect something that matches the "+
228+
"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))
229+
}
230+
}
231+
198232
// Prevent sharing of default KMS key encrypted volumes with other aws users
199233
if len(c.AMIUsers) > 0 || len(c.AMIOrgArns) > 0 || len(c.AMIOuArns) > 0 {
200234
if len(c.AMIKmsKeyId) == 0 && len(c.AMIRegionKMSKeyIDs) == 0 && c.AMIEncryptBootVolume.True() {

builder/common/ami_config_test.go

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -283,3 +283,78 @@ func TestAMINameValidation(t *testing.T) {
283283
}
284284

285285
}
286+
287+
// Make sure only valid ARNs are accepted
288+
func TestAMIARNValidation(t *testing.T) {
289+
tests := []struct {
290+
name string
291+
OrgARN []string
292+
OUARN []string
293+
expectErr bool
294+
}{
295+
{
296+
"Nothing defined, should succeed",
297+
nil,
298+
nil,
299+
false,
300+
},
301+
{
302+
"Invalid OrgARN defined, should fail",
303+
[]string{"arn:aws:organizations::1234"},
304+
nil,
305+
true,
306+
},
307+
{
308+
"Invalid OUARN defined, should fail",
309+
nil,
310+
[]string{"arn:aws:organizations::1234:ou/o-1234567890/ou-aced"},
311+
true,
312+
},
313+
{
314+
"Valid OrgARN, invalid OUARN defined, should fail",
315+
[]string{"arn:aws:organizations::123456789012:organization/o-1234567890000"},
316+
[]string{"arn:aws:organizations::1234:ou/o-1234567890/ou-aced"},
317+
true,
318+
},
319+
{
320+
"Invalid OrgARN, valid OUARN defined, should fail",
321+
[]string{"arn:aws:organizations::1234:organization"},
322+
[]string{"arn:aws:organizations::123456789012:ou/o-12345abcdef/ou-ab12-12345678"},
323+
true,
324+
},
325+
{
326+
"Valid OrgARN, valid OUARN defined, should succeed",
327+
[]string{"arn:aws:organizations::123456789012:organization/o-1234567890000"},
328+
[]string{"arn:aws:organizations::123456789012:ou/o-12345abcdef/ou-ab12-12345678"},
329+
false,
330+
},
331+
{
332+
"Valid OrgARN as OU ARN, should fail",
333+
[]string{"arn:aws:organizations::123456789012:ou/o-12345abcdef/ou-ab12-12345678"},
334+
nil,
335+
true,
336+
},
337+
{
338+
"Valid OU ARN as Org ARN, should fail",
339+
nil,
340+
[]string{"arn:aws:organizations::123456789012:organization/o-1234567890000"},
341+
true,
342+
},
343+
}
344+
345+
for _, tt := range tests {
346+
t.Run(tt.name, func(t *testing.T) {
347+
c := testAMIConfig()
348+
c.AMIOrgArns = tt.OrgARN
349+
c.AMIOuArns = tt.OUARN
350+
351+
errs := c.Prepare(FakeAccessConfig(), nil)
352+
if len(errs) != 0 && !tt.expectErr {
353+
t.Errorf("Unexpected error %v; expected none.", errs)
354+
}
355+
if len(errs) == 0 && tt.expectErr {
356+
t.Errorf("Expected error, got none.")
357+
}
358+
})
359+
}
360+
}

0 commit comments

Comments
 (0)