-
Notifications
You must be signed in to change notification settings - Fork 9.6k
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
Changes from 5 commits
8d0db63
b2a90cf
e4b4aa8
40d1628
9a3cedb
e2c5b39
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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" | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 { | ||
|
@@ -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()) | ||
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
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 For example:
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) | ||
} | ||
|
@@ -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 | ||
} | ||
|
@@ -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) | ||
} |
There was a problem hiding this comment.
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? 🤔There was a problem hiding this comment.
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 callString()
for string formatting, that is what thefmt.Stringer
interface does for you).