|
| 1 | +## Context |
| 2 | + |
| 3 | +See proposal.md — Why. The three serverless project resources are Terraform Plugin Framework resources. CRUD goes through the generated serverless client in `ec/internal/gen/serverless/`, not `cloud-sdk-go`. Create, patch, and read bodies are built by hand in `ec/ecresource/projectresource` (`elasticsearch.go`, `observability.go`, `security.go`) from the generated models. |
| 4 | + |
| 5 | +The raw project API on `elastic/serverless-api-specification` `main` already defines `monitoring.logging.audit` (create, read, and patch, including JSON null to reset a category). Those schemas are marked `x-exclude-from-documentation: true`. The file this provider vendors, `specifications/generated/production/public-user-serverless-api-dereferenced.yml`, strips them. The committed spec (`serverless-project-api.source` ref `059c09ac`) has no `monitoring` object. |
| 6 | + |
| 7 | +`client-config.yaml` uses default oapi-codegen. Nullable properties become `*T` with `json:",omitempty"`. A nil pointer is omitted, so it cannot encode `null`. `OptionalTrafficFilters` is cleared today by a non-nil empty slice (`expandTrafficFilterIdsForPatch`), which marshals as `[]`. Linked-project nulls work only because they are nil map values, which `omitempty` does not strip. |
| 8 | + |
| 9 | +`modify_spec.sh` already rewrites generated project schemas. Optional attributes that appear on both create and read come out `computed_optional`. Attributes required on create come out `required`. Read-only attributes come out `computed`. `traffic_filters` is deleted and replaced by the string set `traffic_filter_ids`. `linked.statuses` is a computed attribute beside the practitioner-controlled `projects` map. |
| 10 | + |
| 11 | +## Goals / Non-Goals |
| 12 | + |
| 13 | +**Goals:** |
| 14 | + |
| 15 | +- One optional `audit` nested attribute, mapped the same way on all three project resources. |
| 16 | +- Generated models once the public bundle contains the fields. Hand-written code maps them. Generated files are not edited by hand. |
| 17 | +- Unit tests assert marshalled JSON for `audit` and `ignore_filters` null clears. Acceptance stays a human/Buildkite step. |
| 18 | + |
| 19 | +**Non-Goals:** |
| 20 | + |
| 21 | +- Vendoring the raw user spec, or deleting `x-exclude-from-documentation` in a local copy of the public bundle. |
| 22 | +- Turning on oapi-codegen `nullable-type` in `client-config.yaml`. That would change `OptionalLinkConfiguration`, `OptionalTrafficFilters`, `OptionalElasticsearchSearchLake`, and their callers. |
| 23 | +- A shared Terraform resource type. The three resources stay separate. |
| 24 | +- Modeling `logging` categories other than `audit`. |
| 25 | + |
| 26 | +## Decisions |
| 27 | + |
| 28 | +- **Stop until the public bundle contains the fields.** Bump `serverless-project-api.source` `ref` only to a commit whose `public-user-serverless-api-dereferenced.yml` includes `monitoring.logging.audit`, then run `scripts/update-serverless-spec.sh` and `env -u TF_ACC make gen`. Alternative: hand-write the Go types now and regenerate later — rejected because the next `make gen` would delete them. |
| 29 | +- **Force the nested attributes the generator would mark computed_optional.** In `modify_spec.sh`, set `monitoring`, `logging`, and `audit` to optional. Leave `destination` required when `audit` is set, because the create schema requires `destination`, not because `project_id` is required inside it. Leave `enabled` computed_optional and give it `Default: true`. A default on a non-computed attribute fails provider schema validation. Set `destination.project_type` to required and replace its generated `OneOf` with `observability` and `security`: the API schema marks it optional with the full `ProjectType` enum, but project-api rejects a request without it (403 "Unable to determine project type") and `ValidateLoggingDestination` accepts only those two types, so the schema fails at plan instead of apply. Leave `destination.status` computed inside `destination`. `destination` is one nested object, so a computed child does not hide removal the way a computed field inside the linked-project map entry does. Create and patch copy `project_id` and `project_type` only. |
| 30 | +- **Replace `ignore_filters` with a set of ids.** The API field is an array of `{id}` objects with no order promise, so the generator emits a nested list. Delete that attribute and add an optional set of strings, the same kind of attribute as `traffic_filter_ids`. A list would plan a diff whenever the API returned ids in another order. Re-add `setvalidator.SizeAtMost(10)` because replacing the attribute drops the generated `maxItems` validator. Do not reject an empty set. Null and empty both mean no filters. The mapper writes each string as `{"id":"..."}`. |
| 31 | +- **Clear with JSON null, and keep the configured absence.** The project API documents `ignore_filters: null` as the way to remove every filter. Removing a non-empty set, whether the new configuration is null or empty, sends that null. A generated nil pointer cannot encode it, because `omitempty` drops the key. The same raw patch body that sends `"audit":null` sends `"ignore_filters":null`. On read, an absent or empty API array keeps the null or empty set already in the plan or state, so `ignore_filter_ids = []` does not come back as null; a non-empty prior set becomes null, so removed filters show as drift. Create omits `ignore_filters` for both null and empty. |
| 32 | +- **Clear the category with JSON null.** Removing `audit` must send `"audit":null` and must not send `"monitoring":null`. A nil `*OptionalLoggingCategory` would omit `audit` and leave the category in place. The clear path uses `Patch*ProjectWithBodyWithResponse` on `ClientWithResponsesInterface`. Tests compare the marshalled body, not the Go struct. Alternative: global `nullable-type` — rejected under Non-Goals. |
| 33 | +- **Read keeps a configured empty shell.** No `monitoring.logging.audit` does not by itself null `monitoring`. If the API has no audit and the plan or state `audit` is already null, keep that `monitoring` value, including `monitoring = {}` or `monitoring = { logging = {} }`. Store `monitoring` as null when the API has no audit and the plan or state `monitoring` is null. When the API returns audit and the plan or state `audit` is null, shell or not, store the API object with `ignore_filter_ids` null if the API array is absent or empty, so import and refresh can see console configuration and a shell cannot hide it. When the plan or state has audit and the API has none, store `monitoring` as null. One `Read` serves create, update, and refresh with no call-site marker, so after an apply Terraform's inconsistent-result check reports the dropped write, the same way `linked` behaves today. |
| 34 | + |
| 35 | +## Risks / Trade-offs |
| 36 | + |
| 37 | +- [Public bundle still strips the schemas] → Implementation stops before `make gen`. No local spec fork. Dropping `x-exclude-from-documentation` is an upstream project-API change; this repo does not publish that bundle. |
| 38 | +- [Hand-built patch JSON drifts from the generated request struct] → Use `Patch*ProjectWithBodyWithResponse` when the body must contain `audit` or `ignore_filters` null. Other patch fields stay on the generated struct, merged into that JSON. The unit test locks those bytes. |
| 39 | +- [`destination.status` shows as known after apply] → It is computed and not sent on write. An unrelated update may mark it `(known after apply)`. That is expected. |
| 40 | +- [API later resolves `project_type` from `project_id` or widens destination types] → Relaxing a required attribute or widening a validator is not a breaking change; a follow-up can do it once the API does. |
| 41 | +- [API adds another `logging` key later] → This change only reads and writes `audit`. A null `audit` leaves sibling keys alone. |
| 42 | +- [Acceptance needs a real second project as the destination] → Unit tests cover the request body. A human runs any `TestAcc…` case; the agent does not set `TF_ACC`. |
| 43 | + |
| 44 | +## Migration Plan |
| 45 | + |
| 46 | +Projects created by this provider with no `monitoring` block stay unconfigured. A project whose audit logging was set outside Terraform, such as in the console, will show a plan to remove it after the first refresh. Add the block to configuration before applying, or the apply sends `audit: null`. Removing a block that this provider manages also sends `audit: null`. No state migration. |
| 47 | + |
| 48 | +## Open Questions |
| 49 | + |
| 50 | +None. |
0 commit comments