dont d.Set() on pointer to pointer

[wleese]

go-marathon provides us with a struct, sometimes containing pointers.

schema.resourceData's Set() will resolve a pointer once. A pointer to pointer will not properly be resolved to an underlying value.

Just use the original pointer instead.

[banno-autobot]

Can one of the admins verify this patch?

[rjeczalik]

Sorry for chiming in, but I don't see it. d.Set takes care of first indirection (**string -> *string) and eventually deeper in the call chain mapstructure.Decode takes care of another indirection (*string -> string).

Do you have a test-case so I could reproduce it myself?

• [0] func (d *ResourceData) Set(key string, value interface{}) error
• [1] func (w *MapFieldWriter) WriteField(addr []string, value interface{}) error
• [2] func (w *MapFieldWriter) setPrimitive(...) error

[wleese]

Do you have a test-case so I could reproduce it myself?

That would be:

1. Deploy something
2. Change mem amount in the marathon UI
3. See if terraform refresh picks it up

[rjeczalik]

@wleese Yeah, indeed this does not work, I was wrong. mapstructure.Decode does not handle pointers at all and all the d.Set calls fail here with non-nil errors, which are ignored.

[edit] I wonder whether adding (*ResourceData).MustSet would help spotting such cases.

[wleese]

@rjeczalik MustSet()? I cannot find any references to this function.

But even if there is a Set() variant which would catch a pointer to a pointer and complain/fail, do we even have a good reason to sent a reference to the memory address for those fields anyway?

For example

d.Set("mem", &app.Mem) // what was the purpose of this?

marathon.Application:

type Application struct {
  ID                    string              `json:"id,omitempty"`
  Cmd                   *string             `json:"cmd,omitempty"`
  Args                  *[]string           `json:"args,omitempty"`
  Constraints           *[][]string         `json:"constraints,omitempty"`
  Container             *Container          `json:"container,omitempty"`
  CPUs                  float64             `json:"cpus,omitempty"`
  Disk                  *float64            `json:"disk,omitempty"`
  Env                   *map[string]string  `json:"env,omitempty"`
  Executor              *string             `json:"executor,omitempty"`
  HealthChecks          *[]HealthCheck      `json:"healthChecks,omitempty"`
  Instances             *int                `json:"instances,omitempty"`
  Mem                   *float64            `json:"mem,omitempty"`
  Tasks                 []*Task             `json:"tasks,omitempty"`
  Ports                 []int               `json:"ports"`
  PortDefinitions       *[]PortDefinition   `json:"portDefinitions,omitempty"`
  RequirePorts          *bool               `json:"requirePorts,omitempty"`
  BackoffSeconds        *float64            `json:"backoffSeconds,omitempty"`
  BackoffFactor         *float64            `json:"backoffFactor,omitempty"`
  MaxLaunchDelaySeconds *float64            `json:"maxLaunchDelaySeconds,omitempty"`
  Deployments           []map[string]string `json:"deployments,omitempty"`
  Dependencies          []string            `json:"dependencies"`
  TasksRunning          int                 `json:"tasksRunning,omitempty"`
  TasksStaged           int                 `json:"tasksStaged,omitempty"`
  TasksHealthy          int                 `json:"tasksHealthy,omitempty"`
  TasksUnhealthy        int                 `json:"tasksUnhealthy,omitempty"`
  User                  string              `json:"user,omitempty"`
  UpgradeStrategy       *UpgradeStrategy    `json:"upgradeStrategy,omitempty"`
  Uris                  *[]string           `json:"uris,omitempty"`
  Version               string              `json:"version,omitempty"`
  VersionInfo           *VersionInfo        `json:"versionInfo,omitempty"`
  Labels                *map[string]string  `json:"labels,omitempty"`
  AcceptedResourceRoles []string            `json:"acceptedResourceRoles,omitempty"`
  LastTaskFailure       *LastTaskFailure    `json:"lastTaskFailure,omitempty"`
  Fetch                 *[]Fetch            `json:"fetch,omitempty"`
}

Considering along the line Set() takes care of any single depth indirection, we can just pass these values along directly without adding any additional indirection.