Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

terraform modules cmd human view #36062

Merged
merged 6 commits into from
Dec 16, 2024
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 6 additions & 10 deletions internal/command/modules.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,15 +48,6 @@ func (c *ModulesCommand) Run(rawArgs []string) int {
// Set up the command's view
view := views.NewModules(c.viewType, c.View)

// TODO: Remove this check once a human readable view is supported
// for this command
if c.viewType != arguments.ViewJSON {
c.Ui.Error(
"The `terraform modules` command requires the `-json` flag.\n")
c.Ui.Error(modulesCommandHelp)
return 1
}

rootModPath, err := ModulePath([]string{})
if err != nil {
diags = diags.Append(err)
Expand Down Expand Up @@ -129,8 +120,13 @@ func (c *ModulesCommand) internalManifest() (modsdir.Manifest, tfdiags.Diagnosti
}

const modulesCommandHelp = `
Usage: terraform [global options] modules -json
Usage: terraform [global options] modules [options]

Prints out a list of all declared Terraform modules and their resolved versions
in a Terraform working directory.

Options:

-json If specified, output declared Terraform modules and
their resolved versions in a machine-readable format.
`
56 changes: 44 additions & 12 deletions internal/command/modules_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,18 @@ import (
"strings"
"testing"

"github.com/google/go-cmp/cmp"

"github.com/hashicorp/cli"
"github.com/hashicorp/terraform/internal/moduleref"
)

func TestModules_noJsonFlag(t *testing.T) {
dir := t.TempDir()
os.MkdirAll(dir, 0755)
testCopyDir(t, testFixturePath("modules-nested-dependencies"), dir)
ui := new(cli.MockUi)
view, _ := testView(t)
view, done := testView(t)
defer testChdir(t, dir)()

cmd := &ModulesCommand{
Expand All @@ -32,24 +35,51 @@ func TestModules_noJsonFlag(t *testing.T) {

args := []string{}
code := cmd.Run(args)
if code == 0 {
t.Fatal("expected an non zero exit status\n")
if code != 0 {
t.Fatalf("Got a non-zero exit code: %d\n", code)
}

output := ui.ErrorWriter.String()
if !strings.Contains(output, "The `terraform modules` command requires the `-json` flag.\n") {
t.Fatal("expected an error message about requiring -json flag.\n")
actual := done(t).All()

for _, part := range expectedOutputHuman {
if !strings.Contains(actual, part) {
t.Fatalf("unexpected output: %s\n", part)
}
}
}

if !strings.Contains(output, modulesCommandHelp) {
t.Fatal("expected the modules command help to be displayed\n")
func TestModules_noJsonFlag_noModules(t *testing.T) {
dir := t.TempDir()
os.MkdirAll(dir, 0755)
ui := new(cli.MockUi)
view, done := testView(t)
defer testChdir(t, dir)()

cmd := &ModulesCommand{
Meta: Meta{
testingOverrides: metaOverridesForProvider(testProvider()),
Ui: ui,
View: view,
},
}

args := []string{}
code := cmd.Run(args)
if code != 0 {
t.Fatalf("Got a non-zero exit code: %d\n", code)
}

actual := done(t).All()

if diff := cmp.Diff("No modules found in configuration.\n", actual); diff != "" {
t.Fatalf("unexpected output (-want +got):\n%s", diff)
}
}

func TestModules_fullCmd(t *testing.T) {
dir := t.TempDir()
os.MkdirAll(dir, 0755)
testCopyDir(t, testFixturePath("modules"), dir)
testCopyDir(t, testFixturePath("modules-nested-dependencies"), dir)

ui := new(cli.MockUi)
view, done := testView(t)
Expand All @@ -70,7 +100,7 @@ func TestModules_fullCmd(t *testing.T) {
}

output := done(t).All()
compareJSONOutput(t, output, expectedOutput)
compareJSONOutput(t, output, expectedOutputJSON)
}

func TestModules_fullCmd_unreferencedEntries(t *testing.T) {
Expand All @@ -96,7 +126,7 @@ func TestModules_fullCmd_unreferencedEntries(t *testing.T) {
t.Fatalf("Got a non-zero exit code: %d\n", code)
}
output := done(t).All()
compareJSONOutput(t, output, expectedOutput)
compareJSONOutput(t, output, expectedOutputJSON)
}

func TestModules_uninstalledModules(t *testing.T) {
Expand Down Expand Up @@ -154,4 +184,6 @@ func compareJSONOutput(t *testing.T, got string, want string) {
}
}

var expectedOutput = `{"format_version":"1.0","modules":[{"key":"child","source":"./child","version":""},{"key":"count_child","source":"./child","version":""}]}`
var expectedOutputJSON = `{"format_version":"1.0","modules":[{"key":"test","source":"./mods/test","version":""},{"key":"test2","source":"./test2","version":""},{"key":"test3","source":"./test3","version":""},{"key":"other","source":"./mods/other","version":""}]}`

var expectedOutputHuman = []string{"── \"other\"[./mods/other]", "── \"test\"[./mods/test]\n └── \"test2\"[./test2]\n └── \"test3\"[./test3]"}
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
{
"Modules": [
{
"Key": "",
"Source": "",
"Dir": "."
},
{
"Key": "other",
"Source": "./mods/other",
"Dir": "mods/other"
},
{
"Key": "test",
"Source": "./mods/test",
"Dir": "mods/test"
},
{
"Key": "test.test2",
"Source": "./test2",
"Dir": "mods/test/test2"
},
{
"Key": "test.test2.test3",
"Source": "./test3",
"Dir": "mods/test/test2/test3"
}
]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
module "test" {
source = "./mods/test"
}

module "other" {
source = "./mods/other"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
resource "test_instance" "test" {
}
output "myoutput" {
value = "bar"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
module "test2" {
source = "./test2"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
module "test3" {
source = "./test3"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
resource "test_instance" "test" {
}
output "myoutput" {
value = "bar"
}
66 changes: 65 additions & 1 deletion internal/command/views/modules.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"github.com/hashicorp/terraform/internal/command/arguments"
"github.com/hashicorp/terraform/internal/moduleref"
"github.com/hashicorp/terraform/internal/tfdiags"
"github.com/xlab/treeprint"
)

type Modules interface {
Expand Down Expand Up @@ -38,9 +39,34 @@ type ModulesHuman struct {
var _ Modules = (*ModulesHuman)(nil)

func (v *ModulesHuman) Display(manifest moduleref.Manifest) int {
if len(manifest.Records) == 0 {
v.view.streams.Println("No modules found in configuration.")
return 0
}
printRoot := treeprint.New()
populateTreeNode(printRoot, &moduleref.Record{
Children: manifest.Records,
})

v.view.streams.Println(fmt.Sprintf("\nModules declared by configuration:\n%s", printRoot.String()))
return 0
}

func populateTreeNode(tree treeprint.Tree, node *moduleref.Record) {
for _, childNode := range node.Children {
item := fmt.Sprintf("\"%s\"[%s]", childNode.Key, childNode.Source.String())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd have expected %q instead of \"%s\" but I'm more curious if there is a reason for quoting the module key in the human output at all? 🤔

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I understand the code correctly I don't think this will fail, but %q should still be used to produce correctly quoted strings. (BTW you don't need to call String() for string formatting, that is what the fmt.Stringer interface does for you).

if childNode.Version != nil {
item += fmt.Sprintf(" %s", childNode.Version)
// Avoid rendering the version constraint if an exact version is given i.e. 'version = "1.2.3"'
if childNode.VersionConstraints != nil && childNode.VersionConstraints.String() != childNode.Version.String() {
item += fmt.Sprintf(" (%s)", childNode.VersionConstraints.String())
}
Comment on lines +60 to +63
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the motivation behind this? I'd assume that version constraints will never be so long that they would worsen the readability but correct me if I'm wrong.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's more so to handle the case where the version constraint is specified to the exact version which would lead to a bit of redundancy– such as "mod-key' [mod-source] 1.0.0 (1.0.0). Felt that is was simpler to just omit the constraint in this case as it isn't clear that it is a version constraint. Though it might have made more sense to just append a "=" here to make the constraint more clear.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it's a redundancy because a constraint is a different thing to the version actually installed. Relatedly, there can be many semantically equal version constraints which are represented by the same canonical format via String().

For example:

  • 1.0.0
  • =1.0.0
  • = 1.0.0

These are all equivalent but I'm not sure they would result in the same behaviour as above, which would further confuse users.

I would just keep it simple and always render it.

}
branch := tree.AddBranch(item)
populateTreeNode(branch, childNode)
}
}

func (v *ModulesHuman) Diagnostics(diags tfdiags.Diagnostics) {
v.view.Diagnostics(diags)
}
Expand All @@ -54,7 +80,9 @@ var _ Modules = (*ModulesHuman)(nil)
func (v *ModulesJSON) Display(manifest moduleref.Manifest) int {
var bytes []byte
var err error
if bytes, err = encJson.Marshal(manifest); err != nil {

flattenedManifest := flattenManifest(manifest)
if bytes, err = encJson.Marshal(flattenedManifest); err != nil {
v.view.streams.Eprintf("error marshalling manifest: %v", err)
return 1
}
Expand All @@ -63,6 +91,42 @@ func (v *ModulesJSON) Display(manifest moduleref.Manifest) int {
return 0
}

// FlattenManifest returns the nested contents of a moduleref.Manifest in
// a flattened format with the VersionConstraints and Children attributes
// ommited for the purposes of the json format of the modules command
func flattenManifest(m moduleref.Manifest) map[string]interface{} {
var flatten func(records []*moduleref.Record)
var recordList []map[string]string
flatten = func(records []*moduleref.Record) {
for _, record := range records {
if record.Version != nil {
recordList = append(recordList, map[string]string{
"key": record.Key,
"source": record.Source.String(),
"version": record.Version.String(),
})
} else {
recordList = append(recordList, map[string]string{
"key": record.Key,
"source": record.Source.String(),
"version": "",
})
}

if len(record.Children) > 0 {
flatten(record.Children)
}
}
}

flatten(m.Records)
ret := map[string]interface{}{
"format_version": m.FormatVersion,
"modules": recordList,
}
return ret
}

func (v *ModulesJSON) Diagnostics(diags tfdiags.Diagnostics) {
v.view.Diagnostics(diags)
}
29 changes: 17 additions & 12 deletions internal/moduleref/record.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,29 +3,34 @@

package moduleref

import "github.com/hashicorp/terraform/internal/modsdir"
import (
"github.com/hashicorp/go-version"
"github.com/hashicorp/terraform/internal/addrs"
)

const FormatVersion = "1.0"

// ModuleRecord is the implementation of a module entry defined in the module
// manifest that is declared by configuration.
type Record struct {
Key string `json:"key"`
Source string `json:"source"`
Version string `json:"version"`
Key string
Source addrs.ModuleSource
Version *version.Version
VersionConstraints version.Constraints
Maed223 marked this conversation as resolved.
Show resolved Hide resolved
Children []*Record
}

// ModuleRecordManifest is the view implementation of module entries declared
// in configuration
type Manifest struct {
FormatVersion string `json:"format_version"`
Records []Record `json:"modules"`
FormatVersion string
Records []*Record
}

func (m *Manifest) addModuleEntry(entry modsdir.Record) {
m.Records = append(m.Records, Record{
Key: entry.Key,
Source: entry.SourceAddr,
Version: entry.VersionStr,
})
func (m *Manifest) addModuleEntry(entry *Record) {
m.Records = append(m.Records, entry)
}

func (r *Record) addChild(child *Record) {
r.Children = append(r.Children, child)
}
Loading
Loading