Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
21 changes: 18 additions & 3 deletions extract/parameter.go
Original file line number Diff line number Diff line change
Expand Up @@ -267,6 +267,7 @@ func optionalStringEnum[T ~string](block *terraform.Block, key string, def T, va
func requiredString(block *terraform.Block, key string) (string, *hcl.Diagnostic) {
tyAttr := block.GetAttribute(key)
tyVal := tyAttr.Value()

if tyVal.Type() != cty.String {
typeName := "<nil>"
if !tyVal.Type().Equals(cty.NilType) {
Expand All @@ -282,7 +283,6 @@ func requiredString(block *terraform.Block, key string) (string, *hcl.Diagnostic

if tyAttr.IsNotNil() {
diag.Subject = &(tyAttr.HCLAttribute().Range)
// diag.Context = &(block.HCLBlock().DefRange)
diag.Expression = tyAttr.HCLAttribute().Expr
}

Expand All @@ -297,8 +297,23 @@ func requiredString(block *terraform.Block, key string) (string, *hcl.Diagnostic
return "", diag
}

// nolint:gocritic // string type asserted
return tyVal.AsString(), nil
tyValStr, ok := hclext.AsString(tyVal)
if !ok {
// Either the val is unknown or null
diag := &hcl.Diagnostic{
Severity: hcl.DiagError,
Summary: fmt.Sprintf("Invalid %q attribute for block %s", key, block.Label()),
Detail: "Expected a string, got an unknown or null value",
EvalContext: block.Context().Inner(),
}

if tyAttr.IsNotNil() {
diag.Subject = &(tyAttr.HCLAttribute().Range)
diag.Expression = tyAttr.HCLAttribute().Expr
}
return "", diag
}
return tyValStr, nil
}

func optionalBoolean(block *terraform.Block, key string) bool {
Expand Down
10 changes: 5 additions & 5 deletions hclext/merge.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,12 @@ func MergeObjects(a, b cty.Value) cty.Value {
output[key] = val
}
b.ForEachElement(func(key, val cty.Value) (stop bool) {
// TODO: Should this error be captured?
if key.Type() != cty.String {
return true
}
//nolint:gocritic // string type asserted above
k := key.AsString()
k, ok := AsString(key)
if !ok {
// TODO: Should this error be captured?
Copy link
Member

Choose a reason for hiding this comment

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

Probably no harm to drop a warn diag here, if possible?

Copy link
Member Author

Choose a reason for hiding this comment

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

Unfortunately I cannot really. This is all for merging context values. Which happens in a hook that cannot raise errors.

The context is supposed to store variable (reference) values. So this failing just means a reference/var is skipped. Which leaves it's value as unknown when the context tries to load the value.

Which is the correct behavior at a high level. In reality though, I do not think this should happen. Maybe with some complex structs/types? I need to test more.

return stop
}
Comment on lines -14 to +19
Copy link
Member Author

Choose a reason for hiding this comment

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

Invalid values were skipped before, so behavior is unchanged. Just no potential for a panic.

old := output[k]
if old.IsKnown() && isNotEmptyObject(old) && isNotEmptyObject(val) {
output[k] = MergeObjects(old, val)
Expand Down
7 changes: 6 additions & 1 deletion hclext/references.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,12 @@ func CreateDotReferenceFromTraversal(traversals ...hcl.Traversal) string {
switch {
case part.Key.Type().Equals(cty.String):
//nolint:gocritic // string type asserted above
refParts = append(refParts, fmt.Sprintf("[%s]", part.Key.AsString()))
stringName, ok := AsString(part.Key)
if !ok {
// Nothing we can do, just put a placeholder
stringName = "??"
}
refParts = append(refParts, fmt.Sprintf("[%s]", stringName))
Comment on lines -71 to +76
Copy link
Member Author

Choose a reason for hiding this comment

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

CreateDotReferenceFromTraversal converts a reference to a string for error messages and logs. So the ?? is ok if that value is really unknown.

case part.Key.Type().Equals(cty.Number):
idx, _ := part.Key.AsBigFloat().Int64()
refParts = append(refParts, fmt.Sprintf("[%d]", idx))
Expand Down
8 changes: 7 additions & 1 deletion init.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import (
"github.com/zclconf/go-cty/cty"
"github.com/zclconf/go-cty/cty/function"
"golang.org/x/xerrors"

"github.com/coder/preview/hclext"
)

// init intends to override some of the default functions afforded by terraform.
Expand All @@ -27,7 +29,11 @@ func init() {
Impl: func(args []cty.Value, retType cty.Type) (cty.Value, error) {
// This code is taken directly from https://github.com/mitchellh/go-homedir/blob/af06845cf3004701891bf4fdb884bfe4920b3727/homedir.go#L58
// The only change is that instead of expanding the path, we return an error
path := args[0].AsString()
path, ok := hclext.AsString(args[0])
if !ok {
return cty.NilVal, xerrors.Errorf("invalid path argument")
}
Comment on lines +32 to +35
Copy link
Member Author

Choose a reason for hiding this comment

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

Being safe


if len(path) == 0 {
return cty.StringVal(path), nil
}
Expand Down
6 changes: 4 additions & 2 deletions paramhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,10 @@ func parameterContextsEvalHook(input Input) func(ctx *tfcontext.Context, blocks
continue
}

//nolint:gocritic // string type asserted
name := nameVal.AsString()
name, ok := hclext.AsString(nameVal)
if !ok {
continue
}
Comment on lines +42 to +45
Copy link
Member Author

Choose a reason for hiding this comment

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

The code above does the same thing as this.

var value cty.Value
pv, ok := input.RichParameterValue(name)
if ok {
Expand Down
70 changes: 0 additions & 70 deletions types/primitive.go

This file was deleted.

Loading