diff --git a/cmd/tofu/commands.go b/cmd/tofu/commands.go index e77fcc31e0..5959982142 100644 --- a/cmd/tofu/commands.go +++ b/cmd/tofu/commands.go @@ -123,7 +123,10 @@ func initCommands( AllowExperimentalFeatures: experimentsAreAllowed(), - ProviderSourceLocationConfig: providerSourceLocationConfig(), + // ProviderSourceLocationConfig is used for some commands that do not make + // use of the OpenTofu configuration files. Therefore, there is no way to configure + // the retries from other places than env vars. + ProviderSourceLocationConfig: providerSourceLocationConfigFromEnv(), } // The command list is included in the tofu -help diff --git a/cmd/tofu/provider_source.go b/cmd/tofu/provider_source.go index ef71968a08..b4427bdd0c 100644 --- a/cmd/tofu/provider_source.go +++ b/cmd/tofu/provider_source.go @@ -12,7 +12,6 @@ import ( "net/url" "os" "path/filepath" - "strconv" "github.com/apparentlymart/go-userdirs/userdirs" "github.com/opentofu/svchost/disco" @@ -61,7 +60,7 @@ func explicitProviderSource( log.Printf("[DEBUG] Explicit provider installation configuration is set") for _, methodConfig := range config.Methods { - source, moreDiags := providerSourceForCLIConfigLocation(ctx, methodConfig.Location, registryClientConfig, services, getOCICredsPolicy) + source, moreDiags := providerSourceForCLIConfigLocation(ctx, methodConfig.Location, methodConfig.Retries, registryClientConfig, services, getOCICredsPolicy) diags = diags.Append(moreDiags) if moreDiags.HasErrors() { continue @@ -208,7 +207,7 @@ func implicitProviderSource( // local copy will take precedence. searchRules = append(searchRules, getproviders.MultiSourceSelector{ Source: getproviders.NewMemoizeSource( - getproviders.NewRegistrySource(ctx, services, newRegistryHTTPClient(ctx, registryClientConfig), providerSourceLocationConfig()), + getproviders.NewRegistrySource(ctx, services, newRegistryHTTPClient(ctx, registryClientConfig), providerSourceLocationConfigFromEnv()), ), Exclude: directExcluded, }) @@ -219,13 +218,14 @@ func implicitProviderSource( func providerSourceForCLIConfigLocation( ctx context.Context, loc cliconfig.ProviderInstallationLocation, + locationRetries cliconfig.ProviderInstallationMethodRetries, registryClientConfig *cliconfig.RegistryProtocolsConfig, services *disco.Disco, makeOCICredsPolicy ociCredsPolicyBuilder, ) (getproviders.Source, tfdiags.Diagnostics) { if loc == cliconfig.ProviderInstallationDirect { return getproviders.NewMemoizeSource( - getproviders.NewRegistrySource(ctx, services, newRegistryHTTPClient(ctx, registryClientConfig), providerSourceLocationConfig()), + getproviders.NewRegistrySource(ctx, services, newRegistryHTTPClient(ctx, registryClientConfig), providerSourceLocationConfig(locationRetries)), ), nil } @@ -259,7 +259,7 @@ func providerSourceForCLIConfigLocation( // this client is not suitable for the HTTP mirror source, so we // don't use this client directly. httpTimeout := newRegistryHTTPClient(ctx, registryClientConfig).HTTPClient.Timeout - return getproviders.NewHTTPMirrorSource(ctx, url, services.CredentialsSource(), httpTimeout, providerSourceLocationConfig()), nil + return getproviders.NewHTTPMirrorSource(ctx, url, services.CredentialsSource(), httpTimeout, providerSourceLocationConfig(locationRetries)), nil case cliconfig.ProviderInstallationOCIMirror: mappingFunc := loc.RepositoryMapping @@ -300,33 +300,28 @@ func providerDevOverrides(configs []*cliconfig.ProviderInstallation) map[addrs.P return configs[0].DevOverrides } -const ( - // providerDownloadRetryCountEnvName is the environment variable name used to customize - // the HTTP retry count for module downloads. - providerDownloadRetryCountEnvName = "TF_PROVIDER_DOWNLOAD_RETRY" - - providerDownloadDefaultRetry = 2 -) - -// providerDownloadRetry will attempt for requests with retryable errors, like 502 status codes -func providerDownloadRetry() int { - res := providerDownloadDefaultRetry - if v := os.Getenv(providerDownloadRetryCountEnvName); v != "" { - retry, err := strconv.Atoi(v) - if err == nil && retry > 0 { - res = retry - } - } - return res -} - // providerSourceLocationConfig is meant to build a global configuration for the // remote locations to download a provider from. This is built out of the // TF_PROVIDER_DOWNLOAD_RETRY env variable and is meant to be passed through // [getproviders.Source] all the way down to the [getproviders.PackageLocation] // to be able to tweak the configurations of the http clients used there. -func providerSourceLocationConfig() getproviders.LocationConfig { +func providerSourceLocationConfig(locationRetries cliconfig.ProviderInstallationMethodRetries) getproviders.LocationConfig { + // If there is no configuration for the retries in .tofurc, get the one from env variable + retries, configured := locationRetries() + if !configured { + retries = cliconfig.ProviderDownloadRetries() + } return getproviders.LocationConfig{ - ProviderDownloadRetries: providerDownloadRetry(), + ProviderDownloadRetries: retries, + } +} + +// providerSourceLocationConfigFromEnv is similar to providerSourceLocationConfig but does not +// take into account the information from the configuration. This is like so because for some +// commands, there is no specific tofurc configuration for the retry, so we want to use the +// env variable if defined and if not, its default. +func providerSourceLocationConfigFromEnv() getproviders.LocationConfig { + return getproviders.LocationConfig{ + ProviderDownloadRetries: cliconfig.ProviderDownloadRetries(), } } diff --git a/cmd/tofu/provider_source_test.go b/cmd/tofu/provider_source_test.go index e3dd1568bd..0fac9d0a01 100644 --- a/cmd/tofu/provider_source_test.go +++ b/cmd/tofu/provider_source_test.go @@ -7,8 +7,11 @@ package main import ( "context" + "net/http" + "net/http/httptest" "os" "path/filepath" + "strings" "testing" "time" @@ -160,6 +163,121 @@ func TestProviderSource(t *testing.T) { } } +// TestInitProviderSourceForCLIConfigLocationWithRetries tests how the retries are handled and passed over to the +// [getproviders.PackageLocation]. +// Normally, this should have been an e2e test, but there is no easy way to mock a `direct` registry +// to be able to test that the retries actually work properly. +// +// This test contains no tests for [cliconfig.ProviderInstallationNetworkMirror] on purpose since +// there is no way to create [getproviders.HTTPMirrorSource] +// with an on-the-fly generated TLS certificate since it's required by the +// [getproviders.HTTPMirrorSource] inner working. +// Instead, the retries functionality for `network_mirror` is tested into an e2e called +// "TestProviderNetworkMirrorRetries" since that way we were able to provide the TLS certificate +// into the child process instead. +func TestInitProviderSourceForCLIConfigLocationWithRetries(t *testing.T) { + registryHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/providers/v1/test/exists/0.0.0/download/foo_os/bar_arch": + _, _ = w.Write([]byte(`{"os":"foo_os","arch":"bar_arch","download_url":"/providers/v1/test/exists_0.0.0.zip","shasum":"4cbc33c22abdebe3a3679666d4052ec95c40bd8904a9458f90cf934363a14cc7"}`)) + case "/providers/v1/test/exists_0.0.0.zip": + w.WriteHeader(http.StatusInternalServerError) + return + default: + w.WriteHeader(http.StatusNotFound) + } + }) + cases := map[string]struct { + methodType cliconfig.ProviderInstallationLocation + tofurcRetries cliconfig.ProviderInstallationMethodRetries + envVars map[string]string + expectedErrMsg string + }{ + "no tofurc.direct.download_retry_count configured, no TF_PROVIDER_DOWNLOAD_RETRY, default TF_PROVIDER_DOWNLOAD_RETRY used": { + methodType: cliconfig.ProviderInstallationDirect, + tofurcRetries: func() (int, bool) { + return 0, false + }, + envVars: nil, + expectedErrMsg: "/providers/v1/test/exists_0.0.0.zip giving up after 3 attempt(s)", + }, + "no tofurc.direct.download_retry_count, TF_PROVIDER_DOWNLOAD_RETRY defined, TF_PROVIDER_DOWNLOAD_RETRY used": { + methodType: cliconfig.ProviderInstallationDirect, + tofurcRetries: func() (int, bool) { + return 0, false + }, + envVars: map[string]string{ + "TF_PROVIDER_DOWNLOAD_RETRY": "1", + }, + expectedErrMsg: "/providers/v1/test/exists_0.0.0.zip giving up after 2 attempt(s)", + }, + "defined tofurc.direct.download_retry_count as 0, no TF_PROVIDER_DOWNLOAD_RETRY, tofurc used": { + methodType: cliconfig.ProviderInstallationDirect, + tofurcRetries: func() (int, bool) { + return 0, true + }, + envVars: nil, + expectedErrMsg: "/providers/v1/test/exists_0.0.0.zip giving up after 1 attempt(s)", + }, + "defined tofurc.direct.download_retry_count as 1, no TF_PROVIDER_DOWNLOAD_RETRY, tofurc used": { + methodType: cliconfig.ProviderInstallationDirect, + tofurcRetries: func() (int, bool) { + return 1, true + }, + envVars: nil, + expectedErrMsg: "/providers/v1/test/exists_0.0.0.zip giving up after 2 attempt(s)", + }, + "defined tofurc.direct.download_retry_count as 1, TF_PROVIDER_DOWNLOAD_RETRY defined as 2, tofurc used": { + methodType: cliconfig.ProviderInstallationDirect, + tofurcRetries: func() (int, bool) { + return 1, true + }, + envVars: map[string]string{ + "TF_PROVIDER_DOWNLOAD_RETRY": "2", + }, + expectedErrMsg: "/providers/v1/test/exists_0.0.0.zip giving up after 2 attempt(s)", + }, + } + for name, tt := range cases { + t.Run(name, func(t *testing.T) { + methodType := tt.methodType + retries := tt.tofurcRetries // This is what simulates the configuration of .tofurc + for k, v := range tt.envVars { + t.Setenv(k, v) + } + + server := httptest.NewServer(registryHandler) + defer server.Close() + disco := disco.New(disco.WithHTTPClient(server.Client())) + disco.ForceHostServices("local.testing", map[string]any{ + "providers.v1": server.URL + "/providers/v1/", + }) + + providerSrc, diags := providerSourceForCLIConfigLocation(t.Context(), methodType, retries, &cliconfig.RegistryProtocolsConfig{}, disco, nil) + if diags.HasErrors() { + t.Fatalf("unexpected error creating the provider source: %s", diags) + } + provider := addrs.MustParseProviderSourceString("local.testing/test/exists") + platform := getproviders.Platform{OS: "foo_os", Arch: "bar_arch"} + packageMeta, err := providerSrc.PackageMeta(t.Context(), provider, getproviders.UnspecifiedVersion, platform) + if err != nil { + t.Fatalf("unexpected error getting the package meta information: %s", err) + } + // We nullify the authentication since the meaning of this test is not to + packageMeta.Authentication = nil + + instDir := t.TempDir() + _, err = packageMeta.Location.InstallProviderPackage(t.Context(), packageMeta, instDir, nil) + if err == nil { + t.Fatalf("expected to get an error from the installation of the provider but got nothing") + } + if contains := tt.expectedErrMsg; !strings.Contains(err.Error(), contains) { + t.Fatalf("expected the error from the installation to contain %q but it didn't: %s", contains, err) + } + }) + } +} + func TestConfigureProviderDownloadRetry(t *testing.T) { tests := []struct { name string @@ -170,7 +288,7 @@ func TestConfigureProviderDownloadRetry(t *testing.T) { { name: "when no TF_PROVIDER_DOWNLOAD_RETRY env var, default retry attempts used for provider download", expectedConfig: getproviders.LocationConfig{ - ProviderDownloadRetries: providerDownloadDefaultRetry, + ProviderDownloadRetries: 2, }, }, { @@ -191,7 +309,9 @@ func TestConfigureProviderDownloadRetry(t *testing.T) { } // Call the function under test - got := providerSourceLocationConfig() + got := providerSourceLocationConfig(func() (int, bool) { + return 0, false + }) if diff := cmp.Diff(tt.expectedConfig, got); diff != "" { t.Fatalf("expected no diff. got:\n%s", diff) diff --git a/internal/command/cliconfig/cliconfig.go b/internal/command/cliconfig/cliconfig.go index de2bd9441c..fb14320d2c 100644 --- a/internal/command/cliconfig/cliconfig.go +++ b/internal/command/cliconfig/cliconfig.go @@ -21,6 +21,7 @@ import ( "log" "os" "path/filepath" + "strconv" "strings" "github.com/hashicorp/hcl" @@ -501,3 +502,23 @@ func cliConfigFileOverride() string { } return configFilePath } + +const ( + // providerDownloadRetryCountEnvName is the environment variable name used to customize + // the HTTP retry count for module downloads. + providerDownloadRetryCountEnvName = "TF_PROVIDER_DOWNLOAD_RETRY" + + providerDownloadDefaultRetry = 2 +) + +// ProviderDownloadRetries will attempt for requests with retryable errors, like 502 status codes +func ProviderDownloadRetries() int { + r := providerDownloadDefaultRetry + if v := os.Getenv(providerDownloadRetryCountEnvName); v != "" { + retry, err := strconv.Atoi(v) + if err == nil && retry > 0 { + r = retry + } + } + return r +} diff --git a/internal/command/cliconfig/credentials_test.go b/internal/command/cliconfig/credentials_test.go index 0ae9b3ae46..6dfd388cc8 100644 --- a/internal/command/cliconfig/credentials_test.go +++ b/internal/command/cliconfig/credentials_test.go @@ -153,7 +153,7 @@ func TestCredentialsForHost(t *testing.T) { } }) - t.Run("periods are ok", func(t *testing.T) { + t.Run("periods are Ok", func(t *testing.T) { envName := "TF_TOKEN_configured.example.com" expectedToken := "configured-by-env" diff --git a/internal/command/cliconfig/provider_installation.go b/internal/command/cliconfig/provider_installation.go index 50577c1f6b..343105ce41 100644 --- a/internal/command/cliconfig/provider_installation.go +++ b/internal/command/cliconfig/provider_installation.go @@ -149,11 +149,13 @@ func decodeProviderInstallationFromConfig(hclFile *hclast.File) ([]*ProviderInst methodTypeStr := methodBlock.Keys[0].Token.Value().(string) var location ProviderInstallationLocation var include, exclude []string + var retriesF ProviderInstallationMethodRetries switch methodTypeStr { case "direct": type BodyContent struct { - Include []string `hcl:"include"` - Exclude []string `hcl:"exclude"` + Include []string `hcl:"include"` + Exclude []string `hcl:"exclude"` + DownloadRetries *int `hcl:"download_retry_count"` } var bodyContent BodyContent err := hcl.DecodeObject(&bodyContent, methodBody) @@ -168,6 +170,12 @@ func decodeProviderInstallationFromConfig(hclFile *hclast.File) ([]*ProviderInst location = ProviderInstallationDirect include = bodyContent.Include exclude = bodyContent.Exclude + retriesF = func() (int, bool) { + if bodyContent.DownloadRetries == nil { + return 0, false + } + return *bodyContent.DownloadRetries, true + } case "filesystem_mirror": type BodyContent struct { Path string `hcl:"path"` @@ -197,9 +205,10 @@ func decodeProviderInstallationFromConfig(hclFile *hclast.File) ([]*ProviderInst exclude = bodyContent.Exclude case "network_mirror": type BodyContent struct { - URL string `hcl:"url"` - Include []string `hcl:"include"` - Exclude []string `hcl:"exclude"` + URL string `hcl:"url"` + Include []string `hcl:"include"` + Exclude []string `hcl:"exclude"` + DownloadRetries *int `hcl:"download_retry_count"` } var bodyContent BodyContent err := hcl.DecodeObject(&bodyContent, methodBody) @@ -222,6 +231,12 @@ func decodeProviderInstallationFromConfig(hclFile *hclast.File) ([]*ProviderInst location = ProviderInstallationNetworkMirror(bodyContent.URL) include = bodyContent.Include exclude = bodyContent.Exclude + retriesF = func() (int, bool) { + if bodyContent.DownloadRetries == nil { + return 0, false + } + return *bodyContent.DownloadRetries, true + } case "oci_mirror": var moreDiags tfdiags.Diagnostics location, include, exclude, moreDiags = decodeOCIMirrorInstallationMethodBlock(methodBody) @@ -229,6 +244,9 @@ func decodeProviderInstallationFromConfig(hclFile *hclast.File) ([]*ProviderInst if moreDiags.HasErrors() { continue } + // NOTE: We want to introduce a retry and a timeout for the oci_mirror block too, but that needs + // a different design than the one we have for direct and network_mirror. + // Details in: https://github.com/opentofu/opentofu/issues/3392 case "dev_overrides": if len(pi.Methods) > 0 { // We require dev_overrides to appear first if it's present, @@ -289,6 +307,7 @@ func decodeProviderInstallationFromConfig(hclFile *hclast.File) ([]*ProviderInst Location: location, Include: include, Exclude: exclude, + Retries: retriesF, }) } @@ -564,6 +583,7 @@ type ProviderInstallationMethod struct { Location ProviderInstallationLocation Include []string `hcl:"include"` Exclude []string `hcl:"exclude"` + Retries ProviderInstallationMethodRetries } // ProviderInstallationLocation is an interface type representing the @@ -639,3 +659,12 @@ func (i ProviderInstallationOCIMirror) GoString() string { // mismatches in tests, so just naming the type is good enough for now. return "cliconfig.ProviderInstallationNetworkMirror{/*...*/}" } + +// ProviderInstallationMethodRetries defines the function to return the +// configured, or lack of, retries (`download_retry_count`) configured for +// a provider installation method. +// This will return a number >= 0 and a bool in case the value actually +// comes from the CLI configuration. When the bool returned is [false], the +// number returned will be 0, meaning that the configuration was not +// specified. +type ProviderInstallationMethodRetries func() (int, bool) diff --git a/internal/command/cliconfig/provider_installation_test.go b/internal/command/cliconfig/provider_installation_test.go index 807ee8b61b..2cddd37e7d 100644 --- a/internal/command/cliconfig/provider_installation_test.go +++ b/internal/command/cliconfig/provider_installation_test.go @@ -37,6 +37,9 @@ func TestLoadConfig_providerInstallation(t *testing.T) { Location: ProviderInstallationNetworkMirror("https://tf-Mirror.example.com/"), Include: []string{"registry.opentofu.org/*/*"}, Exclude: []string{"registry.OpenTofu.org/foobar/*"}, + Retries: func() (int, bool) { + return 2, true + }, }, { Location: ProviderInstallationFilesystemMirror("/tmp/example2"), @@ -44,6 +47,9 @@ func TestLoadConfig_providerInstallation(t *testing.T) { { Location: ProviderInstallationDirect, Exclude: []string{"example.com/*/*"}, + Retries: func() (int, bool) { + return 3, true + }, }, }, @@ -55,7 +61,17 @@ func TestLoadConfig_providerInstallation(t *testing.T) { }, } - if diff := cmp.Diff(want, got); diff != "" { + if diff := cmp.Diff(want, got, cmp.Comparer(func(a, b ProviderInstallationMethodRetries) bool { + if (a == nil && b != nil) || (a != nil && b == nil) { + return false + } + if a == nil && b == nil { + return true + } + ar, aok := a() + br, bok := b() + return ar == br && aok == bok + })); diff != "" { t.Errorf("wrong result\n%s", diff) } }) diff --git a/internal/command/cliconfig/testdata/provider-installation b/internal/command/cliconfig/testdata/provider-installation index 6f14db2984..247a24b1a4 100644 --- a/internal/command/cliconfig/testdata/provider-installation +++ b/internal/command/cliconfig/testdata/provider-installation @@ -11,11 +11,13 @@ provider_installation { url = "https://tf-Mirror.example.com/" include = ["registry.opentofu.org/*/*"] exclude = ["registry.OpenTofu.org/foobar/*"] + download_retry_count = 2 } filesystem_mirror { path = "/tmp/example2" } direct { exclude = ["example.com/*/*"] + download_retry_count = 3 } } diff --git a/internal/command/cliconfig/testdata/provider-installation.json b/internal/command/cliconfig/testdata/provider-installation.json index f73023523e..fa717f0d97 100644 --- a/internal/command/cliconfig/testdata/provider-installation.json +++ b/internal/command/cliconfig/testdata/provider-installation.json @@ -11,13 +11,15 @@ "network_mirror": [{ "url": "https://tf-Mirror.example.com/", "include": ["registry.opentofu.org/*/*"], - "exclude": ["registry.OpenTofu.org/foobar/*"] + "exclude": ["registry.OpenTofu.org/foobar/*"], + "download_retry_count": 2 }], "filesystem_mirror": [{ "path": "/tmp/example2" }], "direct": [{ - "exclude": ["example.com/*/*"] + "exclude": ["example.com/*/*"], + "download_retry_count": 3 }] } } diff --git a/internal/command/e2etest/provider_network_mirror_test.go b/internal/command/e2etest/provider_network_mirror_test.go new file mode 100644 index 0000000000..2432e880fa --- /dev/null +++ b/internal/command/e2etest/provider_network_mirror_test.go @@ -0,0 +1,148 @@ +// Copyright (c) The OpenTofu Authors +// SPDX-License-Identifier: MPL-2.0 +// Copyright (c) 2023 HashiCorp, Inc. +// SPDX-License-Identifier: MPL-2.0 + +package e2etest + +import ( + "fmt" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "runtime" + "strings" + "testing" + + "github.com/opentofu/opentofu/internal/e2e" +) + +// TestProviderNetworkMirrorRetries checks that the retries configuration for downloading the +// provider from a network mirror source is handled correctly alone and also together with +// the TF_PROVIDER_DOWNLOAD_RETRY env var. +// +// This is testing the same thing as TestInitProviderSourceForCLIConfigLocationWithRetries but +// it's an e2e test instead because there is no way to inject the TLS certificate properly into +// the underlying implementation that talks with a remote network mirror source. +func TestProviderNetworkMirrorRetries(t *testing.T) { + // Our typical rule for external service access in e2etests is that it's okay + // to access servers run by the OpenTofu project when TF_ACC=1 is set in the + // environment. However, this particular test is checking a network mirror source + // that needs to be configured with a TLS certificate and that certificate to be given + // in the `tofu` child process env vars to be used for talking with the server. + // The entirety of this process can be done without actual network access so it can run + // without the TF_ACC=1. + // + // We restrict this for only linux_amd64 to make it easier to maintain since the whole purpose + // of the test has nothing to do with the actual platform it runs on. + // + // The fake server uses a locally-generated TLS certificate and so we'll need to + // override the trusted certs for the child process so it can be used successfully. + // The Go toolchain only allows doing that by environment variable on Unix systems + // other than macOS. + // Additionally, for ease of maintenance, the stubbed data inside this test is only + // for linux_amd64 so we cannot run this for other platforms that still support the + // previously mentioned limitation. + if runtime.GOOS != "linux" || runtime.GOARCH != "amd64" { + t.Skip("this test is suitable only for linux_amd64") + } + networkMirrorHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/example.com/test/test/index.json": + w.Header().Add("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"versions":{"0.0.1":{}}}`)) + return + case "/example.com/test/test/0.0.1.json": + w.Header().Add("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"archives": {"linux_amd64": {"url": "terraform-provider-test_0.0.1_linux_amd64.zip","hashes": []}}}`)) + return + case "/example.com/test/test/terraform-provider-test_0.0.1_linux_amd64.zip": + w.WriteHeader(http.StatusInternalServerError) + return + default: + w.WriteHeader(http.StatusNotFound) + } + }) + + cases := map[string]struct { + tofurcRetriesConfigEntry string + envVars map[string]string + expectedErrMsg string + }{ + "no tofurc.network_mirror.download_retry_count, no TF_PROVIDER_DOWNLOAD_RETRY, default TF_PROVIDER_DOWNLOAD_RETRY used": { + tofurcRetriesConfigEntry: "", + envVars: nil, + expectedErrMsg: "/example.com/test/test/terraform-provider-test_0.0.1_linux_amd64.zip giving up after 3 attempt(s)", + }, + "no tofurc.network_mirror.download_retry_count, TF_PROVIDER_DOWNLOAD_RETRY defined, TF_PROVIDER_DOWNLOAD_RETRY used": { + tofurcRetriesConfigEntry: "", + envVars: map[string]string{ + "TF_PROVIDER_DOWNLOAD_RETRY": "1", + }, + expectedErrMsg: "/example.com/test/test/terraform-provider-test_0.0.1_linux_amd64.zip giving up after 2 attempt(s)", + }, + "defined tofurc.network_mirror.download_retry_count as 0, no TF_PROVIDER_DOWNLOAD_RETRY, tofurc used": { + tofurcRetriesConfigEntry: "download_retry_count = 0", + envVars: nil, + expectedErrMsg: "/example.com/test/test/terraform-provider-test_0.0.1_linux_amd64.zip giving up after 1 attempt(s)", + }, + "defined tofurc.network_mirror.download_retry_count as 1, no TF_PROVIDER_DOWNLOAD_RETRY, tofurc used": { + tofurcRetriesConfigEntry: "download_retry_count = 1", + envVars: nil, + expectedErrMsg: "/example.com/test/test/terraform-provider-test_0.0.1_linux_amd64.zip giving up after 2 attempt(s)", + }, + "defined tofurc.network_mirror.download_retry_count as 1, TF_PROVIDER_DOWNLOAD_RETRY defined as 2, tofurc used": { + tofurcRetriesConfigEntry: "download_retry_count = 1", + envVars: map[string]string{ + "TF_PROVIDER_DOWNLOAD_RETRY": "2", + }, + expectedErrMsg: "/example.com/test/test/terraform-provider-test_0.0.1_linux_amd64.zip giving up after 2 attempt(s)", + }, + } + for name, tt := range cases { + t.Run(name, func(t *testing.T) { + server := httptest.NewTLSServer(networkMirrorHandler) + defer server.Close() + registryAddr := server.URL + + // We need to pass both our fake registry server's certificate and the CLI config + // for interacting with it to the child process using some temporary files on disk. + tempDir := t.TempDir() + certFile := filepath.Join(tempDir, "testserver.crt") + if err := writeCertificatePEMFile(certFile, server.Certificate()); err != nil { + t.Fatalf("failed to create temporary certificate file: %s", err) + } + cliConfigFile := filepath.Join(tempDir, "cliconfig.tfrc") + cliConfigSrc := fmt.Sprintf(` + provider_installation { + network_mirror { + url = "%s" + %s + } + } + `, registryAddr, tt.tofurcRetriesConfigEntry) + if err := os.WriteFile(cliConfigFile, []byte(cliConfigSrc), os.ModePerm); err != nil { + t.Fatalf("failed to create temporary CLI configuration file: %s", err) + } + dataDir := filepath.Join(tempDir, ".terraform") + + tf := e2e.NewBinary(t, tofuBin, "testdata/provider-network-mirror") + tf.AddEnv("SSL_CERT_FILE=" + certFile) + tf.AddEnv("TF_CLI_CONFIG_FILE=" + cliConfigFile) + tf.AddEnv("TF_DATA_DIR=" + dataDir) + for k, v := range tt.envVars { + tf.AddEnv(fmt.Sprintf("%s=%s", k, v)) + } + _, stderr, err := tf.Run("init", "-backend=false") + if err == nil { + t.Fatalf("expected `tofu init` to fail but got no error") + } + t.Logf("stderr:\n%s", stderr) + cleanStderr := SanitizeStderr(stderr) + if contains := tt.expectedErrMsg; !strings.Contains(cleanStderr, contains) { + t.Fatalf("expected the error from the installation to contain %q but it doesn't", contains) + } + }) + } +} diff --git a/internal/command/e2etest/testdata/provider-network-mirror/main.tf b/internal/command/e2etest/testdata/provider-network-mirror/main.tf new file mode 100644 index 0000000000..855d5591ac --- /dev/null +++ b/internal/command/e2etest/testdata/provider-network-mirror/main.tf @@ -0,0 +1,8 @@ +terraform { + required_providers { + test = { + source = "example.com/test/test" + version = "0.0.1" + } + } +} diff --git a/website/docs/cli/config/config-file.mdx b/website/docs/cli/config/config-file.mdx index 92fa09e0bb..211247bac8 100644 --- a/website/docs/cli/config/config-file.mdx +++ b/website/docs/cli/config/config-file.mdx @@ -278,6 +278,29 @@ identity, but a network mirror with a TLS certificate can potentially serve modified copies of upstream providers with malicious content. ::: +The `direct` and `network_mirror` methods support an additional argument +`download_retry_count`. When specified, it will be used to retry downloading +the provider binary in case of retryable errors (connection reset and a range of 500 errors). +```hcl +provider_installation { + network_mirror { + url = "https://example.com/" + include = ["example.com/*/*"] + download_retry_count = 2 + } + direct { + exclude = ["example.com/*/*"] + download_retry_count = 3 + } +} +``` +You can use this argument to disable fully the retries in case of a specific method (`download_retry_count = 0`). +:::info +When used together with [`TF_PROVIDER_DOWNLOAD_RETRY`](./environment-variables.mdx#tf_provider_download_retry), the configuration value +will be used for each method that it is configured with. For the methods +with no such configuration, the environment variable will be used instead. +::: + OpenTofu will try all of the specified methods whose include and exclude patterns match a given provider, and select the newest version available across all of those methods that matches the version constraint given in each diff --git a/website/docs/cli/config/environment-variables.mdx b/website/docs/cli/config/environment-variables.mdx index 80e8d32695..29faf0a686 100644 --- a/website/docs/cli/config/environment-variables.mdx +++ b/website/docs/cli/config/environment-variables.mdx @@ -165,7 +165,9 @@ For more details on `.terraformignore`, please see [Excluding Files from Upload Set `TF_PROVIDER_DOWNLOAD_RETRY` to configure the max number of request retries the remote provider client will attempt for client connection errors or 500-range responses that are safe to retry. - +:::info +This will be used only for the methods without the argument `download_retry_count` specified in [OpenTofu CLI configuration file](./config-file.mdx#explicit-installation-method-configuration). +::: ```shell export TF_PROVIDER_DOWNLOAD_RETRY=3 ```