Skip to content

Conversation

@RaynorChavez
Copy link
Member

No description provided.

func normalizeIndexSettings(state *IndexResourceModel, newState *IndexResourceModel) {
// Handle boolean defaults
if state.Settings.TreatUrlsAndPointersAsMedia.IsNull() &&
!newState.Settings.TreatUrlsAndPointersAsMedia.ValueBool() {
Copy link
Collaborator

@orlade orlade Dec 17, 2024

Choose a reason for hiding this comment

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

If newState.Settings.TreatUrlsAndPointersAsMedia.ValueBool() falsey if the boolean value is set to false? If so, this logic would be wrong, so want to check.

return nil, false
}

func normalizeIndexSettings(state *IndexResourceModel, newState *IndexResourceModel) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

This feels very brittle to changes in schema. In the worst case, adding new fields could potentially cause existing indexes to be deleted if we don't update this function.

Feels like it should have a more systematic solution, i.e. walk the newState fields and descendant fields, and propagate nullish values.

Copy link
Member Author

@RaynorChavez RaynorChavez Dec 18, 2024

Choose a reason for hiding this comment

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

I thought so as well. So I'm not super happy with it yet.

problems:

  1. user does not define these fields so they are null
  2. api returns default values
  3. mismatch between null and the default values

Manual Solutions I've tried

  1. set incoming fields to null if they are null locally (not ideal, want to show full state to user)
  2. set undefined fields with the default value before create (not good, default value drift from cloud is a possibility)

Both don't seem appealing.

I'm looking for a way to indicate to terraform that some fields will be undefined in the user's statefile but should be expected from the api. Looking through the terraform documentation I've found this which seems to fit the function

var (
_ resource.Resource = &indicesResource{}
_ resource.ResourceWithConfigure = &indicesResource{}
//_ resource.ResourceWithModifyPlan = &indicesResource{}
Copy link
Collaborator

Choose a reason for hiding this comment

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

uncomment or remove

Copy link
Member Author

Choose a reason for hiding this comment

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

wait this isnt ready for review yet haha

Comment on lines +178 to +181
//Computed: true,
//PlanModifiers: []planmodifier.String{
// stringplanmodifier.UseStateForUnknown(),
//},
Copy link
Collaborator

Choose a reason for hiding this comment

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

uncomment or remove

Copy link
Collaborator

@orlade orlade left a comment

Choose a reason for hiding this comment

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

and CI

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants