hashicorp / hashicorp/terraform-plugin-sdk

Allow providers to assign type hints to attributes

Open
#223 5 comments 1 reaction 0 assignees View on GitHub
enhancement subsystem/types upstream-protocol upstream-terraform
Dominant language
Go
Stars
485
Forks
244
Avg merge
19h 57m
Merged PRs (30d)
4

Description

## Problem Statement (current situation)

Providers can today specify primitive types, such as `TypeString` or `TypeInt` to fields which store more complex types. Different providers take different approaches to store complex types, but these are common situations:

- XML (`TypeString`)
- YAML (`TypeString`)
- JSON (`TypeString`)
- date (unix time as `TypeInt` or RFC3339/ISO8601 timestamp as `TypeString`)
- base64 representation of a string (`TypeString`)

A few problems arise with such "dummy" representation across all providers as described below.

### Validation

While providers (provider developers) are aware that certain fields are XML/YAML/JSON and should never contain arbitrary string, it is often not trivial or not obvious how to validate that. We have some prior art in that area:

- [`validation.ValidateJsonString`](https://godoc.org/github.com/hashicorp/terraform-plugin-sdk/helper/validation#ValidateJsonString)
- [`validation.ValidateRFC3339TimeString`](https://godoc.org/github.com/hashicorp/terraform-plugin-sdk/helper/validation#ValidateRFC3339TimeString)

but no consistent story.

As a result providers often rely on server-side API validation and end up making unnecessary round trips with invalid data that could have been caught at plan-time, or implement their own validation and effectively duplicate work that was already done & tested in other provider(s).

### Diffing

Terraform 0.12 applies simple heuristics to render JSON differences in a human-readable way, but this is currently [only implemented for JSON](https://github.com/hashicorp/terraform/blob/54a3e9124e1c4d4db545af625a9f5b7397337d73/command/format/diff.go#L500-L523).

As a result differences of such complex-typed fields such as YAML are not human readable as these are just rendered as single line of text, e.g.

```diff
~ value =
- "replicaCount: 3\n\nimage:\n repository: basisai/consul-esm\n tag: 0.3.3\n\nresources: {\"limits\":{\"memory\":\"256Mi\"},\"requests\":{\"cpu\":\"200m\"}}\n # We usually recommend not to specify default resources and to leave this as a conscious\n # choice for the user. This also increases chances charts run on environments with little\n # resources, such as Minikube. If you do want to specify resources, uncomment the following\n # lines, adjust them as necessary, and remove the curly braces after 'resources:'.\n # limits:\n # cpu: 100m\n # memory: 128Mi\n # requests:\n # cpu: 100m\n # memory: 128Mi\n\nenv: [{\"name\":\"HOST_IP\",\"valueFrom\":{\"fieldRef\":{\"fieldPath\":\"status.hostIP\"}}},{\"name\":\"CONSUL_HTTP_ADDR\",\"value\":\"$(HOST_IP):8500\"}]\n\ninitContainerSetSysCtl: 0\n\nconfig:\n logLevel: \"INFO\"\n\n # The service name for this agent to use when registering itself with Consul.\n serviceName: \"consul-esm\"\n\n # The service tag for this agent to use when registering itself with Consul.\n # ESM instances that share a service name/tag combination will have the work\n # of running health checks and pings for any external nodes in the catalog\n # divided evenly amongst themselves.\n serviceTag: \"\"\n\n # The directory in the Consul KV store to use for storing runtime data.\n kvPath: \"consul-esm/\"\n\n # The node metadata values used for the ESM to qualify a node in the catalog\n # as an \"external node\".\n\n externalNodeMeta: {\"external-node\":\"true\"}\n # The length of time to wait before reaping an external node due to failed\n # pings.\n nodeReconnectTimeout: \"72h\"\n\n # The interval to ping and update coordinates for external nodes that have\n # 'external-probe' set to true. By default, ESM will attempt to ping and\n # update the coordinates for all nodes it is watching every 10 seconds.\n nodeProbeInterval: \"10s\"\n\n # The address of the local Consul agent. Can also be provided through the\n # CONSUL_HTTP_ADDR environment variable.\n httpAddr: \"\"\n\n # The method to use for pinging external nodes. Defaults to \"udp\" but can\n # also be set to \"socket\" to use ICMP (which requires root privileges).\n pingType: \"udp\"\n",
+ "replicaCount: 3\n\nimage:\n repository: basisai/consul-esm\n tag: 0.3.3\n\nresources: {\"limits\":{\"memory\":\"256Mi\"},\"requests\":{\"cpu\":\"200m\"}}\n # We usually recommend not to specify default resources and to leave this as a conscious\n # choice for the user. This also increases chances charts run on environments with little\n # resources, such as Minikube. If you do want to specify resources, uncomment the following\n # lines, adjust them as necessary, and remove the curly braces after 'resources:'.\n # limits:\n # cpu: 100m\n # memory: 128Mi\n # requests:\n # cpu: 100m\n # memory: 128Mi\n\nenv: [{\"name\":\"HOST_IP\",\"valueFrom\":{\"fieldRef\":{\"fieldPath\":\"status.hostIP\"}}},{\"name\":\"CONSUL_HTTP_ADDR\",\"value\":\"$(HOST_IP):8500\"}]\n\ninitContainerSetSysCtl: false\n\nconfig:\n logLevel: \"INFO\"\n\n # The service name for this agent to use when registering itself with Consul.\n serviceName: \"consul-esm\"\n\n # The service tag for this agent to use when registering itself with Consul.\n # ESM instances that share a service name/tag combination will have the work\n # of running health checks and pings for any external nodes in the catalog\n # divided evenly amongst themselves.\n serviceTag: \"\"\n\n # The directory in the Consul KV store to use for storing runtime data.\n kvPath: \"consul-esm/\"\n\n # The node metadata values used for the ESM to qualify a node in the catalog\n # as an \"external node\".\n\n externalNodeMeta: {\"external-node\":\"true\"}\n # The length of time to wait before reaping an external node due to failed\n # pings.\n nodeReconnectTimeout: \"72h\"\n\n # The interval to ping and update coordinates for external nodes that have\n # 'external-probe' set to true. By default, ESM will attempt to ping and\n # update the coordinates for all nodes it is watching every 10 seconds.\n nodeProbeInterval: \"10s\"\n\n # The address of the local Consul agent. Can also be provided through the\n # CONSUL_HTTP_ADDR environment variable.\n httpAddr: \"\"\n\n # The method to use for pinging external nodes. Defaults to \"udp\" but can\n # also be set to \"socket\" to use ICMP (which requires root privileges).\n pingType: \"udp\"\n",
```

Additionally most (all?) providers want to suppress no-op diffs, such as whitespace changes. They use `DiffSuppressFunc` with custom implementations or [`structure.SuppressJsonDiff`](https://godoc.org/github.com/hashicorp/terraform-plugin-sdk/helper/structure#SuppressJsonDiff) to achieve this. Again - we have some prior art but no consistent story here.

### State

To ensure consistency and correct diff calculation provider needs to ensure these values are saved in their "canonical" form to the state, which is often achieved via `StateFunc` and we have some prior art in that area:

- [`structure.NormalizeJsonString`](https://godoc.org/github.com/hashicorp/terraform-plugin-sdk/helper/structure#NormalizeJsonString)

but no consistent story.

### Customisability

While many providers could be satisfied with simple "JSON" or "YAML" field, there are providers which likely require custom rules for diffing. Diffing of AWS IAM policies is one great example - AWS IAM API doesn't document any canonical format of policies and often treats things like `"values": ["single"]` and `"values": "single"` as equal and interchangeable.

## Proposal

This will likely require full RFC and some discussion/scrutiny, but rough plan is below:

0. Probe official providers' codebases to understand what common data types do they use and what formats of dates do they use
1. [SDK] Create canonical basic validation functions for XML, YAML, date (common formats), base64
2. [tf core] Refactor to make diffing logic in `command/format` more easily customisable/pluggable
3. [tf core] Implement diffing logic for XML, YAML, date and base64
4. [SDK/core] Allow schema to hint back to core how to diff given field

----

_Just for the sake of better search-ability here are some names other people have used when referring to this problem/feature:_ virtual types, diff hints, dynamic attribute diffing

----

Related: https://github.com/hashicorp/terraform/issues/21817

Related for core - function parity for working with these types and converting.

Contributor guide

Open the contributing guide

Research direction

Begin with Terraform's command/format diffing logic and the SDK helper/validation and helper/structure prior art named in the issue. Compare official provider implementations to identify common complex types and date formats, then use that research to define the RFC scope across SDK and core. Done means an agreed design and implementation plan for validation, canonicalization, and customizable diff hints.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
backend-api-design, devtools
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.