|
| 1 | +## Context |
| 2 | + |
| 3 | +The `tencentcloud_dts_sync_job` resource defines its `tags` field as `schema.TypeList`. Tags are sent to the DTS cloud API via `CreateSyncJob` (as `[]*dts.TagItem`) and read back via `DescribeSyncJobs` (as `[]*dts.TagItem`). The cloud API does not guarantee tag ordering on read — the same set of tags may come back in a different order than what was sent. With `TypeList`, Terraform compares element-by-element by index, so any reordering produces a non-empty diff on every refresh. This is a well-known pattern in the codebase: other resources (e.g. MongoDB `availability_zone_list`, DTS sync_config sub-fields) already use `TypeSet` for unordered collections. |
| 4 | + |
| 5 | +Constraints: |
| 6 | +- The `tags` field is `ForceNew` + `Computed` + `Optional`, so it is set only at create time and refreshed on read. There is no update path that needs changing. |
| 7 | +- The cloud API request/response types (`dts.TagItem`) are unchanged — only the Terraform-side collection type changes. |
| 8 | +- HCL block-list syntax is identical for `TypeList` and `TypeSet`, so user `.tf` files require no modification. |
| 9 | + |
| 10 | +## Goals / Non-Goals |
| 11 | + |
| 12 | +**Goals:** |
| 13 | +- Eliminate spurious diffs on the `tags` field caused by unordered cloud API responses. |
| 14 | +- Keep the nested element schema (`tag_key`, `tag_value`) unchanged. |
| 15 | +- Keep the HCL configuration syntax backwards-compatible (no change to user `.tf` files). |
| 16 | + |
| 17 | +**Non-Goals:** |
| 18 | +- Do not change the cloud API calls (`CreateSyncJob`, `DescribeSyncJobs`). |
| 19 | +- Do not change the `dts.TagItem` SDK struct usage. |
| 20 | +- Do not modify other fields of `tencentcloud_dts_sync_job`. |
| 21 | +- Do not change the `tags` handling in the data source `tencentcloud_dts_sync_jobs` (it uses a different schema and is not affected by the reported issue). |
| 22 | + |
| 23 | +## Decisions |
| 24 | + |
| 25 | +### D1: TypeSet with default HashResource |
| 26 | + |
| 27 | +**Decision**: Change `Type: schema.TypeList` to `Type: schema.TypeSet`, keeping `Elem: &schema.Resource{...}`. Use the default `Set` hash function (`schema.HashResource`), which hashes the full resource schema — i.e. the combination of `tag_key` + `tag_value`. |
| 28 | + |
| 29 | +**Rationale**: |
| 30 | +- `tag_key` + `tag_value` together form a unique identity for a tag. The default `HashResource` hashes both fields, so duplicate tags are correctly de-duplicated and order is irrelevant. |
| 31 | +- An alternative is a custom `schema.HashResource` with a custom function, but that adds complexity for no benefit since the default already covers both fields. |
| 32 | + |
| 33 | +**Alternatives considered**: |
| 34 | +- Custom hash function keyed only on `tag_key`: rejected because two tags with the same key but different values would collide; the cloud API allows the same key with different values in some contexts, so hashing both fields is safer. |
| 35 | + |
| 36 | +### D2: Create handler reads Set via `(*schema.Set).List()` |
| 37 | + |
| 38 | +**Decision**: In `resourceTencentCloudDtsSyncJobCreate`, change `v.([]interface{})` to `v.(*schema.Set).List()` when iterating the `tags` field. The iteration body that builds `dts.TagItem` is unchanged. |
| 39 | + |
| 40 | +**Rationale**: `d.GetOk("tags")` returns `*schema.Set` for a TypeSet field. Calling `.List()` converts it to `[]interface{}`, which is the same type the existing loop already expects. |
| 41 | + |
| 42 | +### D3: Read handler stays as `d.Set("tags", tagsList)` with `[]interface{}` |
| 43 | + |
| 44 | +**Decision**: The Read handler already builds `tagsList` as `[]interface{}` of `map[string]interface{}` and calls `d.Set("tags", tagsList)`. This works correctly with `TypeSet` — Terraform SDK converts the slice into the Set using the same hash function. No change needed in the read body besides keeping the existing logic. |
| 45 | + |
| 46 | +**Rationale**: `schema.ResourceData.Set` accepts `[]interface{}` for both TypeList and TypeSet. For TypeSet it hashes each element with the configured hash function. The existing code already produces the right shape. |
| 47 | + |
| 48 | +### D4: Test assertions updated to Set semantics |
| 49 | + |
| 50 | +**Decision**: In `resource_tc_dts_sync_job_test.go`, replace the `resource.TestCheckResourceAttrSet(..., "tags.#")` check with `resource.TestCheckTypeSetElemNestedAttrs` to verify the specific tag key/value pair regardless of order. The HCL config stays in block syntax (valid for Set). |
| 51 | + |
| 52 | +## Risks / Trade-offs |
| 53 | + |
| 54 | +- **[R1] State format change** → On first `terraform plan`/`apply` after upgrade, the state representation of `tags` changes from an ordered list to a set. Users see a one-time refresh diff; no actual resource is modified. This is the standard SDKv2 List→Set migration, well-tested in this codebase (MongoDB precedent). Mitigation: document the one-time refresh in the changelog. |
| 55 | +- **[R2] TypeSet hash collisions** → If two tags have identical `tag_key` + `tag_value`, they collapse into one Set element. The cloud API also treats identical tags as duplicates, so this matches API semantics. No risk. |
| 56 | +- **[R3] Import compatibility** → `tags` is `ForceNew`+`Computed`, not a read-from-id field, so import (`schema.ImportStatePassthrough`) is unaffected. |
| 57 | + |
| 58 | +## Migration Plan |
| 59 | + |
| 60 | +1. Apply the schema + Create handler change in `resource_tc_dts_sync_job.go`. |
| 61 | +2. Update test file assertions. |
| 62 | +3. Run `gofmt` on modified `.go` files (finalize step). |
| 63 | +4. Regenerate docs via `make doc` (finalize step). |
| 64 | +5. Rollback: `git revert` the change; the state automatically reverts to List representation on next refresh with the old provider version. |
0 commit comments