From ae98bd12a72ddc85355cc646d34e550dcc96c3b7 Mon Sep 17 00:00:00 2001 From: Alisdair McDiarmid Date: Wed, 6 May 2020 13:35:35 -0400 Subject: [PATCH] command: Rework 0.13upgrade sub-command This commit implements most of the intended functionality of the upgrade command for rewriting configurations. For a given module, it makes a list of all providers in use. Then it attempts to detect the source address for providers without an explicit source. Once this step is complete, the tool rewrites the relevant configuration files. This results in a single "required_providers" block for the module, with a source for each provider. Any providers for which the source cannot be detected (for example, unofficial providers) will need a source to be defined by the user. The tool writes an explanatory comment to the configuration to help with this. --- command/013_config_upgrade.go | 554 +++++++++++++++--- command/013_config_upgrade_test.go | 309 +++++++--- .../expected/main.tf | 5 + .../expected/providers.tf | 16 + .../input/main.tf | 5 + .../input/providers.tf | 3 + .../expected/main.tf | 19 + .../expected/providers.tf | 13 - .../input/main.tf | 12 + .../expected/providers.tf | 10 +- .../013upgrade-file-exists/input/providers.tf | 2 + .../013upgrade-file-exists/providers.tf | 2 - .../expected/main.tf | 1 + .../expected/providers.tf | 16 + .../input/main.tf | 1 + .../expected/main.tf | 5 + .../expected/providers.tf | 11 +- .../input/main.tf | 5 + .../013upgrade-implicit-providers/main.tf | 2 - command/testdata/013upgrade-invalid/main.tf | 3 - .../expected/bar-baz.tf | 5 + .../expected/foo.tf | 1 + .../expected/providers.tf | 15 + .../input/bar-baz.tf | 10 + .../013upgrade-multiple-blocks/input/foo.tf | 11 + .../expected/main.tf | 4 + .../expected/providers.tf | 16 + .../013upgrade-multiple-files/input/main.tf | 9 + .../input/providers.tf | 12 + .../{ => expected}/main.tf | 0 .../013upgrade-no-providers/input/main.tf | 11 + .../expected/main.tf | 16 + .../input/main.tf | 10 + .../expected/main.tf | 26 + .../input}/main.tf | 2 +- .../expected/main.tf | 7 + .../input/main.tf | 7 + .../expected/bar_override.tf | 3 + .../013upgrade-skipped-files/expected/main.tf | 19 + .../expected/variables.tf.json | 12 + .../input/bar_override.tf | 3 + .../013upgrade-skipped-files/input/main.tf | 12 + .../input/variables.tf.json | 12 + .../013upgrade-subdir/expected/providers.tf | 10 - .../testdata/013upgrade-subdir/subdir/main.tf | 2 - .../expected-module/main.tf | 8 + .../expected-module/module/main.tf | 1 + .../expected-module/module/providers.tf | 7 + .../013upgrade-submodule/expected/main.tf | 9 + .../expected/module/main.tf | 1 + .../013upgrade-submodule/input/main.tf | 8 + .../013upgrade-submodule/input/module/main.tf | 1 + configs/provider_requirements.go | 4 +- configs/provider_requirements_test.go | 7 + internal/getproviders/legacy_lookup.go | 5 + 55 files changed, 1073 insertions(+), 207 deletions(-) create mode 100644 command/testdata/013upgrade-existing-providers-tf/expected/main.tf create mode 100644 command/testdata/013upgrade-existing-providers-tf/expected/providers.tf create mode 100644 command/testdata/013upgrade-existing-providers-tf/input/main.tf create mode 100644 command/testdata/013upgrade-existing-providers-tf/input/providers.tf create mode 100644 command/testdata/013upgrade-explicit-providers/expected/main.tf delete mode 100644 command/testdata/013upgrade-explicit-providers/expected/providers.tf create mode 100644 command/testdata/013upgrade-explicit-providers/input/main.tf create mode 100644 command/testdata/013upgrade-file-exists/input/providers.tf delete mode 100644 command/testdata/013upgrade-file-exists/providers.tf create mode 100644 command/testdata/013upgrade-implicit-not-found/expected/main.tf create mode 100644 command/testdata/013upgrade-implicit-not-found/expected/providers.tf create mode 100644 command/testdata/013upgrade-implicit-not-found/input/main.tf create mode 100644 command/testdata/013upgrade-implicit-providers/expected/main.tf create mode 100644 command/testdata/013upgrade-implicit-providers/input/main.tf delete mode 100644 command/testdata/013upgrade-implicit-providers/main.tf delete mode 100644 command/testdata/013upgrade-invalid/main.tf create mode 100644 command/testdata/013upgrade-multiple-blocks/expected/bar-baz.tf create mode 100644 command/testdata/013upgrade-multiple-blocks/expected/foo.tf create mode 100644 command/testdata/013upgrade-multiple-blocks/expected/providers.tf create mode 100644 command/testdata/013upgrade-multiple-blocks/input/bar-baz.tf create mode 100644 command/testdata/013upgrade-multiple-blocks/input/foo.tf create mode 100644 command/testdata/013upgrade-multiple-files/expected/main.tf create mode 100644 command/testdata/013upgrade-multiple-files/expected/providers.tf create mode 100644 command/testdata/013upgrade-multiple-files/input/main.tf create mode 100644 command/testdata/013upgrade-multiple-files/input/providers.tf rename command/testdata/013upgrade-no-providers/{ => expected}/main.tf (100%) create mode 100644 command/testdata/013upgrade-no-providers/input/main.tf create mode 100644 command/testdata/013upgrade-preserves-comments/expected/main.tf create mode 100644 command/testdata/013upgrade-preserves-comments/input/main.tf create mode 100644 command/testdata/013upgrade-provider-not-found/expected/main.tf rename command/testdata/{013upgrade-explicit-providers => 013upgrade-provider-not-found/input}/main.tf (87%) create mode 100644 command/testdata/013upgrade-providers-with-source/expected/main.tf create mode 100644 command/testdata/013upgrade-providers-with-source/input/main.tf create mode 100644 command/testdata/013upgrade-skipped-files/expected/bar_override.tf create mode 100644 command/testdata/013upgrade-skipped-files/expected/main.tf create mode 100644 command/testdata/013upgrade-skipped-files/expected/variables.tf.json create mode 100644 command/testdata/013upgrade-skipped-files/input/bar_override.tf create mode 100644 command/testdata/013upgrade-skipped-files/input/main.tf create mode 100644 command/testdata/013upgrade-skipped-files/input/variables.tf.json delete mode 100644 command/testdata/013upgrade-subdir/expected/providers.tf delete mode 100644 command/testdata/013upgrade-subdir/subdir/main.tf create mode 100644 command/testdata/013upgrade-submodule/expected-module/main.tf create mode 100644 command/testdata/013upgrade-submodule/expected-module/module/main.tf create mode 100644 command/testdata/013upgrade-submodule/expected-module/module/providers.tf create mode 100644 command/testdata/013upgrade-submodule/expected/main.tf create mode 100644 command/testdata/013upgrade-submodule/expected/module/main.tf create mode 100644 command/testdata/013upgrade-submodule/input/main.tf create mode 100644 command/testdata/013upgrade-submodule/input/module/main.tf diff --git a/command/013_config_upgrade.go b/command/013_config_upgrade.go index 1c3a0a67b..850dfa525 100644 --- a/command/013_config_upgrade.go +++ b/command/013_config_upgrade.go @@ -2,15 +2,20 @@ package command import ( "fmt" + "io/ioutil" "os" - "path/filepath" + "path" + "sort" "strings" - "text/template" + "github.com/hashicorp/hcl/v2" + "github.com/hashicorp/hcl/v2/hclsyntax" + "github.com/hashicorp/hcl/v2/hclwrite" + "github.com/hashicorp/terraform/addrs" "github.com/hashicorp/terraform/configs" - "github.com/hashicorp/terraform/internal/initwd" - "github.com/hashicorp/terraform/moduledeps" + "github.com/hashicorp/terraform/internal/getproviders" "github.com/hashicorp/terraform/tfdiags" + "github.com/zclconf/go-cty/cty" ) // ZeroThirteenUpgradeCommand upgrades configuration files for a module @@ -19,6 +24,9 @@ type ZeroThirteenUpgradeCommand struct { Meta } +// Warning diagnostic detail message used for JSON and override config files +const skippedConfigurationFileWarning = "The %s configuration file %q was skipped, because %s files are assumed to be generated. The program that generated this file may need to be updated for changes to the configuration language." + func (c *ZeroThirteenUpgradeCommand) Run(args []string) int { args = c.Meta.process(args) flags := c.Meta.defaultFlagSet("0.13upgrade") @@ -71,58 +79,412 @@ func (c *ZeroThirteenUpgradeCommand) Run(args []string) int { return 1 } - // Early-load the config so that we can check provider dependencies - earlyConfig, earlyConfDiags := c.loadConfigEarly(dir) - if earlyConfDiags.HasErrors() { + // Set up the config loader and find all the config files + loader, err := c.initConfigLoader() + if err != nil { + diags = diags.Append(err) + c.showDiagnostics(diags) + return 1 + } + parser := loader.Parser() + primary, overrides, hclDiags := parser.ConfigDirFiles(dir) + diags = diags.Append(hclDiags) + if diags.HasErrors() { c.Ui.Error(strings.TrimSpace("Failed to load configuration")) - diags = diags.Append(earlyConfDiags) c.showDiagnostics(diags) return 1 } - { - // Before we go further, we'll check to make sure none of the modules - // in the configuration declare that they don't support this Terraform - // version, so we can produce a version-related error message rather - // than potentially-confusing downstream errors. - versionDiags := initwd.CheckCoreVersionRequirements(earlyConfig) - diags = diags.Append(versionDiags) - if versionDiags.HasErrors() { - c.showDiagnostics(diags) - return 1 + // Load and parse all primary files + files := make(map[string]*configs.File) + for _, path := range primary { + // Skip JSON configuration files, because we can't rewrite them and + // they're probably generated anyway. + if strings.HasSuffix(strings.ToLower(path), ".json") { + diags = diags.Append(tfdiags.Sourceless( + tfdiags.Warning, + "JSON configuration file was not rewritten", + fmt.Sprintf( + skippedConfigurationFileWarning, + "JSON", + path, + "JSON", + ), + )) + continue + } + file, fileDiags := parser.LoadConfigFile(path) + diags = diags.Append(fileDiags) + if file != nil { + files[path] = file + } + } + if diags.HasErrors() { + c.Ui.Error(strings.TrimSpace("Failed to load configuration")) + c.showDiagnostics(diags) + return 1 + } + + // It's not clear what the correct behaviour is for upgrading override + // files. For now, just log that we're ignoring the file. + for _, path := range overrides { + diags = diags.Append(tfdiags.Sourceless( + tfdiags.Warning, + "Override configuration file was not rewritten", + fmt.Sprintf( + skippedConfigurationFileWarning, + "override", + path, + "override", + ), + )) + } + + // Build up a list of required providers, uniquely by local name + requiredProviders := make(map[string]*configs.RequiredProvider) + var rewritePaths []string + + // Step 1: copy all explicit provider requirements across + for path, file := range files { + for _, rps := range file.RequiredProviders { + rewritePaths = append(rewritePaths, path) + for _, rp := range rps.RequiredProviders { + if previous, exist := requiredProviders[rp.Name]; exist { + diags = diags.Append(&hcl.Diagnostic{ + Summary: "Duplicate required provider configuration", + Detail: fmt.Sprintf("Found duplicate required provider configuration for %q.Previously configured at %s", rp.Name, previous.DeclRange), + Severity: hcl.DiagWarning, + Context: rps.DeclRange.Ptr(), + Subject: rp.DeclRange.Ptr(), + }) + } else { + // We're copying the struct here to ensure that any + // mutation does not affect the original, if we rewrite + // this file + requiredProviders[rp.Name] = &configs.RequiredProvider{ + Name: rp.Name, + Source: rp.Source, + Type: rp.Type, + Requirement: rp.Requirement, + DeclRange: rp.DeclRange, + } + } + } } } - // Find the provider dependencies - configDeps, depsDiags := earlyConfig.ProviderDependencies() - if depsDiags.HasErrors() { - c.Ui.Error(strings.TrimSpace("Could not detect provider dependencies")) - diags = diags.Append(depsDiags) - c.showDiagnostics(diags) - return 1 + for _, file := range files { + // Step 2: add missing provider requirements from provider blocks + for _, p := range file.ProviderConfigs { + // If no explicit provider configuration exists for the + // provider configuration's local name, add one with a legacy + // provider address. + if _, exist := requiredProviders[p.Name]; !exist { + requiredProviders[p.Name] = &configs.RequiredProvider{ + Name: p.Name, + } + } + } + + // Step 3: add missing provider requirements from resources + resources := [][]*configs.Resource{file.ManagedResources, file.DataResources} + for _, rs := range resources { + for _, r := range rs { + // Find the appropriate provider local name for this resource + var localName string + + // If there's a provider config, use that to determine the + // local name. Otherwise use the implied provider local name + // based on the resource's address. + if r.ProviderConfigRef != nil { + localName = r.ProviderConfigRef.Name + } else { + localName = r.Addr().ImpliedProvider() + } + + // If no explicit provider configuration exists for this local + // name, add one with a legacy provider address. + if _, exist := requiredProviders[localName]; !exist { + requiredProviders[localName] = &configs.RequiredProvider{ + Name: localName, + } + } + } + } } - // Detect source for each provider - providerSources, detectDiags := detectProviderSources(configDeps.Providers) - if detectDiags.HasErrors() { - c.Ui.Error(strings.TrimSpace("Unable to detect sources for providers")) + // We should now have a complete understanding of the provider requirements + // stated in the config. If there are any providers, attempt to detect + // their sources, and rewrite the config. + if len(requiredProviders) > 0 { + detectDiags := c.detectProviderSources(requiredProviders) diags = diags.Append(detectDiags) - c.showDiagnostics(diags) - return 1 - } + if diags.HasErrors() { + c.Ui.Error("Unable to detect sources for providers") + c.showDiagnostics(diags) + return 1 + } - if len(providerSources) == 0 { - c.Ui.Output("No non-default providers found. Your configuration is ready to use!") - return 0 - } + // Default output filename is "providers.tf" + filename := path.Join(dir, "providers.tf") - // Generate the required providers configuration - genDiags := generateRequiredProviders(providerSources, dir) - diags = diags.Append(genDiags) + // Special case: if we only have one file with a required providers + // block, output to that file instead. + if len(rewritePaths) == 1 { + filename = rewritePaths[0] + } + + // Remove the output file from the list of paths we want to rewrite + // later. Otherwise we'd delete the required providers block after + // writing it. + for i, path := range rewritePaths { + if path == filename { + rewritePaths = append(rewritePaths[:i], rewritePaths[i+1:]...) + break + } + } + + var out *hclwrite.File + + // If the output file doesn't exist, just create a new empty file + if _, err := os.Stat(filename); os.IsNotExist(err) { + out = hclwrite.NewEmptyFile() + } else if err != nil { + diags = diags.Append(tfdiags.Sourceless( + tfdiags.Error, + "Unable to read configuration file", + fmt.Sprintf("Error when reading configuration file %q: %s", filename, err), + )) + c.showDiagnostics(diags) + return 1 + } else { + // Configuration file already exists, so load and parse it + config, err := ioutil.ReadFile(filename) + if err != nil { + diags = diags.Append(tfdiags.Sourceless( + tfdiags.Error, + "Unable to read configuration file", + fmt.Sprintf("Error when reading configuration file %q: %s", filename, err), + )) + c.showDiagnostics(diags) + return 1 + } + var parseDiags hcl.Diagnostics + out, parseDiags = hclwrite.ParseConfig(config, filename, hcl.InitialPos) + diags = diags.Append(parseDiags) + } + + if diags.HasErrors() { + c.showDiagnostics(diags) + return 1 + } + + // Find all required_providers blocks, and store them alongside a map + // back to the parent terraform block. + var requiredProviderBlocks []*hclwrite.Block + parentBlocks := make(map[*hclwrite.Block]*hclwrite.Block) + root := out.Body() + for _, rootBlock := range root.Blocks() { + if rootBlock.Type() != "terraform" { + continue + } + for _, childBlock := range rootBlock.Body().Blocks() { + if childBlock.Type() == "required_providers" { + requiredProviderBlocks = append(requiredProviderBlocks, childBlock) + parentBlocks[childBlock] = rootBlock + } + } + } + + // First required provider block, and the rest found in this file. + var first *hclwrite.Block + var rest []*hclwrite.Block + + if len(requiredProviderBlocks) > 0 { + // If we already have one or more required provider blocks, we'll rewrite + // the first one, and remove the rest. + first, rest = requiredProviderBlocks[0], requiredProviderBlocks[1:] + } else { + // Otherwise, find or a create a terraform block, and add a new + // empty required providers block to it. + var tfBlock *hclwrite.Block + for _, rootBlock := range root.Blocks() { + if rootBlock.Type() == "terraform" { + tfBlock = rootBlock + break + } + } + if tfBlock == nil { + tfBlock = root.AppendNewBlock("terraform", nil) + } + first = tfBlock.Body().AppendNewBlock("required_providers", nil) + } + + // Find the body of the first block to prepare for rewriting it + body := first.Body() + + // Build a sorted list of provider local names, for consistent ordering + var localNames []string + for localName := range requiredProviders { + localNames = append(localNames, localName) + } + sort.Strings(localNames) + + // Populate the required providers block + for _, localName := range localNames { + requiredProvider := requiredProviders[localName] + var attributes = make(map[string]cty.Value) + + if !requiredProvider.Type.IsZero() { + attributes["source"] = cty.StringVal(requiredProvider.Type.ForDisplay()) + } + + if version := requiredProvider.Requirement.Required.String(); version != "" { + attributes["version"] = cty.StringVal(version) + } + + var attributesObject cty.Value + if len(attributes) > 0 { + attributesObject = cty.ObjectVal(attributes) + } else { + attributesObject = cty.EmptyObjectVal + } + body.SetAttributeValue(localName, attributesObject) + + // If we don't have a source attribute, manually construct a commented + // block explaining what to do + if _, hasSource := attributes["source"]; !hasSource { + // Generate the token stream for the required provider + rp := body.GetAttribute(localName) + expr := rp.Expr().BuildTokens(nil) + + // Partition the tokens into before and after the opening brace + before, after := partitionTokensAfter(expr, hclsyntax.TokenOBrace) + + // If the value is an empty object, add a newline between the + // braces so that the comment is not on the same line as either + // brace. + if len(before) == 1 && len(after) == 1 { + newline := &hclwrite.Token{ + Type: hclsyntax.TokenNewline, + Bytes: []byte{'\n'}, + } + after = append(hclwrite.Tokens{newline}, after...) + } + + // Generate the comment and insert it at the start of the object + comment := noSourceDetectedComment(localName) + commentedBlock := append(before, comment...) + commentedBlock = append(commentedBlock, after...) + + // Set the required provider object to this raw token stream + body.SetAttributeRaw(localName, commentedBlock) + } + } + + // Remove the rest of the blocks (and the parent block, if it's empty) + for _, rpBlock := range rest { + tfBlock := parentBlocks[rpBlock] + tfBody := tfBlock.Body() + tfBody.RemoveBlock(rpBlock) + + // If the terraform block has no blocks and no attributes, it's + // basically empty (aside from comments and whitespace), so it's + // more useful to remove it than leave it in. + if len(tfBody.Blocks()) == 0 && len(tfBody.Attributes()) == 0 { + root.RemoveBlock(tfBlock) + } + } + + // Write the config back to the file + f, err := os.OpenFile(filename, os.O_TRUNC|os.O_CREATE|os.O_WRONLY, 0644) + if err != nil { + diags = diags.Append(tfdiags.Sourceless( + tfdiags.Error, + "Unable to open configuration file for writing", + fmt.Sprintf("Error when reading configuration file %q: %s", filename, err), + )) + c.showDiagnostics(diags) + return 1 + } + _, err = out.WriteTo(f) + if err != nil { + diags = diags.Append(tfdiags.Sourceless( + tfdiags.Error, + "Unable to rewrite configuration file", + fmt.Sprintf("Error when rewriting configuration file %q: %s", filename, err), + )) + c.showDiagnostics(diags) + return 1 + } + + // After successfully writing the new configuration, remove all other + // required provider blocks from remaining configuration files. + for _, path := range rewritePaths { + // Read and parse the existing file + config, err := ioutil.ReadFile(path) + if err != nil { + diags = diags.Append(tfdiags.Sourceless( + tfdiags.Error, + "Unable to read configuration file", + fmt.Sprintf("Error when reading configuration file %q: %s", filename, err), + )) + c.showDiagnostics(diags) + return 1 + } + file, parseDiags := hclwrite.ParseConfig(config, filename, hcl.InitialPos) + diags = diags.Append(parseDiags) + if diags.HasErrors() { + c.showDiagnostics(diags) + return 1 + } + + // Find and remove all terraform.required_providers blocks + root := file.Body() + for _, rootBlock := range root.Blocks() { + if rootBlock.Type() != "terraform" { + continue + } + tfBody := rootBlock.Body() + for _, childBlock := range tfBody.Blocks() { + if childBlock.Type() == "required_providers" { + rootBlock.Body().RemoveBlock(childBlock) + + // If the terraform block is now empty, remove it + if len(tfBody.Blocks()) == 0 && len(tfBody.Attributes()) == 0 { + root.RemoveBlock(rootBlock) + } + } + } + } + + // Write the config back to the file + f, err := os.OpenFile(path, os.O_TRUNC|os.O_CREATE|os.O_WRONLY, 0644) + if err != nil { + diags = diags.Append(tfdiags.Sourceless( + tfdiags.Error, + "Unable to open configuration file for writing", + fmt.Sprintf("Error when reading configuration file %q: %s", filename, err), + )) + c.showDiagnostics(diags) + return 1 + } + _, err = file.WriteTo(f) + if err != nil { + diags = diags.Append(tfdiags.Sourceless( + tfdiags.Error, + "Unable to rewrite configuration file", + fmt.Sprintf("Error when rewriting configuration file %q: %s", filename, err), + )) + c.showDiagnostics(diags) + return 1 + } + } + } c.showDiagnostics(diags) if diags.HasErrors() { - return 2 + return 1 } if len(diags) != 0 { @@ -138,64 +500,78 @@ necessary adjustments, and then commit. return 0 } -// For providers which need a source attribute, detect and return source -// FIXME: currently does not filter or detect sources -func detectProviderSources(providers moduledeps.Providers) (map[string]string, tfdiags.Diagnostics) { - sources := make(map[string]string) - for provider := range providers { - sources[provider.Type] = provider.String() - } - return sources, nil -} - -var providersTemplate = template.Must(template.New("providers.tf").Parse(`terraform { - required_providers { - {{- range $type, $source := .}} - {{$type}} = { - source = "{{$source}}" - } - {{- end}} - } -} -`)) - -// Generate a file with terraform.required_providers blocks for each provider -func generateRequiredProviders(providerSources map[string]string, dir string) tfdiags.Diagnostics { +// For providers which need a source attribute, detect the source +func (c *ZeroThirteenUpgradeCommand) detectProviderSources(requiredProviders map[string]*configs.RequiredProvider) tfdiags.Diagnostics { + source := c.providerInstallSource() var diags tfdiags.Diagnostics - // Find unused file named "providers.tf", or fall back to e.g. "providers-1.tf" - path := filepath.Join(dir, "providers.tf") - if _, err := os.Stat(path); !os.IsNotExist(err) { - for i := 1; ; i++ { - path = filepath.Join(dir, fmt.Sprintf("providers-%d.tf", i)) - if _, err := os.Stat(path); os.IsNotExist(err) { - break + for name, rp := range requiredProviders { + // If there's already an explicit source, skip it + if rp.Source != "" { + continue + } + + // Construct a legacy provider FQN using the required provider local + // name. This ignores any auto-generated provider FQN from the load & + // parse process, because we know that without an explicit source it is + // not explicitly specified. + addr := addrs.NewLegacyProvider(name) + p, err := getproviders.LookupLegacyProvider(addr, source) + if err == nil { + rp.Type = p + } else { + if _, ok := err.(getproviders.ErrProviderNotKnown); ok { + // Setting the provider address to a zero value struct + // indicates that there is no known FQN for this provider, + // which will cause us to write an explanatory comment in the + // HCL output advising the user what to do about this. + rp.Type = addrs.Provider{} } + diags = diags.Append(tfdiags.Sourceless( + tfdiags.Warning, + "Could not detect provider source", + fmt.Sprintf("Error looking up provider source for %q: %s", name, err), + )) } } - f, err := os.Create(path) - if err != nil { - diags = diags.Append(tfdiags.Sourceless( - tfdiags.Error, - "Unable to create providers file", - fmt.Sprintf("Error when generating providers configuration at '%s': %s", path, err), - )) - return diags - } - defer f.Close() + return diags +} - err = providersTemplate.Execute(f, providerSources) - if err != nil { - diags = diags.Append(tfdiags.Sourceless( - tfdiags.Error, - "Unable to create providers file", - fmt.Sprintf("Error when generating providers configuration at '%s': %s", path, err), - )) - return diags +// Take a list of tokens and a separator token, and return two lists: one up to +// and including the first instance of the separator, and the rest of the +// tokens. If the separator is not present, return the entire list in the first +// return value. +func partitionTokensAfter(tokens hclwrite.Tokens, separator hclsyntax.TokenType) (hclwrite.Tokens, hclwrite.Tokens) { + for i := 0; i < len(tokens); i++ { + if tokens[i].Type == separator { + return tokens[0 : i+1], tokens[i+1:] + } } - return nil + return tokens, nil +} + +// Generate a list of tokens for a comment explaining that a provider source +// could not be detected. +func noSourceDetectedComment(name string) hclwrite.Tokens { + comment := fmt.Sprintf(`# TF-UPGRADE-TODO +# +# No source detected for this provider. You must add a source address +# in the following format: +# +# source = "your-registry.example.com/organization/%s" +# +# For more information, see the provider source documentation: +# +# https://www.terraform.io/docs/configuration/providers.html#provider-source`, name) + + var tokens hclwrite.Tokens + for _, line := range strings.Split(comment, "\n") { + tokens = append(tokens, &hclwrite.Token{Type: hclsyntax.TokenNewline, Bytes: []byte{'\n'}}) + tokens = append(tokens, &hclwrite.Token{Type: hclsyntax.TokenComment, Bytes: []byte(line)}) + } + return tokens } func (c *ZeroThirteenUpgradeCommand) Help() string { diff --git a/command/013_config_upgrade_test.go b/command/013_config_upgrade_test.go index 219beb551..b302cfc4b 100644 --- a/command/013_config_upgrade_test.go +++ b/command/013_config_upgrade_test.go @@ -1,44 +1,115 @@ package command import ( - "bytes" "io/ioutil" + "net/http" + "net/http/httptest" "os" + "path" + "path/filepath" "strings" "testing" + "github.com/google/go-cmp/cmp" + svchost "github.com/hashicorp/terraform-svchost" + "github.com/hashicorp/terraform-svchost/disco" "github.com/hashicorp/terraform/helper/copy" + "github.com/hashicorp/terraform/internal/getproviders" "github.com/mitchellh/cli" ) -func TestZeroThirteenUpgrade_success(t *testing.T) { - testCases := map[string]struct { - path string - args []string - out string - }{ - "implicit": { - path: "013upgrade-implicit-providers", - out: "providers.tf", - }, - "explicit": { - path: "013upgrade-explicit-providers", - out: "providers.tf", - }, - "subdir": { - path: "013upgrade-subdir", - args: []string{"subdir"}, - out: "subdir/providers.tf", - }, - "fileExists": { - path: "013upgrade-file-exists", - out: "providers-1.tf", - }, +// This map from provider type name to namespace is used by the fake registry +// when called via LookupLegacyProvider. Providers not in this map will return +// a 404 Not Found error. +var legacyProviderNamespaces = map[string]string{ + "foo": "hashicorp", + "bar": "hashicorp", + "baz": "terraform-providers", +} + +func verifyExpectedFiles(t *testing.T, expectedPath string) { + // Compare output and expected file trees + var outputFiles, expectedFiles []string + + // Gather list of output files in the current working directory + err := filepath.Walk(".", func(path string, info os.FileInfo, err error) error { + if !info.IsDir() { + outputFiles = append(outputFiles, path) + } + return nil + }) + if err != nil { + t.Fatal("error listing output files:", err) } - for name, tc := range testCases { + + // Gather list of expected files + revertChdir := testChdir(t, expectedPath) + err = filepath.Walk(".", func(path string, info os.FileInfo, err error) error { + if !info.IsDir() { + expectedFiles = append(expectedFiles, path) + } + return nil + }) + if err != nil { + t.Fatal("error listing expected files:", err) + } + revertChdir() + + // If the file trees don't match, give up early + if diff := cmp.Diff(expectedFiles, outputFiles); diff != "" { + t.Fatalf("expected and output file trees do not match\n%s", diff) + } + + // Check that the contents of each file is correct + for _, filePath := range outputFiles { + output, err := ioutil.ReadFile(path.Join(".", filePath)) + if err != nil { + t.Fatalf("failed to read output %s: %s", filePath, err) + } + expected, err := ioutil.ReadFile(path.Join(expectedPath, filePath)) + if err != nil { + t.Fatalf("failed to read expected %s: %s", filePath, err) + } + + if diff := cmp.Diff(expected, output); diff != "" { + t.Fatalf("expected and output file for %s do not match\n%s", filePath, diff) + } + } +} + +func TestZeroThirteenUpgrade_success(t *testing.T) { + registrySource, close := testRegistrySource(t) + defer close() + + testCases := map[string]string{ + "implicit": "013upgrade-implicit-providers", + "explicit": "013upgrade-explicit-providers", + "provider not found": "013upgrade-provider-not-found", + "implicit not found": "013upgrade-implicit-not-found", + "file exists": "013upgrade-file-exists", + "no providers": "013upgrade-no-providers", + "submodule": "013upgrade-submodule", + "providers with source": "013upgrade-providers-with-source", + "preserves comments": "013upgrade-preserves-comments", + "multiple blocks": "013upgrade-multiple-blocks", + "multiple files": "013upgrade-multiple-files", + "existing providers.tf": "013upgrade-existing-providers-tf", + "skipped files": "013upgrade-skipped-files", + } + for name, testPath := range testCases { t.Run(name, func(t *testing.T) { + inputPath, err := filepath.Abs(testFixturePath(path.Join(testPath, "input"))) + if err != nil { + t.Fatalf("failed to find input path %s: %s", testPath, err) + } + + expectedPath, err := filepath.Abs(testFixturePath(path.Join(testPath, "expected"))) + if err != nil { + t.Fatalf("failed to find expected path %s: %s", testPath, err) + } + td := tempDir(t) - copy.CopyDir(testFixturePath(tc.path), td) + copy.CopyDir(inputPath, td) defer os.RemoveAll(td) defer testChdir(t, td)() @@ -46,11 +117,12 @@ func TestZeroThirteenUpgrade_success(t *testing.T) { c := &ZeroThirteenUpgradeCommand{ Meta: Meta{ testingOverrides: metaOverridesForProvider(testProvider()), + ProviderSource: registrySource, Ui: ui, }, } - if code := c.Run(tc.args); code != 0 { + if code := c.Run(nil); code != 0 { t.Fatalf("bad: \n%s", ui.ErrorWriter.String()) } @@ -59,22 +131,95 @@ func TestZeroThirteenUpgrade_success(t *testing.T) { t.Fatal("unexpected output:", output) } - actual, err := ioutil.ReadFile(tc.out) - if err != nil { - t.Fatalf("failed to read output %s: %s", tc.out, err) - } - expected, err := ioutil.ReadFile("expected/providers.tf") - if err != nil { - t.Fatal("failed to read expected/providers.tf", err) - } - - if !bytes.Equal(actual, expected) { - t.Fatalf("actual output: \n%s\nexpected output: \n%s", string(actual), string(expected)) - } + verifyExpectedFiles(t, expectedPath) }) } } +// Ensure that non-default upgrade paths are supported, and that the output is +// in the correct place. This test is very similar to the table tests above, +// but with a different expected output path, and with an argument passed to +// the Run call. +func TestZeroThirteenUpgrade_submodule(t *testing.T) { + registrySource, close := testRegistrySource(t) + defer close() + + testPath := "013upgrade-submodule" + + inputPath, err := filepath.Abs(testFixturePath(path.Join(testPath, "input"))) + if err != nil { + t.Fatalf("failed to find input path %s: %s", testPath, err) + } + + // The expected output for processing a submodule is different + expectedPath, err := filepath.Abs(testFixturePath(path.Join(testPath, "expected-module"))) + if err != nil { + t.Fatalf("failed to find expected path %s: %s", testPath, err) + } + + td := tempDir(t) + copy.CopyDir(inputPath, td) + defer os.RemoveAll(td) + defer testChdir(t, td)() + + ui := new(cli.MockUi) + c := &ZeroThirteenUpgradeCommand{ + Meta: Meta{ + testingOverrides: metaOverridesForProvider(testProvider()), + ProviderSource: registrySource, + Ui: ui, + }, + } + + // Here we pass a target module directory to process + if code := c.Run([]string{"module"}); code != 0 { + t.Fatalf("bad: \n%s", ui.ErrorWriter.String()) + } + + output := ui.OutputWriter.String() + if !strings.Contains(output, "Upgrade complete") { + t.Fatal("unexpected output:", output) + } + + verifyExpectedFiles(t, expectedPath) +} + +// Verify that JSON and override files are skipped with a warning. Generated +// output for this config is verified in the table driven tests above. +func TestZeroThirteenUpgrade_skippedFiles(t *testing.T) { + inputPath := testFixturePath(path.Join("013upgrade-skipped-files", "input")) + + td := tempDir(t) + copy.CopyDir(inputPath, td) + defer os.RemoveAll(td) + defer testChdir(t, td)() + + ui := new(cli.MockUi) + c := &ZeroThirteenUpgradeCommand{ + Meta: Meta{ + testingOverrides: metaOverridesForProvider(testProvider()), + Ui: ui, + }, + } + + if code := c.Run(nil); code != 0 { + t.Fatalf("bad: \n%s", ui.ErrorWriter.String()) + } + + output := ui.OutputWriter.String() + if !strings.Contains(output, "Upgrade complete") { + t.Fatal("unexpected output:", output) + } + + errMsg := ui.ErrorWriter.String() + if !strings.Contains(errMsg, `The JSON configuration file "variables.tf.json" was skipped`) { + t.Fatal("missing JSON skipped file warning:", errMsg) + } + if !strings.Contains(errMsg, `The override configuration file "bar_override.tf" was skipped`) { + t.Fatal("missing override skipped file warning:", errMsg) + } +} + func TestZeroThirteenUpgrade_invalidFlags(t *testing.T) { td := tempDir(t) os.MkdirAll(td, 0755) @@ -147,54 +292,66 @@ func TestZeroThirteenUpgrade_empty(t *testing.T) { } } -func TestZeroThirteenUpgrade_invalidProviderVersion(t *testing.T) { - td := tempDir(t) - copy.CopyDir(testFixturePath("013upgrade-invalid"), td) - defer os.RemoveAll(td) - defer testChdir(t, td)() +// testServices starts up a local HTTP server running a fake provider registry +// service which responds only to discovery requests and legacy provider lookup +// API calls. +// +// The final return value is a function to call at the end of a test function +// to shut down the test server. After you call that function, the discovery +// object becomes useless. +func testServices(t *testing.T) (services *disco.Disco, cleanup func()) { + server := httptest.NewServer(http.HandlerFunc(fakeRegistryHandler)) - ui := new(cli.MockUi) - c := &ZeroThirteenUpgradeCommand{ - Meta: Meta{ - testingOverrides: metaOverridesForProvider(testProvider()), - Ui: ui, - }, - } + services = disco.New() + services.ForceHostServices(svchost.Hostname("registry.terraform.io"), map[string]interface{}{ + "providers.v1": server.URL + "/providers/v1/", + }) - if code := c.Run(nil); code == 0 { - t.Fatal("expected error, got:", ui.OutputWriter) - } - - errMsg := ui.ErrorWriter.String() - if !strings.Contains(errMsg, "Invalid provider version constraint") { - t.Fatal("unexpected error:", errMsg) + return services, func() { + server.Close() } } -func TestZeroThirteenUpgrade_noProviders(t *testing.T) { - td := tempDir(t) - copy.CopyDir(testFixturePath("013upgrade-no-providers"), td) - defer os.RemoveAll(td) - defer testChdir(t, td)() +// testRegistrySource is a wrapper around testServices that uses the created +// discovery object to produce a Source instance that is ready to use with the +// fake registry services. +// +// As with testServices, the final return value is a function to call at the end +// of your test in order to shut down the test server. +func testRegistrySource(t *testing.T) (source *getproviders.RegistrySource, cleanup func()) { + services, close := testServices(t) + source = getproviders.NewRegistrySource(services) + return source, close +} - ui := new(cli.MockUi) - c := &ZeroThirteenUpgradeCommand{ - Meta: Meta{ - testingOverrides: metaOverridesForProvider(testProvider()), - Ui: ui, - }, +func fakeRegistryHandler(resp http.ResponseWriter, req *http.Request) { + path := req.URL.EscapedPath() + + if !strings.HasPrefix(path, "/providers/v1/") { + resp.WriteHeader(404) + resp.Write([]byte(`not a provider registry endpoint`)) + return } - if code := c.Run(nil); code != 0 { - t.Fatalf("bad: \n%s", ui.ErrorWriter.String()) + pathParts := strings.Split(path, "/")[3:] + + if len(pathParts) != 2 { + resp.WriteHeader(404) + resp.Write([]byte(`unrecognized path scheme`)) + return } - output := ui.OutputWriter.String() - if !strings.Contains(output, "No non-default providers found") { - t.Fatal("unexpected output:", output) + if pathParts[0] != "-" { + resp.WriteHeader(404) + resp.Write([]byte(`this registry only supports legacy namespace lookup requests`)) } - if _, err := os.Stat("providers.tf"); !os.IsNotExist(err) { - t.Fatal("unexpected providers.tf created") + if namespace, ok := legacyProviderNamespaces[pathParts[1]]; ok { + resp.Header().Set("Content-Type", "application/json") + resp.WriteHeader(200) + resp.Write([]byte(`{"namespace":"` + namespace + `"}`)) + } else { + resp.WriteHeader(404) + resp.Write([]byte(`provider not found`)) } } diff --git a/command/testdata/013upgrade-existing-providers-tf/expected/main.tf b/command/testdata/013upgrade-existing-providers-tf/expected/main.tf new file mode 100644 index 000000000..109e0699c --- /dev/null +++ b/command/testdata/013upgrade-existing-providers-tf/expected/main.tf @@ -0,0 +1,5 @@ +resource foo_resource b {} +resource bar_resource c {} +resource bar_resource ab { + provider = baz +} diff --git a/command/testdata/013upgrade-existing-providers-tf/expected/providers.tf b/command/testdata/013upgrade-existing-providers-tf/expected/providers.tf new file mode 100644 index 000000000..8394c756e --- /dev/null +++ b/command/testdata/013upgrade-existing-providers-tf/expected/providers.tf @@ -0,0 +1,16 @@ +# This is a file called providers.tf which does not originally have a +# required_providers block. +resource foo_resource a {} +terraform { + required_providers { + bar = { + source = "hashicorp/bar" + } + baz = { + source = "terraform-providers/baz" + } + foo = { + source = "hashicorp/foo" + } + } +} diff --git a/command/testdata/013upgrade-existing-providers-tf/input/main.tf b/command/testdata/013upgrade-existing-providers-tf/input/main.tf new file mode 100644 index 000000000..109e0699c --- /dev/null +++ b/command/testdata/013upgrade-existing-providers-tf/input/main.tf @@ -0,0 +1,5 @@ +resource foo_resource b {} +resource bar_resource c {} +resource bar_resource ab { + provider = baz +} diff --git a/command/testdata/013upgrade-existing-providers-tf/input/providers.tf b/command/testdata/013upgrade-existing-providers-tf/input/providers.tf new file mode 100644 index 000000000..04203320a --- /dev/null +++ b/command/testdata/013upgrade-existing-providers-tf/input/providers.tf @@ -0,0 +1,3 @@ +# This is a file called providers.tf which does not originally have a +# required_providers block. +resource foo_resource a {} diff --git a/command/testdata/013upgrade-explicit-providers/expected/main.tf b/command/testdata/013upgrade-explicit-providers/expected/main.tf new file mode 100644 index 000000000..83d1158f4 --- /dev/null +++ b/command/testdata/013upgrade-explicit-providers/expected/main.tf @@ -0,0 +1,19 @@ +provider foo { + version = "1.2.3" +} + +terraform { + required_providers { + bar = { + source = "hashicorp/bar" + version = "1.0.0" + } + baz = { + source = "terraform-providers/baz" + version = "~> 2.0.0" + } + foo = { + source = "hashicorp/foo" + } + } +} diff --git a/command/testdata/013upgrade-explicit-providers/expected/providers.tf b/command/testdata/013upgrade-explicit-providers/expected/providers.tf deleted file mode 100644 index c12b7df0a..000000000 --- a/command/testdata/013upgrade-explicit-providers/expected/providers.tf +++ /dev/null @@ -1,13 +0,0 @@ -terraform { - required_providers { - bar = { - source = "registry.terraform.io/-/bar" - } - baz = { - source = "registry.terraform.io/-/baz" - } - foo = { - source = "registry.terraform.io/-/foo" - } - } -} diff --git a/command/testdata/013upgrade-explicit-providers/input/main.tf b/command/testdata/013upgrade-explicit-providers/input/main.tf new file mode 100644 index 000000000..f3c982a04 --- /dev/null +++ b/command/testdata/013upgrade-explicit-providers/input/main.tf @@ -0,0 +1,12 @@ +provider foo { + version = "1.2.3" +} + +terraform { + required_providers { + bar = "1.0.0" + baz = { + version = "~> 2.0.0" + } + } +} diff --git a/command/testdata/013upgrade-file-exists/expected/providers.tf b/command/testdata/013upgrade-file-exists/expected/providers.tf index 5c4b69105..f0f674de4 100644 --- a/command/testdata/013upgrade-file-exists/expected/providers.tf +++ b/command/testdata/013upgrade-file-exists/expected/providers.tf @@ -1,10 +1,12 @@ +provider foo {} +provider bar {} terraform { required_providers { - alpha = { - source = "registry.terraform.io/-/alpha" + bar = { + source = "hashicorp/bar" } - beta = { - source = "registry.terraform.io/-/beta" + foo = { + source = "hashicorp/foo" } } } diff --git a/command/testdata/013upgrade-file-exists/input/providers.tf b/command/testdata/013upgrade-file-exists/input/providers.tf new file mode 100644 index 000000000..e9f22a979 --- /dev/null +++ b/command/testdata/013upgrade-file-exists/input/providers.tf @@ -0,0 +1,2 @@ +provider foo {} +provider bar {} diff --git a/command/testdata/013upgrade-file-exists/providers.tf b/command/testdata/013upgrade-file-exists/providers.tf deleted file mode 100644 index a13ab6d45..000000000 --- a/command/testdata/013upgrade-file-exists/providers.tf +++ /dev/null @@ -1,2 +0,0 @@ -provider alpha {} -provider beta {} diff --git a/command/testdata/013upgrade-implicit-not-found/expected/main.tf b/command/testdata/013upgrade-implicit-not-found/expected/main.tf new file mode 100644 index 000000000..afb4083b5 --- /dev/null +++ b/command/testdata/013upgrade-implicit-not-found/expected/main.tf @@ -0,0 +1 @@ +resource something_resource a {} diff --git a/command/testdata/013upgrade-implicit-not-found/expected/providers.tf b/command/testdata/013upgrade-implicit-not-found/expected/providers.tf new file mode 100644 index 000000000..d076393f4 --- /dev/null +++ b/command/testdata/013upgrade-implicit-not-found/expected/providers.tf @@ -0,0 +1,16 @@ +terraform { + required_providers { + something = { + # TF-UPGRADE-TODO + # + # No source detected for this provider. You must add a source address + # in the following format: + # + # source = "your-registry.example.com/organization/something" + # + # For more information, see the provider source documentation: + # + # https://www.terraform.io/docs/configuration/providers.html#provider-source + } + } +} diff --git a/command/testdata/013upgrade-implicit-not-found/input/main.tf b/command/testdata/013upgrade-implicit-not-found/input/main.tf new file mode 100644 index 000000000..afb4083b5 --- /dev/null +++ b/command/testdata/013upgrade-implicit-not-found/input/main.tf @@ -0,0 +1 @@ +resource something_resource a {} diff --git a/command/testdata/013upgrade-implicit-providers/expected/main.tf b/command/testdata/013upgrade-implicit-providers/expected/main.tf new file mode 100644 index 000000000..109e0699c --- /dev/null +++ b/command/testdata/013upgrade-implicit-providers/expected/main.tf @@ -0,0 +1,5 @@ +resource foo_resource b {} +resource bar_resource c {} +resource bar_resource ab { + provider = baz +} diff --git a/command/testdata/013upgrade-implicit-providers/expected/providers.tf b/command/testdata/013upgrade-implicit-providers/expected/providers.tf index 1f1064007..9b2a488e5 100644 --- a/command/testdata/013upgrade-implicit-providers/expected/providers.tf +++ b/command/testdata/013upgrade-implicit-providers/expected/providers.tf @@ -1,10 +1,13 @@ terraform { required_providers { - cloud = { - source = "registry.terraform.io/-/cloud" + bar = { + source = "hashicorp/bar" } - some = { - source = "registry.terraform.io/-/some" + baz = { + source = "terraform-providers/baz" + } + foo = { + source = "hashicorp/foo" } } } diff --git a/command/testdata/013upgrade-implicit-providers/input/main.tf b/command/testdata/013upgrade-implicit-providers/input/main.tf new file mode 100644 index 000000000..109e0699c --- /dev/null +++ b/command/testdata/013upgrade-implicit-providers/input/main.tf @@ -0,0 +1,5 @@ +resource foo_resource b {} +resource bar_resource c {} +resource bar_resource ab { + provider = baz +} diff --git a/command/testdata/013upgrade-implicit-providers/main.tf b/command/testdata/013upgrade-implicit-providers/main.tf deleted file mode 100644 index be9fa73e1..000000000 --- a/command/testdata/013upgrade-implicit-providers/main.tf +++ /dev/null @@ -1,2 +0,0 @@ -resource some_resource a {} -resource cloud_horse x {} diff --git a/command/testdata/013upgrade-invalid/main.tf b/command/testdata/013upgrade-invalid/main.tf deleted file mode 100644 index b81807c3e..000000000 --- a/command/testdata/013upgrade-invalid/main.tf +++ /dev/null @@ -1,3 +0,0 @@ -provider "invalid" { - version = "invalid" -} diff --git a/command/testdata/013upgrade-multiple-blocks/expected/bar-baz.tf b/command/testdata/013upgrade-multiple-blocks/expected/bar-baz.tf new file mode 100644 index 000000000..6887569dd --- /dev/null +++ b/command/testdata/013upgrade-multiple-blocks/expected/bar-baz.tf @@ -0,0 +1,5 @@ +resource bar_instance a {} +resource baz_instance b {} +terraform { + required_version = "> 0.12.0" +} diff --git a/command/testdata/013upgrade-multiple-blocks/expected/foo.tf b/command/testdata/013upgrade-multiple-blocks/expected/foo.tf new file mode 100644 index 000000000..5d9203723 --- /dev/null +++ b/command/testdata/013upgrade-multiple-blocks/expected/foo.tf @@ -0,0 +1 @@ +resource foo_instance c {} diff --git a/command/testdata/013upgrade-multiple-blocks/expected/providers.tf b/command/testdata/013upgrade-multiple-blocks/expected/providers.tf new file mode 100644 index 000000000..195b21389 --- /dev/null +++ b/command/testdata/013upgrade-multiple-blocks/expected/providers.tf @@ -0,0 +1,15 @@ +terraform { + required_providers { + bar = { + source = "registry.acme.corp/acme/bar" + } + baz = { + source = "terraform-providers/baz" + version = "~> 2.0.0" + } + foo = { + source = "hashicorp/foo" + version = "0.5" + } + } +} diff --git a/command/testdata/013upgrade-multiple-blocks/input/bar-baz.tf b/command/testdata/013upgrade-multiple-blocks/input/bar-baz.tf new file mode 100644 index 000000000..d0e6fe8c6 --- /dev/null +++ b/command/testdata/013upgrade-multiple-blocks/input/bar-baz.tf @@ -0,0 +1,10 @@ +resource bar_instance a {} +resource baz_instance b {} +terraform { + required_version = "> 0.12.0" + required_providers { + bar = { + source = "registry.acme.corp/acme/bar" + } + } +} diff --git a/command/testdata/013upgrade-multiple-blocks/input/foo.tf b/command/testdata/013upgrade-multiple-blocks/input/foo.tf new file mode 100644 index 000000000..99aaf52a0 --- /dev/null +++ b/command/testdata/013upgrade-multiple-blocks/input/foo.tf @@ -0,0 +1,11 @@ +terraform { + required_providers { + baz = "~> 2.0.0" + } +} +terraform { + required_providers { + foo = "0.5" + } +} +resource foo_instance c {} diff --git a/command/testdata/013upgrade-multiple-files/expected/main.tf b/command/testdata/013upgrade-multiple-files/expected/main.tf new file mode 100644 index 000000000..fa25239e9 --- /dev/null +++ b/command/testdata/013upgrade-multiple-files/expected/main.tf @@ -0,0 +1,4 @@ +# This file starts with a resource and a required providers block, and should +# end up with just the resource. +resource foo_instance a {} + diff --git a/command/testdata/013upgrade-multiple-files/expected/providers.tf b/command/testdata/013upgrade-multiple-files/expected/providers.tf new file mode 100644 index 000000000..b46e7180d --- /dev/null +++ b/command/testdata/013upgrade-multiple-files/expected/providers.tf @@ -0,0 +1,16 @@ +# This file starts with a resource and a required providers block, and should +# end up with the full required providers configuration. This file is chosen +# to keep the required providers block because its file name is "providers.tf". +resource bar_instance b {} + +terraform { + required_providers { + bar = { + source = "registry.acme.corp/acme/bar" + } + foo = { + source = "hashicorp/foo" + version = "1.0.0" + } + } +} diff --git a/command/testdata/013upgrade-multiple-files/input/main.tf b/command/testdata/013upgrade-multiple-files/input/main.tf new file mode 100644 index 000000000..168ebff7e --- /dev/null +++ b/command/testdata/013upgrade-multiple-files/input/main.tf @@ -0,0 +1,9 @@ +# This file starts with a resource and a required providers block, and should +# end up with just the resource. +resource foo_instance a {} + +terraform { + required_providers { + foo = "1.0.0" + } +} diff --git a/command/testdata/013upgrade-multiple-files/input/providers.tf b/command/testdata/013upgrade-multiple-files/input/providers.tf new file mode 100644 index 000000000..8823c6fe6 --- /dev/null +++ b/command/testdata/013upgrade-multiple-files/input/providers.tf @@ -0,0 +1,12 @@ +# This file starts with a resource and a required providers block, and should +# end up with the full required providers configuration. This file is chosen +# to keep the required providers block because its file name is "providers.tf". +resource bar_instance b {} + +terraform { + required_providers { + bar = { + source = "registry.acme.corp/acme/bar" + } + } +} diff --git a/command/testdata/013upgrade-no-providers/main.tf b/command/testdata/013upgrade-no-providers/expected/main.tf similarity index 100% rename from command/testdata/013upgrade-no-providers/main.tf rename to command/testdata/013upgrade-no-providers/expected/main.tf diff --git a/command/testdata/013upgrade-no-providers/input/main.tf b/command/testdata/013upgrade-no-providers/input/main.tf new file mode 100644 index 000000000..d7ad43496 --- /dev/null +++ b/command/testdata/013upgrade-no-providers/input/main.tf @@ -0,0 +1,11 @@ +variable "x" { + default = 3 +} + +variable "y" { + default = 5 +} + +output "product" { + value = var.x * var.y +} diff --git a/command/testdata/013upgrade-preserves-comments/expected/main.tf b/command/testdata/013upgrade-preserves-comments/expected/main.tf new file mode 100644 index 000000000..13bbe0b95 --- /dev/null +++ b/command/testdata/013upgrade-preserves-comments/expected/main.tf @@ -0,0 +1,16 @@ +resource foo_instance a {} +resource bar_instance b {} + +terraform { + # Provider requirements go here + required_providers { + # Pin bar to this version + bar = { + source = "hashicorp/bar" + version = "0.5.0" + } + foo = { + source = "hashicorp/foo" + } + } +} diff --git a/command/testdata/013upgrade-preserves-comments/input/main.tf b/command/testdata/013upgrade-preserves-comments/input/main.tf new file mode 100644 index 000000000..c384d2819 --- /dev/null +++ b/command/testdata/013upgrade-preserves-comments/input/main.tf @@ -0,0 +1,10 @@ +resource foo_instance a {} +resource bar_instance b {} + +terraform { + # Provider requirements go here + required_providers { + # Pin bar to this version + bar = "0.5.0" + } +} diff --git a/command/testdata/013upgrade-provider-not-found/expected/main.tf b/command/testdata/013upgrade-provider-not-found/expected/main.tf new file mode 100644 index 000000000..951db47d7 --- /dev/null +++ b/command/testdata/013upgrade-provider-not-found/expected/main.tf @@ -0,0 +1,26 @@ +provider foo {} + +terraform { + required_providers { + bar = { + source = "hashicorp/bar" + version = "1.0.0" + } + unknown = { + # TF-UPGRADE-TODO + # + # No source detected for this provider. You must add a source address + # in the following format: + # + # source = "your-registry.example.com/organization/unknown" + # + # For more information, see the provider source documentation: + # + # https://www.terraform.io/docs/configuration/providers.html#provider-source + version = "~> 2.0.0" + } + foo = { + source = "hashicorp/foo" + } + } +} diff --git a/command/testdata/013upgrade-explicit-providers/main.tf b/command/testdata/013upgrade-provider-not-found/input/main.tf similarity index 87% rename from command/testdata/013upgrade-explicit-providers/main.tf rename to command/testdata/013upgrade-provider-not-found/input/main.tf index 80ee3b7fe..9b6e38104 100644 --- a/command/testdata/013upgrade-explicit-providers/main.tf +++ b/command/testdata/013upgrade-provider-not-found/input/main.tf @@ -3,7 +3,7 @@ provider foo {} terraform { required_providers { bar = "1.0.0" - baz = { + unknown = { version = "~> 2.0.0" } } diff --git a/command/testdata/013upgrade-providers-with-source/expected/main.tf b/command/testdata/013upgrade-providers-with-source/expected/main.tf new file mode 100644 index 000000000..3d366ebfa --- /dev/null +++ b/command/testdata/013upgrade-providers-with-source/expected/main.tf @@ -0,0 +1,7 @@ +terraform { + required_providers { + aws = { + source = "registry.acme.corp/acme/aws" + } + } +} diff --git a/command/testdata/013upgrade-providers-with-source/input/main.tf b/command/testdata/013upgrade-providers-with-source/input/main.tf new file mode 100644 index 000000000..3d366ebfa --- /dev/null +++ b/command/testdata/013upgrade-providers-with-source/input/main.tf @@ -0,0 +1,7 @@ +terraform { + required_providers { + aws = { + source = "registry.acme.corp/acme/aws" + } + } +} diff --git a/command/testdata/013upgrade-skipped-files/expected/bar_override.tf b/command/testdata/013upgrade-skipped-files/expected/bar_override.tf new file mode 100644 index 000000000..524ef2c57 --- /dev/null +++ b/command/testdata/013upgrade-skipped-files/expected/bar_override.tf @@ -0,0 +1,3 @@ +provider bar { + version = "1.0.0" +} diff --git a/command/testdata/013upgrade-skipped-files/expected/main.tf b/command/testdata/013upgrade-skipped-files/expected/main.tf new file mode 100644 index 000000000..83d1158f4 --- /dev/null +++ b/command/testdata/013upgrade-skipped-files/expected/main.tf @@ -0,0 +1,19 @@ +provider foo { + version = "1.2.3" +} + +terraform { + required_providers { + bar = { + source = "hashicorp/bar" + version = "1.0.0" + } + baz = { + source = "terraform-providers/baz" + version = "~> 2.0.0" + } + foo = { + source = "hashicorp/foo" + } + } +} diff --git a/command/testdata/013upgrade-skipped-files/expected/variables.tf.json b/command/testdata/013upgrade-skipped-files/expected/variables.tf.json new file mode 100644 index 000000000..6126f1f0c --- /dev/null +++ b/command/testdata/013upgrade-skipped-files/expected/variables.tf.json @@ -0,0 +1,12 @@ +{ + "variable": { + "example": { + "default": "hello" + } + } + "terraform": { + "required_providers": { + "aws": "2.50.0" + } + } +} diff --git a/command/testdata/013upgrade-skipped-files/input/bar_override.tf b/command/testdata/013upgrade-skipped-files/input/bar_override.tf new file mode 100644 index 000000000..524ef2c57 --- /dev/null +++ b/command/testdata/013upgrade-skipped-files/input/bar_override.tf @@ -0,0 +1,3 @@ +provider bar { + version = "1.0.0" +} diff --git a/command/testdata/013upgrade-skipped-files/input/main.tf b/command/testdata/013upgrade-skipped-files/input/main.tf new file mode 100644 index 000000000..f3c982a04 --- /dev/null +++ b/command/testdata/013upgrade-skipped-files/input/main.tf @@ -0,0 +1,12 @@ +provider foo { + version = "1.2.3" +} + +terraform { + required_providers { + bar = "1.0.0" + baz = { + version = "~> 2.0.0" + } + } +} diff --git a/command/testdata/013upgrade-skipped-files/input/variables.tf.json b/command/testdata/013upgrade-skipped-files/input/variables.tf.json new file mode 100644 index 000000000..6126f1f0c --- /dev/null +++ b/command/testdata/013upgrade-skipped-files/input/variables.tf.json @@ -0,0 +1,12 @@ +{ + "variable": { + "example": { + "default": "hello" + } + } + "terraform": { + "required_providers": { + "aws": "2.50.0" + } + } +} diff --git a/command/testdata/013upgrade-subdir/expected/providers.tf b/command/testdata/013upgrade-subdir/expected/providers.tf deleted file mode 100644 index 5c4b69105..000000000 --- a/command/testdata/013upgrade-subdir/expected/providers.tf +++ /dev/null @@ -1,10 +0,0 @@ -terraform { - required_providers { - alpha = { - source = "registry.terraform.io/-/alpha" - } - beta = { - source = "registry.terraform.io/-/beta" - } - } -} diff --git a/command/testdata/013upgrade-subdir/subdir/main.tf b/command/testdata/013upgrade-subdir/subdir/main.tf deleted file mode 100644 index 19cf29fdc..000000000 --- a/command/testdata/013upgrade-subdir/subdir/main.tf +++ /dev/null @@ -1,2 +0,0 @@ -resource beta_resource b {} -resource alpha_resource a {} diff --git a/command/testdata/013upgrade-submodule/expected-module/main.tf b/command/testdata/013upgrade-submodule/expected-module/main.tf new file mode 100644 index 000000000..6c4de24ad --- /dev/null +++ b/command/testdata/013upgrade-submodule/expected-module/main.tf @@ -0,0 +1,8 @@ +terraform { + required_providers { + bar = { + version = "~> 2.0.0" + } + } +} + diff --git a/command/testdata/013upgrade-submodule/expected-module/module/main.tf b/command/testdata/013upgrade-submodule/expected-module/module/main.tf new file mode 100644 index 000000000..0f5566e56 --- /dev/null +++ b/command/testdata/013upgrade-submodule/expected-module/module/main.tf @@ -0,0 +1 @@ +resource foo_resource b {} diff --git a/command/testdata/013upgrade-submodule/expected-module/module/providers.tf b/command/testdata/013upgrade-submodule/expected-module/module/providers.tf new file mode 100644 index 000000000..3af569d04 --- /dev/null +++ b/command/testdata/013upgrade-submodule/expected-module/module/providers.tf @@ -0,0 +1,7 @@ +terraform { + required_providers { + foo = { + source = "hashicorp/foo" + } + } +} diff --git a/command/testdata/013upgrade-submodule/expected/main.tf b/command/testdata/013upgrade-submodule/expected/main.tf new file mode 100644 index 000000000..89b6f4027 --- /dev/null +++ b/command/testdata/013upgrade-submodule/expected/main.tf @@ -0,0 +1,9 @@ +terraform { + required_providers { + bar = { + source = "hashicorp/bar" + version = "~> 2.0.0" + } + } +} + diff --git a/command/testdata/013upgrade-submodule/expected/module/main.tf b/command/testdata/013upgrade-submodule/expected/module/main.tf new file mode 100644 index 000000000..0f5566e56 --- /dev/null +++ b/command/testdata/013upgrade-submodule/expected/module/main.tf @@ -0,0 +1 @@ +resource foo_resource b {} diff --git a/command/testdata/013upgrade-submodule/input/main.tf b/command/testdata/013upgrade-submodule/input/main.tf new file mode 100644 index 000000000..6c4de24ad --- /dev/null +++ b/command/testdata/013upgrade-submodule/input/main.tf @@ -0,0 +1,8 @@ +terraform { + required_providers { + bar = { + version = "~> 2.0.0" + } + } +} + diff --git a/command/testdata/013upgrade-submodule/input/module/main.tf b/command/testdata/013upgrade-submodule/input/module/main.tf new file mode 100644 index 000000000..0f5566e56 --- /dev/null +++ b/command/testdata/013upgrade-submodule/input/module/main.tf @@ -0,0 +1 @@ +resource foo_resource b {} diff --git a/configs/provider_requirements.go b/configs/provider_requirements.go index d4218a6fc..531ba4f7f 100644 --- a/configs/provider_requirements.go +++ b/configs/provider_requirements.go @@ -12,6 +12,7 @@ import ( // parent. type RequiredProvider struct { Name string + Source string Type addrs.Provider Requirement VersionConstraint DeclRange hcl.Range @@ -67,7 +68,8 @@ func decodeRequiredProvidersBlock(block *hcl.Block) (*RequiredProviders, hcl.Dia } } if expr.Type().HasAttribute("source") { - fqn, sourceDiags := addrs.ParseProviderSourceString(expr.GetAttr("source").AsString()) + rp.Source = expr.GetAttr("source").AsString() + fqn, sourceDiags := addrs.ParseProviderSourceString(rp.Source) if sourceDiags.HasErrors() { hclDiags := sourceDiags.ToHCL() diff --git a/configs/provider_requirements_test.go b/configs/provider_requirements_test.go index a04714f55..d204223b9 100644 --- a/configs/provider_requirements_test.go +++ b/configs/provider_requirements_test.go @@ -21,6 +21,9 @@ var ( if x.Type != y.Type { return false } + if x.Source != y.Source { + return false + } if x.Requirement.Required.String() != y.Requirement.Required.String() { return false } @@ -90,6 +93,7 @@ func TestDecodeRequiredProvidersBlock(t *testing.T) { RequiredProviders: map[string]*RequiredProvider{ "my_test": { Name: "my_test", + Source: "mycloud/test", Type: addrs.NewProvider(addrs.DefaultRegistryHost, "mycloud", "test"), Requirement: testVC("2.0.0"), DeclRange: mockRange, @@ -128,6 +132,7 @@ func TestDecodeRequiredProvidersBlock(t *testing.T) { }, "my_test": { Name: "my_test", + Source: "mycloud/test", Type: addrs.NewProvider(addrs.DefaultRegistryHost, "mycloud", "test"), Requirement: testVC("2.0.0"), DeclRange: mockRange, @@ -183,6 +188,7 @@ func TestDecodeRequiredProvidersBlock(t *testing.T) { RequiredProviders: map[string]*RequiredProvider{ "my_test": { Name: "my_test", + Source: "some/invalid/provider/source/test", Type: addrs.Provider{}, Requirement: testVC("~>2.0.0"), DeclRange: mockRange, @@ -240,6 +246,7 @@ func TestDecodeRequiredProvidersBlock(t *testing.T) { RequiredProviders: map[string]*RequiredProvider{ "my_test": { Name: "my_test", + Source: "mycloud/test", Type: addrs.NewProvider(addrs.DefaultRegistryHost, "mycloud", "test"), DeclRange: mockRange, }, diff --git a/internal/getproviders/legacy_lookup.go b/internal/getproviders/legacy_lookup.go index 96901abab..71d9b15cd 100644 --- a/internal/getproviders/legacy_lookup.go +++ b/internal/getproviders/legacy_lookup.go @@ -80,6 +80,11 @@ func findLegacyProviderLookupSource(host svchost.Hostname, source Source) *Regis // just use it. return source + case *MemoizeSource: + // Also easy: the source is a memoize wrapper, so defer to its + // underlying source. + return findLegacyProviderLookupSource(host, source.underlying) + case MultiSource: // Trickier case: if it's a multisource then we need to scan over // its selectors until we find one that is a *RegistrySource _and_