From 81ad18a16c4ba6b00288d1f9d61e19cd405c529a Mon Sep 17 00:00:00 2001 From: Dylan Ratcliffe Date: Tue, 9 Jul 2024 08:56:24 +0000 Subject: [PATCH] Use the text (known after apply) instead of removing --- cmd/changes_submit_plan.go | 56 ++++++++++++++++++--------------- cmd/changes_submit_plan_test.go | 50 +++++++++++++++-------------- 2 files changed, 57 insertions(+), 49 deletions(-) diff --git a/cmd/changes_submit_plan.go b/cmd/changes_submit_plan.go index 2a28a03b..c6dda7fb 100644 --- a/cmd/changes_submit_plan.go +++ b/cmd/changes_submit_plan.go @@ -144,7 +144,7 @@ func itemDiffFromResourceChange(resourceChange ResourceChange) (*sdp.ItemDiff, e return nil, fmt.Errorf("failed to parse after attributes: %w", err) } - err = removeKnownAfterApply(beforeAttributes, afterAttributes, resourceChange.Change.AfterUnknown) + err = handleKnownAfterApply(beforeAttributes, afterAttributes, resourceChange.Change.AfterUnknown) if err != nil { return nil, fmt.Errorf("failed to remove known after apply fields: %w", err) } @@ -204,10 +204,9 @@ func itemDiffFromResourceChange(resourceChange ResourceChange) (*sdp.ItemDiff, e return result, nil } -// Removes fields from the `before` and `after` attributes that are known after -// apply. This is because these fields are not "real" changes and we don't want -// to show them in the UI -func removeKnownAfterApply(before, after *sdp.ItemAttributes, afterUnknown json.RawMessage) error { +// Finds fields from the `before` and `after` attributes that are known after +// apply and replaces the "after" value with the string "(known after apply)" +func handleKnownAfterApply(before, after *sdp.ItemAttributes, afterUnknown json.RawMessage) error { var afterUnknownInterface interface{} err := json.Unmarshal(afterUnknown, &afterUnknownInterface) if err != nil { @@ -228,7 +227,7 @@ func removeKnownAfterApply(before, after *sdp.ItemAttributes, afterUnknown json. }, } - err = removeUnknownFields(&beforeValue, &afterValue, afterUnknownInterface) + err = insertKnownAfterApply(&beforeValue, &afterValue, afterUnknownInterface) if err != nil { return fmt.Errorf("failed to remove known after apply fields: %w", err) @@ -237,27 +236,33 @@ func removeKnownAfterApply(before, after *sdp.ItemAttributes, afterUnknown json. return nil } -// Recursively remove fields from the before and after values that are known -// after apply. This is done by comparing the afterUnknown interface to the -// before and after values and removing the fields that are true. -// -// AfterUnknown is an object value with similar structure to After, but with all -// unknown leaf values replaced with true, and all known leaf values omitted. -// This can be combined with After to reconstruct a full value after the action, -// including values which will only be known after apply. -func removeUnknownFields(before, after *structpb.Value, afterUnknown interface{}) error { +const KnownAfterApply = `(known after apply)` + +// Inserts the text "(known after apply)" in place of null values in the planned +// "after" values for fields that are known after apply. By default these are +// `null` which produces a bad diff, so we replace them with (known after apply) +// to more accurately mirror what Terraform does in the CLI +func insertKnownAfterApply(before, after *structpb.Value, afterUnknown interface{}) error { switch afterUnknown.(type) { case map[string]interface{}: for k, v := range afterUnknown.(map[string]interface{}) { if v == true { - delete(before.GetStructValue().GetFields(), k) - delete(after.GetStructValue().GetFields(), k) + if afterFields := after.GetStructValue().GetFields(); afterFields != nil { + if _, exists := afterFields[k]; exists { + afterFields[k] = &structpb.Value{ + Kind: &structpb.Value_StringValue{ + StringValue: KnownAfterApply, + }, + } + } + + } } else if v == false { // Do nothing continue } else { // Recurse into the nested fields - err := removeUnknownFields(before.GetStructValue().GetFields()[k], after.GetStructValue().GetFields()[k], v) + err := insertKnownAfterApply(before.GetStructValue().GetFields()[k], after.GetStructValue().GetFields()[k], v) if err != nil { return err } @@ -266,13 +271,14 @@ func removeUnknownFields(before, after *structpb.Value, afterUnknown interface{} case []interface{}: for i, v := range afterUnknown.([]interface{}) { if v == true { - // If this value in a slice is true, remove the corresponding - // values from the before and after - if before.GetListValue() != nil && len(before.GetListValue().GetValues()) > i { - before.GetListValue().Values = append(before.GetListValue().GetValues()[:i], before.GetListValue().GetValues()[i+1:]...) - } + // If this value in a slice is true, set the corresponding value + // in after to (know after apply) if after.GetListValue() != nil && len(after.GetListValue().GetValues()) > i { - after.GetListValue().Values = append(after.GetListValue().GetValues()[:i], after.GetListValue().GetValues()[i+1:]...) + after.GetListValue().Values[i] = &structpb.Value{ + Kind: &structpb.Value_StringValue{ + StringValue: KnownAfterApply, + }, + } } } else if v == false { // Do nothing @@ -295,7 +301,7 @@ func removeUnknownFields(before, after *structpb.Value, afterUnknown interface{} nestedAfterValue = afterListValues[i] } - err := removeUnknownFields(nestedBeforeValue, nestedAfterValue, v) + err := insertKnownAfterApply(nestedBeforeValue, nestedAfterValue, v) if err != nil { return err } diff --git a/cmd/changes_submit_plan_test.go b/cmd/changes_submit_plan_test.go index c0a4e2ed..96b78c83 100644 --- a/cmd/changes_submit_plan_test.go +++ b/cmd/changes_submit_plan_test.go @@ -3,6 +3,7 @@ package cmd import ( "context" "encoding/json" + "fmt" "testing" "github.com/overmindtech/sdp-go" @@ -147,7 +148,7 @@ func TestExtractProviderNameFromConfigKey(t *testing.T) { } } -func TestRemoveKnownAfterApply(t *testing.T) { +func TestHandleKnownAfterApply(t *testing.T) { before, err := sdp.ToAttributes(map[string]interface{}{ "string_value": "foo", "int_value": 42, @@ -233,42 +234,43 @@ func TestRemoveKnownAfterApply(t *testing.T) { ] }`) - err = removeKnownAfterApply(before, after, afterUnknown) + err = handleKnownAfterApply(before, after, afterUnknown) if err != nil { t.Fatal(err) } - if _, err := before.Get("int_value"); err == nil { - t.Errorf("Expected int_value to be removed from the before, but it's still there") + beforeJSON, err := json.MarshalIndent(before, "", " ") + if err != nil { + t.Fatal(err) } - - if _, err := before.Get("bool_value"); err == nil { - t.Errorf("Expected bool_value to be removed from the before, but it's still there") + afterJSON, err := json.MarshalIndent(after, "", " ") + if err != nil { + t.Fatal(err) } - if _, err := after.Get("int_value"); err == nil { - t.Errorf("Expected int_value to be removed from the after, but it's still there") - } + fmt.Println("BEFORE:") + fmt.Println(string(beforeJSON)) + fmt.Println("\n\nAFTER:") + fmt.Println(string(afterJSON)) - if _, err := after.Get("bool_value"); err == nil { - t.Errorf("Expected bool_value to be removed from the after, but it's still there") + if val, _ := after.Get("int_value"); val != KnownAfterApply { + t.Errorf("expected int_value to be %v, got %v", KnownAfterApply, val) } - if list, err := before.Get("list_value"); err != nil { - t.Errorf("Expected list_value to be there, but it's not: %v", err) - } else { + if val, _ := after.Get("bool_value"); val != KnownAfterApply { + t.Errorf("expected bool_value to be %v, got %v", KnownAfterApply, val) + } - if len(list.([]interface{})) != 2 { - t.Error("Expected list_value to have 2 elements") - } + i, err := after.Get("list_value") + if err != nil { + t.Error(err) } - if list, err := after.Get("list_value"); err != nil { - t.Errorf("Expected list_value to be there, but it's not: %v", err) - } else { - if len(list.([]interface{})) != 2 { - t.Error("Expected list_value to have 2 elements") + if list, ok := i.([]interface{}); ok { + if list[3] != KnownAfterApply { + t.Errorf("expected third string_value to be %v, got %v", KnownAfterApply, list[3]) } + } else { + t.Error("list_value is not a string slice") } - }