implement custom data type "trilean" (tri-state-boolean) to track booleans which have a "null" or "unset" state. Previously we used *bool for these template options, but it turns out that those won't work because "unset" will evaluate to "false" if a user is using template variables to set the option that maps to a *bool.

This commit is contained in:
Megan Marsh 2019-08-19 09:48:32 -07:00
parent 17d9a85895
commit 3c3f7f26ce
10 changed files with 216 additions and 50 deletions

View File

@ -4,6 +4,7 @@ import (
"reflect" "reflect"
"testing" "testing"
helperconfig "github.com/hashicorp/packer/helper/config"
"github.com/hashicorp/packer/packer" "github.com/hashicorp/packer/packer"
) )
@ -126,13 +127,16 @@ func TestBuilderPrepare_Devices(t *testing.T) {
if err != nil { if err != nil {
t.Fatalf("should not have error: %s", err) t.Fatalf("should not have error: %s", err)
} }
if !reflect.DeepEqual(b.config.ECSSystemDiskMapping, AlicloudDiskDevice{ expected := AlicloudDiskDevice{
DiskCategory: "cloud", DiskCategory: "cloud",
Description: "system disk", Description: "system disk",
DiskName: "system_disk", DiskName: "system_disk",
DiskSize: 60, DiskSize: 60,
}) { RawEncrypted: helperconfig.TriUnset,
t.Fatalf("system disk is not set properly, actual: %#v", b.config.ECSSystemDiskMapping) Encrypted: nil,
}
if !reflect.DeepEqual(b.config.ECSSystemDiskMapping, expected) {
t.Fatalf("system disk is not set properly, actual: %v; expected: %v", b.config.ECSSystemDiskMapping, expected)
} }
if !reflect.DeepEqual(b.config.ECSImagesDiskMappings, []AlicloudDiskDevice{ if !reflect.DeepEqual(b.config.ECSImagesDiskMappings, []AlicloudDiskDevice{
{ {

View File

@ -5,6 +5,7 @@ import (
"regexp" "regexp"
"strings" "strings"
"github.com/hashicorp/packer/helper/config"
"github.com/hashicorp/packer/template/interpolate" "github.com/hashicorp/packer/template/interpolate"
) )
@ -16,7 +17,9 @@ type AlicloudDiskDevice struct {
Description string `mapstructure:"disk_description"` Description string `mapstructure:"disk_description"`
DeleteWithInstance bool `mapstructure:"disk_delete_with_instance"` DeleteWithInstance bool `mapstructure:"disk_delete_with_instance"`
Device string `mapstructure:"disk_device"` Device string `mapstructure:"disk_device"`
Encrypted *bool `mapstructure:"disk_encrypted"` RawEncrypted config.Trilean `mapstructure:"disk_encrypted"`
Encrypted *bool
} }
type AlicloudDiskDevices struct { type AlicloudDiskDevices struct {
@ -32,7 +35,7 @@ type AlicloudImageConfig struct {
AlicloudImageUNShareAccounts []string `mapstructure:"image_unshare_account"` AlicloudImageUNShareAccounts []string `mapstructure:"image_unshare_account"`
AlicloudImageDestinationRegions []string `mapstructure:"image_copy_regions"` AlicloudImageDestinationRegions []string `mapstructure:"image_copy_regions"`
AlicloudImageDestinationNames []string `mapstructure:"image_copy_names"` AlicloudImageDestinationNames []string `mapstructure:"image_copy_names"`
ImageEncrypted *bool `mapstructure:"image_encrypted"` RawImageEncrypted config.Trilean `mapstructure:"image_encrypted"`
AlicloudImageForceDelete bool `mapstructure:"image_force_delete"` AlicloudImageForceDelete bool `mapstructure:"image_force_delete"`
AlicloudImageForceDeleteSnapshots bool `mapstructure:"image_force_delete_snapshots"` AlicloudImageForceDeleteSnapshots bool `mapstructure:"image_force_delete_snapshots"`
AlicloudImageForceDeleteInstances bool `mapstructure:"image_force_delete_instances"` AlicloudImageForceDeleteInstances bool `mapstructure:"image_force_delete_instances"`
@ -40,6 +43,8 @@ type AlicloudImageConfig struct {
AlicloudImageSkipRegionValidation bool `mapstructure:"skip_region_validation"` AlicloudImageSkipRegionValidation bool `mapstructure:"skip_region_validation"`
AlicloudImageTags map[string]string `mapstructure:"tags"` AlicloudImageTags map[string]string `mapstructure:"tags"`
AlicloudDiskDevices `mapstructure:",squash"` AlicloudDiskDevices `mapstructure:",squash"`
ImageEncrypted *bool
} }
func (c *AlicloudImageConfig) Prepare(ctx *interpolate.Context) []error { func (c *AlicloudImageConfig) Prepare(ctx *interpolate.Context) []error {
@ -75,6 +80,13 @@ func (c *AlicloudImageConfig) Prepare(ctx *interpolate.Context) []error {
c.AlicloudImageDestinationRegions = regions c.AlicloudImageDestinationRegions = regions
} }
c.ImageEncrypted = c.RawImageEncrypted.ToBoolPointer()
c.ECSSystemDiskMapping.Encrypted = c.RawImageEncrypted.ToBoolPointer()
for i := range c.ECSImagesDiskMappings {
c.ECSImagesDiskMappings[i].Encrypted = c.RawImageEncrypted.ToBoolPointer()
}
if len(errs) > 0 { if len(errs) > 0 {
return errs return errs
} }

View File

@ -8,13 +8,14 @@ import (
"github.com/hashicorp/packer/common/uuid" "github.com/hashicorp/packer/common/uuid"
"github.com/hashicorp/packer/helper/communicator" "github.com/hashicorp/packer/helper/communicator"
"github.com/hashicorp/packer/helper/config"
"github.com/hashicorp/packer/template/interpolate" "github.com/hashicorp/packer/template/interpolate"
) )
type RunConfig struct { type RunConfig struct {
AssociatePublicIpAddress bool `mapstructure:"associate_public_ip_address"` AssociatePublicIpAddress bool `mapstructure:"associate_public_ip_address"`
ZoneId string `mapstructure:"zone_id"` ZoneId string `mapstructure:"zone_id"`
IOOptimized *bool `mapstructure:"io_optimized"` RawIOOptimized config.Trilean `mapstructure:"io_optimized"`
InstanceType string `mapstructure:"instance_type"` InstanceType string `mapstructure:"instance_type"`
Description string `mapstructure:"description"` Description string `mapstructure:"description"`
AlicloudSourceImage string `mapstructure:"source_image"` AlicloudSourceImage string `mapstructure:"source_image"`
@ -37,6 +38,7 @@ type RunConfig struct {
// Communicator settings // Communicator settings
Comm communicator.Config `mapstructure:",squash"` Comm communicator.Config `mapstructure:",squash"`
SSHPrivateIp bool `mapstructure:"ssh_private_ip"` SSHPrivateIp bool `mapstructure:"ssh_private_ip"`
IOOptimized *bool
} }
func (c *RunConfig) Prepare(ctx *interpolate.Context) []error { func (c *RunConfig) Prepare(ctx *interpolate.Context) []error {
@ -68,5 +70,7 @@ func (c *RunConfig) Prepare(ctx *interpolate.Context) []error {
} }
} }
c.IOOptimized = c.RawIOOptimized.ToBoolPointer()
return errs return errs
} }

View File

@ -5,6 +5,7 @@ import (
"log" "log"
"regexp" "regexp"
"github.com/hashicorp/packer/helper/config"
"github.com/hashicorp/packer/template/interpolate" "github.com/hashicorp/packer/template/interpolate"
) )
@ -19,17 +20,21 @@ type AMIConfig struct {
AMIRegions []string `mapstructure:"ami_regions"` AMIRegions []string `mapstructure:"ami_regions"`
AMISkipRegionValidation bool `mapstructure:"skip_region_validation"` AMISkipRegionValidation bool `mapstructure:"skip_region_validation"`
AMITags TagMap `mapstructure:"tags"` AMITags TagMap `mapstructure:"tags"`
AMIENASupport *bool `mapstructure:"ena_support"` RawAMIENASupport config.Trilean `mapstructure:"ena_support"`
AMISriovNetSupport bool `mapstructure:"sriov_support"` AMISriovNetSupport bool `mapstructure:"sriov_support"`
AMIForceDeregister bool `mapstructure:"force_deregister"` AMIForceDeregister bool `mapstructure:"force_deregister"`
AMIForceDeleteSnapshot bool `mapstructure:"force_delete_snapshot"` AMIForceDeleteSnapshot bool `mapstructure:"force_delete_snapshot"`
AMIEncryptBootVolume *bool `mapstructure:"encrypt_boot"` RawAMIEncryptBootVolume config.Trilean `mapstructure:"encrypt_boot"`
AMIKmsKeyId string `mapstructure:"kms_key_id"` AMIKmsKeyId string `mapstructure:"kms_key_id"`
AMIRegionKMSKeyIDs map[string]string `mapstructure:"region_kms_key_ids"` AMIRegionKMSKeyIDs map[string]string `mapstructure:"region_kms_key_ids"`
SnapshotTags TagMap `mapstructure:"snapshot_tags"` SnapshotTags TagMap `mapstructure:"snapshot_tags"`
SnapshotUsers []string `mapstructure:"snapshot_users"` SnapshotUsers []string `mapstructure:"snapshot_users"`
SnapshotGroups []string `mapstructure:"snapshot_groups"` SnapshotGroups []string `mapstructure:"snapshot_groups"`
AMISkipBuildRegion bool `mapstructure:"skip_save_build_region"` AMISkipBuildRegion bool `mapstructure:"skip_save_build_region"`
// parsed from RawAMIENASupport above. Used in steps.
AMIENASupport *bool
AMIEncryptBootVolume *bool
} }
func stringInSlice(s []string, searchstr string) bool { func stringInSlice(s []string, searchstr string) bool {
@ -60,6 +65,8 @@ func (c *AMIConfig) Prepare(accessConfig *AccessConfig, ctx *interpolate.Context
errs = append(errs, c.prepareRegions(accessConfig)...) errs = append(errs, c.prepareRegions(accessConfig)...)
c.AMIENASupport = c.RawAMIENASupport.ToBoolPointer()
c.AMIEncryptBootVolume = c.RawAMIEncryptBootVolume.ToBoolPointer()
// Prevent sharing of default KMS key encrypted volumes with other aws users // Prevent sharing of default KMS key encrypted volumes with other aws users
if len(c.AMIUsers) > 0 { if len(c.AMIUsers) > 0 {
if len(c.AMIKmsKeyId) == 0 && c.AMIEncryptBootVolume != nil && *c.AMIEncryptBootVolume { if len(c.AMIKmsKeyId) == 0 && c.AMIEncryptBootVolume != nil && *c.AMIEncryptBootVolume {

View File

@ -8,6 +8,7 @@ import (
"github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/service/ec2" "github.com/aws/aws-sdk-go/service/ec2"
"github.com/aws/aws-sdk-go/service/ec2/ec2iface" "github.com/aws/aws-sdk-go/service/ec2/ec2iface"
"github.com/hashicorp/packer/helper/config"
) )
func testAMIConfig() *AMIConfig { func testAMIConfig() *AMIConfig {
@ -138,7 +139,7 @@ func TestAMIConfigPrepare_regions(t *testing.T) {
c.SnapshotUsers = []string{"foo", "bar"} c.SnapshotUsers = []string{"foo", "bar"}
c.AMIKmsKeyId = "123-abc-456" c.AMIKmsKeyId = "123-abc-456"
c.AMIEncryptBootVolume = &[]bool{true}[0] c.RawAMIEncryptBootVolume = config.TriTrue
c.AMIRegions = []string{"us-east-1", "us-west-1"} c.AMIRegions = []string{"us-east-1", "us-west-1"}
c.AMIRegionKMSKeyIDs = map[string]string{ c.AMIRegionKMSKeyIDs = map[string]string{
"us-east-1": "123-456-7890", "us-east-1": "123-456-7890",
@ -161,7 +162,7 @@ func TestAMIConfigPrepare_regions(t *testing.T) {
func TestAMIConfigPrepare_Share_EncryptedBoot(t *testing.T) { func TestAMIConfigPrepare_Share_EncryptedBoot(t *testing.T) {
c := testAMIConfig() c := testAMIConfig()
c.AMIUsers = []string{"testAccountID"} c.AMIUsers = []string{"testAccountID"}
c.AMIEncryptBootVolume = &[]bool{true}[0] c.RawAMIEncryptBootVolume = config.TriTrue
accessConf := testAccessConfig() accessConf := testAccessConfig()

View File

@ -6,6 +6,7 @@ import (
"github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/service/ec2" "github.com/aws/aws-sdk-go/service/ec2"
"github.com/hashicorp/packer/helper/config"
"github.com/hashicorp/packer/template/interpolate" "github.com/hashicorp/packer/template/interpolate"
) )
@ -13,7 +14,7 @@ import (
type BlockDevice struct { type BlockDevice struct {
DeleteOnTermination bool `mapstructure:"delete_on_termination"` DeleteOnTermination bool `mapstructure:"delete_on_termination"`
DeviceName string `mapstructure:"device_name"` DeviceName string `mapstructure:"device_name"`
Encrypted *bool `mapstructure:"encrypted"` RawEncrypted config.Trilean `mapstructure:"encrypted"`
IOPS int64 `mapstructure:"iops"` IOPS int64 `mapstructure:"iops"`
NoDevice bool `mapstructure:"no_device"` NoDevice bool `mapstructure:"no_device"`
SnapshotId string `mapstructure:"snapshot_id"` SnapshotId string `mapstructure:"snapshot_id"`
@ -23,6 +24,8 @@ type BlockDevice struct {
KmsKeyId string `mapstructure:"kms_key_id"` KmsKeyId string `mapstructure:"kms_key_id"`
// ebssurrogate only // ebssurrogate only
OmitFromArtifact bool `mapstructure:"omit_from_artifact"` OmitFromArtifact bool `mapstructure:"omit_from_artifact"`
Encrypted *bool
} }
type BlockDevices struct { type BlockDevices struct {
@ -93,6 +96,7 @@ func (b *BlockDevice) Prepare(ctx *interpolate.Context) error {
return fmt.Errorf("The `device_name` must be specified " + return fmt.Errorf("The `device_name` must be specified " +
"for every device in the block device mapping.") "for every device in the block device mapping.")
} }
b.Encrypted = b.RawEncrypted.ToBoolPointer()
// Warn that encrypted must be true or nil when setting kms_key_id // Warn that encrypted must be true or nil when setting kms_key_id
if b.KmsKeyId != "" && b.Encrypted != nil && *b.Encrypted == false { if b.KmsKeyId != "" && b.Encrypted != nil && *b.Encrypted == false {
return fmt.Errorf("The device %v, must also have `encrypted: "+ return fmt.Errorf("The device %v, must also have `encrypted: "+

View File

@ -24,11 +24,13 @@ type Config struct {
awscommon.RunConfig `mapstructure:",squash"` awscommon.RunConfig `mapstructure:",squash"`
VolumeMappings []BlockDevice `mapstructure:"ebs_volumes"` VolumeMappings []BlockDevice `mapstructure:"ebs_volumes"`
AMIENASupport *bool `mapstructure:"ena_support"` RawAMIENASupport config.Trilean `mapstructure:"ena_support"`
AMISriovNetSupport bool `mapstructure:"sriov_support"` AMISriovNetSupport bool `mapstructure:"sriov_support"`
launchBlockDevices awscommon.BlockDevices launchBlockDevices awscommon.BlockDevices
ctx interpolate.Context ctx interpolate.Context
AMIENASupport *bool
} }
type Builder struct { type Builder struct {
@ -75,6 +77,8 @@ func (b *Builder) Prepare(raws ...interface{}) ([]string, error) {
errs = packer.MultiErrorAppend(errs, err) errs = packer.MultiErrorAppend(errs, err)
} }
b.config.AMIENASupport = b.config.RawAMIENASupport.ToBoolPointer()
if b.config.IsSpotInstance() && ((b.config.AMIENASupport != nil && *b.config.AMIENASupport) || b.config.AMISriovNetSupport) { if b.config.IsSpotInstance() && ((b.config.AMIENASupport != nil && *b.config.AMIENASupport) || b.config.AMISriovNetSupport) {
errs = packer.MultiErrorAppend(errs, errs = packer.MultiErrorAppend(errs,
fmt.Errorf("Spot instances do not support modification, which is required "+ fmt.Errorf("Spot instances do not support modification, which is required "+

View File

@ -0,0 +1,72 @@
package config
import (
"strconv"
)
type Trilean uint8
const (
// This will assign unset to 0, which is the default value in interpolation
TriUnset Trilean = iota
TriTrue
TriFalse
)
func (t Trilean) ToString() string {
if t == TriTrue {
return "TriTrue"
} else if t == TriFalse {
return "TriFalse"
}
return "TriUnset"
}
func (t Trilean) ToBoolPointer() *bool {
if t == TriTrue {
return boolPointer(true)
} else if t == TriFalse {
return boolPointer(false)
}
return nil
}
func (t Trilean) True() bool {
if t == TriTrue {
return true
}
return false
}
func (t Trilean) False() bool {
if t == TriFalse {
return true
}
return false
}
func TrileanFromString(s string) (Trilean, error) {
if s == "" {
return TriUnset, nil
}
b, err := strconv.ParseBool(s)
if err != nil {
return TriUnset, err
} else if b == true {
return TriTrue, nil
} else {
return TriFalse, nil
}
}
func TrileanFromBool(b bool) Trilean {
if b {
return TriTrue
}
return TriFalse
}
func boolPointer(b bool) *bool {
return &b
}

View File

@ -0,0 +1,30 @@
package config
import (
"testing"
)
func TestTrilianParsing(t *testing.T) {
type testCase struct {
Input string
Output Trilean
ErrExpected bool
}
testCases := []testCase{
{"true", TriTrue, false}, {"True", TriTrue, false},
{"false", TriFalse, false}, {"False", TriFalse, false},
{"", TriUnset, false}, {"badvalue", TriUnset, true},
{"FAlse", TriUnset, true}, {"TrUe", TriUnset, true},
}
for _, tc := range testCases {
tril, err := TrileanFromString(tc.Input)
if err != nil {
if tc.ErrExpected == false {
t.Fatalf("Didn't expect error: %v", tc)
}
}
if tc.Output != tril {
t.Fatalf("Didn't return proper trilean. %v", tc)
}
}
}

View File

@ -29,6 +29,7 @@ type DecodeOpts struct {
var DefaultDecodeHookFuncs = []mapstructure.DecodeHookFunc{ var DefaultDecodeHookFuncs = []mapstructure.DecodeHookFunc{
uint8ToStringHook, uint8ToStringHook,
stringToTrilean,
mapstructure.StringToSliceHookFunc(","), mapstructure.StringToSliceHookFunc(","),
mapstructure.StringToTimeDurationHookFunc(), mapstructure.StringToTimeDurationHookFunc(),
} }
@ -154,3 +155,30 @@ func uint8ToStringHook(f reflect.Kind, t reflect.Kind, v interface{}) (interface
return v, nil return v, nil
} }
func stringToTrilean(f reflect.Type, t reflect.Type, v interface{}) (interface{}, error) {
// We have a custom data type, config, which we read from a string and
// then cast to a *bool. Why? So that we can appropriately read "unset"
// *bool values in order to intelligently default, even when the values are
// being set by a template variable.
testTril, _ := TrileanFromString("")
if t == reflect.TypeOf(testTril) {
// From value is string
if f == reflect.TypeOf("") {
tril, err := TrileanFromString(v.(string))
if err != nil {
return v, fmt.Errorf("Error parsing bool from given var: %s", err)
}
return tril, nil
} else {
// From value is boolean
if f == reflect.TypeOf(true) {
tril := TrileanFromBool(v.(bool))
return tril, nil
}
}
}
return v, nil
}