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

logging: add trace statements when changes are ignored #36259

Merged
merged 3 commits into from
Jan 3, 2025
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
3 changes: 1 addition & 2 deletions internal/backend/local/backend_apply.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ import (
"github.com/hashicorp/terraform/internal/addrs"
"github.com/hashicorp/terraform/internal/backend/backendrun"
"github.com/hashicorp/terraform/internal/command/views"
viewsjson "github.com/hashicorp/terraform/internal/command/views/json"
"github.com/hashicorp/terraform/internal/configs"
"github.com/hashicorp/terraform/internal/logging"
"github.com/hashicorp/terraform/internal/plans"
Expand Down Expand Up @@ -348,7 +347,7 @@ func (b *Local) opApply(
diags = diags.Append(&hcl.Diagnostic{
Severity: hcl.DiagError,
Summary: "Can't change variable when applying a saved plan",
Detail: fmt.Sprintf("The variable %s cannot be set using the -var and -var-file options when applying a saved plan file, because a saved plan includes the variable values that were set when it was created. The saved plan specifies %s as the value whereas during apply the value %s was %s. To declare an ephemeral variable which is not saved in the plan file, use ephemeral = true.", varName, viewsjson.CompactValueStr(parsedVar.Value), viewsjson.CompactValueStr(plannedVar), parsedVar.SourceType.DiagnosticLabel()),
Detail: fmt.Sprintf("The variable %s cannot be set using the -var and -var-file options when applying a saved plan file, because a saved plan includes the variable values that were set when it was created. The saved plan specifies %s as the value whereas during apply the value %s was %s. To declare an ephemeral variable which is not saved in the plan file, use ephemeral = true.", varName, tfdiags.CompactValueStr(parsedVar.Value), tfdiags.CompactValueStr(plannedVar), parsedVar.SourceType.DiagnosticLabel()),
Subject: rng,
})
}
Expand Down
134 changes: 4 additions & 130 deletions internal/command/views/json/diagnostic.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ package json

import (
"bufio"
"bytes"
"fmt"
"sort"
"strings"
Expand All @@ -14,9 +13,10 @@ import (
"github.com/hashicorp/hcl/v2/hcled"
"github.com/hashicorp/hcl/v2/hclparse"
"github.com/hashicorp/hcl/v2/hclsyntax"
"github.com/zclconf/go-cty/cty"

"github.com/hashicorp/terraform/internal/lang/marks"
"github.com/hashicorp/terraform/internal/tfdiags"
"github.com/zclconf/go-cty/cty"
)

// These severities map to the tfdiags.Severity values, plus an explicit
Expand Down Expand Up @@ -289,7 +289,7 @@ func NewDiagnostic(diag tfdiags.Diagnostic, sources map[string][]byte) *Diagnost
continue
}

traversalStr := traversalStr(traversal)
traversalStr := tfdiags.TraversalStr(traversal)
if _, exists := seen[traversalStr]; exists {
continue Traversals // don't show duplicates when the same variable is referenced multiple times
}
Expand Down Expand Up @@ -382,7 +382,7 @@ func NewDiagnostic(diag tfdiags.Diagnostic, sources map[string][]byte) *Diagnost
value.Statement = "will be known only after apply"
}
default:
value.Statement = fmt.Sprintf("is %s", compactValueStr(val))
value.Statement = fmt.Sprintf("is %s", tfdiags.CompactValueStr(val))
}
values = append(values, value)
seen[traversalStr] = struct{}{}
Expand Down Expand Up @@ -437,129 +437,3 @@ func parseRange(src []byte, rng hcl.Range) (*hcl.File, int) {

return file, offset
}

// CompactValueStr produces a compact, single-line summary of a given value
// that is suitable for display in the UI.
//
// For primitives it returns a full representation, while for more complex
// types it instead summarizes the type, size, etc to produce something
// that is hopefully still somewhat useful but not as verbose as a rendering
// of the entire data structure.
func CompactValueStr(val cty.Value) string {
return compactValueStr(val)
}

func compactValueStr(val cty.Value) string {
// This is a specialized subset of value rendering tailored to producing
// helpful but concise messages in diagnostics. It is not comprehensive
// nor intended to be used for other purposes.

val, valMarks := val.Unmark()
for mark := range valMarks {
switch mark {
case marks.Sensitive:
// We check this in here just to make sure, but note that the caller
// of compactValueStr ought to have already checked this and skipped
// calling into compactValueStr anyway, so this shouldn't actually
// be reachable.
return "(sensitive value)"
case marks.Ephemeral:
// A non-sensitive ephemeral value is fine to show in the UI. Values
// that are both ephemeral and sensitive should have both markings
// and should therefore get caught by the marks.Sensitive case
// above.
continue
default:
// We don't know about any other marks, so we'll be conservative.
// This shouldn't actuallyr eachable since the caller should've
// checked this and skipped calling compactValueStr anyway.
return "value with unrecognized marks (this is a bug in Terraform)"
}
}

// WARNING: We've only checked that the value isn't sensitive _shallowly_
// here, and so we must never show any element values from complex types
// in here. However, it's fine to show map keys and attribute names because
// those are never sensitive in isolation: the entire value would be
// sensitive in that case.

ty := val.Type()
switch {
case val.IsNull():
return "null"
case !val.IsKnown():
// Should never happen here because we should filter before we get
// in here, but we'll do something reasonable rather than panic.
return "(not yet known)"
case ty == cty.Bool:
if val.True() {
return "true"
}
return "false"
case ty == cty.Number:
bf := val.AsBigFloat()
return bf.Text('g', 10)
case ty == cty.String:
// Go string syntax is not exactly the same as HCL native string syntax,
// but we'll accept the minor edge-cases where this is different here
// for now, just to get something reasonable here.
return fmt.Sprintf("%q", val.AsString())
case ty.IsCollectionType() || ty.IsTupleType():
l := val.LengthInt()
switch l {
case 0:
return "empty " + ty.FriendlyName()
case 1:
return ty.FriendlyName() + " with 1 element"
default:
return fmt.Sprintf("%s with %d elements", ty.FriendlyName(), l)
}
case ty.IsObjectType():
atys := ty.AttributeTypes()
l := len(atys)
switch l {
case 0:
return "object with no attributes"
case 1:
var name string
for k := range atys {
name = k
}
return fmt.Sprintf("object with 1 attribute %q", name)
default:
return fmt.Sprintf("object with %d attributes", l)
}
default:
return ty.FriendlyName()
}
}

// traversalStr produces a representation of an HCL traversal that is compact,
// resembles HCL native syntax, and is suitable for display in the UI.
func traversalStr(traversal hcl.Traversal) string {
// This is a specialized subset of traversal rendering tailored to
// producing helpful contextual messages in diagnostics. It is not
// comprehensive nor intended to be used for other purposes.

var buf bytes.Buffer
for _, step := range traversal {
switch tStep := step.(type) {
case hcl.TraverseRoot:
buf.WriteString(tStep.Name)
case hcl.TraverseAttr:
buf.WriteByte('.')
buf.WriteString(tStep.Name)
case hcl.TraverseIndex:
buf.WriteByte('[')
if keyTy := tStep.Key.Type(); keyTy.IsPrimitiveType() {
buf.WriteString(compactValueStr(tStep.Key))
} else {
// We'll just use a placeholder for more complex values,
// since otherwise our result could grow ridiculously long.
buf.WriteString("...")
}
buf.WriteByte(']')
}
}
return buf.String()
}
2 changes: 1 addition & 1 deletion internal/terraform/eval_context_builtin.go
Original file line number Diff line number Diff line change
Expand Up @@ -407,7 +407,7 @@ func (ctx *BuiltinEvalContext) EvaluateReplaceTriggeredBy(expr hcl.Expression, r
return nil, false, diags
}

path := traversalToPath(ref.Remaining)
path, _ := traversalToPath(ref.Remaining)
attrBefore, _ := path.Apply(change.Before)
attrAfter, _ := path.Apply(change.After)

Expand Down
28 changes: 22 additions & 6 deletions internal/terraform/node_resource_abstract_instance.go
Original file line number Diff line number Diff line change
Expand Up @@ -1252,14 +1252,16 @@ func (n *NodeAbstractResource) processIgnoreChanges(prior, config cty.Value, sch
return config, nil
}

ignoreChanges := traversalsToPaths(n.Config.Managed.IgnoreChanges)
ignoreChanges, keys := traversalsToPaths(n.Config.Managed.IgnoreChanges)
ignoreAll := n.Config.Managed.IgnoreAllChanges

if len(ignoreChanges) == 0 && !ignoreAll {
return config, nil
}

if ignoreAll {
log.Printf("[TRACE] processIgnoreChanges: Ignoring all changes for %s", n.Addr)

// Legacy providers need up to clean up their invalid plans and ensure
// no changes are passed though, but that also means making an invalid
// config with computed values. In that case we just don't supply a
Expand All @@ -1282,6 +1284,7 @@ func (n *NodeAbstractResource) processIgnoreChanges(prior, config cty.Value, sch

return ret, nil
}
log.Printf("[TRACE] processIgnoreChanges: Ignoring changes for %s at [%s]", n.Addr, strings.Join(keys, ", "))

if prior.IsNull() || config.IsNull() {
// Ignore changes doesn't apply when we're creating for the first time.
Expand All @@ -1296,36 +1299,49 @@ func (n *NodeAbstractResource) processIgnoreChanges(prior, config cty.Value, sch

// Convert the hcl.Traversal values we get form the configuration to the
// cty.Path values we need to operate on the cty.Values
func traversalsToPaths(traversals []hcl.Traversal) []cty.Path {
func traversalsToPaths(traversals []hcl.Traversal) ([]cty.Path, []string) {
paths := make([]cty.Path, len(traversals))
keys := make([]string, len(traversals))
for i, traversal := range traversals {
path := traversalToPath(traversal)
path, key := traversalToPath(traversal)
paths[i] = path
keys[i] = key
}
return paths
return paths, keys
}

func traversalToPath(traversal hcl.Traversal) cty.Path {
func traversalToPath(traversal hcl.Traversal) (cty.Path, string) {
path := make(cty.Path, len(traversal))
var key strings.Builder
for si, step := range traversal {
switch ts := step.(type) {
case hcl.TraverseRoot:
path[si] = cty.GetAttrStep{
Name: ts.Name,
}
key.WriteString(ts.Name)
case hcl.TraverseAttr:
path[si] = cty.GetAttrStep{
Name: ts.Name,
}
key.WriteString(".")
key.WriteString(ts.Name)
case hcl.TraverseIndex:
path[si] = cty.IndexStep{
Key: ts.Key,
}
if ts.Key.Type().IsPrimitiveType() {
key.WriteString("[")
key.WriteString(tfdiags.CompactValueStr(ts.Key))
key.WriteString("]")
} else {
key.WriteString("[...]")
}
default:
panic(fmt.Sprintf("unsupported traversal step %#v", step))
}
}
return path
return path, key.String()
}

func processIgnoreChangesIndividual(prior, config cty.Value, ignoreChangesPath []cty.Path) (cty.Value, tfdiags.Diagnostics) {
Expand Down
2 changes: 1 addition & 1 deletion internal/terraform/node_resource_validate.go
Original file line number Diff line number Diff line change
Expand Up @@ -366,7 +366,7 @@ func (n *NodeValidatableResource) validateResource(ctx EvalContext) tfdiags.Diag
// use that to check whether the Attribute is Computed and
// non-Optional.
if !diags.HasErrors() {
path := traversalToPath(traversal)
path, _ := traversalToPath(traversal)

attrSchema := schema.AttributeByPath(path)

Expand Down
3 changes: 2 additions & 1 deletion internal/terraform/reduce_plan_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -433,7 +433,8 @@ func TestProcessIgnoreChangesIndividual(t *testing.T) {
ignore[i] = trav
}

ret, diags := processIgnoreChangesIndividual(test.Old, test.New, traversalsToPaths(ignore))
paths, _ := traversalsToPaths(ignore)
ret, diags := processIgnoreChangesIndividual(test.Old, test.New, paths)
if diags.HasErrors() {
t.Fatal(diags.Err())
}
Expand Down
Loading
Loading