From 59e3ec722dd8bcc7ce327c345885e03152906b10 Mon Sep 17 00:00:00 2001 From: Megan Marsh Date: Wed, 25 Apr 2018 10:47:52 -0700 Subject: [PATCH 01/68] allow user to mount entire block device in chroot builder --- builder/amazon/chroot/builder.go | 6 +++--- builder/amazon/chroot/step_mount_device.go | 7 ++++--- website/source/docs/builders/amazon-chroot.html.md | 10 ++++++---- 3 files changed, 13 insertions(+), 10 deletions(-) diff --git a/builder/amazon/chroot/builder.go b/builder/amazon/chroot/builder.go index 607ad06a9..02923ce31 100644 --- a/builder/amazon/chroot/builder.go +++ b/builder/amazon/chroot/builder.go @@ -35,7 +35,7 @@ type Config struct { DevicePath string `mapstructure:"device_path"` FromScratch bool `mapstructure:"from_scratch"` MountOptions []string `mapstructure:"mount_options"` - MountPartition int `mapstructure:"mount_partition"` + MountPartition string `mapstructure:"mount_partition"` MountPath string `mapstructure:"mount_path"` PostMountCommands []string `mapstructure:"post_mount_commands"` PreMountCommands []string `mapstructure:"pre_mount_commands"` @@ -112,8 +112,8 @@ func (b *Builder) Prepare(raws ...interface{}) ([]string, error) { b.config.MountPath = "/mnt/packer-amazon-chroot-volumes/{{.Device}}" } - if b.config.MountPartition == 0 { - b.config.MountPartition = 1 + if b.config.MountPartition == "" { + b.config.MountPartition = "1" } // Accumulate any errors or warnings diff --git a/builder/amazon/chroot/step_mount_device.go b/builder/amazon/chroot/step_mount_device.go index f9fc7b0a8..c05ae2e77 100644 --- a/builder/amazon/chroot/step_mount_device.go +++ b/builder/amazon/chroot/step_mount_device.go @@ -26,7 +26,7 @@ type mountPathData struct { // mount_device_cleanup CleanupFunc - To perform early cleanup type StepMountDevice struct { MountOptions []string - MountPartition int + MountPartition string mountPath string } @@ -75,8 +75,9 @@ func (s *StepMountDevice) Run(_ context.Context, state multistep.StateBag) multi } deviceMount := device - if virtualizationType == "hvm" { - deviceMount = fmt.Sprintf("%s%d", device, s.MountPartition) + + if virtualizationType == "hvm" && s.MountPartition != "0" { + deviceMount = fmt.Sprintf("%s%s", device, s.MountPartition) } state.Put("deviceMount", deviceMount) diff --git a/website/source/docs/builders/amazon-chroot.html.md b/website/source/docs/builders/amazon-chroot.html.md index 3738d4e7d..da1752144 100644 --- a/website/source/docs/builders/amazon-chroot.html.md +++ b/website/source/docs/builders/amazon-chroot.html.md @@ -213,8 +213,10 @@ each category, the available configuration keys are alphabetized. where the `.Device` variable is replaced with the name of the device where the volume is attached. -- `mount_partition` (number) - The partition number containing the - / partition. By default this is the first partition of the volume. +- `mount_partition` (string) - The partition number containing the + / partition. By default this is the first partition of the volume, (for + example, `xvda1`) but you can designate the entire block device by setting + `"mount_partition": "0"` in your config, which will mount `xvda` instead. - `mount_options` (array of strings) - Options to supply the `mount` command when mounting devices. Each option will be prefixed with `-o` and supplied @@ -291,9 +293,9 @@ each category, the available configuration keys are alphabetized. This is most useful for selecting a daily distro build. You may set this in place of `source_ami` or in conjunction with it. If you - set this in conjunction with `source_ami`, the `source_ami` will be added to + set this in conjunction with `source_ami`, the `source_ami` will be added to the filter. The provided `source_ami` must meet all of the filtering criteria - provided in `source_ami_filter`; this pins the AMI returned by the filter, + provided in `source_ami_filter`; this pins the AMI returned by the filter, but will cause Packer to fail if the `source_ami` does not exist. - `sriov_support` (boolean) - Enable enhanced networking (SriovNetSupport but not ENA) From 616b41e58f1acfd445851543634ac61bb7a6b6ae Mon Sep 17 00:00:00 2001 From: Megan Marsh Date: Fri, 23 Feb 2018 13:26:31 -0800 Subject: [PATCH 02/68] deduplicate the nearly identical communicators for the shell-local provisioner and post-processor, moving single communicator into a new common/shell-local module --- builder/amazon/chroot/run_local_commands.go | 7 ++- .../shell-local/communicator.go | 38 ++++++++--- .../shell-local/communicator_test.go | 2 +- post-processor/shell-local/communicator.go | 63 ------------------- .../shell-local/communicator_test.go | 43 ------------- post-processor/shell-local/post-processor.go | 6 +- provisioner/shell-local/provisioner.go | 25 +------- 7 files changed, 41 insertions(+), 143 deletions(-) rename {provisioner => common}/shell-local/communicator.go (71%) rename {provisioner => common}/shell-local/communicator_test.go (97%) delete mode 100644 post-processor/shell-local/communicator.go delete mode 100644 post-processor/shell-local/communicator_test.go diff --git a/builder/amazon/chroot/run_local_commands.go b/builder/amazon/chroot/run_local_commands.go index 024a208f8..154d37a4f 100644 --- a/builder/amazon/chroot/run_local_commands.go +++ b/builder/amazon/chroot/run_local_commands.go @@ -3,8 +3,8 @@ package chroot import ( "fmt" + sl "github.com/hashicorp/packer/common/shell-local" "github.com/hashicorp/packer/packer" - "github.com/hashicorp/packer/post-processor/shell-local" "github.com/hashicorp/packer/template/interpolate" ) @@ -21,7 +21,10 @@ func RunLocalCommands(commands []string, wrappedCommand CommandWrapper, ctx inte } ui.Say(fmt.Sprintf("Executing command: %s", command)) - comm := &shell_local.Communicator{} + comm := &sl.Communicator{ + Ctx: ctx, + ExecuteCommand: []string{""}, + } cmd := &packer.RemoteCmd{Command: command} if err := cmd.StartWithUi(comm, ui); err != nil { return fmt.Errorf("Error executing command: %s", err) diff --git a/provisioner/shell-local/communicator.go b/common/shell-local/communicator.go similarity index 71% rename from provisioner/shell-local/communicator.go rename to common/shell-local/communicator.go index 2afbe1028..dc84b575a 100644 --- a/provisioner/shell-local/communicator.go +++ b/common/shell-local/communicator.go @@ -1,10 +1,11 @@ -package shell +package shell_local import ( "fmt" "io" "os" "os/exec" + "runtime" "syscall" "github.com/hashicorp/packer/packer" @@ -17,17 +18,34 @@ type Communicator struct { } func (c *Communicator) Start(cmd *packer.RemoteCmd) error { - // Render the template so that we know how to execute the command - c.Ctx.Data = &ExecuteCommandTemplate{ - Command: cmd.Command, - } - for i, field := range c.ExecuteCommand { - command, err := interpolate.Render(field, &c.Ctx) - if err != nil { - return fmt.Errorf("Error processing command: %s", err) + if len(c.ExecuteCommand) == 0 { + // Get default Execute Command + if runtime.GOOS == "windows" { + c.ExecuteCommand = []string{ + "cmd", + "/C", + "{{.Command}}", + } + } else { + c.ExecuteCommand = []string{ + "/bin/sh", + "-c", + "{{.Command}}", + } } + } else { + // Render the template so that we know how to execute the command + c.Ctx.Data = &ExecuteCommandTemplate{ + Command: cmd.Command, + } + for i, field := range c.ExecuteCommand { + command, err := interpolate.Render(field, &c.Ctx) + if err != nil { + return fmt.Errorf("Error processing command: %s", err) + } - c.ExecuteCommand[i] = command + c.ExecuteCommand[i] = command + } } // Build the local command to execute diff --git a/provisioner/shell-local/communicator_test.go b/common/shell-local/communicator_test.go similarity index 97% rename from provisioner/shell-local/communicator_test.go rename to common/shell-local/communicator_test.go index 8ebd4fa60..903ab154d 100644 --- a/provisioner/shell-local/communicator_test.go +++ b/common/shell-local/communicator_test.go @@ -1,4 +1,4 @@ -package shell +package shell_local import ( "bytes" diff --git a/post-processor/shell-local/communicator.go b/post-processor/shell-local/communicator.go deleted file mode 100644 index b0bfb008f..000000000 --- a/post-processor/shell-local/communicator.go +++ /dev/null @@ -1,63 +0,0 @@ -package shell_local - -import ( - "fmt" - "io" - "os" - "os/exec" - "syscall" - - "github.com/hashicorp/packer/packer" -) - -type Communicator struct{} - -func (c *Communicator) Start(cmd *packer.RemoteCmd) error { - localCmd := exec.Command("sh", "-c", cmd.Command) - localCmd.Stdin = cmd.Stdin - localCmd.Stdout = cmd.Stdout - localCmd.Stderr = cmd.Stderr - - // Start it. If it doesn't work, then error right away. - if err := localCmd.Start(); err != nil { - return err - } - - // We've started successfully. Start a goroutine to wait for - // it to complete and track exit status. - go func() { - var exitStatus int - err := localCmd.Wait() - if err != nil { - if exitErr, ok := err.(*exec.ExitError); ok { - exitStatus = 1 - - // There is no process-independent way to get the REAL - // exit status so we just try to go deeper. - if status, ok := exitErr.Sys().(syscall.WaitStatus); ok { - exitStatus = status.ExitStatus() - } - } - } - - cmd.SetExited(exitStatus) - }() - - return nil -} - -func (c *Communicator) Upload(string, io.Reader, *os.FileInfo) error { - return fmt.Errorf("upload not supported") -} - -func (c *Communicator) UploadDir(string, string, []string) error { - return fmt.Errorf("uploadDir not supported") -} - -func (c *Communicator) Download(string, io.Writer) error { - return fmt.Errorf("download not supported") -} - -func (c *Communicator) DownloadDir(src string, dst string, exclude []string) error { - return fmt.Errorf("downloadDir not supported") -} diff --git a/post-processor/shell-local/communicator_test.go b/post-processor/shell-local/communicator_test.go deleted file mode 100644 index 025deec54..000000000 --- a/post-processor/shell-local/communicator_test.go +++ /dev/null @@ -1,43 +0,0 @@ -package shell_local - -import ( - "bytes" - "runtime" - "strings" - "testing" - - "github.com/hashicorp/packer/packer" -) - -func TestCommunicator_impl(t *testing.T) { - var _ packer.Communicator = new(Communicator) -} - -func TestCommunicator(t *testing.T) { - if runtime.GOOS == "windows" { - t.Skip("windows not supported for this test") - return - } - - c := &Communicator{} - - var buf bytes.Buffer - cmd := &packer.RemoteCmd{ - Command: "/bin/echo foo", - Stdout: &buf, - } - - if err := c.Start(cmd); err != nil { - t.Fatalf("err: %s", err) - } - - cmd.Wait() - - if cmd.ExitStatus != 0 { - t.Fatalf("err bad exit status: %d", cmd.ExitStatus) - } - - if strings.TrimSpace(buf.String()) != "foo" { - t.Fatalf("bad: %s", buf.String()) - } -} diff --git a/post-processor/shell-local/post-processor.go b/post-processor/shell-local/post-processor.go index c2bd2d5c0..d77086177 100644 --- a/post-processor/shell-local/post-processor.go +++ b/post-processor/shell-local/post-processor.go @@ -11,6 +11,7 @@ import ( "strings" "github.com/hashicorp/packer/common" + sl "github.com/hashicorp/packer/common/shell-local" "github.com/hashicorp/packer/helper/config" "github.com/hashicorp/packer/packer" "github.com/hashicorp/packer/template/interpolate" @@ -178,7 +179,10 @@ func (p *PostProcessor) PostProcess(ui packer.Ui, artifact packer.Artifact) (pac ui.Say(fmt.Sprintf("Post processing with local shell script: %s", script)) - comm := &Communicator{} + comm := &sl.Communicator{ + Ctx: p.config.ctx, + ExecuteCommand: []string{p.config.ExecuteCommand}, + } cmd := &packer.RemoteCmd{Command: command} diff --git a/provisioner/shell-local/provisioner.go b/provisioner/shell-local/provisioner.go index 3f8222c19..ecd59fa98 100644 --- a/provisioner/shell-local/provisioner.go +++ b/provisioner/shell-local/provisioner.go @@ -3,9 +3,9 @@ package shell import ( "errors" "fmt" - "runtime" "github.com/hashicorp/packer/common" + sl "github.com/hashicorp/packer/common/shell-local" "github.com/hashicorp/packer/helper/config" "github.com/hashicorp/packer/packer" "github.com/hashicorp/packer/template/interpolate" @@ -41,33 +41,12 @@ func (p *Provisioner) Prepare(raws ...interface{}) error { return err } - if len(p.config.ExecuteCommand) == 0 { - if runtime.GOOS == "windows" { - p.config.ExecuteCommand = []string{ - "cmd", - "/C", - "{{.Command}}", - } - } else { - p.config.ExecuteCommand = []string{ - "/bin/sh", - "-c", - "{{.Command}}", - } - } - } - var errs *packer.MultiError if p.config.Command == "" { errs = packer.MultiErrorAppend(errs, errors.New("command must be specified")) } - if len(p.config.ExecuteCommand) == 0 { - errs = packer.MultiErrorAppend(errs, - errors.New("execute_command must not be empty")) - } - if errs != nil && len(errs.Errors) > 0 { return errs } @@ -77,7 +56,7 @@ func (p *Provisioner) Prepare(raws ...interface{}) error { func (p *Provisioner) Provision(ui packer.Ui, _ packer.Communicator) error { // Make another communicator for local - comm := &Communicator{ + comm := &sl.Communicator{ Ctx: p.config.ctx, ExecuteCommand: p.config.ExecuteCommand, } From 926327bebadf68119089074ccd3d0d72033de70b Mon Sep 17 00:00:00 2001 From: Megan Marsh Date: Tue, 27 Feb 2018 12:50:42 -0800 Subject: [PATCH 03/68] deduplicate all validation and interpolation of the shell-local config, sharing options between shell-local provisioner and post-processor. Maintain backwards compatibility with shell-local provisioner. --- common/shell-local/config.go | 154 +++++++++++++++++++ post-processor/shell-local/post-processor.go | 112 +------------- provisioner/shell-local/provisioner.go | 42 +---- 3 files changed, 166 insertions(+), 142 deletions(-) create mode 100644 common/shell-local/config.go diff --git a/common/shell-local/config.go b/common/shell-local/config.go new file mode 100644 index 000000000..b54f8d713 --- /dev/null +++ b/common/shell-local/config.go @@ -0,0 +1,154 @@ +package shell_local + +import ( + "errors" + "fmt" + "os" + "runtime" + "strings" + + "github.com/hashicorp/packer/common" + configHelper "github.com/hashicorp/packer/helper/config" + "github.com/hashicorp/packer/packer" + "github.com/hashicorp/packer/template/interpolate" +) + +type Config struct { + common.PackerConfig `mapstructure:",squash"` + + // ** DEPRECATED: USE INLINE INSTEAD ** + // ** Only Present for backwards compatibiltiy ** + // Command is the command to execute + Command string + + // An inline script to execute. Multiple strings are all executed + // in the context of a single shell. + Inline []string + + // The shebang value used when running inline scripts. + InlineShebang string `mapstructure:"inline_shebang"` + + // The local path of the shell script to upload and execute. + Script string + + // An array of multiple scripts to run. + Scripts []string + + // An array of environment variables that will be injected before + // your command(s) are executed. + Vars []string `mapstructure:"environment_vars"` + // End dedupe with postprocessor + + // The command used to execute the script. The '{{ .Path }}' variable + // should be used to specify where the script goes, {{ .Vars }} + // can be used to inject the environment_vars into the environment. + ExecuteCommand []string `mapstructure:"execute_command"` + + Ctx interpolate.Context +} + +func Decode(config *Config, raws ...interface{}) error { + err := configHelper.Decode(&config, &configHelper.DecodeOpts{ + Interpolate: true, + InterpolateContext: &config.Ctx, + InterpolateFilter: &interpolate.RenderFilter{ + Exclude: []string{ + "execute_command", + }, + }, + }, raws...) + if err != nil { + return err + } + + return Validate(config) +} + +func Validate(config *Config) error { + var errs *packer.MultiError + + if runtime.GOOS == "windows" { + if config.InlineShebang == "" { + config.InlineShebang = "" + } + if len(config.ExecuteCommand) == 0 { + config.ExecuteCommand = []string{`{{.Vars}} "{{.Script}}"`} + } + } else { + if config.InlineShebang == "" { + // TODO: verify that provisioner defaulted to this as well + config.InlineShebang = "/bin/sh -e" + } + if len(config.ExecuteCommand) == 0 { + config.ExecuteCommand = []string{`chmod +x "{{.Script}}"; {{.Vars}} "{{.Script}}"`} + } + } + + // Clean up input + if config.Inline != nil && len(config.Inline) == 0 { + config.Inline = nil + } + + if config.Scripts == nil { + config.Scripts = make([]string, 0) + } + + if config.Vars == nil { + config.Vars = make([]string, 0) + } + + // Verify that the user has given us a command to run + if config.Command != "" && len(config.Inline) == 0 && + len(config.Scripts) == 0 && config.Script == "" { + errs = packer.MultiErrorAppend(errs, + errors.New("Command, Inline, Script and Scripts options cannot all be empty.")) + } + + if config.Command != "" { + // Backwards Compatibility: Before v1.2.2, the shell-local + // provisioner only allowed a single Command, and to run + // multiple commands you needed to run several provisioners in a + // row, one for each command. In deduplicating the post-processor and + // provisioner code, we've changed this to allow an array of scripts or + // inline commands just like in the post-processor. This conditional + // grandfathers in the "Command" option, allowing the original usage to + // continue to work. + config.Inline = append(config.Inline, config.Command) + } + + if config.Script != "" && len(config.Scripts) > 0 { + errs = packer.MultiErrorAppend(errs, + errors.New("Only one of script or scripts can be specified.")) + } + + if config.Script != "" { + config.Scripts = []string{config.Script} + } + + if len(config.Scripts) > 0 && config.Inline != nil { + errs = packer.MultiErrorAppend(errs, + errors.New("You may specify either a script file(s) or an inline script(s), but not both.")) + } + + for _, path := range config.Scripts { + if _, err := os.Stat(path); err != nil { + errs = packer.MultiErrorAppend(errs, + fmt.Errorf("Bad script '%s': %s", path, err)) + } + } + + // Do a check for bad environment variables, such as '=foo', 'foobar' + for _, kv := range config.Vars { + vs := strings.SplitN(kv, "=", 2) + if len(vs) != 2 || vs[0] == "" { + errs = packer.MultiErrorAppend(errs, + fmt.Errorf("Environment variable not in format 'key=value': %s", kv)) + } + } + + if errs != nil && len(errs.Errors) > 0 { + return errs + } + + return nil +} diff --git a/post-processor/shell-local/post-processor.go b/post-processor/shell-local/post-processor.go index d77086177..818a8b44e 100644 --- a/post-processor/shell-local/post-processor.go +++ b/post-processor/shell-local/post-processor.go @@ -2,7 +2,6 @@ package shell_local import ( "bufio" - "errors" "fmt" "io/ioutil" "log" @@ -10,43 +9,13 @@ import ( "sort" "strings" - "github.com/hashicorp/packer/common" sl "github.com/hashicorp/packer/common/shell-local" - "github.com/hashicorp/packer/helper/config" "github.com/hashicorp/packer/packer" "github.com/hashicorp/packer/template/interpolate" ) -type Config struct { - common.PackerConfig `mapstructure:",squash"` - - // An inline script to execute. Multiple strings are all executed - // in the context of a single shell. - Inline []string - - // The shebang value used when running inline scripts. - InlineShebang string `mapstructure:"inline_shebang"` - - // The local path of the shell script to upload and execute. - Script string - - // An array of multiple scripts to run. - Scripts []string - - // An array of environment variables that will be injected before - // your command(s) are executed. - Vars []string `mapstructure:"environment_vars"` - - // The command used to execute the script. The '{{ .Path }}' variable - // should be used to specify where the script goes, {{ .Vars }} - // can be used to inject the environment_vars into the environment. - ExecuteCommand string `mapstructure:"execute_command"` - - ctx interpolate.Context -} - type PostProcessor struct { - config Config + config sl.Config } type ExecuteCommandTemplate struct { @@ -55,78 +24,12 @@ type ExecuteCommandTemplate struct { } func (p *PostProcessor) Configure(raws ...interface{}) error { - err := config.Decode(&p.config, &config.DecodeOpts{ - Interpolate: true, - InterpolateContext: &p.config.ctx, - InterpolateFilter: &interpolate.RenderFilter{ - Exclude: []string{ - "execute_command", - }, - }, - }, raws...) + err := sl.Decode(&p.config, raws) if err != nil { return err } - if p.config.ExecuteCommand == "" { - p.config.ExecuteCommand = `chmod +x "{{.Script}}"; {{.Vars}} "{{.Script}}"` - } - - if p.config.Inline != nil && len(p.config.Inline) == 0 { - p.config.Inline = nil - } - - if p.config.InlineShebang == "" { - p.config.InlineShebang = "/bin/sh -e" - } - - if p.config.Scripts == nil { - p.config.Scripts = make([]string, 0) - } - - if p.config.Vars == nil { - p.config.Vars = make([]string, 0) - } - - var errs *packer.MultiError - if p.config.Script != "" && len(p.config.Scripts) > 0 { - errs = packer.MultiErrorAppend(errs, - errors.New("Only one of script or scripts can be specified.")) - } - - if p.config.Script != "" { - p.config.Scripts = []string{p.config.Script} - } - - if len(p.config.Scripts) == 0 && p.config.Inline == nil { - errs = packer.MultiErrorAppend(errs, - errors.New("Either a script file or inline script must be specified.")) - } else if len(p.config.Scripts) > 0 && p.config.Inline != nil { - errs = packer.MultiErrorAppend(errs, - errors.New("Only a script file or an inline script can be specified, not both.")) - } - - for _, path := range p.config.Scripts { - if _, err := os.Stat(path); err != nil { - errs = packer.MultiErrorAppend(errs, - fmt.Errorf("Bad script '%s': %s", path, err)) - } - } - - // Do a check for bad environment variables, such as '=foo', 'foobar' - for _, kv := range p.config.Vars { - vs := strings.SplitN(kv, "=", 2) - if len(vs) != 2 || vs[0] == "" { - errs = packer.MultiErrorAppend(errs, - fmt.Errorf("Environment variable not in format 'key=value': %s", kv)) - } - } - - if errs != nil && len(errs.Errors) > 0 { - return errs - } - - return nil + return sl.Validate(&p.config) } func (p *PostProcessor) PostProcess(ui packer.Ui, artifact packer.Artifact) (packer.Artifact, bool, error) { @@ -167,12 +70,13 @@ func (p *PostProcessor) PostProcess(ui packer.Ui, artifact packer.Artifact) (pac for _, script := range scripts { - p.config.ctx.Data = &ExecuteCommandTemplate{ + p.config.Ctx.Data = &ExecuteCommandTemplate{ Vars: flattenedEnvVars, Script: script, } - command, err := interpolate.Render(p.config.ExecuteCommand, &p.config.ctx) + flattenedCmd := strings.Join(p.config.ExecuteCommand, " ") + command, err := interpolate.Render(flattenedCmd, &p.config.Ctx) if err != nil { return nil, false, fmt.Errorf("Error processing command: %s", err) } @@ -180,8 +84,8 @@ func (p *PostProcessor) PostProcess(ui packer.Ui, artifact packer.Artifact) (pac ui.Say(fmt.Sprintf("Post processing with local shell script: %s", script)) comm := &sl.Communicator{ - Ctx: p.config.ctx, - ExecuteCommand: []string{p.config.ExecuteCommand}, + Ctx: p.config.Ctx, + ExecuteCommand: []string{flattenedCmd}, } cmd := &packer.RemoteCmd{Command: command} diff --git a/provisioner/shell-local/provisioner.go b/provisioner/shell-local/provisioner.go index ecd59fa98..615a7eb24 100644 --- a/provisioner/shell-local/provisioner.go +++ b/provisioner/shell-local/provisioner.go @@ -1,63 +1,29 @@ package shell import ( - "errors" "fmt" - "github.com/hashicorp/packer/common" sl "github.com/hashicorp/packer/common/shell-local" - "github.com/hashicorp/packer/helper/config" "github.com/hashicorp/packer/packer" - "github.com/hashicorp/packer/template/interpolate" ) -type Config struct { - common.PackerConfig `mapstructure:",squash"` - - // Command is the command to execute - Command string - - // ExecuteCommand is the command used to execute the command. - ExecuteCommand []string `mapstructure:"execute_command"` - - ctx interpolate.Context -} - type Provisioner struct { - config Config + config sl.Config } func (p *Provisioner) Prepare(raws ...interface{}) error { - err := config.Decode(&p.config, &config.DecodeOpts{ - Interpolate: true, - InterpolateContext: &p.config.ctx, - InterpolateFilter: &interpolate.RenderFilter{ - Exclude: []string{ - "execute_command", - }, - }, - }, raws...) + err := sl.Decode(&p.config, raws) if err != nil { return err } - var errs *packer.MultiError - if p.config.Command == "" { - errs = packer.MultiErrorAppend(errs, - errors.New("command must be specified")) - } - - if errs != nil && len(errs.Errors) > 0 { - return errs - } - - return nil + return sl.Validate(&p.config) } func (p *Provisioner) Provision(ui packer.Ui, _ packer.Communicator) error { // Make another communicator for local comm := &sl.Communicator{ - Ctx: p.config.ctx, + Ctx: p.config.Ctx, ExecuteCommand: p.config.ExecuteCommand, } From c7c66bedcba477d145dae01a4e07e0aa9d6cf806 Mon Sep 17 00:00:00 2001 From: Megan Marsh Date: Wed, 28 Feb 2018 09:45:29 -0800 Subject: [PATCH 04/68] set inline to an empty array, rather than nil --- common/shell-local/config.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/shell-local/config.go b/common/shell-local/config.go index b54f8d713..a6a0a279d 100644 --- a/common/shell-local/config.go +++ b/common/shell-local/config.go @@ -86,7 +86,7 @@ func Validate(config *Config) error { // Clean up input if config.Inline != nil && len(config.Inline) == 0 { - config.Inline = nil + config.Inline = make([]string, 0) } if config.Scripts == nil { From 6dc4b1cbdc1be33ff953d11a1b4643f366f05ae0 Mon Sep 17 00:00:00 2001 From: Megan Marsh Date: Wed, 28 Feb 2018 11:53:53 -0800 Subject: [PATCH 05/68] move all of the run commands for shell-local provisioner and postprocessor into common library too --- builder/amazon/chroot/run_local_commands.go | 3 +- common/shell-local/communicator.go | 21 +-- common/shell-local/run.go | 160 +++++++++++++++++++ post-processor/shell-local/post-processor.go | 116 +------------- provisioner/shell-local/provisioner.go | 29 +--- 5 files changed, 172 insertions(+), 157 deletions(-) create mode 100644 common/shell-local/run.go diff --git a/builder/amazon/chroot/run_local_commands.go b/builder/amazon/chroot/run_local_commands.go index 154d37a4f..4d5b0f75c 100644 --- a/builder/amazon/chroot/run_local_commands.go +++ b/builder/amazon/chroot/run_local_commands.go @@ -22,8 +22,7 @@ func RunLocalCommands(commands []string, wrappedCommand CommandWrapper, ctx inte ui.Say(fmt.Sprintf("Executing command: %s", command)) comm := &sl.Communicator{ - Ctx: ctx, - ExecuteCommand: []string{""}, + ExecuteCommand: []string{command}, } cmd := &packer.RemoteCmd{Command: command} if err := cmd.StartWithUi(comm, ui); err != nil { diff --git a/common/shell-local/communicator.go b/common/shell-local/communicator.go index dc84b575a..5532143c9 100644 --- a/common/shell-local/communicator.go +++ b/common/shell-local/communicator.go @@ -9,12 +9,10 @@ import ( "syscall" "github.com/hashicorp/packer/packer" - "github.com/hashicorp/packer/template/interpolate" ) type Communicator struct { ExecuteCommand []string - Ctx interpolate.Context } func (c *Communicator) Start(cmd *packer.RemoteCmd) error { @@ -24,28 +22,17 @@ func (c *Communicator) Start(cmd *packer.RemoteCmd) error { c.ExecuteCommand = []string{ "cmd", "/C", + "{{.Vars}}", "{{.Command}}", } } else { c.ExecuteCommand = []string{ "/bin/sh", "-c", + "{{.Vars}}", "{{.Command}}", } } - } else { - // Render the template so that we know how to execute the command - c.Ctx.Data = &ExecuteCommandTemplate{ - Command: cmd.Command, - } - for i, field := range c.ExecuteCommand { - command, err := interpolate.Render(field, &c.Ctx) - if err != nil { - return fmt.Errorf("Error processing command: %s", err) - } - - c.ExecuteCommand[i] = command - } } // Build the local command to execute @@ -97,7 +84,3 @@ func (c *Communicator) Download(string, io.Writer) error { func (c *Communicator) DownloadDir(string, string, []string) error { return fmt.Errorf("downloadDir not supported") } - -type ExecuteCommandTemplate struct { - Command string -} diff --git a/common/shell-local/run.go b/common/shell-local/run.go new file mode 100644 index 000000000..a42cb3216 --- /dev/null +++ b/common/shell-local/run.go @@ -0,0 +1,160 @@ +package shell_local + +import ( + "bufio" + "fmt" + "io/ioutil" + "log" + "os" + "runtime" + "sort" + "strings" + + "github.com/hashicorp/packer/packer" + "github.com/hashicorp/packer/template/interpolate" +) + +type ExecuteCommandTemplate struct { + Vars string + Script string +} + +func Run(ui packer.Ui, config *Config) (bool, error) { + scripts := make([]string, len(config.Scripts)) + copy(scripts, config.Scripts) + + // If we have an inline script, then turn that into a temporary + // shell script and use that. + if config.Inline != nil { + tf, err := ioutil.TempFile("", "packer-shell") + if err != nil { + return false, fmt.Errorf("Error preparing shell script: %s", err) + } + defer os.Remove(tf.Name()) + + // Set the path to the temporary file + scripts = append(scripts, tf.Name()) + + // Write our contents to it + writer := bufio.NewWriter(tf) + writer.WriteString(fmt.Sprintf("#!%s\n", config.InlineShebang)) + for _, command := range config.Inline { + if _, err := writer.WriteString(command + "\n"); err != nil { + return false, fmt.Errorf("Error preparing shell script: %s", err) + } + } + + if err := writer.Flush(); err != nil { + return false, fmt.Errorf("Error preparing shell script: %s", err) + } + + tf.Close() + } + + // Create environment variables to set before executing the command + flattenedEnvVars := createFlattenedEnvVars(config) + + for _, script := range scripts { + interpolatedCmds, err := createInterpolatedCommands(config, script, flattenedEnvVars) + if err != nil { + return false, err + } + ui.Say(fmt.Sprintf("Post processing with local shell script: %s", script)) + + comm := &Communicator{ + ExecuteCommand: interpolatedCmds, + } + + // The remoteCmd generated here isn't actually run, but it allows us to + // use the same interafce for the shell-local communicator as we use for + // the other communicators; ultimately, this command is just used for + // buffers and for reading the final exit status. + flattenedCmd := strings.Join(interpolatedCmds, " ") + cmd := &packer.RemoteCmd{Command: flattenedCmd} + log.Printf("starting local command: %s", flattenedCmd) + + if err := cmd.StartWithUi(comm, ui); err != nil { + return false, fmt.Errorf( + "Error executing script: %s\n\n"+ + "Please see output above for more information.", + script) + } + if cmd.ExitStatus != 0 { + return false, fmt.Errorf( + "Erroneous exit code %d while executing script: %s\n\n"+ + "Please see output above for more information.", + cmd.ExitStatus, + script) + } + } + + return true, nil +} + +// Generates the final command to send to the communicator, using either the +// user-provided ExecuteCommand or defaulting to something that makes sense for +// the host OS +func createInterpolatedCommands(config *Config, script string, flattenedEnvVars string) ([]string, error) { + config.Ctx.Data = &ExecuteCommandTemplate{ + Vars: flattenedEnvVars, + Script: script, + } + + if len(config.ExecuteCommand) == 0 { + // Get default Execute Command + if runtime.GOOS == "windows" { + config.ExecuteCommand = []string{ + "cmd", + "/C", + "{{.Vars}}", + "{{.Script}}", + } + } else { + config.ExecuteCommand = []string{ + "/bin/sh", + "-c", + "{{.Vars}}", + "{{.Script}}", + } + } + } + interpolatedCmds := make([]string, len(config.ExecuteCommand)) + for i, cmd := range config.ExecuteCommand { + interpolatedCmd, err := interpolate.Render(cmd, &config.Ctx) + if err != nil { + return nil, fmt.Errorf("Error processing command: %s", err) + } + interpolatedCmds[i] = interpolatedCmd + } + return interpolatedCmds, nil +} + +func createFlattenedEnvVars(config *Config) (flattened string) { + flattened = "" + envVars := make(map[string]string) + + // Always available Packer provided env vars + envVars["PACKER_BUILD_NAME"] = fmt.Sprintf("%s", config.PackerBuildName) + envVars["PACKER_BUILDER_TYPE"] = fmt.Sprintf("%s", config.PackerBuilderType) + + // Split vars into key/value components + for _, envVar := range config.Vars { + keyValue := strings.SplitN(envVar, "=", 2) + // Store pair, replacing any single quotes in value so they parse + // correctly with required environment variable format + envVars[keyValue[0]] = strings.Replace(keyValue[1], "'", `'"'"'`, -1) + } + + // Create a list of env var keys in sorted order + var keys []string + for k := range envVars { + keys = append(keys, k) + } + sort.Strings(keys) + + // Re-assemble vars surrounding value with single quotes and flatten + for _, key := range keys { + flattened += fmt.Sprintf("%s='%s' ", key, envVars[key]) + } + return +} diff --git a/post-processor/shell-local/post-processor.go b/post-processor/shell-local/post-processor.go index 818a8b44e..b1585a228 100644 --- a/post-processor/shell-local/post-processor.go +++ b/post-processor/shell-local/post-processor.go @@ -1,17 +1,8 @@ package shell_local import ( - "bufio" - "fmt" - "io/ioutil" - "log" - "os" - "sort" - "strings" - sl "github.com/hashicorp/packer/common/shell-local" "github.com/hashicorp/packer/packer" - "github.com/hashicorp/packer/template/interpolate" ) type PostProcessor struct { @@ -33,108 +24,13 @@ func (p *PostProcessor) Configure(raws ...interface{}) error { } func (p *PostProcessor) PostProcess(ui packer.Ui, artifact packer.Artifact) (packer.Artifact, bool, error) { + // this particular post-processor doesn't do anything with the artifact + // except to return it. - scripts := make([]string, len(p.config.Scripts)) - copy(scripts, p.config.Scripts) - - // If we have an inline script, then turn that into a temporary - // shell script and use that. - if p.config.Inline != nil { - tf, err := ioutil.TempFile("", "packer-shell") - if err != nil { - return nil, false, fmt.Errorf("Error preparing shell script: %s", err) - } - defer os.Remove(tf.Name()) - - // Set the path to the temporary file - scripts = append(scripts, tf.Name()) - - // Write our contents to it - writer := bufio.NewWriter(tf) - writer.WriteString(fmt.Sprintf("#!%s\n", p.config.InlineShebang)) - for _, command := range p.config.Inline { - if _, err := writer.WriteString(command + "\n"); err != nil { - return nil, false, fmt.Errorf("Error preparing shell script: %s", err) - } - } - - if err := writer.Flush(); err != nil { - return nil, false, fmt.Errorf("Error preparing shell script: %s", err) - } - - tf.Close() + retBool, retErr := sl.Run(ui, &p.config) + if !retBool { + return nil, retBool, retErr } - // Create environment variables to set before executing the command - flattenedEnvVars := p.createFlattenedEnvVars() - - for _, script := range scripts { - - p.config.Ctx.Data = &ExecuteCommandTemplate{ - Vars: flattenedEnvVars, - Script: script, - } - - flattenedCmd := strings.Join(p.config.ExecuteCommand, " ") - command, err := interpolate.Render(flattenedCmd, &p.config.Ctx) - if err != nil { - return nil, false, fmt.Errorf("Error processing command: %s", err) - } - - ui.Say(fmt.Sprintf("Post processing with local shell script: %s", script)) - - comm := &sl.Communicator{ - Ctx: p.config.Ctx, - ExecuteCommand: []string{flattenedCmd}, - } - - cmd := &packer.RemoteCmd{Command: command} - - log.Printf("starting local command: %s", command) - if err := cmd.StartWithUi(comm, ui); err != nil { - return nil, false, fmt.Errorf( - "Error executing script: %s\n\n"+ - "Please see output above for more information.", - script) - } - if cmd.ExitStatus != 0 { - return nil, false, fmt.Errorf( - "Erroneous exit code %d while executing script: %s\n\n"+ - "Please see output above for more information.", - cmd.ExitStatus, - script) - } - } - - return artifact, true, nil -} - -func (p *PostProcessor) createFlattenedEnvVars() (flattened string) { - flattened = "" - envVars := make(map[string]string) - - // Always available Packer provided env vars - envVars["PACKER_BUILD_NAME"] = fmt.Sprintf("%s", p.config.PackerBuildName) - envVars["PACKER_BUILDER_TYPE"] = fmt.Sprintf("%s", p.config.PackerBuilderType) - - // Split vars into key/value components - for _, envVar := range p.config.Vars { - keyValue := strings.SplitN(envVar, "=", 2) - // Store pair, replacing any single quotes in value so they parse - // correctly with required environment variable format - envVars[keyValue[0]] = strings.Replace(keyValue[1], "'", `'"'"'`, -1) - } - - // Create a list of env var keys in sorted order - var keys []string - for k := range envVars { - keys = append(keys, k) - } - sort.Strings(keys) - - // Re-assemble vars surrounding value with single quotes and flatten - for _, key := range keys { - flattened += fmt.Sprintf("%s='%s' ", key, envVars[key]) - } - return + return artifact, retBool, retErr } diff --git a/provisioner/shell-local/provisioner.go b/provisioner/shell-local/provisioner.go index 615a7eb24..a56553245 100644 --- a/provisioner/shell-local/provisioner.go +++ b/provisioner/shell-local/provisioner.go @@ -1,8 +1,6 @@ package shell import ( - "fmt" - sl "github.com/hashicorp/packer/common/shell-local" "github.com/hashicorp/packer/packer" ) @@ -21,30 +19,9 @@ func (p *Provisioner) Prepare(raws ...interface{}) error { } func (p *Provisioner) Provision(ui packer.Ui, _ packer.Communicator) error { - // Make another communicator for local - comm := &sl.Communicator{ - Ctx: p.config.Ctx, - ExecuteCommand: p.config.ExecuteCommand, - } - - // Build the remote command - cmd := &packer.RemoteCmd{Command: p.config.Command} - - ui.Say(fmt.Sprintf( - "Executing local command: %s", - p.config.Command)) - if err := cmd.StartWithUi(comm, ui); err != nil { - return fmt.Errorf( - "Error executing command: %s\n\n"+ - "Please see output above for more information.", - p.config.Command) - } - if cmd.ExitStatus != 0 { - return fmt.Errorf( - "Erroneous exit code %d while executing command: %s\n\n"+ - "Please see output above for more information.", - cmd.ExitStatus, - p.config.Command) + _, retErr := sl.Run(ui, &p.config) + if retErr != nil { + return retErr } return nil From 67739270bb32cb850caee1e086948d80fa71448d Mon Sep 17 00:00:00 2001 From: Megan Marsh Date: Wed, 28 Feb 2018 12:17:40 -0800 Subject: [PATCH 06/68] pull temp file writing into its own function for easier testing --- common/shell-local/run.go | 48 ++++++++++++++++++++++----------------- 1 file changed, 27 insertions(+), 21 deletions(-) diff --git a/common/shell-local/run.go b/common/shell-local/run.go index a42cb3216..22366c27f 100644 --- a/common/shell-local/run.go +++ b/common/shell-local/run.go @@ -26,29 +26,12 @@ func Run(ui packer.Ui, config *Config) (bool, error) { // If we have an inline script, then turn that into a temporary // shell script and use that. if config.Inline != nil { - tf, err := ioutil.TempFile("", "packer-shell") + tempScriptFileName, err := createInlineScriptFile(config) if err != nil { - return false, fmt.Errorf("Error preparing shell script: %s", err) + return false, err } - defer os.Remove(tf.Name()) - - // Set the path to the temporary file - scripts = append(scripts, tf.Name()) - - // Write our contents to it - writer := bufio.NewWriter(tf) - writer.WriteString(fmt.Sprintf("#!%s\n", config.InlineShebang)) - for _, command := range config.Inline { - if _, err := writer.WriteString(command + "\n"); err != nil { - return false, fmt.Errorf("Error preparing shell script: %s", err) - } - } - - if err := writer.Flush(); err != nil { - return false, fmt.Errorf("Error preparing shell script: %s", err) - } - - tf.Close() + defer os.Remove(tempScriptFileName) + scripts = append(scripts, tempScriptFileName) } // Create environment variables to set before executing the command @@ -91,6 +74,29 @@ func Run(ui packer.Ui, config *Config) (bool, error) { return true, nil } +func createInlineScriptFile(config *Config) (string, error) { + tf, err := ioutil.TempFile("", "packer-shell") + if err != nil { + return "", fmt.Errorf("Error preparing shell script: %s", err) + } + + // Write our contents to it + writer := bufio.NewWriter(tf) + writer.WriteString(fmt.Sprintf("#!%s\n", config.InlineShebang)) + for _, command := range config.Inline { + if _, err := writer.WriteString(command + "\n"); err != nil { + return "", fmt.Errorf("Error preparing shell script: %s", err) + } + } + + if err := writer.Flush(); err != nil { + return "", fmt.Errorf("Error preparing shell script: %s", err) + } + + tf.Close() + return tf.Name(), nil +} + // Generates the final command to send to the communicator, using either the // user-provided ExecuteCommand or defaulting to something that makes sense for // the host OS From d30423472543a2a81f3b518963c37b495fcf719b Mon Sep 17 00:00:00 2001 From: Megan Marsh Date: Wed, 28 Feb 2018 14:35:42 -0800 Subject: [PATCH 07/68] fix tests --- common/shell-local/communicator.go | 18 +-- common/shell-local/communicator_test.go | 5 +- common/shell-local/config.go | 6 +- post-processor/shell-local/post-processor.go | 2 +- .../shell-local/post-processor_test.go | 130 +++++++----------- 5 files changed, 55 insertions(+), 106 deletions(-) diff --git a/common/shell-local/communicator.go b/common/shell-local/communicator.go index 5532143c9..7664bc896 100644 --- a/common/shell-local/communicator.go +++ b/common/shell-local/communicator.go @@ -5,7 +5,6 @@ import ( "io" "os" "os/exec" - "runtime" "syscall" "github.com/hashicorp/packer/packer" @@ -17,22 +16,7 @@ type Communicator struct { func (c *Communicator) Start(cmd *packer.RemoteCmd) error { if len(c.ExecuteCommand) == 0 { - // Get default Execute Command - if runtime.GOOS == "windows" { - c.ExecuteCommand = []string{ - "cmd", - "/C", - "{{.Vars}}", - "{{.Command}}", - } - } else { - c.ExecuteCommand = []string{ - "/bin/sh", - "-c", - "{{.Vars}}", - "{{.Command}}", - } - } + return fmt.Errorf("Error launching command via shell-local communicator: No ExecuteCommand provided") } // Build the local command to execute diff --git a/common/shell-local/communicator_test.go b/common/shell-local/communicator_test.go index 903ab154d..9a8cb9057 100644 --- a/common/shell-local/communicator_test.go +++ b/common/shell-local/communicator_test.go @@ -20,13 +20,12 @@ func TestCommunicator(t *testing.T) { } c := &Communicator{ - ExecuteCommand: []string{"/bin/sh", "-c", "{{.Command}}"}, + ExecuteCommand: []string{"/bin/sh", "-c", "echo foo"}, } var buf bytes.Buffer cmd := &packer.RemoteCmd{ - Command: "echo foo", - Stdout: &buf, + Stdout: &buf, } if err := c.Start(cmd); err != nil { diff --git a/common/shell-local/config.go b/common/shell-local/config.go index a6a0a279d..dfd3623b9 100644 --- a/common/shell-local/config.go +++ b/common/shell-local/config.go @@ -58,10 +58,10 @@ func Decode(config *Config, raws ...interface{}) error { }, }, raws...) if err != nil { - return err + return fmt.Errorf("Error decoding config: %s, config is %#v, and raws is %#v", err, config, raws) } - return Validate(config) + return nil } func Validate(config *Config) error { @@ -98,7 +98,7 @@ func Validate(config *Config) error { } // Verify that the user has given us a command to run - if config.Command != "" && len(config.Inline) == 0 && + if config.Command == "" && len(config.Inline) == 0 && len(config.Scripts) == 0 && config.Script == "" { errs = packer.MultiErrorAppend(errs, errors.New("Command, Inline, Script and Scripts options cannot all be empty.")) diff --git a/post-processor/shell-local/post-processor.go b/post-processor/shell-local/post-processor.go index b1585a228..91bc5acc9 100644 --- a/post-processor/shell-local/post-processor.go +++ b/post-processor/shell-local/post-processor.go @@ -15,7 +15,7 @@ type ExecuteCommandTemplate struct { } func (p *PostProcessor) Configure(raws ...interface{}) error { - err := sl.Decode(&p.config, raws) + err := sl.Decode(&p.config, raws...) if err != nil { return err } diff --git a/post-processor/shell-local/post-processor_test.go b/post-processor/shell-local/post-processor_test.go index 7bdef1c32..caf4f5a42 100644 --- a/post-processor/shell-local/post-processor_test.go +++ b/post-processor/shell-local/post-processor_test.go @@ -28,20 +28,20 @@ func TestPostProcessor_Impl(t *testing.T) { func TestPostProcessorPrepare_Defaults(t *testing.T) { var p PostProcessor - config := testConfig() + raws := testConfig() - err := p.Configure(config) + err := p.Configure(raws) if err != nil { t.Fatalf("err: %s", err) } } func TestPostProcessorPrepare_InlineShebang(t *testing.T) { - config := testConfig() + raws := testConfig() - delete(config, "inline_shebang") + delete(raws, "inline_shebang") p := new(PostProcessor) - err := p.Configure(config) + err := p.Configure(raws) if err != nil { t.Fatalf("should not have error: %s", err) } @@ -51,9 +51,9 @@ func TestPostProcessorPrepare_InlineShebang(t *testing.T) { } // Test with a good one - config["inline_shebang"] = "foo" + raws["inline_shebang"] = "foo" p = new(PostProcessor) - err = p.Configure(config) + err = p.Configure(raws) if err != nil { t.Fatalf("should not have error: %s", err) } @@ -65,23 +65,23 @@ func TestPostProcessorPrepare_InlineShebang(t *testing.T) { func TestPostProcessorPrepare_InvalidKey(t *testing.T) { var p PostProcessor - config := testConfig() + raws := testConfig() // Add a random key - config["i_should_not_be_valid"] = true - err := p.Configure(config) + raws["i_should_not_be_valid"] = true + err := p.Configure(raws) if err == nil { t.Fatal("should have error") } } func TestPostProcessorPrepare_Script(t *testing.T) { - config := testConfig() - delete(config, "inline") + raws := testConfig() + delete(raws, "inline") - config["script"] = "/this/should/not/exist" + raws["script"] = "/this/should/not/exist" p := new(PostProcessor) - err := p.Configure(config) + err := p.Configure(raws) if err == nil { t.Fatal("should have error") } @@ -93,9 +93,9 @@ func TestPostProcessorPrepare_Script(t *testing.T) { } defer os.Remove(tf.Name()) - config["script"] = tf.Name() + raws["script"] = tf.Name() p = new(PostProcessor) - err = p.Configure(config) + err = p.Configure(raws) if err != nil { t.Fatalf("should not have error: %s", err) } @@ -103,13 +103,16 @@ func TestPostProcessorPrepare_Script(t *testing.T) { func TestPostProcessorPrepare_ScriptAndInline(t *testing.T) { var p PostProcessor - config := testConfig() + raws := testConfig() - delete(config, "inline") - delete(config, "script") - err := p.Configure(config) + // Error if no scripts/inline commands provided + delete(raws, "inline") + delete(raws, "script") + delete(raws, "command") + delete(raws, "scripts") + err := p.Configure(raws) if err == nil { - t.Fatal("should have error") + t.Fatalf("should error when no scripts/inline commands are provided: %#v", raws) } // Test with both @@ -119,9 +122,9 @@ func TestPostProcessorPrepare_ScriptAndInline(t *testing.T) { } defer os.Remove(tf.Name()) - config["inline"] = []interface{}{"foo"} - config["script"] = tf.Name() - err = p.Configure(config) + raws["inline"] = []interface{}{"foo"} + raws["script"] = tf.Name() + err = p.Configure(raws) if err == nil { t.Fatal("should have error") } @@ -129,7 +132,7 @@ func TestPostProcessorPrepare_ScriptAndInline(t *testing.T) { func TestPostProcessorPrepare_ScriptAndScripts(t *testing.T) { var p PostProcessor - config := testConfig() + raws := testConfig() // Test with both tf, err := ioutil.TempFile("", "packer") @@ -138,21 +141,21 @@ func TestPostProcessorPrepare_ScriptAndScripts(t *testing.T) { } defer os.Remove(tf.Name()) - config["inline"] = []interface{}{"foo"} - config["scripts"] = []string{tf.Name()} - err = p.Configure(config) + raws["inline"] = []interface{}{"foo"} + raws["scripts"] = []string{tf.Name()} + err = p.Configure(raws) if err == nil { t.Fatal("should have error") } } func TestPostProcessorPrepare_Scripts(t *testing.T) { - config := testConfig() - delete(config, "inline") + raws := testConfig() + delete(raws, "inline") - config["scripts"] = []string{} + raws["scripts"] = []string{} p := new(PostProcessor) - err := p.Configure(config) + err := p.Configure(raws) if err == nil { t.Fatal("should have error") } @@ -164,92 +167,55 @@ func TestPostProcessorPrepare_Scripts(t *testing.T) { } defer os.Remove(tf.Name()) - config["scripts"] = []string{tf.Name()} + raws["scripts"] = []string{tf.Name()} p = new(PostProcessor) - err = p.Configure(config) + err = p.Configure(raws) if err != nil { t.Fatalf("should not have error: %s", err) } } func TestPostProcessorPrepare_EnvironmentVars(t *testing.T) { - config := testConfig() + raws := testConfig() // Test with a bad case - config["environment_vars"] = []string{"badvar", "good=var"} + raws["environment_vars"] = []string{"badvar", "good=var"} p := new(PostProcessor) - err := p.Configure(config) + err := p.Configure(raws) if err == nil { t.Fatal("should have error") } // Test with a trickier case - config["environment_vars"] = []string{"=bad"} + raws["environment_vars"] = []string{"=bad"} p = new(PostProcessor) - err = p.Configure(config) + err = p.Configure(raws) if err == nil { t.Fatal("should have error") } // Test with a good case // Note: baz= is a real env variable, just empty - config["environment_vars"] = []string{"FOO=bar", "baz="} + raws["environment_vars"] = []string{"FOO=bar", "baz="} p = new(PostProcessor) - err = p.Configure(config) + err = p.Configure(raws) if err != nil { t.Fatalf("should not have error: %s", err) } // Test when the env variable value contains an equals sign - config["environment_vars"] = []string{"good=withequals=true"} + raws["environment_vars"] = []string{"good=withequals=true"} p = new(PostProcessor) - err = p.Configure(config) + err = p.Configure(raws) if err != nil { t.Fatalf("should not have error: %s", err) } // Test when the env variable value starts with an equals sign - config["environment_vars"] = []string{"good==true"} + raws["environment_vars"] = []string{"good==true"} p = new(PostProcessor) - err = p.Configure(config) + err = p.Configure(raws) if err != nil { t.Fatalf("should not have error: %s", err) } } - -func TestPostProcessor_createFlattenedEnvVars(t *testing.T) { - var flattenedEnvVars string - config := testConfig() - - userEnvVarTests := [][]string{ - {}, // No user env var - {"FOO=bar"}, // Single user env var - {"FOO=bar's"}, // User env var with single quote in value - {"FOO=bar", "BAZ=qux"}, // Multiple user env vars - {"FOO=bar=baz"}, // User env var with value containing equals - {"FOO==bar"}, // User env var with value starting with equals - } - expected := []string{ - `PACKER_BUILDER_TYPE='iso' PACKER_BUILD_NAME='vmware' `, - `FOO='bar' PACKER_BUILDER_TYPE='iso' PACKER_BUILD_NAME='vmware' `, - `FOO='bar'"'"'s' PACKER_BUILDER_TYPE='iso' PACKER_BUILD_NAME='vmware' `, - `BAZ='qux' FOO='bar' PACKER_BUILDER_TYPE='iso' PACKER_BUILD_NAME='vmware' `, - `FOO='bar=baz' PACKER_BUILDER_TYPE='iso' PACKER_BUILD_NAME='vmware' `, - `FOO='=bar' PACKER_BUILDER_TYPE='iso' PACKER_BUILD_NAME='vmware' `, - } - - p := new(PostProcessor) - p.Configure(config) - - // Defaults provided by Packer - p.config.PackerBuildName = "vmware" - p.config.PackerBuilderType = "iso" - - for i, expectedValue := range expected { - p.config.Vars = userEnvVarTests[i] - flattenedEnvVars = p.createFlattenedEnvVars() - if flattenedEnvVars != expectedValue { - t.Fatalf("expected flattened env vars to be: %s, got %s.", expectedValue, flattenedEnvVars) - } - } -} From 479d36734ded8c59742a5885b11c15a97b030de8 Mon Sep 17 00:00:00 2001 From: Megan Marsh Date: Wed, 28 Feb 2018 14:43:58 -0800 Subject: [PATCH 08/68] consolidate shell-local defaulting of InlineShebang and ExecuteCommand to the config validation --- common/shell-local/config.go | 18 ++++++++++++------ common/shell-local/run.go | 19 ------------------- 2 files changed, 12 insertions(+), 25 deletions(-) diff --git a/common/shell-local/config.go b/common/shell-local/config.go index dfd3623b9..5b086a794 100644 --- a/common/shell-local/config.go +++ b/common/shell-local/config.go @@ -68,18 +68,24 @@ func Validate(config *Config) error { var errs *packer.MultiError if runtime.GOOS == "windows" { - if config.InlineShebang == "" { - config.InlineShebang = "" - } if len(config.ExecuteCommand) == 0 { - config.ExecuteCommand = []string{`{{.Vars}} "{{.Script}}"`} - } + config.ExecuteCommand = []string{ + "cmd", + "/C", + "{{.Vars}}", + "{{.Script}}", + } } else { if config.InlineShebang == "" { - // TODO: verify that provisioner defaulted to this as well config.InlineShebang = "/bin/sh -e" } if len(config.ExecuteCommand) == 0 { + config.ExecuteCommand = []string{ + "/bin/sh", + "-c", + "{{.Vars}}", + "{{.Script}}", + } config.ExecuteCommand = []string{`chmod +x "{{.Script}}"; {{.Vars}} "{{.Script}}"`} } } diff --git a/common/shell-local/run.go b/common/shell-local/run.go index 22366c27f..04d653389 100644 --- a/common/shell-local/run.go +++ b/common/shell-local/run.go @@ -6,7 +6,6 @@ import ( "io/ioutil" "log" "os" - "runtime" "sort" "strings" @@ -106,24 +105,6 @@ func createInterpolatedCommands(config *Config, script string, flattenedEnvVars Script: script, } - if len(config.ExecuteCommand) == 0 { - // Get default Execute Command - if runtime.GOOS == "windows" { - config.ExecuteCommand = []string{ - "cmd", - "/C", - "{{.Vars}}", - "{{.Script}}", - } - } else { - config.ExecuteCommand = []string{ - "/bin/sh", - "-c", - "{{.Vars}}", - "{{.Script}}", - } - } - } interpolatedCmds := make([]string, len(config.ExecuteCommand)) for i, cmd := range config.ExecuteCommand { interpolatedCmd, err := interpolate.Render(cmd, &config.Ctx) From f799003b66d64d859bff0229550e4123df4e8ef6 Mon Sep 17 00:00:00 2001 From: Megan Marsh Date: Wed, 28 Feb 2018 15:19:28 -0800 Subject: [PATCH 09/68] tighten up shell-local config validation --- common/shell-local/config.go | 35 ++++++++++++++++++----------------- 1 file changed, 18 insertions(+), 17 deletions(-) diff --git a/common/shell-local/config.go b/common/shell-local/config.go index 5b086a794..76c82c793 100644 --- a/common/shell-local/config.go +++ b/common/shell-local/config.go @@ -75,6 +75,7 @@ func Validate(config *Config) error { "{{.Vars}}", "{{.Script}}", } + } } else { if config.InlineShebang == "" { config.InlineShebang = "/bin/sh -e" @@ -110,32 +111,32 @@ func Validate(config *Config) error { errors.New("Command, Inline, Script and Scripts options cannot all be empty.")) } - if config.Command != "" { - // Backwards Compatibility: Before v1.2.2, the shell-local - // provisioner only allowed a single Command, and to run - // multiple commands you needed to run several provisioners in a - // row, one for each command. In deduplicating the post-processor and - // provisioner code, we've changed this to allow an array of scripts or - // inline commands just like in the post-processor. This conditional - // grandfathers in the "Command" option, allowing the original usage to - // continue to work. - config.Inline = append(config.Inline, config.Command) - } + // Check that user hasn't given us too many commands to run + tooManyOptionsErr := errors.New("You may only specify one of the " + + "following options: Command, Inline, Script or Scripts. Please" + + " consolidate these options in your config.") - if config.Script != "" && len(config.Scripts) > 0 { - errs = packer.MultiErrorAppend(errs, - errors.New("Only one of script or scripts can be specified.")) + if config.Command != "" { + if len(config.Inline) != 0 || len(config.Scripts) != 0 || config.Script != "" { + errs = packer.MultiErrorAppend(errs, tooManyOptionsErr) + } else { + config.Inline = []string{config.Command} + } } if config.Script != "" { - config.Scripts = []string{config.Script} + if len(config.Scripts) > 0 || len(config.Inline) > 0 { + errs = packer.MultiErrorAppend(errs, tooManyOptionsErr) + } else { + config.Scripts = []string{config.Script} + } } if len(config.Scripts) > 0 && config.Inline != nil { - errs = packer.MultiErrorAppend(errs, - errors.New("You may specify either a script file(s) or an inline script(s), but not both.")) + errs = packer.MultiErrorAppend(errs, tooManyOptionsErr) } + // Check that all scripts we need to run exist locally for _, path := range config.Scripts { if _, err := os.Stat(path); err != nil { errs = packer.MultiErrorAppend(errs, From 854d6fb141ae89ccf88c8f5c75ecf0c4de588454 Mon Sep 17 00:00:00 2001 From: Megan Marsh Date: Thu, 1 Mar 2018 08:48:21 -0800 Subject: [PATCH 10/68] add tests making sure post-processor has backwards compatability --- common/shell-local/config.go | 1 - post-processor/shell-local/post-processor.go | 14 +++++ .../shell-local/post-processor_test.go | 53 +++++++++++++++++-- 3 files changed, 64 insertions(+), 4 deletions(-) diff --git a/common/shell-local/config.go b/common/shell-local/config.go index 76c82c793..2d31b7f01 100644 --- a/common/shell-local/config.go +++ b/common/shell-local/config.go @@ -87,7 +87,6 @@ func Validate(config *Config) error { "{{.Vars}}", "{{.Script}}", } - config.ExecuteCommand = []string{`chmod +x "{{.Script}}"; {{.Vars}} "{{.Script}}"`} } } diff --git a/post-processor/shell-local/post-processor.go b/post-processor/shell-local/post-processor.go index 91bc5acc9..557fe7eba 100644 --- a/post-processor/shell-local/post-processor.go +++ b/post-processor/shell-local/post-processor.go @@ -1,6 +1,8 @@ package shell_local import ( + "runtime" + sl "github.com/hashicorp/packer/common/shell-local" "github.com/hashicorp/packer/packer" ) @@ -19,6 +21,18 @@ func (p *PostProcessor) Configure(raws ...interface{}) error { if err != nil { return err } + if len(p.config.ExecuteCommand) == 0 && runtime.GOOS != "windows" { + // Backwards compatibility from before post-processor merge with + // provisioner. Don't need to default separately for windows becuase the + // post-processor never worked for windows before the merge with the + // provisioner code, so the provisioner defaults are fine. + p.config.ExecuteCommand = []string{"sh", "-c", `chmod +x "{{.Script}}"; {{.Vars}} "{{.Script}}"`} + } else if len(p.config.ExecuteCommand) == 1 { + // Backwards compatibility -- before merge, post-processor didn't have + // configurable call to shell program, meaning users may not have + // defined this in their call + p.config.ExecuteCommand = append([]string{"sh", "-c"}, p.config.ExecuteCommand...) + } return sl.Validate(&p.config) } diff --git a/post-processor/shell-local/post-processor_test.go b/post-processor/shell-local/post-processor_test.go index caf4f5a42..afec79f81 100644 --- a/post-processor/shell-local/post-processor_test.go +++ b/post-processor/shell-local/post-processor_test.go @@ -3,6 +3,8 @@ package shell_local import ( "io/ioutil" "os" + "runtime" + "strings" "testing" "github.com/hashicorp/packer/packer" @@ -45,8 +47,11 @@ func TestPostProcessorPrepare_InlineShebang(t *testing.T) { if err != nil { t.Fatalf("should not have error: %s", err) } - - if p.config.InlineShebang != "/bin/sh -e" { + expected := "" + if runtime.GOOS != "windows" { + expected = "/bin/sh -e" + } + if p.config.InlineShebang != expected { t.Fatalf("bad value: %s", p.config.InlineShebang) } @@ -101,6 +106,48 @@ func TestPostProcessorPrepare_Script(t *testing.T) { } } +func TestPostProcessorPrepare_ExecuteCommand(t *testing.T) { + // Check that passing a string will work (Backwards Compatibility) + p := new(PostProcessor) + raws := testConfig() + raws["execute_command"] = "foo bar" + err := p.Configure(raws) + expected := []string{"sh", "-c", "foo bar"} + if err != nil { + t.Fatalf("should handle backwards compatibility: %s", err) + } + if strings.Compare(strings.Join(p.config.ExecuteCommand, " "), strings.Join(expected, " ")) != 0 { + t.Fatalf("Did not get expected execute_command: expected: %#v; received %#v", expected, p.config.ExecuteCommand) + } + + // Check that passing a list will work + p = new(PostProcessor) + raws = testConfig() + raws["execute_command"] = []string{"foo", "bar"} + err = p.Configure(raws) + if err != nil { + t.Fatalf("should handle backwards compatibility: %s", err) + } + expected = []string{"foo", "bar"} + if strings.Compare(strings.Join(p.config.ExecuteCommand, " "), strings.Join(expected, " ")) != 0 { + t.Fatalf("Did not get expected execute_command: expected: %#v; received %#v", expected, p.config.ExecuteCommand) + } + + // Check that default is as expected + raws = testConfig() + delete(raws, "execute_command") + p = new(PostProcessor) + p.Configure(raws) + if runtime.GOOS != "windows" { + expected = []string{"sh", "-c", `chmod +x "{{.Script}}"; {{.Vars}} "{{.Script}}"`} + } else { + expected = []string{"cmd", "/C", "{{.Vars}}", "{{.Script}}"} + } + if strings.Compare(strings.Join(p.config.ExecuteCommand, " "), strings.Join(expected, " ")) != 0 { + t.Fatalf("Did not get expected default: expected: %#v; received %#v", expected, p.config.ExecuteCommand) + } +} + func TestPostProcessorPrepare_ScriptAndInline(t *testing.T) { var p PostProcessor raws := testConfig() @@ -112,7 +159,7 @@ func TestPostProcessorPrepare_ScriptAndInline(t *testing.T) { delete(raws, "scripts") err := p.Configure(raws) if err == nil { - t.Fatalf("should error when no scripts/inline commands are provided: %#v", raws) + t.Fatalf("should error when no scripts/inline commands are provided") } // Test with both From 5da4377f210d92e922210275aec809a7e2ebb2f9 Mon Sep 17 00:00:00 2001 From: Megan Marsh Date: Thu, 1 Mar 2018 10:56:30 -0800 Subject: [PATCH 11/68] first pass at docs update --- common/shell-local/communicator.go | 2 + post-processor/shell-local/post-processor.go | 6 ++- .../docs/post-processors/shell-local.html.md | 32 ++++++++--- .../docs/provisioners/shell-local.html.md | 53 +++++++++++++++++-- 4 files changed, 83 insertions(+), 10 deletions(-) diff --git a/common/shell-local/communicator.go b/common/shell-local/communicator.go index 7664bc896..b51d309d9 100644 --- a/common/shell-local/communicator.go +++ b/common/shell-local/communicator.go @@ -3,6 +3,7 @@ package shell_local import ( "fmt" "io" + "log" "os" "os/exec" "syscall" @@ -20,6 +21,7 @@ func (c *Communicator) Start(cmd *packer.RemoteCmd) error { } // Build the local command to execute + log.Printf("Executing local shell command %s", c.ExecuteCommand) localCmd := exec.Command(c.ExecuteCommand[0], c.ExecuteCommand[1:]...) localCmd.Stdin = cmd.Stdin localCmd.Stdout = cmd.Stdout diff --git a/post-processor/shell-local/post-processor.go b/post-processor/shell-local/post-processor.go index 557fe7eba..cc1f2845e 100644 --- a/post-processor/shell-local/post-processor.go +++ b/post-processor/shell-local/post-processor.go @@ -30,7 +30,11 @@ func (p *PostProcessor) Configure(raws ...interface{}) error { } else if len(p.config.ExecuteCommand) == 1 { // Backwards compatibility -- before merge, post-processor didn't have // configurable call to shell program, meaning users may not have - // defined this in their call + // defined this in their call. If users are still using the old way of + // defining ExecuteCommand (e.g. just supplying a single string that is + // now being interpolated as a slice with one item), then assume we need + // to prepend this call still, and use the one that the post-processor + // defaulted to before. p.config.ExecuteCommand = append([]string{"sh", "-c"}, p.config.ExecuteCommand...) } diff --git a/website/source/docs/post-processors/shell-local.html.md b/website/source/docs/post-processors/shell-local.html.md index d23780731..26ef7871f 100644 --- a/website/source/docs/post-processors/shell-local.html.md +++ b/website/source/docs/post-processors/shell-local.html.md @@ -13,7 +13,7 @@ Type: `shell-local` The local shell post processor executes scripts locally during the post processing stage. Shell local provides a convenient way to automate executing -some task with the packer outputs. +some task with packer outputs and variables. ## Basic example @@ -33,6 +33,9 @@ required element is either "inline" or "script". Every other option is optional. Exactly *one* of the following is required: +- `command` (string) - This is a single command to execute. It will be written + to a temporary file and run using the `execute_command` call below. + - `inline` (array of strings) - This is an array of commands to execute. The commands are concatenated by newlines and turned into a single file, so they are all executed within the same context. This allows you to change @@ -52,15 +55,32 @@ Exactly *one* of the following is required: Optional parameters: - `environment_vars` (array of strings) - An array of key/value pairs to - inject prior to the execute\_command. The format should be `key=value`. + inject prior to the `execute_command`. The format should be `key=value`. Packer injects some environmental variables by default into the environment, as well, which are covered in the section below. -- `execute_command` (string) - The command to use to execute the script. By - default this is `chmod +x "{{.Script}}"; {{.Vars}} "{{.Script}}"`. - The value of this is treated as [template engine](/docs/templates/engine.html). +- `execute_command` (array of strings) - The command used to execute the script. By + default this is `["sh", "-c", "chmod +x \"{{.Script}}\"; {{.Vars}} \"{{.Script}}\""]` + on unix and `["cmd", "/c", "{{.Vars}}", "{{.Script}}"]` on windows. + This is treated as a [template engine](/docs/templates/engine.html). There are two available variables: `Script`, which is the path to the script - to run, `Vars`, which is the list of `environment_vars`, if configured. + to run, and `Vars`, which is the list of `environment_vars`, if configured. + If you choose to set this option, make sure that the first element in the + array is the shell program you want to use (for example, "sh" or + "/usr/local/bin/zsh" or even "powershell.exe" although anything other than + a flavor of the shell command language is not explicitly supported and may + be broken by assumptions made within Packer). + + For backwards compatibility, `execute_command` will accept a string insetad + of an array of strings. If a single string or an array of strings with only + one element is provided, Packer will replicate past behavior by appending + your `execute_command` to the array of strings `["sh", "-c"]`. For example, + if you set `"execute_command": "foo bar"`, the final `execute_command` that + Packer runs will be ["sh", "-c", "foo bar"]. If you set `"execute_command": ["foo", "bar"]`, + the final execute_command will remain `["foo", "bar"]`. + + Again, the above is only provided as a backwards compatibility fix; we + strongly recommend that you set execute_command as an array of strings. - `inline_shebang` (string) - The [shebang](http://en.wikipedia.org/wiki/Shebang_%28Unix%29) value to use when diff --git a/website/source/docs/provisioners/shell-local.html.md b/website/source/docs/provisioners/shell-local.html.md index c52bcd893..bd66e74c7 100644 --- a/website/source/docs/provisioners/shell-local.html.md +++ b/website/source/docs/provisioners/shell-local.html.md @@ -37,10 +37,26 @@ The example below is fully functional. The reference of available configuration options is listed below. The only required element is "command". -Required: +Exactly *one* of the following is required: -- `command` (string) - The command to execute. This will be executed within - the context of a shell as specified by `execute_command`. +- `command` (string) - This is a single command to execute. It will be written + to a temporary file and run using the `execute_command` call below. + +- `inline` (array of strings) - This is an array of commands to execute. The + commands are concatenated by newlines and turned into a single file, so they + are all executed within the same context. This allows you to change + directories in one command and use something in the directory in the next + and so on. Inline scripts are the easiest way to pull off simple tasks + within the machine. + +- `script` (string) - The path to a script to execute. This path can be + absolute or relative. If it is relative, it is relative to the working + directory when Packer is executed. + +- `scripts` (array of strings) - An array of scripts to execute. The scripts + will be executed in the order specified. Each script is executed in + isolation, so state such as variables from one script won't carry on to the + next. Optional parameters: @@ -50,3 +66,34 @@ Optional parameters: treated as [configuration template](/docs/templates/engine.html). The only available variable is `Command` which is the command to execute. + +- `environment_vars` (array of strings) - An array of key/value pairs to + inject prior to the `execute_command`. The format should be `key=value`. + Packer injects some environmental variables by default into the environment, + as well, which are covered in the section below. + +- `execute_command` (array of strings) - The command used to execute the script. + By default this is `["/bin/sh", "-c", "{{.Vars}}, "{{.Script}}"]` + on unix and `["cmd", "/c", "{{.Vars}}", "{{.Script}}"]` on windows. + This is treated as a [template engine](/docs/templates/engine.html). + There are two available variables: `Script`, which is the path to the script + to run, and `Vars`, which is the list of `environment_vars`, if configured + If you choose to set this option, make sure that the first element in the + array is the shell program you want to use (for example, "sh" or + "/usr/local/bin/zsh" or even "powershell.exe" although anything other than + a flavor of the shell command language is not explicitly supported and may + be broken by assumptions made within Packer), and a later element in the + array must be `{{.Script}}`. + + For backwards compatability, {{.Command}} is also available to use in + `execute_command` but it is decoded the same way as {{.Script}}. We + recommend using {{.Script}} for the sake of clarity, as even when you set + only a single `command` to run, Packer writes it to a temporary file and + then runs it as a script. + +- `inline_shebang` (string) - The + [shebang](http://en.wikipedia.org/wiki/Shebang_%28Unix%29) value to use when + running commands specified by `inline`. By default, this is `/bin/sh -e`. If + you're not using `inline`, then this configuration has no effect. + **Important:** If you customize this, be sure to include something like the + `-e` flag, otherwise individual steps failing won't fail the provisioner. From e983a94a88251a32d6426269d321117d58e33266 Mon Sep 17 00:00:00 2001 From: Megan Marsh Date: Fri, 2 Mar 2018 12:32:34 -0800 Subject: [PATCH 12/68] fix default windows bash call for shell-local provisioner and move chmod command from the execute_command array into the portion of code where we actually generate inline scripts, sparing users the need to think about this modification which Packer should really handle on its own make bash call work on windows --- common/shell-local/config.go | 8 +++- common/shell-local/run.go | 6 ++- post-processor/shell-local/post-processor.go | 26 +++++------- provisioner/shell-local/provisioner.go | 43 +++++++++++++++++++- 4 files changed, 62 insertions(+), 21 deletions(-) diff --git a/common/shell-local/config.go b/common/shell-local/config.go index 2d31b7f01..8c73d0f57 100644 --- a/common/shell-local/config.go +++ b/common/shell-local/config.go @@ -67,6 +67,11 @@ func Decode(config *Config, raws ...interface{}) error { func Validate(config *Config) error { var errs *packer.MultiError + // Do not treat these defaults as a source of truth; the shell-local + // provisioner sets these defaults before Validate is called. Eventually + // we will have to bring the provisioner and post-processor defaults in + // line with one another, but for now the following may or may not be + // applied depending on where Validate is being called from. if runtime.GOOS == "windows" { if len(config.ExecuteCommand) == 0 { config.ExecuteCommand = []string{ @@ -84,8 +89,7 @@ func Validate(config *Config) error { config.ExecuteCommand = []string{ "/bin/sh", "-c", - "{{.Vars}}", - "{{.Script}}", + "{{.Vars}} {{.Script}}", } } } diff --git a/common/shell-local/run.go b/common/shell-local/run.go index 04d653389..5545d56d7 100644 --- a/common/shell-local/run.go +++ b/common/shell-local/run.go @@ -41,7 +41,7 @@ func Run(ui packer.Ui, config *Config) (bool, error) { if err != nil { return false, err } - ui.Say(fmt.Sprintf("Post processing with local shell script: %s", script)) + ui.Say(fmt.Sprintf("Running local shell script: %s", script)) comm := &Communicator{ ExecuteCommand: interpolatedCmds, @@ -93,6 +93,10 @@ func createInlineScriptFile(config *Config) (string, error) { } tf.Close() + err = os.Chmod(tf.Name(), 0555) + if err != nil { + log.Printf("error modifying permissions of temp script file: %s", err.Error()) + } return tf.Name(), nil } diff --git a/post-processor/shell-local/post-processor.go b/post-processor/shell-local/post-processor.go index cc1f2845e..c761a19f4 100644 --- a/post-processor/shell-local/post-processor.go +++ b/post-processor/shell-local/post-processor.go @@ -1,8 +1,6 @@ package shell_local import ( - "runtime" - sl "github.com/hashicorp/packer/common/shell-local" "github.com/hashicorp/packer/packer" ) @@ -21,20 +19,16 @@ func (p *PostProcessor) Configure(raws ...interface{}) error { if err != nil { return err } - if len(p.config.ExecuteCommand) == 0 && runtime.GOOS != "windows" { - // Backwards compatibility from before post-processor merge with - // provisioner. Don't need to default separately for windows becuase the - // post-processor never worked for windows before the merge with the - // provisioner code, so the provisioner defaults are fine. - p.config.ExecuteCommand = []string{"sh", "-c", `chmod +x "{{.Script}}"; {{.Vars}} "{{.Script}}"`} - } else if len(p.config.ExecuteCommand) == 1 { - // Backwards compatibility -- before merge, post-processor didn't have - // configurable call to shell program, meaning users may not have - // defined this in their call. If users are still using the old way of - // defining ExecuteCommand (e.g. just supplying a single string that is - // now being interpolated as a slice with one item), then assume we need - // to prepend this call still, and use the one that the post-processor - // defaulted to before. + if len(p.config.ExecuteCommand) == 1 { + // Backwards compatibility -- before we merged the shell-local + // post-processor and provisioners, the post-processor accepted + // execute_command as a string rather than a slice of strings. It didn't + // have a configurable call to shell program, automatically prepending + // the user-supplied execute_command string with "sh -c". If users are + // still using the old way of defining ExecuteCommand (by supplying a + // single string rather than a slice of strings) then we need to + // prepend this command with the call that the post-processor defaulted + // to before. p.config.ExecuteCommand = append([]string{"sh", "-c"}, p.config.ExecuteCommand...) } diff --git a/provisioner/shell-local/provisioner.go b/provisioner/shell-local/provisioner.go index a56553245..a58f0c859 100644 --- a/provisioner/shell-local/provisioner.go +++ b/provisioner/shell-local/provisioner.go @@ -1,6 +1,11 @@ package shell import ( + "fmt" + "path/filepath" + "runtime" + "strings" + sl "github.com/hashicorp/packer/common/shell-local" "github.com/hashicorp/packer/packer" ) @@ -10,12 +15,46 @@ type Provisioner struct { } func (p *Provisioner) Prepare(raws ...interface{}) error { - err := sl.Decode(&p.config, raws) + err := sl.Decode(&p.config, raws...) + if err != nil { + return err + } + convertPath := false + if len(p.config.ExecuteCommand) == 0 && runtime.GOOS == "windows" { + convertPath = true + p.config.ExecuteCommand = []string{ + "bash", + "-c", + "{{.Vars}} {{.Script}}", + } + } + + err = sl.Validate(&p.config) if err != nil { return err } - return sl.Validate(&p.config) + if convertPath { + for index, script := range p.config.Scripts { + p.config.Scripts[index], err = convertToWindowsBashPath(script) + if err != nil { + return err + } + } + } + + return nil +} + +func convertToWindowsBashPath(winPath string) (string, error) { + // get absolute path of script, and morph it into the bash path + winAbsPath, err := filepath.Abs(winPath) + if err != nil { + return "", fmt.Errorf("Error converting %s to absolute path: %s", winPath, err.Error()) + } + winAbsPath = strings.Replace(winAbsPath, "\\", "/", -1) + winBashPath := strings.Replace(winAbsPath, "C:/", "/mnt/c/", 1) + return winBashPath, nil } func (p *Provisioner) Provision(ui packer.Ui, _ packer.Communicator) error { From 51bcc7aa136ccb7331435333cf162d0c5d007feb Mon Sep 17 00:00:00 2001 From: Megan Marsh Date: Thu, 8 Mar 2018 16:42:17 -0800 Subject: [PATCH 13/68] add new feature for telling shell-local whether to use linux pathing on windows; update docs with some examples. --- common/shell-local/config.go | 33 ++++++++++++--- common/shell-local/run.go | 10 +++++ provisioner/shell-local/provisioner.go | 34 --------------- .../docs/post-processors/shell-local.html.md | 41 +++++++++++++++++-- 4 files changed, 75 insertions(+), 43 deletions(-) diff --git a/common/shell-local/config.go b/common/shell-local/config.go index 8c73d0f57..80751aee7 100644 --- a/common/shell-local/config.go +++ b/common/shell-local/config.go @@ -4,6 +4,7 @@ import ( "errors" "fmt" "os" + "path/filepath" "runtime" "strings" @@ -44,6 +45,8 @@ type Config struct { // can be used to inject the environment_vars into the environment. ExecuteCommand []string `mapstructure:"execute_command"` + UseLinuxPathing bool `mapstructure:"use_linux_pathing"` + Ctx interpolate.Context } @@ -67,11 +70,6 @@ func Decode(config *Config, raws ...interface{}) error { func Validate(config *Config) error { var errs *packer.MultiError - // Do not treat these defaults as a source of truth; the shell-local - // provisioner sets these defaults before Validate is called. Eventually - // we will have to bring the provisioner and post-processor defaults in - // line with one another, but for now the following may or may not be - // applied depending on where Validate is being called from. if runtime.GOOS == "windows" { if len(config.ExecuteCommand) == 0 { config.ExecuteCommand = []string{ @@ -89,7 +87,8 @@ func Validate(config *Config) error { config.ExecuteCommand = []string{ "/bin/sh", "-c", - "{{.Vars}} {{.Script}}", + "{{.Vars}}", + "{{.Script}}", } } } @@ -146,6 +145,15 @@ func Validate(config *Config) error { fmt.Errorf("Bad script '%s': %s", path, err)) } } + if config.UseLinuxPathing { + for index, script := range config.Scripts { + converted, err := convertToLinuxPath(script) + if err != nil { + return err + } + config.Scripts[index] = converted + } + } // Do a check for bad environment variables, such as '=foo', 'foobar' for _, kv := range config.Vars { @@ -162,3 +170,16 @@ func Validate(config *Config) error { return nil } + +// C:/path/to/your/file becomes /mnt/c/path/to/your/file +func convertToLinuxPath(winPath string) (string, error) { + // get absolute path of script, and morph it into the bash path + winAbsPath, err := filepath.Abs(winPath) + if err != nil { + return "", fmt.Errorf("Error converting %s to absolute path: %s", winPath, err.Error()) + } + winAbsPath = strings.Replace(winAbsPath, "\\", "/", -1) + splitPath := strings.SplitN(winAbsPath, ":/", 2) + winBashPath := fmt.Sprintf("/mnt/%s/%s", strings.ToLower(splitPath[0]), splitPath[1]) + return winBashPath, nil +} diff --git a/common/shell-local/run.go b/common/shell-local/run.go index 5545d56d7..ea8043737 100644 --- a/common/shell-local/run.go +++ b/common/shell-local/run.go @@ -6,6 +6,7 @@ import ( "io/ioutil" "log" "os" + "runtime" "sort" "strings" @@ -144,8 +145,17 @@ func createFlattenedEnvVars(config *Config) (flattened string) { sort.Strings(keys) // Re-assemble vars surrounding value with single quotes and flatten + if runtime.GOOS == "windows" { + log.Printf("MEGAN NEED TO IMPLEMENT") + // createEnvVarsSourceFileWindows() + } for _, key := range keys { flattened += fmt.Sprintf("%s='%s' ", key, envVars[key]) } return } + +// func createFlattenedEnvVarsWindows( +// // The default shell, cmd, can set vars via dot sourcing +// // set TESTXYZ=XYZ +// ) diff --git a/provisioner/shell-local/provisioner.go b/provisioner/shell-local/provisioner.go index a58f0c859..16c3806e4 100644 --- a/provisioner/shell-local/provisioner.go +++ b/provisioner/shell-local/provisioner.go @@ -1,11 +1,6 @@ package shell import ( - "fmt" - "path/filepath" - "runtime" - "strings" - sl "github.com/hashicorp/packer/common/shell-local" "github.com/hashicorp/packer/packer" ) @@ -19,44 +14,15 @@ func (p *Provisioner) Prepare(raws ...interface{}) error { if err != nil { return err } - convertPath := false - if len(p.config.ExecuteCommand) == 0 && runtime.GOOS == "windows" { - convertPath = true - p.config.ExecuteCommand = []string{ - "bash", - "-c", - "{{.Vars}} {{.Script}}", - } - } err = sl.Validate(&p.config) if err != nil { return err } - if convertPath { - for index, script := range p.config.Scripts { - p.config.Scripts[index], err = convertToWindowsBashPath(script) - if err != nil { - return err - } - } - } - return nil } -func convertToWindowsBashPath(winPath string) (string, error) { - // get absolute path of script, and morph it into the bash path - winAbsPath, err := filepath.Abs(winPath) - if err != nil { - return "", fmt.Errorf("Error converting %s to absolute path: %s", winPath, err.Error()) - } - winAbsPath = strings.Replace(winAbsPath, "\\", "/", -1) - winBashPath := strings.Replace(winAbsPath, "C:/", "/mnt/c/", 1) - return winBashPath, nil -} - func (p *Provisioner) Provision(ui packer.Ui, _ packer.Communicator) error { _, retErr := sl.Run(ui, &p.config) if retErr != nil { diff --git a/website/source/docs/post-processors/shell-local.html.md b/website/source/docs/post-processors/shell-local.html.md index 26ef7871f..e2bc324da 100644 --- a/website/source/docs/post-processors/shell-local.html.md +++ b/website/source/docs/post-processors/shell-local.html.md @@ -60,7 +60,7 @@ Optional parameters: as well, which are covered in the section below. - `execute_command` (array of strings) - The command used to execute the script. By - default this is `["sh", "-c", "chmod +x \"{{.Script}}\"; {{.Vars}} \"{{.Script}}\""]` + default this is `["/bin/sh", "-c", "{{.Vars}}, "{{.Script}}"]` on unix and `["cmd", "/c", "{{.Vars}}", "{{.Script}}"]` on windows. This is treated as a [template engine](/docs/templates/engine.html). There are two available variables: `Script`, which is the path to the script @@ -69,7 +69,9 @@ Optional parameters: array is the shell program you want to use (for example, "sh" or "/usr/local/bin/zsh" or even "powershell.exe" although anything other than a flavor of the shell command language is not explicitly supported and may - be broken by assumptions made within Packer). + be broken by assumptions made within Packer). It's worth noting that if you + choose to try to use shell-local for Powershell or other Windows commands, + the environment variables will not be set properly for your environment. For backwards compatibility, `execute_command` will accept a string insetad of an array of strings. If a single string or an array of strings with only @@ -89,13 +91,46 @@ Optional parameters: **Important:** If you customize this, be sure to include something like the `-e` flag, otherwise individual steps failing won't fail the provisioner. -## Execute Command Example +- `use_linux_pathing` (bool) - This is only relevant to windows hosts. If you + are running Packer in a Windows environment with the Windows Subsystem for + Linux feature enabled, and would like to invoke a bash script rather than + invoking a Cmd script, you'll need to set this flag to true; it tells Packer + to use the linux subsystem path for your script rather than the Windows path. + (e.g. /mnt/c/path/to/your/file instead of C:/path/to/your/file). + +## Execute Command To many new users, the `execute_command` is puzzling. However, it provides an important function: customization of how the command is executed. The most common use case for this is dealing with **sudo password prompts**. You may also need to customize this if you use a non-POSIX shell, such as `tcsh` on FreeBSD. +### The Windows Linux Subsystem + +If you have a bash script that you'd like to run on your Windows Linux +Subsystem as part of the shell-local post-processor, you must set +`execute_command` and `use_linux_pathing`. + +The example below is a fully functional test config. + +``` +{ + "builders": [ + { + "type": "null", + "communicator": "none" + } + ], + "provisioners": [ + { + "type": "shell-local", + "environment_vars": ["PROVISIONERTEST=ProvisionerTest1"], + "execute_command": ["bash", "-c", "{{.Vars}} {{.Script}}"] + "use_linux_pathing": true + "scripts": ["./scripts/.sh"] + }, +``` + ## Default Environmental Variables In addition to being able to specify custom environmental variables using the From dd183f22d9c78524884de923eaf791e5ca3c7eed Mon Sep 17 00:00:00 2001 From: Megan Marsh Date: Fri, 9 Mar 2018 15:14:52 -0800 Subject: [PATCH 14/68] update docs and add warnings around WSL limitations --- common/shell-local/config.go | 12 ++- .../docs/post-processors/shell-local.html.md | 36 +++++-- .../docs/provisioners/shell-local.html.md | 97 +++++++++++++++++++ 3 files changed, 136 insertions(+), 9 deletions(-) diff --git a/common/shell-local/config.go b/common/shell-local/config.go index 80751aee7..64dfe25c1 100644 --- a/common/shell-local/config.go +++ b/common/shell-local/config.go @@ -147,12 +147,20 @@ func Validate(config *Config) error { } if config.UseLinuxPathing { for index, script := range config.Scripts { - converted, err := convertToLinuxPath(script) + converted, err := ConvertToLinuxPath(script) if err != nil { return err } config.Scripts[index] = converted } + // Interoperability issues with WSL makes creating and running tempfiles + // via golang's os package basically impossible. + if len(config.Inline) > 0 { + errs = packer.MultiErrorAppend(errs, + fmt.Errorf("Packer is unable to use the Command and Inline "+ + "features with the Windows Linux Subsystem. Please use "+ + "the Script or Scripts options instead")) + } } // Do a check for bad environment variables, such as '=foo', 'foobar' @@ -172,7 +180,7 @@ func Validate(config *Config) error { } // C:/path/to/your/file becomes /mnt/c/path/to/your/file -func convertToLinuxPath(winPath string) (string, error) { +func ConvertToLinuxPath(winPath string) (string, error) { // get absolute path of script, and morph it into the bash path winAbsPath, err := filepath.Abs(winPath) if err != nil { diff --git a/website/source/docs/post-processors/shell-local.html.md b/website/source/docs/post-processors/shell-local.html.md index e2bc324da..ab2f4bba3 100644 --- a/website/source/docs/post-processors/shell-local.html.md +++ b/website/source/docs/post-processors/shell-local.html.md @@ -96,7 +96,10 @@ Optional parameters: Linux feature enabled, and would like to invoke a bash script rather than invoking a Cmd script, you'll need to set this flag to true; it tells Packer to use the linux subsystem path for your script rather than the Windows path. - (e.g. /mnt/c/path/to/your/file instead of C:/path/to/your/file). + (e.g. /mnt/c/path/to/your/file instead of C:/path/to/your/file). Please see + the example below for more guidance on how to use this feature. If you are + not on a Windows host, or you do not intend to use the shell-local + post-processor to run a bash script, please ignore this option. ## Execute Command @@ -107,12 +110,22 @@ need to customize this if you use a non-POSIX shell, such as `tcsh` on FreeBSD. ### The Windows Linux Subsystem -If you have a bash script that you'd like to run on your Windows Linux -Subsystem as part of the shell-local post-processor, you must set -`execute_command` and `use_linux_pathing`. +The shell-local post-processor was designed with the idea of allowing you to run +commands in your local operating system's native shell. For Windows, we've +assumed in our defaults that this is Cmd. However, it is possible to run a +bash script as part of the Windows Linux Subsystem from the shell-local +post-processor, by modifying the `execute_command` and the `use_linux_pathing` +options in the post-processor config. The example below is a fully functional test config. +One limitation of this offering is that "inline" and "command" options are not +available to you; please limit yourself to using the "script" or "scripts" +options instead. + +Please note that the WSL is a beta feature, and this tool is not guaranteed to +work as you expect it to. + ``` { "builders": [ @@ -125,10 +138,19 @@ The example below is a fully functional test config. { "type": "shell-local", "environment_vars": ["PROVISIONERTEST=ProvisionerTest1"], - "execute_command": ["bash", "-c", "{{.Vars}} {{.Script}}"] - "use_linux_pathing": true - "scripts": ["./scripts/.sh"] + "execute_command": ["bash", "-c", "{{.Vars}} {{.Script}}"], + "use_linux_pathing": true, + "scripts": ["C:/Users/me/scripts/example_bash.sh"] }, + { + "type": "shell-local", + "environment_vars": ["PROVISIONERTEST=ProvisionerTest2"], + "execute_command": ["bash", "-c", "{{.Vars}} {{.Script}}"], + "use_linux_pathing": true, + "script": "C:/Users/me/scripts/example_bash.sh" + } + ] +} ``` ## Default Environmental Variables diff --git a/website/source/docs/provisioners/shell-local.html.md b/website/source/docs/provisioners/shell-local.html.md index bd66e74c7..f40c4e9f0 100644 --- a/website/source/docs/provisioners/shell-local.html.md +++ b/website/source/docs/provisioners/shell-local.html.md @@ -97,3 +97,100 @@ Optional parameters: you're not using `inline`, then this configuration has no effect. **Important:** If you customize this, be sure to include something like the `-e` flag, otherwise individual steps failing won't fail the provisioner. + +- `use_linux_pathing` (bool) - This is only relevant to windows hosts. If you + are running Packer in a Windows environment with the Windows Subsystem for + Linux feature enabled, and would like to invoke a bash script rather than + invoking a Cmd script, you'll need to set this flag to true; it tells Packer + to use the linux subsystem path for your script rather than the Windows path. + (e.g. /mnt/c/path/to/your/file instead of C:/path/to/your/file). Please see + the example below for more guidance on how to use this feature. If you are + not on a Windows host, or you do not intend to use the shell-local + provisioner to run a bash script, please ignore this option. + +## Execute Command + +To many new users, the `execute_command` is puzzling. However, it provides an +important function: customization of how the command is executed. The most +common use case for this is dealing with **sudo password prompts**. You may also +need to customize this if you use a non-POSIX shell, such as `tcsh` on FreeBSD. + +### The Windows Linux Subsystem + +The shell-local provisioner was designed with the idea of allowing you to run +commands in your local operating system's native shell. For Windows, we've +assumed in our defaults that this is Cmd. However, it is possible to run a +bash script as part of the Windows Linux Subsystem from the shell-local +provisioner, by modifying the `execute_command` and the `use_linux_pathing` +options in the provisioner config. + +The example below is a fully functional test config. + +One limitation of this offering is that "inline" and "command" options are not +available to you; please limit yourself to using the "script" or "scripts" +options instead. + +Please note that the WSL is a beta feature, and this tool is not guaranteed to +work as you expect it to. + +``` +{ + "builders": [ + { + "type": "null", + "communicator": "none" + } + ], + "provisioners": [ + { + "type": "shell-local", + "environment_vars": ["PROVISIONERTEST=ProvisionerTest1"], + "execute_command": ["bash", "-c", "{{.Vars}} {{.Script}}"], + "use_linux_pathing": true, + "scripts": ["C:/Users/me/scripts/example_bash.sh"] + }, + { + "type": "shell-local", + "environment_vars": ["PROVISIONERTEST=ProvisionerTest2"], + "execute_command": ["bash", "-c", "{{.Vars}} {{.Script}}"], + "use_linux_pathing": true, + "script": "C:/Users/me/scripts/example_bash.sh" + } + ] +} +``` + +## Default Environmental Variables + +In addition to being able to specify custom environmental variables using the +`environment_vars` configuration, the provisioner automatically defines certain +commonly useful environmental variables: + +- `PACKER_BUILD_NAME` is set to the name of the build that Packer is running. + This is most useful when Packer is making multiple builds and you want to + distinguish them slightly from a common provisioning script. + +- `PACKER_BUILDER_TYPE` is the type of the builder that was used to create the + machine that the script is running on. This is useful if you want to run + only certain parts of the script on systems built with certain builders. + +## Safely Writing A Script + +Whether you use the `inline` option, or pass it a direct `script` or `scripts`, +it is important to understand a few things about how the shell-local +provisioner works to run it safely and easily. This understanding will save +you much time in the process. + +### Once Per Builder + +The `shell-local` script(s) you pass are run once per builder. That means that +if you have an `amazon-ebs` builder and a `docker` builder, your script will be +run twice. If you have 3 builders, it will run 3 times, once for each builder. + +### Always Exit Intentionally + +If any provisioner fails, the `packer build` stops and all interim artifacts +are cleaned up. + +For a shell script, that means the script **must** exit with a zero code. You +*must* be extra careful to `exit 0` when necessary. From 9651432378952cecf47e9b24647021809cb161bc Mon Sep 17 00:00:00 2001 From: Megan Marsh Date: Fri, 9 Mar 2018 15:36:11 -0800 Subject: [PATCH 15/68] preserver BC for people using 'command' option --- common/shell-local/run.go | 10 +++++--- .../docs/provisioners/shell-local.html.md | 25 +++++++++++-------- 2 files changed, 20 insertions(+), 15 deletions(-) diff --git a/common/shell-local/run.go b/common/shell-local/run.go index ea8043737..a16573e7a 100644 --- a/common/shell-local/run.go +++ b/common/shell-local/run.go @@ -15,8 +15,9 @@ import ( ) type ExecuteCommandTemplate struct { - Vars string - Script string + Vars string + Script string + Command string } func Run(ui packer.Ui, config *Config) (bool, error) { @@ -106,8 +107,9 @@ func createInlineScriptFile(config *Config) (string, error) { // the host OS func createInterpolatedCommands(config *Config, script string, flattenedEnvVars string) ([]string, error) { config.Ctx.Data = &ExecuteCommandTemplate{ - Vars: flattenedEnvVars, - Script: script, + Vars: flattenedEnvVars, + Script: script, + Command: script, } interpolatedCmds := make([]string, len(config.ExecuteCommand)) diff --git a/website/source/docs/provisioners/shell-local.html.md b/website/source/docs/provisioners/shell-local.html.md index f40c4e9f0..eae9cf2ff 100644 --- a/website/source/docs/provisioners/shell-local.html.md +++ b/website/source/docs/provisioners/shell-local.html.md @@ -78,18 +78,21 @@ Optional parameters: This is treated as a [template engine](/docs/templates/engine.html). There are two available variables: `Script`, which is the path to the script to run, and `Vars`, which is the list of `environment_vars`, if configured - If you choose to set this option, make sure that the first element in the - array is the shell program you want to use (for example, "sh" or - "/usr/local/bin/zsh" or even "powershell.exe" although anything other than - a flavor of the shell command language is not explicitly supported and may - be broken by assumptions made within Packer), and a later element in the - array must be `{{.Script}}`. - For backwards compatability, {{.Command}} is also available to use in - `execute_command` but it is decoded the same way as {{.Script}}. We - recommend using {{.Script}} for the sake of clarity, as even when you set - only a single `command` to run, Packer writes it to a temporary file and - then runs it as a script. + If you choose to set this option, make sure that the first element in the + array is the shell program you want to use (for example, "sh"), and a later + element in the array must be `{{.Script}}`. + + This option provides you a great deal of flexibility. You may choose to + provide your own shell program, for example "/usr/local/bin/zsh" or even + "powershell.exe". However, with great power comes great responsibility - + these commands are not officially supported and things like environment + variables may not work if you use a different shell than the default. + + For backwards compatability, you may also use {{.Command}}, but it is + decoded the same way as {{.Script}}. We recommend using {{.Script}} for the + sake of clarity, as even when you set only a single `command` to run, + Packer writes it to a temporary file and then runs it as a script. - `inline_shebang` (string) - The [shebang](http://en.wikipedia.org/wiki/Shebang_%28Unix%29) value to use when From fabd1a651771c099aec387f0f8b1fa9c5aec0ba1 Mon Sep 17 00:00:00 2001 From: Megan Marsh Date: Mon, 12 Mar 2018 11:25:39 -0700 Subject: [PATCH 16/68] windows cmd env vars --- common/shell-local/config.go | 11 +++++++++ common/shell-local/run.go | 24 +++++++------------ .../shell-local/post-processor_test.go | 2 +- 3 files changed, 20 insertions(+), 17 deletions(-) diff --git a/common/shell-local/config.go b/common/shell-local/config.go index 64dfe25c1..f4514a014 100644 --- a/common/shell-local/config.go +++ b/common/shell-local/config.go @@ -38,6 +38,8 @@ type Config struct { // An array of environment variables that will be injected before // your command(s) are executed. Vars []string `mapstructure:"environment_vars"` + + EnvVarFormat string // End dedupe with postprocessor // The command used to execute the script. The '{{ .Path }}' variable @@ -162,6 +164,15 @@ func Validate(config *Config) error { "the Script or Scripts options instead")) } } + // This is currently undocumented and not a feature users are expected to + // interact with. + if config.EnvVarFormat == "" { + if (runtime.GOOS == "windows") && !config.UseLinuxPathing { + config.EnvVarFormat = `set "%s=%s" && ` + } else { + config.EnvVarFormat = "%s='%s' " + } + } // Do a check for bad environment variables, such as '=foo', 'foobar' for _, kv := range config.Vars { diff --git a/common/shell-local/run.go b/common/shell-local/run.go index a16573e7a..9e17d2d87 100644 --- a/common/shell-local/run.go +++ b/common/shell-local/run.go @@ -6,7 +6,6 @@ import ( "io/ioutil" "log" "os" - "runtime" "sort" "strings" @@ -36,7 +35,10 @@ func Run(ui packer.Ui, config *Config) (bool, error) { } // Create environment variables to set before executing the command - flattenedEnvVars := createFlattenedEnvVars(config) + flattenedEnvVars, err := createFlattenedEnvVars(config) + if err != nil { + return false, err + } for _, script := range scripts { interpolatedCmds, err := createInterpolatedCommands(config, script, flattenedEnvVars) @@ -123,8 +125,8 @@ func createInterpolatedCommands(config *Config, script string, flattenedEnvVars return interpolatedCmds, nil } -func createFlattenedEnvVars(config *Config) (flattened string) { - flattened = "" +func createFlattenedEnvVars(config *Config) (string, error) { + flattened := "" envVars := make(map[string]string) // Always available Packer provided env vars @@ -146,18 +148,8 @@ func createFlattenedEnvVars(config *Config) (flattened string) { } sort.Strings(keys) - // Re-assemble vars surrounding value with single quotes and flatten - if runtime.GOOS == "windows" { - log.Printf("MEGAN NEED TO IMPLEMENT") - // createEnvVarsSourceFileWindows() - } for _, key := range keys { - flattened += fmt.Sprintf("%s='%s' ", key, envVars[key]) + flattened += fmt.Sprintf(config.EnvVarFormat, key, envVars[key]) } - return + return flattened, nil } - -// func createFlattenedEnvVarsWindows( -// // The default shell, cmd, can set vars via dot sourcing -// // set TESTXYZ=XYZ -// ) diff --git a/post-processor/shell-local/post-processor_test.go b/post-processor/shell-local/post-processor_test.go index afec79f81..5fabac124 100644 --- a/post-processor/shell-local/post-processor_test.go +++ b/post-processor/shell-local/post-processor_test.go @@ -139,7 +139,7 @@ func TestPostProcessorPrepare_ExecuteCommand(t *testing.T) { p = new(PostProcessor) p.Configure(raws) if runtime.GOOS != "windows" { - expected = []string{"sh", "-c", `chmod +x "{{.Script}}"; {{.Vars}} "{{.Script}}"`} + expected = []string{"/bin/sh", "-c", "{{.Vars}}", "{{.Script}}"} } else { expected = []string{"cmd", "/C", "{{.Vars}}", "{{.Script}}"} } From 1bea658e16cb5a45ea043b78276e81f2c4ec62db Mon Sep 17 00:00:00 2001 From: Megan Marsh Date: Wed, 4 Apr 2018 11:07:10 -0700 Subject: [PATCH 17/68] fix command and inline calls on windows --- common/shell-local/config.go | 19 +++- common/shell-local/run.go | 17 ++- .../docs/post-processors/shell-local.html.md | 102 ++++++++++++++++++ .../docs/provisioners/shell-local.html.md | 101 +++++++++++++++++ 4 files changed, 231 insertions(+), 8 deletions(-) diff --git a/common/shell-local/config.go b/common/shell-local/config.go index f4514a014..846e4b4a4 100644 --- a/common/shell-local/config.go +++ b/common/shell-local/config.go @@ -29,6 +29,9 @@ type Config struct { // The shebang value used when running inline scripts. InlineShebang string `mapstructure:"inline_shebang"` + // The file extension to use for the file generated from the inline commands + TempfileExtension string `mapstructure:"tempfile_extension"` + // The local path of the shell script to upload and execute. Script string @@ -39,7 +42,7 @@ type Config struct { // your command(s) are executed. Vars []string `mapstructure:"environment_vars"` - EnvVarFormat string + EnvVarFormat string `mapstructure:"env_var_format"` // End dedupe with postprocessor // The command used to execute the script. The '{{ .Path }}' variable @@ -76,8 +79,10 @@ func Validate(config *Config) error { if len(config.ExecuteCommand) == 0 { config.ExecuteCommand = []string{ "cmd", + "/V", "/C", "{{.Vars}}", + "call", "{{.Script}}", } } @@ -89,8 +94,7 @@ func Validate(config *Config) error { config.ExecuteCommand = []string{ "/bin/sh", "-c", - "{{.Vars}}", - "{{.Script}}", + "{{.Vars}} {{.Script}}", } } } @@ -168,12 +172,19 @@ func Validate(config *Config) error { // interact with. if config.EnvVarFormat == "" { if (runtime.GOOS == "windows") && !config.UseLinuxPathing { - config.EnvVarFormat = `set "%s=%s" && ` + config.EnvVarFormat = "set %s=%s && " } else { config.EnvVarFormat = "%s='%s' " } } + // drop unnecessary "." in extension; we add this later. + if config.TempfileExtension != "" { + if strings.HasPrefix(config.TempfileExtension, ".") { + config.TempfileExtension = config.TempfileExtension[1:] + } + } + // Do a check for bad environment variables, such as '=foo', 'foobar' for _, kv := range config.Vars { vs := strings.SplitN(kv, "=", 2) diff --git a/common/shell-local/run.go b/common/shell-local/run.go index 9e17d2d87..6af406522 100644 --- a/common/shell-local/run.go +++ b/common/shell-local/run.go @@ -30,8 +30,13 @@ func Run(ui packer.Ui, config *Config) (bool, error) { if err != nil { return false, err } - defer os.Remove(tempScriptFileName) scripts = append(scripts, tempScriptFileName) + + defer os.Remove(tempScriptFileName) + // figure out what extension the file should have, and rename it. + if config.TempfileExtension != "" { + os.Rename(tempScriptFileName, fmt.Sprintf("%s.%s", tempScriptFileName, config.TempfileExtension)) + } } // Create environment variables to set before executing the command @@ -78,14 +83,18 @@ func Run(ui packer.Ui, config *Config) (bool, error) { } func createInlineScriptFile(config *Config) (string, error) { - tf, err := ioutil.TempFile("", "packer-shell") + tf, err := ioutil.TempFile(os.TempDir(), "packer-shell") if err != nil { return "", fmt.Errorf("Error preparing shell script: %s", err) } - + defer tf.Close() // Write our contents to it writer := bufio.NewWriter(tf) - writer.WriteString(fmt.Sprintf("#!%s\n", config.InlineShebang)) + if config.InlineShebang != "" { + shebang := fmt.Sprintf("#!%s\n", config.InlineShebang) + log.Printf("Prepending inline script with %s", shebang) + writer.WriteString(shebang) + } for _, command := range config.Inline { if _, err := writer.WriteString(command + "\n"); err != nil { return "", fmt.Errorf("Error preparing shell script: %s", err) diff --git a/website/source/docs/post-processors/shell-local.html.md b/website/source/docs/post-processors/shell-local.html.md index ab2f4bba3..3ace72792 100644 --- a/website/source/docs/post-processors/shell-local.html.md +++ b/website/source/docs/post-processors/shell-local.html.md @@ -227,3 +227,105 @@ are cleaned up. For a shell script, that means the script **must** exit with a zero code. You *must* be extra careful to `exit 0` when necessary. + + +## Usage Examples: + +Example of running a .cmd file on windows: + +``` + { + "type": "shell-local", + "environment_vars": ["SHELLLOCALTEST=ShellTest1"], + "scripts": ["./scripts/test_cmd.cmd"] + }, +``` + +Contents of "test_cmd.cmd": + +``` +echo %SHELLLOCALTEST% +``` + +Example of running an inline command on windows: +Required customization: tempfile_extension + +``` + { + "type": "shell-local", + "environment_vars": ["SHELLLOCALTEST=ShellTest2"], + "tempfile_extension": ".cmd", + "inline": ["echo %SHELLLOCALTEST%"] + }, +``` + +Example of running a bash command on windows using WSL: +Required customizations: use_linux_pathing and execute_command + +``` + { + "type": "shell-local", + "environment_vars": ["SHELLLOCALTEST=ShellTest3"], + "execute_command": ["bash", "-c", "{{.Vars}} {{.Script}}"], + "use_linux_pathing": true, + "script": "./scripts/example_bash.sh" + } +``` + +Contents of "example_bash.sh": + +``` +#!/bin/bash +echo $SHELLLOCALTEST +``` + +Example of running a powershell script on windows: +Required customizations: env_var_format and execute_command + +``` + + { + "type": "shell-local", + "environment_vars": ["SHELLLOCALTEST=ShellTest4"], + "execute_command": ["powershell.exe", "{{.Vars}} {{.Script}}"], + "env_var_format": "$env:%s=\"%s\"; ", + } +``` + +Example of running a powershell script on windows as "inline": +Required customizations: env_var_format, tempfile_extension, and execute_command + +``` + { + "type": "shell-local", + "tempfile_extension": ".ps1", + "environment_vars": ["SHELLLOCALTEST=ShellTest5"], + "execute_command": ["powershell.exe", "{{.Vars}} {{.Script}}"], + "env_var_format": "$env:%s=\"%s\"; ", + "inline": ["write-output $env:SHELLLOCALTEST"] + } +``` + + +Example of running a bash script on linux: + +``` + { + "type": "shell-local", + "environment_vars": ["PROVISIONERTEST=ProvisionerTest1"], + "scripts": ["./scripts/dummy_bash.sh"] + } +``` + +Example of running a bash "inline" on linux: + +``` + { + "type": "shell-local", + "environment_vars": ["PROVISIONERTEST=ProvisionerTest2"], + "inline": ["echo hello", + "echo $PROVISIONERTEST"] + } +``` + + diff --git a/website/source/docs/provisioners/shell-local.html.md b/website/source/docs/provisioners/shell-local.html.md index eae9cf2ff..cadb1d6a1 100644 --- a/website/source/docs/provisioners/shell-local.html.md +++ b/website/source/docs/provisioners/shell-local.html.md @@ -197,3 +197,104 @@ are cleaned up. For a shell script, that means the script **must** exit with a zero code. You *must* be extra careful to `exit 0` when necessary. + + +## Usage Examples: + +Example of running a .cmd file on windows: + +``` + { + "type": "shell-local", + "environment_vars": ["SHELLLOCALTEST=ShellTest1"], + "scripts": ["./scripts/test_cmd.cmd"] + }, +``` + +Contents of "test_cmd.cmd": + +``` +echo %SHELLLOCALTEST% +``` + +Example of running an inline command on windows: +Required customization: tempfile_extension + +``` + { + "type": "shell-local", + "environment_vars": ["SHELLLOCALTEST=ShellTest2"], + "tempfile_extension": ".cmd", + "inline": ["echo %SHELLLOCALTEST%"] + }, +``` + +Example of running a bash command on windows using WSL: +Required customizations: use_linux_pathing and execute_command + +``` + { + "type": "shell-local", + "environment_vars": ["SHELLLOCALTEST=ShellTest3"], + "execute_command": ["bash", "-c", "{{.Vars}} {{.Script}}"], + "use_linux_pathing": true, + "script": "./scripts/example_bash.sh" + } +``` + +Contents of "example_bash.sh": + +``` +#!/bin/bash +echo $SHELLLOCALTEST +``` + +Example of running a powershell script on windows: +Required customizations: env_var_format and execute_command + +``` + + { + "type": "shell-local", + "environment_vars": ["SHELLLOCALTEST=ShellTest4"], + "execute_command": ["powershell.exe", "{{.Vars}} {{.Script}}"], + "env_var_format": "$env:%s=\"%s\"; ", + } +``` + +Example of running a powershell script on windows as "inline": +Required customizations: env_var_format, tempfile_extension, and execute_command + +``` + { + "type": "shell-local", + "tempfile_extension": ".ps1", + "environment_vars": ["SHELLLOCALTEST=ShellTest5"], + "execute_command": ["powershell.exe", "{{.Vars}} {{.Script}}"], + "env_var_format": "$env:%s=\"%s\"; ", + "inline": ["write-output $env:SHELLLOCALTEST"] + } +``` + + +Example of running a bash script on linux: + +``` + { + "type": "shell-local", + "environment_vars": ["PROVISIONERTEST=ProvisionerTest1"], + "scripts": ["./scripts/dummy_bash.sh"] + } +``` + +Example of running a bash "inline" on linux: + +``` + { + "type": "shell-local", + "environment_vars": ["PROVISIONERTEST=ProvisionerTest2"], + "inline": ["echo hello", + "echo $PROVISIONERTEST"] + } +``` + From 2b2bd5715c96814caa88b631290945140bfe87e0 Mon Sep 17 00:00:00 2001 From: Megan Marsh Date: Wed, 4 Apr 2018 15:29:37 -0700 Subject: [PATCH 18/68] fix docs --- .../source/docs/post-processors/shell-local.html.md | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/website/source/docs/post-processors/shell-local.html.md b/website/source/docs/post-processors/shell-local.html.md index 3ace72792..6812fac2b 100644 --- a/website/source/docs/post-processors/shell-local.html.md +++ b/website/source/docs/post-processors/shell-local.html.md @@ -100,6 +100,8 @@ Optional parameters: the example below for more guidance on how to use this feature. If you are not on a Windows host, or you do not intend to use the shell-local post-processor to run a bash script, please ignore this option. + If you set this flag to true, you still need to provide the standard windows + path to the script when providing a `script`. This is a beta feature. ## Execute Command @@ -123,8 +125,10 @@ One limitation of this offering is that "inline" and "command" options are not available to you; please limit yourself to using the "script" or "scripts" options instead. -Please note that the WSL is a beta feature, and this tool is not guaranteed to -work as you expect it to. +Please note that this feature is still in beta, as the underlying WSL is also +still in beta. There will be some limitations as a result. For example, it will +likely not work unless both Packer and the scripts you want to run are both on +the C drive. ``` { @@ -289,6 +293,7 @@ Required customizations: env_var_format and execute_command "environment_vars": ["SHELLLOCALTEST=ShellTest4"], "execute_command": ["powershell.exe", "{{.Vars}} {{.Script}}"], "env_var_format": "$env:%s=\"%s\"; ", + "script": "./scripts/example_ps.ps1" } ``` @@ -313,7 +318,7 @@ Example of running a bash script on linux: { "type": "shell-local", "environment_vars": ["PROVISIONERTEST=ProvisionerTest1"], - "scripts": ["./scripts/dummy_bash.sh"] + "scripts": ["./scripts/example_bash.sh"] } ``` From 58acb7f436cee34c350ff36e3c4084ee24267221 Mon Sep 17 00:00:00 2001 From: Megan Marsh Date: Wed, 4 Apr 2018 15:52:39 -0700 Subject: [PATCH 19/68] fix windows test --- post-processor/shell-local/post-processor_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/post-processor/shell-local/post-processor_test.go b/post-processor/shell-local/post-processor_test.go index 5fabac124..ee7e27d70 100644 --- a/post-processor/shell-local/post-processor_test.go +++ b/post-processor/shell-local/post-processor_test.go @@ -141,7 +141,7 @@ func TestPostProcessorPrepare_ExecuteCommand(t *testing.T) { if runtime.GOOS != "windows" { expected = []string{"/bin/sh", "-c", "{{.Vars}}", "{{.Script}}"} } else { - expected = []string{"cmd", "/C", "{{.Vars}}", "{{.Script}}"} + expected = []string{"cmd", "/V", "/C", "{{.Vars}}", "call", "{{.Script}}"} } if strings.Compare(strings.Join(p.config.ExecuteCommand, " "), strings.Join(expected, " ")) != 0 { t.Fatalf("Did not get expected default: expected: %#v; received %#v", expected, p.config.ExecuteCommand) From aeadd039b77c6a175982884839327ff3d6818273 Mon Sep 17 00:00:00 2001 From: DanHam Date: Thu, 10 May 2018 13:33:56 +0100 Subject: [PATCH 20/68] Fix #6240 by way of an update to github.com/masterzen/winrm (& winrm/soap) $ govendor fetch -v github.com/masterzen/winrm $ govendor fetch -v github.com/masterzen/winrm/soap * In #6240 users reported problems that could be traced to the use of RunWithString in communicator/winrm/communicator.go. * https://github.com/masterzen/winrm/pull/78 apparently fixed a race condition in RunWithString that only materialises with Go <= 1.10; This is possibly why we are only seeing this with recent releases. Additionally, the intermittent nature of the errors and error messages seen are indicative of this type of problem... so here's hoping this fixes things... --- vendor/github.com/masterzen/winrm/client.go | 33 +++++++++++++++++---- vendor/vendor.json | 10 +++---- 2 files changed, 33 insertions(+), 10 deletions(-) diff --git a/vendor/github.com/masterzen/winrm/client.go b/vendor/github.com/masterzen/winrm/client.go index 732dd61cb..c19515194 100644 --- a/vendor/github.com/masterzen/winrm/client.go +++ b/vendor/github.com/masterzen/winrm/client.go @@ -152,10 +152,20 @@ func (c *Client) RunWithString(command string, stdin string) (string, string, in } var outWriter, errWriter bytes.Buffer - go io.Copy(&outWriter, cmd.Stdout) - go io.Copy(&errWriter, cmd.Stderr) + var wg sync.WaitGroup + wg.Add(2) + go func() { + defer wg.Done() + io.Copy(&outWriter, cmd.Stdout) + }() + + go func() { + defer wg.Done() + io.Copy(&errWriter, cmd.Stderr) + }() cmd.Wait() + wg.Wait() return outWriter.String(), errWriter.String(), cmd.ExitCode(), cmd.err } @@ -176,11 +186,24 @@ func (c Client) RunWithInput(command string, stdout, stderr io.Writer, stdin io. return 1, err } - go io.Copy(cmd.Stdin, stdin) - go io.Copy(stdout, cmd.Stdout) - go io.Copy(stderr, cmd.Stderr) + var wg sync.WaitGroup + wg.Add(3) + + go func() { + defer wg.Done() + io.Copy(cmd.Stdin, stdin) + }() + go func() { + defer wg.Done() + io.Copy(stdout, cmd.Stdout) + }() + go func() { + defer wg.Done() + io.Copy(stderr, cmd.Stderr) + }() cmd.Wait() + wg.Wait() return cmd.ExitCode(), cmd.err diff --git a/vendor/vendor.json b/vendor/vendor.json index 1f08d096b..f3e751ef9 100644 --- a/vendor/vendor.json +++ b/vendor/vendor.json @@ -988,16 +988,16 @@ "revision": "95ba30457eb1121fa27753627c774c7cd4e90083" }, { - "checksumSHA1": "8z5kCCFRsBkhXic9jxxeIV3bBn8=", + "checksumSHA1": "dVQEUn5TxdIAXczK7rh6qUrq44Q=", "path": "github.com/masterzen/winrm", - "revision": "a2df6b1315e6fd5885eb15c67ed259e85854125f", - "revisionTime": "2017-08-14T13:39:27Z" + "revision": "7e40f93ae939004a1ef3bd5ff5c88c756ee762bb", + "revisionTime": "2018-02-24T16:03:50Z" }, { "checksumSHA1": "XFSXma+KmkhkIPsh4dTd/eyja5s=", "path": "github.com/masterzen/winrm/soap", - "revision": "a2df6b1315e6fd5885eb15c67ed259e85854125f", - "revisionTime": "2017-08-14T13:39:27Z" + "revision": "7e40f93ae939004a1ef3bd5ff5c88c756ee762bb", + "revisionTime": "2018-02-24T16:03:50Z" }, { "checksumSHA1": "NkbetqlpWBi3gP08JDneC+axTKw=", From fc734b6bd9575905dbab5f69980f688151336f56 Mon Sep 17 00:00:00 2001 From: Unknown Date: Sun, 6 May 2018 07:54:58 +1000 Subject: [PATCH 21/68] Using vmconnect to display gui for hyper-v vmconnect.exe comes as part of Hyper-V and is the tool used by Hyper-V Manager to connect with a virtual machine. This commits sets behaviour the same as virtualbox and vmware to display the virtual machine connection unless headless is set in the template. --- builder/hyperv/common/driver.go | 10 +++++++++ builder/hyperv/common/driver_mock.go | 22 +++++++++++++++++++ builder/hyperv/common/driver_ps_4.go | 12 ++++++++++ builder/hyperv/common/step_run.go | 14 +++++++++++- builder/hyperv/iso/builder.go | 6 ++++- builder/hyperv/vmcx/builder.go | 6 ++++- common/powershell/hyperv/hyperv.go | 13 +++++++++++ .../docs/builders/hyperv-iso.html.md.erb | 5 +++++ .../docs/builders/hyperv-vmcx.html.md.erb | 5 +++++ 9 files changed, 90 insertions(+), 3 deletions(-) diff --git a/builder/hyperv/common/driver.go b/builder/hyperv/common/driver.go index bce480227..187c601cd 100644 --- a/builder/hyperv/common/driver.go +++ b/builder/hyperv/common/driver.go @@ -1,5 +1,9 @@ package common +import ( + "context" +) + // A driver is able to talk to HyperV and perform certain // operations with it. Some of the operations on here may seem overly // specific, but they were built specifically in mind to handle features @@ -109,4 +113,10 @@ type Driver interface { MountFloppyDrive(string, string) error UnmountFloppyDrive(string) error + + // Connect connects to a VM specified by the name given. + Connect(string) context.CancelFunc + + // Disconnect disconnects to a VM specified by the context cancel function. + Disconnect(context.CancelFunc) } diff --git a/builder/hyperv/common/driver_mock.go b/builder/hyperv/common/driver_mock.go index f33cfe29f..674bd5b67 100644 --- a/builder/hyperv/common/driver_mock.go +++ b/builder/hyperv/common/driver_mock.go @@ -1,5 +1,9 @@ package common +import ( + "context" +) + type DriverMock struct { IsRunning_Called bool IsRunning_VmName string @@ -240,6 +244,13 @@ type DriverMock struct { UnmountFloppyDrive_Called bool UnmountFloppyDrive_VmName string UnmountFloppyDrive_Err error + + Connect_Called bool + Connect_VmName string + Connect_Cancel context.CancelFunc + + Disconnect_Called bool + Disconnect_Cancel context.CancelFunc } func (d *DriverMock) IsRunning(vmName string) (bool, error) { @@ -553,3 +564,14 @@ func (d *DriverMock) UnmountFloppyDrive(vmName string) error { d.UnmountFloppyDrive_VmName = vmName return d.UnmountFloppyDrive_Err } + +func (d *DriverMock) Connect(vmName string) context.CancelFunc { + d.Connect_Called = true + d.Connect_VmName = vmName + return d.Connect_Cancel +} + +func (d *DriverMock) Disconnect(cancel context.CancelFunc) { + d.Disconnect_Called = true + d.Disconnect_Cancel = cancel +} diff --git a/builder/hyperv/common/driver_ps_4.go b/builder/hyperv/common/driver_ps_4.go index ef948867a..4973ea0b0 100644 --- a/builder/hyperv/common/driver_ps_4.go +++ b/builder/hyperv/common/driver_ps_4.go @@ -1,6 +1,7 @@ package common import ( + "context" "fmt" "log" "runtime" @@ -347,3 +348,14 @@ func (d *HypervPS4Driver) verifyHypervPermissions() error { return nil } + +// Connect connects to a VM specified by the name given. +func (d *HypervPS4Driver) Connect(vmName string) context.CancelFunc { + return hyperv.ConnectVirtualMachine(vmName) +} + +// Disconnect disconnects to a VM specified by calling the context cancel function returned +// from Connect. +func (d *HypervPS4Driver) Disconnect(cancel context.CancelFunc) { + hyperv.DisconnectVirtualMachine(cancel) +} diff --git a/builder/hyperv/common/step_run.go b/builder/hyperv/common/step_run.go index 02996fb6f..c6cf78d2e 100644 --- a/builder/hyperv/common/step_run.go +++ b/builder/hyperv/common/step_run.go @@ -9,7 +9,8 @@ import ( ) type StepRun struct { - vmName string + Headless bool + vmName string } func (s *StepRun) Run(_ context.Context, state multistep.StateBag) multistep.StepAction { @@ -29,6 +30,11 @@ func (s *StepRun) Run(_ context.Context, state multistep.StateBag) multistep.Ste s.vmName = vmName + if !s.Headless { + ui.Say("Connecting to vmconnect...") + cancel := driver.Connect(vmName) + state.Put("guiCancelFunc", cancel) + } return multistep.ActionContinue } @@ -39,6 +45,12 @@ func (s *StepRun) Cleanup(state multistep.StateBag) { driver := state.Get("driver").(Driver) ui := state.Get("ui").(packer.Ui) + guiCancelFunc := state.Get("guiCancelFunc").(context.CancelFunc) + + if guiCancelFunc != nil { + ui.Say("Disconnecting from vmconnect...") + guiCancelFunc() + } if running, _ := driver.IsRunning(s.vmName); running { if err := driver.Stop(s.vmName); err != nil { diff --git a/builder/hyperv/iso/builder.go b/builder/hyperv/iso/builder.go index 7afd40c52..d50e5e26a 100644 --- a/builder/hyperv/iso/builder.go +++ b/builder/hyperv/iso/builder.go @@ -115,6 +115,8 @@ type Config struct { // Create the VM with a Fixed VHD format disk instead of Dynamic VHDX FixedVHD bool `mapstructure:"use_fixed_vhd_format"` + Headless bool `mapstructure:"headless"` + ctx interpolate.Context } @@ -427,7 +429,9 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe SwitchVlanId: b.config.SwitchVlanId, }, - &hypervcommon.StepRun{}, + &hypervcommon.StepRun{ + Headless: b.config.Headless, + }, &hypervcommon.StepTypeBootCommand{ BootCommand: b.config.FlatBootCommand(), diff --git a/builder/hyperv/vmcx/builder.go b/builder/hyperv/vmcx/builder.go index 83d911e30..987c41a12 100644 --- a/builder/hyperv/vmcx/builder.go +++ b/builder/hyperv/vmcx/builder.go @@ -98,6 +98,8 @@ type Config struct { SkipExport bool `mapstructure:"skip_export"` + Headless bool `mapstructure:"headless"` + ctx interpolate.Context } @@ -436,7 +438,9 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe SwitchVlanId: b.config.SwitchVlanId, }, - &hypervcommon.StepRun{}, + &hypervcommon.StepRun{ + Headless: b.config.Headless, + }, &hypervcommon.StepTypeBootCommand{ BootCommand: b.config.FlatBootCommand(), diff --git a/common/powershell/hyperv/hyperv.go b/common/powershell/hyperv/hyperv.go index ef85d269e..6503b2327 100644 --- a/common/powershell/hyperv/hyperv.go +++ b/common/powershell/hyperv/hyperv.go @@ -1,7 +1,9 @@ package hyperv import ( + "context" "errors" + "os/exec" "strconv" "strings" @@ -1244,3 +1246,14 @@ param([string]$vmName, [string]$scanCodes) err := ps.Run(script, vmName, scanCodes) return err } + +func ConnectVirtualMachine(vmName string) context.CancelFunc { + ctx, cancel := context.WithCancel(context.Background()) + cmd := exec.CommandContext(ctx, "vmconnect.exe", "localhost", vmName) + cmd.Start() + return cancel +} + +func DisconnectVirtualMachine(cancel context.CancelFunc) { + cancel() +} diff --git a/website/source/docs/builders/hyperv-iso.html.md.erb b/website/source/docs/builders/hyperv-iso.html.md.erb index 85b093acc..a2a97a789 100644 --- a/website/source/docs/builders/hyperv-iso.html.md.erb +++ b/website/source/docs/builders/hyperv-iso.html.md.erb @@ -102,6 +102,11 @@ can be configured for this builder. - `differencing_disk` (boolean) - If true enables differencing disks. Only the changes will be written to the new disk. This is especially useful if your source is a vhd/vhdx. This defaults to false. +- `headless` (boolean) - Packer defaults to building Hyper-V virtual + machines by launching a GUI that shows the console of the machine + being built. When this value is set to true, the machine will start without + a console. + - `skip_export` (boolean) - If true skips VM export. If you are interested only in the vhd/vhdx files, you can enable this option. This will create inline disks which improves the build performance. There will not be any copying of source vhds to temp directory. This defaults to false. diff --git a/website/source/docs/builders/hyperv-vmcx.html.md.erb b/website/source/docs/builders/hyperv-vmcx.html.md.erb index 3433decda..2bb00b0b8 100644 --- a/website/source/docs/builders/hyperv-vmcx.html.md.erb +++ b/website/source/docs/builders/hyperv-vmcx.html.md.erb @@ -139,6 +139,11 @@ can be configured for this builder. - `guest_additions_path` (string) - The path to the iso image for guest additions. +- `headless` (boolean) - Packer defaults to building Hyper-V virtual + machines by launching a GUI that shows the console of the machine + being built. When this value is set to true, the machine will start without + a console. + - `http_directory` (string) - Path to a directory to serve using an HTTP server. The files in this directory will be available over HTTP that will be requestable from the virtual machine. This is useful for hosting From 29c4b4436de4737e4480fb365112ab158d5f5697 Mon Sep 17 00:00:00 2001 From: Unknown Date: Thu, 10 May 2018 21:50:56 +1000 Subject: [PATCH 22/68] Changes requested in PR #6243 - Logging error if vmconnect.exe fails. - Using StepRun struct rather than StateBag for command Cancel function - Better handling in Disconnect when headless is true or vmconnect failed in Start --- builder/hyperv/common/driver.go | 2 +- builder/hyperv/common/driver_mock.go | 5 +++-- builder/hyperv/common/driver_ps_4.go | 2 +- builder/hyperv/common/step_run.go | 17 ++++++++++------- common/powershell/hyperv/hyperv.go | 10 +++++++--- 5 files changed, 22 insertions(+), 14 deletions(-) diff --git a/builder/hyperv/common/driver.go b/builder/hyperv/common/driver.go index 187c601cd..535652923 100644 --- a/builder/hyperv/common/driver.go +++ b/builder/hyperv/common/driver.go @@ -115,7 +115,7 @@ type Driver interface { UnmountFloppyDrive(string) error // Connect connects to a VM specified by the name given. - Connect(string) context.CancelFunc + Connect(string) (context.CancelFunc, error) // Disconnect disconnects to a VM specified by the context cancel function. Disconnect(context.CancelFunc) diff --git a/builder/hyperv/common/driver_mock.go b/builder/hyperv/common/driver_mock.go index 674bd5b67..3fcb46323 100644 --- a/builder/hyperv/common/driver_mock.go +++ b/builder/hyperv/common/driver_mock.go @@ -248,6 +248,7 @@ type DriverMock struct { Connect_Called bool Connect_VmName string Connect_Cancel context.CancelFunc + Connect_Err error Disconnect_Called bool Disconnect_Cancel context.CancelFunc @@ -565,10 +566,10 @@ func (d *DriverMock) UnmountFloppyDrive(vmName string) error { return d.UnmountFloppyDrive_Err } -func (d *DriverMock) Connect(vmName string) context.CancelFunc { +func (d *DriverMock) Connect(vmName string) (context.CancelFunc, error) { d.Connect_Called = true d.Connect_VmName = vmName - return d.Connect_Cancel + return d.Connect_Cancel, d.Connect_Err } func (d *DriverMock) Disconnect(cancel context.CancelFunc) { diff --git a/builder/hyperv/common/driver_ps_4.go b/builder/hyperv/common/driver_ps_4.go index 4973ea0b0..cadaa2ba5 100644 --- a/builder/hyperv/common/driver_ps_4.go +++ b/builder/hyperv/common/driver_ps_4.go @@ -350,7 +350,7 @@ func (d *HypervPS4Driver) verifyHypervPermissions() error { } // Connect connects to a VM specified by the name given. -func (d *HypervPS4Driver) Connect(vmName string) context.CancelFunc { +func (d *HypervPS4Driver) Connect(vmName string) (context.CancelFunc, error) { return hyperv.ConnectVirtualMachine(vmName) } diff --git a/builder/hyperv/common/step_run.go b/builder/hyperv/common/step_run.go index c6cf78d2e..13bf640df 100644 --- a/builder/hyperv/common/step_run.go +++ b/builder/hyperv/common/step_run.go @@ -3,14 +3,16 @@ package common import ( "context" "fmt" + "log" "github.com/hashicorp/packer/helper/multistep" "github.com/hashicorp/packer/packer" ) type StepRun struct { - Headless bool - vmName string + GuiCancelFunc context.CancelFunc + Headless bool + vmName string } func (s *StepRun) Run(_ context.Context, state multistep.StateBag) multistep.StepAction { @@ -32,8 +34,10 @@ func (s *StepRun) Run(_ context.Context, state multistep.StateBag) multistep.Ste if !s.Headless { ui.Say("Connecting to vmconnect...") - cancel := driver.Connect(vmName) - state.Put("guiCancelFunc", cancel) + s.GuiCancelFunc, err = driver.Connect(vmName) + if err != nil { + log.Printf(fmt.Sprintf("Non-fatal error starting vmconnect: %s. continuing...", err)) + } } return multistep.ActionContinue } @@ -45,11 +49,10 @@ func (s *StepRun) Cleanup(state multistep.StateBag) { driver := state.Get("driver").(Driver) ui := state.Get("ui").(packer.Ui) - guiCancelFunc := state.Get("guiCancelFunc").(context.CancelFunc) - if guiCancelFunc != nil { + if !s.Headless && s.GuiCancelFunc != nil { ui.Say("Disconnecting from vmconnect...") - guiCancelFunc() + s.GuiCancelFunc() } if running, _ := driver.IsRunning(s.vmName); running { diff --git a/common/powershell/hyperv/hyperv.go b/common/powershell/hyperv/hyperv.go index 6503b2327..f1ed1e45a 100644 --- a/common/powershell/hyperv/hyperv.go +++ b/common/powershell/hyperv/hyperv.go @@ -1247,11 +1247,15 @@ param([string]$vmName, [string]$scanCodes) return err } -func ConnectVirtualMachine(vmName string) context.CancelFunc { +func ConnectVirtualMachine(vmName string) (context.CancelFunc, error) { ctx, cancel := context.WithCancel(context.Background()) cmd := exec.CommandContext(ctx, "vmconnect.exe", "localhost", vmName) - cmd.Start() - return cancel + err := cmd.Start() + if err != nil { + // Failed to start so cancel function not required + cancel = nil + } + return cancel, err } func DisconnectVirtualMachine(cancel context.CancelFunc) { From 5710c0aca17979c519422b1e2572820c640e2782 Mon Sep 17 00:00:00 2001 From: Unknown Date: Mon, 14 May 2018 20:53:51 +1000 Subject: [PATCH 23/68] Making log output clearer for hyper-v gui connection --- builder/hyperv/common/step_run.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/builder/hyperv/common/step_run.go b/builder/hyperv/common/step_run.go index 13bf640df..bb415ffa1 100644 --- a/builder/hyperv/common/step_run.go +++ b/builder/hyperv/common/step_run.go @@ -33,7 +33,7 @@ func (s *StepRun) Run(_ context.Context, state multistep.StateBag) multistep.Ste s.vmName = vmName if !s.Headless { - ui.Say("Connecting to vmconnect...") + ui.Say("Attempting to connect with vmconnect...") s.GuiCancelFunc, err = driver.Connect(vmName) if err != nil { log.Printf(fmt.Sprintf("Non-fatal error starting vmconnect: %s. continuing...", err)) From c8c9bbb22ad0458709c26f06a3830ef1542b966c Mon Sep 17 00:00:00 2001 From: Hariharan Jayaraman Date: Mon, 14 May 2018 20:06:23 -0700 Subject: [PATCH 24/68] Async delete Resource Group --- builder/azure/arm/builder.go | 1 + builder/azure/arm/config.go | 3 +++ builder/azure/arm/step_create_resource_group.go | 12 +++++++++--- builder/azure/arm/step_delete_resource_group.go | 8 +++++++- builder/azure/common/constants/stateBag.go | 1 + website/source/docs/builders/azure.html.md | 4 ++++ 6 files changed, 25 insertions(+), 4 deletions(-) diff --git a/builder/azure/arm/builder.go b/builder/azure/arm/builder.go index 5603f0e52..e59f0f84b 100644 --- a/builder/azure/arm/builder.go +++ b/builder/azure/arm/builder.go @@ -344,6 +344,7 @@ func (b *Builder) configureStateBag(stateBag multistep.StateBag) { stateBag.Put(constants.ArmIsManagedImage, b.config.isManagedImage()) stateBag.Put(constants.ArmManagedImageResourceGroupName, b.config.ManagedImageResourceGroupName) stateBag.Put(constants.ArmManagedImageName, b.config.ManagedImageName) + stateBag.Put(constants.ArmAsyncRGDelete, b.config.AsyncRGDelete) } // Parameters that are only known at runtime after querying Azure. diff --git a/builder/azure/arm/config.go b/builder/azure/arm/config.go index de3a0784e..ded561a58 100644 --- a/builder/azure/arm/config.go +++ b/builder/azure/arm/config.go @@ -151,6 +151,9 @@ type Config struct { Comm communicator.Config `mapstructure:",squash"` ctx *interpolate.Context + + //Cleanup + AsyncRGDelete bool `mapstructure:"async_resourcegroup_delete"` } type keyVaultCertificate struct { diff --git a/builder/azure/arm/step_create_resource_group.go b/builder/azure/arm/step_create_resource_group.go index 9262f868a..8036adb36 100644 --- a/builder/azure/arm/step_create_resource_group.go +++ b/builder/azure/arm/step_create_resource_group.go @@ -118,14 +118,20 @@ func (s *StepCreateResourceGroup) Cleanup(state multistep.StateBag) { ctx := context.TODO() f, err := s.client.GroupsClient.Delete(ctx, resourceGroupName) if err == nil { - err = f.WaitForCompletion(ctx, s.client.GroupsClient.Client) + if state.Get(constants.ArmAsyncRGDelete).(bool) { + s.say(fmt.Sprintf("\n Not waiting for Resource Group delete as requested by user. Resource Group Name is %s", resourceGroupName)) + } else { + err = f.WaitForCompletion(ctx, s.client.GroupsClient.Client) + } } if err != nil { ui.Error(fmt.Sprintf("Error deleting resource group. Please delete it manually.\n\n"+ "Name: %s\n"+ "Error: %s", resourceGroupName, err)) + return + } + if !state.Get(constants.ArmAsyncRGDelete).(bool) { + ui.Say("Resource group has been deleted.") } - - ui.Say("Resource group has been deleted.") } } diff --git a/builder/azure/arm/step_delete_resource_group.go b/builder/azure/arm/step_delete_resource_group.go index 714f43c1b..e6445ce1d 100644 --- a/builder/azure/arm/step_delete_resource_group.go +++ b/builder/azure/arm/step_delete_resource_group.go @@ -53,7 +53,13 @@ func (s *StepDeleteResourceGroup) deleteResourceGroup(ctx context.Context, state s.say("\nThe resource group was created by Packer, deleting ...") f, err := s.client.GroupsClient.Delete(ctx, resourceGroupName) if err == nil { - f.WaitForCompletion(ctx, s.client.GroupsClient.Client) + if state.Get(constants.ArmAsyncRGDelete).(bool) { + // No need to wait for the complition for delete if request is Accepted + s.say(fmt.Sprintf("\nResource Group is being deleted, not waiting for deletion due to config. Resource Group Name '%s'", resourceGroupName)) + } else { + f.WaitForCompletion(ctx, s.client.GroupsClient.Client) + } + } if err != nil { diff --git a/builder/azure/common/constants/stateBag.go b/builder/azure/common/constants/stateBag.go index 3576f6820..a90c47e16 100644 --- a/builder/azure/common/constants/stateBag.go +++ b/builder/azure/common/constants/stateBag.go @@ -35,4 +35,5 @@ const ( ArmManagedImageResourceGroupName string = "arm.ManagedImageResourceGroupName" ArmManagedImageLocation string = "arm.ManagedImageLocation" ArmManagedImageName string = "arm.ManagedImageName" + ArmAsyncRGDelete string = "arm.AsyncRGDelete" ) diff --git a/website/source/docs/builders/azure.html.md b/website/source/docs/builders/azure.html.md index 9019e035e..23f104269 100644 --- a/website/source/docs/builders/azure.html.md +++ b/website/source/docs/builders/azure.html.md @@ -221,6 +221,10 @@ Providing `temp_resource_group_name` or `location` in combination with `build_re CLI example `azure vm sizes -l westus` +- `async_resourcegroup_delete` (boolean) If you want packer to delete the temporary resource group asynchronously. Its a boolean value + and defaults to false. **Important** Setting this true would mean that your builds are faster however this is a very + small chance that the temporary resource group is not deleted by Azure. + ## Basic Example Here is a basic example for Azure. From 2c339b99d297bac61831a39afaee50e37b84a018 Mon Sep 17 00:00:00 2001 From: DanHam Date: Sat, 12 May 2018 16:14:48 +0100 Subject: [PATCH 25/68] Sort run config options alphabetically --- builder/amazon/common/run_config.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/builder/amazon/common/run_config.go b/builder/amazon/common/run_config.go index f647182aa..a75c1df9e 100644 --- a/builder/amazon/common/run_config.go +++ b/builder/amazon/common/run_config.go @@ -30,25 +30,25 @@ func (d *AmiFilterOptions) Empty() bool { type RunConfig struct { AssociatePublicIpAddress bool `mapstructure:"associate_public_ip_address"` AvailabilityZone string `mapstructure:"availability_zone"` + DisableStopInstance bool `mapstructure:"disable_stop_instance"` EbsOptimized bool `mapstructure:"ebs_optimized"` IamInstanceProfile string `mapstructure:"iam_instance_profile"` + InstanceInitiatedShutdownBehavior string `mapstructure:"shutdown_behavior"` InstanceType string `mapstructure:"instance_type"` RunTags map[string]string `mapstructure:"run_tags"` + SecurityGroupId string `mapstructure:"security_group_id"` + SecurityGroupIds []string `mapstructure:"security_group_ids"` SourceAmi string `mapstructure:"source_ami"` SourceAmiFilter AmiFilterOptions `mapstructure:"source_ami_filter"` SpotPrice string `mapstructure:"spot_price"` SpotPriceAutoProduct string `mapstructure:"spot_price_auto_product"` - DisableStopInstance bool `mapstructure:"disable_stop_instance"` - SecurityGroupId string `mapstructure:"security_group_id"` - SecurityGroupIds []string `mapstructure:"security_group_ids"` - TemporarySGSourceCidr string `mapstructure:"temporary_security_group_source_cidr"` SubnetId string `mapstructure:"subnet_id"` TemporaryKeyPairName string `mapstructure:"temporary_key_pair_name"` + TemporarySGSourceCidr string `mapstructure:"temporary_security_group_source_cidr"` UserData string `mapstructure:"user_data"` UserDataFile string `mapstructure:"user_data_file"` - WindowsPasswordTimeout time.Duration `mapstructure:"windows_password_timeout"` VpcId string `mapstructure:"vpc_id"` - InstanceInitiatedShutdownBehavior string `mapstructure:"shutdown_behavior"` + WindowsPasswordTimeout time.Duration `mapstructure:"windows_password_timeout"` // Communicator settings Comm communicator.Config `mapstructure:",squash"` From 482629ae9025ba7bb9c4f3e151682b4df399dbc3 Mon Sep 17 00:00:00 2001 From: DanHam Date: Sun, 13 May 2018 15:21:30 +0100 Subject: [PATCH 26/68] Add config option to enable/disable T2 Unlimited for the launched instance --- builder/amazon/common/run_config.go | 1 + 1 file changed, 1 insertion(+) diff --git a/builder/amazon/common/run_config.go b/builder/amazon/common/run_config.go index a75c1df9e..5922060c2 100644 --- a/builder/amazon/common/run_config.go +++ b/builder/amazon/common/run_config.go @@ -32,6 +32,7 @@ type RunConfig struct { AvailabilityZone string `mapstructure:"availability_zone"` DisableStopInstance bool `mapstructure:"disable_stop_instance"` EbsOptimized bool `mapstructure:"ebs_optimized"` + EnableT2Unlimited bool `mapstructure:"enable_t2_unlimited"` IamInstanceProfile string `mapstructure:"iam_instance_profile"` InstanceInitiatedShutdownBehavior string `mapstructure:"shutdown_behavior"` InstanceType string `mapstructure:"instance_type"` From be02b3f61387bb49ce8954ed2cd333f2ffaa97a5 Mon Sep 17 00:00:00 2001 From: DanHam Date: Sat, 12 May 2018 16:20:01 +0100 Subject: [PATCH 27/68] Validate template settings when T2 Unlimited has been enabled * T2 Unlimited cannot be used with anything other than T2 instance types * T2 Unlimited cannot be used with Spot Instances --- builder/amazon/common/run_config.go | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/builder/amazon/common/run_config.go b/builder/amazon/common/run_config.go index 5922060c2..cd40c9dc4 100644 --- a/builder/amazon/common/run_config.go +++ b/builder/amazon/common/run_config.go @@ -6,6 +6,7 @@ import ( "net" "os" "regexp" + "strings" "time" "github.com/hashicorp/packer/common/uuid" @@ -142,6 +143,18 @@ func (c *RunConfig) Prepare(ctx *interpolate.Context) []error { errs = append(errs, fmt.Errorf("shutdown_behavior only accepts 'stop' or 'terminate' values.")) } + if c.EnableT2Unlimited { + if c.SpotPrice != "" { + errs = append(errs, fmt.Errorf("Error: T2 Unlimited cannot be used in conjuction with Spot Instances")) + } + firstDotIndex := strings.Index(c.InstanceType, ".") + if firstDotIndex == -1 { + errs = append(errs, fmt.Errorf("Error determining main Instance Type from: %s", c.InstanceType)) + } else if c.InstanceType[0:firstDotIndex] != "t2" { + errs = append(errs, fmt.Errorf("Error: T2 Unlimited enabled with a non-T2 Instance Type: %s", c.InstanceType)) + } + } + return errs } From df7fb869840e1aec1083ecef91a8ad12b374e1f1 Mon Sep 17 00:00:00 2001 From: DanHam Date: Sun, 13 May 2018 16:13:20 +0100 Subject: [PATCH 28/68] Add tests for T2 Unlimited configuration --- builder/amazon/common/run_config_test.go | 35 ++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/builder/amazon/common/run_config_test.go b/builder/amazon/common/run_config_test.go index a88730e82..212f70c02 100644 --- a/builder/amazon/common/run_config_test.go +++ b/builder/amazon/common/run_config_test.go @@ -79,6 +79,41 @@ func TestRunConfigPrepare_SourceAmiFilterGood(t *testing.T) { } } +func TestRunConfigPrepare_EnableT2UnlimitedGood(t *testing.T) { + c := testConfig() + // Must have a T2 instance type if T2 Unlimited is enabled + c.InstanceType = "t2.micro" + c.EnableT2Unlimited = true + err := c.Prepare(nil) + if len(err) > 0 { + t.Fatalf("err: %s", err) + } +} + +func TestRunConfigPrepare_EnableT2UnlimitedBadInstanceType(t *testing.T) { + c := testConfig() + // T2 Unlimited cannot be used with instance types other than T2 + c.InstanceType = "m5.large" + c.EnableT2Unlimited = true + err := c.Prepare(nil) + if len(err) != 1 { + t.Fatalf("T2 Unlimited should not work with non-T2 instance types") + } +} + +func TestRunConfigPrepare_EnableT2UnlimitedBadWithSpotInstanceRequest(t *testing.T) { + c := testConfig() + // T2 Unlimited cannot be used with Spot Instances + c.InstanceType = "t2.micro" + c.EnableT2Unlimited = true + c.SpotPrice = "auto" + c.SpotPriceAutoProduct = "Linux/UNIX" + err := c.Prepare(nil) + if len(err) != 1 { + t.Fatalf("T2 Unlimited cannot be used in conjuntion with Spot Price requests") + } +} + func TestRunConfigPrepare_SpotAuto(t *testing.T) { c := testConfig() c.SpotPrice = "auto" From 6fc68754d779e4d5fc2c54baf081e7a898497990 Mon Sep 17 00:00:00 2001 From: DanHam Date: Sun, 13 May 2018 16:32:27 +0100 Subject: [PATCH 29/68] Allow use of T2 unlimited by adding appropriate request for the instance --- builder/amazon/common/step_run_source_instance.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/builder/amazon/common/step_run_source_instance.go b/builder/amazon/common/step_run_source_instance.go index 114da38e1..4dd8fbe74 100644 --- a/builder/amazon/common/step_run_source_instance.go +++ b/builder/amazon/common/step_run_source_instance.go @@ -24,6 +24,7 @@ type StepRunSourceInstance struct { Ctx interpolate.Context Debug bool EbsOptimized bool + EnableT2Unlimited bool ExpectedRootDevice string IamInstanceProfile string InstanceInitiatedShutdownBehavior string @@ -116,6 +117,11 @@ func (s *StepRunSourceInstance) Run(ctx context.Context, state multistep.StateBa EbsOptimized: &s.EbsOptimized, } + if s.EnableT2Unlimited { + creditOption := "unlimited" + runOpts.CreditSpecification = &ec2.CreditSpecificationRequest{CpuCredits: &creditOption} + } + // Collect tags for tagging on resource creation var tagSpecs []*ec2.TagSpecification From d5304a25e928dcf43373ba09734fc5a76b796d29 Mon Sep 17 00:00:00 2001 From: DanHam Date: Sun, 13 May 2018 17:16:10 +0100 Subject: [PATCH 30/68] Pass T2 Unlimited settings to run instance step for appropriate EC2 builders --- builder/amazon/ebs/builder.go | 1 + builder/amazon/ebssurrogate/builder.go | 1 + builder/amazon/ebsvolume/builder.go | 1 + builder/amazon/instance/builder.go | 1 + 4 files changed, 4 insertions(+) diff --git a/builder/amazon/ebs/builder.go b/builder/amazon/ebs/builder.go index 5e63b05c8..665bf8098 100644 --- a/builder/amazon/ebs/builder.go +++ b/builder/amazon/ebs/builder.go @@ -148,6 +148,7 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe Ctx: b.config.ctx, Debug: b.config.PackerDebug, EbsOptimized: b.config.EbsOptimized, + EnableT2Unlimited: b.config.EnableT2Unlimited, ExpectedRootDevice: "ebs", IamInstanceProfile: b.config.IamInstanceProfile, InstanceInitiatedShutdownBehavior: b.config.InstanceInitiatedShutdownBehavior, diff --git a/builder/amazon/ebssurrogate/builder.go b/builder/amazon/ebssurrogate/builder.go index 31f47164f..52a151b22 100644 --- a/builder/amazon/ebssurrogate/builder.go +++ b/builder/amazon/ebssurrogate/builder.go @@ -162,6 +162,7 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe Ctx: b.config.ctx, Debug: b.config.PackerDebug, EbsOptimized: b.config.EbsOptimized, + EnableT2Unlimited: b.config.EnableT2Unlimited, ExpectedRootDevice: "ebs", IamInstanceProfile: b.config.IamInstanceProfile, InstanceInitiatedShutdownBehavior: b.config.InstanceInitiatedShutdownBehavior, diff --git a/builder/amazon/ebsvolume/builder.go b/builder/amazon/ebsvolume/builder.go index a1cc1fd2a..1a79b964e 100644 --- a/builder/amazon/ebsvolume/builder.go +++ b/builder/amazon/ebsvolume/builder.go @@ -145,6 +145,7 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe Ctx: b.config.ctx, Debug: b.config.PackerDebug, EbsOptimized: b.config.EbsOptimized, + EnableT2Unlimited: b.config.EnableT2Unlimited, ExpectedRootDevice: "ebs", IamInstanceProfile: b.config.IamInstanceProfile, InstanceInitiatedShutdownBehavior: b.config.InstanceInitiatedShutdownBehavior, diff --git a/builder/amazon/instance/builder.go b/builder/amazon/instance/builder.go index eb3ecdbda..c197cadeb 100644 --- a/builder/amazon/instance/builder.go +++ b/builder/amazon/instance/builder.go @@ -230,6 +230,7 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe Ctx: b.config.ctx, Debug: b.config.PackerDebug, EbsOptimized: b.config.EbsOptimized, + EnableT2Unlimited: b.config.EnableT2Unlimited, IamInstanceProfile: b.config.IamInstanceProfile, InstanceType: b.config.InstanceType, IsRestricted: b.config.IsChinaCloud() || b.config.IsGovCloud(), From a9aa9908cd41196552603d1894968417e571d131 Mon Sep 17 00:00:00 2001 From: DanHam Date: Sun, 13 May 2018 18:55:21 +0100 Subject: [PATCH 31/68] Document use of T2 Unlimited for enabled Amazon builders --- .../source/docs/builders/amazon-ebs.html.md | 24 +++++++++++++++++++ .../docs/builders/amazon-ebssurrogate.html.md | 24 +++++++++++++++++++ .../docs/builders/amazon-ebsvolume.html.md | 24 +++++++++++++++++++ .../docs/builders/amazon-instance.html.md | 24 +++++++++++++++++++ 4 files changed, 96 insertions(+) diff --git a/website/source/docs/builders/amazon-ebs.html.md b/website/source/docs/builders/amazon-ebs.html.md index d5ed5b497..2901bd249 100644 --- a/website/source/docs/builders/amazon-ebs.html.md +++ b/website/source/docs/builders/amazon-ebs.html.md @@ -169,6 +169,30 @@ builder. Note: you must make sure enhanced networking is enabled on your instance. See [Amazon's documentation on enabling enhanced networking](https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/enhanced-networking.html#enabling_enhanced_networking). Default `false`. +- `enable_t2_unlimited` (boolean) - Enabling T2 Unlimited allows the source + instance to burst additional CPU beyond its available [CPU Credits] + (https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/t2-credits-baseline-concepts.html) + for as long as the demand exists. + This is in contrast to the standard configuration that only allows an + instance to consume up to its available CPU Credits. + See the AWS documentation for [T2 Unlimited] + (https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/t2-unlimited.html) + and the 'T2 Unlimited Pricing' section of the [Amazon EC2 On-Demand + Pricing](https://aws.amazon.com/ec2/pricing/on-demand/) document for more + information. + By default this option is disabled and Packer will set up a [T2 + Standard](https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/t2-std.html) + instance instead. + + To use T2 Unlimited you must use a T2 instance type e.g. t2.micro. + Additionally, T2 Unlimited cannot be used in conjunction with Spot + Instances e.g. when the `spot_price` option has been configured. + Attempting to do so will cause an error. + + !> **Warning!** Additional costs may be incurred by enabling T2 + Unlimited - even for instances that would usually qualify for the + [AWS Free Tier](https://aws.amazon.com/free/). + - `force_deregister` (boolean) - Force Packer to first deregister an existing AMI if one with the same name already exists. Default `false`. diff --git a/website/source/docs/builders/amazon-ebssurrogate.html.md b/website/source/docs/builders/amazon-ebssurrogate.html.md index 8cf9508b4..720521536 100644 --- a/website/source/docs/builders/amazon-ebssurrogate.html.md +++ b/website/source/docs/builders/amazon-ebssurrogate.html.md @@ -162,6 +162,30 @@ builder. Note: you must make sure enhanced networking is enabled on your instance. See [Amazon's documentation on enabling enhanced networking](https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/enhanced-networking.html#enabling_enhanced_networking). Default `false`. +- `enable_t2_unlimited` (boolean) - Enabling T2 Unlimited allows the source + instance to burst additional CPU beyond its available [CPU Credits] + (https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/t2-credits-baseline-concepts.html) + for as long as the demand exists. + This is in contrast to the standard configuration that only allows an + instance to consume up to its available CPU Credits. + See the AWS documentation for [T2 Unlimited] + (https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/t2-unlimited.html) + and the 'T2 Unlimited Pricing' section of the [Amazon EC2 On-Demand + Pricing](https://aws.amazon.com/ec2/pricing/on-demand/) document for more + information. + By default this option is disabled and Packer will set up a [T2 + Standard](https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/t2-std.html) + instance instead. + + To use T2 Unlimited you must use a T2 instance type e.g. t2.micro. + Additionally, T2 Unlimited cannot be used in conjunction with Spot + Instances e.g. when the `spot_price` option has been configured. + Attempting to do so will cause an error. + + !> **Warning!** Additional costs may be incurred by enabling T2 + Unlimited - even for instances that would usually qualify for the + [AWS Free Tier](https://aws.amazon.com/free/). + - `force_deregister` (boolean) - Force Packer to first deregister an existing AMI if one with the same name already exists. Default `false`. diff --git a/website/source/docs/builders/amazon-ebsvolume.html.md b/website/source/docs/builders/amazon-ebsvolume.html.md index a39a31fcd..1bab5eb62 100644 --- a/website/source/docs/builders/amazon-ebsvolume.html.md +++ b/website/source/docs/builders/amazon-ebsvolume.html.md @@ -120,6 +120,30 @@ builder. Note: you must make sure enhanced networking is enabled on your instance. See [Amazon's documentation on enabling enhanced networking](https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/enhanced-networking.html#enabling_enhanced_networking). Default `false`. +- `enable_t2_unlimited` (boolean) - Enabling T2 Unlimited allows the source + instance to burst additional CPU beyond its available [CPU Credits] + (https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/t2-credits-baseline-concepts.html) + for as long as the demand exists. + This is in contrast to the standard configuration that only allows an + instance to consume up to its available CPU Credits. + See the AWS documentation for [T2 Unlimited] + (https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/t2-unlimited.html) + and the 'T2 Unlimited Pricing' section of the [Amazon EC2 On-Demand + Pricing](https://aws.amazon.com/ec2/pricing/on-demand/) document for more + information. + By default this option is disabled and Packer will set up a [T2 + Standard](https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/t2-std.html) + instance instead. + + To use T2 Unlimited you must use a T2 instance type e.g. t2.micro. + Additionally, T2 Unlimited cannot be used in conjunction with Spot + Instances e.g. when the `spot_price` option has been configured. + Attempting to do so will cause an error. + + !> **Warning!** Additional costs may be incurred by enabling T2 + Unlimited - even for instances that would usually qualify for the + [AWS Free Tier](https://aws.amazon.com/free/). + - `iam_instance_profile` (string) - The name of an [IAM instance profile](https://docs.aws.amazon.com/IAM/latest/UserGuide/instance-profiles.html) to launch the EC2 instance with. diff --git a/website/source/docs/builders/amazon-instance.html.md b/website/source/docs/builders/amazon-instance.html.md index 77d7c2f5b..52686aeec 100644 --- a/website/source/docs/builders/amazon-instance.html.md +++ b/website/source/docs/builders/amazon-instance.html.md @@ -193,6 +193,30 @@ builder. Note: you must make sure enhanced networking is enabled on your instance. See [Amazon's documentation on enabling enhanced networking](https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/enhanced-networking.html#enabling_enhanced_networking). Default `false`. +- `enable_t2_unlimited` (boolean) - Enabling T2 Unlimited allows the source + instance to burst additional CPU beyond its available [CPU Credits] + (https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/t2-credits-baseline-concepts.html) + for as long as the demand exists. + This is in contrast to the standard configuration that only allows an + instance to consume up to its available CPU Credits. + See the AWS documentation for [T2 Unlimited] + (https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/t2-unlimited.html) + and the 'T2 Unlimited Pricing' section of the [Amazon EC2 On-Demand + Pricing](https://aws.amazon.com/ec2/pricing/on-demand/) document for more + information. + By default this option is disabled and Packer will set up a [T2 + Standard](https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/t2-std.html) + instance instead. + + To use T2 Unlimited you must use a T2 instance type e.g. t2.micro. + Additionally, T2 Unlimited cannot be used in conjunction with Spot + Instances e.g. when the `spot_price` option has been configured. + Attempting to do so will cause an error. + + !> **Warning!** Additional costs may be incurred by enabling T2 + Unlimited - even for instances that would usually qualify for the + [AWS Free Tier](https://aws.amazon.com/free/). + - `force_deregister` (boolean) - Force Packer to first deregister an existing AMI if one with the same name already exists. Defaults to `false`. From 99e3487795278bec044684cec5ae94ff21dd34ae Mon Sep 17 00:00:00 2001 From: DanHam Date: Mon, 14 May 2018 00:54:51 +0100 Subject: [PATCH 32/68] Add missing validation and tests for Spot Instance requests --- builder/amazon/common/run_config.go | 7 +++++++ builder/amazon/common/run_config_test.go | 8 +++++++- 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/builder/amazon/common/run_config.go b/builder/amazon/common/run_config.go index cd40c9dc4..99cdc86ae 100644 --- a/builder/amazon/common/run_config.go +++ b/builder/amazon/common/run_config.go @@ -112,6 +112,13 @@ func (c *RunConfig) Prepare(ctx *interpolate.Context) []error { } } + if c.SpotPriceAutoProduct != "" { + if c.SpotPrice != "auto" { + errs = append(errs, errors.New( + "spot_price should be set to auto when spot_price_auto_product is specified")) + } + } + if c.UserData != "" && c.UserDataFile != "" { errs = append(errs, fmt.Errorf("Only one of user_data or user_data_file can be specified.")) } else if c.UserDataFile != "" { diff --git a/builder/amazon/common/run_config_test.go b/builder/amazon/common/run_config_test.go index 212f70c02..ae9a547c0 100644 --- a/builder/amazon/common/run_config_test.go +++ b/builder/amazon/common/run_config_test.go @@ -118,13 +118,19 @@ func TestRunConfigPrepare_SpotAuto(t *testing.T) { c := testConfig() c.SpotPrice = "auto" if err := c.Prepare(nil); len(err) != 1 { - t.Fatalf("err: %s", err) + t.Fatalf("spot_price_auto_product should be set when spot_price is set to auto") } + // Good - SpotPrice and SpotPriceAutoProduct are correctly set c.SpotPriceAutoProduct = "foo" if err := c.Prepare(nil); len(err) != 0 { t.Fatalf("err: %s", err) } + + c.SpotPrice = "" + if err := c.Prepare(nil); len(err) != 1 { + t.Fatalf("spot_price should be set to auto when spot_price_auto_product is set") + } } func TestRunConfigPrepare_SSHPort(t *testing.T) { From 82c8710af5f03237d8eed6f99ff7d580b68f4983 Mon Sep 17 00:00:00 2001 From: DanHam Date: Tue, 15 May 2018 10:07:09 +0100 Subject: [PATCH 33/68] Use fmt.Errorf over errors.New as we only require basic error reporting --- builder/amazon/common/run_config.go | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/builder/amazon/common/run_config.go b/builder/amazon/common/run_config.go index 99cdc86ae..bc596e580 100644 --- a/builder/amazon/common/run_config.go +++ b/builder/amazon/common/run_config.go @@ -1,7 +1,6 @@ package common import ( - "errors" "fmt" "net" "os" @@ -86,35 +85,35 @@ func (c *RunConfig) Prepare(ctx *interpolate.Context) []error { c.SSHInterface != "public_dns" && c.SSHInterface != "private_dns" && c.SSHInterface != "" { - errs = append(errs, errors.New(fmt.Sprintf("Unknown interface type: %s", c.SSHInterface))) + errs = append(errs, fmt.Errorf(fmt.Sprintf("Unknown interface type: %s", c.SSHInterface))) } if c.SSHKeyPairName != "" { if c.Comm.Type == "winrm" && c.Comm.WinRMPassword == "" && c.Comm.SSHPrivateKey == "" { - errs = append(errs, errors.New("ssh_private_key_file must be provided to retrieve the winrm password when using ssh_keypair_name.")) + errs = append(errs, fmt.Errorf("ssh_private_key_file must be provided to retrieve the winrm password when using ssh_keypair_name.")) } else if c.Comm.SSHPrivateKey == "" && !c.Comm.SSHAgentAuth { - errs = append(errs, errors.New("ssh_private_key_file must be provided or ssh_agent_auth enabled when ssh_keypair_name is specified.")) + errs = append(errs, fmt.Errorf("ssh_private_key_file must be provided or ssh_agent_auth enabled when ssh_keypair_name is specified.")) } } if c.SourceAmi == "" && c.SourceAmiFilter.Empty() { - errs = append(errs, errors.New("A source_ami or source_ami_filter must be specified")) + errs = append(errs, fmt.Errorf("A source_ami or source_ami_filter must be specified")) } if c.InstanceType == "" { - errs = append(errs, errors.New("An instance_type must be specified")) + errs = append(errs, fmt.Errorf("An instance_type must be specified")) } if c.SpotPrice == "auto" { if c.SpotPriceAutoProduct == "" { - errs = append(errs, errors.New( + errs = append(errs, fmt.Errorf( "spot_price_auto_product must be specified when spot_price is auto")) } } if c.SpotPriceAutoProduct != "" { if c.SpotPrice != "auto" { - errs = append(errs, errors.New( + errs = append(errs, fmt.Errorf( "spot_price should be set to auto when spot_price_auto_product is specified")) } } From ec8b70721cf216216128e930de1595b0fbb87eae Mon Sep 17 00:00:00 2001 From: DanHam Date: Tue, 15 May 2018 11:44:58 +0100 Subject: [PATCH 34/68] Use an explicit error message when an error is expected and we don't get one Previously, if the validation check generating the error in the main code is removed, the 'should error' tests would just return an empty message --- builder/amazon/common/run_config_test.go | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/builder/amazon/common/run_config_test.go b/builder/amazon/common/run_config_test.go index ae9a547c0..fde9ef760 100644 --- a/builder/amazon/common/run_config_test.go +++ b/builder/amazon/common/run_config_test.go @@ -48,7 +48,7 @@ func TestRunConfigPrepare_InstanceType(t *testing.T) { c := testConfig() c.InstanceType = "" if err := c.Prepare(nil); len(err) != 1 { - t.Fatalf("err: %s", err) + t.Fatalf("Should error if an instance_type is not specified") } } @@ -56,14 +56,14 @@ func TestRunConfigPrepare_SourceAmi(t *testing.T) { c := testConfig() c.SourceAmi = "" if err := c.Prepare(nil); len(err) != 1 { - t.Fatalf("err: %s", err) + t.Fatalf("Should error if a source_ami (or source_ami_filter) is not specified") } } func TestRunConfigPrepare_SourceAmiFilterBlank(t *testing.T) { c := testConfigFilter() if err := c.Prepare(nil); len(err) != 1 { - t.Fatalf("err: %s", err) + t.Fatalf("Should error if source_ami_filter is empty or not specified (and source_ami is not specified)") } } @@ -97,7 +97,7 @@ func TestRunConfigPrepare_EnableT2UnlimitedBadInstanceType(t *testing.T) { c.EnableT2Unlimited = true err := c.Prepare(nil) if len(err) != 1 { - t.Fatalf("T2 Unlimited should not work with non-T2 instance types") + t.Fatalf("Should error if T2 Unlimited is enabled with non-T2 instance_type") } } @@ -110,7 +110,7 @@ func TestRunConfigPrepare_EnableT2UnlimitedBadWithSpotInstanceRequest(t *testing c.SpotPriceAutoProduct = "Linux/UNIX" err := c.Prepare(nil) if len(err) != 1 { - t.Fatalf("T2 Unlimited cannot be used in conjuntion with Spot Price requests") + t.Fatalf("Should error if T2 Unlimited has been used in conjuntion with a Spot Price request") } } @@ -118,7 +118,7 @@ func TestRunConfigPrepare_SpotAuto(t *testing.T) { c := testConfig() c.SpotPrice = "auto" if err := c.Prepare(nil); len(err) != 1 { - t.Fatalf("spot_price_auto_product should be set when spot_price is set to auto") + t.Fatalf("Should error if spot_price_auto_product is not set and spot_price is set to auto") } // Good - SpotPrice and SpotPriceAutoProduct are correctly set @@ -129,7 +129,7 @@ func TestRunConfigPrepare_SpotAuto(t *testing.T) { c.SpotPrice = "" if err := c.Prepare(nil); len(err) != 1 { - t.Fatalf("spot_price should be set to auto when spot_price_auto_product is set") + t.Fatalf("Should error if spot_price is not set to auto and spot_price_auto_product is set") } } @@ -166,7 +166,7 @@ func TestRunConfigPrepare_UserData(t *testing.T) { c.UserData = "foo" c.UserDataFile = tf.Name() if err := c.Prepare(nil); len(err) != 1 { - t.Fatalf("err: %s", err) + t.Fatalf("Should error if user_data string and user_data_file have both been specified") } } @@ -178,7 +178,7 @@ func TestRunConfigPrepare_UserDataFile(t *testing.T) { c.UserDataFile = "idontexistidontthink" if err := c.Prepare(nil); len(err) != 1 { - t.Fatalf("err: %s", err) + t.Fatalf("Should error if the file specified by user_data_file does not exist") } tf, err := ioutil.TempFile("", "packer") From e1b18d594a604545e874bc416b3e194106cdf4af Mon Sep 17 00:00:00 2001 From: Hariharan Jayaraman Date: Tue, 15 May 2018 11:41:26 -0700 Subject: [PATCH 35/68] Updates based on PR feedback --- builder/azure/arm/builder.go | 2 +- builder/azure/arm/builder_test.go | 1 + builder/azure/arm/config.go | 2 +- builder/azure/arm/config_test.go | 67 +++++++++++++++++++ .../azure/arm/step_create_resource_group.go | 4 +- .../azure/arm/step_delete_resource_group.go | 2 +- builder/azure/common/constants/stateBag.go | 2 +- website/source/docs/builders/azure.html.md | 5 +- 8 files changed, 76 insertions(+), 9 deletions(-) diff --git a/builder/azure/arm/builder.go b/builder/azure/arm/builder.go index e59f0f84b..c21e95114 100644 --- a/builder/azure/arm/builder.go +++ b/builder/azure/arm/builder.go @@ -344,7 +344,7 @@ func (b *Builder) configureStateBag(stateBag multistep.StateBag) { stateBag.Put(constants.ArmIsManagedImage, b.config.isManagedImage()) stateBag.Put(constants.ArmManagedImageResourceGroupName, b.config.ManagedImageResourceGroupName) stateBag.Put(constants.ArmManagedImageName, b.config.ManagedImageName) - stateBag.Put(constants.ArmAsyncRGDelete, b.config.AsyncRGDelete) + stateBag.Put(constants.ArmAsyncResourceGroupDelete, b.config.AsyncResourceGroupDelete) } // Parameters that are only known at runtime after querying Azure. diff --git a/builder/azure/arm/builder_test.go b/builder/azure/arm/builder_test.go index 2db475a02..99855333d 100644 --- a/builder/azure/arm/builder_test.go +++ b/builder/azure/arm/builder_test.go @@ -25,6 +25,7 @@ func TestStateBagShouldBePopulatedExpectedValues(t *testing.T) { constants.ArmStorageAccountName, constants.ArmVirtualMachineCaptureParameters, constants.ArmPublicIPAddressName, + constants.ArmAsyncResourceGroupDelete, } for _, v := range expectedStateBagKeys { diff --git a/builder/azure/arm/config.go b/builder/azure/arm/config.go index ded561a58..2dfb69809 100644 --- a/builder/azure/arm/config.go +++ b/builder/azure/arm/config.go @@ -153,7 +153,7 @@ type Config struct { ctx *interpolate.Context //Cleanup - AsyncRGDelete bool `mapstructure:"async_resourcegroup_delete"` + AsyncResourceGroupDelete bool `mapstructure:"async_resourcegroup_delete"` } type keyVaultCertificate struct { diff --git a/builder/azure/arm/config_test.go b/builder/azure/arm/config_test.go index 909ce2c3a..8e8bd3d68 100644 --- a/builder/azure/arm/config_test.go +++ b/builder/azure/arm/config_test.go @@ -1264,6 +1264,73 @@ func TestConfigShouldAllowTempNameOverrides(t *testing.T) { } } +func TestConfigShouldAllowAsyncResourceGroupOverride(t *testing.T) { + config := map[string]interface{}{ + "image_offer": "ignore", + "image_publisher": "ignore", + "image_sku": "ignore", + "location": "ignore", + "subscription_id": "ignore", + "communicator": "none", + "os_type": "linux", + "managed_image_name": "ignore", + "managed_image_resource_group_name": "ignore", + "async_resourcegroup_delete": "true", + } + + c, _, err := newConfig(config, getPackerConfiguration()) + if err != nil { + t.Errorf("newConfig failed with %q", err) + } + + if c.AsyncResourceGroupDelete != true { + t.Errorf("expected async_resourcegroup_delete to be %q, but got %t", "async_resourcegroup_delete", c.AsyncResourceGroupDelete) + } +} +func TestConfigShouldAllowAsyncResourceGroupOverrideNoValue(t *testing.T) { + config := map[string]interface{}{ + "image_offer": "ignore", + "image_publisher": "ignore", + "image_sku": "ignore", + "location": "ignore", + "subscription_id": "ignore", + "communicator": "none", + "os_type": "linux", + "managed_image_name": "ignore", + "managed_image_resource_group_name": "ignore", + } + + c, _, err := newConfig(config, getPackerConfiguration()) + if err != nil { + t.Errorf("newConfig failed with %q", err) + } + + if c.AsyncResourceGroupDelete != false { + t.Errorf("expected async_resourcegroup_delete to be %q, but got %t", "async_resourcegroup_delete", c.AsyncResourceGroupDelete) + } +} +func TestConfigShouldAllowAsyncResourceGroupOverrideBadValue(t *testing.T) { + config := map[string]interface{}{ + "image_offer": "ignore", + "image_publisher": "ignore", + "image_sku": "ignore", + "location": "ignore", + "subscription_id": "ignore", + "communicator": "none", + "os_type": "linux", + "managed_image_name": "ignore", + "managed_image_resource_group_name": "ignore", + "async_resourcegroup_delete": "asdasda", + } + + c, _, err := newConfig(config, getPackerConfiguration()) + if err != nil && c == nil { + t.Log("newConfig failed which is expected ", err) + + } + +} + func getArmBuilderConfiguration() map[string]string { m := make(map[string]string) for _, v := range requiredConfigValues { diff --git a/builder/azure/arm/step_create_resource_group.go b/builder/azure/arm/step_create_resource_group.go index 8036adb36..f835933db 100644 --- a/builder/azure/arm/step_create_resource_group.go +++ b/builder/azure/arm/step_create_resource_group.go @@ -118,7 +118,7 @@ func (s *StepCreateResourceGroup) Cleanup(state multistep.StateBag) { ctx := context.TODO() f, err := s.client.GroupsClient.Delete(ctx, resourceGroupName) if err == nil { - if state.Get(constants.ArmAsyncRGDelete).(bool) { + if state.Get(constants.ArmAsyncResourceGroupDelete).(bool) { s.say(fmt.Sprintf("\n Not waiting for Resource Group delete as requested by user. Resource Group Name is %s", resourceGroupName)) } else { err = f.WaitForCompletion(ctx, s.client.GroupsClient.Client) @@ -130,7 +130,7 @@ func (s *StepCreateResourceGroup) Cleanup(state multistep.StateBag) { "Error: %s", resourceGroupName, err)) return } - if !state.Get(constants.ArmAsyncRGDelete).(bool) { + if !state.Get(constants.ArmAsyncResourceGroupDelete).(bool) { ui.Say("Resource group has been deleted.") } } diff --git a/builder/azure/arm/step_delete_resource_group.go b/builder/azure/arm/step_delete_resource_group.go index e6445ce1d..56d2f15c5 100644 --- a/builder/azure/arm/step_delete_resource_group.go +++ b/builder/azure/arm/step_delete_resource_group.go @@ -53,7 +53,7 @@ func (s *StepDeleteResourceGroup) deleteResourceGroup(ctx context.Context, state s.say("\nThe resource group was created by Packer, deleting ...") f, err := s.client.GroupsClient.Delete(ctx, resourceGroupName) if err == nil { - if state.Get(constants.ArmAsyncRGDelete).(bool) { + if state.Get(constants.ArmAsyncResourceGroupDelete).(bool) { // No need to wait for the complition for delete if request is Accepted s.say(fmt.Sprintf("\nResource Group is being deleted, not waiting for deletion due to config. Resource Group Name '%s'", resourceGroupName)) } else { diff --git a/builder/azure/common/constants/stateBag.go b/builder/azure/common/constants/stateBag.go index a90c47e16..e1fe8a66b 100644 --- a/builder/azure/common/constants/stateBag.go +++ b/builder/azure/common/constants/stateBag.go @@ -35,5 +35,5 @@ const ( ArmManagedImageResourceGroupName string = "arm.ManagedImageResourceGroupName" ArmManagedImageLocation string = "arm.ManagedImageLocation" ArmManagedImageName string = "arm.ManagedImageName" - ArmAsyncRGDelete string = "arm.AsyncRGDelete" + ArmAsyncResourceGroupDelete string = "arm.AsyncResourceGroupDelete" ) diff --git a/website/source/docs/builders/azure.html.md b/website/source/docs/builders/azure.html.md index 23f104269..99538700e 100644 --- a/website/source/docs/builders/azure.html.md +++ b/website/source/docs/builders/azure.html.md @@ -221,9 +221,8 @@ Providing `temp_resource_group_name` or `location` in combination with `build_re CLI example `azure vm sizes -l westus` -- `async_resourcegroup_delete` (boolean) If you want packer to delete the temporary resource group asynchronously. Its a boolean value - and defaults to false. **Important** Setting this true would mean that your builds are faster however this is a very - small chance that the temporary resource group is not deleted by Azure. +- `async_resourcegroup_delete` (boolean) If you want packer to delete the temporary resource group asynchronously. It's a boolean value + and defaults to false. **Important** Setting this true means that your builds are faster, any failed deletes are not reported. ## Basic Example From 784c7973c21dee556334af170e92e345843c689f Mon Sep 17 00:00:00 2001 From: Hariharan Jayaraman Date: Tue, 15 May 2018 11:45:24 -0700 Subject: [PATCH 36/68] minor updates to docs --- website/source/docs/builders/azure.html.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/website/source/docs/builders/azure.html.md b/website/source/docs/builders/azure.html.md index 99538700e..e697a3e42 100644 --- a/website/source/docs/builders/azure.html.md +++ b/website/source/docs/builders/azure.html.md @@ -221,8 +221,8 @@ Providing `temp_resource_group_name` or `location` in combination with `build_re CLI example `azure vm sizes -l westus` -- `async_resourcegroup_delete` (boolean) If you want packer to delete the temporary resource group asynchronously. It's a boolean value - and defaults to false. **Important** Setting this true means that your builds are faster, any failed deletes are not reported. +- `async_resourcegroup_delete` (boolean) If you want packer to delete the temporary resource group asynchronously set this value. It's a boolean value + and defaults to false. **Important** Setting this true means that your builds are faster, however any failed deletes are not reported. ## Basic Example From 2939cd75aefe6b6e97befde194315a13f42840de Mon Sep 17 00:00:00 2001 From: DanHam Date: Wed, 16 May 2018 12:17:17 +0100 Subject: [PATCH 37/68] Revert "Report the result of the disk compaction step" Unfortunately this broke the ability to build on remote (ESXi) hosts. This reverts commit 08f9d619a9299d81495d96cf10160f38cdb2476d. --- builder/vmware/common/step_compact_disk.go | 31 ---------------------- 1 file changed, 31 deletions(-) diff --git a/builder/vmware/common/step_compact_disk.go b/builder/vmware/common/step_compact_disk.go index 8d315a67e..1fdef293c 100644 --- a/builder/vmware/common/step_compact_disk.go +++ b/builder/vmware/common/step_compact_disk.go @@ -4,8 +4,6 @@ import ( "context" "fmt" "log" - "math" - "os" "github.com/hashicorp/packer/helper/multistep" "github.com/hashicorp/packer/packer" @@ -38,39 +36,10 @@ func (s StepCompactDisk) Run(_ context.Context, state multistep.StateBag) multis ui.Say("Compacting all attached virtual disks...") for i, diskFullPath := range diskFullPaths { ui.Message(fmt.Sprintf("Compacting virtual disk %d", i+1)) - // Get the file size of the virtual disk prior to compaction - fi, err := os.Stat(diskFullPath) - if err != nil { - state.Put("error", fmt.Errorf("Error getting virtual disk file info pre compaction: %s", err)) - return multistep.ActionHalt - } - diskFileSizeStart := fi.Size() - // Defragment and compact the disk if err := driver.CompactDisk(diskFullPath); err != nil { state.Put("error", fmt.Errorf("Error compacting disk: %s", err)) return multistep.ActionHalt } - // Get the file size of the virtual disk post compaction - fi, err = os.Stat(diskFullPath) - if err != nil { - state.Put("error", fmt.Errorf("Error getting virtual disk file info post compaction: %s", err)) - return multistep.ActionHalt - } - diskFileSizeEnd := fi.Size() - // Report compaction results - log.Printf("Before compaction the disk file size was: %d", diskFileSizeStart) - log.Printf("After compaction the disk file size was: %d", diskFileSizeEnd) - if diskFileSizeStart > 0 { - percentChange := ((float64(diskFileSizeEnd) / float64(diskFileSizeStart)) * 100.0) - 100.0 - switch { - case percentChange < 0: - ui.Message(fmt.Sprintf("Compacting reduced the disk file size by %.2f%%", math.Abs(percentChange))) - case percentChange == 0: - ui.Message(fmt.Sprintf("The compacting operation left the disk file size unchanged")) - case percentChange > 0: - ui.Message(fmt.Sprintf("WARNING: Compacting increased the disk file size by %.2f%%", percentChange)) - } - } } return multistep.ActionContinue From 73eb9a629ebbc5352e4e99ec063e6a461358feb9 Mon Sep 17 00:00:00 2001 From: DanHam Date: Wed, 16 May 2018 13:10:33 +0100 Subject: [PATCH 38/68] Revert "Fix test - reporting compaction results requires a tmp file" This reverts commit f342975ff31e5a59b4534b3fb216250365468b1f. --- .../vmware/common/step_compact_disk_test.go | 25 +++---------------- 1 file changed, 3 insertions(+), 22 deletions(-) diff --git a/builder/vmware/common/step_compact_disk_test.go b/builder/vmware/common/step_compact_disk_test.go index a07755229..df0fc5fca 100644 --- a/builder/vmware/common/step_compact_disk_test.go +++ b/builder/vmware/common/step_compact_disk_test.go @@ -2,8 +2,6 @@ package common import ( "context" - "io/ioutil" - "os" "testing" "github.com/hashicorp/packer/helper/multistep" @@ -17,25 +15,8 @@ func TestStepCompactDisk(t *testing.T) { state := testState(t) step := new(StepCompactDisk) - // Create a fake vmdk file for disk file size operations - diskFile, err := ioutil.TempFile("", "disk.vmdk") - if err != nil { - t.Fatalf("Error creating fake vmdk file: %s", err) - } - - diskFullPath := diskFile.Name() - defer os.Remove(diskFullPath) - - content := []byte("I am the fake vmdk's contents") - if _, err := diskFile.Write(content); err != nil { - t.Fatalf("Error writing to fake vmdk file: %s", err) - } - if err := diskFile.Close(); err != nil { - t.Fatalf("Error closing fake vmdk file: %s", err) - } - - // Set up required state - state.Put("disk_full_paths", []string{diskFullPath}) + diskFullPaths := []string{"foo"} + state.Put("disk_full_paths", diskFullPaths) driver := state.Get("driver").(*DriverMock) @@ -51,7 +32,7 @@ func TestStepCompactDisk(t *testing.T) { if !driver.CompactDiskCalled { t.Fatal("should've called") } - if driver.CompactDiskPath != diskFullPath { + if driver.CompactDiskPath != "foo" { t.Fatal("should call with right path") } } From b747877222260dbe0e86fa020ec4017391bbac56 Mon Sep 17 00:00:00 2001 From: WaaZaa666 <28536686+WaaZaa666@users.noreply.github.com> Date: Thu, 17 May 2018 14:50:18 +0200 Subject: [PATCH 39/68] Fixing #6267, multiple hyper-v disks --- common/powershell/hyperv/hyperv.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/powershell/hyperv/hyperv.go b/common/powershell/hyperv/hyperv.go index ef85d269e..a7cbb6a06 100644 --- a/common/powershell/hyperv/hyperv.go +++ b/common/powershell/hyperv/hyperv.go @@ -900,7 +900,7 @@ Hyper-V\Get-VMNetworkAdapter -VMName $vmName | Hyper-V\Connect-VMNetworkAdapter func AddVirtualMachineHardDiskDrive(vmName string, vhdRoot string, vhdName string, vhdSizeBytes int64, vhdBlockSize int64, controllerType string) error { var script = ` -param([string]$vmName,[string]$vhdRoot, [string]$vhdName, [string]$vhdSizeInBytes,[string]$vhdBlockSizeInByte [string]$controllerType) +param([string]$vmName,[string]$vhdRoot, [string]$vhdName, [string]$vhdSizeInBytes, [string]$vhdBlockSizeInByte, [string]$controllerType) $vhdPath = Join-Path -Path $vhdRoot -ChildPath $vhdName Hyper-V\New-VHD -path $vhdPath -SizeBytes $vhdSizeInBytes -BlockSizeBytes $vhdBlockSizeInByte Hyper-V\Add-VMHardDiskDrive -VMName $vmName -path $vhdPath -controllerType $controllerType From 1f46271a6b2e2eadb4036bdcc49b2b943e761464 Mon Sep 17 00:00:00 2001 From: Hariharan Jayaraman Date: Fri, 18 May 2018 00:32:01 -0700 Subject: [PATCH 40/68] Ensuring device login works for Windows build --- builder/azure/arm/azure_client.go | 2 +- builder/azure/arm/builder.go | 33 ++++- builder/azure/arm/builder_acc_test.go | 51 ++++++- builder/azure/arm/config.go | 3 - builder/azure/arm/config_test.go | 35 ----- builder/azure/common/devicelogin.go | 29 ++-- .../autorest/azure/environments.go | 2 +- .../dgrijalva/jwt-go/MIGRATION_GUIDE.md | 5 +- vendor/github.com/dgrijalva/jwt-go/README.md | 35 +++-- .../dgrijalva/jwt-go/VERSION_HISTORY.md | 13 ++ vendor/github.com/dgrijalva/jwt-go/ecdsa.go | 1 + vendor/github.com/dgrijalva/jwt-go/errors.go | 6 +- vendor/github.com/dgrijalva/jwt-go/hmac.go | 3 +- vendor/github.com/dgrijalva/jwt-go/parser.go | 134 ++++++++++-------- vendor/github.com/dgrijalva/jwt-go/rsa.go | 5 +- .../github.com/dgrijalva/jwt-go/rsa_utils.go | 32 +++++ vendor/vendor.json | 7 +- .../source/docs/builders/azure-setup.html.md | 5 +- website/source/docs/builders/azure.html.md | 7 - 19 files changed, 258 insertions(+), 150 deletions(-) diff --git a/builder/azure/arm/azure_client.go b/builder/azure/arm/azure_client.go index f8477d235..7e4f5324e 100644 --- a/builder/azure/arm/azure_client.go +++ b/builder/azure/arm/azure_client.go @@ -122,7 +122,7 @@ func byConcatDecorators(decorators ...autorest.RespondDecorator) autorest.Respon } func NewAzureClient(subscriptionID, resourceGroupName, storageAccountName string, - cloud *azure.Environment, + cloud *azure.Environment, tenantID string, isDeviceLogin bool, servicePrincipalToken, servicePrincipalTokenVault *adal.ServicePrincipalToken) (*AzureClient, error) { var azureClient = &AzureClient{} diff --git a/builder/azure/arm/builder.go b/builder/azure/arm/builder.go index f71d920cf..8f4b66aaa 100644 --- a/builder/azure/arm/builder.go +++ b/builder/azure/arm/builder.go @@ -4,17 +4,17 @@ import ( "context" "errors" "fmt" + packerAzureCommon "github.com/hashicorp/packer/builder/azure/common" "log" "os" "runtime" "strings" "time" - packerAzureCommon "github.com/hashicorp/packer/builder/azure/common" - armstorage "github.com/Azure/azure-sdk-for-go/services/storage/mgmt/2017-10-01/storage" "github.com/Azure/azure-sdk-for-go/storage" "github.com/Azure/go-autorest/autorest/adal" + "github.com/dgrijalva/jwt-go" "github.com/hashicorp/packer/builder/azure/common/constants" "github.com/hashicorp/packer/builder/azure/common/lin" packerCommon "github.com/hashicorp/packer/common" @@ -52,6 +52,10 @@ func (b *Builder) Prepare(raws ...interface{}) ([]string, error) { } func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packer.Artifact, error) { + + claims := jwt.MapClaims{} + var p jwt.Parser + ui.Say("Running builder ...") ctx, cancel := context.WithCancel(context.Background()) @@ -79,9 +83,10 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe b.config.ResourceGroupName, b.config.StorageAccount, b.config.cloudEnvironment, + b.config.TenantID, + b.config.useDeviceLogin, spnCloud, spnKeyVault) - if err != nil { return nil, err } @@ -91,6 +96,18 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe return nil, err } + _, _, err = p.ParseUnverified(spnCloud.OAuthToken(), claims) + + if err != nil { + return nil, err + } + b.config.ObjectID = claims["oid"].(string) + + if b.config.ObjectID == "" && b.config.OSType != constants.Target_Linux { + ui.Error("\n Got empty Object ID in the OAuth token , we need this for Key vault Access, bailing") + return nil, err + } + if b.config.isManagedImage() { group, err := azureClient.GroupsClient.Get(ctx, b.config.ManagedImageResourceGroupName) if err != nil { @@ -371,10 +388,15 @@ func (b *Builder) getServicePrincipalTokens(say func(string)) (*adal.ServicePrin var err error if b.config.useDeviceLogin { - servicePrincipalToken, err = packerAzureCommon.Authenticate(*b.config.cloudEnvironment, b.config.TenantID, say) + servicePrincipalToken, err = packerAzureCommon.Authenticate(*b.config.cloudEnvironment, b.config.TenantID, say, b.config.cloudEnvironment.ServiceManagementEndpoint) if err != nil { return nil, nil, err } + servicePrincipalTokenVault, err = packerAzureCommon.Authenticate(*b.config.cloudEnvironment, b.config.TenantID, say, b.config.cloudEnvironment.KeyVaultEndpoint) + if err != nil { + return nil, nil, err + } + } else { auth := NewAuthenticate(*b.config.cloudEnvironment, b.config.ClientID, b.config.ClientSecret, b.config.TenantID) @@ -382,6 +404,7 @@ func (b *Builder) getServicePrincipalTokens(say func(string)) (*adal.ServicePrin if err != nil { return nil, nil, err } + servicePrincipalToken.EnsureFresh() servicePrincipalTokenVault, err = auth.getServicePrincipalTokenWithResource( strings.TrimRight(b.config.cloudEnvironment.KeyVaultEndpoint, "/")) @@ -389,6 +412,8 @@ func (b *Builder) getServicePrincipalTokens(say func(string)) (*adal.ServicePrin if err != nil { return nil, nil, err } + servicePrincipalTokenVault.EnsureFresh() + } return servicePrincipalToken, servicePrincipalTokenVault, nil diff --git a/builder/azure/arm/builder_acc_test.go b/builder/azure/arm/builder_acc_test.go index 4c579de2f..4f16ea229 100644 --- a/builder/azure/arm/builder_acc_test.go +++ b/builder/azure/arm/builder_acc_test.go @@ -34,6 +34,14 @@ func TestBuilderAcc_ManagedDisk_Windows(t *testing.T) { }) } +func TestBuilderAcc_ManagedDisk_Windows_DeviceLogin(t *testing.T) { + builderT.Test(t, builderT.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Builder: &Builder{}, + Template: testBuilderAccManagedDiskWindowsDeviceLogin, + }) +} + func TestBuilderAcc_ManagedDisk_Linux(t *testing.T) { builderT.Test(t, builderT.TestCase{ PreCheck: func() { testAccPreCheck(t) }, @@ -65,8 +73,7 @@ const testBuilderAccManagedDiskWindows = ` "variables": { "client_id": "{{env ` + "`ARM_CLIENT_ID`" + `}}", "client_secret": "{{env ` + "`ARM_CLIENT_SECRET`" + `}}", - "subscription_id": "{{env ` + "`ARM_SUBSCRIPTION_ID`" + `}}", - "object_id": "{{env ` + "`ARM_OBJECT_ID`" + `}}" + "subscription_id": "{{env ` + "`ARM_SUBSCRIPTION_ID`" + `}}" }, "builders": [{ "type": "test", @@ -74,7 +81,6 @@ const testBuilderAccManagedDiskWindows = ` "client_id": "{{user ` + "`client_id`" + `}}", "client_secret": "{{user ` + "`client_secret`" + `}}", "subscription_id": "{{user ` + "`subscription_id`" + `}}", - "object_id": "{{user ` + "`object_id`" + `}}", "managed_image_resource_group_name": "packer-acceptance-test", "managed_image_name": "testBuilderAccManagedDiskWindows-{{timestamp}}", @@ -89,8 +95,39 @@ const testBuilderAccManagedDiskWindows = ` "winrm_insecure": "true", "winrm_timeout": "3m", "winrm_username": "packer", + "async_resourcegroup_delete": "true", - "location": "West US", + "location": "South Central US", + "vm_size": "Standard_DS2_v2" + }] +} +` + +const testBuilderAccManagedDiskWindowsDeviceLogin = ` +{ + "variables": { + "subscription_id": "{{env ` + "`ARM_SUBSCRIPTION_ID`" + `}}" + }, + "builders": [{ + "type": "test", + + "subscription_id": "{{user ` + "`subscription_id`" + `}}", + + "managed_image_resource_group_name": "packer-acceptance-test", + "managed_image_name": "testBuilderAccManagedDiskWindowsDeviceLogin-{{timestamp}}", + + "os_type": "Windows", + "image_publisher": "MicrosoftWindowsServer", + "image_offer": "WindowsServer", + "image_sku": "2012-R2-Datacenter", + + "communicator": "winrm", + "winrm_use_ssl": "true", + "winrm_insecure": "true", + "winrm_timeout": "3m", + "winrm_username": "packer", + + "location": "South Central US", "vm_size": "Standard_DS2_v2" }] } @@ -118,7 +155,7 @@ const testBuilderAccManagedDiskLinux = ` "image_offer": "UbuntuServer", "image_sku": "16.04-LTS", - "location": "West US", + "location": "South Central US", "vm_size": "Standard_DS2_v2" }] } @@ -157,7 +194,7 @@ const testBuilderAccBlobWindows = ` "winrm_timeout": "3m", "winrm_username": "packer", - "location": "West US", + "location": "South Central US", "vm_size": "Standard_DS2_v2" }] } @@ -188,7 +225,7 @@ const testBuilderAccBlobLinux = ` "image_offer": "UbuntuServer", "image_sku": "16.04-LTS", - "location": "West US", + "location": "South Central US", "vm_size": "Standard_DS2_v2" }] } diff --git a/builder/azure/arm/config.go b/builder/azure/arm/config.go index 2dfb69809..7fef5f0ae 100644 --- a/builder/azure/arm/config.go +++ b/builder/azure/arm/config.go @@ -493,9 +493,6 @@ func assertRequiredParametersSet(c *Config, errs *packer.MultiError) { // readable by the ObjectID of the App. There may be another way to handle // this case, but I am not currently aware of it - send feedback. isUseDeviceLogin := func(c *Config) bool { - if c.OSType == constants.Target_Windows { - return false - } return c.SubscriptionID != "" && c.ClientID == "" && diff --git a/builder/azure/arm/config_test.go b/builder/azure/arm/config_test.go index 8e8bd3d68..a52956917 100644 --- a/builder/azure/arm/config_test.go +++ b/builder/azure/arm/config_test.go @@ -2,13 +2,11 @@ package arm import ( "fmt" - "strings" "testing" "time" "github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2018-04-01/compute" "github.com/hashicorp/packer/builder/azure/common/constants" - "github.com/hashicorp/packer/packer" ) // List of configuration parameters that are required by the ARM builder. @@ -448,39 +446,6 @@ func TestUserDeviceLoginIsEnabledForLinux(t *testing.T) { } } -func TestUseDeviceLoginIsDisabledForWindows(t *testing.T) { - config := map[string]string{ - "capture_name_prefix": "ignore", - "capture_container_name": "ignore", - "image_offer": "ignore", - "image_publisher": "ignore", - "image_sku": "ignore", - "location": "ignore", - "storage_account": "ignore", - "resource_group_name": "ignore", - "subscription_id": "ignore", - "os_type": constants.Target_Windows, - "communicator": "none", - } - - _, _, err := newConfig(config, getPackerConfiguration()) - if err == nil { - t.Fatal("Expected test to fail, but it succeeded") - } - - multiError, _ := err.(*packer.MultiError) - if len(multiError.Errors) != 2 { - t.Errorf("Expected to find 2 errors, but found %d errors", len(multiError.Errors)) - } - - if !strings.Contains(err.Error(), "client_id must be specified") { - t.Error("Expected to find error for 'client_id must be specified") - } - if !strings.Contains(err.Error(), "client_secret must be specified") { - t.Error("Expected to find error for 'client_secret must be specified") - } -} - func TestConfigShouldRejectMalformedCaptureNamePrefix(t *testing.T) { config := map[string]string{ "capture_container_name": "ignore", diff --git a/builder/azure/common/devicelogin.go b/builder/azure/common/devicelogin.go index a63f34cc1..ea87767ca 100644 --- a/builder/azure/common/devicelogin.go +++ b/builder/azure/common/devicelogin.go @@ -7,6 +7,7 @@ import ( "os" "path/filepath" "regexp" + "strings" "github.com/Azure/azure-sdk-for-go/services/resources/mgmt/2016-06-01/subscriptions" "github.com/Azure/go-autorest/autorest" @@ -40,8 +41,11 @@ var ( // Authenticate fetches a token from the local file cache or initiates a consent // flow and waits for token to be obtained. -func Authenticate(env azure.Environment, tenantID string, say func(string)) (*adal.ServicePrincipalToken, error) { +func Authenticate(env azure.Environment, tenantID string, say func(string), apiScope string) (*adal.ServicePrincipalToken, error) { clientID, ok := clientIDs[env.Name] + var resourceid string + var endpoint string + if !ok { return nil, fmt.Errorf("packer-azure application not set up for Azure environment %q", env.Name) } @@ -53,9 +57,17 @@ func Authenticate(env azure.Environment, tenantID string, say func(string)) (*ad // for AzurePublicCloud (https://management.core.windows.net/), this old // Service Management scope covers both ASM and ARM. - apiScope := env.ServiceManagementEndpoint + //apiScope := env.ServiceManagementEndpoint - tokenPath := tokenCachePath(tenantID) + if strings.Contains(apiScope, "vault") { + resourceid = "vault" + endpoint = env.KeyVaultEndpoint + } else { + resourceid = "mgmt" + endpoint = env.ResourceManagerEndpoint + } + + tokenPath := tokenCachePath(tenantID + resourceid) saveToken := mkTokenCallback(tokenPath) saveTokenCallback := func(t adal.Token) error { say("Azure token expired. Saving the refreshed token...") @@ -82,7 +94,7 @@ func Authenticate(env azure.Environment, tenantID string, say func(string)) (*ad // will go stale every 14 days and we will delete the token file, // re-initiate the device flow. say("Validating the token.") - if err = validateToken(env, spt); err != nil { + if err = validateToken(endpoint, spt); err != nil { say(fmt.Sprintf("Error: %v", err)) say("Stored Azure credentials expired. Please reauthenticate.") say(fmt.Sprintf("Deleting %s", tokenPath)) @@ -187,12 +199,11 @@ func mkTokenCallback(path string) adal.TokenRefreshCallback { // sure if the access_token valid, if not it uses SDK’s functionality to // automatically refresh the token using refresh_token (which might have // expired). This check is essentially to make sure refresh_token is good. -func validateToken(env azure.Environment, token *adal.ServicePrincipalToken) error { - c := subscriptions.NewClientWithBaseURI(env.ResourceManagerEndpoint) - c.Authorizer = autorest.NewBearerAuthorizer(token) - _, err := c.List(context.TODO()) +func validateToken(env string, token *adal.ServicePrincipalToken) error { + err := token.EnsureFresh() + if err != nil { - return fmt.Errorf("Token validity check failed: %v", err) + return fmt.Errorf("%s token validity check failed: %v", env,err) } return nil } diff --git a/vendor/github.com/Azure/go-autorest/autorest/azure/environments.go b/vendor/github.com/Azure/go-autorest/autorest/azure/environments.go index 7e41f7fd9..b6b4010b1 100644 --- a/vendor/github.com/Azure/go-autorest/autorest/azure/environments.go +++ b/vendor/github.com/Azure/go-autorest/autorest/azure/environments.go @@ -67,7 +67,7 @@ var ( ResourceManagerEndpoint: "https://management.azure.com/", ActiveDirectoryEndpoint: "https://login.microsoftonline.com/", GalleryEndpoint: "https://gallery.azure.com/", - KeyVaultEndpoint: "https://vault.azure.net/", + KeyVaultEndpoint: "https://vault.azure.net", GraphEndpoint: "https://graph.windows.net/", ServiceBusEndpoint: "https://servicebus.windows.net/", BatchManagementEndpoint: "https://batch.core.windows.net/", diff --git a/vendor/github.com/dgrijalva/jwt-go/MIGRATION_GUIDE.md b/vendor/github.com/dgrijalva/jwt-go/MIGRATION_GUIDE.md index fd62e9490..7fc1f793c 100644 --- a/vendor/github.com/dgrijalva/jwt-go/MIGRATION_GUIDE.md +++ b/vendor/github.com/dgrijalva/jwt-go/MIGRATION_GUIDE.md @@ -56,8 +56,9 @@ This simple parsing example: is directly mapped to: ```go - if token, err := request.ParseFromRequest(tokenString, request.OAuth2Extractor, req, keyLookupFunc); err == nil { - fmt.Printf("Token for user %v expires %v", token.Claims["user"], token.Claims["exp"]) + if token, err := request.ParseFromRequest(req, request.OAuth2Extractor, keyLookupFunc); err == nil { + claims := token.Claims.(jwt.MapClaims) + fmt.Printf("Token for user %v expires %v", claims["user"], claims["exp"]) } ``` diff --git a/vendor/github.com/dgrijalva/jwt-go/README.md b/vendor/github.com/dgrijalva/jwt-go/README.md index 00f613672..d358d881b 100644 --- a/vendor/github.com/dgrijalva/jwt-go/README.md +++ b/vendor/github.com/dgrijalva/jwt-go/README.md @@ -1,11 +1,15 @@ -A [go](http://www.golang.org) (or 'golang' for search engine friendliness) implementation of [JSON Web Tokens](http://self-issued.info/docs/draft-ietf-oauth-json-web-token.html) +# jwt-go [![Build Status](https://travis-ci.org/dgrijalva/jwt-go.svg?branch=master)](https://travis-ci.org/dgrijalva/jwt-go) +[![GoDoc](https://godoc.org/github.com/dgrijalva/jwt-go?status.svg)](https://godoc.org/github.com/dgrijalva/jwt-go) -**BREAKING CHANGES:*** Version 3.0.0 is here. It includes _a lot_ of changes including a few that break the API. We've tried to break as few things as possible, so there should just be a few type signature changes. A full list of breaking changes is available in `VERSION_HISTORY.md`. See `MIGRATION_GUIDE.md` for more information on updating your code. +A [go](http://www.golang.org) (or 'golang' for search engine friendliness) implementation of [JSON Web Tokens](http://self-issued.info/docs/draft-ietf-oauth-json-web-token.html) -**NOTICE:** A vulnerability in JWT was [recently published](https://auth0.com/blog/2015/03/31/critical-vulnerabilities-in-json-web-token-libraries/). As this library doesn't force users to validate the `alg` is what they expected, it's possible your usage is effected. There will be an update soon to remedy this, and it will likey require backwards-incompatible changes to the API. In the short term, please make sure your implementation verifies the `alg` is what you expect. +**NEW VERSION COMING:** There have been a lot of improvements suggested since the version 3.0.0 released in 2016. I'm working now on cutting two different releases: 3.2.0 will contain any non-breaking changes or enhancements. 4.0.0 will follow shortly which will include breaking changes. See the 4.0.0 milestone to get an idea of what's coming. If you have other ideas, or would like to participate in 4.0.0, now's the time. If you depend on this library and don't want to be interrupted, I recommend you use your dependency mangement tool to pin to version 3. +**SECURITY NOTICE:** Some older versions of Go have a security issue in the cryotp/elliptic. Recommendation is to upgrade to at least 1.8.3. See issue #216 for more detail. + +**SECURITY NOTICE:** It's important that you [validate the `alg` presented is what you expect](https://auth0.com/blog/2015/03/31/critical-vulnerabilities-in-json-web-token-libraries/). This library attempts to make it easy to do the right thing by requiring key types match the expected alg, but you should take the extra step to verify it in your usage. See the examples provided. ## What the heck is a JWT? @@ -25,8 +29,8 @@ This library supports the parsing and verification as well as the generation and See [the project documentation](https://godoc.org/github.com/dgrijalva/jwt-go) for examples of usage: -* [Simple example of parsing and validating a token](https://godoc.org/github.com/dgrijalva/jwt-go#example_Parse_hmac) -* [Simple example of building and signing a token](https://godoc.org/github.com/dgrijalva/jwt-go#example_New_hmac) +* [Simple example of parsing and validating a token](https://godoc.org/github.com/dgrijalva/jwt-go#example-Parse--Hmac) +* [Simple example of building and signing a token](https://godoc.org/github.com/dgrijalva/jwt-go#example-New--Hmac) * [Directory of Examples](https://godoc.org/github.com/dgrijalva/jwt-go#pkg-examples) ## Extensions @@ -37,7 +41,7 @@ Here's an example of an extension that integrates with the Google App Engine sig ## Compliance -This library was last reviewed to comply with [RTF 7519](http://www.rfc-editor.org/info/rfc7519) dated May 2015 with a few notable differences: +This library was last reviewed to comply with [RTF 7519](http://www.rfc-editor.org/info/rfc7519) dated May 2015 with a few notable differences: * In order to protect against accidental use of [Unsecured JWTs](http://self-issued.info/docs/draft-ietf-oauth-json-web-token.html#UnsecuredJWT), tokens using `alg=none` will only be accepted if the constant `jwt.UnsafeAllowNoneSignatureType` is provided as the key. @@ -47,7 +51,10 @@ This library is considered production ready. Feedback and feature requests are This project uses [Semantic Versioning 2.0.0](http://semver.org). Accepted pull requests will land on `master`. Periodically, versions will be tagged from `master`. You can find all the releases on [the project releases page](https://github.com/dgrijalva/jwt-go/releases). -While we try to make it obvious when we make breaking changes, there isn't a great mechanism for pushing announcements out to users. You may want to use this alternative package include: `gopkg.in/dgrijalva/jwt-go.v2`. It will do the right thing WRT semantic versioning. +While we try to make it obvious when we make breaking changes, there isn't a great mechanism for pushing announcements out to users. You may want to use this alternative package include: `gopkg.in/dgrijalva/jwt-go.v3`. It will do the right thing WRT semantic versioning. + +**BREAKING CHANGES:*** +* Version 3.0.0 includes _a lot_ of changes from the 2.x line, including a few that break the API. We've tried to break as few things as possible, so there should just be a few type signature changes. A full list of breaking changes is available in `VERSION_HISTORY.md`. See `MIGRATION_GUIDE.md` for more information on updating your code. ## Usage Tips @@ -68,18 +75,26 @@ Symmetric signing methods, such as HSA, use only a single secret. This is probab Asymmetric signing methods, such as RSA, use different keys for signing and verifying tokens. This makes it possible to produce tokens with a private key, and allow any consumer to access the public key for verification. +### Signing Methods and Key Types + +Each signing method expects a different object type for its signing keys. See the package documentation for details. Here are the most common ones: + +* The [HMAC signing method](https://godoc.org/github.com/dgrijalva/jwt-go#SigningMethodHMAC) (`HS256`,`HS384`,`HS512`) expect `[]byte` values for signing and validation +* The [RSA signing method](https://godoc.org/github.com/dgrijalva/jwt-go#SigningMethodRSA) (`RS256`,`RS384`,`RS512`) expect `*rsa.PrivateKey` for signing and `*rsa.PublicKey` for validation +* The [ECDSA signing method](https://godoc.org/github.com/dgrijalva/jwt-go#SigningMethodECDSA) (`ES256`,`ES384`,`ES512`) expect `*ecdsa.PrivateKey` for signing and `*ecdsa.PublicKey` for validation + ### JWT and OAuth It's worth mentioning that OAuth and JWT are not the same thing. A JWT token is simply a signed JSON object. It can be used anywhere such a thing is useful. There is some confusion, though, as JWT is the most common type of bearer token used in OAuth2 authentication. Without going too far down the rabbit hole, here's a description of the interaction of these technologies: -* OAuth is a protocol for allowing an identity provider to be separate from the service a user is logging in to. For example, whenever you use Facebook to log into a different service (Yelp, Spotify, etc), you are using OAuth. +* OAuth is a protocol for allowing an identity provider to be separate from the service a user is logging in to. For example, whenever you use Facebook to log into a different service (Yelp, Spotify, etc), you are using OAuth. * OAuth defines several options for passing around authentication data. One popular method is called a "bearer token". A bearer token is simply a string that _should_ only be held by an authenticated user. Thus, simply presenting this token proves your identity. You can probably derive from here why a JWT might make a good bearer token. * Because bearer tokens are used for authentication, it's important they're kept secret. This is why transactions that use bearer tokens typically happen over SSL. - + ## More Documentation can be found [on godoc.org](http://godoc.org/github.com/dgrijalva/jwt-go). -The command line utility included in this project (cmd/jwt) provides a straightforward example of token creation and parsing as well as a useful tool for debugging your own integration. You'll also find several implementation examples in to documentation. +The command line utility included in this project (cmd/jwt) provides a straightforward example of token creation and parsing as well as a useful tool for debugging your own integration. You'll also find several implementation examples in the documentation. diff --git a/vendor/github.com/dgrijalva/jwt-go/VERSION_HISTORY.md b/vendor/github.com/dgrijalva/jwt-go/VERSION_HISTORY.md index b605b4509..637029831 100644 --- a/vendor/github.com/dgrijalva/jwt-go/VERSION_HISTORY.md +++ b/vendor/github.com/dgrijalva/jwt-go/VERSION_HISTORY.md @@ -1,5 +1,18 @@ ## `jwt-go` Version History +#### 3.2.0 + +* Added method `ParseUnverified` to allow users to split up the tasks of parsing and validation +* HMAC signing method returns `ErrInvalidKeyType` instead of `ErrInvalidKey` where appropriate +* Added options to `request.ParseFromRequest`, which allows for an arbitrary list of modifiers to parsing behavior. Initial set include `WithClaims` and `WithParser`. Existing usage of this function will continue to work as before. +* Deprecated `ParseFromRequestWithClaims` to simplify API in the future. + +#### 3.1.0 + +* Improvements to `jwt` command line tool +* Added `SkipClaimsValidation` option to `Parser` +* Documentation updates + #### 3.0.0 * **Compatibility Breaking Changes**: See MIGRATION_GUIDE.md for tips on updating your code diff --git a/vendor/github.com/dgrijalva/jwt-go/ecdsa.go b/vendor/github.com/dgrijalva/jwt-go/ecdsa.go index 2f59a2223..f97738124 100644 --- a/vendor/github.com/dgrijalva/jwt-go/ecdsa.go +++ b/vendor/github.com/dgrijalva/jwt-go/ecdsa.go @@ -14,6 +14,7 @@ var ( ) // Implements the ECDSA family of signing methods signing methods +// Expects *ecdsa.PrivateKey for signing and *ecdsa.PublicKey for verification type SigningMethodECDSA struct { Name string Hash crypto.Hash diff --git a/vendor/github.com/dgrijalva/jwt-go/errors.go b/vendor/github.com/dgrijalva/jwt-go/errors.go index 662df19d4..1c93024aa 100644 --- a/vendor/github.com/dgrijalva/jwt-go/errors.go +++ b/vendor/github.com/dgrijalva/jwt-go/errors.go @@ -51,13 +51,9 @@ func (e ValidationError) Error() string { } else { return "token is invalid" } - return e.Inner.Error() } // No errors func (e *ValidationError) valid() bool { - if e.Errors > 0 { - return false - } - return true + return e.Errors == 0 } diff --git a/vendor/github.com/dgrijalva/jwt-go/hmac.go b/vendor/github.com/dgrijalva/jwt-go/hmac.go index c22991925..addbe5d40 100644 --- a/vendor/github.com/dgrijalva/jwt-go/hmac.go +++ b/vendor/github.com/dgrijalva/jwt-go/hmac.go @@ -7,6 +7,7 @@ import ( ) // Implements the HMAC-SHA family of signing methods signing methods +// Expects key type of []byte for both signing and validation type SigningMethodHMAC struct { Name string Hash crypto.Hash @@ -90,5 +91,5 @@ func (m *SigningMethodHMAC) Sign(signingString string, key interface{}) (string, return EncodeSegment(hasher.Sum(nil)), nil } - return "", ErrInvalidKey + return "", ErrInvalidKeyType } diff --git a/vendor/github.com/dgrijalva/jwt-go/parser.go b/vendor/github.com/dgrijalva/jwt-go/parser.go index 7020c52a1..d6901d9ad 100644 --- a/vendor/github.com/dgrijalva/jwt-go/parser.go +++ b/vendor/github.com/dgrijalva/jwt-go/parser.go @@ -8,8 +8,9 @@ import ( ) type Parser struct { - ValidMethods []string // If populated, only these methods will be considered valid - UseJSONNumber bool // Use JSON Number format in JSON decoder + ValidMethods []string // If populated, only these methods will be considered valid + UseJSONNumber bool // Use JSON Number format in JSON decoder + SkipClaimsValidation bool // Skip claims validation during token parsing } // Parse, validate, and return a token. @@ -20,55 +21,9 @@ func (p *Parser) Parse(tokenString string, keyFunc Keyfunc) (*Token, error) { } func (p *Parser) ParseWithClaims(tokenString string, claims Claims, keyFunc Keyfunc) (*Token, error) { - parts := strings.Split(tokenString, ".") - if len(parts) != 3 { - return nil, NewValidationError("token contains an invalid number of segments", ValidationErrorMalformed) - } - - var err error - token := &Token{Raw: tokenString} - - // parse Header - var headerBytes []byte - if headerBytes, err = DecodeSegment(parts[0]); err != nil { - if strings.HasPrefix(strings.ToLower(tokenString), "bearer ") { - return token, NewValidationError("tokenstring should not contain 'bearer '", ValidationErrorMalformed) - } - return token, &ValidationError{Inner: err, Errors: ValidationErrorMalformed} - } - if err = json.Unmarshal(headerBytes, &token.Header); err != nil { - return token, &ValidationError{Inner: err, Errors: ValidationErrorMalformed} - } - - // parse Claims - var claimBytes []byte - token.Claims = claims - - if claimBytes, err = DecodeSegment(parts[1]); err != nil { - return token, &ValidationError{Inner: err, Errors: ValidationErrorMalformed} - } - dec := json.NewDecoder(bytes.NewBuffer(claimBytes)) - if p.UseJSONNumber { - dec.UseNumber() - } - // JSON Decode. Special case for map type to avoid weird pointer behavior - if c, ok := token.Claims.(MapClaims); ok { - err = dec.Decode(&c) - } else { - err = dec.Decode(&claims) - } - // Handle decode error + token, parts, err := p.ParseUnverified(tokenString, claims) if err != nil { - return token, &ValidationError{Inner: err, Errors: ValidationErrorMalformed} - } - - // Lookup signature method - if method, ok := token.Header["alg"].(string); ok { - if token.Method = GetSigningMethod(method); token.Method == nil { - return token, NewValidationError("signing method (alg) is unavailable.", ValidationErrorUnverifiable) - } - } else { - return token, NewValidationError("signing method (alg) is unspecified.", ValidationErrorUnverifiable) + return token, err } // Verify signing method is in the required set @@ -95,20 +50,25 @@ func (p *Parser) ParseWithClaims(tokenString string, claims Claims, keyFunc Keyf } if key, err = keyFunc(token); err != nil { // keyFunc returned an error + if ve, ok := err.(*ValidationError); ok { + return token, ve + } return token, &ValidationError{Inner: err, Errors: ValidationErrorUnverifiable} } vErr := &ValidationError{} // Validate Claims - if err := token.Claims.Valid(); err != nil { + if !p.SkipClaimsValidation { + if err := token.Claims.Valid(); err != nil { - // If the Claims Valid returned an error, check if it is a validation error, - // If it was another error type, create a ValidationError with a generic ClaimsInvalid flag set - if e, ok := err.(*ValidationError); !ok { - vErr = &ValidationError{Inner: err, Errors: ValidationErrorClaimsInvalid} - } else { - vErr = e + // If the Claims Valid returned an error, check if it is a validation error, + // If it was another error type, create a ValidationError with a generic ClaimsInvalid flag set + if e, ok := err.(*ValidationError); !ok { + vErr = &ValidationError{Inner: err, Errors: ValidationErrorClaimsInvalid} + } else { + vErr = e + } } } @@ -126,3 +86,63 @@ func (p *Parser) ParseWithClaims(tokenString string, claims Claims, keyFunc Keyf return token, vErr } + +// WARNING: Don't use this method unless you know what you're doing +// +// This method parses the token but doesn't validate the signature. It's only +// ever useful in cases where you know the signature is valid (because it has +// been checked previously in the stack) and you want to extract values from +// it. +func (p *Parser) ParseUnverified(tokenString string, claims Claims) (token *Token, parts []string, err error) { + parts = strings.Split(tokenString, ".") + if len(parts) != 3 { + return nil, parts, NewValidationError("token contains an invalid number of segments", ValidationErrorMalformed) + } + + token = &Token{Raw: tokenString} + + // parse Header + var headerBytes []byte + if headerBytes, err = DecodeSegment(parts[0]); err != nil { + if strings.HasPrefix(strings.ToLower(tokenString), "bearer ") { + return token, parts, NewValidationError("tokenstring should not contain 'bearer '", ValidationErrorMalformed) + } + return token, parts, &ValidationError{Inner: err, Errors: ValidationErrorMalformed} + } + if err = json.Unmarshal(headerBytes, &token.Header); err != nil { + return token, parts, &ValidationError{Inner: err, Errors: ValidationErrorMalformed} + } + + // parse Claims + var claimBytes []byte + token.Claims = claims + + if claimBytes, err = DecodeSegment(parts[1]); err != nil { + return token, parts, &ValidationError{Inner: err, Errors: ValidationErrorMalformed} + } + dec := json.NewDecoder(bytes.NewBuffer(claimBytes)) + if p.UseJSONNumber { + dec.UseNumber() + } + // JSON Decode. Special case for map type to avoid weird pointer behavior + if c, ok := token.Claims.(MapClaims); ok { + err = dec.Decode(&c) + } else { + err = dec.Decode(&claims) + } + // Handle decode error + if err != nil { + return token, parts, &ValidationError{Inner: err, Errors: ValidationErrorMalformed} + } + + // Lookup signature method + if method, ok := token.Header["alg"].(string); ok { + if token.Method = GetSigningMethod(method); token.Method == nil { + return token, parts, NewValidationError("signing method (alg) is unavailable.", ValidationErrorUnverifiable) + } + } else { + return token, parts, NewValidationError("signing method (alg) is unspecified.", ValidationErrorUnverifiable) + } + + return token, parts, nil +} diff --git a/vendor/github.com/dgrijalva/jwt-go/rsa.go b/vendor/github.com/dgrijalva/jwt-go/rsa.go index 0ae0b1984..e4caf1ca4 100644 --- a/vendor/github.com/dgrijalva/jwt-go/rsa.go +++ b/vendor/github.com/dgrijalva/jwt-go/rsa.go @@ -7,6 +7,7 @@ import ( ) // Implements the RSA family of signing methods signing methods +// Expects *rsa.PrivateKey for signing and *rsa.PublicKey for validation type SigningMethodRSA struct { Name string Hash crypto.Hash @@ -44,7 +45,7 @@ func (m *SigningMethodRSA) Alg() string { } // Implements the Verify method from SigningMethod -// For this signing method, must be an rsa.PublicKey structure. +// For this signing method, must be an *rsa.PublicKey structure. func (m *SigningMethodRSA) Verify(signingString, signature string, key interface{}) error { var err error @@ -73,7 +74,7 @@ func (m *SigningMethodRSA) Verify(signingString, signature string, key interface } // Implements the Sign method from SigningMethod -// For this signing method, must be an rsa.PrivateKey structure. +// For this signing method, must be an *rsa.PrivateKey structure. func (m *SigningMethodRSA) Sign(signingString string, key interface{}) (string, error) { var rsaKey *rsa.PrivateKey var ok bool diff --git a/vendor/github.com/dgrijalva/jwt-go/rsa_utils.go b/vendor/github.com/dgrijalva/jwt-go/rsa_utils.go index 213a90dbb..a5ababf95 100644 --- a/vendor/github.com/dgrijalva/jwt-go/rsa_utils.go +++ b/vendor/github.com/dgrijalva/jwt-go/rsa_utils.go @@ -39,6 +39,38 @@ func ParseRSAPrivateKeyFromPEM(key []byte) (*rsa.PrivateKey, error) { return pkey, nil } +// Parse PEM encoded PKCS1 or PKCS8 private key protected with password +func ParseRSAPrivateKeyFromPEMWithPassword(key []byte, password string) (*rsa.PrivateKey, error) { + var err error + + // Parse PEM block + var block *pem.Block + if block, _ = pem.Decode(key); block == nil { + return nil, ErrKeyMustBePEMEncoded + } + + var parsedKey interface{} + + var blockDecrypted []byte + if blockDecrypted, err = x509.DecryptPEMBlock(block, []byte(password)); err != nil { + return nil, err + } + + if parsedKey, err = x509.ParsePKCS1PrivateKey(blockDecrypted); err != nil { + if parsedKey, err = x509.ParsePKCS8PrivateKey(blockDecrypted); err != nil { + return nil, err + } + } + + var pkey *rsa.PrivateKey + var ok bool + if pkey, ok = parsedKey.(*rsa.PrivateKey); !ok { + return nil, ErrNotRSAPrivateKey + } + + return pkey, nil +} + // Parse PEM encoded PKCS1 or PKCS8 public key func ParseRSAPublicKeyFromPEM(key []byte) (*rsa.PublicKey, error) { var err error diff --git a/vendor/vendor.json b/vendor/vendor.json index 1f08d096b..0680363b1 100644 --- a/vendor/vendor.json +++ b/vendor/vendor.json @@ -629,10 +629,11 @@ "revisionTime": "2017-11-27T16:20:29Z" }, { - "checksumSHA1": "D37uI+U+FYvTJIdG2TTozXe7i7U=", - "comment": "v3.0.0", + "checksumSHA1": "4772zXrOaPVeDeSgdiV7Vp4KEjk=", + "comment": "v3.2.0", "path": "github.com/dgrijalva/jwt-go", - "revision": "d2709f9f1f31ebcda9651b03077758c1f3a0018c" + "revision": "06ea1031745cb8b3dab3f6a236daf2b0aa468b7e", + "revisionTime": "2018-03-08T23:13:08Z" }, { "checksumSHA1": "W1LGm0UNirwMDVCMFv5vZrOpUJI=", diff --git a/website/source/docs/builders/azure-setup.html.md b/website/source/docs/builders/azure-setup.html.md index 92ae04325..22df6be63 100644 --- a/website/source/docs/builders/azure-setup.html.md +++ b/website/source/docs/builders/azure-setup.html.md @@ -31,8 +31,7 @@ In order to get all of the items above, you will need a username and password fo Device login is an alternative way to authorize in Azure Packer. Device login only requires you to know your Subscription ID. (Device login is only supported for Linux based VMs.) Device login is intended for those who are first -time users, and just want to ''kick the tires.'' We recommend the SPN approach if you intend to automate Packer, or for -deploying Windows VMs. +time users, and just want to ''kick the tires.'' We recommend the SPN approach if you intend to automate Packer. > Device login is for **interactive** builds, and SPN is **automated** builds. @@ -44,7 +43,7 @@ There are three pieces of information you must provide to enable device login mo > Device login mode is enabled by not setting client\_id and client\_secret. -> Device login mode is for the Public and US Gov clouds only, and Linux VMs only. +> Device login mode is for the Public and US Gov clouds only. The device login flow asks that you open a web browser, navigate to , and input the supplied code. This authorizes the Packer for Azure application to act on your behalf. An OAuth token will be created, and stored diff --git a/website/source/docs/builders/azure.html.md b/website/source/docs/builders/azure.html.md index 4b62c6764..3946f00d3 100644 --- a/website/source/docs/builders/azure.html.md +++ b/website/source/docs/builders/azure.html.md @@ -140,11 +140,6 @@ Providing `temp_resource_group_name` or `location` in combination with `build_re account type for a managed image. Valid values are Standard_LRS and Premium\_LRS. The default is Standard\_LRS. -- `object_id` (string) Specify an OAuth Object ID to protect WinRM certificates - created at runtime. This variable is required when creating images based on - Windows; this variable is not used by non-Windows builds. See `Windows` - behavior for `os_type`, below. - - `os_disk_size_gb` (number) Specify the size of the OS disk in GB (gigabytes). Values of zero or less than zero are ignored. @@ -412,8 +407,6 @@ A Windows build requires two templates and two deployments. Unfortunately, the K the same time hence the need for two templates and deployments. The time required to deploy a KeyVault template is minimal, so overall impact is small. -> The KeyVault certificate is protected using the object\_id of the SPN. This is why Windows builds require object\_id, -> and an SPN. The KeyVault is deleted when the resource group is deleted. See the [examples/azure](https://github.com/hashicorp/packer/tree/master/examples/azure) folder in the packer project for more examples. From df5cc234fc02f3e996ca53ce9a1eb1e15a01af70 Mon Sep 17 00:00:00 2001 From: Hariharan Jayaraman Date: Fri, 18 May 2018 00:39:57 -0700 Subject: [PATCH 41/68] updates --- builder/azure/arm/azure_client.go | 2 +- builder/azure/arm/builder.go | 2 -- builder/azure/common/devicelogin.go | 2 +- .../github.com/Azure/go-autorest/autorest/azure/environments.go | 2 +- 4 files changed, 3 insertions(+), 5 deletions(-) diff --git a/builder/azure/arm/azure_client.go b/builder/azure/arm/azure_client.go index 7e4f5324e..f8477d235 100644 --- a/builder/azure/arm/azure_client.go +++ b/builder/azure/arm/azure_client.go @@ -122,7 +122,7 @@ func byConcatDecorators(decorators ...autorest.RespondDecorator) autorest.Respon } func NewAzureClient(subscriptionID, resourceGroupName, storageAccountName string, - cloud *azure.Environment, tenantID string, isDeviceLogin bool, + cloud *azure.Environment, servicePrincipalToken, servicePrincipalTokenVault *adal.ServicePrincipalToken) (*AzureClient, error) { var azureClient = &AzureClient{} diff --git a/builder/azure/arm/builder.go b/builder/azure/arm/builder.go index 8f4b66aaa..4575ede15 100644 --- a/builder/azure/arm/builder.go +++ b/builder/azure/arm/builder.go @@ -83,8 +83,6 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe b.config.ResourceGroupName, b.config.StorageAccount, b.config.cloudEnvironment, - b.config.TenantID, - b.config.useDeviceLogin, spnCloud, spnKeyVault) if err != nil { diff --git a/builder/azure/common/devicelogin.go b/builder/azure/common/devicelogin.go index ea87767ca..ad4a2ce36 100644 --- a/builder/azure/common/devicelogin.go +++ b/builder/azure/common/devicelogin.go @@ -203,7 +203,7 @@ func validateToken(env string, token *adal.ServicePrincipalToken) error { err := token.EnsureFresh() if err != nil { - return fmt.Errorf("%s token validity check failed: %v", env,err) + return fmt.Errorf("%s token validity check failed: %v", env, err) } return nil } diff --git a/vendor/github.com/Azure/go-autorest/autorest/azure/environments.go b/vendor/github.com/Azure/go-autorest/autorest/azure/environments.go index b6b4010b1..7e41f7fd9 100644 --- a/vendor/github.com/Azure/go-autorest/autorest/azure/environments.go +++ b/vendor/github.com/Azure/go-autorest/autorest/azure/environments.go @@ -67,7 +67,7 @@ var ( ResourceManagerEndpoint: "https://management.azure.com/", ActiveDirectoryEndpoint: "https://login.microsoftonline.com/", GalleryEndpoint: "https://gallery.azure.com/", - KeyVaultEndpoint: "https://vault.azure.net", + KeyVaultEndpoint: "https://vault.azure.net/", GraphEndpoint: "https://graph.windows.net/", ServiceBusEndpoint: "https://servicebus.windows.net/", BatchManagementEndpoint: "https://batch.core.windows.net/", From 91eed4da524386d43009ba983c49593de39228f9 Mon Sep 17 00:00:00 2001 From: Hariharan Jayaraman Date: Fri, 18 May 2018 00:44:25 -0700 Subject: [PATCH 42/68] trim right of the keyvault url --- builder/azure/arm/builder.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/builder/azure/arm/builder.go b/builder/azure/arm/builder.go index 4575ede15..6493fa54b 100644 --- a/builder/azure/arm/builder.go +++ b/builder/azure/arm/builder.go @@ -390,7 +390,7 @@ func (b *Builder) getServicePrincipalTokens(say func(string)) (*adal.ServicePrin if err != nil { return nil, nil, err } - servicePrincipalTokenVault, err = packerAzureCommon.Authenticate(*b.config.cloudEnvironment, b.config.TenantID, say, b.config.cloudEnvironment.KeyVaultEndpoint) + servicePrincipalTokenVault, err = packerAzureCommon.Authenticate(*b.config.cloudEnvironment, b.config.TenantID, say, strings.TrimRight(b.config.cloudEnvironment.KeyVaultEndpoint, "/")) if err != nil { return nil, nil, err } From de1783240fbae8c1c69a61abe33e6b3565e9c2bd Mon Sep 17 00:00:00 2001 From: Hariharan Jayaraman Date: Fri, 18 May 2018 00:53:44 -0700 Subject: [PATCH 43/68] Updates to remove space changes --- builder/azure/arm/builder.go | 1 + 1 file changed, 1 insertion(+) diff --git a/builder/azure/arm/builder.go b/builder/azure/arm/builder.go index 6493fa54b..f7876b463 100644 --- a/builder/azure/arm/builder.go +++ b/builder/azure/arm/builder.go @@ -85,6 +85,7 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe b.config.cloudEnvironment, spnCloud, spnKeyVault) + if err != nil { return nil, err } From 77fe1bffe4cfb48cd939f2219cc319e45b6e6948 Mon Sep 17 00:00:00 2001 From: Hariharan Jayaraman Date: Fri, 18 May 2018 01:25:19 -0700 Subject: [PATCH 44/68] Ensure that Device Login tests dont block general acceptance tests --- builder/azure/arm/builder_acc_test.go | 49 +++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/builder/azure/arm/builder_acc_test.go b/builder/azure/arm/builder_acc_test.go index 4f16ea229..df2cc8d9d 100644 --- a/builder/azure/arm/builder_acc_test.go +++ b/builder/azure/arm/builder_acc_test.go @@ -24,8 +24,12 @@ import ( "testing" builderT "github.com/hashicorp/packer/helper/builder/testing" + "os" + "fmt" ) +const DeviceLoginAcceptanceTest = "DEVICELOGIN_TEST" + func TestBuilderAcc_ManagedDisk_Windows(t *testing.T) { builderT.Test(t, builderT.TestCase{ PreCheck: func() { testAccPreCheck(t) }, @@ -35,6 +39,12 @@ func TestBuilderAcc_ManagedDisk_Windows(t *testing.T) { } func TestBuilderAcc_ManagedDisk_Windows_DeviceLogin(t *testing.T) { + if os.Getenv(DeviceLoginAcceptanceTest) == "" { + t.Skip(fmt.Sprintf( + "Device Login Acceptance tests skipped unless env '%s' set, as its requires manual step during execution", + DeviceLoginAcceptanceTest)) + return + } builderT.Test(t, builderT.TestCase{ PreCheck: func() { testAccPreCheck(t) }, Builder: &Builder{}, @@ -50,6 +60,21 @@ func TestBuilderAcc_ManagedDisk_Linux(t *testing.T) { }) } +func TestBuilderAcc_ManagedDisk_Linux_DeviceLogin(t *testing.T) { + if os.Getenv(DeviceLoginAcceptanceTest) == "" { + t.Skip(fmt.Sprintf( + "Device Login Acceptance tests skipped unless env '%s' set, as its requires manual step during execution", + DeviceLoginAcceptanceTest)) + return + } + builderT.Test(t, builderT.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Builder: &Builder{}, + Template: testBuilderAccManagedDiskLinuxDeviceLogin, + }) +} + + func TestBuilderAcc_Blob_Windows(t *testing.T) { builderT.Test(t, builderT.TestCase{ PreCheck: func() { testAccPreCheck(t) }, @@ -160,6 +185,30 @@ const testBuilderAccManagedDiskLinux = ` }] } ` +const testBuilderAccManagedDiskLinuxDeviceLogin = ` +{ + "variables": { + "subscription_id": "{{env ` + "`ARM_SUBSCRIPTION_ID`" + `}}" + }, + "builders": [{ + "type": "test", + + "subscription_id": "{{user ` + "`subscription_id`" + `}}", + + "managed_image_resource_group_name": "packer-acceptance-test", + "managed_image_name": "testBuilderAccManagedDiskLinuxDeviceLogin-{{timestamp}}", + + "os_type": "Linux", + "image_publisher": "Canonical", + "image_offer": "UbuntuServer", + "image_sku": "16.04-LTS", + "async_resourcegroup_delete": "true", + + "location": "South Central US", + "vm_size": "Standard_DS2_v2" + }] +} +` const testBuilderAccBlobWindows = ` { From 7f2277676a20665f44297ed718a22f3f54e50cbf Mon Sep 17 00:00:00 2001 From: Hariharan Jayaraman Date: Fri, 18 May 2018 01:34:12 -0700 Subject: [PATCH 45/68] Ensure that Device Login tests dont block general acceptance tests --- builder/azure/arm/builder_acc_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/builder/azure/arm/builder_acc_test.go b/builder/azure/arm/builder_acc_test.go index df2cc8d9d..63bfe82c0 100644 --- a/builder/azure/arm/builder_acc_test.go +++ b/builder/azure/arm/builder_acc_test.go @@ -12,7 +12,7 @@ package arm // The subscription in question should have a resource group // called "packer-acceptance-test" in "West US" region. The // storage account refered to in the above variable should -// be inside this resource group and in "West US" as well. +// be inside this resource group and in "South Central US" as well. // // In addition, the PACKER_ACC variable should also be set to // a non-empty value to enable Packer acceptance tests and the From 667113338a1eb98051d3a1575e51b4a62401774c Mon Sep 17 00:00:00 2001 From: Hariharan Jayaraman Date: Fri, 18 May 2018 01:41:00 -0700 Subject: [PATCH 46/68] missed formating --- builder/azure/arm/builder.go | 2 +- builder/azure/arm/builder_acc_test.go | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/builder/azure/arm/builder.go b/builder/azure/arm/builder.go index f7876b463..1c7badb8d 100644 --- a/builder/azure/arm/builder.go +++ b/builder/azure/arm/builder.go @@ -85,7 +85,7 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe b.config.cloudEnvironment, spnCloud, spnKeyVault) - + if err != nil { return nil, err } diff --git a/builder/azure/arm/builder_acc_test.go b/builder/azure/arm/builder_acc_test.go index 63bfe82c0..056ee6e0b 100644 --- a/builder/azure/arm/builder_acc_test.go +++ b/builder/azure/arm/builder_acc_test.go @@ -23,9 +23,9 @@ package arm import ( "testing" + "fmt" builderT "github.com/hashicorp/packer/helper/builder/testing" "os" - "fmt" ) const DeviceLoginAcceptanceTest = "DEVICELOGIN_TEST" @@ -74,7 +74,6 @@ func TestBuilderAcc_ManagedDisk_Linux_DeviceLogin(t *testing.T) { }) } - func TestBuilderAcc_Blob_Windows(t *testing.T) { builderT.Test(t, builderT.TestCase{ PreCheck: func() { testAccPreCheck(t) }, From 3ca4a7208fcc4db3f9b153a14758cc12be4abba0 Mon Sep 17 00:00:00 2001 From: Hariharan Jayaraman Date: Fri, 18 May 2018 08:12:43 -0700 Subject: [PATCH 47/68] Updated Samples and added a windows quick start as well --- examples/azure/windows.json | 4 +-- examples/azure/windows_quickstart.json | 36 ++++++++++++++++++++++++++ 2 files changed, 37 insertions(+), 3 deletions(-) create mode 100644 examples/azure/windows_quickstart.json diff --git a/examples/azure/windows.json b/examples/azure/windows.json index b2e0e49fd..e0b03b7ae 100644 --- a/examples/azure/windows.json +++ b/examples/azure/windows.json @@ -2,8 +2,7 @@ "variables": { "client_id": "{{env `ARM_CLIENT_ID`}}", "client_secret": "{{env `ARM_CLIENT_SECRET`}}", - "subscription_id": "{{env `ARM_SUBSCRIPTION_ID`}}", - "object_id": "{{env `ARM_OBJECT_ID`}}" + "subscription_id": "{{env `ARM_SUBSCRIPTION_ID`}}" }, "builders": [{ "type": "azure-arm", @@ -11,7 +10,6 @@ "client_id": "{{user `client_id`}}", "client_secret": "{{user `client_secret`}}", "subscription_id": "{{user `subscription_id`}}", - "object_id": "{{user `object_id`}}", "managed_image_resource_group_name": "packertest", "managed_image_name": "MyWindowsOSImage", diff --git a/examples/azure/windows_quickstart.json b/examples/azure/windows_quickstart.json new file mode 100644 index 000000000..3f7a1e9bb --- /dev/null +++ b/examples/azure/windows_quickstart.json @@ -0,0 +1,36 @@ +{ + "variables": { + "subscription_id": "{{env `ARM_SUBSCRIPTION_ID`}}" + }, + "builders": [{ + "type": "azure-arm", + + "subscription_id": "{{user `subscription_id`}}", + + "managed_image_resource_group_name": "packertest", + "managed_image_name": "MyWindowsOSImage", + + "os_type": "Windows", + "image_publisher": "MicrosoftWindowsServer", + "image_offer": "WindowsServer", + "image_sku": "2012-R2-Datacenter", + + "communicator": "winrm", + "winrm_use_ssl": "true", + "winrm_insecure": "true", + "winrm_timeout": "3m", + "winrm_username": "packer", + + "location": "South Central US", + "vm_size": "Standard_DS2_v2" + }], + "provisioners": [{ + "type": "powershell", + "inline": [ + "if( Test-Path $Env:SystemRoot\\windows\\system32\\Sysprep\\unattend.xml ){ rm $Env:SystemRoot\\windows\\system32\\Sysprep\\unattend.xml -Force}", + "& $env:SystemRoot\\System32\\Sysprep\\Sysprep.exe /oobe /generalize /quiet /quit", + "while($true) { $imageState = Get-ItemProperty HKLM:\\SOFTWARE\\Microsoft\\Windows\\CurrentVersion\\Setup\\State | Select ImageState; if($imageState.ImageState -ne 'IMAGE_STATE_GENERALIZE_RESEAL_TO_OOBE') { Write-Output $imageState.ImageState; Start-Sleep -s 10 } else { break } }" + ] + }] +} + From ea9b2a8b5fdef856da8fefa4d2a591ba9928ed09 Mon Sep 17 00:00:00 2001 From: Hariharan Jayaraman Date: Fri, 18 May 2018 12:09:42 -0700 Subject: [PATCH 48/68] review feedback --- builder/azure/arm/builder.go | 26 +++++++++----- builder/azure/arm/builder_acc_test.go | 2 +- builder/azure/common/devicelogin.go | 50 +++------------------------ 3 files changed, 23 insertions(+), 55 deletions(-) diff --git a/builder/azure/arm/builder.go b/builder/azure/arm/builder.go index 1c7badb8d..7eb9e47f3 100644 --- a/builder/azure/arm/builder.go +++ b/builder/azure/arm/builder.go @@ -4,7 +4,6 @@ import ( "context" "errors" "fmt" - packerAzureCommon "github.com/hashicorp/packer/builder/azure/common" "log" "os" "runtime" @@ -15,6 +14,7 @@ import ( "github.com/Azure/azure-sdk-for-go/storage" "github.com/Azure/go-autorest/autorest/adal" "github.com/dgrijalva/jwt-go" + packerAzureCommon "github.com/hashicorp/packer/builder/azure/common" "github.com/hashicorp/packer/builder/azure/common/constants" "github.com/hashicorp/packer/builder/azure/common/lin" packerCommon "github.com/hashicorp/packer/common" @@ -53,9 +53,6 @@ func (b *Builder) Prepare(raws ...interface{}) ([]string, error) { func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packer.Artifact, error) { - claims := jwt.MapClaims{} - var p jwt.Parser - ui.Say("Running builder ...") ctx, cancel := context.WithCancel(context.Background()) @@ -95,6 +92,9 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe return nil, err } + claims := jwt.MapClaims{} + var p jwt.Parser + _, _, err = p.ParseUnverified(spnCloud.OAuthToken(), claims) if err != nil { @@ -103,8 +103,7 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe b.config.ObjectID = claims["oid"].(string) if b.config.ObjectID == "" && b.config.OSType != constants.Target_Linux { - ui.Error("\n Got empty Object ID in the OAuth token , we need this for Key vault Access, bailing") - return nil, err + return nil, fmt.Errorf("could not determined the ObjectID for the user, which is required for Windows builds") } if b.config.isManagedImage() { @@ -403,17 +402,26 @@ func (b *Builder) getServicePrincipalTokens(say func(string)) (*adal.ServicePrin if err != nil { return nil, nil, err } - servicePrincipalToken.EnsureFresh() servicePrincipalTokenVault, err = auth.getServicePrincipalTokenWithResource( strings.TrimRight(b.config.cloudEnvironment.KeyVaultEndpoint, "/")) - if err != nil { return nil, nil, err } - servicePrincipalTokenVault.EnsureFresh() } + err = servicePrincipalToken.EnsureFresh() + + if err != nil { + return nil, nil, err + } + + err = servicePrincipalTokenVault.EnsureFresh() + + if err != nil { + return nil, nil, err + } + return servicePrincipalToken, servicePrincipalTokenVault, nil } diff --git a/builder/azure/arm/builder_acc_test.go b/builder/azure/arm/builder_acc_test.go index 056ee6e0b..3b03025d4 100644 --- a/builder/azure/arm/builder_acc_test.go +++ b/builder/azure/arm/builder_acc_test.go @@ -10,7 +10,7 @@ package arm // * ARM_STORAGE_ACCOUNT // // The subscription in question should have a resource group -// called "packer-acceptance-test" in "West US" region. The +// called "packer-acceptance-test" in "South Central US" region. The // storage account refered to in the above variable should // be inside this resource group and in "South Central US" as well. // diff --git a/builder/azure/common/devicelogin.go b/builder/azure/common/devicelogin.go index ad4a2ce36..8d053f802 100644 --- a/builder/azure/common/devicelogin.go +++ b/builder/azure/common/devicelogin.go @@ -41,10 +41,9 @@ var ( // Authenticate fetches a token from the local file cache or initiates a consent // flow and waits for token to be obtained. -func Authenticate(env azure.Environment, tenantID string, say func(string), apiScope string) (*adal.ServicePrincipalToken, error) { +func Authenticate(env azure.Environment, tenantID string, say func(string), scope string) (*adal.ServicePrincipalToken, error) { clientID, ok := clientIDs[env.Name] var resourceid string - var endpoint string if !ok { return nil, fmt.Errorf("packer-azure application not set up for Azure environment %q", env.Name) @@ -57,14 +56,11 @@ func Authenticate(env azure.Environment, tenantID string, say func(string), apiS // for AzurePublicCloud (https://management.core.windows.net/), this old // Service Management scope covers both ASM and ARM. - //apiScope := env.ServiceManagementEndpoint - if strings.Contains(apiScope, "vault") { + if strings.Contains(scope, "vault") { resourceid = "vault" - endpoint = env.KeyVaultEndpoint } else { resourceid = "mgmt" - endpoint = env.ResourceManagerEndpoint } tokenPath := tokenCachePath(tenantID + resourceid) @@ -75,41 +71,18 @@ func Authenticate(env azure.Environment, tenantID string, say func(string), apiS } // Lookup the token cache file for an existing token. - spt, err := tokenFromFile(say, *oauthCfg, tokenPath, clientID, apiScope, saveTokenCallback) + spt, err := tokenFromFile(say, *oauthCfg, tokenPath, clientID, scope, saveTokenCallback) if err != nil { return nil, err } if spt != nil { say(fmt.Sprintf("Auth token found in file: %s", tokenPath)) - - // NOTE(ahmetalpbalkan): The token file we found may contain an - // expired access_token. In that case, the first call to Azure SDK will - // attempt to refresh the token using refresh_token, which might have - // expired[1], in that case we will get an error and we shall remove the - // token file and initiate token flow again so that the user would not - // need removing the token cache file manually. - // - // [1]: expiration date of refresh_token is not returned in AAD /token - // response, we just know it is 14 days. Therefore user’s token - // will go stale every 14 days and we will delete the token file, - // re-initiate the device flow. - say("Validating the token.") - if err = validateToken(endpoint, spt); err != nil { - say(fmt.Sprintf("Error: %v", err)) - say("Stored Azure credentials expired. Please reauthenticate.") - say(fmt.Sprintf("Deleting %s", tokenPath)) - if err := os.RemoveAll(tokenPath); err != nil { - return nil, fmt.Errorf("Error deleting stale token file: %v", err) - } - } else { - say("Token works.") - return spt, nil - } + return spt, nil } // Start an OAuth 2.0 device flow say(fmt.Sprintf("Initiating device flow: %s", tokenPath)) - spt, err = tokenFromDeviceFlow(say, *oauthCfg, clientID, apiScope) + spt, err = tokenFromDeviceFlow(say, *oauthCfg, clientID, scope) if err != nil { return nil, err } @@ -195,19 +168,6 @@ func mkTokenCallback(path string) adal.TokenRefreshCallback { } } -// validateToken makes a call to Azure SDK with given token, essentially making -// sure if the access_token valid, if not it uses SDK’s functionality to -// automatically refresh the token using refresh_token (which might have -// expired). This check is essentially to make sure refresh_token is good. -func validateToken(env string, token *adal.ServicePrincipalToken) error { - err := token.EnsureFresh() - - if err != nil { - return fmt.Errorf("%s token validity check failed: %v", env, err) - } - return nil -} - // FindTenantID figures out the AAD tenant ID of the subscription by making an // unauthenticated request to the Get Subscription Details endpoint and parses // the value from WWW-Authenticate header. From 00e809cb7e10944f2e2e13f292cfca57694217df Mon Sep 17 00:00:00 2001 From: Hariharan Jayaraman Date: Fri, 18 May 2018 15:21:49 -0700 Subject: [PATCH 49/68] Refactored the change into a new function --- builder/azure/arm/builder.go | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/builder/azure/arm/builder.go b/builder/azure/arm/builder.go index 7eb9e47f3..3aa7bcd5b 100644 --- a/builder/azure/arm/builder.go +++ b/builder/azure/arm/builder.go @@ -92,15 +92,7 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe return nil, err } - claims := jwt.MapClaims{} - var p jwt.Parser - - _, _, err = p.ParseUnverified(spnCloud.OAuthToken(), claims) - - if err != nil { - return nil, err - } - b.config.ObjectID = claims["oid"].(string) + b.config.ObjectID = getObjectIdFromToken(spnCloud) if b.config.ObjectID == "" && b.config.OSType != constants.Target_Linux { return nil, fmt.Errorf("could not determined the ObjectID for the user, which is required for Windows builds") @@ -425,3 +417,18 @@ func (b *Builder) getServicePrincipalTokens(say func(string)) (*adal.ServicePrin return servicePrincipalToken, servicePrincipalTokenVault, nil } + +func getObjectIdFromToken(token *adal.ServicePrincipalToken) (oid string) { + claims := jwt.MapClaims{} + var p jwt.Parser + + var err error + + _, _, err = p.ParseUnverified(token.OAuthToken(), claims) + + if err != nil { + return "" + } + return claims["oid"].(string) + +} From 4992429e8c83f7a6189ffcdff471a85a5cb503d0 Mon Sep 17 00:00:00 2001 From: Hariharan Jayaraman Date: Fri, 18 May 2018 17:34:01 -0700 Subject: [PATCH 50/68] Minor comment fixes --- builder/azure/arm/builder.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/builder/azure/arm/builder.go b/builder/azure/arm/builder.go index 3aa7bcd5b..5da549a29 100644 --- a/builder/azure/arm/builder.go +++ b/builder/azure/arm/builder.go @@ -418,7 +418,7 @@ func (b *Builder) getServicePrincipalTokens(say func(string)) (*adal.ServicePrin return servicePrincipalToken, servicePrincipalTokenVault, nil } -func getObjectIdFromToken(token *adal.ServicePrincipalToken) (oid string) { +func getObjectIdFromToken(token *adal.ServicePrincipalToken) string { claims := jwt.MapClaims{} var p jwt.Parser From da67df6d03a8c7f15fbf49ccfe1e64d380f5e487 Mon Sep 17 00:00:00 2001 From: Hariharan Jayaraman Date: Fri, 18 May 2018 21:17:19 -0700 Subject: [PATCH 51/68] space fix --- builder/azure/arm/builder_acc_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/builder/azure/arm/builder_acc_test.go b/builder/azure/arm/builder_acc_test.go index 3b03025d4..0b1f85656 100644 --- a/builder/azure/arm/builder_acc_test.go +++ b/builder/azure/arm/builder_acc_test.go @@ -119,7 +119,7 @@ const testBuilderAccManagedDiskWindows = ` "winrm_insecure": "true", "winrm_timeout": "3m", "winrm_username": "packer", - "async_resourcegroup_delete": "true", + "async_resourcegroup_delete": "true", "location": "South Central US", "vm_size": "Standard_DS2_v2" @@ -201,7 +201,7 @@ const testBuilderAccManagedDiskLinuxDeviceLogin = ` "image_publisher": "Canonical", "image_offer": "UbuntuServer", "image_sku": "16.04-LTS", - "async_resourcegroup_delete": "true", + "async_resourcegroup_delete": "true", "location": "South Central US", "vm_size": "Standard_DS2_v2" From a54fcc9efe3840c81c8599da3612cea2a3dbbf89 Mon Sep 17 00:00:00 2001 From: Hariharan Jayaraman Date: Sat, 19 May 2018 13:16:57 -0700 Subject: [PATCH 52/68] missed doc fixes to remove referece for object_id, note keeping the command for now for how to get object ID for older releases --- website/source/docs/builders/azure-setup.html.md | 2 -- 1 file changed, 2 deletions(-) diff --git a/website/source/docs/builders/azure-setup.html.md b/website/source/docs/builders/azure-setup.html.md index 22df6be63..07e292e24 100644 --- a/website/source/docs/builders/azure-setup.html.md +++ b/website/source/docs/builders/azure-setup.html.md @@ -17,8 +17,6 @@ In order to build VMs in Azure Packer needs 6 configuration options to be specif - `client_secret` - service principal secret / password -- `object_id` - service principal object id (OSType = Windows Only) - - `resource_group_name` - name of the resource group where your VHD(s) will be stored - `storage_account` - name of the storage account where your VHD(s) will be stored From 8a3e599cad771a59b39cb5306c982e5efebb4a71 Mon Sep 17 00:00:00 2001 From: Hariharan Jayaraman Date: Mon, 21 May 2018 11:05:59 -0700 Subject: [PATCH 53/68] Added text to point out two device auth --- builder/azure/arm/builder.go | 2 ++ website/source/docs/builders/azure-setup.html.md | 3 ++- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/builder/azure/arm/builder.go b/builder/azure/arm/builder.go index 5da549a29..eecef1b19 100644 --- a/builder/azure/arm/builder.go +++ b/builder/azure/arm/builder.go @@ -378,10 +378,12 @@ func (b *Builder) getServicePrincipalTokens(say func(string)) (*adal.ServicePrin var err error if b.config.useDeviceLogin { + say("Getting auth token for Service management endpoint") servicePrincipalToken, err = packerAzureCommon.Authenticate(*b.config.cloudEnvironment, b.config.TenantID, say, b.config.cloudEnvironment.ServiceManagementEndpoint) if err != nil { return nil, nil, err } + say("Getting token for Vault resource") servicePrincipalTokenVault, err = packerAzureCommon.Authenticate(*b.config.cloudEnvironment, b.config.TenantID, say, strings.TrimRight(b.config.cloudEnvironment.KeyVaultEndpoint, "/")) if err != nil { return nil, nil, err diff --git a/website/source/docs/builders/azure-setup.html.md b/website/source/docs/builders/azure-setup.html.md index 07e292e24..fe57033c4 100644 --- a/website/source/docs/builders/azure-setup.html.md +++ b/website/source/docs/builders/azure-setup.html.md @@ -46,7 +46,8 @@ There are three pieces of information you must provide to enable device login mo The device login flow asks that you open a web browser, navigate to , and input the supplied code. This authorizes the Packer for Azure application to act on your behalf. An OAuth token will be created, and stored in the user's home directory (~/.azure/packer/oauth-TenantID.json). This token is used if the token file exists, and it -is refreshed as necessary. The token file prevents the need to continually execute the device login flow. +is refreshed as necessary. The token file prevents the need to continually execute the device login flow. Packer will ask +for two device login auth, one for service management endpoint and another for accessing temp keyvault secrets that it creates. ## Install the Azure CLI From 1fdf763d0f5ddd6e2541a0b825dfce1ed13b61c2 Mon Sep 17 00:00:00 2001 From: Megan Marsh Date: Mon, 21 May 2018 11:25:51 -0700 Subject: [PATCH 54/68] fancier logging --- common/shell-local/communicator.go | 2 +- common/shell-local/run.go | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/common/shell-local/communicator.go b/common/shell-local/communicator.go index b51d309d9..4055c96b5 100644 --- a/common/shell-local/communicator.go +++ b/common/shell-local/communicator.go @@ -21,7 +21,7 @@ func (c *Communicator) Start(cmd *packer.RemoteCmd) error { } // Build the local command to execute - log.Printf("Executing local shell command %s", c.ExecuteCommand) + log.Printf("[INFO] (shell-local communicator): Executing local shell command %s", c.ExecuteCommand) localCmd := exec.Command(c.ExecuteCommand[0], c.ExecuteCommand[1:]...) localCmd.Stdin = cmd.Stdin localCmd.Stdout = cmd.Stdout diff --git a/common/shell-local/run.go b/common/shell-local/run.go index 6af406522..0457536fa 100644 --- a/common/shell-local/run.go +++ b/common/shell-local/run.go @@ -62,7 +62,7 @@ func Run(ui packer.Ui, config *Config) (bool, error) { // buffers and for reading the final exit status. flattenedCmd := strings.Join(interpolatedCmds, " ") cmd := &packer.RemoteCmd{Command: flattenedCmd} - log.Printf("starting local command: %s", flattenedCmd) + log.Printf("[INFO] (shell-local): starting local command: %s", flattenedCmd) if err := cmd.StartWithUi(comm, ui); err != nil { return false, fmt.Errorf( @@ -92,7 +92,7 @@ func createInlineScriptFile(config *Config) (string, error) { writer := bufio.NewWriter(tf) if config.InlineShebang != "" { shebang := fmt.Sprintf("#!%s\n", config.InlineShebang) - log.Printf("Prepending inline script with %s", shebang) + log.Printf("[INFO] (shell-local): Prepending inline script with %s", shebang) writer.WriteString(shebang) } for _, command := range config.Inline { @@ -108,7 +108,7 @@ func createInlineScriptFile(config *Config) (string, error) { tf.Close() err = os.Chmod(tf.Name(), 0555) if err != nil { - log.Printf("error modifying permissions of temp script file: %s", err.Error()) + log.Printf("[ERROR] (shell-local): error modifying permissions of temp script file: %s", err.Error()) } return tf.Name(), nil } From 969201a2d4e60e3d26e682355417c12e0484aa9f Mon Sep 17 00:00:00 2001 From: Megan Marsh Date: Mon, 21 May 2018 14:56:44 -0700 Subject: [PATCH 55/68] handle minor shell-local PR suggestions and corrections --- common/shell-local/config.go | 12 ++++++------ common/shell-local/run.go | 8 ++++---- .../source/docs/post-processors/shell-local.html.md | 2 +- website/source/docs/provisioners/shell-local.html.md | 2 +- 4 files changed, 12 insertions(+), 12 deletions(-) diff --git a/common/shell-local/config.go b/common/shell-local/config.go index 846e4b4a4..9eb657ff9 100644 --- a/common/shell-local/config.go +++ b/common/shell-local/config.go @@ -153,7 +153,11 @@ func Validate(config *Config) error { } if config.UseLinuxPathing { for index, script := range config.Scripts { - converted, err := ConvertToLinuxPath(script) + scriptAbsPath, err := filepath.Abs(script) + if err != nil { + return fmt.Errorf("Error converting %s to absolute path: %s", script, err.Error()) + } + converted, err := ConvertToLinuxPath(scriptAbsPath) if err != nil { return err } @@ -202,12 +206,8 @@ func Validate(config *Config) error { } // C:/path/to/your/file becomes /mnt/c/path/to/your/file -func ConvertToLinuxPath(winPath string) (string, error) { +func ConvertToLinuxPath(winAbsPath string) (string, error) { // get absolute path of script, and morph it into the bash path - winAbsPath, err := filepath.Abs(winPath) - if err != nil { - return "", fmt.Errorf("Error converting %s to absolute path: %s", winPath, err.Error()) - } winAbsPath = strings.Replace(winAbsPath, "\\", "/", -1) splitPath := strings.SplitN(winAbsPath, ":/", 2) winBashPath := fmt.Sprintf("/mnt/%s/%s", strings.ToLower(splitPath[0]), splitPath[1]) diff --git a/common/shell-local/run.go b/common/shell-local/run.go index 0457536fa..7ab93e346 100644 --- a/common/shell-local/run.go +++ b/common/shell-local/run.go @@ -32,11 +32,12 @@ func Run(ui packer.Ui, config *Config) (bool, error) { } scripts = append(scripts, tempScriptFileName) - defer os.Remove(tempScriptFileName) // figure out what extension the file should have, and rename it. if config.TempfileExtension != "" { os.Rename(tempScriptFileName, fmt.Sprintf("%s.%s", tempScriptFileName, config.TempfileExtension)) + tempScriptFileName = fmt.Sprintf("%s.%s", tempScriptFileName, config.TempfileExtension) } + defer os.Remove(tempScriptFileName) } // Create environment variables to set before executing the command @@ -83,7 +84,7 @@ func Run(ui packer.Ui, config *Config) (bool, error) { } func createInlineScriptFile(config *Config) (string, error) { - tf, err := ioutil.TempFile(os.TempDir(), "packer-shell") + tf, err := ioutil.TempFile("", "packer-shell") if err != nil { return "", fmt.Errorf("Error preparing shell script: %s", err) } @@ -105,8 +106,7 @@ func createInlineScriptFile(config *Config) (string, error) { return "", fmt.Errorf("Error preparing shell script: %s", err) } - tf.Close() - err = os.Chmod(tf.Name(), 0555) + err = os.Chmod(tf.Name(), 0700) if err != nil { log.Printf("[ERROR] (shell-local): error modifying permissions of temp script file: %s", err.Error()) } diff --git a/website/source/docs/post-processors/shell-local.html.md b/website/source/docs/post-processors/shell-local.html.md index 6812fac2b..ac8056407 100644 --- a/website/source/docs/post-processors/shell-local.html.md +++ b/website/source/docs/post-processors/shell-local.html.md @@ -73,7 +73,7 @@ Optional parameters: choose to try to use shell-local for Powershell or other Windows commands, the environment variables will not be set properly for your environment. - For backwards compatibility, `execute_command` will accept a string insetad + For backwards compatibility, `execute_command` will accept a string instead of an array of strings. If a single string or an array of strings with only one element is provided, Packer will replicate past behavior by appending your `execute_command` to the array of strings `["sh", "-c"]`. For example, diff --git a/website/source/docs/provisioners/shell-local.html.md b/website/source/docs/provisioners/shell-local.html.md index cadb1d6a1..a7400c589 100644 --- a/website/source/docs/provisioners/shell-local.html.md +++ b/website/source/docs/provisioners/shell-local.html.md @@ -89,7 +89,7 @@ Optional parameters: these commands are not officially supported and things like environment variables may not work if you use a different shell than the default. - For backwards compatability, you may also use {{.Command}}, but it is + For backwards compatibility, you may also use {{.Command}}, but it is decoded the same way as {{.Script}}. We recommend using {{.Script}} for the sake of clarity, as even when you set only a single `command` to run, Packer writes it to a temporary file and then runs it as a script. From d1e31c0f2360f8460aae16ed934ad7a35addb3db Mon Sep 17 00:00:00 2001 From: Megan Marsh Date: Mon, 21 May 2018 15:19:27 -0700 Subject: [PATCH 56/68] use if/else to clarify code --- common/shell-local/run.go | 10 ++++---- .../Azure/azure-sdk-for-go/storage/README.md | 1 + vendor/vendor.json | 24 +++++++++---------- 3 files changed, 18 insertions(+), 17 deletions(-) diff --git a/common/shell-local/run.go b/common/shell-local/run.go index 7ab93e346..b65196ea9 100644 --- a/common/shell-local/run.go +++ b/common/shell-local/run.go @@ -21,11 +21,11 @@ type ExecuteCommandTemplate struct { func Run(ui packer.Ui, config *Config) (bool, error) { scripts := make([]string, len(config.Scripts)) - copy(scripts, config.Scripts) - - // If we have an inline script, then turn that into a temporary - // shell script and use that. - if config.Inline != nil { + if len(config.Scripts) > 0 { + copy(scripts, config.Scripts) + } else if config.Inline != nil { + // If we have an inline script, then turn that into a temporary + // shell script and use that. tempScriptFileName, err := createInlineScriptFile(config) if err != nil { return false, err diff --git a/vendor/github.com/Azure/azure-sdk-for-go/storage/README.md b/vendor/github.com/Azure/azure-sdk-for-go/storage/README.md index ed90cf8bc..49e48cdf1 100644 --- a/vendor/github.com/Azure/azure-sdk-for-go/storage/README.md +++ b/vendor/github.com/Azure/azure-sdk-for-go/storage/README.md @@ -15,3 +15,4 @@ at [github.com/Azure/azure-sdk-for-go/services/storage](https://github.com/Azure This package also supports the [Azure Storage Emulator](https://azure.microsoft.com/documentation/articles/storage-use-emulator/) (Windows only). + diff --git a/vendor/vendor.json b/vendor/vendor.json index a979c6b6f..1f08d096b 100644 --- a/vendor/vendor.json +++ b/vendor/vendor.json @@ -9,7 +9,7 @@ "revisionTime": "2016-08-11T22:04:02Z" }, { - "checksumSHA1": "XZVCJXyy79hy5KBOI6flZ6iHnHY=", + "checksumSHA1": "cJxhrzJRtddboU3S0TPyvEPBqsc=", "path": "github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2018-04-01/compute", "revision": "56332fec5b308fbb6615fa1af6117394cdba186d", "revisionTime": "2018-03-26T23:29:47Z", @@ -17,7 +17,7 @@ "versionExact": "v15.0.0" }, { - "checksumSHA1": "738URn/O+S8TN9psssjK7cteZXA=", + "checksumSHA1": "VDwUBYd9RVKy09Y17al0EQ7ivYI=", "path": "github.com/Azure/azure-sdk-for-go/services/network/mgmt/2018-01-01/network", "revision": "56332fec5b308fbb6615fa1af6117394cdba186d", "revisionTime": "2018-03-26T23:29:47Z", @@ -25,7 +25,7 @@ "versionExact": "v15.0.0" }, { - "checksumSHA1": "KDrlouaRfBHk+qH/yljC0JnsV4Y=", + "checksumSHA1": "woz67BK+/NdoZm4GzVYnJwzl61A=", "path": "github.com/Azure/azure-sdk-for-go/services/resources/mgmt/2016-06-01/subscriptions", "revision": "56332fec5b308fbb6615fa1af6117394cdba186d", "revisionTime": "2018-03-26T23:29:47Z", @@ -33,7 +33,7 @@ "versionExact": "v15.0.0" }, { - "checksumSHA1": "BMd5SfQ0KfqEUvi9zAt+QAB/JPQ=", + "checksumSHA1": "1W8UIxg6Rycuzg41FQFu35vkCEU=", "path": "github.com/Azure/azure-sdk-for-go/services/resources/mgmt/2018-02-01/resources", "revision": "56332fec5b308fbb6615fa1af6117394cdba186d", "revisionTime": "2018-03-26T23:29:47Z", @@ -41,7 +41,7 @@ "versionExact": "v15.0.0" }, { - "checksumSHA1": "qHMzicMTsihjgKyS/VB8oguXmmc=", + "checksumSHA1": "g9eP5AgV9yXRkY36M8h7aDW9oi8=", "path": "github.com/Azure/azure-sdk-for-go/services/storage/mgmt/2017-10-01/storage", "revision": "56332fec5b308fbb6615fa1af6117394cdba186d", "revisionTime": "2018-03-26T23:29:47Z", @@ -49,7 +49,7 @@ "versionExact": "v15.0.0" }, { - "checksumSHA1": "s/831Hsxh0h6PCHCoMOiOdh1Hwg=", + "checksumSHA1": "3N5Et8QnWsHJYN+v/0J/VSQUkJ0=", "path": "github.com/Azure/azure-sdk-for-go/storage", "revision": "56332fec5b308fbb6615fa1af6117394cdba186d", "revisionTime": "2018-03-26T23:29:47Z", @@ -57,13 +57,13 @@ "versionExact": "v15.0.0" }, { - "checksumSHA1": "kbpNrLhdZinIK0H1vsJh7eSB2JM=", + "checksumSHA1": "Fb2OanEbwZVaGHYLf9Y4FAajsOM=", "path": "github.com/Azure/azure-sdk-for-go/version", "revision": "56332fec5b308fbb6615fa1af6117394cdba186d", "revisionTime": "2018-03-26T23:29:47Z" }, { - "checksumSHA1": "LaWzRZq1p8T0iqZTD4+QL7qlJPg=", + "checksumSHA1": "+P6HOINDh/n2z4GqEkluzuGP5p0=", "comment": "v7.0.7", "path": "github.com/Azure/go-autorest/autorest", "revision": "ed4b7f5bf1ec0c9ede1fda2681d96771282f2862", @@ -72,7 +72,7 @@ "versionExact": "v10.4.0" }, { - "checksumSHA1": "HzA52MbMWnsR31CFrub5biN90/Q=", + "checksumSHA1": "4Z3yO++uYspufDkuaIydTpT787c=", "path": "github.com/Azure/go-autorest/autorest/adal", "revision": "ed4b7f5bf1ec0c9ede1fda2681d96771282f2862", "revisionTime": "2018-03-26T17:06:54Z", @@ -80,7 +80,7 @@ "versionExact": "v10.4.0" }, { - "checksumSHA1": "5698vgeScEFD2bOOCssAfMFP4Mg=", + "checksumSHA1": "bDFbLGwpCT8TRmqEKtPY/U1DAY8=", "comment": "v7.0.7", "path": "github.com/Azure/go-autorest/autorest/azure", "revision": "ed4b7f5bf1ec0c9ede1fda2681d96771282f2862", @@ -107,7 +107,7 @@ "versionExact": "v8.0.0" }, { - "checksumSHA1": "CdDkG+J8wqXQVQ0f0xal+eolB1w=", + "checksumSHA1": "5UH4IFIB/98iowPCzzVs4M4MXiQ=", "path": "github.com/Azure/go-autorest/autorest/validation", "revision": "ed4b7f5bf1ec0c9ede1fda2681d96771282f2862", "revisionTime": "2018-03-26T17:06:54Z", @@ -965,7 +965,7 @@ "revision": "2788f0dbd16903de03cb8186e5c7d97b69ad387b" }, { - "checksumSHA1": "9Ok54so+GJLC4rMpb7XqZzlfieI=", + "checksumSHA1": "T9E+5mKBQ/BX4wlNxgaPfetxdeI=", "path": "github.com/marstr/guid", "revision": "8bdf7d1a087ccc975cf37dd6507da50698fd19ca", "revisionTime": "2017-04-27T23:51:15Z" From 7e9a653da7d565a51ee11e20c83e7b6f016fbd7c Mon Sep 17 00:00:00 2001 From: Megan Marsh Date: Mon, 21 May 2018 15:26:57 -0700 Subject: [PATCH 57/68] use testify instead of homegrown string compare --- .../shell-local/post-processor_test.go | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/post-processor/shell-local/post-processor_test.go b/post-processor/shell-local/post-processor_test.go index ee7e27d70..515704f9d 100644 --- a/post-processor/shell-local/post-processor_test.go +++ b/post-processor/shell-local/post-processor_test.go @@ -4,10 +4,10 @@ import ( "io/ioutil" "os" "runtime" - "strings" "testing" "github.com/hashicorp/packer/packer" + "github.com/stretchr/testify/assert" ) func TestPostProcessor_ImplementsPostProcessor(t *testing.T) { @@ -116,9 +116,8 @@ func TestPostProcessorPrepare_ExecuteCommand(t *testing.T) { if err != nil { t.Fatalf("should handle backwards compatibility: %s", err) } - if strings.Compare(strings.Join(p.config.ExecuteCommand, " "), strings.Join(expected, " ")) != 0 { - t.Fatalf("Did not get expected execute_command: expected: %#v; received %#v", expected, p.config.ExecuteCommand) - } + assert.Equal(t, p.config.ExecuteCommand, expected, + "Did not get expected execute_command: expected: %#v; received %#v", expected, p.config.ExecuteCommand) // Check that passing a list will work p = new(PostProcessor) @@ -129,9 +128,8 @@ func TestPostProcessorPrepare_ExecuteCommand(t *testing.T) { t.Fatalf("should handle backwards compatibility: %s", err) } expected = []string{"foo", "bar"} - if strings.Compare(strings.Join(p.config.ExecuteCommand, " "), strings.Join(expected, " ")) != 0 { - t.Fatalf("Did not get expected execute_command: expected: %#v; received %#v", expected, p.config.ExecuteCommand) - } + assert.Equal(t, p.config.ExecuteCommand, expected, + "Did not get expected execute_command: expected: %#v; received %#v", expected, p.config.ExecuteCommand) // Check that default is as expected raws = testConfig() @@ -139,13 +137,12 @@ func TestPostProcessorPrepare_ExecuteCommand(t *testing.T) { p = new(PostProcessor) p.Configure(raws) if runtime.GOOS != "windows" { - expected = []string{"/bin/sh", "-c", "{{.Vars}}", "{{.Script}}"} + expected = []string{"/bin/sh", "-c", "{{.Vars}} {{.Script}}"} } else { expected = []string{"cmd", "/V", "/C", "{{.Vars}}", "call", "{{.Script}}"} } - if strings.Compare(strings.Join(p.config.ExecuteCommand, " "), strings.Join(expected, " ")) != 0 { - t.Fatalf("Did not get expected default: expected: %#v; received %#v", expected, p.config.ExecuteCommand) - } + assert.Equal(t, p.config.ExecuteCommand, expected, + "Did not get expected default: expected: %#v; received %#v", expected, p.config.ExecuteCommand) } func TestPostProcessorPrepare_ScriptAndInline(t *testing.T) { From 1bd7aa534e55aa21d1cd79d0e5b8533b15b8b29a Mon Sep 17 00:00:00 2001 From: Hariharan Jayaraman Date: Mon, 21 May 2018 21:38:41 -0700 Subject: [PATCH 58/68] Addressed PR feedback --- builder/azure/arm/builder.go | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/builder/azure/arm/builder.go b/builder/azure/arm/builder.go index eecef1b19..bb67e5622 100644 --- a/builder/azure/arm/builder.go +++ b/builder/azure/arm/builder.go @@ -91,11 +91,14 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe if err := resolver.Resolve(b.config); err != nil { return nil, err } - - b.config.ObjectID = getObjectIdFromToken(spnCloud) + if b.config.ObjectID == "" { + b.config.ObjectID = getObjectIdFromToken(spnCloud) + } else { + ui.Message("You have provided Object_ID which is no longer needed, azure packer builder determines this dynamically from the authentication token") + } if b.config.ObjectID == "" && b.config.OSType != constants.Target_Linux { - return nil, fmt.Errorf("could not determined the ObjectID for the user, which is required for Windows builds") + return nil, fmt.Errorf("could not determine the ObjectID for the user, which is required for Windows builds") } if b.config.isManagedImage() { From a13a2511f986bddd4bdefc04e3a7555bf6faf831 Mon Sep 17 00:00:00 2001 From: Hariharan Jayaraman Date: Mon, 21 May 2018 22:20:36 -0700 Subject: [PATCH 59/68] Added additional error message if we failed to parse token --- builder/azure/arm/builder.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/builder/azure/arm/builder.go b/builder/azure/arm/builder.go index bb67e5622..23d3d2181 100644 --- a/builder/azure/arm/builder.go +++ b/builder/azure/arm/builder.go @@ -92,7 +92,7 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe return nil, err } if b.config.ObjectID == "" { - b.config.ObjectID = getObjectIdFromToken(spnCloud) + b.config.ObjectID = getObjectIdFromToken(ui, spnCloud) } else { ui.Message("You have provided Object_ID which is no longer needed, azure packer builder determines this dynamically from the authentication token") } @@ -423,7 +423,7 @@ func (b *Builder) getServicePrincipalTokens(say func(string)) (*adal.ServicePrin return servicePrincipalToken, servicePrincipalTokenVault, nil } -func getObjectIdFromToken(token *adal.ServicePrincipalToken) string { +func getObjectIdFromToken(ui packer.Ui, token *adal.ServicePrincipalToken) string { claims := jwt.MapClaims{} var p jwt.Parser @@ -432,6 +432,7 @@ func getObjectIdFromToken(token *adal.ServicePrincipalToken) string { _, _, err = p.ParseUnverified(token.OAuthToken(), claims) if err != nil { + ui.Error(fmt.Sprintf("Failed to parse the token,Error: %s", err.Error())) return "" } return claims["oid"].(string) From 97652b62e538a30d5a190948316521f6874cbcfe Mon Sep 17 00:00:00 2001 From: Paul Meyer Date: Tue, 22 May 2018 11:28:00 -0700 Subject: [PATCH 60/68] Update CHANGELOG.md --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 57cd1230d..8fde73f0c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ ### IMPROVEMENTS: * builder/azure: Updated Azure SDK to v15.0.0 [GH-6224] +* builder/azure: Devicelogin Support for Windows [GH-6285] ## 1.2.3 (April 25, 2018) From e670eed315a628ed089db3f42c822e11e0bea698 Mon Sep 17 00:00:00 2001 From: Megan Marsh Date: Wed, 25 Apr 2018 11:58:04 -0700 Subject: [PATCH 61/68] Add new option, nvme_device_path, so that we can properly mount nvme block devices. --- builder/amazon/chroot/builder.go | 1 + builder/amazon/chroot/step_mount_device.go | 7 ++++++- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/builder/amazon/chroot/builder.go b/builder/amazon/chroot/builder.go index 02923ce31..a8b20276a 100644 --- a/builder/amazon/chroot/builder.go +++ b/builder/amazon/chroot/builder.go @@ -33,6 +33,7 @@ type Config struct { CommandWrapper string `mapstructure:"command_wrapper"` CopyFiles []string `mapstructure:"copy_files"` DevicePath string `mapstructure:"device_path"` + NVMEDevicePath string `mapstructure:"nvme_device_path"` FromScratch bool `mapstructure:"from_scratch"` MountOptions []string `mapstructure:"mount_options"` MountPartition string `mapstructure:"mount_partition"` diff --git a/builder/amazon/chroot/step_mount_device.go b/builder/amazon/chroot/step_mount_device.go index c05ae2e77..38ec62164 100644 --- a/builder/amazon/chroot/step_mount_device.go +++ b/builder/amazon/chroot/step_mount_device.go @@ -35,6 +35,10 @@ func (s *StepMountDevice) Run(_ context.Context, state multistep.StateBag) multi config := state.Get("config").(*Config) ui := state.Get("ui").(packer.Ui) device := state.Get("device").(string) + if config.NVMEDevicePath != "" { + // customizable device path for mounting NVME block devices on c5 and m5 HVM + device = config.NVMEDevicePath + } wrappedCommand := state.Get("wrappedCommand").(CommandWrapper) var virtualizationType string @@ -47,6 +51,7 @@ func (s *StepMountDevice) Run(_ context.Context, state multistep.StateBag) multi } ctx := config.ctx + ctx.Data = &mountPathData{Device: filepath.Base(device)} mountPath, err := interpolate.Render(config.MountPath, &ctx) @@ -98,7 +103,7 @@ func (s *StepMountDevice) Run(_ context.Context, state multistep.StateBag) multi ui.Error(err.Error()) return multistep.ActionHalt } - + log.Printf("[DEBUG] (step mount) mount command is %s", mountCommand) cmd := ShellCommand(mountCommand) cmd.Stderr = stderr if err := cmd.Run(); err != nil { From b5095539a7e85b9ebc4f0cf16f0654fc8690e954 Mon Sep 17 00:00:00 2001 From: Megan Marsh Date: Wed, 23 May 2018 09:58:15 -0700 Subject: [PATCH 62/68] add docs --- .../docs/builders/amazon-chroot.html.md | 58 +++++++++++++++++++ 1 file changed, 58 insertions(+) diff --git a/website/source/docs/builders/amazon-chroot.html.md b/website/source/docs/builders/amazon-chroot.html.md index da1752144..ce03970ee 100644 --- a/website/source/docs/builders/amazon-chroot.html.md +++ b/website/source/docs/builders/amazon-chroot.html.md @@ -225,6 +225,15 @@ each category, the available configuration keys are alphabetized. command](http://linuxcommand.org/man_pages/mount8.html) for valid file system specific options +- `nvme_device_path` (string) - When we call the mount command (by default + `mount -o device dir`), the string provided in `nvme_mount_path` will + replace `device` in that command. When this option is not set, `device` in + that command will be something like `/dev/sdf1`, mirroring the attached + device name. This assumption works for most instances but will fail with c5 + and m5 instances. In order to use the chroot builder with c5 and m5 + instances, you must manually set `nvme_device_path`, `device_path`, and + `mount_path`. + - `pre_mount_commands` (array of strings) - A series of commands to execute after attaching the root volume and before mounting the chroot. This is not required unless using `from_scratch`. If so, this should include any @@ -370,6 +379,7 @@ its internals such as finding an available device. ## Gotchas +### Unmounting the Filesystem One of the difficulties with using the chroot builder is that your provisioning scripts must not leave any processes running or packer will be unable to unmount the filesystem. @@ -399,6 +409,54 @@ services: } ``` +### Using Instances with NVMe block devices. +In C5, C5d, M5, and i3.metal instances, EBS volumes are exposed as NVMe block +devices [reference](https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/nvme-ebs-volumes.html). +In order to correctly mount these devices, you have to do some extra legwork, +involving the `nvme_device_path` option above. Read that for more information. + +A working example for mounting an NVMe device is below: + +``` +{ + "variables": { + "region" : "us-east-2" + }, + "builders": [ + { + "type": "amazon-chroot", + "region": "{{user `region`}}", + "source_ami_filter": { + "filters": { + "virtualization-type": "hvm", + "name": "amzn-ami-hvm-*", + "root-device-type": "ebs" + }, + "owners": ["137112412989"], + "most_recent": true + }, + "ena_support": true, + "ami_name": "amazon-chroot-test-{{timestamp}}", + "mount_path": "/mnt/my/mount/path/", + "nvme_device_path": "/dev/nvme1n1p", + "device_path": "/dev/sdf" + } + ], + + "provisioners": [ + { + "type": "shell", + "inline": ["echo Test > /tmp/test.txt"] + } + ] +} +``` + +Note that in the `nvme_device_path` you must end with the `p`; if you try to +define the partition in this path (e.g. "nvme_device_path": `/dev/nvme1n1p1`) +and haven't also set the `"mount_partition": 0`, a `1` will be appended to the +`nvme_device_path` and Packer will fail. + ## Building From Scratch This example demonstrates the essentials of building an image from scratch. A From 974e464f356b0446f59878b476db47a78a704063 Mon Sep 17 00:00:00 2001 From: Megan Marsh Date: Wed, 23 May 2018 13:34:56 -0700 Subject: [PATCH 63/68] fix docs becuase we dont need to actually set the mount path --- website/source/docs/builders/amazon-chroot.html.md | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/website/source/docs/builders/amazon-chroot.html.md b/website/source/docs/builders/amazon-chroot.html.md index ce03970ee..9c16fc320 100644 --- a/website/source/docs/builders/amazon-chroot.html.md +++ b/website/source/docs/builders/amazon-chroot.html.md @@ -231,8 +231,7 @@ each category, the available configuration keys are alphabetized. that command will be something like `/dev/sdf1`, mirroring the attached device name. This assumption works for most instances but will fail with c5 and m5 instances. In order to use the chroot builder with c5 and m5 - instances, you must manually set `nvme_device_path`, `device_path`, and - `mount_path`. + instances, you must manually set `nvme_device_path` and `device_path`. - `pre_mount_commands` (array of strings) - A series of commands to execute after attaching the root volume and before mounting the chroot. This is not @@ -437,7 +436,6 @@ A working example for mounting an NVMe device is below: }, "ena_support": true, "ami_name": "amazon-chroot-test-{{timestamp}}", - "mount_path": "/mnt/my/mount/path/", "nvme_device_path": "/dev/nvme1n1p", "device_path": "/dev/sdf" } From 0a8f7f28a0de31556ede83894ff1b11c672f9b63 Mon Sep 17 00:00:00 2001 From: Matthew Hooker Date: Wed, 23 May 2018 15:24:18 -0700 Subject: [PATCH 64/68] add note about key interval to virtualbox docs --- website/source/docs/builders/virtualbox-iso.html.md.erb | 5 ++++- website/source/docs/builders/virtualbox-ovf.html.md.erb | 5 ++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/website/source/docs/builders/virtualbox-iso.html.md.erb b/website/source/docs/builders/virtualbox-iso.html.md.erb index 78de79af8..788f5e709 100644 --- a/website/source/docs/builders/virtualbox-iso.html.md.erb +++ b/website/source/docs/builders/virtualbox-iso.html.md.erb @@ -335,7 +335,10 @@ all typed in sequence. It is an array only to improve readability within the template. The boot command is sent to the VM through the `VBoxManage` utility in as few -invocations as possible. +invocations as possible. We send each character in groups of 25, with a default +delay of 100ms between groups. The delay alleviates issues with latency and CPU +contention. If you notice missing keys, you can tune this delay by specifying e.g. +`PACKER_KEY_INTERVAL=500ms` to wait longer between each group of characters. <%= partial "partials/builders/boot-command" %> diff --git a/website/source/docs/builders/virtualbox-ovf.html.md.erb b/website/source/docs/builders/virtualbox-ovf.html.md.erb index 89edcd89f..ae4317240 100644 --- a/website/source/docs/builders/virtualbox-ovf.html.md.erb +++ b/website/source/docs/builders/virtualbox-ovf.html.md.erb @@ -298,7 +298,10 @@ all typed in sequence. It is an array only to improve readability within the template. The boot command is sent to the VM through the `VBoxManage` utility in as few -invocations as possible. +invocations as possible. We send each character in groups of 25, with a default +delay of 100ms between groups. The delay alleviates issues with latency and CPU +contention. If you notice missing keys, you can tune this delay by specifying e.g. +`PACKER_KEY_INTERVAL=500ms` to wait longer between each group of characters. <%= partial "partials/builders/boot-command" %> From af7f7f2ce9149e2d7b853e6d91547316c959f92b Mon Sep 17 00:00:00 2001 From: DanHam Date: Thu, 24 May 2018 21:05:03 +0100 Subject: [PATCH 65/68] Fix redundant Sprintf --- builder/amazon/common/run_config.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/builder/amazon/common/run_config.go b/builder/amazon/common/run_config.go index bc596e580..129d4d541 100644 --- a/builder/amazon/common/run_config.go +++ b/builder/amazon/common/run_config.go @@ -85,7 +85,7 @@ func (c *RunConfig) Prepare(ctx *interpolate.Context) []error { c.SSHInterface != "public_dns" && c.SSHInterface != "private_dns" && c.SSHInterface != "" { - errs = append(errs, fmt.Errorf(fmt.Sprintf("Unknown interface type: %s", c.SSHInterface))) + errs = append(errs, fmt.Errorf("Unknown interface type: %s", c.SSHInterface)) } if c.SSHKeyPairName != "" { From 494e8f9d82d2640a6ad2c4b725d8e602d407a3d4 Mon Sep 17 00:00:00 2001 From: James Nugent Date: Thu, 24 May 2018 21:27:24 +0100 Subject: [PATCH 66/68] Update CHANGELOG.md --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8fde73f0c..be9b5ceff 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ ### IMPROVEMENTS: +* builder/amazon: Amazon builders other than `chroot` now support T2 unlimited instances [GH-6265] * builder/azure: Updated Azure SDK to v15.0.0 [GH-6224] * builder/azure: Devicelogin Support for Windows [GH-6285] From cd6390ca171db4598764042c5a5c54e9f4a673af Mon Sep 17 00:00:00 2001 From: Megan Marsh Date: Thu, 24 May 2018 17:09:17 -0700 Subject: [PATCH 67/68] update changelog pre- 1.2.4 release --- CHANGELOG.md | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index be9b5ceff..abfd31afb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,12 +4,34 @@ * builder/vmware-esxi: Remove floppy files from the remote server on cleanup. [GH-6206] * core: When using `-on-error=[abort|ask]`, output the error to the user. [GH-6252] +* builder/amazon: Can now force the chroot builder to mount an entire block device instead of a partition [GH-6194] +* builder/chroot: A new template option, `nvme_device_path` has been added to provide a workaround for users who need the amazon-chroot builder to mount a NVMe volume on their instances. [GH-6295] +* communicator/winrm: Updated dependencies to fix a race condition [GH-6261] +* builder/hyper-v: Fix command for mounting multiple disks [GH-6267] +* provisioner/shell: Remove file stat that was causing problems uploading files [GH-6239] +* provisioner/puppet: Extra-Arguments are no longer prematurely interpolated.[GH-6215] +* builder/azure: windows-sql-cloud is now in the default list of projects to check for provided images. [GH-6210] +* builder/hyperv: Enable IP retrieval for Server 2008 R2 hosts. [GH-6219] +* builder/hyperv: Fix bug in MAC address specification on Hyper-V. [GH-6187] +* builder/parallels-pvm: Add missing disk compaction step. [GH-6202] ### IMPROVEMENTS: * builder/amazon: Amazon builders other than `chroot` now support T2 unlimited instances [GH-6265] * builder/azure: Updated Azure SDK to v15.0.0 [GH-6224] * builder/azure: Devicelogin Support for Windows [GH-6285] +* builder/hyper-v: Hyper-V builds now connect to vnc display by default when building [GH-6243] +* provisoner/shell-local: New options have been added to create feature parity with the shell-local post-processor. This feature now works on Windows hosts. [GH-5956] +* post-processor/shell-local: New options have been added to create feature parity with the shell-local provisioner. This feature now works on Windows hosts. [GH-5956] +* builder/hyper-v: New `use_fixed_vhd_format` allows vm export in an Azure-compatible format [GH-6101] +* builder/azure: Faster deletion of Azure Resource Groups. [GH-6269] +* builder/hyperv: New config option for specifying what secure boot template to use, allowing secure boot of linux vms. [GH-5883] +* provisioner/chef: New config option allows user to skip cleanup of chef client staging directory. [GH-4300] +* builder/azure: Allow device login for US government cloud. [GH-6105] +* builder/qemu: Add support for hvf accelerator. [GH-6193] +* builder/azure: Enable simultaneous builds within one resource group. [GH-6231] +* builder/scaleway: Fix SSH communicator connection issue. [GH-6238] +* core: Add opt-in Packer top-level command autocomplete [GH-5454] ## 1.2.3 (April 25, 2018) From 788418cff20a047a9f0124a02d465c8d2eeda2c7 Mon Sep 17 00:00:00 2001 From: Matthew Hooker Date: Fri, 25 May 2018 03:42:40 -0700 Subject: [PATCH 68/68] Add unit test to show that we handle tars safely --- .../decompress-tar/outside_parent.tar | Bin 0 -> 10240 bytes post-processor/vagrant/virtualbox.go | 8 ++++++++ post-processor/vagrant/virtualbox_test.go | 18 ++++++++++++++++++ 3 files changed, 26 insertions(+) create mode 100644 common/test-fixtures/decompress-tar/outside_parent.tar diff --git a/common/test-fixtures/decompress-tar/outside_parent.tar b/common/test-fixtures/decompress-tar/outside_parent.tar new file mode 100644 index 0000000000000000000000000000000000000000..f08df1e6560b1137d6103ab0968518339bb3566c GIT binary patch literal 10240 zcmeIxOAdlC5P;#XIYo_oC@t12cmvnSBanbKg7>#*OppZ|Bq8RX&9t;>r(c*bjPjy& z;mu`Ytx!I!QewzQdw6A~lXo6I8Y5z(4^|s(wG}E({6-