From 37544f4d5fcd4bd0898c32bbc1c7a1866c75363b Mon Sep 17 00:00:00 2001 From: Chris Chilvers Date: Sat, 18 Jul 2020 23:54:17 +0100 Subject: [PATCH 1/3] Support using WinRM over an IAP tunnel This avoids the need to expose WinRM ports on the internet and allows using instances with only an internal private IP address. When using a WinRM tunnel there is a race condition between the tunnel connection attempt timing out and packer assuming the connection was successful. To allow for this, when using WinRM the default success timeout is increased to 40 seconds. --- builder/googlecompute/config.go | 44 ++++++++++++--- builder/googlecompute/config_test.go | 62 ++++++++++++++++++++-- builder/googlecompute/step_start_tunnel.go | 13 +++-- 3 files changed, 104 insertions(+), 15 deletions(-) diff --git a/builder/googlecompute/config.go b/builder/googlecompute/config.go index 6b0acac08..16ee632d6 100644 --- a/builder/googlecompute/config.go +++ b/builder/googlecompute/config.go @@ -426,18 +426,28 @@ func (c *Config) Prepare(raws ...interface{}) ([]string, error) { } } if c.IAPConfig.IAPTunnelLaunchWait == 0 { - c.IAPConfig.IAPTunnelLaunchWait = 30 + if c.Comm.Type == "winrm" { + // when starting up, WinRM can cause the tunnel to take 30 seconds + // before timing out + c.IAPConfig.IAPTunnelLaunchWait = 40 + } else { + c.IAPConfig.IAPTunnelLaunchWait = 30 + } } // Configure IAP: Update SSH config to use localhost proxy instead if c.IAPConfig.IAP { - if c.Comm.Type == "ssh" { - c.Comm.SSHHost = "localhost" - } else { - err := fmt.Errorf("Error: IAP tunnel currently only implemnted for" + - " SSH communicator") + if !SupportsIAPTunnel(&c.Comm) { + err := fmt.Errorf("Error: IAP tunnel is not implemented for %s communicator", c.Comm.Type) errs = packer.MultiErrorAppend(errs, err) } + // These configuration values are copied early to the generic host parameter when configuring + // StepConnect. As such they must be set now. Ideally we would handle this as part of + // ApplyIAPTunnel and set them during StepStartTunnel but that means defering when the + // CommHost function reads the value from the configuration, perhaps pass in b.config.Comm + // instead of b.config.Comm.Host()? + c.Comm.SSHHost = "localhost" + c.Comm.WinRMHost = "localhost" } // Process required parameters. @@ -541,3 +551,25 @@ func (k *CustomerEncryptionKey) ComputeType() *compute.CustomerEncryptionKey { RawKey: k.RawKey, } } + +func SupportsIAPTunnel(c *communicator.Config) bool { + switch c.Type { + case "ssh", "winrm": + return true + default: + return false + } +} + +func ApplyIAPTunnel(c *communicator.Config, port int) error { + switch c.Type { + case "ssh": + c.SSHPort = port + return nil + case "winrm": + c.WinRMPort = port + return nil + default: + return fmt.Errorf("IAP tunnel is not implemented for %s communicator", c.Type) + } +} diff --git a/builder/googlecompute/config_test.go b/builder/googlecompute/config_test.go index 051c3797b..89ba003d5 100644 --- a/builder/googlecompute/config_test.go +++ b/builder/googlecompute/config_test.go @@ -7,6 +7,8 @@ import ( "runtime" "strings" "testing" + + "github.com/hashicorp/packer/helper/communicator" ) func TestConfigPrepare(t *testing.T) { @@ -414,9 +416,6 @@ func TestConfigPrepareIAP(t *testing.T) { t.Fatalf("IAP hashbang didn't default correctly to /bin/sh.") } } - if c.Comm.SSHHost != "localhost" { - t.Fatalf("Didn't correctly override the ssh host.") - } } func TestConfigPrepareIAP_failures(t *testing.T) { @@ -425,7 +424,7 @@ func TestConfigPrepareIAP_failures(t *testing.T) { "source_image": "foo", "winrm_username": "packer", "zone": "us-central1-a", - "communicator": "winrm", + "communicator": "none", "iap_hashbang": "/bin/bash", "iap_ext": ".ps1", "use_iap": true, @@ -434,7 +433,7 @@ func TestConfigPrepareIAP_failures(t *testing.T) { var c Config _, errs := c.Prepare(config) if errs == nil { - t.Fatalf("Should have errored because we're using winrm.") + t.Fatalf("Should have errored because we're using none.") } if c.IAPHashBang != "/bin/bash" { t.Fatalf("IAP hashbang defaulted even though set.") @@ -500,6 +499,59 @@ func TestRegion(t *testing.T) { } } +func TestApplyIAPTunnel_SSH(t *testing.T) { + c := &communicator.Config{ + Type: "ssh", + SSH: communicator.SSH{ + SSHHost: "example", + SSHPort: 1234, + }, + } + + err := ApplyIAPTunnel(c, 8447) + if err != nil { + t.Fatalf("Shouldn't have errors") + } + if c.SSHHost != "localhost" { + t.Fatalf("Should have set SSHHost") + } + if c.SSHPort != 8447 { + t.Fatalf("Should have set SSHPort") + } +} + +func TestApplyIAPTunnel_WinRM(t *testing.T) { + c := &communicator.Config{ + Type: "winrm", + WinRM: communicator.WinRM{ + WinRMHost: "example", + WinRMPort: 1234, + }, + } + + err := ApplyIAPTunnel(c, 8447) + if err != nil { + t.Fatalf("Shouldn't have errors") + } + if c.WinRMHost != "localhost" { + t.Fatalf("Should have set WinRMHost") + } + if c.WinRMPort != 8447 { + t.Fatalf("Should have set WinRMPort") + } +} + +func TestApplyIAPTunnel_none(t *testing.T) { + c := &communicator.Config{ + Type: "none", + } + + err := ApplyIAPTunnel(c, 8447) + if err == nil { + t.Fatalf("Should have errors, none is not supported") + } +} + // Helper stuff below func testConfig(t *testing.T) (config map[string]interface{}, tempAccountFile string) { diff --git a/builder/googlecompute/step_start_tunnel.go b/builder/googlecompute/step_start_tunnel.go index 15020281a..f52839e8a 100644 --- a/builder/googlecompute/step_start_tunnel.go +++ b/builder/googlecompute/step_start_tunnel.go @@ -34,8 +34,6 @@ type IAPConfig struct { // - You must have the gcloud sdk installed on the computer running Packer. // - You must be using a Service Account with a credentials file (using the // account_file option in the Packer template) - // - This is currently only implemented for the SSH communicator, not the - // WinRM Communicator. // - You must add the given service account to project level IAP permissions // in https://console.cloud.google.com/security/iap. To do so, click // "project" > "SSH and TCP resoures" > "All Tunnel Resources" > @@ -52,7 +50,7 @@ type IAPConfig struct { // Default: ".sh" IAPExt string `mapstructure:"iap_ext" required:"false"` // How long to wait, in seconds, before assuming a tunnel launch was - // successful. Defaults to 30 seconds. + // successful. Defaults to 30 seconds for SSH or 40 seconds for WinRM. IAPTunnelLaunchWait int `mapstructure:"iap_tunnel_launch_wait" required:"false"` } @@ -283,7 +281,14 @@ func (s *StepStartTunnel) Run(ctx context.Context, state multistep.StateBag) mul // This is the port the IAP tunnel listens on, on localhost. // TODO make setting LocalHostPort optional - s.CommConf.SSHPort = s.IAPConf.IAPLocalhostPort + err = ApplyIAPTunnel(s.CommConf, s.IAPConf.IAPLocalhostPort) + if err != nil { + // this should not occur as the config should validate that the communicator + // supports using an IAP tunnel + state.Put("error", err) + ui.Error(err.Error()) + return multistep.ActionHalt + } log.Printf("Creating tunnel launch script with args %#v", args) // Create temp file that contains both gcloud authentication, and gcloud From 63eedf841e5d26ee8a4987d1a732ca3c5982a08a Mon Sep 17 00:00:00 2001 From: Chris Chilvers Date: Wed, 22 Jul 2020 12:58:05 +0100 Subject: [PATCH 2/3] Fix failing tests due to IAP communicator host name Moved setting the host from ApplyIAPTunnel to Config.Prepare but forgot to update the related tests. --- builder/googlecompute/config_test.go | 65 +++++++++++++++++++--------- 1 file changed, 44 insertions(+), 21 deletions(-) diff --git a/builder/googlecompute/config_test.go b/builder/googlecompute/config_test.go index 89ba003d5..dab12c9f9 100644 --- a/builder/googlecompute/config_test.go +++ b/builder/googlecompute/config_test.go @@ -385,7 +385,7 @@ func TestConfigPrepareStartupScriptFile(t *testing.T) { } } -func TestConfigPrepareIAP(t *testing.T) { +func TestConfigPrepareIAP_SSH(t *testing.T) { config := map[string]interface{}{ "project_id": "project", "source_image": "foo", @@ -400,22 +400,33 @@ func TestConfigPrepareIAP(t *testing.T) { if err != nil { t.Fatalf("Shouldn't have errors. Err = %s", err) } + if c.Comm.SSHHost != "localhost" { + t.Fatalf("Should have set SSHHost") + } - if runtime.GOOS == "windows" { - if c.IAPExt != ".cmd" { - t.Fatalf("IAP tempfile extension didn't default correctly to .cmd") - } - if c.IAPHashBang != "" { - t.Fatalf("IAP hashbang didn't default correctly to nothing.") - } - } else { - if c.IAPExt != "" { - t.Fatalf("IAP tempfile extension should default to empty on unix mahcines") - } - if c.IAPHashBang != "/bin/sh" { - t.Fatalf("IAP hashbang didn't default correctly to /bin/sh.") - } + testIAPScript(t, &c) +} + +func TestConfigPrepareIAP_WinRM(t *testing.T) { + config := map[string]interface{}{ + "project_id": "project", + "source_image": "foo", + "winrm_username": "packer", + "zone": "us-central1-a", + "communicator": "winrm", + "use_iap": true, + } + + var c Config + _, err := c.Prepare(config) + if err != nil { + t.Fatalf("Shouldn't have errors. Err = %s", err) } + if c.Comm.WinRMHost != "localhost" { + t.Fatalf("Should have set WinRMHost") + } + + testIAPScript(t, &c) } func TestConfigPrepareIAP_failures(t *testing.T) { @@ -512,9 +523,6 @@ func TestApplyIAPTunnel_SSH(t *testing.T) { if err != nil { t.Fatalf("Shouldn't have errors") } - if c.SSHHost != "localhost" { - t.Fatalf("Should have set SSHHost") - } if c.SSHPort != 8447 { t.Fatalf("Should have set SSHPort") } @@ -533,9 +541,6 @@ func TestApplyIAPTunnel_WinRM(t *testing.T) { if err != nil { t.Fatalf("Shouldn't have errors") } - if c.WinRMHost != "localhost" { - t.Fatalf("Should have set WinRMHost") - } if c.WinRMPort != 8447 { t.Fatalf("Should have set WinRMPort") } @@ -628,6 +633,24 @@ func testAccountFile(t *testing.T) string { return tf.Name() } +func testIAPScript(t *testing.T, c *Config) { + if runtime.GOOS == "windows" { + if c.IAPExt != ".cmd" { + t.Fatalf("IAP tempfile extension didn't default correctly to .cmd") + } + if c.IAPHashBang != "" { + t.Fatalf("IAP hashbang didn't default correctly to nothing.") + } + } else { + if c.IAPExt != "" { + t.Fatalf("IAP tempfile extension should default to empty on unix mahcines") + } + if c.IAPHashBang != "/bin/sh" { + t.Fatalf("IAP hashbang didn't default correctly to /bin/sh.") + } + } +} + const testMetadataFileContent = `testMetadata` func testMetadataFile(t *testing.T) string { From 67494ff45be37302482c373b71334242f3f8e0a8 Mon Sep 17 00:00:00 2001 From: Chris Chilvers Date: Wed, 22 Jul 2020 17:34:45 +0100 Subject: [PATCH 3/3] update IAP docs to include WinRM --- .../partials/builder/googlecompute/IAPConfig-not-required.mdx | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/website/pages/partials/builder/googlecompute/IAPConfig-not-required.mdx b/website/pages/partials/builder/googlecompute/IAPConfig-not-required.mdx index 7e9f2aa21..0ce215f50 100644 --- a/website/pages/partials/builder/googlecompute/IAPConfig-not-required.mdx +++ b/website/pages/partials/builder/googlecompute/IAPConfig-not-required.mdx @@ -6,8 +6,6 @@ - You must have the gcloud sdk installed on the computer running Packer. - You must be using a Service Account with a credentials file (using the account_file option in the Packer template) - - This is currently only implemented for the SSH communicator, not the - WinRM Communicator. - You must add the given service account to project level IAP permissions in https://console.cloud.google.com/security/iap. To do so, click "project" > "SSH and TCP resoures" > "All Tunnel Resources" > @@ -24,4 +22,4 @@ Default: ".sh" - `iap_tunnel_launch_wait` (int) - How long to wait, in seconds, before assuming a tunnel launch was - successful. Defaults to 30 seconds. + successful. Defaults to 30 seconds for SSH or 40 seconds for WinRM.