From dff49df12903a96597d46d6d3d4b46aa26e0aa03 Mon Sep 17 00:00:00 2001 From: Lucas Bajolet Date: Tue, 30 Jan 2024 11:49:03 -0500 Subject: [PATCH] hcl2template: check bucket name at parse-time Not validating the bucket's name during parse leads to configurations being marked as valid, even if the bucket name is not, which will fail during a real build afterwards. To avoid this problem and fail with an error as quickly as possible, we add a check during parsing, so that it gets reported for validate as well. --- .../testdata/hcp_par/invalid_bucket.pkr.hcl | 12 ++ .../testdata/hcp_par/long-description.pkr.hcl | 6 + .../testdata/hcp_par/long_bucket.pkr.hcl | 12 ++ .../testdata/hcp_par/ok_bucket.pkr.hcl | 12 ++ .../testdata/hcp_par/short_bucket.pkr.hcl | 12 ++ .../types.build.hcp_packer_registry.go | 11 ++ .../types.build.hcp_packer_registry_test.go | 111 ++++++++++++++++++ hcl2template/types.packer_config_test.go | 1 + 8 files changed, 177 insertions(+) create mode 100644 hcl2template/testdata/hcp_par/invalid_bucket.pkr.hcl create mode 100644 hcl2template/testdata/hcp_par/long_bucket.pkr.hcl create mode 100644 hcl2template/testdata/hcp_par/ok_bucket.pkr.hcl create mode 100644 hcl2template/testdata/hcp_par/short_bucket.pkr.hcl diff --git a/hcl2template/testdata/hcp_par/invalid_bucket.pkr.hcl b/hcl2template/testdata/hcp_par/invalid_bucket.pkr.hcl new file mode 100644 index 000000000..6f2d36afa --- /dev/null +++ b/hcl2template/testdata/hcp_par/invalid_bucket.pkr.hcl @@ -0,0 +1,12 @@ +source "null" "test" { + communicator = "none" +} + +build { + name = "bucket-slug" + hcp_packer_registry { + bucket_name = "invalid_bucket" + } + + sources = ["null.test"] +} diff --git a/hcl2template/testdata/hcp_par/long-description.pkr.hcl b/hcl2template/testdata/hcp_par/long-description.pkr.hcl index c20502e1b..68abb110c 100644 --- a/hcl2template/testdata/hcp_par/long-description.pkr.hcl +++ b/hcl2template/testdata/hcp_par/long-description.pkr.hcl @@ -1,3 +1,7 @@ +source "null" "test" { + communicator = "none" +} + build { name = "bucket-slug" hcp_packer_registry { @@ -7,4 +11,6 @@ super super super super super super super super super super super super super su super super super long description EOT } + + sources = ["null.test"] } diff --git a/hcl2template/testdata/hcp_par/long_bucket.pkr.hcl b/hcl2template/testdata/hcp_par/long_bucket.pkr.hcl new file mode 100644 index 000000000..1a234647d --- /dev/null +++ b/hcl2template/testdata/hcp_par/long_bucket.pkr.hcl @@ -0,0 +1,12 @@ +source "null" "test" { + communicator = "none" +} + +build { + name = "bucket-slug" + hcp_packer_registry { + bucket_name = "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb" + } + + sources = ["null.test"] +} diff --git a/hcl2template/testdata/hcp_par/ok_bucket.pkr.hcl b/hcl2template/testdata/hcp_par/ok_bucket.pkr.hcl new file mode 100644 index 000000000..6f2cfbfff --- /dev/null +++ b/hcl2template/testdata/hcp_par/ok_bucket.pkr.hcl @@ -0,0 +1,12 @@ +source "null" "test" { + communicator = "none" +} + +build { + name = "bucket-slug" + hcp_packer_registry { + bucket_name = "ok-Bucket-name-1" + } + + sources = ["null.test"] +} diff --git a/hcl2template/testdata/hcp_par/short_bucket.pkr.hcl b/hcl2template/testdata/hcp_par/short_bucket.pkr.hcl new file mode 100644 index 000000000..734dd8b46 --- /dev/null +++ b/hcl2template/testdata/hcp_par/short_bucket.pkr.hcl @@ -0,0 +1,12 @@ +source "null" "test" { + communicator = "none" +} + +build { + name = "bucket-slug" + hcp_packer_registry { + bucket_name = "ba" + } + + sources = ["null.test"] +} diff --git a/hcl2template/types.build.hcp_packer_registry.go b/hcl2template/types.build.hcp_packer_registry.go index b64b3ff4e..ba8935b5d 100644 --- a/hcl2template/types.build.hcp_packer_registry.go +++ b/hcl2template/types.build.hcp_packer_registry.go @@ -5,6 +5,7 @@ package hcl2template import ( "fmt" + "regexp" "github.com/hashicorp/hcl/v2" "github.com/hashicorp/hcl/v2/gohcl" @@ -23,6 +24,8 @@ type HCPPackerRegistryBlock struct { HCL2Ref } +var bucketNameRegexp = regexp.MustCompile("^[a-zA-Z0-9-]{3,36}$") + func (p *Parser) decodeHCPRegistry(block *hcl.Block, cfg *PackerConfig) (*HCPPackerRegistryBlock, hcl.Diagnostics) { par := &HCPPackerRegistryBlock{} body := block.Body @@ -51,6 +54,14 @@ func (p *Parser) decodeHCPRegistry(block *hcl.Block, cfg *PackerConfig) (*HCPPac return nil, diags } + if !bucketNameRegexp.MatchString(b.Slug) { + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: fmt.Sprintf("%s.bucket_name can only contain between 3 and 36 ASCII letters, numbers and hyphens", buildHCPPackerRegistryLabel), + Subject: block.DefRange.Ptr(), + }) + } + par.Slug = b.Slug par.Description = b.Description diff --git a/hcl2template/types.build.hcp_packer_registry_test.go b/hcl2template/types.build.hcp_packer_registry_test.go index 3ada389c2..b27b80e5b 100644 --- a/hcl2template/types.build.hcp_packer_registry_test.go +++ b/hcl2template/types.build.hcp_packer_registry_test.go @@ -7,7 +7,9 @@ import ( "path/filepath" "testing" + "github.com/hashicorp/hcl/v2" packersdk "github.com/hashicorp/packer-plugin-sdk/packer" + "github.com/hashicorp/packer/builder/null" "github.com/hashicorp/packer/packer" "github.com/zclconf/go-cty/cty" ) @@ -129,11 +131,120 @@ func Test_ParseHCPPackerRegistryBlock(t *testing.T) { &PackerConfig{ CorePackerVersionString: lockedVersion, Basedir: filepath.Join("testdata", "hcp_par"), + Sources: map[SourceRef]SourceBlock{ + refNull: { + Type: "null", + Name: "test", + block: &hcl.Block{ + Type: "source", + }, + }, + }, + }, + true, true, + nil, + false, + }, + {"bucket name too short", + defaultParser, + parseTestArgs{"testdata/hcp_par/short_bucket.pkr.hcl", nil, nil}, + &PackerConfig{ + CorePackerVersionString: lockedVersion, + Basedir: filepath.Join("testdata", "hcp_par"), + Sources: map[SourceRef]SourceBlock{ + refNull: { + Type: "null", + Name: "test", + block: &hcl.Block{ + Type: "source", + }, + }, + }, }, true, true, nil, false, }, + {"bucket name too long", + defaultParser, + parseTestArgs{"testdata/hcp_par/long_bucket.pkr.hcl", nil, nil}, + &PackerConfig{ + CorePackerVersionString: lockedVersion, + Basedir: filepath.Join("testdata", "hcp_par"), + Sources: map[SourceRef]SourceBlock{ + refNull: { + Type: "null", + Name: "test", + block: &hcl.Block{ + Type: "source", + }, + }, + }, + }, + true, true, + nil, + false, + }, + {"bucket name invalid chars", + defaultParser, + parseTestArgs{"testdata/hcp_par/invalid_bucket.pkr.hcl", nil, nil}, + &PackerConfig{ + CorePackerVersionString: lockedVersion, + Basedir: filepath.Join("testdata", "hcp_par"), + Sources: map[SourceRef]SourceBlock{ + refNull: { + Type: "null", + Name: "test", + block: &hcl.Block{ + Type: "source", + }, + }, + }, + }, + true, true, + nil, + false, + }, + {"bucket name OK", + defaultParser, + parseTestArgs{"testdata/hcp_par/ok_bucket.pkr.hcl", nil, nil}, + &PackerConfig{ + CorePackerVersionString: lockedVersion, + Basedir: filepath.Join("testdata", "hcp_par"), + Sources: map[SourceRef]SourceBlock{ + refNull: { + Type: "null", + Name: "test", + block: &hcl.Block{ + Type: "source", + }, + }, + }, + Builds: Builds{ + { + Name: "bucket-slug", + HCPPackerRegistry: &HCPPackerRegistryBlock{Slug: "ok-Bucket-name-1"}, + Sources: []SourceUseBlock{ + { + SourceRef: refNull, + }, + }, + }, + }, + }, + false, false, + []packersdk.Build{ + &packer.CoreBuild{ + BuildName: "bucket-slug", + Type: "null.test", + Builder: &null.Builder{}, + Provisioners: []packer.CoreBuildProvisioner{}, + PostProcessors: [][]packer.CoreBuildPostProcessor{}, + Prepared: true, + }, + }, + false, + }, } testParse(t, tests) } diff --git a/hcl2template/types.packer_config_test.go b/hcl2template/types.packer_config_test.go index 5eae6a2f2..69bc2da2c 100644 --- a/hcl2template/types.packer_config_test.go +++ b/hcl2template/types.packer_config_test.go @@ -22,6 +22,7 @@ var ( refVBIsoUbuntu1204 = SourceRef{Type: "virtualbox-iso", Name: "ubuntu-1204"} refAWSEBSUbuntu1604 = SourceRef{Type: "amazon-ebs", Name: "ubuntu-1604"} refAWSV3MyImage = SourceRef{Type: "amazon-v3-ebs", Name: "my-image"} + refNull = SourceRef{Type: "null", Name: "test"} pTrue = pointerToBool(true) )