From c7bbc09631783c6aaecdacfc58a135ced6bc0129 Mon Sep 17 00:00:00 2001 From: UKEME BASSEY Date: Mon, 15 Apr 2024 19:28:30 -0400 Subject: [PATCH] move all command args from command/init to args/init --- internal/command/apply.go | 4 +- internal/command/arguments/extended.go | 12 +- internal/command/arguments/flags.go | 60 +++++----- internal/command/arguments/init.go | 49 +++++++- internal/command/arguments/init_test.go | 12 +- internal/command/arguments/test.go | 2 +- internal/command/flag_kv.go | 13 -- internal/command/init.go | 83 +++++-------- internal/command/init_test.go | 3 +- internal/command/meta.go | 8 +- internal/command/meta_config.go | 57 --------- internal/command/meta_vars.go | 10 +- internal/command/plan.go | 4 +- internal/command/providers_lock.go | 3 +- internal/command/providers_mirror.go | 3 +- internal/command/refresh.go | 4 +- internal/command/test.go | 26 ++-- .../command/testdata/init-get/output.jsonlog | 1 - internal/command/views/init.go | 113 +++++++++++------- internal/command/views/init_test.go | 41 ++----- 20 files changed, 223 insertions(+), 285 deletions(-) diff --git a/internal/command/apply.go b/internal/command/apply.go index e332f94b47..1223e3d3b4 100644 --- a/internal/command/apply.go +++ b/internal/command/apply.go @@ -319,12 +319,12 @@ func (c *ApplyCommand) GatherVariables(opReq *backendrun.Operation, args *argume // package directly, removing this shim layer. varArgs := args.All() - items := make([]rawFlag, len(varArgs)) + items := make([]arguments.FlagNameValue, len(varArgs)) for i := range varArgs { items[i].Name = varArgs[i].Name items[i].Value = varArgs[i].Value } - c.Meta.variableArgs = rawFlags{items: &items} + c.Meta.variableArgs = arguments.FlagNameValueSlice{Items: &items} opReq.Variables, diags = c.collectVariableValues() return diags diff --git a/internal/command/arguments/extended.go b/internal/command/arguments/extended.go index 6e9aadea2a..3b3c7cc759 100644 --- a/internal/command/arguments/extended.go +++ b/internal/command/arguments/extended.go @@ -192,13 +192,13 @@ func (o *Operation) Parse() tfdiags.Diagnostics { } // Vars describes arguments which specify non-default variable values. This -// interfce is unfortunately obscure, because the order of the CLI arguments +// interface is unfortunately obscure, because the order of the CLI arguments // determines the final value of the gathered variables. In future it might be // desirable for the arguments package to handle the gathering of variables // directly, returning a map of variable values. type Vars struct { - vars *flagNameValueSlice - varFiles *flagNameValueSlice + vars *FlagNameValueSlice + varFiles *FlagNameValueSlice } func (v *Vars) All() []FlagNameValue { @@ -239,14 +239,14 @@ func extendedFlagSet(name string, state *State, operation *Operation, vars *Vars f.BoolVar(&operation.Refresh, "refresh", true, "refresh") f.BoolVar(&operation.destroyRaw, "destroy", false, "destroy") f.BoolVar(&operation.refreshOnlyRaw, "refresh-only", false, "refresh-only") - f.Var((*flagStringSlice)(&operation.targetsRaw), "target", "target") - f.Var((*flagStringSlice)(&operation.forceReplaceRaw), "replace", "replace") + f.Var((*FlagStringSlice)(&operation.targetsRaw), "target", "target") + f.Var((*FlagStringSlice)(&operation.forceReplaceRaw), "replace", "replace") } // Gather all -var and -var-file arguments into one heterogenous structure // to preserve the overall order. if vars != nil { - varsFlags := newFlagNameValueSlice("-var") + varsFlags := NewFlagNameValueSlice("-var") varFilesFlags := varsFlags.Alias("-var-file") vars.vars = &varsFlags vars.varFiles = &varFilesFlags diff --git a/internal/command/arguments/flags.go b/internal/command/arguments/flags.go index 7a19a544ee..64bf18ddd9 100644 --- a/internal/command/arguments/flags.go +++ b/internal/command/arguments/flags.go @@ -8,72 +8,68 @@ import ( "fmt" ) -// flagStringSlice is a flag.Value implementation which allows collecting +// FlagStringSlice is a flag.Value implementation which allows collecting // multiple instances of a single flag into a slice. This is used for flags // such as -target=aws_instance.foo and -var x=y. -type flagStringSlice []string +type FlagStringSlice []string -var _ flag.Value = (*flagStringSlice)(nil) +var _ flag.Value = (*FlagStringSlice)(nil) -func (v *flagStringSlice) String() string { +func (v *FlagStringSlice) String() string { return "" } -func (v *flagStringSlice) Set(raw string) error { +func (v *FlagStringSlice) Set(raw string) error { *v = append(*v, raw) return nil } -// flagNameValueSlice is a flag.Value implementation that appends raw flag +// FlagNameValueSlice is a flag.Value implementation that appends raw flag // names and values to a slice. This is used to collect a sequence of flags // with possibly different names, preserving the overall order. -// -// FIXME: this is a copy of rawFlags from command/meta_config.go, with the -// eventual aim of replacing it altogether by gathering variables in the -// arguments package. -type flagNameValueSlice struct { - flagName string - items *[]FlagNameValue +type FlagNameValueSlice struct { + FlagName string + Items *[]FlagNameValue } -var _ flag.Value = flagNameValueSlice{} +var _ flag.Value = FlagNameValueSlice{} -func newFlagNameValueSlice(flagName string) flagNameValueSlice { +func NewFlagNameValueSlice(flagName string) FlagNameValueSlice { var items []FlagNameValue - return flagNameValueSlice{ - flagName: flagName, - items: &items, + return FlagNameValueSlice{ + FlagName: flagName, + Items: &items, } } -func (f flagNameValueSlice) Empty() bool { - if f.items == nil { +func (f FlagNameValueSlice) Empty() bool { + if f.Items == nil { return true } - return len(*f.items) == 0 + return len(*f.Items) == 0 } -func (f flagNameValueSlice) AllItems() []FlagNameValue { - if f.items == nil { +func (f FlagNameValueSlice) AllItems() []FlagNameValue { + if f.Items == nil { return nil } - return *f.items + return *f.Items } -func (f flagNameValueSlice) Alias(flagName string) flagNameValueSlice { - return flagNameValueSlice{ - flagName: flagName, - items: f.items, +func (f FlagNameValueSlice) Alias(flagName string) FlagNameValueSlice { + return FlagNameValueSlice{ + FlagName: flagName, + Items: f.Items, } } -func (f flagNameValueSlice) String() string { +func (f FlagNameValueSlice) String() string { return "" } -func (f flagNameValueSlice) Set(str string) error { - *f.items = append(*f.items, FlagNameValue{ - Name: f.flagName, +func (f FlagNameValueSlice) Set(str string) error { + *f.Items = append(*f.Items, FlagNameValue{ + Name: f.FlagName, Value: str, }) return nil diff --git a/internal/command/arguments/init.go b/internal/command/arguments/init.go index fc4a702769..b6ae8f98c2 100644 --- a/internal/command/arguments/init.go +++ b/internal/command/arguments/init.go @@ -4,7 +4,6 @@ package arguments import ( - "flag" "time" "github.com/hashicorp/terraform/internal/tfdiags" @@ -58,15 +57,39 @@ type Init struct { // IgnoreRemoteVersion specifies whether to ignore remote and local Terraform versions compatibility IgnoreRemoteVersion bool + + BackendConfig FlagNameValueSlice + + Vars *Vars + + // InputEnabled is used to disable interactive input for unspecified + // variable and backend config values. Default is true. + InputEnabled bool + + TargetFlags []string + + CompactWarnings bool + + PluginPath FlagStringSlice + + Args []string } // ParseInit processes CLI arguments, returning an Init value and errors. // If errors are encountered, an Init value is still returned representing // the best effort interpretation of the arguments. -func ParseInit(args []string, cmdFlags *flag.FlagSet) (*Init, tfdiags.Diagnostics) { +func ParseInit(args []string) (*Init, tfdiags.Diagnostics) { var diags tfdiags.Diagnostics - init := &Init{} + init := &Init{ + Vars: &Vars{}, + } + init.BackendConfig = NewFlagNameValueSlice("-backend-config") + + cmdFlags := extendedFlagSet("init", nil, nil, init.Vars) + cmdFlags.Var((*FlagStringSlice)(&init.TargetFlags), "target", "resource to target") + cmdFlags.BoolVar(&init.InputEnabled, "input", true, "input") + cmdFlags.BoolVar(&init.CompactWarnings, "compact-warnings", false, "use compact warnings") cmdFlags.BoolVar(&init.Backend, "backend", true, "") cmdFlags.BoolVar(&init.Cloud, "cloud", true, "") cmdFlags.StringVar(&init.FromModule, "from-module", "", "copy the source of the given module into the directory before init") @@ -81,6 +104,8 @@ func ParseInit(args []string, cmdFlags *flag.FlagSet) (*Init, tfdiags.Diagnostic cmdFlags.BoolVar(&init.IgnoreRemoteVersion, "ignore-remote-version", false, "continue even if remote and local Terraform versions are incompatible") cmdFlags.StringVar(&init.TestsDirectory, "test-directory", "tests", "test-directory") cmdFlags.BoolVar(&init.Json, "json", false, "json") + cmdFlags.Var(&init.BackendConfig, "backend-config", "") + cmdFlags.Var(&init.PluginPath, "plugin-dir", "plugin directory") if err := cmdFlags.Parse(args); err != nil { diags = diags.Append(tfdiags.Sourceless( @@ -90,6 +115,24 @@ func ParseInit(args []string, cmdFlags *flag.FlagSet) (*Init, tfdiags.Diagnostic )) } + if init.MigrateState && init.Json { + diags = diags.Append(tfdiags.Sourceless( + tfdiags.Error, + "The -migrate-state and -json options are mutually-exclusive", + "Terraform cannot ask for interactive approval when -json is set. To use the -migrate-state option, disable the -json option.", + )) + } + + if init.MigrateState && init.Reconfigure { + diags = diags.Append(tfdiags.Sourceless( + tfdiags.Error, + "Invalid init options", + "The -migrate-state and -reconfigure options are mutually-exclusive.", + )) + } + + init.Args = cmdFlags.Args() + backendFlagSet := FlagIsSet(cmdFlags, "backend") cloudFlagSet := FlagIsSet(cmdFlags, "cloud") diff --git a/internal/command/arguments/init_test.go b/internal/command/arguments/init_test.go index 6dacbf6eff..97f7c76a34 100644 --- a/internal/command/arguments/init_test.go +++ b/internal/command/arguments/init_test.go @@ -4,8 +4,6 @@ package arguments import ( - "flag" - "io" "strings" "testing" "time" @@ -86,10 +84,7 @@ func TestParseInit_basicValid(t *testing.T) { for name, tc := range testCases { t.Run(name, func(t *testing.T) { - cmdFlags := flag.NewFlagSet("init", flag.ContinueOnError) - cmdFlags.SetOutput(io.Discard) - - got, diags := ParseInit(tc.args, cmdFlags) + got, diags := ParseInit(tc.args) if len(diags) > 0 { t.Fatalf("unexpected diags: %v", diags) } @@ -118,10 +113,7 @@ func TestParseInit_invalid(t *testing.T) { for name, tc := range testCases { t.Run(name, func(t *testing.T) { - cmdFlags := flag.NewFlagSet("init", flag.ContinueOnError) - cmdFlags.SetOutput(io.Discard) - - got, diags := ParseInit(tc.args, cmdFlags) + got, diags := ParseInit(tc.args) if len(diags) == 0 { t.Fatal("expected diags but got none") } diff --git a/internal/command/arguments/test.go b/internal/command/arguments/test.go index 51e2ab604f..e9168aff11 100644 --- a/internal/command/arguments/test.go +++ b/internal/command/arguments/test.go @@ -50,7 +50,7 @@ func ParseTest(args []string) (*Test, tfdiags.Diagnostics) { var jsonOutput bool cmdFlags := extendedFlagSet("test", nil, nil, test.Vars) - cmdFlags.Var((*flagStringSlice)(&test.Filter), "filter", "filter") + cmdFlags.Var((*FlagStringSlice)(&test.Filter), "filter", "filter") cmdFlags.StringVar(&test.TestDirectory, "test-directory", configs.DefaultTestDirectory, "test-directory") cmdFlags.BoolVar(&jsonOutput, "json", false, "json") cmdFlags.StringVar(&test.JUnitXMLFile, "junit-xml", "", "junit-xml") diff --git a/internal/command/flag_kv.go b/internal/command/flag_kv.go index cf351d2f4a..2d16ca5505 100644 --- a/internal/command/flag_kv.go +++ b/internal/command/flag_kv.go @@ -31,16 +31,3 @@ func (v *FlagStringKV) Set(raw string) error { (*v)[key] = value return nil } - -// FlagStringSlice is a flag.Value implementation for parsing targets from the -// command line, e.g. -target=aws_instance.foo -target=aws_vpc.bar -type FlagStringSlice []string - -func (v *FlagStringSlice) String() string { - return "" -} -func (v *FlagStringSlice) Set(raw string) error { - *v = append(*v, raw) - - return nil -} diff --git a/internal/command/init.go b/internal/command/init.go index 83800452ec..c2b7bb4cc7 100644 --- a/internal/command/init.go +++ b/internal/command/init.go @@ -43,24 +43,9 @@ type InitCommand struct { } func (c *InitCommand) Run(args []string) int { - var flagPluginPath FlagStringSlice - flagConfigExtra := newRawFlags("-backend-config") - var diags tfdiags.Diagnostics args = c.Meta.process(args) - cmdFlags := c.Meta.extendedFlagSet("init") - cmdFlags.Usage = func() { - diags = diags.Append(tfdiags.Sourceless( - tfdiags.Error, - "Failed to parse command-line flags", - c.Help(), - )) - } - - cmdFlags.Var(flagConfigExtra, "backend-config", "") - cmdFlags.Var(&flagPluginPath, "plugin-dir", "plugin directory") - - initArgs, initDiags := arguments.ParseInit(args, cmdFlags) + initArgs, initDiags := arguments.ParseInit(args) view := views.NewInit(initArgs.ViewType, c.View) @@ -76,26 +61,17 @@ func (c *InitCommand) Run(args []string) int { c.reconfigure = initArgs.Reconfigure c.migrateState = initArgs.MigrateState c.Meta.ignoreRemoteVersion = initArgs.IgnoreRemoteVersion - - if initArgs.MigrateState && initArgs.Json { - diags = diags.Append(tfdiags.Sourceless( - tfdiags.Error, - "The -migrate-state and -json options are mutually-exclusive", - "Terraform cannot ask for interactive approval when -json is set. To use the -migrate-state option, disable the -json option.", - )) - view.Diagnostics(diags) - return 1 - } - - if c.migrateState && c.reconfigure { - diags = diags.Append(tfdiags.Sourceless( - tfdiags.Error, - "Invalid init options", - "The -migrate-state and -reconfigure options are mutually-exclusive", - )) - view.Diagnostics(diags) - return 1 + c.Meta.input = initArgs.InputEnabled + c.Meta.targetFlags = initArgs.TargetFlags + c.Meta.compactWarnings = initArgs.CompactWarnings + + varArgs := initArgs.Vars.All() + items := make([]arguments.FlagNameValue, len(varArgs)) + for i := range varArgs { + items[i].Name = varArgs[i].Name + items[i].Value = varArgs[i].Value } + c.Meta.variableArgs = arguments.FlagNameValueSlice{Items: &items} // Copying the state only happens during backend migration, so setting // -force-copy implies -migrate-state @@ -103,13 +79,12 @@ func (c *InitCommand) Run(args []string) int { c.migrateState = true } - if len(flagPluginPath) > 0 { - c.pluginPath = flagPluginPath + if len(initArgs.PluginPath) > 0 { + c.pluginPath = initArgs.PluginPath } // Validate the arg count and get the working directory - args = cmdFlags.Args() - path, err := ModulePath(args) + path, err := ModulePath(initArgs.Args) if err != nil { diags = diags.Append(err) view.Diagnostics(diags) @@ -207,9 +182,9 @@ func (c *InitCommand) Run(args []string) int { switch { case initArgs.Cloud && rootModEarly.CloudConfig != nil: - back, backendOutput, backDiags = c.initCloud(ctx, rootModEarly, flagConfigExtra, initArgs.ViewType, view) + back, backendOutput, backDiags = c.initCloud(ctx, rootModEarly, initArgs.BackendConfig, initArgs.ViewType, view) case initArgs.Backend: - back, backendOutput, backDiags = c.initBackend(ctx, rootModEarly, flagConfigExtra, initArgs.ViewType, view) + back, backendOutput, backDiags = c.initBackend(ctx, rootModEarly, initArgs.BackendConfig, initArgs.ViewType, view) default: // load the previously-stored backend config back, backDiags = c.Meta.backendFromState(ctx) @@ -317,7 +292,7 @@ func (c *InitCommand) Run(args []string) int { } // Now that we have loaded all modules, check the module tree for missing providers. - providersOutput, providersAbort, providerDiags := c.getProviders(ctx, config, state, initArgs.Upgrade, flagPluginPath, initArgs.Lockfile, view) + providersOutput, providersAbort, providerDiags := c.getProviders(ctx, config, state, initArgs.Upgrade, initArgs.PluginPath, initArgs.Lockfile, view) diags = diags.Append(providerDiags) if providersAbort || providerDiags.HasErrors() { view.Diagnostics(diags) @@ -413,7 +388,7 @@ func (c *InitCommand) getModules(ctx context.Context, path, testsDir string, ear return true, installAbort, diags } -func (c *InitCommand) initCloud(ctx context.Context, root *configs.Module, extraConfig rawFlags, viewType arguments.ViewType, view views.Init) (be backend.Backend, output bool, diags tfdiags.Diagnostics) { +func (c *InitCommand) initCloud(ctx context.Context, root *configs.Module, extraConfig arguments.FlagNameValueSlice, viewType arguments.ViewType, view views.Init) (be backend.Backend, output bool, diags tfdiags.Diagnostics) { ctx, span := tracer.Start(ctx, "initialize Terraform Cloud") _ = ctx // prevent staticcheck from complaining to avoid a maintenence hazard of having the wrong ctx in scope here defer span.End() @@ -442,7 +417,7 @@ func (c *InitCommand) initCloud(ctx context.Context, root *configs.Module, extra return back, true, diags } -func (c *InitCommand) initBackend(ctx context.Context, root *configs.Module, extraConfig rawFlags, viewType arguments.ViewType, view views.Init) (be backend.Backend, output bool, diags tfdiags.Diagnostics) { +func (c *InitCommand) initBackend(ctx context.Context, root *configs.Module, extraConfig arguments.FlagNameValueSlice, viewType arguments.ViewType, view views.Init) (be backend.Backend, output bool, diags tfdiags.Diagnostics) { ctx, span := tracer.Start(ctx, "initialize backend") _ = ctx // prevent staticcheck from complaining to avoid a maintenence hazard of having the wrong ctx in scope here defer span.End() @@ -604,10 +579,10 @@ func (c *InitCommand) getProviders(ctx context.Context, config *configs.Config, view.Output(views.InitializingProviderPluginMessage) }, ProviderAlreadyInstalled: func(provider addrs.Provider, selectedVersion getproviders.Version) { - view.Log(views.ProviderAlreadyInstalledMessage, provider.ForDisplay(), selectedVersion) + view.LogInitMessage(views.ProviderAlreadyInstalledMessage, provider.ForDisplay(), selectedVersion) }, BuiltInProviderAvailable: func(provider addrs.Provider) { - view.Log(views.BuiltInProviderAvailableMessage, provider.ForDisplay()) + view.LogInitMessage(views.BuiltInProviderAvailableMessage, provider.ForDisplay()) }, BuiltInProviderFailure: func(provider addrs.Provider, err error) { diags = diags.Append(tfdiags.Sourceless( @@ -618,20 +593,20 @@ func (c *InitCommand) getProviders(ctx context.Context, config *configs.Config, }, QueryPackagesBegin: func(provider addrs.Provider, versionConstraints getproviders.VersionConstraints, locked bool) { if locked { - view.Log(views.ReusingPreviousVersionInfo, provider.ForDisplay()) + view.LogInitMessage(views.ReusingPreviousVersionInfo, provider.ForDisplay()) } else { if len(versionConstraints) > 0 { - view.Log(views.FindingMatchingVersionMessage, provider.ForDisplay(), getproviders.VersionConstraintsString(versionConstraints)) + view.LogInitMessage(views.FindingMatchingVersionMessage, provider.ForDisplay(), getproviders.VersionConstraintsString(versionConstraints)) } else { - view.Log(views.FindingLatestVersionMessage, provider.ForDisplay()) + view.LogInitMessage(views.FindingLatestVersionMessage, provider.ForDisplay()) } } }, LinkFromCacheBegin: func(provider addrs.Provider, version getproviders.Version, cacheRoot string) { - view.Log(views.UsingProviderFromCacheDirInfo, provider.ForDisplay(), version) + view.LogInitMessage(views.UsingProviderFromCacheDirInfo, provider.ForDisplay(), version) }, FetchPackageBegin: func(provider addrs.Provider, version getproviders.Version, location getproviders.PackageLocation) { - view.Log(views.InstallingProviderMessage, provider.ForDisplay(), version) + view.LogInitMessage(views.InstallingProviderMessage, provider.ForDisplay(), version) }, QueryPackagesFailure: func(provider addrs.Provider, err error) { switch errorTy := err.(type) { @@ -831,7 +806,7 @@ func (c *InitCommand) getProviders(ctx context.Context, config *configs.Config, keyID = view.PrepareMessage(views.KeyID, keyID) } - view.Log(views.InstalledProviderVersionInfo, provider.ForDisplay(), version, authResult, keyID) + view.LogInitMessage(views.InstalledProviderVersionInfo, provider.ForDisplay(), version, authResult, keyID) }, ProvidersLockUpdated: func(provider addrs.Provider, version getproviders.Version, localHashes []getproviders.Hash, signedHashes []getproviders.Hash, priorHashes []getproviders.Hash) { // We're going to use this opportunity to track if we have any @@ -877,7 +852,7 @@ func (c *InitCommand) getProviders(ctx context.Context, config *configs.Config, } } if thirdPartySigned { - view.Log(views.PartnerAndCommunityProvidersMessage) + view.LogInitMessage(views.PartnerAndCommunityProvidersMessage) } }, } @@ -983,7 +958,7 @@ func (c *InitCommand) getProviders(ctx context.Context, config *configs.Config, // // If the returned diagnostics contains errors then the returned body may be // incomplete or invalid. -func (c *InitCommand) backendConfigOverrideBody(flags rawFlags, schema *configschema.Block) (hcl.Body, tfdiags.Diagnostics) { +func (c *InitCommand) backendConfigOverrideBody(flags arguments.FlagNameValueSlice, schema *configschema.Block) (hcl.Body, tfdiags.Diagnostics) { items := flags.AllItems() if len(items) == 0 { return nil, nil diff --git a/internal/command/init_test.go b/internal/command/init_test.go index 62ad7f2846..feb8849793 100644 --- a/internal/command/init_test.go +++ b/internal/command/init_test.go @@ -19,6 +19,7 @@ import ( "github.com/zclconf/go-cty/cty" "github.com/hashicorp/terraform/internal/addrs" + "github.com/hashicorp/terraform/internal/command/arguments" "github.com/hashicorp/terraform/internal/configs" "github.com/hashicorp/terraform/internal/configs/configschema" "github.com/hashicorp/terraform/internal/depsfile" @@ -494,7 +495,7 @@ func TestInit_backendConfigFile(t *testing.T) { }, }, } - flagConfigExtra := newRawFlags("-backend-config") + flagConfigExtra := arguments.NewFlagNameValueSlice("-backend-config") flagConfigExtra.Set("input.config") _, diags := c.backendConfigOverrideBody(flagConfigExtra, schema) if len(diags) != 0 { diff --git a/internal/command/meta.go b/internal/command/meta.go index 0d83225169..c9dd11a71d 100644 --- a/internal/command/meta.go +++ b/internal/command/meta.go @@ -204,7 +204,7 @@ type Meta struct { backendState *workdir.BackendState // Variables for the context (private) - variableArgs rawFlags + variableArgs arguments.FlagNameValueSlice input bool // Targets for this context (private) @@ -579,11 +579,11 @@ func (m *Meta) extendedFlagSet(n string) *flag.FlagSet { f := m.defaultFlagSet(n) f.BoolVar(&m.input, "input", true, "input") - f.Var((*FlagStringSlice)(&m.targetFlags), "target", "resource to target") + f.Var((*arguments.FlagStringSlice)(&m.targetFlags), "target", "resource to target") f.BoolVar(&m.compactWarnings, "compact-warnings", false, "use compact warnings") - if m.variableArgs.items == nil { - m.variableArgs = newRawFlags("-var") + if m.variableArgs.Items == nil { + m.variableArgs = arguments.NewFlagNameValueSlice("-var") } varValues := m.variableArgs.Alias("-var") varFiles := m.variableArgs.Alias("-var-file") diff --git a/internal/command/meta_config.go b/internal/command/meta_config.go index 917d9d08a8..594560c1a1 100644 --- a/internal/command/meta_config.go +++ b/internal/command/meta_config.go @@ -411,60 +411,3 @@ func configValueFromCLI(synthFilename, rawValue string, wantType cty.Type) (cty. return val, diags } } - -// rawFlags is a flag.Value implementation that just appends raw flag -// names and values to a slice. -type rawFlags struct { - flagName string - items *[]rawFlag -} - -func newRawFlags(flagName string) rawFlags { - var items []rawFlag - return rawFlags{ - flagName: flagName, - items: &items, - } -} - -func (f rawFlags) Empty() bool { - if f.items == nil { - return true - } - return len(*f.items) == 0 -} - -func (f rawFlags) AllItems() []rawFlag { - if f.items == nil { - return nil - } - return *f.items -} - -func (f rawFlags) Alias(flagName string) rawFlags { - return rawFlags{ - flagName: flagName, - items: f.items, - } -} - -func (f rawFlags) String() string { - return "" -} - -func (f rawFlags) Set(str string) error { - *f.items = append(*f.items, rawFlag{ - Name: f.flagName, - Value: str, - }) - return nil -} - -type rawFlag struct { - Name string - Value string -} - -func (f rawFlag) String() string { - return fmt.Sprintf("%s=%q", f.Name, f.Value) -} diff --git a/internal/command/meta_vars.go b/internal/command/meta_vars.go index 18a1516785..dc06902325 100644 --- a/internal/command/meta_vars.go +++ b/internal/command/meta_vars.go @@ -150,13 +150,13 @@ func (m *Meta) collectVariableValues() (map[string]backendrun.UnparsedVariableVa // Finally we process values given explicitly on the command line, either // as individual literal settings or as additional files to read. - for _, rawFlag := range m.variableArgs.AllItems() { - switch rawFlag.Name { + for _, flagNameValue := range m.variableArgs.AllItems() { + switch flagNameValue.Name { case "-var": // Value should be in the form "name=value", where value is a // raw string whose interpretation will depend on the variable's // parsing mode. - raw := rawFlag.Value + raw := flagNameValue.Value eq := strings.Index(raw, "=") if eq == -1 { diags = diags.Append(tfdiags.Sourceless( @@ -183,13 +183,13 @@ func (m *Meta) collectVariableValues() (map[string]backendrun.UnparsedVariableVa } case "-var-file": - moreDiags := m.addVarsFromFile(rawFlag.Value, terraform.ValueFromNamedFile, ret) + moreDiags := m.addVarsFromFile(flagNameValue.Value, terraform.ValueFromNamedFile, ret) diags = diags.Append(moreDiags) default: // Should never happen; always a bug in the code that built up // the contents of m.variableArgs. - diags = diags.Append(fmt.Errorf("unsupported variable option name %q (this is a bug in Terraform)", rawFlag.Name)) + diags = diags.Append(fmt.Errorf("unsupported variable option name %q (this is a bug in Terraform)", flagNameValue.Name)) } } diff --git a/internal/command/plan.go b/internal/command/plan.go index 056dfd3b97..a477b4061a 100644 --- a/internal/command/plan.go +++ b/internal/command/plan.go @@ -200,12 +200,12 @@ func (c *PlanCommand) GatherVariables(opReq *backendrun.Operation, args *argumen // package directly, removing this shim layer. varArgs := args.All() - items := make([]rawFlag, len(varArgs)) + items := make([]arguments.FlagNameValue, len(varArgs)) for i := range varArgs { items[i].Name = varArgs[i].Name items[i].Value = varArgs[i].Value } - c.Meta.variableArgs = rawFlags{items: &items} + c.Meta.variableArgs = arguments.FlagNameValueSlice{Items: &items} opReq.Variables, diags = c.collectVariableValues() return diags diff --git a/internal/command/providers_lock.go b/internal/command/providers_lock.go index c78d30f8be..2571bbe33a 100644 --- a/internal/command/providers_lock.go +++ b/internal/command/providers_lock.go @@ -10,6 +10,7 @@ import ( "os" "github.com/hashicorp/terraform/internal/addrs" + "github.com/hashicorp/terraform/internal/command/arguments" "github.com/hashicorp/terraform/internal/depsfile" "github.com/hashicorp/terraform/internal/getproviders" "github.com/hashicorp/terraform/internal/providercache" @@ -40,7 +41,7 @@ func (c *ProvidersLockCommand) Synopsis() string { func (c *ProvidersLockCommand) Run(args []string) int { args = c.Meta.process(args) cmdFlags := c.Meta.defaultFlagSet("providers lock") - var optPlatforms FlagStringSlice + var optPlatforms arguments.FlagStringSlice var fsMirrorDir string var netMirrorURL string diff --git a/internal/command/providers_mirror.go b/internal/command/providers_mirror.go index 5b2b127f0c..a7f0d45569 100644 --- a/internal/command/providers_mirror.go +++ b/internal/command/providers_mirror.go @@ -13,6 +13,7 @@ import ( "github.com/apparentlymart/go-versions/versions" "github.com/hashicorp/go-getter" + "github.com/hashicorp/terraform/internal/command/arguments" "github.com/hashicorp/terraform/internal/getproviders" "github.com/hashicorp/terraform/internal/httpclient" "github.com/hashicorp/terraform/internal/tfdiags" @@ -33,7 +34,7 @@ func (c *ProvidersMirrorCommand) Synopsis() string { func (c *ProvidersMirrorCommand) Run(args []string) int { args = c.Meta.process(args) cmdFlags := c.Meta.defaultFlagSet("providers mirror") - var optPlatforms FlagStringSlice + var optPlatforms arguments.FlagStringSlice cmdFlags.Var(&optPlatforms, "platform", "target platform") cmdFlags.Usage = func() { c.Ui.Error(c.Help()) } if err := cmdFlags.Parse(args); err != nil { diff --git a/internal/command/refresh.go b/internal/command/refresh.go index 15160f98d0..4c43d6d580 100644 --- a/internal/command/refresh.go +++ b/internal/command/refresh.go @@ -182,12 +182,12 @@ func (c *RefreshCommand) GatherVariables(opReq *backendrun.Operation, args *argu // package directly, removing this shim layer. varArgs := args.All() - items := make([]rawFlag, len(varArgs)) + items := make([]arguments.FlagNameValue, len(varArgs)) for i := range varArgs { items[i].Name = varArgs[i].Name items[i].Value = varArgs[i].Value } - c.Meta.variableArgs = rawFlags{items: &items} + c.Meta.variableArgs = arguments.FlagNameValueSlice{Items: &items} opReq.Variables, diags = c.collectVariableValues() return diags diff --git a/internal/command/test.go b/internal/command/test.go index cbe2d0b19f..bab14fc38b 100644 --- a/internal/command/test.go +++ b/internal/command/test.go @@ -28,13 +28,13 @@ func (c *TestCommand) Help() string { helpText := ` Usage: terraform [global options] test [options] - Executes automated integration tests against the current Terraform + Executes automated integration tests against the current Terraform configuration. - Terraform will search for .tftest.hcl files within the current configuration - and testing directories. Terraform will then execute the testing run blocks - within any testing files in order, and verify conditional checks and - assertions against the created infrastructure. + Terraform will search for .tftest.hcl files within the current configuration + and testing directories. Terraform will then execute the testing run blocks + within any testing files in order, and verify conditional checks and + assertions against the created infrastructure. This command creates real infrastructure and will attempt to clean up the testing infrastructure on completion. Monitor the output carefully to ensure @@ -42,11 +42,11 @@ Usage: terraform [global options] test [options] Options: - -cloud-run=source If specified, Terraform will execute this test run - remotely using Terraform Cloud. You must specify the + -cloud-run=source If specified, Terraform will execute this test run + remotely using Terraform Cloud. You must specify the source of a module registered in a private module - registry as the argument to this flag. This allows - Terraform to associate the cloud run with the correct + registry as the argument to this flag. This allows + Terraform to associate the cloud run with the correct Terraform Cloud module and organization. -filter=testfile If specified, Terraform will only execute the test files @@ -58,7 +58,7 @@ Options: -no-color If specified, output won't contain any color. - -test-directory=path Set the Terraform test directory, defaults to "tests". + -test-directory=path Set the Terraform test directory, defaults to "tests". -var 'foo=bar' Set a value for one of the input variables in the root module of the configuration. Use this option more than @@ -147,14 +147,14 @@ func (c *TestCommand) Run(rawArgs []string) int { // Users can also specify variables via the command line, so we'll parse // all that here. - var items []rawFlag + var items []arguments.FlagNameValue for _, variable := range args.Vars.All() { - items = append(items, rawFlag{ + items = append(items, arguments.FlagNameValue{ Name: variable.Name, Value: variable.Value, }) } - c.variableArgs = rawFlags{items: &items} + c.variableArgs = arguments.FlagNameValueSlice{Items: &items} // Collect variables for "terraform test" testVariables, variableDiags := c.collectVariableValuesForTests(args.TestDirectory) diff --git a/internal/command/testdata/init-get/output.jsonlog b/internal/command/testdata/init-get/output.jsonlog index 642606d770..87f2d2534f 100644 --- a/internal/command/testdata/init-get/output.jsonlog +++ b/internal/command/testdata/init-get/output.jsonlog @@ -3,6 +3,5 @@ {"@level":"info","@message":"Initializing modules...","@module":"terraform.ui","type":"init_output"} {"@level":"info","@message":"- foo in foo","@module":"terraform.ui","type":"log"} {"@level":"info","@message":"Initializing provider plugins...","@module":"terraform.ui","type":"init_output"} -{"@level":"info","@message":"","@module":"terraform.ui","type":"init_output"} {"@level":"info","@message":"Terraform has been successfully initialized!","@module":"terraform.ui","type":"init_output"} {"@level":"info","@message":"You may now begin working with Terraform. Try running \"terraform plan\" to see\nany changes that are required for your infrastructure. All Terraform commands\nshould now work.\n\nIf you ever set or change modules or backend configuration for Terraform,\nrerun this command to reinitialize your working directory. If you forget, other\ncommands will detect it and remind you to do so if necessary.","@module":"terraform.ui","type":"init_output"} diff --git a/internal/command/views/init.go b/internal/command/views/init.go index 97076b813b..81b5b416f3 100644 --- a/internal/command/views/init.go +++ b/internal/command/views/init.go @@ -16,9 +16,10 @@ import ( // The Init view is used for the init command. type Init interface { Diagnostics(diags tfdiags.Diagnostics) - Output(messageCode string, params ...any) - Log(messageCode string, params ...any) - PrepareMessage(messageCode string, params ...any) string + Output(messageCode InitMessageCode, params ...any) + LogInitMessage(messageCode InitMessageCode, params ...any) + Log(message string, params ...any) + PrepareMessage(messageCode InitMessageCode, params ...any) string } // NewInit returns Init implementation for the given ViewType. @@ -49,19 +50,24 @@ func (v *InitHuman) Diagnostics(diags tfdiags.Diagnostics) { v.view.Diagnostics(diags) } -func (v *InitHuman) Output(messageCode string, params ...any) { +func (v *InitHuman) Output(messageCode InitMessageCode, params ...any) { v.view.streams.Println(v.PrepareMessage(messageCode, params...)) } -func (v *InitHuman) Log(messageCode string, params ...any) { +func (v *InitHuman) LogInitMessage(messageCode InitMessageCode, params ...any) { v.view.streams.Println(v.PrepareMessage(messageCode, params...)) } -func (v *InitHuman) PrepareMessage(messageCode string, params ...any) string { +// this implements log method for use by interfaces that need to log generic string messages, e.g used for logging in hook_module_install.go +func (v *InitHuman) Log(message string, params ...any) { + v.view.streams.Println(strings.TrimSpace(fmt.Sprintf(message, params...))) +} + +func (v *InitHuman) PrepareMessage(messageCode InitMessageCode, params ...any) string { message, ok := MessageRegistry[messageCode] if !ok { // display the message code as fallback if not found in the message registry - return messageCode + return string(messageCode) } if message.HumanValue == "" { @@ -84,29 +90,46 @@ func (v *InitJSON) Diagnostics(diags tfdiags.Diagnostics) { v.view.Diagnostics(diags) } -func (v *InitJSON) Output(messageCode string, params ...any) { - current_timestamp := time.Now().UTC().Format(time.RFC3339) +func (v *InitJSON) Output(messageCode InitMessageCode, params ...any) { + // don't add empty messages to json output + preppedMessage := v.PrepareMessage(messageCode, params...) + if preppedMessage == "" { + return + } + current_timestamp := time.Now().UTC().Format(time.RFC3339) json_data := map[string]string{ - "@level": "info", - "@message": v.PrepareMessage(messageCode, params...), - "@module": "terraform.ui", - "@timestamp": current_timestamp, - "type": "init_output"} + "@level": "info", + "@message": preppedMessage, + "@module": "terraform.ui", + "@timestamp": current_timestamp, + "type": "init_output", + "message_code": string(messageCode), + } init_output, _ := json.Marshal(json_data) v.view.view.streams.Println(string(init_output)) } -func (v *InitJSON) Log(messageCode string, params ...any) { - v.view.Log(v.PrepareMessage(messageCode, params...)) +func (v *InitJSON) LogInitMessage(messageCode InitMessageCode, params ...any) { + preppedMessage := v.PrepareMessage(messageCode, params...) + if preppedMessage == "" { + return + } + + v.view.Log(preppedMessage) } -func (v *InitJSON) PrepareMessage(messageCode string, params ...any) string { +// this implements log method for use by services that need to log generic string messages, e.g usage logging in hook_module_install.go +func (v *InitJSON) Log(message string, params ...any) { + v.view.Log(strings.TrimSpace(fmt.Sprintf(message, params...))) +} + +func (v *InitJSON) PrepareMessage(messageCode InitMessageCode, params ...any) string { message, ok := MessageRegistry[messageCode] if !ok { // display the message code as fallback if not found in the message registry - return messageCode + return string(messageCode) } return strings.TrimSpace(fmt.Sprintf(message.JSONValue, params...)) @@ -118,7 +141,7 @@ type InitMessage struct { JSONValue string } -var MessageRegistry map[string]InitMessage = map[string]InitMessage{ +var MessageRegistry map[InitMessageCode]InitMessage = map[InitMessageCode]InitMessage{ "copying_configuration_message": { HumanValue: "[reset][bold]Copying configuration[reset] from %q...", JSONValue: "Copying configuration from %q...", @@ -221,32 +244,34 @@ var MessageRegistry map[string]InitMessage = map[string]InitMessage{ }, } +type InitMessageCode string + const ( - CopyingConfigurationMessage string = "copying_configuration_message" - EmptyMessage string = "empty_message" - OutputInitEmptyMessage string = "output_init_empty_message" - OutputInitSuccessMessage string = "output_init_success_message" - OutputInitSuccessCloudMessage string = "output_init_success_cloud_message" - OutputInitSuccessCLIMessage string = "output_init_success_cli_message" - OutputInitSuccessCLICloudMessage string = "output_init_success_cli_cloud_message" - UpgradingModulesMessage string = "upgrading_modules_message" - InitializingTerraformCloudMessage string = "initializing_terraform_cloud_message" - InitializingModulesMessage string = "initializing_modules_message" - InitializingBackendMessage string = "initializing_backend_message" - InitializingProviderPluginMessage string = "initializing_provider_plugin_message" - LockInfo string = "lock_info" - DependenciesLockChangesInfo string = "dependencies_lock_changes_info" - ProviderAlreadyInstalledMessage string = "provider_already_installed_message" - BuiltInProviderAvailableMessage string = "built_in_provider_available_message" - ReusingPreviousVersionInfo string = "reusing_previous_version_info" - FindingMatchingVersionMessage string = "finding_matching_version_message" - FindingLatestVersionMessage string = "finding_latest_version_message" - UsingProviderFromCacheDirInfo string = "using_provider_from_cache_dir_info" - InstallingProviderMessage string = "installing_provider_message" - KeyID string = "key_id" - InstalledProviderVersionInfo string = "installed_provider_version_info" - PartnerAndCommunityProvidersMessage string = "partner_and_community_providers_message" - InitConfigError string = "init_config_error" + CopyingConfigurationMessage InitMessageCode = "copying_configuration_message" + EmptyMessage InitMessageCode = "empty_message" + OutputInitEmptyMessage InitMessageCode = "output_init_empty_message" + OutputInitSuccessMessage InitMessageCode = "output_init_success_message" + OutputInitSuccessCloudMessage InitMessageCode = "output_init_success_cloud_message" + OutputInitSuccessCLIMessage InitMessageCode = "output_init_success_cli_message" + OutputInitSuccessCLICloudMessage InitMessageCode = "output_init_success_cli_cloud_message" + UpgradingModulesMessage InitMessageCode = "upgrading_modules_message" + InitializingTerraformCloudMessage InitMessageCode = "initializing_terraform_cloud_message" + InitializingModulesMessage InitMessageCode = "initializing_modules_message" + InitializingBackendMessage InitMessageCode = "initializing_backend_message" + InitializingProviderPluginMessage InitMessageCode = "initializing_provider_plugin_message" + LockInfo InitMessageCode = "lock_info" + DependenciesLockChangesInfo InitMessageCode = "dependencies_lock_changes_info" + ProviderAlreadyInstalledMessage InitMessageCode = "provider_already_installed_message" + BuiltInProviderAvailableMessage InitMessageCode = "built_in_provider_available_message" + ReusingPreviousVersionInfo InitMessageCode = "reusing_previous_version_info" + FindingMatchingVersionMessage InitMessageCode = "finding_matching_version_message" + FindingLatestVersionMessage InitMessageCode = "finding_latest_version_message" + UsingProviderFromCacheDirInfo InitMessageCode = "using_provider_from_cache_dir_info" + InstallingProviderMessage InitMessageCode = "installing_provider_message" + KeyID InitMessageCode = "key_id" + InstalledProviderVersionInfo InitMessageCode = "installed_provider_version_info" + PartnerAndCommunityProvidersMessage InitMessageCode = "partner_and_community_providers_message" + InitConfigError InitMessageCode = "init_config_error" ) const outputInitEmpty = ` diff --git a/internal/command/views/init_test.go b/internal/command/views/init_test.go index fb84d6fad8..ae7e84c2ca 100644 --- a/internal/command/views/init_test.go +++ b/internal/command/views/init_test.go @@ -129,8 +129,7 @@ func TestNewInit_jsonViewOutput(t *testing.T) { t.Fatalf("unexpected return type %t", newInit) } - messageCode := "initializing_provider_plugin_message" - newInit.Output(messageCode) + newInit.Output(InitializingProviderPluginMessage) version := tfversion.String() want := []map[string]interface{}{ @@ -163,8 +162,7 @@ func TestNewInit_jsonViewOutput(t *testing.T) { } packageName := "hashicorp/aws" - messageCode := "finding_latest_version_message" - newInit.Output(messageCode, packageName) + newInit.Output(FindingLatestVersionMessage, packageName) version := tfversion.String() want := []map[string]interface{}{ @@ -197,8 +195,7 @@ func TestNewInit_jsonViewOutput(t *testing.T) { } var packageName, packageVersion = "hashicorp/aws", "3.0.0" - messageCode := "provider_already_installed_message" - newInit.Output(messageCode, packageName, packageVersion) + newInit.Output(ProviderAlreadyInstalledMessage, packageName, packageVersion) version := tfversion.String() want := []map[string]interface{}{ @@ -231,8 +228,7 @@ func TestNewInit_jsonViewLog(t *testing.T) { t.Fatalf("unexpected return type %t", newInit) } - messageCode := "initializing_provider_plugin_message" - newInit.Log(messageCode) + newInit.LogInitMessage(InitializingProviderPluginMessage) version := tfversion.String() want := []map[string]interface{}{ @@ -257,23 +253,6 @@ func TestNewInit_jsonViewLog(t *testing.T) { } func TestNewInit_jsonViewPrepareMessage(t *testing.T) { - t.Run("message code that does not exists", func(t *testing.T) { - streams, _ := terminal.StreamsForTesting(t) - - newInit := NewInit(arguments.ViewJSON, NewView(streams).SetRunningInAutomation(true)) - if _, ok := newInit.(*InitJSON); !ok { - t.Fatalf("unexpected return type %t", newInit) - } - - messageCode := "Terraform has been successfully initialized!" - want := messageCode - - actual := newInit.PrepareMessage(messageCode) - if !cmp.Equal(want, actual) { - t.Errorf("unexpected output: %s", cmp.Diff(want, actual)) - } - }) - t.Run("existing message code", func(t *testing.T) { streams, _ := terminal.StreamsForTesting(t) @@ -282,10 +261,9 @@ func TestNewInit_jsonViewPrepareMessage(t *testing.T) { t.Fatalf("unexpected return type %t", newInit) } - messageCode := "initializing_modules_message" want := "Initializing modules..." - actual := newInit.PrepareMessage(messageCode) + actual := newInit.PrepareMessage(InitializingModulesMessage) if !cmp.Equal(want, actual) { t.Errorf("unexpected output: %s", cmp.Diff(want, actual)) } @@ -301,8 +279,7 @@ func TestNewInit_humanViewOutput(t *testing.T) { t.Fatalf("unexpected return type %t", newInit) } - messageCode := "initializing_provider_plugin_message" - newInit.Output(messageCode) + newInit.Output(InitializingProviderPluginMessage) actual := done(t).All() expected := "Initializing provider plugins..." @@ -320,8 +297,7 @@ func TestNewInit_humanViewOutput(t *testing.T) { } packageName := "hashicorp/aws" - messageCode := "finding_latest_version_message" - newInit.Output(messageCode, packageName) + newInit.Output(FindingLatestVersionMessage, packageName) actual := done(t).All() expected := "Finding latest version of hashicorp/aws" @@ -339,8 +315,7 @@ func TestNewInit_humanViewOutput(t *testing.T) { } var packageName, packageVersion = "hashicorp/aws", "3.0.0" - messageCode := "provider_already_installed_message" - newInit.Output(messageCode, packageName, packageVersion) + newInit.Output(ProviderAlreadyInstalledMessage, packageName, packageVersion) actual := done(t).All() expected := "- Using previously-installed hashicorp/aws v3.0.0"