diff --git a/cmd/tofu/commands.go b/cmd/tofu/commands.go index 241d538c2b..5e9f62de7b 100644 --- a/cmd/tofu/commands.go +++ b/cmd/tofu/commands.go @@ -114,7 +114,7 @@ func initCommands( // This ctx is used only to choose global configuration settings // for the client, and is not retained as part of the result for // making individual HTTP requests. - return newRegistryHTTPClient(ctx) + return newRegistryHTTPClient(ctx, config.RegistryProtocols) }, ModulePackageFetcher: modulePkgFetcher, ProviderSource: providerSrc, diff --git a/cmd/tofu/main.go b/cmd/tofu/main.go index 47d16ba112..8af17e0e3d 100644 --- a/cmd/tofu/main.go +++ b/cmd/tofu/main.go @@ -195,7 +195,7 @@ func realMain() int { log.Printf("[WARN] Cannot initialize remote host credentials manager: %s", err) credsSrc = nil // must be an untyped nil for newServiceDiscovery to understand "no credentials available" } - services := newServiceDiscovery(ctx, credsSrc) + services := newServiceDiscovery(ctx, config.RegistryProtocols, credsSrc) modulePkgFetcher := remoteModulePackageFetcher(ctx, config.OCICredentialsPolicy) @@ -240,7 +240,13 @@ func realMain() int { } } - providerSrc, diags := providerSource(ctx, config.ProviderInstallation, services, config.OCICredentialsPolicy, originalWd) + providerSrc, diags := providerSource(ctx, + config.ProviderInstallation, + config.RegistryProtocols, + services, + config.OCICredentialsPolicy, + originalWd, + ) if len(diags) > 0 { Ui.Error("There are some problems with the provider_installation configuration:") for _, diag := range diags { diff --git a/cmd/tofu/provider_source.go b/cmd/tofu/provider_source.go index 54f28ecdfb..36cdbf7f21 100644 --- a/cmd/tofu/provider_source.go +++ b/cmd/tofu/provider_source.go @@ -26,28 +26,41 @@ import ( // CLI configuration and some default search locations. This will be the // provider source used for provider installation in the "tofu init" // command, unless overridden by the special -plugin-dir option. -func providerSource(ctx context.Context, configs []*cliconfig.ProviderInstallation, services *disco.Disco, getOCICredsPolicy ociCredsPolicyBuilder, originalWorkingDir string) (getproviders.Source, tfdiags.Diagnostics) { +func providerSource( + ctx context.Context, + configs []*cliconfig.ProviderInstallation, + registryClientConfig *cliconfig.RegistryProtocolsConfig, + services *disco.Disco, + getOCICredsPolicy ociCredsPolicyBuilder, + originalWorkingDir string, +) (getproviders.Source, tfdiags.Diagnostics) { if len(configs) == 0 { // If there's no explicit installation configuration then we'll build // up an implicit one with direct registry installation along with // some automatically-selected local filesystem mirrors. - return implicitProviderSource(ctx, services, originalWorkingDir), nil + return implicitProviderSource(ctx, registryClientConfig, services, originalWorkingDir), nil } // There should only be zero or one configurations, which is checked by // the validation logic in the cliconfig package. Therefore we'll just // ignore any additional configurations in here. config := configs[0] - return explicitProviderSource(ctx, config, services, getOCICredsPolicy) + return explicitProviderSource(ctx, config, registryClientConfig, services, getOCICredsPolicy) } -func explicitProviderSource(ctx context.Context, config *cliconfig.ProviderInstallation, services *disco.Disco, getOCICredsPolicy ociCredsPolicyBuilder) (getproviders.Source, tfdiags.Diagnostics) { +func explicitProviderSource( + ctx context.Context, + config *cliconfig.ProviderInstallation, + registryClientConfig *cliconfig.RegistryProtocolsConfig, + services *disco.Disco, + getOCICredsPolicy ociCredsPolicyBuilder, +) (getproviders.Source, tfdiags.Diagnostics) { var diags tfdiags.Diagnostics var searchRules []getproviders.MultiSourceSelector log.Printf("[DEBUG] Explicit provider installation configuration is set") for _, methodConfig := range config.Methods { - source, moreDiags := providerSourceForCLIConfigLocation(ctx, methodConfig.Location, services, getOCICredsPolicy) + source, moreDiags := providerSourceForCLIConfigLocation(ctx, methodConfig.Location, registryClientConfig, services, getOCICredsPolicy) diags = diags.Append(moreDiags) if moreDiags.HasErrors() { continue @@ -92,7 +105,12 @@ func explicitProviderSource(ctx context.Context, config *cliconfig.ProviderInsta // one version available in a local directory are implicitly excluded from // direct installation, as if the user had listed them explicitly in the // "exclude" argument in the direct provider source in the CLI config. -func implicitProviderSource(ctx context.Context, services *disco.Disco, originalWorkingDir string) getproviders.Source { +func implicitProviderSource( + ctx context.Context, + registryClientConfig *cliconfig.RegistryProtocolsConfig, + services *disco.Disco, + originalWorkingDir string, +) getproviders.Source { // The local search directories we use for implicit configuration are: // - The "terraform.d/plugins" directory in the current working directory, // which we've historically documented as a place to put plugins as a @@ -189,7 +207,7 @@ func implicitProviderSource(ctx context.Context, services *disco.Disco, original // local copy will take precedence. searchRules = append(searchRules, getproviders.MultiSourceSelector{ Source: getproviders.NewMemoizeSource( - getproviders.NewRegistrySource(ctx, services, newRegistryHTTPClient(ctx)), + getproviders.NewRegistrySource(ctx, services, newRegistryHTTPClient(ctx, registryClientConfig)), ), Exclude: directExcluded, }) @@ -197,10 +215,16 @@ func implicitProviderSource(ctx context.Context, services *disco.Disco, original return getproviders.MultiSource(searchRules) } -func providerSourceForCLIConfigLocation(ctx context.Context, loc cliconfig.ProviderInstallationLocation, services *disco.Disco, makeOCICredsPolicy ociCredsPolicyBuilder) (getproviders.Source, tfdiags.Diagnostics) { +func providerSourceForCLIConfigLocation( + ctx context.Context, + loc cliconfig.ProviderInstallationLocation, + 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)), + getproviders.NewRegistrySource(ctx, services, newRegistryHTTPClient(ctx, registryClientConfig)), ), nil } @@ -233,7 +257,7 @@ func providerSourceForCLIConfigLocation(ctx context.Context, loc cliconfig.Provi // even though this isn't actually a registry. The other behavior of // this client is not suitable for the HTTP mirror source, so we // don't use this client directly. - httpTimeout := newRegistryHTTPClient(ctx).HTTPClient.Timeout + httpTimeout := newRegistryHTTPClient(ctx, registryClientConfig).HTTPClient.Timeout return getproviders.NewHTTPMirrorSource(ctx, url, services.CredentialsSource(), httpTimeout), nil case cliconfig.ProviderInstallationOCIMirror: diff --git a/cmd/tofu/provider_source_test.go b/cmd/tofu/provider_source_test.go index f39e8a95c3..2e36820665 100644 --- a/cmd/tofu/provider_source_test.go +++ b/cmd/tofu/provider_source_test.go @@ -10,6 +10,7 @@ import ( "os" "path/filepath" "testing" + "time" "github.com/opentofu/opentofu/internal/addrs" "github.com/opentofu/opentofu/internal/command/cliconfig" @@ -121,7 +122,19 @@ func TestProviderSource(t *testing.T) { } // Call the function under test - source, diags := providerSource(context.Background(), []*cliconfig.ProviderInstallation{}, services, ociCredsPolicy, originalWorkingDir) + source, diags := providerSource( + context.Background(), + []*cliconfig.ProviderInstallation{}, + &cliconfig.RegistryProtocolsConfig{ + RetryCount: 1, + RetryCountSet: true, + RequestTimeout: 10 * time.Second, + RequestTimeoutSet: true, + }, + services, + ociCredsPolicy, + originalWorkingDir, + ) // Verify no diagnostics were returned if len(diags) > 0 { diff --git a/cmd/tofu/registries_disco.go b/cmd/tofu/registries_disco.go index c7d793f179..06a091beff 100644 --- a/cmd/tofu/registries_disco.go +++ b/cmd/tofu/registries_disco.go @@ -8,34 +8,16 @@ package main import ( "context" "log" - "os" - "strconv" - "time" "github.com/hashicorp/go-retryablehttp" "github.com/opentofu/svchost/disco" "github.com/opentofu/svchost/svcauth" + "github.com/opentofu/opentofu/internal/command/cliconfig" "github.com/opentofu/opentofu/internal/httpclient" "github.com/opentofu/opentofu/internal/logging" ) -const ( - // registryDiscoveryRetryEnvName is the name of the environment variable that - // can be configured to customize number of retries for module and provider - // discovery requests with the remote registry. - registryDiscoveryRetryEnvName = "TF_REGISTRY_DISCOVERY_RETRY" - registryDiscoveryDefaultRetryCount = 1 - - // registryClientTimeoutEnvName is the name of the environment variable that - // can be configured to customize the timeout duration (seconds) for module - // and provider discovery with a remote registry. For historical reasons - // this also applies to all service discovery requests regardless of whether - // they are registry-related. - registryClientTimeoutEnvName = "TF_REGISTRY_CLIENT_TIMEOUT" - registryClientDefaultRequestTimeout = 10 * time.Second -) - // newServiceDiscovery returns a newly-created [disco.Disco] object that is // configured appropriately for use elsewhere in OpenTofu. // @@ -43,11 +25,11 @@ const ( // object should obtain authentication credentials for service discovery // requests. Passing a nil credSrc is acceptable and means that all discovery // requests are to be made anonymously. -func newServiceDiscovery(ctx context.Context, credSrc svcauth.CredentialsSource) *disco.Disco { +func newServiceDiscovery(ctx context.Context, registryClientConfig *cliconfig.RegistryProtocolsConfig, credSrc svcauth.CredentialsSource) *disco.Disco { // For historical reasons, the registry request retry policy also applies // to all service discovery requests, which we implement by using transport // from a HTTP httpClient that is configured for registry httpClient use. - registryHTTPClient := newRegistryHTTPClient(ctx) + registryHTTPClient := newRegistryHTTPClient(ctx, registryClientConfig) services := disco.New( disco.WithHTTPClient(registryHTTPClient.HTTPClient), disco.WithCredentials(credSrc), @@ -58,26 +40,8 @@ func newServiceDiscovery(ctx context.Context, credSrc svcauth.CredentialsSource) // newRegistryHTTPClient returns a new HTTP client configured to respect the // automatic retry behavior expected for registry requests and service discovery // requests. -func newRegistryHTTPClient(ctx context.Context) *retryablehttp.Client { - // The retry count is configurable by environment variable. - retryCount := registryDiscoveryDefaultRetryCount - if v := os.Getenv(registryDiscoveryRetryEnvName); v != "" { - override, err := strconv.Atoi(v) - if err == nil && override > 0 { - retryCount = override - } - } - - // The timeout is also configurable by environment variable. - timeout := registryClientDefaultRequestTimeout - if v := os.Getenv(registryClientTimeoutEnvName); v != "" { - override, err := strconv.Atoi(v) - if err == nil && timeout > 0 { - timeout = time.Duration(override) * time.Second - } - } - - client := httpclient.NewForRegistryRequests(ctx, retryCount, timeout) +func newRegistryHTTPClient(ctx context.Context, config *cliconfig.RegistryProtocolsConfig) *retryablehttp.Client { + client := httpclient.NewForRegistryRequests(ctx, config.RetryCount, config.RequestTimeout) // Per historical tradition our registry client also generates log messages // describing the requests that it makes. diff --git a/internal/command/cliconfig/cliconfig.go b/internal/command/cliconfig/cliconfig.go index 9b825405eb..de2bd9441c 100644 --- a/internal/command/cliconfig/cliconfig.go +++ b/internal/command/cliconfig/cliconfig.go @@ -57,6 +57,15 @@ type Config struct { Credentials map[string]map[string]interface{} `hcl:"credentials"` CredentialsHelpers map[string]*ConfigCredentialsHelper `hcl:"credentials_helper"` + // RegistryProtocols contains some settings for tailoring the request + // timeout and retry count for metadata requests made by our registry + // protocol clients. + // + // [LoadConfig] guarantees that this will always be present and contain + // values. If these settings are not configured either in files or the + // environment then we use some reasonable default settings. + RegistryProtocols *RegistryProtocolsConfig + // ProviderInstallation represents any provider_installation blocks // in the configuration. Only one of these is allowed across the whole // configuration, but we decode into a slice here so that we can handle @@ -122,7 +131,14 @@ func LoadConfig(_ context.Context) (*Config, tfdiags.Diagnostics) { if _, err := os.Stat(mainFilename); err == nil { mainConfig, mainDiags := loadConfigFile(mainFilename) diags = diags.Append(mainDiags) - config = config.Merge(mainConfig) + // NOTE: The order of arguments to merge below seems confusing + // given that below we tend to do it the other way around, but + // this was a historical inconsistency that we only discovered + // after making BuiltinConfig not just be completely empty and + // so we've swapped these but left the others unswapped to avoid + // changing those behaviors. It was unfortunately never well + // specified what is overriding what here. + config = mainConfig.Merge(config) } } else { diags = diags.Append(mainFileDiags) @@ -188,6 +204,9 @@ func loadConfigFile(path string) (*Config, tfdiags.Diagnostics) { // using a structure that is not compatible with HCL 1's DecodeObject, // or HCL 1 would be too liberal in parsing and thus make it harder // for us to potentially transition to using HCL 2 later. + registryProtocolsConfig, registryProtocolsDiags := decodeRegistryProtocolsConfigFromConfig(obj) + diags = diags.Append(registryProtocolsDiags) + result.RegistryProtocols = registryProtocolsConfig providerInstBlocks, providerInstDiags := decodeProviderInstallationFromConfig(obj) diags = diags.Append(providerInstDiags) result.ProviderInstallation = providerInstBlocks @@ -263,6 +282,11 @@ func envConfig(env map[string]string) *Config { config.PluginCacheMayBreakDependencyLockFile = true } + // The environment config _always_ has opinions about the registry + // protocols, because we include the default values in here if the + // relevant environment variables aren't set. + config.RegistryProtocols = decodeRegistryProtocolsConfigFromEnvironment() + return config } @@ -415,6 +439,8 @@ func (c *Config) Merge(c2 *Config) *Config { } } + result.RegistryProtocols = mergeRegistryProtocolConfigs(c2.RegistryProtocols, c.RegistryProtocols) + if (len(c.ProviderInstallation) + len(c2.ProviderInstallation)) > 0 { result.ProviderInstallation = append(result.ProviderInstallation, c.ProviderInstallation...) result.ProviderInstallation = append(result.ProviderInstallation, c2.ProviderInstallation...) diff --git a/internal/command/cliconfig/registry_protocols.go b/internal/command/cliconfig/registry_protocols.go new file mode 100644 index 0000000000..ab25c99a1b --- /dev/null +++ b/internal/command/cliconfig/registry_protocols.go @@ -0,0 +1,217 @@ +// Copyright (c) The OpenTofu Authors +// SPDX-License-Identifier: MPL-2.0 +// Copyright (c) 2023 HashiCorp, Inc. +// SPDX-License-Identifier: MPL-2.0 + +package cliconfig + +import ( + "fmt" + "os" + "strconv" + "time" + + "github.com/hashicorp/hcl" + hclast "github.com/hashicorp/hcl/hcl/ast" + hcltoken "github.com/hashicorp/hcl/hcl/token" + + "github.com/opentofu/opentofu/internal/tfdiags" +) + +// RegistryProtocolsConfig models the "registry_protocols" configuration block +// and its associated environment variables. +// +// These settings are a little awkward in that they relate to OpenTofu's native +// module and provider registry protocols, but not to any other module or +// provider installation methods. The name of this block type assumes that +// we'd extend these settings to any other OpenTofu-native registry protocols +// we might add in future, although at the time of writing this comment we have +// no plans to introduce any, since we're preferring to use industry-standard +// artifact distribution protocols like OCI Distribution. +// +// They also, due to a historical implementation mistake that we preserved for +// backward-compatibility, also partially influence OpenTofu's native service +// discovery client regardless of which service protocol it's trying to perform +// discovery for. +type RegistryProtocolsConfig struct { + // RetryCount specifies the number of times OpenTofu should + // retry making metadata requests to module or provider registries + // when it encounters a retryable error. + RetryCount int + RetryCountSet bool // for tracking overrides between files + + // RequestTimeout is the amount of time to wait for a response + // to a metadata request to a module or provider registry. + RequestTimeout time.Duration + RequestTimeoutSet bool // for tracking overrides between files +} + +func decodeRegistryProtocolsConfigFromConfig(hclFile *hclast.File) (*RegistryProtocolsConfig, tfdiags.Diagnostics) { + var diags tfdiags.Diagnostics + var ret *RegistryProtocolsConfig + var retPos hcltoken.Pos + + root := hclFile.Node.(*hclast.ObjectList) + for _, block := range root.Items { + if block.Keys[0].Token.Value() != "registry_protocols" { + continue + } + if ret != nil { + // We don't allow multiple registry_protocols blocks in the same + // file because that would be confusing and should never be + // necessary, but note that we _do_ allow different files (or + // other sources like the environment) to each have their own + // block and then our caller is responsible for merging them. + diags = diags.Append(tfdiags.Sourceless( + tfdiags.Error, + "Duplicate registry_protocols block", + fmt.Sprintf("The registry protocol settings were already defined at %s.", retPos), + )) + continue + } + + body, ok := block.Val.(*hclast.ObjectType) + if !ok { + // We'll get here if the config contains something particularly + // weird, such as: registry_protocols = "not_an_object" + // + // HCL 1 makes it hard to handle all of these variations robustly + // with good error messages because its AST can have many different + // shapes depending on what the author wrote, and so we'll just + // accept this generic generic error message and hope this won't + // arise often because people will follow the examples we show + // in our documentation. + diags = diags.Append(tfdiags.Sourceless( + tfdiags.Error, + "Invalid registry_protocols block", + fmt.Sprintf("The registry_protocols item at %s must have an open brace after the block type.", block.Pos()), + )) + continue + } + + config, moreDiags := decodeRegistryProtocolsConfigFromConfigBody(body) + diags = diags.Append(moreDiags) + ret = config + retPos = block.Pos() + } + + return ret, diags +} + +func decodeRegistryProtocolsConfigFromConfigBody(body *hclast.ObjectType) (*RegistryProtocolsConfig, tfdiags.Diagnostics) { + var diags tfdiags.Diagnostics + ret := &RegistryProtocolsConfig{ + // By the time we get here we definitely have a block, regardless of + // whether anything is set in it. We'll populate this selectively below. + } + + type BodyContent struct { + RetryCount *int `hcl:"retry_count"` + RequestTimeout *int `hcl:"request_timeout_seconds"` + } + var bodyContent BodyContent + err := hcl.DecodeObject(&bodyContent, body) + if err != nil { + diags = diags.Append(tfdiags.Sourceless( + tfdiags.Error, + "Invalid registry_protocols block", + fmt.Sprintf("Invalid registry protocol settings at %s: %s.", body.Pos(), err), + )) + return ret, diags + } + if bodyContent.RetryCount != nil { + ret.RetryCount = *bodyContent.RetryCount + ret.RetryCountSet = true + } + if bodyContent.RequestTimeout != nil { + ret.RequestTimeout = time.Duration(*bodyContent.RequestTimeout) * time.Second + ret.RequestTimeoutSet = true + } + return ret, diags +} + +// decodeRegistryProtocolsConfigFromEnvironment returns a +// [RegistryProtocolsConfig] object describing a "virtual" registry_protocols +// configuration block implied by environment variables, if any are set. +func decodeRegistryProtocolsConfigFromEnvironment() *RegistryProtocolsConfig { + ret := RegistryProtocolsConfig{} + if v := os.Getenv(registryDiscoveryRetryEnvName); v != "" { + override, err := strconv.Atoi(v) + if err == nil && override > 0 { + ret.RetryCount = override + ret.RetryCountSet = true + } + } + if v := os.Getenv(registryClientTimeoutEnvName); v != "" { + override, err := strconv.Atoi(v) + if err == nil && override > 0 { + ret.RequestTimeout = time.Duration(override) * time.Second + ret.RequestTimeoutSet = true + } + } + if !ret.RetryCountSet && !ret.RequestTimeoutSet { + // If neither variable is set then we behave as if this virtual + // configuration block is not present at all. + return nil + } + return &ret +} + +// mergeRegistryProtocolConfigs is used by [Config.Merge] for merging registry +// protocol configurations from multiple different files. +func mergeRegistryProtocolConfigs(base, override *RegistryProtocolsConfig) *RegistryProtocolsConfig { + // We only have work to do here if both arguments are set. + if base == nil { + return override + } + if override == nil { + return base + } + + // If we're going to change anything then we must always produce a new + // object, because the caller expects both of the given objects to + // remain unmodified. + ret := &RegistryProtocolsConfig{} + if override.RetryCountSet { + ret.RetryCount = override.RetryCount + ret.RetryCountSet = override.RetryCountSet + } else { + ret.RetryCount = base.RetryCount + ret.RetryCountSet = base.RetryCountSet + } + if override.RequestTimeoutSet { + ret.RequestTimeout = override.RequestTimeout + ret.RequestTimeoutSet = override.RequestTimeoutSet + } else { + ret.RequestTimeout = base.RequestTimeout + ret.RequestTimeoutSet = base.RequestTimeoutSet + } + return ret +} + +func init() { + // BuiltinConfig should contain the default values for a registry_protocols + // block. + BuiltinConfig.RegistryProtocols = &RegistryProtocolsConfig{ + RetryCount: registryDiscoveryDefaultRetryCount, + RetryCountSet: true, + RequestTimeout: registryClientDefaultRequestTimeout, + RequestTimeoutSet: true, + } +} + +const ( + // registryDiscoveryRetryEnvName is the name of the environment variable that + // can be configured to customize number of retries for module and provider + // discovery requests with the remote registry. + registryDiscoveryRetryEnvName = "TF_REGISTRY_DISCOVERY_RETRY" + registryDiscoveryDefaultRetryCount = 1 + + // registryClientTimeoutEnvName is the name of the environment variable that + // can be configured to customize the timeout duration (seconds) for module + // and provider discovery with a remote registry. For historical reasons + // this also applies to all service discovery requests regardless of whether + // they are registry-related. + registryClientTimeoutEnvName = "TF_REGISTRY_CLIENT_TIMEOUT" + registryClientDefaultRequestTimeout = 10 * time.Second +) diff --git a/internal/command/cliconfig/registry_protocols_test.go b/internal/command/cliconfig/registry_protocols_test.go new file mode 100644 index 0000000000..6b67f50afd --- /dev/null +++ b/internal/command/cliconfig/registry_protocols_test.go @@ -0,0 +1,269 @@ +// Copyright (c) The OpenTofu Authors +// SPDX-License-Identifier: MPL-2.0 +// Copyright (c) 2023 HashiCorp, Inc. +// SPDX-License-Identifier: MPL-2.0 + +package cliconfig + +import ( + "path/filepath" + "strings" + "testing" + "time" + + "github.com/google/go-cmp/cmp" +) + +func TestLoadConfig_registryProtocols(t *testing.T) { + tests := map[string]struct { + // The fixture names correspond to files under the "testdata" directory. + fixture string + env map[string]string + want *RegistryProtocolsConfig + wantErr string + }{ + "none": { + "registry-protocols-none", + nil, + &RegistryProtocolsConfig{ + // These are the default settings used when nothing overrides them. + RetryCount: 1, + RetryCountSet: true, + RequestTimeout: 10 * time.Second, + RequestTimeoutSet: true, + }, + ``, + }, + "both": { + "registry-protocols-both", + nil, + &RegistryProtocolsConfig{ + RetryCount: 256, + RetryCountSet: true, + RequestTimeout: 50 * time.Second, + RequestTimeoutSet: true, + }, + ``, + }, + "only-count": { + "registry-protocols-only-count", + nil, + &RegistryProtocolsConfig{ + RetryCount: 256, + RetryCountSet: true, + RequestTimeout: 10 * time.Second, + RequestTimeoutSet: true, + }, + ``, + }, + "only-timeout": { + "registry-protocols-only-timeout", + nil, + &RegistryProtocolsConfig{ + RetryCount: 1, + RetryCountSet: true, + RequestTimeout: 50 * time.Second, + RequestTimeoutSet: true, + }, + ``, + }, + "future": { + "registry-protocols-future", + nil, + &RegistryProtocolsConfig{ + // These just the defaults again because this fixture only + // includes a hypothetical future setting that our current + // code doesn't recognize at all. + RetryCount: 1, + RetryCountSet: true, + RequestTimeout: 10 * time.Second, + RequestTimeoutSet: true, + }, + ``, + }, + "none-env-both": { + "registry-protocols-none", + map[string]string{ + "TF_REGISTRY_DISCOVERY_RETRY": "123", + "TF_REGISTRY_CLIENT_TIMEOUT": "456", + }, + &RegistryProtocolsConfig{ + RetryCount: 123, + RetryCountSet: true, + RequestTimeout: 456 * time.Second, + RequestTimeoutSet: true, + }, + ``, + }, + "none-env-retry": { + "registry-protocols-none", + map[string]string{ + "TF_REGISTRY_DISCOVERY_RETRY": "123", + }, + &RegistryProtocolsConfig{ + RetryCount: 123, + RetryCountSet: true, + RequestTimeout: 10 * time.Second, + RequestTimeoutSet: true, + }, + ``, + }, + "none-env-timeout": { + "registry-protocols-none", + map[string]string{ + "TF_REGISTRY_CLIENT_TIMEOUT": "456", + }, + &RegistryProtocolsConfig{ + RetryCount: 1, + RetryCountSet: true, + RequestTimeout: 456 * time.Second, + RequestTimeoutSet: true, + }, + ``, + }, + "both-env-both": { + "registry-protocols-both", + map[string]string{ + "TF_REGISTRY_DISCOVERY_RETRY": "123", + "TF_REGISTRY_CLIENT_TIMEOUT": "456", + }, + &RegistryProtocolsConfig{ + RetryCount: 123, + RetryCountSet: true, + RequestTimeout: 456 * time.Second, + RequestTimeoutSet: true, + }, + ``, + }, + "both-env-retry": { + "registry-protocols-both", + map[string]string{ + "TF_REGISTRY_DISCOVERY_RETRY": "123", + }, + &RegistryProtocolsConfig{ + RetryCount: 123, + RetryCountSet: true, + RequestTimeout: 50 * time.Second, + RequestTimeoutSet: true, + }, + ``, + }, + "both-env-timeout": { + "registry-protocols-both", + map[string]string{ + "TF_REGISTRY_CLIENT_TIMEOUT": "456", + }, + &RegistryProtocolsConfig{ + RetryCount: 256, + RetryCountSet: true, + RequestTimeout: 456 * time.Second, + RequestTimeoutSet: true, + }, + ``, + }, + "invalid-syntax": { + "registry-protocols-invalid-syntax", + nil, + &RegistryProtocolsConfig{ + RetryCount: 1, + RetryCountSet: true, + RequestTimeout: 10 * time.Second, + RequestTimeoutSet: true, + }, + `The registry_protocols item at 3:1 must have an open brace after the block type`, + }, + "invalid-count": { + "registry-protocols-invalid-count", + nil, + &RegistryProtocolsConfig{ + RetryCount: 1, + RetryCountSet: true, + RequestTimeout: 10 * time.Second, + RequestTimeoutSet: true, + }, + `parsing "not a number": invalid syntax`, + }, + "invalid-timeout": { + "registry-protocols-invalid-timeout", + nil, + &RegistryProtocolsConfig{ + RetryCount: 1, + RetryCountSet: true, + RequestTimeout: 10 * time.Second, + RequestTimeoutSet: true, + }, + `parsing "not a number": invalid syntax`, + }, + "invalid-multi": { + "registry-protocols-invalid-multi", + nil, + &RegistryProtocolsConfig{ + RetryCount: 1, + RetryCountSet: true, + RequestTimeout: 10 * time.Second, + RequestTimeoutSet: true, + }, + `The registry protocol settings were already defined at 4:1`, + }, + "invalid-syntax-env-both": { + // The environment variable settings still work even when the + // config file is invalid, because OpenTofu still makes a best + // effort to proceed even when the CLI configuration has errors. + "registry-protocols-invalid-syntax", + map[string]string{ + "TF_REGISTRY_DISCOVERY_RETRY": "123", + "TF_REGISTRY_CLIENT_TIMEOUT": "456", + }, + &RegistryProtocolsConfig{ + RetryCount: 123, + RetryCountSet: true, + RequestTimeout: 456 * time.Second, + RequestTimeoutSet: true, + }, + `The registry_protocols item at 3:1 must have an open brace after the block type`, + }, + } + + for name, test := range tests { + t.Run(name, func(t *testing.T) { + fixtureFile := filepath.Join("testdata", test.fixture) + + // We set the file to load using this environment variable because + // otherwise we can't use the LoadConfig entrypoint, which we're + // relying on to test inheritance of the default settings and + // overriding settings from the environment variables. :( + t.Setenv("TF_CLI_CONFIG_FILE", fixtureFile) + for name, val := range test.env { + t.Setenv(name, val) + } + // If either of the environment variables we're sensitive to are + // not in the map then we'll explicitly set them to empty so + // that we won't pick up stray values that might be in the real + // environment whereever these tests are being run. + if _, ok := test.env["TF_REGISTRY_DISCOVERY_RETRY"]; !ok { + t.Setenv("TF_REGISTRY_DISCOVERY_RETRY", "") + } + if _, ok := test.env["TF_REGISTRY_CLIENT_TIMEOUT"]; !ok { + t.Setenv("TF_REGISTRY_CLIENT_TIMEOUT", "") + } + + gotConfig, diags := LoadConfig(t.Context()) + if diags.HasErrors() { + errStr := diags.Err().Error() + if test.wantErr == "" { + t.Errorf("unexpected errors: %s", errStr) + } + if !strings.Contains(errStr, test.wantErr) { + t.Errorf("missing expected error\nwant substring: %s\ngot: %s", test.wantErr, errStr) + } + } else if test.wantErr != "" { + t.Errorf("unexpected success\nwant error with substring: %s", test.wantErr) + } + + got := gotConfig.RegistryProtocols + if diff := cmp.Diff(test.want, got); diff != "" { + t.Error("unexpected result\n" + diff) + } + }) + } +} diff --git a/internal/command/cliconfig/testdata/registry-protocols-both b/internal/command/cliconfig/testdata/registry-protocols-both new file mode 100644 index 0000000000..4d64b8f3b8 --- /dev/null +++ b/internal/command/cliconfig/testdata/registry-protocols-both @@ -0,0 +1,4 @@ +registry_protocols { + retry_count = 256 + request_timeout_seconds = 50 +} diff --git a/internal/command/cliconfig/testdata/registry-protocols-future b/internal/command/cliconfig/testdata/registry-protocols-future new file mode 100644 index 0000000000..b3e613710e --- /dev/null +++ b/internal/command/cliconfig/testdata/registry-protocols-future @@ -0,0 +1,5 @@ +registry_protocols { + # This is to test that we silently ignore unrecognized arguments, so that + # future versions of OpenTofu could potentially add more options in here. + hypothetical_future_option = "..." +} diff --git a/internal/command/cliconfig/testdata/registry-protocols-invalid-count b/internal/command/cliconfig/testdata/registry-protocols-invalid-count new file mode 100644 index 0000000000..4136a9b507 --- /dev/null +++ b/internal/command/cliconfig/testdata/registry-protocols-invalid-count @@ -0,0 +1,3 @@ +registry_protocols { + retry_count = "not a number" +} diff --git a/internal/command/cliconfig/testdata/registry-protocols-invalid-multi b/internal/command/cliconfig/testdata/registry-protocols-invalid-multi new file mode 100644 index 0000000000..33cd3463a6 --- /dev/null +++ b/internal/command/cliconfig/testdata/registry-protocols-invalid-multi @@ -0,0 +1,6 @@ +# The following is intentionally invalid: registry_protocols is allowed +# only once per configuration file (but can be specified separately across +# _separate_ configuration files) +registry_protocols {} +registry_protocols {} + diff --git a/internal/command/cliconfig/testdata/registry-protocols-invalid-syntax b/internal/command/cliconfig/testdata/registry-protocols-invalid-syntax new file mode 100644 index 0000000000..066d53b3b3 --- /dev/null +++ b/internal/command/cliconfig/testdata/registry-protocols-invalid-syntax @@ -0,0 +1,3 @@ +# The following is intentionally invalid: registry_protocols is supposed to be +# a block, not a single argument. +registry_protocols = "invalid" diff --git a/internal/command/cliconfig/testdata/registry-protocols-invalid-timeout b/internal/command/cliconfig/testdata/registry-protocols-invalid-timeout new file mode 100644 index 0000000000..b93b6aa027 --- /dev/null +++ b/internal/command/cliconfig/testdata/registry-protocols-invalid-timeout @@ -0,0 +1,3 @@ +registry_protocols { + request_timeout_seconds = "not a number" +} diff --git a/internal/command/cliconfig/testdata/registry-protocols-none b/internal/command/cliconfig/testdata/registry-protocols-none new file mode 100644 index 0000000000..3f277e4fc9 --- /dev/null +++ b/internal/command/cliconfig/testdata/registry-protocols-none @@ -0,0 +1,5 @@ +# This is intentionally empty, representing the absense of any +# registry_protocols block. +# +# This file is used both for testing the defaults and for overriding the +# defaults using environment variables. diff --git a/internal/command/cliconfig/testdata/registry-protocols-only-count b/internal/command/cliconfig/testdata/registry-protocols-only-count new file mode 100644 index 0000000000..95ce964131 --- /dev/null +++ b/internal/command/cliconfig/testdata/registry-protocols-only-count @@ -0,0 +1,3 @@ +registry_protocols { + retry_count = 256 +} diff --git a/internal/command/cliconfig/testdata/registry-protocols-only-timeout b/internal/command/cliconfig/testdata/registry-protocols-only-timeout new file mode 100644 index 0000000000..a288bea858 --- /dev/null +++ b/internal/command/cliconfig/testdata/registry-protocols-only-timeout @@ -0,0 +1,3 @@ +registry_protocols { + request_timeout_seconds = 50 +} diff --git a/website/docs/cli/config/config-file.mdx b/website/docs/cli/config/config-file.mdx index 1fc22261c0..92fa09e0bb 100644 --- a/website/docs/cli/config/config-file.mdx +++ b/website/docs/cli/config/config-file.mdx @@ -75,6 +75,11 @@ The following settings can be set in the CLI configuration file: `tofu init` when installing provider plugins. See [Provider Installation](#provider-installation) below for more information. +* `registry_protocols` - configures some infrequently-needed settings + controlling how OpenTofu requests metadata from module and provider + registries. + Refer to [Registry Protocol Settings](#registry-protocol-settings) below for more information. + ## Credentials When interacting with OpenTofu-specific network services, OpenTofu expects @@ -513,3 +518,42 @@ configure it, and how it interacts with the dependency lock file may all evolve in future OpenTofu releases, including possible breaking changes. We therefore recommend using development overrides only temporarily during provider development work. + +## Registry Protocol Settings + +The CLI configuration block `registry_protocols` controls a small number of +rarely-needed settings for customizing the retry behavior and request timeouts +when OpenTofu makes metadata requests to module and provider registries. + +```hcl +registry_protocols { + + # retry_count specifies how many times OpenTofu can + # retry a registry metadata request when the response + # is a retry-eligible error. + # + # This can also be set using an environment variable: + # TF_REGISTRY_DISCOVERY_RETRY=1 + retry_count = 1 + + # request_timeout_seconds specifies how long OpenTofu + # should wait for a response to a registry metadata + # request, in seconds. + # + # This can also be set using an environment variable: + # TF_REGISTRY_CLIENT_TIMEOUT=10 + request_timeout_seconds = 10 + +} +``` + +The values in the example above are the current defaults, although the +defaults are subject to change in future versions of OpenTofu. + +These settings are used only when making requests to the _OpenTofu-native_ +registry protocols, and when performing service discovery to find service +endpoints for a particular hostname. + +These settings do not affect any other requests made by OpenTofu, including +requests to download the actual module or provider packages, or requests to +other kinds of installation sources such as OCI registries. diff --git a/website/docs/cli/config/environment-variables.mdx b/website/docs/cli/config/environment-variables.mdx index d2bd4d1e1c..80e8d32695 100644 --- a/website/docs/cli/config/environment-variables.mdx +++ b/website/docs/cli/config/environment-variables.mdx @@ -128,17 +128,13 @@ exact output differences can change between minor OpenTofu versions. ## TF_REGISTRY_DISCOVERY_RETRY -Set `TF_REGISTRY_DISCOVERY_RETRY` to configure the max number of request retries -the remote registry client will attempt for client connection errors or -500-range responses that are safe to retry. +Equivalent to the `retry_count` setting in the +[Registry Protocol Settings](../../cli/config/config-file.mdx#registry-protocol-settings). ## TF_REGISTRY_CLIENT_TIMEOUT -The default client timeout for requests to the remote registry is 10s. `TF_REGISTRY_CLIENT_TIMEOUT` can be configured and increased during exceptional circumstances. - -```shell -export TF_REGISTRY_CLIENT_TIMEOUT=15 -``` +Equivalent to the `request_timeout_seconds` setting in the +[Registry Protocol Settings](../../cli/config/config-file.mdx#registry-protocol-settings). ## TF_CLI_CONFIG_FILE