From ed05161fd0ab49175243e128cf7c8d66bf21d6dc Mon Sep 17 00:00:00 2001 From: Kazunori Kojima Date: Mon, 22 Aug 2016 10:24:34 +0900 Subject: [PATCH 1/9] provider/aws: Support import `aws_s3_bucket_notification` --- .../import_aws_s3_bucket_notification_test.go | 66 +++++++++++++++++++ .../resource_aws_s3_bucket_notification.go | 3 + helper/resource/testing_import_state.go | 2 +- .../r/s3_bucket_notification.html.markdown | 7 ++ 4 files changed, 77 insertions(+), 1 deletion(-) create mode 100644 builtin/providers/aws/import_aws_s3_bucket_notification_test.go diff --git a/builtin/providers/aws/import_aws_s3_bucket_notification_test.go b/builtin/providers/aws/import_aws_s3_bucket_notification_test.go new file mode 100644 index 000000000..9ab574fef --- /dev/null +++ b/builtin/providers/aws/import_aws_s3_bucket_notification_test.go @@ -0,0 +1,66 @@ +package aws + +import ( + "testing" + + "github.com/hashicorp/terraform/helper/acctest" + "github.com/hashicorp/terraform/helper/resource" +) + +func TestAccAWSS3BucketNotification_importBasic(t *testing.T) { + resourceName := "aws_s3_bucket_notification.notification" + + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckAWSS3BucketNotificationDestroy, + Steps: []resource.TestStep{ + resource.TestStep{ + Config: testAccAWSS3BucketConfigWithTopicNotification(acctest.RandInt()), + }, + + resource.TestStep{ + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{"bucket"}, + }, + }, + }) + + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckAWSS3BucketNotificationDestroy, + Steps: []resource.TestStep{ + resource.TestStep{ + Config: testAccAWSS3BucketConfigWithQueueNotification(acctest.RandInt()), + }, + + resource.TestStep{ + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{"bucket"}, + }, + }, + }) + + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckAWSS3BucketNotificationDestroy, + Steps: []resource.TestStep{ + resource.TestStep{ + Config: testAccAWSS3BucketConfigWithLambdaNotification(acctest.RandInt()), + }, + + resource.TestStep{ + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{"bucket"}, + }, + }, + }) +} diff --git a/builtin/providers/aws/resource_aws_s3_bucket_notification.go b/builtin/providers/aws/resource_aws_s3_bucket_notification.go index 562dc723a..f3e19b484 100644 --- a/builtin/providers/aws/resource_aws_s3_bucket_notification.go +++ b/builtin/providers/aws/resource_aws_s3_bucket_notification.go @@ -20,6 +20,9 @@ func resourceAwsS3BucketNotification() *schema.Resource { Read: resourceAwsS3BucketNotificationRead, Update: resourceAwsS3BucketNotificationPut, Delete: resourceAwsS3BucketNotificationDelete, + Importer: &schema.ResourceImporter{ + State: schema.ImportStatePassthrough, + }, Schema: map[string]*schema.Schema{ "bucket": &schema.Schema{ diff --git a/helper/resource/testing_import_state.go b/helper/resource/testing_import_state.go index 00ec87324..d5c579629 100644 --- a/helper/resource/testing_import_state.go +++ b/helper/resource/testing_import_state.go @@ -78,7 +78,7 @@ func testStepImportState( // Find the existing resource var oldR *terraform.ResourceState for _, r2 := range old { - if r2.Primary != nil && r2.Primary.ID == r.Primary.ID { + if r2.Primary != nil && r2.Primary.ID == r.Primary.ID && r2.Type == r.Type { oldR = r2 break } diff --git a/website/source/docs/providers/aws/r/s3_bucket_notification.html.markdown b/website/source/docs/providers/aws/r/s3_bucket_notification.html.markdown index 3f6564d17..fa2f258db 100644 --- a/website/source/docs/providers/aws/r/s3_bucket_notification.html.markdown +++ b/website/source/docs/providers/aws/r/s3_bucket_notification.html.markdown @@ -234,3 +234,10 @@ The `lambda_function` notification configuration supports the following: * `filter_prefix` - (Optional) Specifies object key name prefix. * `filter_suffix` - (Optional) Specifies object key name suffix. +## Import + +S3 bucket notification can be imported using the `bucket`, e.g. + +``` +$ terraform import aws_s3_bucket_notification.bucket_notification bucket-name +``` From 8d0540865f18ffa4dbb643af23aa7995d4f0a2b0 Mon Sep 17 00:00:00 2001 From: Jakub Holy Date: Tue, 30 Aug 2016 11:24:36 +0200 Subject: [PATCH 2/9] local-exec: the OS doesn't need to be ready It is not obvious that the resource being created doesn't mean that the OS and system services such as sshd are ready (contrary to `remote-exec`). It is better to make that explicit and same developers like me some headache :-) --- website/source/docs/provisioners/local-exec.html.markdown | 3 +++ 1 file changed, 3 insertions(+) diff --git a/website/source/docs/provisioners/local-exec.html.markdown b/website/source/docs/provisioners/local-exec.html.markdown index 1a41194b2..c700936b3 100644 --- a/website/source/docs/provisioners/local-exec.html.markdown +++ b/website/source/docs/provisioners/local-exec.html.markdown @@ -13,6 +13,9 @@ is created. This invokes a process on the machine running Terraform, not on the resource. See the `remote-exec` [provisioner](/docs/provisioners/remote-exec.html) to run commands on the resource. +Beware that even though the resource is fully created when the provisioner is run, +it doesn't need to have finished booting or starting system services. + ## Example usage ``` From d31656af9138b39b2c0eaae480c9b694f5cc9bc4 Mon Sep 17 00:00:00 2001 From: James Nugent Date: Sat, 3 Sep 2016 15:22:26 -0700 Subject: [PATCH 3/9] docs: Clarify note on local provisioners --- website/source/docs/provisioners/local-exec.html.markdown | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/website/source/docs/provisioners/local-exec.html.markdown b/website/source/docs/provisioners/local-exec.html.markdown index c700936b3..5ec849790 100644 --- a/website/source/docs/provisioners/local-exec.html.markdown +++ b/website/source/docs/provisioners/local-exec.html.markdown @@ -13,8 +13,9 @@ is created. This invokes a process on the machine running Terraform, not on the resource. See the `remote-exec` [provisioner](/docs/provisioners/remote-exec.html) to run commands on the resource. -Beware that even though the resource is fully created when the provisioner is run, -it doesn't need to have finished booting or starting system services. +Note that even though the resource will be ifully created when the provisioner is run, +there is no guarantee that it will be in an operable state - for example system services +such as `sshd` may not be started yet on compute resources. ## Example usage From 609219fc65c1e0f7720b94e468b9456a973cd2ce Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sat, 3 Sep 2016 15:26:49 -0700 Subject: [PATCH 4/9] command/meta: validate config immediately * config: test for validating multi-vars (passes) * command/plan: test invalid run * command/meta: validate module on load --- command/command.go | 7 +++- command/meta.go | 5 +++ command/plan_test.go | 34 +++++++++++++++++++ command/push_test.go | 3 +- command/test-fixtures/plan-invalid/main.tf | 7 ++++ command/test-fixtures/push-tfvars/main.tf | 1 - config/config_test.go | 7 ++++ .../validate-count-resource-var-multi/main.tf | 5 +++ 8 files changed, 65 insertions(+), 4 deletions(-) create mode 100644 command/test-fixtures/plan-invalid/main.tf create mode 100644 config/test-fixtures/validate-count-resource-var-multi/main.tf diff --git a/command/command.go b/command/command.go index d9ad0cf30..a4ca00532 100644 --- a/command/command.go +++ b/command/command.go @@ -2,6 +2,7 @@ package command import ( "fmt" + "log" "github.com/hashicorp/terraform/terraform" "github.com/mitchellh/cli" @@ -27,7 +28,11 @@ const DefaultBackupExtension = ".backup" const DefaultParallelism = 10 func validateContext(ctx *terraform.Context, ui cli.Ui) bool { - if ws, es := ctx.Validate(); len(ws) > 0 || len(es) > 0 { + log.Println("[INFO] Validating the context...") + ws, es := ctx.Validate() + log.Printf("[INFO] Validation result: %d warnings, %d errors", len(ws), len(es)) + + if len(ws) > 0 || len(es) > 0 { ui.Output( "There are warnings and/or errors related to your configuration. Please\n" + "fix these before continuing.\n") diff --git a/command/meta.go b/command/meta.go index 5adf1b667..f18b910d4 100644 --- a/command/meta.go +++ b/command/meta.go @@ -165,6 +165,11 @@ func (m *Meta) Context(copts contextOpts) (*terraform.Context, bool, error) { return nil, false, fmt.Errorf("Error downloading modules: %s", err) } + // Validate the module right away + if err := mod.Validate(); err != nil { + return nil, false, err + } + opts.Module = mod opts.Parallelism = copts.Parallelism opts.State = state.State() diff --git a/command/plan_test.go b/command/plan_test.go index 710d993a9..c4af9c141 100644 --- a/command/plan_test.go +++ b/command/plan_test.go @@ -395,6 +395,40 @@ func TestPlan_statePast(t *testing.T) { } } +func TestPlan_validate(t *testing.T) { + // This is triggered by not asking for input so we have to set this to false + test = false + defer func() { test = true }() + + cwd, err := os.Getwd() + if err != nil { + t.Fatalf("err: %s", err) + } + if err := os.Chdir(testFixturePath("plan-invalid")); err != nil { + t.Fatalf("err: %s", err) + } + defer os.Chdir(cwd) + + p := testProvider() + ui := new(cli.MockUi) + c := &PlanCommand{ + Meta: Meta{ + ContextOpts: testCtxConfig(p), + Ui: ui, + }, + } + + args := []string{} + if code := c.Run(args); code != 1 { + t.Fatalf("bad: %d\n\n%s", code, ui.ErrorWriter.String()) + } + + actual := ui.ErrorWriter.String() + if !strings.Contains(actual, "can't reference") { + t.Fatalf("bad: %s", actual) + } +} + func TestPlan_vars(t *testing.T) { p := testProvider() ui := new(cli.MockUi) diff --git a/command/push_test.go b/command/push_test.go index b48add166..b441edced 100644 --- a/command/push_test.go +++ b/command/push_test.go @@ -765,8 +765,7 @@ func pushTFVars() []atlas.TFVar { return []atlas.TFVar{ {"bar", "foo", false}, {"baz", `{ - A = "a" - interp = "${file("t.txt")}" + A = "a" }`, true}, {"fob", `["a", "quotes \"in\" quotes"]`, true}, {"foo", "bar", false}, diff --git a/command/test-fixtures/plan-invalid/main.tf b/command/test-fixtures/plan-invalid/main.tf new file mode 100644 index 000000000..fd8142032 --- /dev/null +++ b/command/test-fixtures/plan-invalid/main.tf @@ -0,0 +1,7 @@ +resource "test_instance" "foo" { + count = 5 +} + +resource "test_instance" "bar" { + count = "${length(test_instance.foo.*.id)}" +} diff --git a/command/test-fixtures/push-tfvars/main.tf b/command/test-fixtures/push-tfvars/main.tf index 2110bea73..2699d50ee 100644 --- a/command/test-fixtures/push-tfvars/main.tf +++ b/command/test-fixtures/push-tfvars/main.tf @@ -7,7 +7,6 @@ variable "baz" { default = { "A" = "a" - interp = "${file("t.txt")}" } } diff --git a/config/config_test.go b/config/config_test.go index 084a301ee..201a62893 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -195,6 +195,13 @@ func TestConfigValidate_countResourceVar(t *testing.T) { } } +func TestConfigValidate_countResourceVarMulti(t *testing.T) { + c := testConfig(t, "validate-count-resource-var-multi") + if err := c.Validate(); err == nil { + t.Fatal("should not be valid") + } +} + func TestConfigValidate_countUserVar(t *testing.T) { c := testConfig(t, "validate-count-user-var") if err := c.Validate(); err != nil { diff --git a/config/test-fixtures/validate-count-resource-var-multi/main.tf b/config/test-fixtures/validate-count-resource-var-multi/main.tf new file mode 100644 index 000000000..fca11701d --- /dev/null +++ b/config/test-fixtures/validate-count-resource-var-multi/main.tf @@ -0,0 +1,5 @@ +resource "aws_instance" "foo" {} + +resource "aws_instance" "web" { + count = "${length(aws_instance.foo.*.bar)}" +} From d77238951847827d7a2ed6d0ca3376ed3629f798 Mon Sep 17 00:00:00 2001 From: James Nugent Date: Sat, 3 Sep 2016 15:28:09 -0700 Subject: [PATCH 5/9] Update CHANGELOG.md --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 39cc2b96c..6137bbd78 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -36,6 +36,7 @@ IMPROVEMENTS: BUG FIXES: * core: Changing a module source from file to VCS no longer errors [GH-8398] + * core: Configuration is now validated prior to input, fixing an obscure parse error when attempting to interpolate a count [GH-8591] * core: JSON configuration with resources with a single key parse properly [GH-8485] * core: States with duplicate modules are detected and an error is shown [GH-8463] * core: Validate uniqueness of variables/outputs in a module [GH-8482] From 8d0a68e1d44fe2e79c5b56c643aac59ea087efa6 Mon Sep 17 00:00:00 2001 From: James Nugent Date: Sat, 3 Sep 2016 15:38:43 -0700 Subject: [PATCH 6/9] state/remote: Officially Support local backend This is a rework of pull request #6213 submitted by @joshuaspence, adjusted to work with the remote state data source. We also add a deprecation warning for people using the unsupported API, and retain the ability to refer to "_local" as well as "local" for users in a mixed version environment. --- .../providers/terraform/data_source_state.go | 16 +++++++++ .../terraform/data_source_state_test.go | 6 ++-- state/remote/remote.go | 6 ++-- .../source/docs/state/remote/local.html.md | 36 +++++++++++++++++++ website/source/layouts/remotestate.erb | 3 ++ 5 files changed, 60 insertions(+), 7 deletions(-) create mode 100644 website/source/docs/state/remote/local.html.md diff --git a/builtin/providers/terraform/data_source_state.go b/builtin/providers/terraform/data_source_state.go index c87da01fb..2c3c5b050 100644 --- a/builtin/providers/terraform/data_source_state.go +++ b/builtin/providers/terraform/data_source_state.go @@ -16,6 +16,16 @@ func dataSourceRemoteState() *schema.Resource { "backend": { Type: schema.TypeString, Required: true, + ValidateFunc: func(v interface{}, k string) (ws []string, errors []error) { + if vStr, ok := v.(string); ok && vStr == "_local" { + ws = append(ws, "Use of the %q backend is now officially "+ + "supported as %q. Please update your configuration to ensure "+ + "compatibility with future versions of Terraform.", + "_local", "local") + } + + return + }, }, "config": { @@ -38,6 +48,12 @@ func dataSourceRemoteStateRead(d *schema.ResourceData, meta interface{}) error { config[k] = v.(string) } + // Don't break people using the old _local syntax - but note warning above + if backend == "_local" { + log.Println(`[INFO] Switching old (unsupported) backend "_local" to "local"`) + backend = "local" + } + // Create the client to access our remote state log.Printf("[DEBUG] Initializing remote state client: %s", backend) client, err := remote.NewClient(backend, config) diff --git a/builtin/providers/terraform/data_source_state_test.go b/builtin/providers/terraform/data_source_state_test.go index 0bab4b261..1d56823ef 100644 --- a/builtin/providers/terraform/data_source_state_test.go +++ b/builtin/providers/terraform/data_source_state_test.go @@ -32,7 +32,7 @@ func TestState_complexOutputs(t *testing.T) { { Config: testAccState_complexOutputs, Check: resource.ComposeTestCheckFunc( - testAccCheckStateValue("terraform_remote_state.foo", "backend", "_local"), + testAccCheckStateValue("terraform_remote_state.foo", "backend", "local"), testAccCheckStateValue("terraform_remote_state.foo", "config.path", "./test-fixtures/complex_outputs.tfstate"), testAccCheckStateValue("terraform_remote_state.foo", "computed_set.#", "2"), testAccCheckStateValue("terraform_remote_state.foo", `map.%`, "2"), @@ -65,7 +65,7 @@ func testAccCheckStateValue(id, name, value string) resource.TestCheckFunc { const testAccState_basic = ` data "terraform_remote_state" "foo" { - backend = "_local" + backend = "local" config { path = "./test-fixtures/basic.tfstate" @@ -74,7 +74,7 @@ data "terraform_remote_state" "foo" { const testAccState_complexOutputs = ` resource "terraform_remote_state" "foo" { - backend = "_local" + backend = "local" config { path = "./test-fixtures/complex_outputs.tfstate" diff --git a/state/remote/remote.go b/state/remote/remote.go index fdec41bcc..173ab00ca 100644 --- a/state/remote/remote.go +++ b/state/remote/remote.go @@ -36,16 +36,14 @@ func NewClient(t string, conf map[string]string) (Client, error) { // BuiltinClients is the list of built-in clients that can be used with // NewClient. var BuiltinClients = map[string]Factory{ + "artifactory": artifactoryFactory, "atlas": atlasFactory, "azure": azureFactory, "consul": consulFactory, "etcd": etcdFactory, "gcs": gcsFactory, "http": httpFactory, + "local": fileFactory, "s3": s3Factory, "swift": swiftFactory, - "artifactory": artifactoryFactory, - - // This is used for development purposes only. - "_local": fileFactory, } diff --git a/website/source/docs/state/remote/local.html.md b/website/source/docs/state/remote/local.html.md new file mode 100644 index 000000000..b8495a2f5 --- /dev/null +++ b/website/source/docs/state/remote/local.html.md @@ -0,0 +1,36 @@ +--- +layout: "remotestate" +page_title: "Remote State Backend: local" +sidebar_current: "docs-state-remote-local" +description: |- + Remote state stored using the local file system. +--- + +# local + +Remote state backend that uses the local file system. + +## Example Usage + +``` +terraform remote config \ + -backend=local \ + -backend-config="path=/path/to/terraform.tfstate" +``` + +## Example Reference + +``` +data "terraform_remote_state" "foo" { + backend = "local" + config { + path = "${path.module}/../../terraform.tfstate" + } +} +``` + +## Configuration variables + +The following configuration options are supported: + + * `path` - (Required) The path to the `tfstate` file. diff --git a/website/source/layouts/remotestate.erb b/website/source/layouts/remotestate.erb index cd14b9d77..3a3a27cbb 100644 --- a/website/source/layouts/remotestate.erb +++ b/website/source/layouts/remotestate.erb @@ -34,6 +34,9 @@ > http + > + local + > s3 From bc5518f9936688be81641015cfbe113bc0529904 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kristinn=20=C3=96rn=20Sigur=C3=B0sson?= Date: Sun, 4 Sep 2016 00:45:24 +0200 Subject: [PATCH 7/9] provisioners: Allow provisioning over IPv6 --- communicator/shared/shared.go | 17 +++++ communicator/shared/shared_test.go | 26 ++++++++ communicator/ssh/provisioner.go | 10 +++ communicator/ssh/provisioner_test.go | 62 +++++++++++++++++++ communicator/winrm/provisioner.go | 6 ++ communicator/winrm/provisioner_test.go | 86 ++++++++++++++++++++++++++ 6 files changed, 207 insertions(+) create mode 100644 communicator/shared/shared.go create mode 100644 communicator/shared/shared_test.go diff --git a/communicator/shared/shared.go b/communicator/shared/shared.go new file mode 100644 index 000000000..39cb16961 --- /dev/null +++ b/communicator/shared/shared.go @@ -0,0 +1,17 @@ +package shared + +import ( + "fmt" + "net" +) + +// IpFormat formats the IP correctly, so we don't provide IPv6 address in an IPv4 format during node communication. We return the ip parameter as is if it's an IPv4 address or a hostname. +func IpFormat(ip string) string { + ipObj := net.ParseIP(ip) + // Return the ip/host as is if it's either a hostname or an IPv4 address. + if ipObj == nil || ipObj.To4() != nil { + return ip + } + + return fmt.Sprintf("[%s]", ip) +} diff --git a/communicator/shared/shared_test.go b/communicator/shared/shared_test.go new file mode 100644 index 000000000..575e5f78d --- /dev/null +++ b/communicator/shared/shared_test.go @@ -0,0 +1,26 @@ +package shared + +import ( + "testing" +) + +func TestIpFormatting_Ipv4(t *testing.T) { + formatted := IpFormat("127.0.0.1") + if formatted != "127.0.0.1" { + t.Fatal("expected", "127.0.0.1", "got", formatted) + } +} + +func TestIpFormatting_Hostname(t *testing.T) { + formatted := IpFormat("example.com") + if formatted != "example.com" { + t.Fatal("expected", "example.com", "got", formatted) + } +} + +func TestIpFormatting_Ipv6(t *testing.T) { + formatted := IpFormat("::1") + if formatted != "[::1]" { + t.Fatal("expected", "[::1]", "got", formatted) + } +} diff --git a/communicator/ssh/provisioner.go b/communicator/ssh/provisioner.go index 48eaafe38..07d50a051 100644 --- a/communicator/ssh/provisioner.go +++ b/communicator/ssh/provisioner.go @@ -8,6 +8,7 @@ import ( "os" "time" + "github.com/hashicorp/terraform/communicator/shared" "github.com/hashicorp/terraform/helper/pathorcontents" "github.com/hashicorp/terraform/terraform" "github.com/mitchellh/mapstructure" @@ -84,6 +85,11 @@ func parseConnectionInfo(s *terraform.InstanceState) (*connectionInfo, error) { if connInfo.User == "" { connInfo.User = DefaultUser } + + // Format the host if needed. + // Needed for IPv6 support. + connInfo.Host = shared.IpFormat(connInfo.Host) + if connInfo.Port == 0 { connInfo.Port = DefaultPort } @@ -107,6 +113,10 @@ func parseConnectionInfo(s *terraform.InstanceState) (*connectionInfo, error) { // Default all bastion config attrs to their non-bastion counterparts if connInfo.BastionHost != "" { + // Format the bastion host if needed. + // Needed for IPv6 support. + connInfo.BastionHost = shared.IpFormat(connInfo.BastionHost) + if connInfo.BastionUser == "" { connInfo.BastionUser = connInfo.User } diff --git a/communicator/ssh/provisioner_test.go b/communicator/ssh/provisioner_test.go index aa029dad8..051d8d34d 100644 --- a/communicator/ssh/provisioner_test.go +++ b/communicator/ssh/provisioner_test.go @@ -66,6 +66,68 @@ func TestProvisioner_connInfo(t *testing.T) { } } +func TestProvisioner_connInfoIpv6(t *testing.T) { + r := &terraform.InstanceState{ + Ephemeral: terraform.EphemeralState{ + ConnInfo: map[string]string{ + "type": "ssh", + "user": "root", + "password": "supersecret", + "private_key": "someprivatekeycontents", + "host": "::1", + "port": "22", + "timeout": "30s", + + "bastion_host": "::1", + }, + }, + } + + conf, err := parseConnectionInfo(r) + if err != nil { + t.Fatalf("err: %v", err) + } + + if conf.Host != "[::1]" { + t.Fatalf("bad: %v", conf) + } + + if conf.BastionHost != "[::1]" { + t.Fatalf("bad %v", conf) + } +} + +func TestProvisioner_connInfoHostname(t *testing.T) { + r := &terraform.InstanceState{ + Ephemeral: terraform.EphemeralState{ + ConnInfo: map[string]string{ + "type": "ssh", + "user": "root", + "password": "supersecret", + "private_key": "someprivatekeycontents", + "host": "example.com", + "port": "22", + "timeout": "30s", + + "bastion_host": "example.com", + }, + }, + } + + conf, err := parseConnectionInfo(r) + if err != nil { + t.Fatalf("err: %v", err) + } + + if conf.Host != "example.com" { + t.Fatalf("bad: %v", conf) + } + + if conf.BastionHost != "example.com" { + t.Fatalf("bad %v", conf) + } +} + func TestProvisioner_connInfoLegacy(t *testing.T) { r := &terraform.InstanceState{ Ephemeral: terraform.EphemeralState{ diff --git a/communicator/winrm/provisioner.go b/communicator/winrm/provisioner.go index d1562998c..2dab1e97d 100644 --- a/communicator/winrm/provisioner.go +++ b/communicator/winrm/provisioner.go @@ -7,6 +7,7 @@ import ( "strings" "time" + "github.com/hashicorp/terraform/communicator/shared" "github.com/hashicorp/terraform/terraform" "github.com/mitchellh/mapstructure" ) @@ -72,6 +73,11 @@ func parseConnectionInfo(s *terraform.InstanceState) (*connectionInfo, error) { if connInfo.User == "" { connInfo.User = DefaultUser } + + // Format the host if needed. + // Needed for IPv6 support. + connInfo.Host = shared.IpFormat(connInfo.Host) + if connInfo.Port == 0 { connInfo.Port = DefaultPort } diff --git a/communicator/winrm/provisioner_test.go b/communicator/winrm/provisioner_test.go index 9a271ae59..9ed6f095d 100644 --- a/communicator/winrm/provisioner_test.go +++ b/communicator/winrm/provisioner_test.go @@ -49,6 +49,92 @@ func TestProvisioner_connInfo(t *testing.T) { } } +func TestProvisioner_connInfoIpv6(t *testing.T) { + r := &terraform.InstanceState{ + Ephemeral: terraform.EphemeralState{ + ConnInfo: map[string]string{ + "type": "winrm", + "user": "Administrator", + "password": "supersecret", + "host": "::1", + "port": "5985", + "https": "true", + "timeout": "30s", + }, + }, + } + + conf, err := parseConnectionInfo(r) + if err != nil { + t.Fatalf("err: %v", err) + } + + if conf.User != "Administrator" { + t.Fatalf("expected: %v: got: %v", "Administrator", conf) + } + if conf.Password != "supersecret" { + t.Fatalf("expected: %v: got: %v", "supersecret", conf) + } + if conf.Host != "[::1]" { + t.Fatalf("expected: %v: got: %v", "[::1]", conf) + } + if conf.Port != 5985 { + t.Fatalf("expected: %v: got: %v", 5985, conf) + } + if conf.HTTPS != true { + t.Fatalf("expected: %v: got: %v", true, conf) + } + if conf.Timeout != "30s" { + t.Fatalf("expected: %v: got: %v", "30s", conf) + } + if conf.ScriptPath != DefaultScriptPath { + t.Fatalf("expected: %v: got: %v", DefaultScriptPath, conf) + } +} + +func TestProvisioner_connInfoHostname(t *testing.T) { + r := &terraform.InstanceState{ + Ephemeral: terraform.EphemeralState{ + ConnInfo: map[string]string{ + "type": "winrm", + "user": "Administrator", + "password": "supersecret", + "host": "example.com", + "port": "5985", + "https": "true", + "timeout": "30s", + }, + }, + } + + conf, err := parseConnectionInfo(r) + if err != nil { + t.Fatalf("err: %v", err) + } + + if conf.User != "Administrator" { + t.Fatalf("expected: %v: got: %v", "Administrator", conf) + } + if conf.Password != "supersecret" { + t.Fatalf("expected: %v: got: %v", "supersecret", conf) + } + if conf.Host != "example.com" { + t.Fatalf("expected: %v: got: %v", "example.com", conf) + } + if conf.Port != 5985 { + t.Fatalf("expected: %v: got: %v", 5985, conf) + } + if conf.HTTPS != true { + t.Fatalf("expected: %v: got: %v", true, conf) + } + if conf.Timeout != "30s" { + t.Fatalf("expected: %v: got: %v", "30s", conf) + } + if conf.ScriptPath != DefaultScriptPath { + t.Fatalf("expected: %v: got: %v", DefaultScriptPath, conf) + } +} + func TestProvisioner_formatDuration(t *testing.T) { cases := map[string]struct { InstanceState *terraform.InstanceState From 25e5031fb6e40895423aa304181e1ac3aa118308 Mon Sep 17 00:00:00 2001 From: James Nugent Date: Sat, 3 Sep 2016 15:47:37 -0700 Subject: [PATCH 8/9] Update CHANGELOG.md --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6137bbd78..a8de444b3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,7 @@ FEATURES: * **New Resource:** `librato_service` [GH-8170] * Data source blocks can now have a count associated with them [GH-8635] * The count of a resource can now be referenced for interpolations: `self.count` and `type.name.count` work [GH-8581] + * Provisioners now support connection using IPv6 in addition to IPv4 [GH-6616] IMPROVEMENTS: * core: Add wildcard (match all) support to `ignore_changes` [GH-8599] From a9fa042d71eaa008c302b541580932c5e7d2659c Mon Sep 17 00:00:00 2001 From: James Nugent Date: Sat, 3 Sep 2016 15:48:07 -0700 Subject: [PATCH 9/9] Update CHANGELOG.md --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index a8de444b3..37ec6ebed 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ FEATURES: * **New Resource:** `cloudstack_affinity_group` [GH-8360] * **New Resource:** `librato_alert` [GH-8170] * **New Resource:** `librato_service` [GH-8170] + * **New Remote State Backend:** `local` [GH-8647] * Data source blocks can now have a count associated with them [GH-8635] * The count of a resource can now be referenced for interpolations: `self.count` and `type.name.count` work [GH-8581] * Provisioners now support connection using IPv6 in addition to IPv4 [GH-6616]