feat: Add folderUid support - #475
Conversation
|
|
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Introduce model.Check, which embeds synthetic_monitoring.Check and adds a FolderUid field for the API-only folderUid property that is not part of the protobuf definition. All check client methods now accept and return model.Check so callers can read and set folderUid.
90539d7 to
9bf12c7
Compare
|
@VikaCep is this still relevant ? This is a breaking change in public API, are we ok with that ? On example this method is used here https://github.com/grafana/terraform-provider-grafana/blob/85533e88d30360a0617d0a44dc165f36c0aff4fc/internal/resources/syntheticmonitoring/resource_check.go#L848 |
There's no way around it. I think our best bet right now is to simply accept the breakage, but we need to do it once, we cannot do piecemeal updates and break this multiple times. My take is that we take the OpenAPI spec we generate in the API (which is public) and we generate the client from that. |
|
Yes, this is still relevant. The API-side PR (grafana/synthetic-monitoring-api#2023) is already merged, so it accepts and returns I understand the concern about breaking the public API more than once. So what would you say is the preferred path forward?
I'm happy to go either way, but I don't have a good sense of how much time/effort the OpenAPI migration would take. Any thoughts on scope and timeline for that? |
I think we can merge this and update deps this time. Additionally we can create an issue to migrate some existing code here to auto-generated from OpenAPI spec. |
## SM Checks: Add `folder_uid` attribute Adds a `folder_uid` attribute to the `grafana_synthetic_monitoring_check` resource, allowing checks to be associated with a specific Grafana folder. Part of grafana/synthetic-monitoring#589 ### What changed - **Schema**: Added `folder_uid` as an optional string attribute on `grafana_synthetic_monitoring_check` - **CRUD**: `makeCheck` now returns `model.Check` (wrapping `sm.Check` with the API-only `FolderUid` field). `folder_uid` is read from API responses and included in create/update payloads. - **Tests**: HTTP check acceptance test asserts `folder_uid` round-trips correctly - **Examples/Docs**: `http_complex.tf` and generated docs updated to include the new attribute ### Blockers This PR will not compile until the following upstream dependencies are released and bumped in `go.mod`: | Repo | PR | Status | |------|----|--------| | `synthetic-monitoring-api` | [#2023 — feat: Add folderUid to checks](grafana/synthetic-monitoring-api#2023) | Merged | | `synthetic-monitoring-api-go-client` | [#475 — feat: Add folderUid support](grafana/synthetic-monitoring-api-go-client#475) | Merged | Once the Go client is tagged, this PR needs: 1. `go get github.com/grafana/synthetic-monitoring-api-go-client@<new-tag>` 2. `go mod tidy` 3. `make docs` Note: no agent dep bump is needed — `folderUid` is an API-only field and is not part of the protobuf definition. ### Test plan - [x] Bump Go client dep once released - [x] Verify `go build .` succeeds - [x] Run `make docs` and verify no diff - [x] Run HTTP check acceptance test: `TF_ACC=1 TF_ACC_CLOUD_INSTANCE=true go test ./internal/resources/syntheticmonitoring/... -run TestAccResourceCheck_http -v -timeout 30m` - [x] Verify import works (already covered by the ImportState test step)
Add
folderUidsupport to checksPart of https://github.com/grafana/synthetic-monitoring/issues/589
Adds support for the
folderUidfield on checks, allowing checks to be associated with a specific Grafana folder.Why
The SM API now returns and accepts a
folderUidfield on checks. This field is API-only, it is intentionally not part of theprotobufdefinition to avoid propagating it to probes.What changed
model.Check— New wrapper type that embedssynthetic_monitoring.Checkand addsFolderUid. This is the same patternCheckWithAlertsalready used for extending the proto type with API-only fields.folderUidround-trips correctly through add, get, update, list, list-with-alerts, and query.Check client methods now use
model.Checkinstead ofsynthetic_monitoring.Check.