From e8968764e38c6a85ee63272fe104184b639367c7 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Tue, 29 Sep 2026 21:49:41 +0000 Subject: [PATCH 1/2] docs: add a playbook for REST API version upgrades Record how to validate an api-client-go bump and an LD-API-Version pin, including the second HTTP client and generated commands whose beta flag is not in the OpenAPI tag. Co-authored-by: Ramon Niebla --- CONTRIBUTING.md | 4 ++ docs/playbooks/rest-api-version-upgrades.md | 78 +++++++++++++++++++++ 2 files changed, 82 insertions(+) create mode 100644 docs/playbooks/rest-api-version-upgrades.md diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 5a4744ecf..6099aa5e8 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -19,6 +19,10 @@ the `go.mod/go.sum` files are tidy. In addition, pre-commit will run dev server UI tests and build the project to make sure an up-to-date build is being checked in. You will need to install npm. +## Upgrading the REST API version + +Changes that bump `api-client-go` or the `LD-API-Version` header follow [docs/playbooks/rest-api-version-upgrades.md](docs/playbooks/rest-api-version-upgrades.md). Unit tests do not cover the live API or generated beta commands. Walk that checklist before marking the pull request validated. + ## Adding a new command There are a few things you need to do in order to wire up a new top-level command. diff --git a/docs/playbooks/rest-api-version-upgrades.md b/docs/playbooks/rest-api-version-upgrades.md new file mode 100644 index 000000000..2ec9ac290 --- /dev/null +++ b/docs/playbooks/rest-api-version-upgrades.md @@ -0,0 +1,78 @@ +# Validating a LaunchDarkly REST API version upgrade + +Use this when a change bumps `github.com/launchdarkly/api-client-go` or changes the `LD-API-Version` header ldcli sends. The worked example is [PR 832](https://github.com/launchdarkly/ldcli/pull/832) (api-client-go v14 to v24, header `20240415`, [REL-16083](https://launchdarkly.atlassian.net/browse/REL-16083)). + +A green `go test ./...` does not prove the upgrade. The header tests talk to `httptest`. They stay green if a generated command sends the wrong version, if a live response no longer unmarshals, or if login starts sending a version header the device-authorization endpoints do not expect. + +## Two clients, one version + +ldcli talks to the REST API through two stacks. Pinning one and leaving the other on the access token's default version is the failure mode this playbook exists to catch. + +| Stack | Code | Who uses it | +| --- | --- | --- | +| Typed client | `internal/client.New` builds `api-client-go` | setup, quickstart, dev-server flag and environment sync, and the hand-written flags, members, projects, and environments helpers | +| Resources client | `internal/resources.ResourcesClient.MakeRequest` | generated commands in `cmd/resources/resource_cmds.go`, plus hand-written commands that call this client (`flags toggle-on` / `toggle-off`, `flags archive`, `whoami`, `members invite`, `sdk-active`, sourcemaps and symbols upload, setup verify) | + +`api-client-go` v21 and later add `LD-API-Version: 20240415` in `prepareRequest` when the caller did not set a version. The resources client does not use that library. It only sends the header if `MakeRequest` sets it. + +`MakeUnauthenticatedRequest` calls `MakeRequest` with `isBeta` false. Login (`/internal/device-authorization` and `/internal/device-authorization/token`) and the dev-server commands that use the unauthenticated client therefore send the same stable header as every other non-beta call. + +## Before editing + +1. Name the target version string (today `20240415`) and the `api-client-go` major that emits it by default. v21 is the first major with that default. Later majors are fine when they match another LaunchDarkly Go consumer (v24 matches the Terraform provider) and the changelog's breaking changes are accounted for below. +2. Diff the client changelog between the old major and the new one. Record every constructor, field, or required response field ldcli touches. v24 dropped the value argument from `NewPatchOperation`. `internal/flags/client.go` builds `PatchOperation{Op, Path, Value}` directly. v24 models also reject responses that omit a spec-required field. +3. Search `ld-openapi.json` for operations whose `LD-API-Version` parameter is required and whose enum does not include the target version. The generator's beta switch is only `strings.Contains(tag, "(beta)")` in `cmd/resources/resources.go`. A tag without `(beta)` is generated as `IsBeta: false` even when the spec allows only `beta`. +4. Confirm `cmd/resources/resources.go` `makeRequest` still copies only `path` and `query` parameters. The generated `--ld-api-version` flag is not written onto the request. `IsBeta` is the only switch `MakeRequest` honors. + +As of PR 832, the only operations that fail step 3 are the five `ai-configs` agent-graph commands (`list-agent-graphs`, `get-agent-graph`, `create-agent-graph`, `update-agent-graph`, `delete-agent-graph`). Their tag is `AI Configs`. Their header enum is `["beta"]`. They are generated with `IsBeta: false`, so after the pin they send `20240415`. They sent no version header before the pin. Both shapes fail a beta-only route. Fix them in the generator (set `IsBeta` when the tag contains `(beta)` or when the header enum is only `beta`), then regenerate `cmd/resources/resource_cmds.go`. Do not special-case paths inside `MakeRequest`. + +## Code checklist + +- [ ] `go.mod` requires the new `github.com/launchdarkly/api-client-go/vN` module, and no `.go` file still imports the previous major. +- [ ] `internal/resources` keeps a single version constant and `MakeRequest` sets `LD-API-Version` to that constant when `isBeta` is false and to `beta` when `isBeta` is true. +- [ ] Every `api-client-go` call site compiles against the new models. Pay attention to removed constructors and to fields that became required on responses. +- [ ] Flag patches still serialize `op`, `path`, and `value`. A bool `false` must remain in the JSON. `PatchOperation` omits a nil `Value`. +- [ ] Generated commands whose OpenAPI tag contains `(beta)` still pass `IsBeta: true`. +- [ ] Generated commands whose `LD-API-Version` enum is only `beta` also pass `IsBeta: true`, even when the tag has no `(beta)`. +- [ ] The PR describes behavior that changes for tokens whose default version is older than the pin: list page size, omitted `environments` on `flags list` unless `filterEnv` is set, and filters the new version rejects with 400. + +## Tests that have to fail if the pin is wrong + +These are the tests to add or extend. A unit test that passes `isBeta` in directly does not cover the generator. + +| What to assert | Where | +| --- | --- | +| Typed client sends `LD-API-Version: ` on a real method call such as `ProjectsApi.GetProjects` | `internal/client/client_test.go` | +| `MakeRequest` sends `` when `isBeta` is false and `beta` when `isBeta` is true | `internal/resources/client_test.go` | +| A generated command with `(beta)` in its tag calls `MakeRequest` with `isBeta` true | generator test, or a test that loads one `OperationCmd` from `resource_cmds.go` | +| Each operation from the OpenAPI search in step 3 calls `MakeRequest` with `isBeta` true | same | +| A flag patch body contains `"value": false` for toggle-off | typed-client test around `FlagsClient.Update`, not the resources mock used by the quickstart toggle tests | + +Run: + +```bash +go test ./internal/client ./internal/resources ./internal/flags ./cmd/resources +go test ./... +``` + +`golangci-lint` v1.63.4 is what CI runs. `make test` is `go test ./...`. + +## Live checks CI will not do + +Run these against an account whose access token defaults to an older API version than the pin, and again with a token created by `ldcli login` (those tokens already default to the current version). Use `--access-token` so you are not only testing the login-token path. + +- [ ] `projects list`, `flags list`, and one flag `GET` return JSON the CLI prints without an unmarshal error. v24 fails the call when a required field is missing. +- [ ] `flags list` plaintext still prints the existing paging line (`Showing results 1 - 20 of N`) when the project has more flags than the new default page size. +- [ ] One command whose tag contains `(beta)` sends `LD-API-Version: beta`. Capture it with a proxy or with `--base-uri` pointed at a local recorder. +- [ ] `ai-configs list-agent-graphs` sends `beta`, not the stable version. Until the generator fix lands, this check fails and should be called out in the PR rather than treated as covered by the header unit tests. +- [ ] `ldcli login` still completes device authorization. That path is unauthenticated and now sends the stable version header. +- [ ] Dev-server project sync still loads flags. It pages `GetFeatureFlags` and reads `items` and `variations`. + +## What to write in the pull request + +- The target version, the client major, and why that major (minimum with the default header, or a later major aligned with another consumer). +- Which of the two clients changed. +- Breaking model changes and the call sites updated for them. +- Operations that require `beta` and whether they still send it. +- User-visible response changes for older tokens (page size, dropped fields, filters that now return 400). +- Commands you did not exercise against a live account. The PR template's "validated against all supported platform versions" box stays unchecked until the live checks above are done. From e2817df9bb55a11497f1a05e50ca7e70fc0a7498 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Wed, 30 Sep 2026 02:30:01 +0000 Subject: [PATCH 2/2] docs: keep the API upgrade playbook independent of one upgrade Replace PR-specific history, hard-coded version facts, and command lists with lookups that stay correct: a spec query for beta-only operations, searches for each client's callers, and the client's own default header. Co-authored-by: Ramon Niebla --- docs/playbooks/rest-api-version-upgrades.md | 92 +++++++++++++-------- 1 file changed, 56 insertions(+), 36 deletions(-) diff --git a/docs/playbooks/rest-api-version-upgrades.md b/docs/playbooks/rest-api-version-upgrades.md index 2ec9ac290..be265d8da 100644 --- a/docs/playbooks/rest-api-version-upgrades.md +++ b/docs/playbooks/rest-api-version-upgrades.md @@ -1,6 +1,6 @@ # Validating a LaunchDarkly REST API version upgrade -Use this when a change bumps `github.com/launchdarkly/api-client-go` or changes the `LD-API-Version` header ldcli sends. The worked example is [PR 832](https://github.com/launchdarkly/ldcli/pull/832) (api-client-go v14 to v24, header `20240415`, [REL-16083](https://launchdarkly.atlassian.net/browse/REL-16083)). +Use this when a change bumps `github.com/launchdarkly/api-client-go` or changes the `LD-API-Version` header ldcli sends. A green `go test ./...` does not prove the upgrade. The header tests talk to `httptest`. They stay green if a generated command sends the wrong version, if a live response no longer unmarshals, or if login starts sending a version header the device-authorization endpoints do not expect. @@ -8,45 +8,65 @@ A green `go test ./...` does not prove the upgrade. The header tests talk to `ht ldcli talks to the REST API through two stacks. Pinning one and leaving the other on the access token's default version is the failure mode this playbook exists to catch. -| Stack | Code | Who uses it | +| Stack | Entry point | Find its callers | | --- | --- | --- | -| Typed client | `internal/client.New` builds `api-client-go` | setup, quickstart, dev-server flag and environment sync, and the hand-written flags, members, projects, and environments helpers | -| Resources client | `internal/resources.ResourcesClient.MakeRequest` | generated commands in `cmd/resources/resource_cmds.go`, plus hand-written commands that call this client (`flags toggle-on` / `toggle-off`, `flags archive`, `whoami`, `members invite`, `sdk-active`, sourcemaps and symbols upload, setup verify) | +| Typed client | `internal/client.New`, which builds an `api-client-go` client | `rg -n 'client\.New\(' --glob '*.go' --glob '!*_test.go' cmd internal` | +| Resources client | `internal/resources.ResourcesClient.MakeRequest` | `rg -l 'MakeRequest\(\|MakeUnauthenticatedRequest\(' --glob '*.go' --glob '!*_test.go' cmd internal` | -`api-client-go` v21 and later add `LD-API-Version: 20240415` in `prepareRequest` when the caller did not set a version. The resources client does not use that library. It only sends the header if `MakeRequest` sets it. +The resources client serves every generated command in `cmd/resources/resource_cmds.go` and the hand-written commands those searches list. It does not use `api-client-go`, so it sends `LD-API-Version` only when `MakeRequest` sets it. -`MakeUnauthenticatedRequest` calls `MakeRequest` with `isBeta` false. Login (`/internal/device-authorization` and `/internal/device-authorization/token`) and the dev-server commands that use the unauthenticated client therefore send the same stable header as every other non-beta call. +`MakeUnauthenticatedRequest` calls `MakeRequest` with `isBeta` false. Login (`/internal/device-authorization` and `/internal/device-authorization/token`) and the dev-server commands that use the unauthenticated client get whatever header `MakeRequest` sends for non-beta calls. ## Before editing -1. Name the target version string (today `20240415`) and the `api-client-go` major that emits it by default. v21 is the first major with that default. Later majors are fine when they match another LaunchDarkly Go consumer (v24 matches the Terraform provider) and the changelog's breaking changes are accounted for below. -2. Diff the client changelog between the old major and the new one. Record every constructor, field, or required response field ldcli touches. v24 dropped the value argument from `NewPatchOperation`. `internal/flags/client.go` builds `PatchOperation{Op, Path, Value}` directly. v24 models also reject responses that omit a spec-required field. -3. Search `ld-openapi.json` for operations whose `LD-API-Version` parameter is required and whose enum does not include the target version. The generator's beta switch is only `strings.Contains(tag, "(beta)")` in `cmd/resources/resources.go`. A tag without `(beta)` is generated as `IsBeta: false` even when the spec allows only `beta`. -4. Confirm `cmd/resources/resources.go` `makeRequest` still copies only `path` and `query` parameters. The generated `--ld-api-version` flag is not written onto the request. `IsBeta` is the only switch `MakeRequest` honors. - -As of PR 832, the only operations that fail step 3 are the five `ai-configs` agent-graph commands (`list-agent-graphs`, `get-agent-graph`, `create-agent-graph`, `update-agent-graph`, `delete-agent-graph`). Their tag is `AI Configs`. Their header enum is `["beta"]`. They are generated with `IsBeta: false`, so after the pin they send `20240415`. They sent no version header before the pin. Both shapes fail a beta-only route. Fix them in the generator (set `IsBeta` when the tag contains `(beta)` or when the header enum is only `beta`), then regenerate `cmd/resources/resource_cmds.go`. Do not special-case paths inside `MakeRequest`. +1. Name the target version string. The spec's version changelog is in `ld-openapi.json` under `info.description`, in the "API version changelog" table. It also lists what each version changes for callers on older versions. +2. Find the `api-client-go` major whose `prepareRequest` adds that version when the caller sets none. After `go get github.com/launchdarkly/api-client-go/vN`, check: + + ```bash + rg -n 'Header.Add\("LD-API-Version"' "$(go list -m -f '{{.Dir}}' github.com/launchdarkly/api-client-go/vN)/client.go" + ``` + + Choose the lowest major that sends the target, or a later one when there is a reason, such as matching another LaunchDarkly Go consumer. Write the reason in the PR. +3. Read the client's release notes between the old major and the new one. Record every constructor, field, or required response field ldcli uses. Majors can remove constructor arguments and make response fields required, and generated models then reject responses missing those fields. +4. List the operations whose `LD-API-Version` header is required, whose enum does not contain the target version, and whose tag lacks `(beta)`: + + ```bash + jq -r --arg target 20240415 ' + .paths | to_entries[] | .key as $path | .value | to_entries[] + | select(.value | type == "object" and has("operationId")) + | .key as $method | .value as $op + | ($op.parameters // [])[] + | select(.in == "header" and .name == "LD-API-Version" and .required == true) + | select((.schema.enum // []) | index($target) | not) + | select([$op.tags[]? | contains("(beta)")] | any | not) + | "\($op.operationId)\t\($method | ascii_upcase) \($path)\t\(.schema.enum)" + ' ld-openapi.json + ``` + + Replace `20240415` with the target. Every operation this prints needs `IsBeta: true` in `cmd/resources/resource_cmds.go`. If the generator in `cmd/resources/resources.go` sets `IsBeta` only from a `(beta)` tag, these operations come out `false`. Fix them in the generator and regenerate. Do not special-case paths inside `MakeRequest`. +5. Check whether `makeRequest` in `cmd/resources/resources.go` copies header parameters onto the request. If it copies only `path` and `query`, the generated `--ld-api-version` flag has no effect, and `IsBeta` is the only switch `MakeRequest` honors. ## Code checklist -- [ ] `go.mod` requires the new `github.com/launchdarkly/api-client-go/vN` module, and no `.go` file still imports the previous major. -- [ ] `internal/resources` keeps a single version constant and `MakeRequest` sets `LD-API-Version` to that constant when `isBeta` is false and to `beta` when `isBeta` is true. -- [ ] Every `api-client-go` call site compiles against the new models. Pay attention to removed constructors and to fields that became required on responses. -- [ ] Flag patches still serialize `op`, `path`, and `value`. A bool `false` must remain in the JSON. `PatchOperation` omits a nil `Value`. -- [ ] Generated commands whose OpenAPI tag contains `(beta)` still pass `IsBeta: true`. -- [ ] Generated commands whose `LD-API-Version` enum is only `beta` also pass `IsBeta: true`, even when the tag has no `(beta)`. -- [ ] The PR describes behavior that changes for tokens whose default version is older than the pin: list page size, omitted `environments` on `flags list` unless `filterEnv` is set, and filters the new version rejects with 400. +- [ ] `go.mod` requires the new `github.com/launchdarkly/api-client-go/vN` module. `rg 'api-client-go/v' --glob '*.go'` shows only the new major. +- [ ] `MakeRequest` sets `LD-API-Version` from one constant when `isBeta` is false, and to `beta` when `isBeta` is true. +- [ ] Every `api-client-go` call site compiles against the new models, and you have checked each breaking change recorded in step 3. +- [ ] Flag patches still serialize `op`, `path`, and `value`. A bool `false` must remain in the JSON, because `PatchOperation` omits a nil `Value`. +- [ ] Generated commands whose OpenAPI tag contains `(beta)` pass `IsBeta: true`. +- [ ] Every operation from the spec query in step 4 passes `IsBeta: true`. +- [ ] The PR lists what changes for tokens whose default version is older than the target, taken from the spec's version changelog: default page sizes, fields that are omitted, and filters or parameters that now return 400. ## Tests that have to fail if the pin is wrong -These are the tests to add or extend. A unit test that passes `isBeta` in directly does not cover the generator. +Add or extend these. A unit test that passes `isBeta` in directly does not cover the generator. | What to assert | Where | | --- | --- | -| Typed client sends `LD-API-Version: ` on a real method call such as `ProjectsApi.GetProjects` | `internal/client/client_test.go` | -| `MakeRequest` sends `` when `isBeta` is false and `beta` when `isBeta` is true | `internal/resources/client_test.go` | +| Typed client sends the target version on a real method call such as `ProjectsApi.GetProjects` | `internal/client/client_test.go` | +| `MakeRequest` sends the target version when `isBeta` is false and `beta` when `isBeta` is true | `internal/resources/client_test.go` | | A generated command with `(beta)` in its tag calls `MakeRequest` with `isBeta` true | generator test, or a test that loads one `OperationCmd` from `resource_cmds.go` | -| Each operation from the OpenAPI search in step 3 calls `MakeRequest` with `isBeta` true | same | -| A flag patch body contains `"value": false` for toggle-off | typed-client test around `FlagsClient.Update`, not the resources mock used by the quickstart toggle tests | +| Every operation from the step 4 spec query calls `MakeRequest` with `isBeta` true | same | +| The PATCH body sent by `FlagsClient.Update` contains `"value": false` for toggle-off | typed-client test that reads the request body | Run: @@ -55,24 +75,24 @@ go test ./internal/client ./internal/resources ./internal/flags ./cmd/resources go test ./... ``` -`golangci-lint` v1.63.4 is what CI runs. `make test` is `go test ./...`. +CI also runs pre-commit, which runs `golangci-lint` at the `rev` pinned in `.pre-commit-config.yaml`. ## Live checks CI will not do -Run these against an account whose access token defaults to an older API version than the pin, and again with a token created by `ldcli login` (those tokens already default to the current version). Use `--access-token` so you are not only testing the login-token path. +Run these with a token whose default version is older than the target, and again with a token created by `ldcli login`. Pass the older token with `--access-token` so you are not testing only the login-token path. To see headers, point `--base-uri` at a local recorder or run through a proxy. -- [ ] `projects list`, `flags list`, and one flag `GET` return JSON the CLI prints without an unmarshal error. v24 fails the call when a required field is missing. -- [ ] `flags list` plaintext still prints the existing paging line (`Showing results 1 - 20 of N`) when the project has more flags than the new default page size. -- [ ] One command whose tag contains `(beta)` sends `LD-API-Version: beta`. Capture it with a proxy or with `--base-uri` pointed at a local recorder. -- [ ] `ai-configs list-agent-graphs` sends `beta`, not the stable version. Until the generator fix lands, this check fails and should be called out in the PR rather than treated as covered by the header unit tests. -- [ ] `ldcli login` still completes device authorization. That path is unauthenticated and now sends the stable version header. -- [ ] Dev-server project sync still loads flags. It pages `GetFeatureFlags` and reads `items` and `variations`. +- [ ] `projects list`, `flags list`, and one flag `GET` print JSON without an unmarshal error. +- [ ] `flags list` plaintext prints the paging line (`Showing results 1 - N of M`) when the project has more flags than the default page size. +- [ ] One command whose tag contains `(beta)` sends `LD-API-Version: beta`. +- [ ] One command from the step 4 spec query sends `LD-API-Version: beta`. The header unit tests do not prove this. +- [ ] `ldcli login` completes device authorization. That path is unauthenticated and sends the non-beta header from `MakeRequest`. +- [ ] Dev-server project sync loads flags. It pages `GetFeatureFlags` and reads `items` and `variations`. ## What to write in the pull request -- The target version, the client major, and why that major (minimum with the default header, or a later major aligned with another consumer). +- The target version, the client major, and why that major. - Which of the two clients changed. - Breaking model changes and the call sites updated for them. -- Operations that require `beta` and whether they still send it. -- User-visible response changes for older tokens (page size, dropped fields, filters that now return 400). -- Commands you did not exercise against a live account. The PR template's "validated against all supported platform versions" box stays unchecked until the live checks above are done. +- The operations from the step 4 spec query and whether each sends `beta`. +- What changes for older tokens, from the spec's version changelog. +- The live checks you ran and the ones you did not. The PR template's "validated against all supported platform versions" box stays unchecked until the live checks are done.