From a0edaf6c4648841cacfadb0e82f409c7ca319b3b Mon Sep 17 00:00:00 2001 From: Megan Marsh Date: Mon, 10 Sep 2018 16:48:42 -0700 Subject: [PATCH] Going to revert this change for now, becuase of potential issues that arise from calling Prepare() twice Revert "use statebag instead of SetSharedState for winRM password" This reverts commit b35acbd8798a1baa645f0d181731a9cd9318a61c. --- builder/amazon/common/step_get_password.go | 5 +- builder/azure/arm/config.go | 4 ++ builder/azure/arm/step_save_winrm_password.go | 4 +- .../step_create_windows_password.go | 2 + .../oci/step_get_default_credentials.go | 4 +- common/shell-local/config.go | 3 -- common/shell-local/run.go | 14 ++++-- common/step_provision.go | 15 +----- packer/provisioner.go | 13 +----- packer/provisioner_mock.go | 6 +-- provisioner/ansible/provisioner.go | 46 +++++++------------ provisioner/ansible/provisioner_test.go | 4 +- provisioner/powershell/provisioner.go | 46 ++++++------------- provisioner/powershell/provisioner_test.go | 4 +- provisioner/shell-local/provisioner.go | 21 --------- provisioner/shell-local/provisioner_test.go | 4 +- 16 files changed, 67 insertions(+), 128 deletions(-) diff --git a/builder/amazon/common/step_get_password.go b/builder/amazon/common/step_get_password.go index 291e97878..1810cd506 100644 --- a/builder/amazon/common/step_get_password.go +++ b/builder/amazon/common/step_get_password.go @@ -12,6 +12,7 @@ import ( "time" "github.com/aws/aws-sdk-go/service/ec2" + commonhelper "github.com/hashicorp/packer/helper/common" "github.com/hashicorp/packer/helper/communicator" "github.com/hashicorp/packer/helper/multistep" "github.com/hashicorp/packer/packer" @@ -94,13 +95,15 @@ WaitLoop: "Password (since debug is enabled): %s", s.Comm.WinRMPassword)) } // store so that we can access this later during provisioning - state.Put("winrm_password", s.Comm.WinRMPassword) + + commonhelper.SetSharedState("winrm_password", s.Comm.WinRMPassword, s.BuildName) packer.LogSecretFilter.Set(s.Comm.WinRMPassword) return multistep.ActionContinue } func (s *StepGetPassword) Cleanup(multistep.StateBag) { + commonhelper.RemoveSharedStateFile("winrm_password", s.BuildName) } func (s *StepGetPassword) waitForPassword(state multistep.StateBag, cancel <-chan struct{}) (string, error) { diff --git a/builder/azure/arm/config.go b/builder/azure/arm/config.go index eb8142065..17a242338 100644 --- a/builder/azure/arm/config.go +++ b/builder/azure/arm/config.go @@ -22,6 +22,7 @@ import ( "github.com/hashicorp/packer/builder/azure/common/constants" "github.com/hashicorp/packer/builder/azure/pkcs12" "github.com/hashicorp/packer/common" + commonhelper "github.com/hashicorp/packer/helper/common" "github.com/hashicorp/packer/helper/communicator" "github.com/hashicorp/packer/helper/config" "github.com/hashicorp/packer/packer" @@ -359,7 +360,10 @@ func setRuntimeValues(c *Config) { var tempName = NewTempName() c.tmpAdminPassword = tempName.AdminPassword + // store so that we can access this later during provisioning + commonhelper.SetSharedState("winrm_password", c.tmpAdminPassword, c.PackerConfig.PackerBuildName) packer.LogSecretFilter.Set(c.tmpAdminPassword) + c.tmpCertificatePassword = tempName.CertificatePassword if c.TempComputeName == "" { c.tmpComputeName = tempName.ComputeName diff --git a/builder/azure/arm/step_save_winrm_password.go b/builder/azure/arm/step_save_winrm_password.go index ea2026d7a..3afac5ccf 100644 --- a/builder/azure/arm/step_save_winrm_password.go +++ b/builder/azure/arm/step_save_winrm_password.go @@ -3,6 +3,7 @@ package arm import ( "context" + commonhelper "github.com/hashicorp/packer/helper/common" "github.com/hashicorp/packer/helper/multistep" "github.com/hashicorp/packer/packer" ) @@ -14,10 +15,11 @@ type StepSaveWinRMPassword struct { func (s *StepSaveWinRMPassword) Run(_ context.Context, state multistep.StateBag) multistep.StepAction { // store so that we can access this later during provisioning - state.Put("winrm_password", s.Password) + commonhelper.SetSharedState("winrm_password", s.Password, s.BuildName) packer.LogSecretFilter.Set(s.Password) return multistep.ActionContinue } func (s *StepSaveWinRMPassword) Cleanup(multistep.StateBag) { + commonhelper.RemoveSharedStateFile("winrm_password", s.BuildName) } diff --git a/builder/googlecompute/step_create_windows_password.go b/builder/googlecompute/step_create_windows_password.go index d3a2e26a9..2b85aa2ed 100644 --- a/builder/googlecompute/step_create_windows_password.go +++ b/builder/googlecompute/step_create_windows_password.go @@ -13,6 +13,7 @@ import ( "os" "time" + commonhelper "github.com/hashicorp/packer/helper/common" "github.com/hashicorp/packer/helper/multistep" "github.com/hashicorp/packer/packer" ) @@ -113,6 +114,7 @@ func (s *StepCreateWindowsPassword) Run(_ context.Context, state multistep.State } state.Put("winrm_password", data.password) + commonhelper.SetSharedState("winrm_password", data.password, c.PackerConfig.PackerBuildName) packer.LogSecretFilter.Set(data.password) return multistep.ActionContinue diff --git a/builder/oracle/oci/step_get_default_credentials.go b/builder/oracle/oci/step_get_default_credentials.go index ffc52f207..1f4ea20b3 100644 --- a/builder/oracle/oci/step_get_default_credentials.go +++ b/builder/oracle/oci/step_get_default_credentials.go @@ -5,6 +5,7 @@ import ( "fmt" "log" + commonhelper "github.com/hashicorp/packer/helper/common" "github.com/hashicorp/packer/helper/communicator" "github.com/hashicorp/packer/helper/multistep" "github.com/hashicorp/packer/packer" @@ -51,9 +52,8 @@ func (s *stepGetDefaultCredentials) Run(ctx context.Context, state multistep.Sta } // store so that we can access this later during provisioning - state.Put("winrm_password", s.Comm.WinRMPassword) + commonhelper.SetSharedState("winrm_password", s.Comm.WinRMPassword, s.BuildName) packer.LogSecretFilter.Set(s.Comm.WinRMPassword) - return multistep.ActionContinue } diff --git a/common/shell-local/config.go b/common/shell-local/config.go index 845f7856f..56f9e05cd 100644 --- a/common/shell-local/config.go +++ b/common/shell-local/config.go @@ -53,9 +53,6 @@ type Config struct { UseLinuxPathing bool `mapstructure:"use_linux_pathing"` Ctx interpolate.Context - - // internal use only; for the provisioner. - WinRMPassword string } func Decode(config *Config, raws ...interface{}) error { diff --git a/common/shell-local/run.go b/common/shell-local/run.go index a2bae0afc..c8f6160fc 100644 --- a/common/shell-local/run.go +++ b/common/shell-local/run.go @@ -10,6 +10,7 @@ import ( "strings" "github.com/hashicorp/packer/common" + commonhelper "github.com/hashicorp/packer/helper/common" "github.com/hashicorp/packer/packer" "github.com/hashicorp/packer/template/interpolate" ) @@ -69,7 +70,6 @@ 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} - packer.LogSecretFilter.Set(config.WinRMPassword) log.Printf("[INFO] (shell-local): starting local command: %s", flattenedCmd) if err := cmd.StartWithUi(comm, ui); err != nil { return false, fmt.Errorf( @@ -105,7 +105,7 @@ func createInlineScriptFile(config *Config) (string, error) { // generate context so you can interpolate the command config.Ctx.Data = &EnvVarsTemplate{ - WinRMPassword: config.WinRMPassword, + WinRMPassword: getWinRMPassword(config.PackerBuildName), } for _, command := range config.Inline { @@ -139,7 +139,7 @@ func createInterpolatedCommands(config *Config, script string, flattenedEnvVars Vars: flattenedEnvVars, Script: script, Command: script, - WinRMPassword: config.WinRMPassword, + WinRMPassword: getWinRMPassword(config.PackerBuildName), } interpolatedCmds := make([]string, len(config.ExecuteCommand)) @@ -169,7 +169,7 @@ func createFlattenedEnvVars(config *Config) (string, error) { // interpolate environment variables config.Ctx.Data = &EnvVarsTemplate{ - WinRMPassword: config.WinRMPassword, + WinRMPassword: getWinRMPassword(config.PackerBuildName), } // Split vars into key/value components for _, envVar := range config.Vars { @@ -196,3 +196,9 @@ func createFlattenedEnvVars(config *Config) (string, error) { } return flattened, nil } + +func getWinRMPassword(buildName string) string { + winRMPass, _ := commonhelper.RetrieveSharedState("winrm_password", buildName) + packer.LogSecretFilter.Set(winRMPass) + return winRMPass +} diff --git a/common/step_provision.go b/common/step_provision.go index 4fe6f93a9..9c842044b 100644 --- a/common/step_provision.go +++ b/common/step_provision.go @@ -22,10 +22,6 @@ type StepProvision struct { Comm packer.Communicator } -type ProvisionHookData struct { - WinRMPassword string -} - func (s *StepProvision) Run(_ context.Context, state multistep.StateBag) multistep.StepAction { comm := s.Comm if comm == nil { @@ -37,21 +33,12 @@ func (s *StepProvision) Run(_ context.Context, state multistep.StateBag) multist hook := state.Get("hook").(packer.Hook) ui := state.Get("ui").(packer.Ui) - // Save data we need to give to provisioners - WinRMPassword, ok := state.GetOk("winrm_password") - if !ok { - WinRMPassword = "" - } else { - WinRMPassword = WinRMPassword.(string) - } - phd := ProvisionHookData{WinRMPassword.(string)} - // Run the provisioner in a goroutine so we can continually check // for cancellations... log.Println("Running the provision hook") errCh := make(chan error, 1) go func() { - errCh <- hook.Run(packer.HookProvision, ui, comm, phd) + errCh <- hook.Run(packer.HookProvision, ui, comm, nil) }() for { diff --git a/packer/provisioner.go b/packer/provisioner.go index 4b5e39cea..a565c30e2 100644 --- a/packer/provisioner.go +++ b/packer/provisioner.go @@ -44,10 +44,6 @@ type ProvisionHook struct { runningProvisioner Provisioner } -type ProvisionHookData struct { - WinRMPassword string -} - // Runs the provisioners in order. func (h *ProvisionHook) Run(name string, ui Ui, comm Communicator, data interface{}) error { // Shortcut @@ -75,15 +71,8 @@ func (h *ProvisionHook) Run(name string, ui Ui, comm Communicator, data interfac h.lock.Unlock() ts := CheckpointReporter.AddSpan(p.TypeName, "provisioner", p.Config) - // re-run prepare with builder-generated config variables (e.g. WinRMPassword) - // Hack Alert. #SorryNotSorry. - err := p.Provisioner.Prepare(data) - if err != nil { - log.Printf("Error performing secondary Prepare: %s", err) - } - // Finally, provision. - err = p.Provisioner.Provision(ui, comm) + err := p.Provisioner.Provision(ui, comm) ts.End(err) if err != nil { diff --git a/packer/provisioner_mock.go b/packer/provisioner_mock.go index 035767796..62b304ccb 100644 --- a/packer/provisioner_mock.go +++ b/packer/provisioner_mock.go @@ -14,10 +14,8 @@ type MockProvisioner struct { } func (t *MockProvisioner) Prepare(configs ...interface{}) error { - if !t.PrepCalled { - t.PrepCalled = true - t.PrepConfigs = configs - } + t.PrepCalled = true + t.PrepConfigs = configs return nil } diff --git a/provisioner/ansible/provisioner.go b/provisioner/ansible/provisioner.go index d2528218d..84d3d71b5 100644 --- a/provisioner/ansible/provisioner.go +++ b/provisioner/ansible/provisioner.go @@ -26,6 +26,7 @@ import ( "golang.org/x/crypto/ssh" "github.com/hashicorp/packer/common" + commonhelper "github.com/hashicorp/packer/helper/common" "github.com/hashicorp/packer/helper/config" "github.com/hashicorp/packer/packer" "github.com/hashicorp/packer/template/interpolate" @@ -57,8 +58,6 @@ type Config struct { UseSFTP bool `mapstructure:"use_sftp"` InventoryDirectory string `mapstructure:"inventory_directory"` InventoryFile string `mapstructure:"inventory_file"` - // for internal use only; unsettable by user. - winrmpassword string } type Provisioner struct { @@ -74,35 +73,14 @@ type PassthroughTemplate struct { } func (p *Provisioner) Prepare(raws ...interface{}) error { - // This is a bit of a hack. For provisioners that need access to - // auto-generated WinRMPasswords, the mechanism of keeping provisioner data - // and build data totally segregated breaks down. We get around this by - // having the builder stash the WinRMPassword in the state bag, then - // grabbing it out of the statebag inside of StepProvision. Then, when - // the time comes to provision for real, we run the prepare step one more - // time, now with WinRMPassword defined in the raws, and can store the - // password on the provisioner config without overwriting the rest of the - // work we've already done in the first prepare run. - if len(raws) == 1 { - for k, v := range raws[0].(map[interface{}]interface{}) { - if k.(string) == "WinRMPassword" { - p.config.winrmpassword = v.(string) - - // Even if WinRMPassword is not gonna be used, we've stored the - // key and pointed it to an empty string. That means we'll - // always reach this on our second-run of Prepare() - return nil - } - } - } - p.done = make(chan struct{}) - // don't interpolate winrmpassword yet; if we've made it to this line, then - // we have not obtained the winrm password yet. + // Create passthrough for winrm password so we can fill it in once we know + // it p.config.ctx.Data = &PassthroughTemplate{ WinRMPassword: `{{.WinRMPassword}}`, } + err := config.Decode(&p.config, &config.DecodeOpts{ Interpolate: true, InterpolateContext: &p.config.ctx, @@ -222,9 +200,8 @@ func (p *Provisioner) getVersion() error { func (p *Provisioner) Provision(ui packer.Ui, comm packer.Communicator) error { ui.Say("Provisioning with Ansible...") // Interpolate env vars to check for .WinRMPassword - packer.LogSecretFilter.Set(p.config.winrmpassword) p.config.ctx.Data = &PassthroughTemplate{ - WinRMPassword: p.config.winrmpassword, + WinRMPassword: getWinRMPassword(p.config.PackerBuildName), } for i, envVar := range p.config.AnsibleEnvVars { envVar, err := interpolate.Render(envVar, &p.config.ctx) @@ -443,7 +420,12 @@ func (p *Provisioner) executeAnsible(ui packer.Ui, comm packer.Communicator, pri // remove winrm password from command, if it's been added flattenedCmd := strings.Join(cmd.Args, " ") - ui.Say(fmt.Sprintf("Executing Ansible: %s", flattenedCmd)) + sanitized := flattenedCmd + if len(getWinRMPassword(p.config.PackerBuildName)) > 0 { + sanitized = strings.Replace(sanitized, + getWinRMPassword(p.config.PackerBuildName), "*****", -1) + } + ui.Say(fmt.Sprintf("Executing Ansible: %s", sanitized)) if err := cmd.Start(); err != nil { return err @@ -571,6 +553,12 @@ func newSigner(privKeyFile string) (*signer, error) { return signer, nil } +func getWinRMPassword(buildName string) string { + winRMPass, _ := commonhelper.RetrieveSharedState("winrm_password", buildName) + packer.LogSecretFilter.Set(winRMPass) + return winRMPass +} + // Ui provides concurrency-safe access to packer.Ui. type Ui struct { sem chan int diff --git a/provisioner/ansible/provisioner_test.go b/provisioner/ansible/provisioner_test.go index 278f61764..c3300bc6b 100644 --- a/provisioner/ansible/provisioner_test.go +++ b/provisioner/ansible/provisioner_test.go @@ -16,8 +16,8 @@ import ( // Be sure to remove the Ansible stub file in each test with: // defer os.Remove(config["command"].(string)) -func testConfig(t *testing.T) map[interface{}]interface{} { - m := make((map[interface{}]interface{})) +func testConfig(t *testing.T) map[string]interface{} { + m := make(map[string]interface{}) wd, err := os.Getwd() if err != nil { t.Fatalf("err: %s", err) diff --git a/provisioner/powershell/provisioner.go b/provisioner/powershell/provisioner.go index 6e0665c19..0dccf78e7 100644 --- a/provisioner/powershell/provisioner.go +++ b/provisioner/powershell/provisioner.go @@ -17,6 +17,7 @@ import ( "github.com/hashicorp/packer/common" "github.com/hashicorp/packer/common/uuid" + commonhelper "github.com/hashicorp/packer/helper/common" "github.com/hashicorp/packer/helper/config" "github.com/hashicorp/packer/packer" "github.com/hashicorp/packer/template/interpolate" @@ -97,9 +98,6 @@ type Config struct { // Changes will not be effective until the system is rebooted." ValidExitCodes []int `mapstructure:"valid_exit_codes"` - // internal variable, to be written to at provisioner run time. - winrmpassword string - ctx interpolate.Context } @@ -119,28 +117,8 @@ type EnvVarsTemplate struct { } func (p *Provisioner) Prepare(raws ...interface{}) error { - // This is a bit of a hack. For provisioners that need access to - // auto-generated WinRMPasswords, the mechanism of keeping provisioner data - // and build data totally segregated breaks down. We get around this by - // having the builder stash the WinRMPassword in the state bag, then - // grabbing it out of the statebag inside of StepProvision. Then, when - // the time comes to provision for real, we run the prepare step one more - // time, now with WinRMPassword defined in the raws, and can store the - // password on the provisioner config without overwriting the rest of the - // work we've already done in the first prepare run. - if len(raws) == 1 { - for k, v := range raws[0].(map[interface{}]interface{}) { - if k.(string) == "WinRMPassword" { - p.config.winrmpassword = v.(string) - // Even if WinRMPassword is not gonna be used, we've stored the - // key and pointed it to an empty string. That means we'll - // always reach this on our second-run of Prepare() - return nil - } - } - } - - // Only get here if it's the first time the Provisioner's Prepare is run + // Create passthrough for winrm password so we can fill it in once we know + // it p.config.ctx.Data = &EnvVarsTemplate{ WinRMPassword: `{{.WinRMPassword}}`, } @@ -157,7 +135,7 @@ func (p *Provisioner) Prepare(raws ...interface{}) error { }, raws...) if err != nil { - return fmt.Errorf("Error decoding powershell provisioner template: %s", err) + return err } if p.config.EnvVarFormat == "" { @@ -283,7 +261,7 @@ func extractScript(p *Provisioner) (string, error) { func (p *Provisioner) Provision(ui packer.Ui, comm packer.Communicator) error { ui.Say(fmt.Sprintf("Provisioning with Powershell...")) p.communicator = comm - packer.LogSecretFilter.Set(p.config.winrmpassword) + scripts := make([]string, len(p.config.Scripts)) copy(scripts, p.config.Scripts) @@ -414,7 +392,7 @@ func (p *Provisioner) createFlattenedEnvVars(elevated bool) (flattened string) { // interpolate environment variables p.config.ctx.Data = &EnvVarsTemplate{ - WinRMPassword: p.config.winrmpassword, + WinRMPassword: getWinRMPassword(p.config.PackerBuildName), } // Split vars into key/value components for _, envVar := range p.config.Vars { @@ -489,7 +467,7 @@ func (p *Provisioner) createCommandTextNonPrivileged() (command string, err erro p.config.ctx.Data = &ExecuteCommandTemplate{ Path: p.config.RemotePath, Vars: p.config.RemoteEnvVarPath, - WinRMPassword: p.config.winrmpassword, + WinRMPassword: getWinRMPassword(p.config.PackerBuildName), } command, err = interpolate.Render(p.config.ExecuteCommand, &p.config.ctx) @@ -501,6 +479,12 @@ func (p *Provisioner) createCommandTextNonPrivileged() (command string, err erro return command, nil } +func getWinRMPassword(buildName string) string { + winRMPass, _ := commonhelper.RetrieveSharedState("winrm_password", buildName) + packer.LogSecretFilter.Set(winRMPass) + return winRMPass +} + func (p *Provisioner) createCommandTextPrivileged() (command string, err error) { // Prepare everything needed to enable the required env vars within the // remote environment @@ -512,7 +496,7 @@ func (p *Provisioner) createCommandTextPrivileged() (command string, err error) p.config.ctx.Data = &ExecuteCommandTemplate{ Path: p.config.RemotePath, Vars: p.config.RemoteEnvVarPath, - WinRMPassword: p.config.winrmpassword, + WinRMPassword: getWinRMPassword(p.config.PackerBuildName), } command, err = interpolate.Render(p.config.ElevatedExecuteCommand, &p.config.ctx) if err != nil { @@ -571,7 +555,7 @@ func (p *Provisioner) generateElevatedRunner(command string) (uploadedPath strin } // Replace ElevatedPassword for winrm users who used this feature p.config.ctx.Data = &EnvVarsTemplate{ - WinRMPassword: p.config.winrmpassword, + WinRMPassword: getWinRMPassword(p.config.PackerBuildName), } p.config.ElevatedPassword, _ = interpolate.Render(p.config.ElevatedPassword, &p.config.ctx) diff --git a/provisioner/powershell/provisioner_test.go b/provisioner/powershell/provisioner_test.go index bd0ae15e2..7e739016c 100644 --- a/provisioner/powershell/provisioner_test.go +++ b/provisioner/powershell/provisioner_test.go @@ -14,8 +14,8 @@ import ( "github.com/hashicorp/packer/packer" ) -func testConfig() map[interface{}]interface{} { - return map[interface{}]interface{}{ +func testConfig() map[string]interface{} { + return map[string]interface{}{ "inline": []interface{}{"foo", "bar"}, } } diff --git a/provisioner/shell-local/provisioner.go b/provisioner/shell-local/provisioner.go index 6557a89f4..16c3806e4 100644 --- a/provisioner/shell-local/provisioner.go +++ b/provisioner/shell-local/provisioner.go @@ -10,27 +10,6 @@ type Provisioner struct { } func (p *Provisioner) Prepare(raws ...interface{}) error { - // This is a bit of a hack. For provisioners that need access to - // auto-generated WinRMPasswords, the mechanism of keeping provisioner data - // and build data totally segregated breaks down. We get around this by - // having the builder stash the WinRMPassword in the state bag, then - // grabbing it out of the statebag inside of StepProvision. Then, when - // the time comes to provision for real, we run the prepare step one more - // time, now with WinRMPassword defined in the raws, and can store the - // password on the provisioner config without overwriting the rest of the - // work we've already done in the first prepare run. - if len(raws) == 1 { - for k, v := range raws[0].(map[interface{}]interface{}) { - if k.(string) == "WinRMPassword" { - p.config.WinRMPassword = v.(string) - // Even if WinRMPassword is not gonna be used, we've stored the - // key and pointed it to an empty string. That means we'll - // always reach this on our second-run of Prepare() - return nil - } - } - } - err := sl.Decode(&p.config, raws...) if err != nil { return err diff --git a/provisioner/shell-local/provisioner_test.go b/provisioner/shell-local/provisioner_test.go index 1d130dedd..9f4fb9d2a 100644 --- a/provisioner/shell-local/provisioner_test.go +++ b/provisioner/shell-local/provisioner_test.go @@ -48,8 +48,8 @@ func TestConfigPrepare(t *testing.T) { } } -func testConfig(t *testing.T) map[interface{}]interface{} { - return map[interface{}]interface{}{ +func testConfig(t *testing.T) map[string]interface{} { + return map[string]interface{}{ "command": "echo foo", } }