Fix: add ComputePool.Status custom UnmarshalJSON for flat API response by paras-negi-flink · Pull Request #65 · confluentinc/cmf-sdk-go

Paras Negi (paras-negi-flink) · 2026-04-17T05:12:41Z

Description

ComputePool.Status is generated as *map[string]map[string]interface{} because the OpenAPI spec declares status inline with additionalProperties: true instead of $ref-ing the defined ComputePoolStatus schema. The real CMF API returns a flat status like {"phase":"RUNNING","message":null}, which cannot deserialize into that nested map type, so Execute() fails on every valid 200 response for all ComputePool endpoints — breaking every SDK consumer (CLI, CFK, etc.).

Changes

Adds a custom UnmarshalJSON on ComputePool in a new file (v1/model_compute_pool_custom.go), protected via .openapi-generator-ignore so regeneration won't overwrite it.

Behavior:

Every value in status is wrapped as {"value": v} to satisfy the declared inner type map[string]interface{}.
null or missing status leaves Status = nil.
Non-object status shapes (array, top-level scalar) return a decode error — the CMF backend never sends these shapes, so surfacing the error rather than silently dropping it flags API regressions for debugging.
Receiver reuse resets Status so stale values don't leak across decodes.

Consumers read fields uniformly via pool.GetStatus()["<key>"]["value"].

Testing

Unit tests

All passing in v1/model_compute_pool_custom_test.go:

FlatStatus — real API shape, asserts every key gets the {"value":v} wrapper
NullStatus, MissingStatus, EmptyObjectStatus, AllNullValues
NonObjectStatusFails — array, top-level string, top-level number all surface an error mentioning ComputePool.Status
ReceiverReuse — re-decoding into a populated receiver clears old state
PageResponse + EmptyPageItems (empty/missing/null subtests) — ensures the custom method is invoked via ComputePoolsPage
InvalidJSON — top-level JSON errors still propagate as *json.SyntaxError

End-to-end verification against a real CMF server

Verified that the SDK fix alone is sufficient — no consumer-side workaround needed. Setup:

CLI branch with SDK v0.0.6 bumped up
replace github.com/confluentinc/cmf-sdk-go => <local path> pointing at this branch
Real CMF server at http://localhost:9090

$ confluent flink compute-pool list --url http://localhost:9090 --environment test-env-labels
       Creation Time       |      Name      |   Type    |   Phase
---------------------------+----------------+-----------+------------
  2026-04-20T06:39:47.893Z | test-cp-labels | DEDICATED | DEDICATED

$ confluent flink compute-pool list --url http://localhost:9090 --environment test-env-labels --output json
[
  {
    "apiVersion": "cmf.confluent.io/v1",
    "kind": "ComputePool",
    "metadata": {
      "name": "test-cp-labels",
      "creationTimestamp": "2026-04-20T06:39:47.893Z",
      "uid": "34ec0a07-1efd-4905-a6fa-fcafe51bbcd2",
      "labels": {},
      "annotations": {}
    },
    "spec": {
      "type": "DEDICATED",
      "clusterSpec": {
        "flinkVersion": "v1_19",
        "image": "confluentinc/cp-flink:1.19.1-cp1"
      }
    },
    "status": {
      "phase": "DEDICATED"
    }
  }
]

$ confluent flink compute-pool describe test-cp-labels --url http://localhost:9090 --environment test-env-labels
+---------------+--------------------------+
| Creation Time | 2026-04-20T06:39:47.893Z |
| Name          | test-cp-labels           |
| Type          | DEDICATED                |
| Phase         | DEDICATED                |
+---------------+--------------------------+

$ confluent flink compute-pool describe test-cp-labels --url http://localhost:9090 --environment test-env-labels --output yaml
apiVersion: cmf.confluent.io/v1
kind: ComputePool
metadata:
    name: test-cp-labels
    creationTimestamp: "2026-04-20T06:39:47.893Z"
    uid: 34ec0a07-1efd-4905-a6fa-fcafe51bbcd2
    labels: {}
    annotations: {}
spec:
    type: DEDICATED
    clusterSpec:
        flinkVersion: v1_19
        image: confluentinc/cp-flink:1.19.1-cp1
status:
    phase: DEDICATED

CLI flink unit tests also pass: go test ./internal/flink/... → ok (coverage unchanged).

Significant changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
Documentation update

The generated ComputePool.Status type is *map[string]map[string]interface{} (caused by an inline OpenAPI schema with additionalProperties: true instead of a $ref to ComputePoolStatus). The real CMF API returns a flat status object like {"phase":"RUNNING","message":null}, which fails standard JSON decoding against the nested map type and breaks Execute() on all ComputePool endpoints for every SDK consumer. Add a custom UnmarshalJSON in a new file that decodes status as a flat object and wraps each scalar value as {"value": v} to conform to the declared type. Values that are already objects are passed through unchanged to avoid double-wrapping mixed shapes. Non-object status (arrays, scalars) is treated as absent rather than failing the whole response, keeping Execute() succeeding on all valid 200 responses. Both files are listed in .openapi-generator-ignore so they survive regeneration. Remove all three entries once the OpenAPI spec uses $ref to ComputePoolStatus. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Copilot

Pull request overview

Adds a custom JSON unmarshaler to make ComputePool.Status resilient to the CMF API’s flat status object shape despite the generated SDK type being *map[string]map[string]interface{}.

Changes:

Add ComputePool.UnmarshalJSON to wrap flat status scalars as {"value": <scalar>} while passing through already-object values.
Add unit tests covering flat, nested, mixed, null/missing, and non-object status shapes plus page-item decoding.
Update v1/.openapi-generator-ignore to preserve the custom implementation and tests across regeneration.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File	Description
`v1/model_compute_pool_custom.go`	Implements custom `UnmarshalJSON` for `ComputePool` to accept flat `status` objects.
`v1/model_compute_pool_custom_test.go`	Adds tests validating status wrapping behavior and page unmarshaling behavior.
`v1/.openapi-generator-ignore`	Prevents regeneration from overwriting/removing the custom files.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

… guidance Addresses PR review feedback: - Shrink the UnmarshalJSON godoc to match the SDK's one-liner style and drop inline TODO. - Drop the "Remove both entries..." line from .openapi-generator-ignore; keep only the "why" context. - Assert extra-key preservation in TestAlreadyNestedStatus to catch any future passthrough regression that drops keys from object values. - Fix mixed indentation inside the TestEmptyPageItems subtest closure. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Robert Metzger (rmetzger) · 2026-04-20T13:49:52Z

+// {"value": v}; object values pass through unchanged. Non-object shapes
+// (array, scalar) yield a nil Status instead of failing the decode.


Non-object shapes (array, scalar) yield a nil Status instead of failing the decode.

Doesn't that mean we are suppressing error messages?

Yup, swallowing the error hides API regressions. I've fixed this now in the latest commit

Robert Metzger (rmetzger) · 2026-04-20T13:57:29Z

+
+func TestComputePoolUnmarshal_AlreadyNestedStatus(t *testing.T) {
+	// Object values pass through unchanged, including extra keys (no re-wrap).
+	data := makeComputePoolJSON("pool-b", `{"phase":{"value":"PENDING","extra":"meta"}}`)


I'm not a go expert, but does this mean that phase can have a nested json object, and we'll extract PENDING from {"phase":{"value":"PENDING","extra":"meta"}} as the status?
why do we support that kind of nested structure? this will never get returned by the backend, right?

Nice observation, I trimmed this part

Address Robert's comments on PR #65: 1. Non-object status shapes (array, top-level scalar) now return a wrapped decode error instead of silently nil-ing Status. The backend never sends these shapes, so they indicate an API regression worth surfacing. 2. Drop the object-value passthrough branch. The CMF backend only returns flat status, so the passthrough code was unreachable today; keeping it would only matter if the OpenAPI spec were ever fixed to match reality, at which point this whole custom file would be deleted. Simpler to always wrap. Tests updated to match: passthrough cases removed, non-object cases now assert an error is returned. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Tighten the UnmarshalJSON doc to four lines and drop WHAT-narrating comments from tests whose function names and assertions already carry the intent. No logic changes. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-21T10:44:07Z

+		return fmt.Errorf("unmarshal ComputePool.Status: %w", err)
+	}
+	wrapped := make(map[string]map[string]interface{}, len(flat))
+	for k, v := range flat {


wrapped[k] = {"value": v} wraps object values too, which prevents the “pass-through unchanged for object values” behavior described in the PR and makes nested server shapes harder to consume. To support mixed shapes, detect when v is a map[string]interface{} and assign it directly to wrapped[k]; only wrap scalar/null values as { "value": v }.

Suggested change

for k, v := range flat {

for k, v := range flat {

if obj, ok := v.(map[string]interface{}); ok {

wrapped[k] = obj

continue

}

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Robert Metzger (rmetzger)

Thanks!

Robert Metzger (rmetzger)

Milind L (@milindl) has some feedback

Robert Metzger (rmetzger) · 2026-04-24T14:46:51Z

Do we still need this PR with the changes in #66? I guess we can close this PR?

Copilot AI review requested due to automatic review settings April 17, 2026 05:12

Copilot started reviewing on behalf of Paras Negi (paras-negi-flink) April 17, 2026 05:13 View session

Copilot AI reviewed Apr 17, 2026

View reviewed changes

Comment thread v1/model_compute_pool_custom.go Outdated

Comment thread v1/model_compute_pool_custom_test.go Outdated

Paras Negi (paras-negi-flink) marked this pull request as ready for review April 17, 2026 05:38

Paras Negi (paras-negi-flink) requested a review from a team as a code owner April 17, 2026 05:38

Paras Negi (paras-negi-flink) changed the title ~~fix: add ComputePool.Status custom UnmarshalJSON for flat API response~~ Fix: add ComputePool.Status custom UnmarshalJSON for flat API response Apr 17, 2026

Paras Negi (paras-negi-flink) mentioned this pull request Apr 20, 2026

Bump cmf-sdk-go v0.0.5 to v0.0.6 and fix breaking changes confluentinc/cli#3320

Open

13 tasks

Paras Negi (paras-negi-flink) requested a review from Robert Metzger (rmetzger) April 20, 2026 09:23

Robert Metzger (rmetzger) reviewed Apr 20, 2026

View reviewed changes

Paras Negi (paras-negi-flink) requested review from Abhijeet Sharma (abhijeet2096-confluent) and Milind L (milindl) April 21, 2026 05:30

Paras Negi (paras-negi-flink) requested a review from Copilot April 21, 2026 10:41

Copilot started reviewing on behalf of Paras Negi (paras-negi-flink) April 21, 2026 10:41 View session

Copilot AI reviewed Apr 21, 2026

View reviewed changes

Paras Negi (paras-negi-flink) requested a review from Copilot April 22, 2026 02:45

Copilot started reviewing on behalf of Paras Negi (paras-negi-flink) April 22, 2026 02:45 View session

Copilot AI reviewed Apr 22, 2026

View reviewed changes

Robert Metzger (rmetzger) approved these changes Apr 22, 2026

View reviewed changes

Robert Metzger (rmetzger) requested changes Apr 22, 2026

View reviewed changes

Paras Negi (paras-negi-flink) closed this Apr 24, 2026

		// {"value": v}; object values pass through unchanged. Non-object shapes
		// (array, scalar) yield a nil Status instead of failing the decode.

-	for k, v := range flat {
+	for k, v := range flat {
+		if obj, ok := v.(map[string]interface{}); ok {
+			wrapped[k] = obj
+			continue
+		}

Conversation

Paras Negi (paras-negi-flink) commented Apr 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Changes

Testing

Unit tests

End-to-end verification against a real CMF server

Significant changes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Robert Metzger (rmetzger) Apr 20, 2026

Choose a reason for hiding this comment

Uh oh!

Paras Negi (paras-negi-flink) Apr 21, 2026

Choose a reason for hiding this comment

Uh oh!

Robert Metzger (rmetzger) Apr 20, 2026

Choose a reason for hiding this comment

Uh oh!

Paras Negi (paras-negi-flink) Apr 21, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI Apr 21, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Robert Metzger (rmetzger) left a comment

Choose a reason for hiding this comment

Uh oh!

Robert Metzger (rmetzger) left a comment

Choose a reason for hiding this comment

Uh oh!

Robert Metzger (rmetzger) commented Apr 24, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Paras Negi (paras-negi-flink) commented Apr 17, 2026 •

edited

Loading