From e57d8add353766d7c7d8ded65cebd9c317c32910 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Mon, 10 Jun 2024 15:03:28 -0400 Subject: [PATCH 1/3] Check lock file diags in mirror command The lock file diagnostic were not being checked in the mirror command, so an incomplete or broken lock file might cause the cli to crash. --- .../command/e2etest/providers_mirror_test.go | 18 ++++++-- .../.terraform.lock.hcl | 44 +++++++++++++++++++ .../terraform-providers-mirror.tf | 7 +++ internal/command/providers_mirror.go | 12 ++--- 4 files changed, 72 insertions(+), 9 deletions(-) create mode 100644 internal/command/e2etest/testdata/terraform-providers-mirror-with-broken-lock-file/.terraform.lock.hcl create mode 100644 internal/command/e2etest/testdata/terraform-providers-mirror-with-broken-lock-file/terraform-providers-mirror.tf diff --git a/internal/command/e2etest/providers_mirror_test.go b/internal/command/e2etest/providers_mirror_test.go index e3ac3221656e..ed331718a6cf 100644 --- a/internal/command/e2etest/providers_mirror_test.go +++ b/internal/command/e2etest/providers_mirror_test.go @@ -7,6 +7,7 @@ import ( "os" "path/filepath" "sort" + "strings" "testing" "github.com/google/go-cmp/cmp" @@ -20,14 +21,18 @@ import ( // compromise for now to keep these tests relatively simple. func TestTerraformProvidersMirror(t *testing.T) { - testTerraformProvidersMirror(t, "terraform-providers-mirror") + testTerraformProvidersMirror(t, "terraform-providers-mirror", "") } func TestTerraformProvidersMirrorWithLockFile(t *testing.T) { - testTerraformProvidersMirror(t, "terraform-providers-mirror-with-lock-file") + testTerraformProvidersMirror(t, "terraform-providers-mirror-with-lock-file", "") } -func testTerraformProvidersMirror(t *testing.T, fixture string) { +func TestTerraformProvidersMirrorWithBrokenLockFile(t *testing.T) { + testTerraformProvidersMirror(t, "terraform-providers-mirror-with-broken-lock-file", "Inconsistent dependency lock file") +} + +func testTerraformProvidersMirror(t *testing.T, fixture string, errMsg string) { // This test reaches out to releases.hashicorp.com to download the // template and null providers, so it can only run if network access is // allowed. @@ -40,6 +45,13 @@ func testTerraformProvidersMirror(t *testing.T, fixture string) { tf := e2e.NewBinary(t, terraformBin, fixturePath) stdout, stderr, err := tf.Run("providers", "mirror", "-platform=linux_amd64", "-platform=windows_386", outputDir) + if errMsg != "" { + if !strings.Contains(stderr, errMsg) { + t.Fatalf("expected error %q, got %q\n", errMsg, stderr) + } + return + } + if err != nil { t.Fatalf("unexpected error: %s\nstdout:\n%s\nstderr:\n%s", err, stdout, stderr) } diff --git a/internal/command/e2etest/testdata/terraform-providers-mirror-with-broken-lock-file/.terraform.lock.hcl b/internal/command/e2etest/testdata/terraform-providers-mirror-with-broken-lock-file/.terraform.lock.hcl new file mode 100644 index 000000000000..7e16d31628e1 --- /dev/null +++ b/internal/command/e2etest/testdata/terraform-providers-mirror-with-broken-lock-file/.terraform.lock.hcl @@ -0,0 +1,44 @@ +# This file is maintained automatically by "terraform init". +# Manual edits may be lost in future updates. + +provider "registry.terraform.io/hashicorp/template" { + version = "2.1.1" + constraints = "2.1.1" + hashes = [ + "h1:fBNBluCX4pWlYEw5ZyCTHB00E+3BDSe7GjRzF1ojcvU=", + "h1:x2/zuJFN/oOUpE1C1nSk4n86AA2zASOyy2BUdFYcpXw=", + "zh:05fddf3cacb607f623c2b221c3e9ab724079deca0b703b2738e9d55c10e31717", + "zh:1a250b29274f3e340ea775bf9bd57476e982bca1fb4b59343fb3126e75dfd85c", + "zh:284735b9bd0e416ec02c0844e7f4ebbd4b5744140a21606e33f16eb14640cbf1", + "zh:2e9d246094ac8a68951015d40f42145e795b31d7c84fee20fa9f997b3d428906", + "zh:65e8e73860662a0c0698c8a8d35c857302f1fe3f41947e7c048c49a541a9c7f1", + "zh:70dacd22d0c93b2000948c06ded67fa147d992a0353737438f24a61e3f956c41", + "zh:aa1a0321e79e08ffb52789ab0af3896c493d436de7396d154d09a0be7d5d50e1", + "zh:bea4c276c4df9d117f19c4266d060db9b48c865ac7a71d2e77a27866c19bfaf5", + "zh:de04cb0cb046dad184f5bb783659cf98d88c6798db038cbf5a2c3c08e853d444", + "zh:de3c45a4fa1f756aa4db3350c021d1c0f9b23640cff77e0ba4df4eeb8eae957f", + "zh:e3cf2db204f64ad4e288af00fabc6a8af13a6687aba60a7e1ce0ea215a9580b1", + "zh:f795833225207d2eee022b91d26bee18d5e518e70912dd7a1d2a0eff2cbe4f1d", + ] +} + +provider "registry.terraform.io/hashicorp/broken" { + version = "2.1.1" + constraints = "2.1.1" + hashes = [ + "h1:fBNBluCX4pWlYEw5ZyCTHB00E+3BDSe7GjRzF1ojcvU=", + "h1:x2/zuJFN/oOUpE1C1nSk4n86AA2zASOyy2BUdFYcpXw=", + "zh:05fddf3cacb607f623c2b221c3e9ab724079deca0b703b2738e9d55c10e31717", + "zh:1a250b29274f3e340ea775bf9bd57476e982bca1fb4b59343fb3126e75dfd85c", + "zh:284735b9bd0e416ec02c0844e7f4ebbd4b5744140a21606e33f16eb14640cbf1", + "zh:2e9d246094ac8a68951015d40f42145e795b31d7c84fee20fa9f997b3d428906", + "zh:65e8e73860662a0c0698c8a8d35c857302f1fe3f41947e7c048c49a541a9c7f1", + "zh:70dacd22d0c93b2000948c06ded67fa147d992a0353737438f24a61e3f956c41", + "zh:aa1a0321e79e08ffb52789ab0af3896c493d436de7396d154d09a0be7d5d50e1", + "zh:bea4c276c4df9d117f19c4266d060db9b48c865ac7a71d2e77a27866c19bfaf5", + "zh:de04cb0cb046dad184f5bb783659cf98d88c6798db038cbf5a2c3c08e853d444", + "zh:de3c45a4fa1f756aa4db3350c021d1c0f9b23640cff77e0ba4df4eeb8eae957f", + "zh:e3cf2db204f64ad4e288af00fabc6a8af13a6687aba60a7e1ce0ea215a9580b1", + "zh:f795833225207d2eee022b91d26bee18d5e518e70912dd7a1d2a0eff2cbe4f1d", + ] +} diff --git a/internal/command/e2etest/testdata/terraform-providers-mirror-with-broken-lock-file/terraform-providers-mirror.tf b/internal/command/e2etest/testdata/terraform-providers-mirror-with-broken-lock-file/terraform-providers-mirror.tf new file mode 100644 index 000000000000..1598a278354b --- /dev/null +++ b/internal/command/e2etest/testdata/terraform-providers-mirror-with-broken-lock-file/terraform-providers-mirror.tf @@ -0,0 +1,7 @@ +terraform { + required_providers { + template = { source = "hashicorp/template" } + null = { source = "hashicorp/null" } + terraform = { source = "terraform.io/builtin/terraform" } + } +} diff --git a/internal/command/providers_mirror.go b/internal/command/providers_mirror.go index a7f0d4556905..87143a8851bb 100644 --- a/internal/command/providers_mirror.go +++ b/internal/command/providers_mirror.go @@ -88,12 +88,6 @@ func (c *ProvidersMirrorCommand) Run(args []string) int { lockedDeps, lockedDepsDiags := c.Meta.lockedDependencies() diags = diags.Append(lockedDepsDiags) - // If we have any error diagnostics already then we won't proceed further. - if diags.HasErrors() { - c.showDiagnostics(diags) - return 1 - } - // If lock file is present, validate it against configuration if !lockedDeps.Empty() { if errs := config.VerifyDependencySelections(lockedDeps); len(errs) > 0 { @@ -105,6 +99,12 @@ func (c *ProvidersMirrorCommand) Run(args []string) int { } } + // If we have any error diagnostics already then we won't proceed further. + if diags.HasErrors() { + c.showDiagnostics(diags) + return 1 + } + // Unlike other commands, this command always consults the origin registry // for every provider so that it can be used to update a local mirror // directory without needing to first disable that local mirror From 3875ea36560d0383541f48db3f4e863e0e2f7c70 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Tue, 11 Jun 2024 16:33:28 -0400 Subject: [PATCH 2/3] add -lock-file flag to providers mirror command The providers mirror command was updated to inspect the lock file, however that was not part of the original intent for the command, and it's possible that the command needs to be run without a lock file. Since we have been honoring the lock file for the past few releases, let's keep that consistent and allow disabling the file with `-lock-file=false`. --- .../command/e2etest/providers_mirror_test.go | 143 ++++++++++-------- .../terraform-providers-mirror.tf | 4 +- internal/command/providers_mirror.go | 9 +- 3 files changed, 90 insertions(+), 66 deletions(-) diff --git a/internal/command/e2etest/providers_mirror_test.go b/internal/command/e2etest/providers_mirror_test.go index ed331718a6cf..12552882c980 100644 --- a/internal/command/e2etest/providers_mirror_test.go +++ b/internal/command/e2etest/providers_mirror_test.go @@ -19,78 +19,97 @@ import ( // interacts directly with Terraform Registry and the full details of that are // tricky to mock. Such a mock is _possible_, but we're using e2etest as a // compromise for now to keep these tests relatively simple. - func TestTerraformProvidersMirror(t *testing.T) { - testTerraformProvidersMirror(t, "terraform-providers-mirror", "") -} - -func TestTerraformProvidersMirrorWithLockFile(t *testing.T) { - testTerraformProvidersMirror(t, "terraform-providers-mirror-with-lock-file", "") -} - -func TestTerraformProvidersMirrorWithBrokenLockFile(t *testing.T) { - testTerraformProvidersMirror(t, "terraform-providers-mirror-with-broken-lock-file", "Inconsistent dependency lock file") -} - -func testTerraformProvidersMirror(t *testing.T, fixture string, errMsg string) { // This test reaches out to releases.hashicorp.com to download the // template and null providers, so it can only run if network access is // allowed. skipIfCannotAccessNetwork(t) - outputDir := t.TempDir() - t.Logf("creating mirror directory in %s", outputDir) + for _, test := range []struct { + name string + args []string + err string + }{ + { + name: "terraform-providers-mirror", + args: []string{"-platform=linux_amd64", "-platform=windows_386"}, + }, + { + name: "terraform-providers-mirror-with-lock-file", + args: []string{"-platform=linux_amd64", "-platform=windows_386"}, + }, + { + // should ignore lock file + name: "terraform-providers-mirror-with-broken-lock-file", + args: []string{"-platform=linux_amd64", "-platform=windows_386", "-lock-file=false"}, + }, + { + name: "terraform-providers-mirror-with-broken-lock-file", + args: []string{"-platform=linux_amd64", "-platform=windows_386", "-lock-file=true"}, + err: "Inconsistent dependency lock file", + }, + } { + t.Run(test.name, func(t *testing.T) { + outputDir := t.TempDir() + t.Logf("creating mirror directory in %s", outputDir) - fixturePath := filepath.Join("testdata", fixture) - tf := e2e.NewBinary(t, terraformBin, fixturePath) + fixturePath := filepath.Join("testdata", test.name) + tf := e2e.NewBinary(t, terraformBin, fixturePath) - stdout, stderr, err := tf.Run("providers", "mirror", "-platform=linux_amd64", "-platform=windows_386", outputDir) - if errMsg != "" { - if !strings.Contains(stderr, errMsg) { - t.Fatalf("expected error %q, got %q\n", errMsg, stderr) - } - return - } + args := []string{"providers", "mirror"} + args = append(args, test.args...) + args = append(args, outputDir) - if err != nil { - t.Fatalf("unexpected error: %s\nstdout:\n%s\nstderr:\n%s", err, stdout, stderr) - } + stdout, stderr, err := tf.Run(args...) + if test.err != "" { + if !strings.Contains(stderr, test.err) { + t.Fatalf("expected error %q, got %q\n", test.err, stderr) + } + return + } - // The test fixture includes exact version constraints for the two - // providers it depends on so that the following should remain stable. - // In the (unlikely) event that these particular versions of these - // providers are removed from the registry, this test will start to fail. - want := []string{ - "registry.terraform.io/hashicorp/null/2.1.0.json", - "registry.terraform.io/hashicorp/null/index.json", - "registry.terraform.io/hashicorp/null/terraform-provider-null_2.1.0_linux_amd64.zip", - "registry.terraform.io/hashicorp/null/terraform-provider-null_2.1.0_windows_386.zip", - "registry.terraform.io/hashicorp/template/2.1.1.json", - "registry.terraform.io/hashicorp/template/index.json", - "registry.terraform.io/hashicorp/template/terraform-provider-template_2.1.1_linux_amd64.zip", - "registry.terraform.io/hashicorp/template/terraform-provider-template_2.1.1_windows_386.zip", - } - var got []string - walkErr := filepath.Walk(outputDir, func(path string, info os.FileInfo, err error) error { - if err != nil { - return err - } - if info.IsDir() { - return nil // we only care about leaf files for this test - } - relPath, err := filepath.Rel(outputDir, path) - if err != nil { - return err - } - got = append(got, filepath.ToSlash(relPath)) - return nil - }) - if walkErr != nil { - t.Fatal(walkErr) - } - sort.Strings(got) + if err != nil { + t.Fatalf("unexpected error: %s\nstdout:\n%s\nstderr:\n%s", err, stdout, stderr) + } + + // The test fixture includes exact version constraints for the two + // providers it depends on so that the following should remain stable. + // In the (unlikely) event that these particular versions of these + // providers are removed from the registry, this test will start to fail. + want := []string{ + "registry.terraform.io/hashicorp/null/2.1.0.json", + "registry.terraform.io/hashicorp/null/index.json", + "registry.terraform.io/hashicorp/null/terraform-provider-null_2.1.0_linux_amd64.zip", + "registry.terraform.io/hashicorp/null/terraform-provider-null_2.1.0_windows_386.zip", + "registry.terraform.io/hashicorp/template/2.1.1.json", + "registry.terraform.io/hashicorp/template/index.json", + "registry.terraform.io/hashicorp/template/terraform-provider-template_2.1.1_linux_amd64.zip", + "registry.terraform.io/hashicorp/template/terraform-provider-template_2.1.1_windows_386.zip", + } + var got []string + walkErr := filepath.Walk(outputDir, func(path string, info os.FileInfo, err error) error { + if err != nil { + return err + } + if info.IsDir() { + return nil // we only care about leaf files for this test + } + relPath, err := filepath.Rel(outputDir, path) + if err != nil { + return err + } + got = append(got, filepath.ToSlash(relPath)) + return nil + }) + if walkErr != nil { + t.Fatal(walkErr) + } + sort.Strings(got) + + if diff := cmp.Diff(want, got); diff != "" { + t.Errorf("unexpected files in result\n%s", diff) + } - if diff := cmp.Diff(want, got); diff != "" { - t.Errorf("unexpected files in result\n%s", diff) + }) } } diff --git a/internal/command/e2etest/testdata/terraform-providers-mirror-with-broken-lock-file/terraform-providers-mirror.tf b/internal/command/e2etest/testdata/terraform-providers-mirror-with-broken-lock-file/terraform-providers-mirror.tf index 1598a278354b..4b31e0301200 100644 --- a/internal/command/e2etest/testdata/terraform-providers-mirror-with-broken-lock-file/terraform-providers-mirror.tf +++ b/internal/command/e2etest/testdata/terraform-providers-mirror-with-broken-lock-file/terraform-providers-mirror.tf @@ -1,7 +1,7 @@ terraform { required_providers { - template = { source = "hashicorp/template" } - null = { source = "hashicorp/null" } + template = { version = "2.1.1" } + null = { source = "hashicorp/null", version = "2.1.0" } terraform = { source = "terraform.io/builtin/terraform" } } } diff --git a/internal/command/providers_mirror.go b/internal/command/providers_mirror.go index 87143a8851bb..ec2904eda106 100644 --- a/internal/command/providers_mirror.go +++ b/internal/command/providers_mirror.go @@ -34,8 +34,13 @@ func (c *ProvidersMirrorCommand) Synopsis() string { func (c *ProvidersMirrorCommand) Run(args []string) int { args = c.Meta.process(args) cmdFlags := c.Meta.defaultFlagSet("providers mirror") + var optPlatforms arguments.FlagStringSlice cmdFlags.Var(&optPlatforms, "platform", "target platform") + + var optLockFile bool + cmdFlags.BoolVar(&optLockFile, "lock-file", true, "use lock file") + cmdFlags.Usage = func() { c.Ui.Error(c.Help()) } if err := cmdFlags.Parse(args); err != nil { c.Ui.Error(fmt.Sprintf("Error parsing command-line flags: %s\n", err.Error())) @@ -89,7 +94,7 @@ func (c *ProvidersMirrorCommand) Run(args []string) int { diags = diags.Append(lockedDepsDiags) // If lock file is present, validate it against configuration - if !lockedDeps.Empty() { + if !lockedDeps.Empty() && optLockFile { if errs := config.VerifyDependencySelections(lockedDeps); len(errs) > 0 { diags = diags.Append(tfdiags.Sourceless( tfdiags.Error, @@ -161,7 +166,7 @@ func (c *ProvidersMirrorCommand) Run(args []string) int { continue } selected := candidates.Newest() - if !lockedDeps.Empty() { + if !lockedDeps.Empty() && optLockFile { selected = lockedDeps.Provider(provider).Version() c.Ui.Output(fmt.Sprintf(" - Selected v%s to match dependency lock file", selected.String())) } else if len(constraintsStr) > 0 { From 3bf06b18fc1636c6d3731734d717c7860c59ead6 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Fri, 14 Jun 2024 16:23:36 -0400 Subject: [PATCH 3/3] update helptext and docs --- internal/command/providers_mirror.go | 5 +++++ website/docs/cli/commands/providers/mirror.mdx | 4 ++++ 2 files changed, 9 insertions(+) diff --git a/internal/command/providers_mirror.go b/internal/command/providers_mirror.go index ec2904eda106..d2292742f116 100644 --- a/internal/command/providers_mirror.go +++ b/internal/command/providers_mirror.go @@ -384,5 +384,10 @@ Options: Linux operating system running on an AMD64 or x86_64 CPU. Each provider is available only for a limited set of target platforms. + + -lock-file=false Ignore the provider lock file when fetching providers. + By default the mirror command will use the version info + in the lock file if the configuration directory has been + previously initialized. ` } diff --git a/website/docs/cli/commands/providers/mirror.mdx b/website/docs/cli/commands/providers/mirror.mdx index 129c55a658bb..fb48ae6797b9 100644 --- a/website/docs/cli/commands/providers/mirror.mdx +++ b/website/docs/cli/commands/providers/mirror.mdx @@ -55,6 +55,10 @@ This command supports the following additional option: architecture. For example, `linux_amd64` selects the Linux operating system running on an AMD64 or x86_64 CPU. +* `-lock-file=false` - Ignore the provider lock file when fetching providers. +By default the mirror command will use the version info in the lock file if the +configuration directory has been previously initialized. + You can run `terraform providers mirror` again on an existing mirror directory to update it with new packages. For example, you can add packages for a new target platform by re-running the command with the desired new `-platform=...`