32 Commits (25cebfbd8be2c43a25dfafdcf882e79b9ccc3d3d)

Author	SHA1	Message	Date
Martin Atkins	b402fd9d3a	command/views: Remove command-specific runningInAutomation ... We now have RunningInAutomation has a general concern in views.View, so we no longer need to specify it for each command-specific constructor separately. For this initial change I focused only on changing the exported interface of the views package and let the command-specific views go on having their own unexported fields containing a copy of the flag because it made this change less invasive and I wasn't feeling sure yet about whether we ought to have code within command-specific views directly access the internals of views.View. However, maybe we'll simplify this further in a later commit if we conclude that these copies of the flag are burdensome. The general version of this gets set directly inside the main package, which might at some future point allow us to make the command package itself unaware of this "running in automation" idea and thus reinforce that it's intended as a presentation-only thing rather than as a behavioral thing, but we'll save more invasive refactoring for another day.	5 years ago
Martin Atkins	bf396b5f1b	command/views: main View type aware if it's running in automation ... This "running in automation" idea is a best effort thing where we try to avoid printing out specific suggestions of commands to run in the main workflow when the user is running Terraform inside a wrapper script or other automation, because they probably don't want to bypass that automation. This just makes that information available to the main views.View type, so we can then make use of it in the implementation of more specialized view types that embed views.View. However, nothing is using it as of this commit. We'll use it in later commits.	5 years ago
Martin Atkins	b38f3301d1	command/views: Remove baseState argument from plan-rendering views ... In practice the current implementation isn't actually using this, and if we need access to states in future we can access them in either the plan.PriorState or plan.PrevRunState fields, depending on which stage we want a state snapshot of.	5 years ago
Alisdair McDiarmid	3a9b369b43	views/json: Fix diag crash with invalid highlight ... Some diagnostic sources (I'm looking at you, HCL) fail to set the end of the subject range. This is a bug in those code paths, but we can ensure that we generate valid JSON diagnostics by checking for it here. By doing so before the range normalization, we ensure that we generate a unit width highlight whenever possible, so that at least something useful is displayed.	5 years ago
Alisdair McDiarmid	e9c9092c40	Fix missing trailing newline	5 years ago
Alisdair McDiarmid	7b23fa7877	command/views: Add reason to JSON planned change ... Now that we have extra information about the reason for a given resource action, include that in the JSON log output for planned changes.	5 years ago
Alisdair McDiarmid	f72730a02b	cli: Add JSON logs for operations commands	5 years ago
Martin Atkins	b802237e03	plans: Track an optional extra "reason" for some planned actions ... Previously we were repeating some logic in the UI layer in order to recover relevant additional context about a change to report to a user. In order to help keep things consistent, and to have a clearer path for adding more such things in the future, here we capture this user-facing idea of an "action reason" within the plan model, and then use that directly in order to decide how to describe the change to the user. For the moment the "tainted" situation is the only one that gets a special message, matching what we had before, but we can expand on this in future in order to give better feedback about the other replace situations too. This also preemptively includes the "replacing by request" reason, which is currently not reachable but will be used in the near future as part of implementing the -replace=... plan command line option to allow forcing a particular object to be replaced. So far we don't have any special reasons for anything other than replacing, which makes sense because replacing is the only one that is in a sense a special case of another action (Update), but this could expand to other kinds of reasons in the future, such as explaining which of the few different reasons a data source read might be deferred until the apply step.	5 years ago
Martin Atkins	89f986ded6	command+backend: generalized "plan mode" ... So far we've only had "normal mode" and "destroy mode", where the latter is activated either by "terraform plan -destroy" or "terraform destroy". In preparation for introducing a third mode "refresh only" this generalizes how we handle modes so we can potentially deal with an arbitrary number of modes, although for now we only intend to have three. Mostly this is just a different implementation of the same old behavior, but there is one small user-visible difference here: the "terraform apply" command now accepts a -destroy option, mirroring the option of the same name on "terraform plan", which in turn makes "terraform destroy" effectively a shorthand for "terraform apply -destroy". This is intended to make us consistent that "terraform apply" without a plan file argument accepts all of the same plan-customization options that "terraform plan" does, which will in turn avoid us having to add a new alias of "terraform plan" for each new plan mode we might add. The -help output is changed in that vein here, although we'll wait for subsequent commit to make a similar change to the website documentation just so we can deal with the "refresh only mode" docs at the same time.	5 years ago
Martin Atkins	8f233cde4c	cli: Diagnostics can include collections with sensitive elements ... We previously had a shallow IsMarked call in compactValueStr's caller but then a more-conservative deep ContainsMarked call inside compactValueStr with a different resulting message. As well as causing an inconsistency in messages, this was also a bit confusing because it made it seem like a non-sensitive collection containing a sensitive element value was wholly sensitive, making the debug information in the diagnostic messages not trustworthy for debugging certain varieties of problem. I originally considered just removing the redundant check in compactValueStr here, but ultimately I decided to keep it as a sort of defense in depth in case a future refactoring disconnects these two checks. This should also serve as a prompt to someone making later changes to compactValueStr to think about the implications of sensitive values in there, which otherwise wouldn't be mentioned at all. Disclosing information about a collection containing sensitive values is safe here because compactValueStr only discloses information about the value's type and element keys, and neither of those can be sensitive in isolation. (Constructing a map with sensitive keys reduces to a sensitive overall map.)	5 years ago
James Bardin	406ac97965	add the address field to the view diagnostics	5 years ago
Alisdair McDiarmid	841cced6c9	Merge pull request #28138 from hashicorp/alisdair/command-views-validate ... cli: Migrate validate command to views	5 years ago
Alisdair McDiarmid	7242d9af2b	Fix typo in plan no changes output message	5 years ago
Alisdair McDiarmid	dd380d0b58	cli: Migrate validate command to views	5 years ago
Alisdair McDiarmid	d9d88b9243	cli: Add comprehensive JSON diagnostic structure ... This commit adds a comprehensive JSON format for diagnostics, which ensures that all current diagnostic output can be semantically represented in a machine-readable format. The diagnostic formatter interface remains unchanged, but it first transforms its input via the JSON format to ensure that there is only one code path for creating the diagnostic data. The JSON diagnostic renderer extracts the non-presentational logic from the format package, and returns a structure which can either be marshaled into JSON or rendered as text. The resulting text diagnostic output is unchanged for all cases covered by unit tests and my own manual testing. Included in this commit are a number of golden reference files for the marshaled JSON output of a diagnostic. This format should change rarely if at all, and these are in place to ensure that any changes to the format are intentional and considered.	5 years ago
Alisdair McDiarmid	86495f93cb	views: Fix missing source in diagnostic output ... The previous implementation of views was copying and embedding the base View struct in each individual view. While this allowed for easy access to the interface of that struct (both in the view and externally), it more importantly completely broke the ability of the diagnostic printer to output source code snippets. This is because the `configSources` field on the base view is lazily set after the config loader is initialized. In the commands ported to use views, this happens after the base View struct is copied, so we are updating the wrong copy of the struct. This commit fixes this with a simple mechanical refactor: keep a pointer to the base View struct instead, and update all of the individual views to explicitly refer to that struct to access its fields and methods. This is not a particularly satisfying solution, but I can't find anything clearly better. It might be worth exploring the alternative approach in the view for the new test command, which explicitly pulls its dependencies out of the base view, rather than retaining a full reference. Maybe there's a third way which is better still.	5 years ago
Alisdair McDiarmid	3737ee5081	views: Remove outdated comment	5 years ago
Alisdair McDiarmid	8d9a08e3a1	cli: Migrate plan to command views	5 years ago
Alisdair McDiarmid	eeb5dfbf76	Merge pull request #27865 from hashicorp/alisdair/command-views-refresh ... cli: Migrate refresh to command views	5 years ago
Martin Atkins	7f78342953	command: Experimental "terraform test" command ... This is just a prototype to gather some feedback in our ongoing research on integration testing of Terraform modules. The hope is that by having a command integrated into Terraform itself it'll be easier for interested module authors to give it a try, and also easier for us to iterate quickly based on feedback without having to coordinate across multiple codebases. Everything about this is subject to change even in future patch releases. Since it's a CLI command rather than a configuration language feature it's not using the language experiments mechanism, but generates a warning similar to the one language experiments generate in order to be clear that backward compatibility is not guaranteed.	5 years ago
Alisdair McDiarmid	50d58b592a	cli: Migrate refresh to command views	5 years ago
Alisdair McDiarmid	43028c4912	cli: Migrate apply to command views	5 years ago
Alisdair McDiarmid	68558ccd54	backend/local: Replace CLI with view instance ... This commit extracts the remaining UI logic from the local backend, and removes access to the direct CLI output. This is replaced with an instance of a `views.Operation` interface, which codifies the current requirements for the local backend to interact with the user. The exception to this at present is interactivity: approving a plan still depends on the `UIIn` field for the backend. This is out of scope for this commit and can be revisited separately, at which time the `UIOut` field can also be removed. Changes in support of this: - Some instances of direct error output have been replaced with diagnostics, most notably in the emergency state backup handler. This requires reformatting the error messages to allow the diagnostic renderer to line-wrap them; - The "in-automation" logic has moved out of the backend and into the view implementation; - The plan, apply, refresh, and import commands instantiate a view and set it on the `backend.Operation` struct, as these are the only code paths which call the `local.Operation()` method that requires it; - The show command requires the plan rendering code which is now in the views package, so there is a stub implementation of a `views.Show` interface there. Other refactoring work in support of migrating these commands to the common views code structure will come in follow-up PRs, at which point we will be able to remove the UI instances from the unit tests for those commands.	5 years ago
Alisdair McDiarmid	6375c6ce6b	Merge pull request #27787 from hashicorp/alisdair/command-views-state-locker ... clistate: Update clistate.Locker for command views	5 years ago
Alisdair McDiarmid	0aa227e0e3	cli: Fix misapplied terminal colors on run logs ... Due to calling the Colorize function with the full string instead of the format string, plan/apply logs which include resource instance keys or IDs which happen to match color formatting would be rendered incorrectly. This commit fixes this by only colorizing the known-safe format string. We also add full test coverage for the UI hook, although only one of the hooks is tested for this color bugfix due to verbosity of the test. We also add the bold coloring to the provisioner output prefix, which seems to have been an oversight.	5 years ago
Alisdair McDiarmid	1ae3d30383	Merge pull request #27760 from hashicorp/alisdair/command-views-ui-hook ... cli: Migrate Terraform UI hook to command views	5 years ago
Alisdair McDiarmid	96cf29111d	views: Expand test coverage for views.Output ... Now that the view code is separated, we can increase test coverage in unit tests. This commit moves some tests from the command package which were testing only view code, and adds more new test cases.	5 years ago
Alisdair McDiarmid	2d1976bbda	clistate: Update clistate.Locker for command views ... The clistate package includes a Locker interface which provides a simple way for the local backend to lock and unlock state, while providing feedback to the user if there is a delay while waiting for the lock. Prior to this commit, the backend was responsible for initializing the Locker, passing through direct access to the cli.Ui instance. This structure prevented commands from implementing different implementations of the state locker UI. In this commit, we: - Move the responsibility of creating the appropriate Locker to the source of the Operation; - Add the ability to set the context for a Locker via a WithContext method; - Replace the Locker's cli.Ui and Colorize members with a StateLocker view; - Implement views.StateLocker for human-readable UI; - Update the Locker interface to return detailed diagnostics instead of errors, reducing its direct interactions with UI; - Add a Timeout() method on Locker to allow the remote backend to continue to misuse the -lock-timeout flag to cancel pending runs. When an Operation is created, the StateLocker field must now be populated with an implementation of Locker. For situations where locking is disabled, this can be a no-op locker. This change has no significant effect on the operation of Terraform, with the exception of slightly different formatting of errors when state locking or unlocking fails.	5 years ago
Alisdair McDiarmid	a7b7cd29fc	cli: Migrate Terraform UI hook to command views ... Move the code which renders Terraform hook callbacks as UI into the views package, backed by a views.View instead of a cli.Ui. Update test setup accordingly. To allow commands to control this hook, we add a hooks member on the backend Operation struct. This supersedes the hooks in the Terraform context, which is not directly controlled by the command logic. This commit should not change how Terraform works, and is refactoring in preparation for more changes which move UI code out of the backend.	5 years ago
Alisdair McDiarmid	3cd086f46c	views: Use new streams helper functions	5 years ago
Alisdair McDiarmid	57879bfb71	cli: Add global view arguments parser ... Rather than modifying and relying on the existing Meta.process argument extractor, we can more clearly handle global CLI flags using a separate parser step. This allows us to explicitly configure the view in the command.	5 years ago
Alisdair McDiarmid	c5a6aa31d3	cli: Add initial command views abstraction ... Terraform supports multiple output formats for several sub-commands. The default format is user-readable text, but many sub-commands support a `-json` flag to output a machine-readable format for the result. The output command also supports a `-raw` flag for a simpler, scripting- focused machine readable format. This commit adds a "views" abstraction, intended to help ensure consistency between the various output formats. This extracts the render specific code from the command package, and moves it into a views package. Each command is expected to create an interface for its view, and one or more implementations of that interface. By doing so, we separate the concerns of generating the sub-command result from rendering the result in the specified output format. This should make it easier to ensure that all output formats will be updated together when changes occur in the result-generating phase. There are some other consequences of this restructuring: - Views now directly access the terminal streams, rather than the now-redundant cli.Ui instance; - With the reorganization of commands, parsing CLI arguments is now the responsibility of a separate "arguments" package. For now, views are added only for the output sub-command, as an example. Because this command uses code which is shared with the apply and refresh commands, those are also partially updated.	5 years ago