From 62ed0f9eeca38a64c7393f48853079e2d65c8e2c Mon Sep 17 00:00:00 2001 From: Vlad Romanenko Date: Thu, 8 Feb 2018 15:27:46 +0000 Subject: [PATCH 01/23] Minor grammar fix. --- website/intro/getting-started/modules.html.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/website/intro/getting-started/modules.html.md b/website/intro/getting-started/modules.html.md index 0debae300..a1c593226 100644 --- a/website/intro/getting-started/modules.html.md +++ b/website/intro/getting-started/modules.html.md @@ -38,7 +38,7 @@ larger building-blocks for your infrastructure. In this example, we're going to use [the Consul Terraform module for AWS](https://registry.terraform.io/modules/hashicorp/consul/aws), which will set up a complete [Consul](https://www.consul.io) cluster. -This and other modules can found via the search feature on the Terraform +This and other modules can be found via the search feature on the Terraform Registry site. Create a configuration file with the following contents: From fb9531b3d96afc82e80715ad09bc6095e4acea0f Mon Sep 17 00:00:00 2001 From: Paul Tyng Date: Thu, 15 Mar 2018 09:48:09 -0400 Subject: [PATCH 02/23] Rename http client test --- httpclient/client_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/httpclient/client_test.go b/httpclient/client_test.go index 8a748f7c2..e3f97c299 100644 --- a/httpclient/client_test.go +++ b/httpclient/client_test.go @@ -10,7 +10,7 @@ import ( "github.com/hashicorp/terraform/version" ) -func TestUserAgent(t *testing.T) { +func TestNew_userAgent(t *testing.T) { var actualUserAgent string ts := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) { actualUserAgent = req.UserAgent() From 6284f99708f2ad92b2e41a120c174575a4fe1402 Mon Sep 17 00:00:00 2001 From: Paul Tyng Date: Thu, 15 Mar 2018 09:25:57 -0400 Subject: [PATCH 03/23] Allow callers to append to user agent --- httpclient/useragent.go | 16 +++++++++++- httpclient/useragent_test.go | 47 ++++++++++++++++++++++++++++++++++++ 2 files changed, 62 insertions(+), 1 deletion(-) create mode 100644 httpclient/useragent_test.go diff --git a/httpclient/useragent.go b/httpclient/useragent.go index d8db087cf..5e2801768 100644 --- a/httpclient/useragent.go +++ b/httpclient/useragent.go @@ -2,15 +2,29 @@ package httpclient import ( "fmt" + "log" "net/http" + "os" + "strings" "github.com/hashicorp/terraform/version" ) const userAgentFormat = "Terraform/%s" +const uaEnvVar = "TF_APPEND_USER_AGENT" func UserAgentString() string { - return fmt.Sprintf(userAgentFormat, version.Version) + ua := fmt.Sprintf(userAgentFormat, version.Version) + + if add := os.Getenv(uaEnvVar); add != "" { + add = strings.TrimSpace(add) + if len(add) > 0 { + ua += " " + add + log.Printf("[DEBUG] Using modified User-Agent: %s", ua) + } + } + + return ua } type userAgentRoundTripper struct { diff --git a/httpclient/useragent_test.go b/httpclient/useragent_test.go new file mode 100644 index 000000000..9c577da72 --- /dev/null +++ b/httpclient/useragent_test.go @@ -0,0 +1,47 @@ +package httpclient + +import ( + "fmt" + "os" + "testing" + + "github.com/hashicorp/terraform/version" +) + +func TestUserAgentString_env(t *testing.T) { + expectedBase := fmt.Sprintf(userAgentFormat, version.Version) + if oldenv, isSet := os.LookupEnv(uaEnvVar); isSet { + defer os.Setenv(uaEnvVar, oldenv) + } else { + defer os.Unsetenv(uaEnvVar) + } + + for i, c := range []struct { + expected string + additional string + }{ + {expectedBase, ""}, + {expectedBase, " "}, + {expectedBase, " \n"}, + + {fmt.Sprintf("%s test/1", expectedBase), "test/1"}, + {fmt.Sprintf("%s test/2", expectedBase), "test/2 "}, + {fmt.Sprintf("%s test/3", expectedBase), " test/3 "}, + {fmt.Sprintf("%s test/4", expectedBase), "test/4 \n"}, + } { + t.Run(fmt.Sprintf("%d", i), func(t *testing.T) { + if c.additional == "" { + os.Unsetenv(uaEnvVar) + } else { + os.Setenv(uaEnvVar, c.additional) + } + + actual := UserAgentString() + + if c.expected != actual { + t.Fatalf("Expected User-Agent '%s' does not match '%s'", c.expected, actual) + } + }) + } + +} From c4e5de71157c35dcae67c9ad9bfb6f519dbaa653 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Thu, 15 Mar 2018 20:52:58 +0000 Subject: [PATCH 04/23] release: clean up after v0.11.4 --- CHANGELOG.md | 3 +++ version/version.go | 4 ++-- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 73ee22986..ffcba39bd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,6 @@ +## 0.11.5 (Unreleased) + + ## 0.11.4 (March 15, 2018) IMPROVEMENTS: diff --git a/version/version.go b/version/version.go index 5a259e3ad..88a63a13d 100644 --- a/version/version.go +++ b/version/version.go @@ -11,12 +11,12 @@ import ( ) // The main version number that is being run at the moment. -const Version = "0.11.4" +const Version = "0.11.5" // A pre-release marker for the version. If this is "" (empty string) // then it means that it is a final release. Otherwise, this is a pre-release // such as "dev" (in development), "beta", "rc1", etc. -var Prerelease = "" +var Prerelease = "dev" // SemVer is an instance of version.Version. This has the secondary // benefit of verifying during tests and init time that our version is a From ecec59f0f0ad69350dfc8beb76c2d95cfd92f476 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Thu, 15 Mar 2018 17:06:57 -0400 Subject: [PATCH 05/23] udpate CHANGELOG.md remove redundant entry. --- CHANGELOG.md | 1 - 1 file changed, 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ffcba39bd..1e497d120 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,7 +17,6 @@ IMPROVEMENTS: BUG FIXES: * core: Make sure state is locked during initial refresh ([#17422](https://github.com/hashicorp/terraform/issues/17422)) -* core: Fix interpolation error when count references another interpolated count value ([#17368](https://github.com/hashicorp/terraform/issues/17368)) * core: Halt on fatal provisioner errors, rather than retrying until a timeout ([#17359](https://github.com/hashicorp/terraform/issues/17359)) * core: When handling a forced exit due to multiple interrupts, prevent the process from exiting while the state is being written ([#17323](https://github.com/hashicorp/terraform/issues/17323)) * core: Fix handling of locals and outputs at destroy time ([#17241](https://github.com/hashicorp/terraform/issues/17241)) From 426045eb2ec3b93c39004d225c6415fd88e83583 Mon Sep 17 00:00:00 2001 From: Josh Cooley Date: Fri, 16 Mar 2018 09:59:33 -0500 Subject: [PATCH 06/23] Spelling correction --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1e497d120..ebb8bfefd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,7 +7,7 @@ IMPROVEMENTS: * cli: `terraform state list` now accepts a new argument `-id=...` for filtering resources for display by their remote ids ([#17221](https://github.com/hashicorp/terraform/issues/17221)) * cli: `terraform destroy` now uses the option `-auto-approve` instead of `-force`, for consistency with `terraform apply`. The old flag is preserved for backward-compatibility, but is now deprecated; it will be retained for at least one major release. ([#17218](https://github.com/hashicorp/terraform/issues/17218)) -* connection/ssh: Add support for host key verifiation ([#17354](https://github.com/hashicorp/terraform/issues/17354)) +* connection/ssh: Add support for host key verification ([#17354](https://github.com/hashicorp/terraform/issues/17354)) * backend/s3: add support for the cn-northwest-1 region ([#17216](https://github.com/hashicorp/terraform/issues/17216)) * provisioner/local-exec: Allow setting custom environment variables when running commands ([#13880](https://github.com/hashicorp/terraform/issues/13880)) * provisioner/habitat: Detect if hab user exists and only create if necessary ([#17195](https://github.com/hashicorp/terraform/issues/17195)) From 60bc16305add4789dc087dc36b8273f1bfab9b9f Mon Sep 17 00:00:00 2001 From: ScottWinkler Date: Fri, 16 Mar 2018 16:28:58 -0700 Subject: [PATCH 07/23] tools/terraform-bundle: accept custom plugins from a local directory To make it easier to include third-party plugins in generated bundles, we'll now search a local directory for available plugins and skip attempting to install from releases.hashicorp.com if a dependency can be satisfied locally. --- tools/terraform-bundle/README.md | 20 +++-- tools/terraform-bundle/package.go | 121 +++++++++++++++++++++++++----- 2 files changed, 116 insertions(+), 25 deletions(-) diff --git a/tools/terraform-bundle/README.md b/tools/terraform-bundle/README.md index 5e697f528..b95a65ee8 100644 --- a/tools/terraform-bundle/README.md +++ b/tools/terraform-bundle/README.md @@ -53,6 +53,12 @@ providers { # two expressions match different versions then _both_ are included in # the bundle archive. google = ["~> 1.0", "~> 2.0"] + + # Include a custom plugin to the bundle. Will search for the plugin in the + # plugins directory, and package it with the bundle archive. Plugin must have + # a name of the form: terraform-provider-*, and must be build with the operating + # system and architecture that terraform enterprise is running, e.g. linux and amd64 + customplugin = ["0.1"] } ``` @@ -100,6 +106,13 @@ this composite version number so that bundle archives can be easily distinguished from official release archives and from each other when multiple bundles contain the same core Terraform version. +To include custom plugins in the bundle file, create a local directory "./plugins" +and put all the plugins you want to include there. Optionally, you can use the +`-plugin-dir` flag to specify a location where to find the plugins. To be recognized +as a valid plugin, the file must have a name of the form: "terraform-provider-*-v*". In +addition, ensure that the plugin is build using the same operating system and +architecture used for terraform enterprise. Typically this will be linux and amd64. + ## Provider Resolution Behavior Terraform's provider resolution behavior is such that if a given constraint @@ -112,13 +125,6 @@ of the versions available from the bundle. If a suitable version cannot be found in the bundle, Terraform _will_ attempt to satisfy that dependency by automatic installation from the official repository. -To disable automatic installation altogether -- and thus cause a hard failure -if no local plugins match -- the `-plugin-dir` option can be passed to -`terraform init`, giving the directory into which the bundle was extracted. -The presence of this option overrides all of the normal automatic discovery -and installation behavior, and thus forces the use of only the plugins that -can be found in the directory indicated. - The downloaded provider archives are verified using the same signature check that is used for auto-installed plugins, using Hashicorp's release key. At this time, the core Terraform archive itself is _not_ verified in this way; diff --git a/tools/terraform-bundle/package.go b/tools/terraform-bundle/package.go index 6c7ee51ed..faf6ea640 100644 --- a/tools/terraform-bundle/package.go +++ b/tools/terraform-bundle/package.go @@ -15,7 +15,7 @@ import ( getter "github.com/hashicorp/go-getter" "github.com/hashicorp/terraform/plugin" - "github.com/hashicorp/terraform/plugin/discovery" + discovery "github.com/hashicorp/terraform/plugin/discovery" "github.com/mitchellh/cli" ) @@ -23,10 +23,66 @@ type PackageCommand struct { ui cli.Ui } +// shameless stackoverflow copy + pasta https://stackoverflow.com/questions/21060945/simple-way-to-copy-a-file-in-golang +func CopyFile(src, dst string) (err error) { + sfi, err := os.Stat(src) + if err != nil { + return + } + if !sfi.Mode().IsRegular() { + // cannot copy non-regular files (e.g., directories, + // symlinks, devices, etc.) + return fmt.Errorf("CopyFile: non-regular source file %s (%q)", sfi.Name(), sfi.Mode().String()) + } + dfi, err := os.Stat(dst) + if err != nil { + if !os.IsNotExist(err) { + return + } + } else { + if !(dfi.Mode().IsRegular()) { + return fmt.Errorf("CopyFile: non-regular destination file %s (%q)", dfi.Name(), dfi.Mode().String()) + } + if os.SameFile(sfi, dfi) { + return + } + } + if err = os.Link(src, dst); err == nil { + return + } + err = copyFileContents(src, dst) + return +} + +// see above +func copyFileContents(src, dst string) (err error) { + in, err := os.Open(src) + if err != nil { + return + } + defer in.Close() + out, err := os.Create(dst) + if err != nil { + return + } + defer func() { + cerr := out.Close() + if err == nil { + err = cerr + } + }() + if _, err = io.Copy(out, in); err != nil { + return + } + err = out.Sync() + return +} + func (c *PackageCommand) Run(args []string) int { flags := flag.NewFlagSet("package", flag.ExitOnError) osPtr := flags.String("os", "", "Target operating system") archPtr := flags.String("arch", "", "Target CPU architecture") + pluginDirPtr := flags.String("plugin-dir", "", "Path to custom plugins directory") err := flags.Parse(args) if err != nil { c.ui.Error(err.Error()) @@ -35,12 +91,16 @@ func (c *PackageCommand) Run(args []string) int { osName := runtime.GOOS archName := runtime.GOARCH + pluginDir := "./plugins" if *osPtr != "" { osName = *osPtr } if *archPtr != "" { archName = *archPtr } + if *pluginDirPtr != "" { + pluginDir = *pluginDirPtr + } if flags.NArg() != 1 { c.ui.Error("Configuration filename is required") @@ -70,10 +130,17 @@ func (c *PackageCommand) Run(args []string) int { coreZipURL := c.coreURL(config.Terraform.Version, osName, archName) err = getter.Get(workDir, coreZipURL) + if err != nil { c.ui.Error(fmt.Sprintf("Failed to fetch core package from %s: %s", coreZipURL, err)) } + c.ui.Info(fmt.Sprintf("Fetching 3rd party plugins in directory: %s", pluginDir)) + dirs := []string{pluginDir} //FindPlugins requires an array + localPlugins := discovery.FindPlugins("provider", dirs) + for k, _ := range localPlugins { + c.ui.Info(fmt.Sprintf("plugin: %s (%s)", k.Name, k.Version)) + } installer := &discovery.ProviderInstaller{ Dir: workDir, @@ -92,19 +159,29 @@ func (c *PackageCommand) Run(args []string) int { Ui: c.ui, } - if len(config.Providers) > 0 { - c.ui.Output(fmt.Sprintf("Checking for available provider plugins on %s...", - discovery.GetReleaseHost())) - } - - for name, constraints := range config.Providers { - for _, constraint := range constraints { + for name, constraintStrs := range config.Providers { + for _, constraintStr := range constraintStrs { c.ui.Output(fmt.Sprintf("- Resolving %q provider (%s)...", - name, constraint)) - _, err := installer.Get(name, constraint.MustParse()) - if err != nil { - c.ui.Error(fmt.Sprintf("- Failed to resolve %s provider %s: %s", name, constraint, err)) - return 1 + name, constraintStr)) + foundPlugins := discovery.PluginMetaSet{} + constraint := constraintStr.MustParse() + for plugin, _ := range localPlugins { + if plugin.Name == name && constraint.Allows(plugin.Version.MustParse()) { + foundPlugins.Add(plugin) + } + } + + if len(foundPlugins) > 0 { + plugin := foundPlugins.Newest() + CopyFile(plugin.Path, workDir+"/terraform-provider-"+plugin.Name+"-v"+plugin.Version.MustParse().String()) //put into temp dir + } else { //attempt to get from the public registry if not found locally + c.ui.Output(fmt.Sprintf("- Checking for provider plugin on %s...", + discovery.GetReleaseHost())) + _, err := installer.Get(name, constraint) + if err != nil { + c.ui.Error(fmt.Sprintf("- Failed to resolve %s provider %s: %s", name, constraint, err)) + return 1 + } } } } @@ -202,11 +279,13 @@ current working directory containing a Terraform binary along with zero or more provider plugin binaries. Options: - -os=name Target operating system the archive will be built for. Defaults - to that of the system where the command is being run. + -os=name Target operating system the archive will be built for. Defaults + to that of the system where the command is being run. - -arch=name Target CPU architecture the archive will be built for. Defaults - to that of the system where the command is being run. + -arch=name Target CPU architecture the archive will be built for. Defaults + to that of the system where the command is being run. + + -plugin-dir=path The path to the custom plugins directory. Defaults to "./plugins". The resulting zip file can be used to more easily install Terraform and a fixed set of providers together on a server, so that Terraform's provider @@ -233,7 +312,13 @@ not a normal Terraform configuration file. The file format looks like this: # Each item in these lists allows a distinct version to be added. If the # two expressions match different versions then _both_ are included in # the bundle archive. - google = ["~> 1.0", "~> 2.0"] + google = ["~> 1.0", "~> 2.0"] + + #Include a custom plugin to the bundle. Will search for the plugin in the + #plugins directory, and package it with the bundle archive. Plugin must have + #a name of the form: terraform-provider-*-v*, and must be built with the operating + #system and architecture that terraform enterprise is running, e.g. linux and amd64 + customplugin = ["0.1"] } ` From dd935588f738bb87e020b39d0108246deb218432 Mon Sep 17 00:00:00 2001 From: Logan Rakai Date: Sun, 18 Mar 2018 00:01:30 -0600 Subject: [PATCH 08/23] Typo Correction --- website/docs/backends/config.html.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/website/docs/backends/config.html.md b/website/docs/backends/config.html.md index 458104f9f..bc24a5eb5 100644 --- a/website/docs/backends/config.html.md +++ b/website/docs/backends/config.html.md @@ -51,7 +51,7 @@ a configuration in the future: create the new configuration and run You do not need to specify every required argument in the backend configuration. Omitting certain arguments may be desirable to avoid storing secrets, such as access keys, within the main configuration. When some or all of the arguments -are ommitted, we call this a _partial configuration_. +are omitted, we call this a _partial configuration_. With a partial configuration, the remaining configuration arguments must be provided as part of From e10a7917e6c85fbb0e20380101ce8a8f27e81c89 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Mon, 19 Mar 2018 17:09:53 -0400 Subject: [PATCH 09/23] add failing test for windows state locks Refreshing a locked state on windows could return nil if the read path was locked, no state was yet written, and the read path is the same as the write path. Add a test that locks then refreshes a newly initialized state struct. --- state/local_test.go | 39 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/state/local_test.go b/state/local_test.go index 633356028..13dcf6eab 100644 --- a/state/local_test.go +++ b/state/local_test.go @@ -166,3 +166,42 @@ func testLocalState(t *testing.T) *LocalState { return ls } + +// Make sure we can refresh while the state is locked +func TestLocalState_refreshWhileLocked(t *testing.T) { + f, err := ioutil.TempFile("", "tf") + if err != nil { + t.Fatalf("err: %s", err) + } + + err = terraform.WriteState(TestStateInitial(), f) + f.Close() + if err != nil { + t.Fatalf("err: %s", err) + } + + s := &LocalState{Path: f.Name()} + defer os.Remove(s.Path) + + // lock first + info := NewLockInfo() + info.Operation = "test" + lockID, err := s.Lock(info) + if err != nil { + t.Fatal(err) + } + defer func() { + if err := s.Unlock(lockID); err != nil { + t.Fatal(err) + } + }() + + if err := s.RefreshState(); err != nil { + t.Fatal(err) + } + + readState := s.State() + if readState == nil || readState.Lineage == "" { + t.Fatal("missing state") + } +} From 8fb8b2cffc54e4e724dffb1afb45219152c49fda Mon Sep 17 00:00:00 2001 From: James Bardin Date: Mon, 19 Mar 2018 18:15:54 -0400 Subject: [PATCH 10/23] make sure ReadState returns an error ReadState would hide any errors, assuming that it was an empty state. This can mask errors on Windows, where the OS enforces read locks on the state file. --- terraform/state.go | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/terraform/state.go b/terraform/state.go index 89203bbfe..04b14a659 100644 --- a/terraform/state.go +++ b/terraform/state.go @@ -9,6 +9,7 @@ import ( "io" "io/ioutil" "log" + "os" "reflect" "sort" "strconv" @@ -1876,13 +1877,21 @@ var ErrNoState = errors.New("no state") // ReadState reads a state structure out of a reader in the format that // was written by WriteState. func ReadState(src io.Reader) (*State, error) { - buf := bufio.NewReader(src) - if _, err := buf.Peek(1); err != nil { - // the error is either io.EOF or "invalid argument", and both are from - // an empty state. + // check for a nil file specifically, since that produces a platform + // specific error if we try to use it in a bufio.Reader. + if f, ok := src.(*os.File); ok && f == nil { return nil, ErrNoState } + buf := bufio.NewReader(src) + + if _, err := buf.Peek(1); err != nil { + if err == io.EOF { + return nil, ErrNoState + } + return nil, err + } + if err := testForV0State(buf); err != nil { return nil, err } From a84b4a669ae053b17f37d1acab4d65daa82d2591 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Mon, 19 Mar 2018 17:19:50 -0400 Subject: [PATCH 11/23] use the open state file for refresh when possible Only open a new file descriptor for RefreshState if we haven't written a state, and don't have the correct state open already. This prevents windows from failing to refresh a locked state. --- state/local.go | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/state/local.go b/state/local.go index a6d17653b..609881f72 100644 --- a/state/local.go +++ b/state/local.go @@ -119,8 +119,20 @@ func (s *LocalState) RefreshState() error { s.mu.Lock() defer s.mu.Unlock() + if s.PathOut == "" { + s.PathOut = s.Path + } + var reader io.Reader - if !s.written { + + // The s.Path file is only OK to read if we have not written any state out + // (in which case the same state needs to be read in), and no state output file + // has been opened (possibly via a lock) or the input path is different + // than the output path. + // This is important for Windows, as if the input file is the same as the + // output file, and the output file has been locked already, we can't open + // the file again. + if !s.written && (s.stateFileOut == nil || s.Path != s.PathOut) { // we haven't written a state file yet, so load from Path f, err := os.Open(s.Path) if err != nil { From e5f8adfc1a329adb0bb4855ba340e1e92ccb2f0a Mon Sep 17 00:00:00 2001 From: James Bardin Date: Mon, 19 Mar 2018 20:32:37 -0400 Subject: [PATCH 12/23] add failing test for invalid output with targets Outputs that are missing references aren't always removed from the graph, due to being filtered before their dependents are removed. --- terraform/context_plan_test.go | 22 +++++++++++++++++++ .../plan-untargeted-resource-output/main.tf | 8 +++++++ .../mod/main.tf | 15 +++++++++++++ 3 files changed, 45 insertions(+) create mode 100644 terraform/test-fixtures/plan-untargeted-resource-output/main.tf create mode 100644 terraform/test-fixtures/plan-untargeted-resource-output/mod/main.tf diff --git a/terraform/context_plan_test.go b/terraform/context_plan_test.go index 8b680879d..8afb4c320 100644 --- a/terraform/context_plan_test.go +++ b/terraform/context_plan_test.go @@ -2969,6 +2969,28 @@ STATE: } } +// ensure that outputs missing references due to targetting are removed from +// the graph. +func TestContext2Plan_outputContainsTargetedResource(t *testing.T) { + m := testModule(t, "plan-untargeted-resource-output") + p := testProvider("aws") + p.DiffFn = testDiffFn + ctx := testContext2(t, &ContextOpts{ + Module: m, + ProviderResolver: ResourceProviderResolverFixed( + map[string]ResourceProviderFactory{ + "aws": testProviderFuncFixed(p), + }, + ), + Targets: []string{"module.mod.aws_instance.a"}, + }) + + _, err := ctx.Plan() + if err != nil { + t.Fatalf("err: %s", err) + } +} + // https://github.com/hashicorp/terraform/issues/4515 func TestContext2Plan_targetedOverTen(t *testing.T) { m := testModule(t, "plan-targeted-over-ten") diff --git a/terraform/test-fixtures/plan-untargeted-resource-output/main.tf b/terraform/test-fixtures/plan-untargeted-resource-output/main.tf new file mode 100644 index 000000000..9d4a1c882 --- /dev/null +++ b/terraform/test-fixtures/plan-untargeted-resource-output/main.tf @@ -0,0 +1,8 @@ +module "mod" { + source = "./mod" +} + + +resource "aws_instance" "c" { + name = "${module.mod.output}" +} diff --git a/terraform/test-fixtures/plan-untargeted-resource-output/mod/main.tf b/terraform/test-fixtures/plan-untargeted-resource-output/mod/main.tf new file mode 100644 index 000000000..d7ee9a7fa --- /dev/null +++ b/terraform/test-fixtures/plan-untargeted-resource-output/mod/main.tf @@ -0,0 +1,15 @@ +locals { + "one" = 1 +} + +resource "aws_instance" "a" { + count = "${local.one}" +} + +resource "aws_instance" "b" { + count = "${local.one}" +} + +output "output" { + value = "${join("", coalescelist(aws_instance.a.*.id, aws_instance.b.*.id))}" +} From a5c4f7e08e80ee50e1def527bff780a9e44fd191 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Mon, 19 Mar 2018 21:20:06 -0400 Subject: [PATCH 13/23] remove unneeded partial outputs filterPartialOutputs was not taking into account that some dependent resources might yet be removed from the graph. Check that they are not in the targeted set before declaring that the output remain. --- terraform/transform_targets.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/terraform/transform_targets.go b/terraform/transform_targets.go index 0cfcb0ac0..af6defe36 100644 --- a/terraform/transform_targets.go +++ b/terraform/transform_targets.go @@ -217,6 +217,12 @@ func filterPartialOutputs(v interface{}, targetedNodes *dag.Set, g *Graph) bool if _, ok := d.(*NodeCountBoundary); ok { continue } + + if !targetedNodes.Include(d) { + // this one is going to be removed, so it doesn't count + continue + } + // as soon as we see a real dependency, we mark this as // non-removable return true From 07aeea51daad61ad3b44578725a507408986d08d Mon Sep 17 00:00:00 2001 From: Scott Hain Date: Tue, 20 Mar 2018 03:51:14 -0700 Subject: [PATCH 14/23] Updates the chef provisioner to allow specifying a channel (#17355) * Updates the chef provisioner to allow specifying a channel This also updates the omnitruck url to the current url. Signed-off-by: Scott Hain * Update omnitruck URL Signed-off-by: Scott Hain --- .../provisioners/chef/linux_provisioner.go | 4 +- .../chef/linux_provisioner_test.go | 53 +++++++++++----- .../provisioners/chef/resource_provisioner.go | 7 +++ .../provisioners/chef/windows_provisioner.go | 4 +- .../chef/windows_provisioner_test.go | 62 ++++++++++++++++++- 5 files changed, 106 insertions(+), 24 deletions(-) diff --git a/builtin/provisioners/chef/linux_provisioner.go b/builtin/provisioners/chef/linux_provisioner.go index 6d87cfe8a..2252e5dbc 100644 --- a/builtin/provisioners/chef/linux_provisioner.go +++ b/builtin/provisioners/chef/linux_provisioner.go @@ -11,7 +11,7 @@ import ( const ( chmod = "find %s -maxdepth 1 -type f -exec /bin/chmod %d {} +" - installURL = "https://www.chef.io/chef/install.sh" + installURL = "https://omnitruck.chef.io/install.sh" ) func (p *provisioner) linuxInstallChefClient(o terraform.UIOutput, comm communicator.Communicator) error { @@ -34,7 +34,7 @@ func (p *provisioner) linuxInstallChefClient(o terraform.UIOutput, comm communic } // Then execute the install.sh scrip to download and install Chef Client - err = p.runCommand(o, comm, fmt.Sprintf("%sbash ./install.sh -v %q", prefix, p.Version)) + err = p.runCommand(o, comm, fmt.Sprintf("%sbash ./install.sh -v %q -c %s", prefix, p.Version, p.Channel)) if err != nil { return err } diff --git a/builtin/provisioners/chef/linux_provisioner_test.go b/builtin/provisioners/chef/linux_provisioner_test.go index 578bb69ff..aa5b93d1d 100644 --- a/builtin/provisioners/chef/linux_provisioner_test.go +++ b/builtin/provisioners/chef/linux_provisioner_test.go @@ -25,9 +25,9 @@ func TestResourceProvider_linuxInstallChefClient(t *testing.T) { }, Commands: map[string]bool{ - "sudo curl -LO https://www.chef.io/chef/install.sh": true, - "sudo bash ./install.sh -v \"\"": true, - "sudo rm -f install.sh": true, + "sudo curl -LO https://omnitruck.chef.io/install.sh": true, + "sudo bash ./install.sh -v \"\" -c stable": true, + "sudo rm -f install.sh": true, }, }, @@ -43,9 +43,9 @@ func TestResourceProvider_linuxInstallChefClient(t *testing.T) { }, Commands: map[string]bool{ - "curl -LO https://www.chef.io/chef/install.sh": true, - "bash ./install.sh -v \"\"": true, - "rm -f install.sh": true, + "curl -LO https://omnitruck.chef.io/install.sh": true, + "bash ./install.sh -v \"\" -c stable": true, + "rm -f install.sh": true, }, }, @@ -61,9 +61,9 @@ func TestResourceProvider_linuxInstallChefClient(t *testing.T) { }, Commands: map[string]bool{ - "http_proxy='http://proxy.local' curl -LO https://www.chef.io/chef/install.sh": true, - "http_proxy='http://proxy.local' bash ./install.sh -v \"\"": true, - "http_proxy='http://proxy.local' rm -f install.sh": true, + "http_proxy='http://proxy.local' curl -LO https://omnitruck.chef.io/install.sh": true, + "http_proxy='http://proxy.local' bash ./install.sh -v \"\" -c stable": true, + "http_proxy='http://proxy.local' rm -f install.sh": true, }, }, @@ -79,9 +79,9 @@ func TestResourceProvider_linuxInstallChefClient(t *testing.T) { }, Commands: map[string]bool{ - "https_proxy='https://proxy.local' curl -LO https://www.chef.io/chef/install.sh": true, - "https_proxy='https://proxy.local' bash ./install.sh -v \"\"": true, - "https_proxy='https://proxy.local' rm -f install.sh": true, + "https_proxy='https://proxy.local' curl -LO https://omnitruck.chef.io/install.sh": true, + "https_proxy='https://proxy.local' bash ./install.sh -v \"\" -c stable": true, + "https_proxy='https://proxy.local' rm -f install.sh": true, }, }, @@ -99,9 +99,9 @@ func TestResourceProvider_linuxInstallChefClient(t *testing.T) { Commands: map[string]bool{ "http_proxy='http://proxy.local' no_proxy='http://local.local,http://local.org' " + - "curl -LO https://www.chef.io/chef/install.sh": true, + "curl -LO https://omnitruck.chef.io/install.sh": true, "http_proxy='http://proxy.local' no_proxy='http://local.local,http://local.org' " + - "bash ./install.sh -v \"\"": true, + "bash ./install.sh -v \"\" -c stable": true, "http_proxy='http://proxy.local' no_proxy='http://local.local,http://local.org' " + "rm -f install.sh": true, }, @@ -119,9 +119,28 @@ func TestResourceProvider_linuxInstallChefClient(t *testing.T) { }, Commands: map[string]bool{ - "curl -LO https://www.chef.io/chef/install.sh": true, - "bash ./install.sh -v \"11.18.6\"": true, - "rm -f install.sh": true, + "curl -LO https://omnitruck.chef.io/install.sh": true, + "bash ./install.sh -v \"11.18.6\" -c stable": true, + "rm -f install.sh": true, + }, + }, + + "Channel": { + Config: map[string]interface{}{ + "channel": "current", + "node_name": "nodename1", + "prevent_sudo": true, + "run_list": []interface{}{"cookbook::recipe"}, + "server_url": "https://chef.local", + "user_name": "bob", + "user_key": "USER-KEY", + "version": "11.18.6", + }, + + Commands: map[string]bool{ + "curl -LO https://omnitruck.chef.io/install.sh": true, + "bash ./install.sh -v \"11.18.6\" -c current": true, + "rm -f install.sh": true, }, }, } diff --git a/builtin/provisioners/chef/resource_provisioner.go b/builtin/provisioners/chef/resource_provisioner.go index ea44e9fd0..36d3eb88d 100644 --- a/builtin/provisioners/chef/resource_provisioner.go +++ b/builtin/provisioners/chef/resource_provisioner.go @@ -86,6 +86,7 @@ type provisionFn func(terraform.UIOutput, communicator.Communicator) error type provisioner struct { Attributes map[string]interface{} + Channel string ClientOptions []string DisableReporting bool Environment string @@ -149,6 +150,11 @@ func Provisioner() terraform.ResourceProvisioner { Type: schema.TypeString, Optional: true, }, + "channel": &schema.Schema{ + Type: schema.TypeString, + Optional: true, + Default: "stable", + }, "client_options": &schema.Schema{ Type: schema.TypeList, Elem: &schema.Schema{Type: schema.TypeString}, @@ -725,6 +731,7 @@ func (p *provisioner) copyOutput(o terraform.UIOutput, r io.Reader, doneCh chan< func decodeConfig(d *schema.ResourceData) (*provisioner, error) { p := &provisioner{ + Channel: d.Get("channel").(string), ClientOptions: getStringList(d.Get("client_options")), DisableReporting: d.Get("disable_reporting").(bool), Environment: d.Get("environment").(string), diff --git a/builtin/provisioners/chef/windows_provisioner.go b/builtin/provisioners/chef/windows_provisioner.go index 8713affcd..02010acdf 100644 --- a/builtin/provisioners/chef/windows_provisioner.go +++ b/builtin/provisioners/chef/windows_provisioner.go @@ -23,7 +23,7 @@ switch ($winver) if ([System.IntPtr]::Size -eq 4) {$machine_arch = "i686"} else {$machine_arch = "x86_64"} -$url = "http://www.chef.io/chef/download?p=windows&pv=$machine_os&m=$machine_arch&v=%s" +$url = "http://omnitruck.chef.io/%s/chef/download?p=windows&pv=$machine_os&m=$machine_arch&v=%s" $dest = [System.IO.Path]::GetTempFileName() $dest = [System.IO.Path]::ChangeExtension($dest, ".msi") $downloader = New-Object System.Net.WebClient @@ -48,7 +48,7 @@ Start-Process -FilePath msiexec -ArgumentList /qn, /i, $dest -Wait func (p *provisioner) windowsInstallChefClient(o terraform.UIOutput, comm communicator.Communicator) error { script := path.Join(path.Dir(comm.ScriptPath()), "ChefClient.ps1") - content := fmt.Sprintf(installScript, p.Version, p.HTTPProxy, strings.Join(p.NOProxy, ",")) + content := fmt.Sprintf(installScript, p.Channel, p.Version, p.HTTPProxy, strings.Join(p.NOProxy, ",")) // Copy the script to the new instance if err := comm.UploadScript(script, strings.NewReader(content)); err != nil { diff --git a/builtin/provisioners/chef/windows_provisioner_test.go b/builtin/provisioners/chef/windows_provisioner_test.go index 566f8c457..3697b7e24 100644 --- a/builtin/provisioners/chef/windows_provisioner_test.go +++ b/builtin/provisioners/chef/windows_provisioner_test.go @@ -72,6 +72,26 @@ func TestResourceProvider_windowsInstallChefClient(t *testing.T) { "ChefClient.ps1": versionWindowsInstallScript, }, }, + + "Channel": { + Config: map[string]interface{}{ + "channel": "current", + "node_name": "nodename1", + "run_list": []interface{}{"cookbook::recipe"}, + "server_url": "https://chef.local", + "user_name": "bob", + "user_key": "USER-KEY", + "version": "11.18.6", + }, + + Commands: map[string]bool{ + "powershell -NoProfile -ExecutionPolicy Bypass -File ChefClient.ps1": true, + }, + + UploadScripts: map[string]string{ + "ChefClient.ps1": channelWindowsInstallScript, + }, + }, } o := new(terraform.MockUIOutput) @@ -219,7 +239,7 @@ switch ($winver) if ([System.IntPtr]::Size -eq 4) {$machine_arch = "i686"} else {$machine_arch = "x86_64"} -$url = "http://www.chef.io/chef/download?p=windows&pv=$machine_os&m=$machine_arch&v=" +$url = "http://omnitruck.chef.io/stable/chef/download?p=windows&pv=$machine_os&m=$machine_arch&v=" $dest = [System.IO.Path]::GetTempFileName() $dest = [System.IO.Path]::ChangeExtension($dest, ".msi") $downloader = New-Object System.Net.WebClient @@ -256,7 +276,7 @@ switch ($winver) if ([System.IntPtr]::Size -eq 4) {$machine_arch = "i686"} else {$machine_arch = "x86_64"} -$url = "http://www.chef.io/chef/download?p=windows&pv=$machine_os&m=$machine_arch&v=" +$url = "http://omnitruck.chef.io/stable/chef/download?p=windows&pv=$machine_os&m=$machine_arch&v=" $dest = [System.IO.Path]::GetTempFileName() $dest = [System.IO.Path]::ChangeExtension($dest, ".msi") $downloader = New-Object System.Net.WebClient @@ -293,7 +313,43 @@ switch ($winver) if ([System.IntPtr]::Size -eq 4) {$machine_arch = "i686"} else {$machine_arch = "x86_64"} -$url = "http://www.chef.io/chef/download?p=windows&pv=$machine_os&m=$machine_arch&v=11.18.6" +$url = "http://omnitruck.chef.io/stable/chef/download?p=windows&pv=$machine_os&m=$machine_arch&v=11.18.6" +$dest = [System.IO.Path]::GetTempFileName() +$dest = [System.IO.Path]::ChangeExtension($dest, ".msi") +$downloader = New-Object System.Net.WebClient + +$http_proxy = '' +if ($http_proxy -ne '') { + $no_proxy = '' + if ($no_proxy -eq ''){ + $no_proxy = "127.0.0.1" + } + + $proxy = New-Object System.Net.WebProxy($http_proxy, $true, ,$no_proxy.Split(',')) + $downloader.proxy = $proxy +} + +Write-Host 'Downloading Chef Client...' +$downloader.DownloadFile($url, $dest) + +Write-Host 'Installing Chef Client...' +Start-Process -FilePath msiexec -ArgumentList /qn, /i, $dest -Wait +` +const channelWindowsInstallScript = ` +$winver = [System.Environment]::OSVersion.Version | % {"{0}.{1}" -f $_.Major,$_.Minor} + +switch ($winver) +{ + "6.0" {$machine_os = "2008"} + "6.1" {$machine_os = "2008r2"} + "6.2" {$machine_os = "2012"} + "6.3" {$machine_os = "2012"} + default {$machine_os = "2008r2"} +} + +if ([System.IntPtr]::Size -eq 4) {$machine_arch = "i686"} else {$machine_arch = "x86_64"} + +$url = "http://omnitruck.chef.io/current/chef/download?p=windows&pv=$machine_os&m=$machine_arch&v=11.18.6" $dest = [System.IO.Path]::GetTempFileName() $dest = [System.IO.Path]::ChangeExtension($dest, ".msi") $downloader = New-Object System.Net.WebClient From 45ed0e1f44bc830023daae6ab1b9c6cded069e56 Mon Sep 17 00:00:00 2001 From: Sander van Harmelen Date: Tue, 20 Mar 2018 11:55:49 +0100 Subject: [PATCH 15/23] Update CHANGELOG.md --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index ebb8bfefd..a3ca4a66b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,8 @@ ## 0.11.5 (Unreleased) +IMPROVEMENTS: + +* provisioner/chef: Allow specifying a channel [GH-17355] ## 0.11.4 (March 15, 2018) From 36777e30222823ea63ab31766d80ec5aa5d8f105 Mon Sep 17 00:00:00 2001 From: Sander van Harmelen Date: Tue, 20 Mar 2018 12:16:17 +0100 Subject: [PATCH 16/23] Add a line to describe the new Chef provisioner option (#17640) --- website/docs/provisioners/chef.html.markdown | 3 +++ 1 file changed, 3 insertions(+) diff --git a/website/docs/provisioners/chef.html.markdown b/website/docs/provisioners/chef.html.markdown index d6e097927..7ce2267ff 100644 --- a/website/docs/provisioners/chef.html.markdown +++ b/website/docs/provisioners/chef.html.markdown @@ -65,6 +65,9 @@ The following arguments are supported: for the new node. These can also be loaded from a file on disk using the [`file()` interpolation function](/docs/configuration/interpolation.html#file_path_). +* `channel (string)` - (Optional) The Chef Client release channel to install from. If not + set, the `stable` channel will be used. + * `client_options (array)` - (Optional) A list of optional Chef Client configuration options. See the [Chef Client ](https://docs.chef.io/config_rb_client.html) documentation for all available options. From 9b4b5f2a7212a393f00144f9b826852a8b703567 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Tue, 20 Mar 2018 13:06:28 -0400 Subject: [PATCH 17/23] use correct context for communicator.Retry The timeout for a provisioner is expected to only apply to the initial connection. Keep the context for the communicator.Retry separate from the global cancellation context. --- .../provisioners/chef/resource_provisioner.go | 4 ++-- .../provisioners/file/resource_provisioner.go | 8 ++++---- .../habitat/resource_provisioner.go | 4 ++-- .../remote-exec/resource_provisioner.go | 15 +++++++++------ .../salt-masterless/resource_provisioner.go | 18 +++++++++--------- 5 files changed, 26 insertions(+), 23 deletions(-) diff --git a/builtin/provisioners/chef/resource_provisioner.go b/builtin/provisioners/chef/resource_provisioner.go index 36d3eb88d..cfcf8485a 100644 --- a/builtin/provisioners/chef/resource_provisioner.go +++ b/builtin/provisioners/chef/resource_provisioner.go @@ -312,11 +312,11 @@ func applyFn(ctx context.Context) error { return err } - ctx, cancel := context.WithTimeout(ctx, comm.Timeout()) + retryCtx, cancel := context.WithTimeout(ctx, comm.Timeout()) defer cancel() // Wait and retry until we establish the connection - err = communicator.Retry(ctx, func() error { + err = communicator.Retry(retryCtx, func() error { return comm.Connect(o) }) if err != nil { diff --git a/builtin/provisioners/file/resource_provisioner.go b/builtin/provisioners/file/resource_provisioner.go index 5514250d7..26f2f4daf 100644 --- a/builtin/provisioners/file/resource_provisioner.go +++ b/builtin/provisioners/file/resource_provisioner.go @@ -48,9 +48,6 @@ func applyFn(ctx context.Context) error { return err } - ctx, cancel := context.WithTimeout(ctx, comm.Timeout()) - defer cancel() - // Get the source src, deleteSource, err := getSrc(data) if err != nil { @@ -99,8 +96,11 @@ func getSrc(data *schema.ResourceData) (string, bool, error) { // copyFiles is used to copy the files from a source to a destination func copyFiles(ctx context.Context, comm communicator.Communicator, src, dst string) error { + retryCtx, cancel := context.WithTimeout(ctx, comm.Timeout()) + defer cancel() + // Wait and retry until we establish the connection - err := communicator.Retry(ctx, func() error { + err := communicator.Retry(retryCtx, func() error { return comm.Connect(nil) }) if err != nil { diff --git a/builtin/provisioners/habitat/resource_provisioner.go b/builtin/provisioners/habitat/resource_provisioner.go index ada06a88c..8d9403ee6 100644 --- a/builtin/provisioners/habitat/resource_provisioner.go +++ b/builtin/provisioners/habitat/resource_provisioner.go @@ -231,10 +231,10 @@ func applyFn(ctx context.Context) error { return err } - ctx, cancel := context.WithTimeout(ctx, comm.Timeout()) + retryCtx, cancel := context.WithTimeout(ctx, comm.Timeout()) defer cancel() - err = communicator.Retry(ctx, func() error { + err = communicator.Retry(retryCtx, func() error { return comm.Connect(o) }) diff --git a/builtin/provisioners/remote-exec/resource_provisioner.go b/builtin/provisioners/remote-exec/resource_provisioner.go index 8c3d671b9..ca769767d 100644 --- a/builtin/provisioners/remote-exec/resource_provisioner.go +++ b/builtin/provisioners/remote-exec/resource_provisioner.go @@ -157,20 +157,23 @@ func runScripts( comm communicator.Communicator, scripts []io.ReadCloser) error { - // Wait for the context to end and then disconnect - go func() { - <-ctx.Done() - comm.Disconnect() - }() + retryCtx, cancel := context.WithTimeout(ctx, comm.Timeout()) + defer cancel() // Wait and retry until we establish the connection - err := communicator.Retry(ctx, func() error { + err := communicator.Retry(retryCtx, func() error { return comm.Connect(o) }) if err != nil { return err } + // Wait for the context to end and then disconnect + go func() { + <-ctx.Done() + comm.Disconnect() + }() + for _, script := range scripts { var cmd *remote.Cmd diff --git a/builtin/provisioners/salt-masterless/resource_provisioner.go b/builtin/provisioners/salt-masterless/resource_provisioner.go index dd19d419e..1d7594523 100644 --- a/builtin/provisioners/salt-masterless/resource_provisioner.go +++ b/builtin/provisioners/salt-masterless/resource_provisioner.go @@ -131,17 +131,11 @@ func applyFn(ctx context.Context) error { return err } - ctx, cancelFunc := context.WithTimeout(ctx, comm.Timeout()) - defer cancelFunc() - - // Wait for the context to end and then disconnect - go func() { - <-ctx.Done() - comm.Disconnect() - }() + retryCtx, cancel := context.WithTimeout(ctx, comm.Timeout()) + defer cancel() // Wait and retry until we establish the connection - err = communicator.Retry(ctx, func() error { + err = communicator.Retry(retryCtx, func() error { return comm.Connect(o) }) @@ -149,6 +143,12 @@ func applyFn(ctx context.Context) error { return err } + // Wait for the context to end and then disconnect + go func() { + <-ctx.Done() + comm.Disconnect() + }() + var src, dst string o.Output("Provisioning with Salt...") From 2954d9849a78d44801d60eec5edcf1e235ada715 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Tue, 20 Mar 2018 14:23:32 -0400 Subject: [PATCH 18/23] add some more functionality to MockCommunicator --- communicator/communicator_mock.go | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/communicator/communicator_mock.go b/communicator/communicator_mock.go index 7f887b4d3..49304a070 100644 --- a/communicator/communicator_mock.go +++ b/communicator/communicator_mock.go @@ -18,6 +18,9 @@ type MockCommunicator struct { Uploads map[string]string UploadScripts map[string]string UploadDirs map[string]string + CommandFunc func(*remote.Cmd) error + DisconnectFunc func() error + ConnTimeout time.Duration } // Connect implementation of communicator.Communicator interface @@ -27,11 +30,17 @@ func (c *MockCommunicator) Connect(o terraform.UIOutput) error { // Disconnect implementation of communicator.Communicator interface func (c *MockCommunicator) Disconnect() error { + if c.DisconnectFunc != nil { + return c.DisconnectFunc() + } return nil } // Timeout implementation of communicator.Communicator interface func (c *MockCommunicator) Timeout() time.Duration { + if c.ConnTimeout != 0 { + return c.ConnTimeout + } return time.Duration(5 * time.Second) } @@ -44,6 +53,10 @@ func (c *MockCommunicator) ScriptPath() string { func (c *MockCommunicator) Start(r *remote.Cmd) error { r.Init() + if c.CommandFunc != nil { + return c.CommandFunc(r) + } + if !c.Commands[r.Command] { return fmt.Errorf("Command not found!") } From 56acda00bc09c2822b4edd525899cf1381d86b2d Mon Sep 17 00:00:00 2001 From: James Bardin Date: Tue, 20 Mar 2018 14:24:01 -0400 Subject: [PATCH 19/23] add timeout test to remote-exec Add a test to remote-exec to make sure the proper timeout is honored during apply. TODO: we need some test helpers for provisioners, so they can all be verified. --- .../remote-exec/resource_provisioner_test.go | 57 +++++++++++++++++++ 1 file changed, 57 insertions(+) diff --git a/builtin/provisioners/remote-exec/resource_provisioner_test.go b/builtin/provisioners/remote-exec/resource_provisioner_test.go index a6e024fe5..38d5ffdd4 100644 --- a/builtin/provisioners/remote-exec/resource_provisioner_test.go +++ b/builtin/provisioners/remote-exec/resource_provisioner_test.go @@ -2,11 +2,15 @@ package remoteexec import ( "bytes" + "context" "io" "testing" + "time" "strings" + "github.com/hashicorp/terraform/communicator" + "github.com/hashicorp/terraform/communicator/remote" "github.com/hashicorp/terraform/config" "github.com/hashicorp/terraform/helper/schema" "github.com/hashicorp/terraform/terraform" @@ -206,6 +210,59 @@ func TestResourceProvider_CollectScripts_scriptsEmpty(t *testing.T) { } } +func TestProvisionerTimeout(t *testing.T) { + o := new(terraform.MockUIOutput) + c := new(communicator.MockCommunicator) + + disconnected := make(chan struct{}) + c.DisconnectFunc = func() error { + close(disconnected) + return nil + } + + completed := make(chan struct{}) + c.CommandFunc = func(cmd *remote.Cmd) error { + defer close(completed) + cmd.Init() + time.Sleep(2 * time.Second) + cmd.SetExitStatus(0, nil) + return nil + } + c.ConnTimeout = time.Second + c.UploadScripts = map[string]string{"hello": "echo hello"} + c.RemoteScriptPath = "hello" + + p := Provisioner().(*schema.Provisioner) + conf := map[string]interface{}{ + "inline": []interface{}{"echo hello"}, + } + + scripts, err := collectScripts(schema.TestResourceDataRaw( + t, p.Schema, conf)) + if err != nil { + t.Fatal(err) + } + + ctx := context.Background() + + done := make(chan struct{}) + + go func() { + defer close(done) + if err := runScripts(ctx, o, c, scripts); err != nil { + t.Fatal(err) + } + }() + + select { + case <-disconnected: + t.Fatal("communicator disconnected before command completed") + case <-completed: + } + + <-done +} + func testConfig(t *testing.T, c map[string]interface{}) *terraform.ResourceConfig { r, err := config.NewRawConfig(c) if err != nil { From 90a75422fb593bae6e9cb7436fb59528c33b89e1 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Tue, 20 Mar 2018 12:44:12 -0400 Subject: [PATCH 20/23] unlock state in console, import, graph, and push The state locking improvements for the regular command had the side effect of locking the state in the console, import, graph and push commands. Those commands had been updated to get a state via the Backend.Context method, which locks the state whenever possible, and now need to call Unlock directly. Add Unlock calls to all commands that call Context directly. --- command/console.go | 7 +++++++ command/graph.go | 7 +++++++ command/import.go | 7 +++++++ command/import_test.go | 7 +++++++ command/push.go | 7 +++++++ 5 files changed, 35 insertions(+) diff --git a/command/console.go b/command/console.go index cf7e15f61..f8537f3fc 100644 --- a/command/console.go +++ b/command/console.go @@ -81,6 +81,13 @@ func (c *ConsoleCommand) Run(args []string) int { return 1 } + defer func() { + err := opReq.StateLocker.Unlock(nil) + if err != nil { + c.Ui.Error(err.Error()) + } + }() + // Setup the UI so we can output directly to stdout ui := &cli.BasicUi{ Writer: wrappedstreams.Stdout(), diff --git a/command/graph.go b/command/graph.go index 7723043e8..6b9a0c524 100644 --- a/command/graph.go +++ b/command/graph.go @@ -112,6 +112,13 @@ func (c *GraphCommand) Run(args []string) int { return 1 } + defer func() { + err := opReq.StateLocker.Unlock(nil) + if err != nil { + c.Ui.Error(err.Error()) + } + }() + // Determine the graph type graphType := terraform.GraphTypePlan if plan != nil { diff --git a/command/import.go b/command/import.go index cbaeec5f4..b19fa3293 100644 --- a/command/import.go +++ b/command/import.go @@ -184,6 +184,13 @@ func (c *ImportCommand) Run(args []string) int { return 1 } + defer func() { + err := opReq.StateLocker.Unlock(nil) + if err != nil { + c.Ui.Error(err.Error()) + } + }() + // Perform the import. Note that as you can see it is possible for this // API to import more than one resource at once. For now, we only allow // one while we stabilize this feature. diff --git a/command/import_test.go b/command/import_test.go index 587c4393f..9becdcd33 100644 --- a/command/import_test.go +++ b/command/import_test.go @@ -4,6 +4,7 @@ import ( "fmt" "io/ioutil" "os" + "path/filepath" "strings" "testing" @@ -175,11 +176,17 @@ func TestImport_remoteState(t *testing.T) { "test_instance.foo", "bar", } + if code := c.Run(args); code != 0 { fmt.Println(ui.OutputWriter) t.Fatalf("bad: %d\n\n%s", code, ui.ErrorWriter.String()) } + // verify that the local state was unlocked after import + if _, err := os.Stat(filepath.Join(td, fmt.Sprintf(".%s.lock.info", statePath))); !os.IsNotExist(err) { + t.Fatal("state left locked after import") + } + // Verify that we were called if !configured { t.Fatal("Configure should be called") diff --git a/command/push.go b/command/push.go index 039696fd3..a73689d4a 100644 --- a/command/push.go +++ b/command/push.go @@ -146,6 +146,13 @@ func (c *PushCommand) Run(args []string) int { return 1 } + defer func() { + err := opReq.StateLocker.Unlock(nil) + if err != nil { + c.Ui.Error(err.Error()) + } + }() + // Get the configuration config := ctx.Module().Config() if name == "" { From 31f2e6a4638214680c80fcefe9c0559fd20f0801 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Maccagnoni?= Date: Wed, 21 Mar 2018 17:17:24 +0100 Subject: [PATCH 21/23] website: Correct descriptions on provider category pages --- website/docs/providers/type/community-index.html.markdown | 2 +- website/docs/providers/type/infra-index.html.markdown | 2 +- website/docs/providers/type/misc-index.html.markdown | 2 +- website/docs/providers/type/network-index.html.markdown | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/website/docs/providers/type/community-index.html.markdown b/website/docs/providers/type/community-index.html.markdown index 1cc76124c..9260b1a04 100644 --- a/website/docs/providers/type/community-index.html.markdown +++ b/website/docs/providers/type/community-index.html.markdown @@ -3,7 +3,7 @@ layout: "docs" page_title: "Community Providers" sidebar_current: "docs-providers-community" description: |- - Category for database vendors. + Category for community-built providers. --- # Community Providers diff --git a/website/docs/providers/type/infra-index.html.markdown b/website/docs/providers/type/infra-index.html.markdown index 18cb0aef4..73ffbf5b4 100644 --- a/website/docs/providers/type/infra-index.html.markdown +++ b/website/docs/providers/type/infra-index.html.markdown @@ -3,7 +3,7 @@ layout: "docs" page_title: "Infrastructure Software Providers" sidebar_current: "docs-providers-infra" description: |- - Category for standard cloud vendors. + Category for infrastructure management vendors. --- # Infrastructure Software Providers diff --git a/website/docs/providers/type/misc-index.html.markdown b/website/docs/providers/type/misc-index.html.markdown index 288bcdebd..36d2dd93c 100644 --- a/website/docs/providers/type/misc-index.html.markdown +++ b/website/docs/providers/type/misc-index.html.markdown @@ -3,7 +3,7 @@ layout: "docs" page_title: "Misc Providers" sidebar_current: "docs-providers-misc" description: |- - Category for database vendors. + Category for miscellaneous vendors. --- # Miscellaneous Providers diff --git a/website/docs/providers/type/network-index.html.markdown b/website/docs/providers/type/network-index.html.markdown index fd9e4537f..ef0a66557 100644 --- a/website/docs/providers/type/network-index.html.markdown +++ b/website/docs/providers/type/network-index.html.markdown @@ -3,7 +3,7 @@ layout: "docs" page_title: "Network Providers" sidebar_current: "docs-providers-network" description: |- - Category for netowrk vendors. + Category for network vendors. --- # Network Providers From 4cdb6dff0b01faa197be8081949d984a94b070e1 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Wed, 21 Mar 2018 17:02:41 -0400 Subject: [PATCH 22/23] update CHANGELOG.md --- CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index a3ca4a66b..ecfb849f7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,12 @@ IMPROVEMENTS: * provisioner/chef: Allow specifying a channel [GH-17355] +BUG FIXES: + +* core: Fix the timeout handling for provisioners [GH-17646] +* core: Ensure that state is unlocked after running console, import, graph or push commands [GH-17645] +* core: Don't open multiple file descriptors for local state files, which would cause reading the state to fail on Windows [GH-17636] + ## 0.11.4 (March 15, 2018) IMPROVEMENTS: From 342c529aa58d824f2f2ed1f6ec6118059876aad0 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Wed, 21 Mar 2018 21:05:10 +0000 Subject: [PATCH 23/23] v0.11.5 --- CHANGELOG.md | 10 +++++----- version/version.go | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ecfb849f7..94f0b5c53 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,14 +1,14 @@ -## 0.11.5 (Unreleased) +## 0.11.5 (March 21, 2018) IMPROVEMENTS: -* provisioner/chef: Allow specifying a channel [GH-17355] +* provisioner/chef: Allow specifying a channel ([#17355](https://github.com/hashicorp/terraform/issues/17355)) BUG FIXES: -* core: Fix the timeout handling for provisioners [GH-17646] -* core: Ensure that state is unlocked after running console, import, graph or push commands [GH-17645] -* core: Don't open multiple file descriptors for local state files, which would cause reading the state to fail on Windows [GH-17636] +* core: Fix the timeout handling for provisioners ([#17646](https://github.com/hashicorp/terraform/issues/17646)) +* core: Ensure that state is unlocked after running console, import, graph or push commands ([#17645](https://github.com/hashicorp/terraform/issues/17645)) +* core: Don't open multiple file descriptors for local state files, which would cause reading the state to fail on Windows [[#17636](https://github.com/hashicorp/terraform/issues/17636)] ## 0.11.4 (March 15, 2018) diff --git a/version/version.go b/version/version.go index 88a63a13d..8d9562e1c 100644 --- a/version/version.go +++ b/version/version.go @@ -16,7 +16,7 @@ const Version = "0.11.5" // A pre-release marker for the version. If this is "" (empty string) // then it means that it is a final release. Otherwise, this is a pre-release // such as "dev" (in development), "beta", "rc1", etc. -var Prerelease = "dev" +var Prerelease = "" // SemVer is an instance of version.Version. This has the secondary // benefit of verifying during tests and init time that our version is a