From f07e4214cc107b39348043eed14389093a389ce7 Mon Sep 17 00:00:00 2001 From: Akshat Mahajan Date: Sat, 30 Mar 2019 15:47:03 -0700 Subject: [PATCH 1/2] Addresses issues #5384, #5494: Rename and change `temporary_security_group_source_cidr` to accept a list of strings (for Amazon builders). Per this change, `temporary_security_group_source_cidr` in the configuration: 1. Will be renamed to `temporary_security_group_source_cidrs`. 2. Will accept a list of CIDRs. 3. Will have its documentation updated to reflect this change. 4. Will have a fixer attached for newer templates to avail of. --- builder/amazon/common/run_config.go | 12 +++-- builder/amazon/common/step_security_group.go | 33 +++++++----- builder/amazon/ebs/builder.go | 8 +-- builder/amazon/ebssurrogate/builder.go | 8 +-- builder/amazon/ebsvolume/builder.go | 8 +-- builder/amazon/instance/builder.go | 8 +-- fix/fixer.go | 2 + ...r_amazon_temporary_security_group_cidrs.go | 53 +++++++++++++++++++ ...zon_temporary_security_group_cidrs_test.go | 50 +++++++++++++++++ .../source/docs/builders/amazon-ebs.html.md | 9 ++-- .../docs/builders/amazon-ebssurrogate.html.md | 9 ++-- .../docs/builders/amazon-ebsvolume.html.md | 9 ++-- .../docs/builders/amazon-instance.html.md | 9 ++-- 13 files changed, 164 insertions(+), 54 deletions(-) create mode 100644 fix/fixer_amazon_temporary_security_group_cidrs.go create mode 100644 fix/fixer_amazon_temporary_security_group_cidrs_test.go diff --git a/builder/amazon/common/run_config.go b/builder/amazon/common/run_config.go index 6973d6f35..db8a76488 100644 --- a/builder/amazon/common/run_config.go +++ b/builder/amazon/common/run_config.go @@ -79,7 +79,7 @@ type RunConfig struct { SubnetFilter SubnetFilterOptions `mapstructure:"subnet_filter"` SubnetId string `mapstructure:"subnet_id"` TemporaryKeyPairName string `mapstructure:"temporary_key_pair_name"` - TemporarySGSourceCidr string `mapstructure:"temporary_security_group_source_cidr"` + TemporarySGSourceCidrs []string `mapstructure:"temporary_security_group_source_cidrs"` UserData string `mapstructure:"user_data"` UserDataFile string `mapstructure:"user_data_file"` VpcFilter VpcFilterOptions `mapstructure:"vpc_filter"` @@ -184,11 +184,13 @@ func (c *RunConfig) Prepare(ctx *interpolate.Context) []error { } } - if c.TemporarySGSourceCidr == "" { - c.TemporarySGSourceCidr = "0.0.0.0/0" + if len(c.TemporarySGSourceCidrs) == 0 { + c.TemporarySGSourceCidrs = []string{"0.0.0.0/0"} } else { - if _, _, err := net.ParseCIDR(c.TemporarySGSourceCidr); err != nil { - errs = append(errs, fmt.Errorf("Error parsing temporary_security_group_source_cidr: %s", err.Error())) + for _, cidr := range c.TemporarySGSourceCidrs { + if _, _, err := net.ParseCIDR(cidr); err != nil { + errs = append(errs, fmt.Errorf("Error parsing CIDR in temporary_security_group_source_cidrs: %s", err.Error())) + } } } diff --git a/builder/amazon/common/step_security_group.go b/builder/amazon/common/step_security_group.go index 97bf556f6..f799b23b7 100644 --- a/builder/amazon/common/step_security_group.go +++ b/builder/amazon/common/step_security_group.go @@ -17,10 +17,10 @@ import ( ) type StepSecurityGroup struct { - CommConfig *communicator.Config - SecurityGroupFilter SecurityGroupFilterOptions - SecurityGroupIds []string - TemporarySGSourceCidr string + CommConfig *communicator.Config + SecurityGroupFilter SecurityGroupFilterOptions + SecurityGroupIds []string + TemporarySGSourceCidrs []string createdGroupId string } @@ -119,26 +119,33 @@ func (s *StepSecurityGroup) Run(_ context.Context, state multistep.StateBag) mul return multistep.ActionHalt } + // map the list of temporary security group CIDRs bundled with config to + // types expected by EC2. + groupIpRanges := []*ec2.IpRange{} + for _, cidr := range s.TemporarySGSourceCidrs { + ipRange := ec2.IpRange{ + CidrIp: aws.String(cidr), + } + groupIpRanges = append(groupIpRanges, &ipRange) + } + // Authorize the SSH access for the security group groupRules := &ec2.AuthorizeSecurityGroupIngressInput{ GroupId: groupResp.GroupId, IpPermissions: []*ec2.IpPermission{ { - FromPort: aws.Int64(int64(port)), - ToPort: aws.Int64(int64(port)), - IpRanges: []*ec2.IpRange{ - { - CidrIp: aws.String(s.TemporarySGSourceCidr), - }, - }, + FromPort: aws.Int64(int64(port)), + ToPort: aws.Int64(int64(port)), + IpRanges: groupIpRanges, IpProtocol: aws.String("tcp"), }, }, } ui.Say(fmt.Sprintf( - "Authorizing access to port %d from %s in the temporary security group...", - port, s.TemporarySGSourceCidr)) + "Authorizing access to port %d from %v in the temporary security groups...", + port, s.TemporarySGSourceCidrs), + ) _, err = ec2conn.AuthorizeSecurityGroupIngress(groupRules) if err != nil { err := fmt.Errorf("Error authorizing temporary security group: %s", err) diff --git a/builder/amazon/ebs/builder.go b/builder/amazon/ebs/builder.go index ae318c2de..248c22623 100644 --- a/builder/amazon/ebs/builder.go +++ b/builder/amazon/ebs/builder.go @@ -178,10 +178,10 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook) (packer.Artifact, error) { DebugKeyPath: fmt.Sprintf("ec2_%s.pem", b.config.PackerBuildName), }, &awscommon.StepSecurityGroup{ - SecurityGroupFilter: b.config.SecurityGroupFilter, - SecurityGroupIds: b.config.SecurityGroupIds, - CommConfig: &b.config.RunConfig.Comm, - TemporarySGSourceCidr: b.config.TemporarySGSourceCidr, + SecurityGroupFilter: b.config.SecurityGroupFilter, + SecurityGroupIds: b.config.SecurityGroupIds, + CommConfig: &b.config.RunConfig.Comm, + TemporarySGSourceCidrs: b.config.TemporarySGSourceCidrs, }, &awscommon.StepCleanupVolumes{ BlockDevices: b.config.BlockDevices, diff --git a/builder/amazon/ebssurrogate/builder.go b/builder/amazon/ebssurrogate/builder.go index b1d4dbf8e..cab8550ae 100644 --- a/builder/amazon/ebssurrogate/builder.go +++ b/builder/amazon/ebssurrogate/builder.go @@ -194,10 +194,10 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook) (packer.Artifact, error) { DebugKeyPath: fmt.Sprintf("ec2_%s.pem", b.config.PackerBuildName), }, &awscommon.StepSecurityGroup{ - SecurityGroupFilter: b.config.SecurityGroupFilter, - SecurityGroupIds: b.config.SecurityGroupIds, - CommConfig: &b.config.RunConfig.Comm, - TemporarySGSourceCidr: b.config.TemporarySGSourceCidr, + SecurityGroupFilter: b.config.SecurityGroupFilter, + SecurityGroupIds: b.config.SecurityGroupIds, + CommConfig: &b.config.RunConfig.Comm, + TemporarySGSourceCidrs: b.config.TemporarySGSourceCidrs, }, &awscommon.StepCleanupVolumes{ BlockDevices: b.config.BlockDevices, diff --git a/builder/amazon/ebsvolume/builder.go b/builder/amazon/ebsvolume/builder.go index f284f0e7e..0a5daeb44 100644 --- a/builder/amazon/ebsvolume/builder.go +++ b/builder/amazon/ebsvolume/builder.go @@ -171,10 +171,10 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook) (packer.Artifact, error) { DebugKeyPath: fmt.Sprintf("ec2_%s.pem", b.config.PackerBuildName), }, &awscommon.StepSecurityGroup{ - SecurityGroupFilter: b.config.SecurityGroupFilter, - SecurityGroupIds: b.config.SecurityGroupIds, - CommConfig: &b.config.RunConfig.Comm, - TemporarySGSourceCidr: b.config.TemporarySGSourceCidr, + SecurityGroupFilter: b.config.SecurityGroupFilter, + SecurityGroupIds: b.config.SecurityGroupIds, + CommConfig: &b.config.RunConfig.Comm, + TemporarySGSourceCidrs: b.config.TemporarySGSourceCidrs, }, instanceStep, &stepTagEBSVolumes{ diff --git a/builder/amazon/instance/builder.go b/builder/amazon/instance/builder.go index 3d7a5482e..c9510278c 100644 --- a/builder/amazon/instance/builder.go +++ b/builder/amazon/instance/builder.go @@ -255,10 +255,10 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook) (packer.Artifact, error) { DebugKeyPath: fmt.Sprintf("ec2_%s.pem", b.config.PackerBuildName), }, &awscommon.StepSecurityGroup{ - CommConfig: &b.config.RunConfig.Comm, - SecurityGroupFilter: b.config.SecurityGroupFilter, - SecurityGroupIds: b.config.SecurityGroupIds, - TemporarySGSourceCidr: b.config.TemporarySGSourceCidr, + CommConfig: &b.config.RunConfig.Comm, + SecurityGroupFilter: b.config.SecurityGroupFilter, + SecurityGroupIds: b.config.SecurityGroupIds, + TemporarySGSourceCidrs: b.config.TemporarySGSourceCidrs, }, instanceStep, &awscommon.StepGetPassword{ diff --git a/fix/fixer.go b/fix/fixer.go index 41dd8e9fd..1249e2916 100644 --- a/fix/fixer.go +++ b/fix/fixer.go @@ -35,6 +35,7 @@ func init() { "amazon-shutdown_behavior": new(FixerAmazonShutdownBehavior), "amazon-enhanced-networking": new(FixerAmazonEnhancedNetworking), "amazon-private-ip": new(FixerAmazonPrivateIP), + "amazon-temp-sec-cidrs": new(FixerAmazonTemporarySecurityCIDRs), "docker-email": new(FixerDockerEmail), "powershell-escapes": new(FixerPowerShellEscapes), "hyperv-deprecations": new(FixerHypervDeprecations), @@ -58,6 +59,7 @@ func init() { "amazon-shutdown_behavior", "amazon-enhanced-networking", "amazon-private-ip", + "amazon-temp-sec-cidrs", "docker-email", "powershell-escapes", "vmware-compaction", diff --git a/fix/fixer_amazon_temporary_security_group_cidrs.go b/fix/fixer_amazon_temporary_security_group_cidrs.go new file mode 100644 index 000000000..10048a617 --- /dev/null +++ b/fix/fixer_amazon_temporary_security_group_cidrs.go @@ -0,0 +1,53 @@ +package fix + +import ( + "github.com/mitchellh/mapstructure" + "strings" +) + +type FixerAmazonTemporarySecurityCIDRs struct{} + +func (FixerAmazonTemporarySecurityCIDRs) Fix(input map[string]interface{}) (map[string]interface{}, error) { + // Our template type we'll use for this fixer only + type template struct { + Builders []map[string]interface{} + } + + // Decode the input into our structure, if we can + var tpl template + if err := mapstructure.Decode(input, &tpl); err != nil { + return nil, err + } + + // Go through each builder and replace the temporary_security_group_cidr if we can + for _, builder := range tpl.Builders { + builderTypeRaw, ok := builder["type"] + if !ok { + continue + } + + builderType, ok := builderTypeRaw.(string) + if !ok { + continue + } + + if !strings.HasPrefix(builderType, "amazon-") { + continue + } + + temporarySecurityGroupCIDR, ok := builder["temporary_security_group_cidr"].(string) + if !ok { + continue + } + + delete(builder, "temporary_security_group_cidr") + builder["temporary_security_group_cidrs"] = []string{temporarySecurityGroupCIDR} + } + + input["builders"] = tpl.Builders + return input, nil +} + +func (FixerAmazonTemporarySecurityCIDRs) Synopsis() string { + return `Replaces "temporary_security_group_cidr" (string) with "temporary_security_group_cidrs" (list of strings)` +} diff --git a/fix/fixer_amazon_temporary_security_group_cidrs_test.go b/fix/fixer_amazon_temporary_security_group_cidrs_test.go new file mode 100644 index 000000000..1c0144ac4 --- /dev/null +++ b/fix/fixer_amazon_temporary_security_group_cidrs_test.go @@ -0,0 +1,50 @@ +package fix + +import ( + "reflect" + "testing" +) + +func TestFixerAmazonTemporarySecurityCIDRs_Impl(t *testing.T) { + var _ Fixer = new(FixerAmazonTemporarySecurityCIDRs) +} + +func TestFixerAmazonTemporarySecurityCIDRs(t *testing.T) { + cases := []struct { + Input map[string]interface{} + Expected map[string]interface{} + }{ + { + Input: map[string]interface{}{ + "type": "amazon-ebs", + "temporary_security_group_cidr": "0.0.0.0/0", + }, + + Expected: map[string]interface{}{ + "type": "amazon-ebs", + "temporary_security_group_cidrs": []string{"0.0.0.0/0"}, + }, + }, + } + + for _, tc := range cases { + var f FixerAmazonTemporarySecurityCIDRs + + input := map[string]interface{}{ + "builders": []map[string]interface{}{tc.Input}, + } + + expected := map[string]interface{}{ + "builders": []map[string]interface{}{tc.Expected}, + } + + output, err := f.Fix(input) + if err != nil { + t.Fatalf("err: %s", err) + } + + if !reflect.DeepEqual(output, expected) { + t.Fatalf("unexpected: %#v\nexpected: %#v\n", output, expected) + } + } +} diff --git a/website/source/docs/builders/amazon-ebs.html.md b/website/source/docs/builders/amazon-ebs.html.md index edebf7885..7736b797f 100644 --- a/website/source/docs/builders/amazon-ebs.html.md +++ b/website/source/docs/builders/amazon-ebs.html.md @@ -490,11 +490,10 @@ builder. generate. By default, Packer generates a name that looks like `packer_`, where <UUID> is a 36 character unique identifier. -- `temporary_security_group_source_cidr` (string) - An IPv4 CIDR block to be - authorized access to the instance, when packer is creating a temporary - security group. The default is `0.0.0.0/0` (i.e., allow any IPv4 source). - This is only used when `security_group_id` or `security_group_ids` is not - specified. +- `temporary_security_group_source_cidrs` (list of string) - A list of IPv4 + CIDR blocks to be authorized access to the instance, when packer is creating a temporary security group. + + The default is [`0.0.0.0/0`] (i.e., allow any IPv4 source). This is only used when `security_group_id` or `security_group_ids` is not specified. - `token` (string) - The access token to use. This is different from the access key and secret key. If you're not sure what this is, then you diff --git a/website/source/docs/builders/amazon-ebssurrogate.html.md b/website/source/docs/builders/amazon-ebssurrogate.html.md index da3bb2d66..093a29523 100644 --- a/website/source/docs/builders/amazon-ebssurrogate.html.md +++ b/website/source/docs/builders/amazon-ebssurrogate.html.md @@ -479,11 +479,10 @@ builder. - `temporary_key_pair_name` (string) - The name of the temporary keypair to generate. By default, Packer generates a name with a UUID. -- `temporary_security_group_source_cidr` (string) - An IPv4 CIDR block to be - authorized access to the instance, when packer is creating a temporary - security group. The default is `0.0.0.0/0` (i.e., allow any IPv4 source). - This is only used when `security_group_id` or `security_group_ids` is not - specified. +- `temporary_security_group_source_cidrs` (list of string) - A list of IPv4 + CIDR blocks to be authorized access to the instance, when packer is creating a temporary security group. + + The default is [`0.0.0.0/0`] (i.e., allow any IPv4 source). This is only used when `security_group_id` or `security_group_ids` is not specified. - `token` (string) - The access token to use. This is different from the access key and secret key. If you're not sure what this is, then you diff --git a/website/source/docs/builders/amazon-ebsvolume.html.md b/website/source/docs/builders/amazon-ebsvolume.html.md index bc74acb4f..82138690c 100644 --- a/website/source/docs/builders/amazon-ebsvolume.html.md +++ b/website/source/docs/builders/amazon-ebsvolume.html.md @@ -390,11 +390,10 @@ builder. generate. By default, Packer generates a name that looks like `packer_`, where <UUID> is a 36 character unique identifier. -- `temporary_security_group_source_cidr` (string) - An IPv4 CIDR block to be - authorized access to the instance, when packer is creating a temporary - security group. The default is `0.0.0.0/0` (i.e., allow any IPv4 source). - This is only used when `security_group_id` or `security_group_ids` is not - specified. +- `temporary_security_group_source_cidrs` (list of string) - A list of IPv4 + CIDR blocks to be authorized access to the instance, when packer is creating a temporary security group. + + The default is [`0.0.0.0/0`] (i.e., allow any IPv4 source). This is only used when `security_group_id` or `security_group_ids` is not specified. - `token` (string) - The access token to use. This is different from the access key and secret key. If you're not sure what this is, then you diff --git a/website/source/docs/builders/amazon-instance.html.md b/website/source/docs/builders/amazon-instance.html.md index 2414615a7..8ace80559 100644 --- a/website/source/docs/builders/amazon-instance.html.md +++ b/website/source/docs/builders/amazon-instance.html.md @@ -477,11 +477,10 @@ builder. generate. By default, Packer generates a name that looks like `packer_`, where <UUID> is a 36 character unique identifier. -- `temporary_security_group_source_cidr` (string) - An IPv4 CIDR block to be - authorized access to the instance, when packer is creating a temporary - security group. The default is `0.0.0.0/0` (i.e., allow any IPv4 source). - This is only used when `security_group_id` or `security_group_ids` is not - specified. +- `temporary_security_group_source_cidrs` (list of string) - A list of IPv4 + CIDR blocks to be authorized access to the instance, when packer is creating a temporary security group. + + The default is [`0.0.0.0/0`] (i.e., allow any IPv4 source). This is only used when `security_group_id` or `security_group_ids` is not specified. - `user_data` (string) - User data to apply when launching the instance. Note that you need to be careful about escaping characters due to the templates From 22504200cc561a2adf40d8523852198ce3536b9d Mon Sep 17 00:00:00 2001 From: Akshat Mahajan Date: Mon, 1 Apr 2019 23:42:31 -0700 Subject: [PATCH 2/2] Fix typographical errors in the fixers --- fix/fixer_amazon_temporary_security_group_cidrs.go | 10 +++++----- ...fixer_amazon_temporary_security_group_cidrs_test.go | 8 ++++---- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/fix/fixer_amazon_temporary_security_group_cidrs.go b/fix/fixer_amazon_temporary_security_group_cidrs.go index 10048a617..c48a28011 100644 --- a/fix/fixer_amazon_temporary_security_group_cidrs.go +++ b/fix/fixer_amazon_temporary_security_group_cidrs.go @@ -19,7 +19,7 @@ func (FixerAmazonTemporarySecurityCIDRs) Fix(input map[string]interface{}) (map[ return nil, err } - // Go through each builder and replace the temporary_security_group_cidr if we can + // Go through each builder and replace the temporary_security_group_source_cidr if we can for _, builder := range tpl.Builders { builderTypeRaw, ok := builder["type"] if !ok { @@ -35,13 +35,13 @@ func (FixerAmazonTemporarySecurityCIDRs) Fix(input map[string]interface{}) (map[ continue } - temporarySecurityGroupCIDR, ok := builder["temporary_security_group_cidr"].(string) + temporarySecurityGroupCIDR, ok := builder["temporary_security_group_source_cidr"].(string) if !ok { continue } - delete(builder, "temporary_security_group_cidr") - builder["temporary_security_group_cidrs"] = []string{temporarySecurityGroupCIDR} + delete(builder, "temporary_security_group_source_cidr") + builder["temporary_security_group_source_cidrs"] = []string{temporarySecurityGroupCIDR} } input["builders"] = tpl.Builders @@ -49,5 +49,5 @@ func (FixerAmazonTemporarySecurityCIDRs) Fix(input map[string]interface{}) (map[ } func (FixerAmazonTemporarySecurityCIDRs) Synopsis() string { - return `Replaces "temporary_security_group_cidr" (string) with "temporary_security_group_cidrs" (list of strings)` + return `Replaces "temporary_security_group_source_cidr" (string) with "temporary_security_group_source_cidrs" (list of strings)` } diff --git a/fix/fixer_amazon_temporary_security_group_cidrs_test.go b/fix/fixer_amazon_temporary_security_group_cidrs_test.go index 1c0144ac4..1a17e3f18 100644 --- a/fix/fixer_amazon_temporary_security_group_cidrs_test.go +++ b/fix/fixer_amazon_temporary_security_group_cidrs_test.go @@ -16,13 +16,13 @@ func TestFixerAmazonTemporarySecurityCIDRs(t *testing.T) { }{ { Input: map[string]interface{}{ - "type": "amazon-ebs", - "temporary_security_group_cidr": "0.0.0.0/0", + "type": "amazon-ebs", + "temporary_security_group_source_cidr": "0.0.0.0/0", }, Expected: map[string]interface{}{ - "type": "amazon-ebs", - "temporary_security_group_cidrs": []string{"0.0.0.0/0"}, + "type": "amazon-ebs", + "temporary_security_group_source_cidrs": []string{"0.0.0.0/0"}, }, }, }