Skip to content
Open
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
2 changes: 2 additions & 0 deletions NEXT_CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@

### Bug Fixes

* Fixed syncing of effective fields in plugin framework implementation of share resource ([#4969](https://github.com/databricks/terraform-provider-databricks/pull/4969))

### Documentation

### Exporter
Expand Down
35 changes: 26 additions & 9 deletions internal/providers/pluginfw/products/sharing/resource_share.go
Original file line number Diff line number Diff line change
Expand Up @@ -439,16 +439,33 @@ func (effectiveFieldsActionRead) objectLevel(ctx context.Context, state *sharing
state.SyncFieldsDuringRead(ctx, plan)
}

func (r *ShareResource) syncEffectiveFields(ctx context.Context, plan, state ShareInfoExtended, mode effectiveFieldsAction) (ShareInfoExtended, diag.Diagnostics) {
// syncEffectiveFields syncs the effective fields between existingState and newState
// and returns the newState
//
// existingState: infrastructure values that are recorded in the existing terraform state.
// newState: latest infrastructure values that are returned by the CRUD API calls.
//
// HCL config is compared with this newState to determine what changes are to be made
// to the infrastructure and then the newState values are recorded in the terraform state.
// Hence we ignore the values in existingState which are not present in newState.
func (r *ShareResource) syncEffectiveFields(ctx context.Context, existingState, newState ShareInfoExtended, mode effectiveFieldsAction) (ShareInfoExtended, diag.Diagnostics) {
var d diag.Diagnostics
mode.resourceLevel(ctx, &state, plan.ShareInfo_SdkV2)
planObjects, _ := plan.GetObjects(ctx)
stateObjects, _ := state.GetObjects(ctx)
mode.resourceLevel(ctx, &newState, existingState.ShareInfo_SdkV2)
existingStateObjects, _ := existingState.GetObjects(ctx)
newStateObjects, _ := newState.GetObjects(ctx)
Comment on lines +454 to +455
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's not drop these errors here. I know this was like this before, but let's try to eliminate this tech debt.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We aren't dropping the errors: https://github.com/databricks/terraform-provider-databricks/blob/main/internal/service/sharing_tf/legacy_model.go#L4440

	if d.HasError() {
		panic(pluginfwcommon.DiagToString(d))
	}

finalObjects := []sharing_tf.SharedDataObject_SdkV2{}
for i := range stateObjects {
mode.objectLevel(ctx, &stateObjects[i], planObjects[i])
finalObjects = append(finalObjects, stateObjects[i])
for i := range newStateObjects {
// For each object in the new state, we check if it exists in the existing state
// and if it does, we sync the effective fields.
// If it does not exist, we keep the new state object as is.
for j := range existingStateObjects {
if newStateObjects[i].Name == existingStateObjects[j].Name {
mode.objectLevel(ctx, &newStateObjects[i], existingStateObjects[j])
break
}
}
finalObjects = append(finalObjects, newStateObjects[i])
Copy link
Contributor

Choose a reason for hiding this comment

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

Just to clarify, the order is the server-returned order. What happens if the order of shares in the config is changed?

}
state.SetObjects(ctx, finalObjects)
return state, d
newState.SetObjects(ctx, finalObjects)
return newState, d
}
142 changes: 142 additions & 0 deletions internal/providers/pluginfw/products/sharing/resource_share_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,142 @@
package sharing

import (
"context"
"testing"

"github.com/databricks/databricks-sdk-go/service/sharing"
"github.com/databricks/terraform-provider-databricks/internal/providers/pluginfw/converters"
"github.com/stretchr/testify/assert"
)

func TestShareSyncEffectiveFields(t *testing.T) {
shareName := "test-share-name"
ctx := context.Background()
shares := ShareResource{}

tests := []struct {
name string
planGoSDK sharing.ShareInfo
stateGoSDK sharing.ShareInfo
}{
{
name: "plan with less objects",
planGoSDK: sharing.ShareInfo{
Name: shareName,
Objects: []sharing.SharedDataObject{
{
Name: "obj-1",
Partitions: []sharing.Partition{
{Values: []sharing.PartitionValue{{Value: "part-1"}}},
},
},
{
Name: "obj-3",
Partitions: []sharing.Partition{
{Values: []sharing.PartitionValue{{Value: "part-3"}}},
},
},
},
},
stateGoSDK: sharing.ShareInfo{
Name: shareName,
Objects: []sharing.SharedDataObject{
{
Name: "obj-1",
Partitions: []sharing.Partition{
{Values: []sharing.PartitionValue{{Value: "part-1"}}},
},
},
{
Name: "obj-2",
Partitions: []sharing.Partition{
{Values: []sharing.PartitionValue{{Value: "part-2"}}},
},
},
{
Name: "obj-3",
Partitions: []sharing.Partition{
{Values: []sharing.PartitionValue{{Value: "part-3"}}},
},
},
},
},
},
{
name: "plan with more objects",
planGoSDK: sharing.ShareInfo{
Name: shareName,
Objects: []sharing.SharedDataObject{
{
Name: "obj-1",
Partitions: []sharing.Partition{
{Values: []sharing.PartitionValue{{Value: "part-1"}}},
},
},
{
Name: "obj-2",
Partitions: []sharing.Partition{
{Values: []sharing.PartitionValue{{Value: "part-2"}}},
},
},
{
Name: "obj-3",
Partitions: []sharing.Partition{
{Values: []sharing.PartitionValue{{Value: "part-3"}}},
},
},
},
},
stateGoSDK: sharing.ShareInfo{
Name: shareName,
Objects: []sharing.SharedDataObject{
{
Name: "obj-1",
Partitions: []sharing.Partition{
{Values: []sharing.PartitionValue{{Value: "part-1"}}},
},
},
{
Name: "obj-3",
Partitions: []sharing.Partition{
{Values: []sharing.PartitionValue{{Value: "part-3"}}},
},
},
},
},
},
{
name: "empty plan",
planGoSDK: sharing.ShareInfo{
Name: shareName,
Objects: []sharing.SharedDataObject{},
},
stateGoSDK: sharing.ShareInfo{
Name: shareName,
Objects: []sharing.SharedDataObject{
{
Name: "obj-1",
Partitions: []sharing.Partition{
{Values: []sharing.PartitionValue{{Value: "part-1"}}},
},
},
},
},
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
var planTFSDK ShareInfoExtended
diagnostics := converters.GoSdkToTfSdkStruct(ctx, tt.planGoSDK, &planTFSDK)
assert.False(t, diagnostics.HasError())

var stateTFSDK ShareInfoExtended
diagnostics = converters.GoSdkToTfSdkStruct(ctx, tt.stateGoSDK, &stateTFSDK)
assert.False(t, diagnostics.HasError())

_, diagnostics = shares.syncEffectiveFields(ctx, planTFSDK, stateTFSDK, effectiveFieldsActionRead{})
assert.False(t, diagnostics.HasError())
})
}
}
Loading