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.

Sign up for GitHub

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

Revert "Remove experimental flag for SkipDetailedDiff (#1893)" #1929

Merged
merged 1 commit into from
May 6, 2024
Merged

Revert "Remove experimental flag for SkipDetailedDiff (#1893)" #1929

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
19 changes: 11 additions & 8 deletions pkg/tfbridge/diff_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2071,13 +2071,14 @@ func TestListNestedAddMaxItemsOne(t *testing.T) {
}

type diffTestCase struct {
resourceSchema map[string]*schema.Schema
resourceFields map[string]*SchemaInfo
state resource.PropertyMap
inputs resource.PropertyMap
expected map[string]*pulumirpc.PropertyDiff
expectedDiffChanges pulumirpc.DiffResponse_DiffChanges
ignoreChanges []string
resourceSchema map[string]*schema.Schema
resourceFields map[string]*SchemaInfo
state resource.PropertyMap
inputs resource.PropertyMap
expected map[string]*pulumirpc.PropertyDiff
expectedDiffChanges pulumirpc.DiffResponse_DiffChanges
ignoreChanges []string
XSkipDetailedDiffForChanges bool
}

func diffTest2(t *testing.T, tc diffTestCase) {
Expand All @@ -2099,6 +2100,7 @@ func diffTest2(t *testing.T, tc diffTestCase) {
p := Provider{
tf: provider,
info: ProviderInfo{
XSkipDetailedDiffForChanges: tc.XSkipDetailedDiffForChanges,
Resources: map[string]*ResourceInfo{
"p_resource": {
Tok: "pkg:index:PResource",
Expand Down Expand Up @@ -2149,7 +2151,8 @@ func TestChangingMaxItems1FilterProperty(t *testing.T) {
},
}
diffTest2(t, diffTestCase{
resourceSchema: schema,
XSkipDetailedDiffForChanges: true,
resourceSchema: schema,
state: resource.PropertyMap{
"rules": resource.NewArrayProperty(
[]resource.PropertyValue{
Expand Down
5 changes: 5 additions & 0 deletions pkg/tfbridge/info/info.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,11 @@ type Provider struct {
// See also: pulumi/pulumi-terraform-bridge#1448
SkipValidateProviderConfigForPluginFramework bool

// Disables using detailed diff to determine diff changes and falls back on the length of TF Diff Attributes.
//
// See https://github.com/pulumi/pulumi-terraform-bridge/issues/1501
XSkipDetailedDiffForChanges bool

// Enables generation of a trimmed, runtime-only metadata file
// to help reduce resource plugin start time
//
Expand Down
2 changes: 1 addition & 1 deletion pkg/tfbridge/provider.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -960,7 +960,7 @@ func (p *Provider) Diff(ctx context.Context, req *pulumirpc.DiffRequest) (*pulum
// We will still use `detailedDiff` for diff display purposes.

// See also https://github.com/pulumi/pulumi-terraform-bridge/issues/1501.
if len(diff.Attributes()) > 0 {
if p.info.XSkipDetailedDiffForChanges && len(diff.Attributes()) > 0 {
changes = pulumirpc.DiffResponse_DIFF_SOME
// Perhaps collectionDiffs can shed some light and locate the changes to the end-user.
for path, diff := range dd.collectionDiffs {
Expand Down
31 changes: 4 additions & 27 deletions pkg/tfbridge/provider_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2156,6 +2156,9 @@ func TestSkipDetailedDiff(t *testing.T) {
Schema: &ResourceInfo{Tok: "Replace"},
},
},
info: ProviderInfo{
XSkipDetailedDiffForChanges: skipDetailedDiffForChanges,
},
}
}
t.Run("Diff", func(t *testing.T) {
Expand Down Expand Up @@ -2422,26 +2425,6 @@ func TestMaxItemOneWrongStateDiff(t *testing.T) {
},
}
t.Run("DiffListAndVal", func(t *testing.T) {
testutils.Replay(t, provider, `
{
"method": "/pulumirpc.ResourceProvider/Diff",
"request": {
"urn": "urn:pulumi:dev::teststack::NestedStrRes::exres",
"id": "0",
"olds": {
"nested_str": [""]
},
"news": {
"nested_str": ""
}
},
"response": {
"changes": "DIFF_SOME",
"hasDetailedDiff": true
}
}`)
})
t.Run("DiffNilListAndVal", func(t *testing.T) {
testutils.Replay(t, provider, `
{
"method": "/pulumirpc.ResourceProvider/Diff",
Expand All @@ -2457,13 +2440,7 @@ func TestMaxItemOneWrongStateDiff(t *testing.T) {
},
"response": {
"changes": "DIFF_SOME",
"hasDetailedDiff": true,
"detailedDiff": {
"nested_str": {
"kind": "UPDATE"
}
},
"diffs": ["nested_str"]
"hasDetailedDiff": true
}
}`)
})
Expand Down