Skip to content

nil TypeMap permadiff #221

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

Closed
appilon opened this issue Oct 29, 2021 · 11 comments
Closed

nil TypeMap permadiff #221

appilon opened this issue Oct 29, 2021 · 11 comments
Labels
bug Something isn't working types Issues and pull requests about our types abstraction and implementations.

Comments

@appilon
Copy link

appilon commented Oct 29, 2021

Module version

0.4.2

Relevant provider source code

"permissions": {
				Type:        types.MapType{ElemType: types.BoolType},
				Optional:    true,
			},

Terraform Configuration Files

resource "sample_profile" "test" {
  name            = "test"
  description     = "test update"
  # permissions = {}
}

Expected Behavior

After applying the initial config, I expect a subsequent plan to be noop.

Actual Behavior

Terraform seems to think the current state has an empty set (but not nil) which causes a diff with the plan (nil).

- permissions     = {} -> null

Steps to Reproduce

References

@appilon appilon added the bug Something isn't working label Oct 29, 2021
@appilon
Copy link
Author

appilon commented Oct 29, 2021

I attempted to massage the plan to match

type fixEmptyMap struct {
	emptyDescriptions
}

func (fixEmptyMap) Modify(_ context.Context, req tfsdk.ModifyAttributePlanRequest, resp *tfsdk.ModifyAttributePlanResponse) {
	if req.AttributeState == nil {
		return
	}
	state := req.AttributeState.(types.Map)
	plan := req.AttributePlan.(types.Map)
	if len(state.Elems) == 0 && !state.Null && plan.Null {
		plan.Null = false
		plan.Elems = make(map[string]attr.Value, 0)
		resp.AttributePlan = plan
	}
}

But got the following error on plan

╷
│ Error: Provider produced invalid plan
│
│ Provider "registry.terraform.io/hashicorp/salesforce" planned an invalid value for salesforce_profile.test.permissions: planned value cty.MapValEmpty(cty.Bool) for a non-computed attribute.
│
│ This is a bug in the provider, which should be reported in the provider's own issue tracker.
╵

@bflad
Copy link
Contributor

bflad commented Oct 29, 2021

Would you be able to show how the attribute is being set into state (if it is) during Create and/or Read?

@appilon
Copy link
Author

appilon commented Oct 29, 2021

The permissions go through a bit of an expansion from the API response but the logic is the following:

        // expand permissions
	permissions := make(map[string]attr.Value, len(includePermissions))
	for _, k := range includePermissions {
		v, ok := p[k]
		trimmedKey := strings.TrimPrefix(k, "Permissions")
		if ok {
			permissions[trimmedKey] = types.Bool{Value: v.(bool)}
		} else {
			// set to unknown, maybe we should panic?
			permissions[trimmedKey] = types.Bool{Unknown: true}
		}
	}
	if len(permissions) > 0 {
		data.Permissions = types.Map{ElemType: types.BoolType, Elems: permissions}
	}

I am certain len(includePermissions) == 0 therefore, data.Permissions should just be the zero value of that type. I can confirm

State setting code is very simple within Read

	d := pMap.ToStateData(data.PermissionKeys("Permissions")...)
	// copy the ID back over
	d.Id = data.Id

	resp.Diagnostics = resp.State.Set(ctx, &d)

@appilon
Copy link
Author

appilon commented Oct 29, 2021

Confirmed, Permissions is whatever the zero value is

data := profileResourceData{
		Name:          p["Name"].(string),
		UserLicenseId: p["UserLicenseId"].(string),
		// Permissions is not touched
	}

@bflad
Copy link
Contributor

bflad commented Oct 29, 2021

This may be similar to #201. Does explicitly setting the value Null: true help? e.g. something like:

if len(permissions) > 0 {
	data.Permissions = types.Map{ElemType: types.BoolType, Elems: permissions}
} else {
	data.Permissions = types.Map{ElemType: types.BoolType, Null: true}
}

Since types.Map uses Null bool under the hood, its zero-value would be false. This may however cause a different issue where if the configuration is permissions = {} that may now permadiff.

It might be possible to first manually passthrough the Config value (e.g. retrieved via GetAttribute) into Permissions then overwrite it after that length check.

@bflad
Copy link
Contributor

bflad commented Oct 29, 2021

It may also be possible to use Permissions map[string]bool or something similar in the data type to workaround this.

@bflad bflad added the types Issues and pull requests about our types abstraction and implementations. label Nov 1, 2021
@bflad
Copy link
Contributor

bflad commented Mar 16, 2022

Hey again @appilon 👋 Did you find a resolution for this situation or does either of the above help?

@appilon
Copy link
Author

appilon commented Mar 16, 2022

@bflad I will prioritize revisiting this this week.

@project0
Copy link

I encountered exactly the same problem today.
The backend always provides a empty Map when no entries are set, no nil pointer. Now i have two options, either return the empty map, or when no entries are set set to Null:true. Both will always conflict with one of the scenarios in the terraform config.

// assuming someone generates a empty map
Label={}
// or not set at all
Label=null

Technically i dont care, both will always be empty for me as it is not computed.
Not sure how to handle this, maybe something like a custom plan may help like "NullAreEmpty"

@bflad
Copy link
Contributor

bflad commented Sep 23, 2022

We're discussing a potential path forward here over in #447 with potentially modifying the zero-values of the types value types to be null, so I'm going to close this issue out so focus efforts and discussion in one place.

@project0 if you didn't try it already, I think you may be able to do something like you propose by setting the map attribute as Computed: true and introducing the mentioned attribute plan modifier that converts a null configuration value to an empty map plan value.

@bflad bflad closed this as completed Sep 23, 2022
@github-actions
Copy link

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.
If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 24, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working types Issues and pull requests about our types abstraction and implementations.
Projects
None yet
Development

No branches or pull requests

3 participants