Skip to content

fix(deps): bump cmf-sdk-go v0.0.5 to v0.0.7 and handle breaking changes#3333

Open
Paras Negi (paras-negi-flink) wants to merge 3 commits intomainfrom
bump-cmf-sdk-v0.0.7
Open

fix(deps): bump cmf-sdk-go v0.0.5 to v0.0.7 and handle breaking changes#3333
Paras Negi (paras-negi-flink) wants to merge 3 commits intomainfrom
bump-cmf-sdk-v0.0.7

Conversation

@paras-negi-flink
Copy link
Copy Markdown

@paras-negi-flink Paras Negi (paras-negi-flink) commented Apr 24, 2026

Release Notes

Breaking Changes

  • None. (CLI surface unchanged. The SDK's own Go API has breaking changes — internal callers updated in this PR.)

New Features

  • None.

Bug Fixes

  • Restores confluent flink compute-pool list/describe/create on Confluent Platform, which had been broken by an upstream cmf-sdk-go v0.0.6 schema regression that caused ComputePool.status to fail to decode.

Checklist

  • I have successfully built and used a custom CLI binary, without linter issues from this PR.
  • I have clearly specified in the What section below whether this PR applies to Confluent Cloud, Confluent Platform, or both.
  • I have attached manual CLI verification results or screenshots in the Test & Review section below.
  • I have added appropriate CLI integration or unit tests for any new or updated commands and functionality.
  • I confirm that this PR introduces no breaking changes or backward compatibility issues.
  • I have indicated the potential customer impact if something goes wrong in the Blast Radius section below.
  • I have put checkmarks below confirming that the feature associated with this PR is enabled in:
    • Confluent Cloud prod
    • Confluent Cloud stag
    • Confluent Platform
    • Check this box if the feature is enabled for certain organizations only

What

This PR bumps github.com/confluentinc/cmf-sdk-go from v0.0.5 → v0.0.7 and adapts the CLI to two breaking type changes the new SDK introduces. Targets Confluent Platform / CP Flink (CMF on-prem).

  • ComputePool.Status: *ComputePoolStatus*map[string]interface{}

    • The typed status struct was replaced by an untyped map. Phase is no longer a struct field — it's an entry in the map.
    • Added extractComputePoolPhase(pool) helper in internal/flink/command_compute_pool.go, used by the 4 consumer sites (convertSdkComputePoolToLocalComputePool, _create_onprem, _describe_onprem, _list_onprem).
    • A non-string phase value (server/schema contract violation) is logged at debug level via log.CliLogger.Debugf — matches the existing pkg/featureflags/feature_flags.go pattern.
  • PostEnvironment.Name: string*string

    • Writes use &name. Reads use the SDK-generated nil-safe GetName() accessor (pkg/flink/cmf_rest_client.go, test/test-server/flink_onprem_handler.go).
  • Drive-bys:

Blast Radius

  • Scope is limited to on-prem Flink commands that consume ComputePool.Status or PostEnvironment.Name: confluent flink compute-pool {list,describe,create} and confluent flink environment {create,update}.
  • If something goes wrong:
    • Compute-pool commands could show empty/wrong Phase (helper returns "" for any non-string value; debug log fires for unexpected types).
    • Environment create/update could submit an empty/nil name if the helper logic regresses; current code reads from a required CLI positional, so the field is always populated in practice.
  • No changes to Confluent Cloud commands, no changes to other Flink commands (statements, applications, savepoints, catalogs).
  • Reverting is a single-commit revert: switching back will re-introduce the v0.0.6 decode bug, so the recommended rollback is to pin the SDK to v0.0.5 and reverse the type adaptations.

References

Test & Review

Environment

  • Repo: confluentinc/cli
  • Branch: bump-cmf-sdk-v0.0.7
  • SDK: github.com/confluentinc/cmf-sdk-go v0.0.7
  • CMF: tested locally against cmf-service (kubectl port-forward svc/cmf-service 9090:80)

Automated

  • go build ./... clean
  • go vet ./... clean
  • go test ./internal/flink/... passes
  • go test ./pkg/flink/... — only pre-existing flake TestInteractiveOutputControllerTestSuite/TestCloseTableViewOnUserInput (unrelated to SDK; UI-input test)

Manual CLI validation

Attached in the comment below.

Copilot AI review requested due to automatic review settings April 24, 2026 19:07
@paras-negi-flink Paras Negi (paras-negi-flink) requested a review from a team as a code owner April 24, 2026 19:07
@confluent-cla-assistant
Copy link
Copy Markdown

🎉 All Contributor License Agreements have been signed. Ready to merge.
Please push an empty commit if you would like to re-run the checks to verify CLA status for all contributors.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates the CLI’s CMF integration to use github.com/confluentinc/cmf-sdk-go v0.0.7 and adapts the on-prem Flink commands/tests to two upstream breaking model changes (ComputePool status becoming an untyped map; PostEnvironment name becoming a pointer).

Changes:

  • Bump cmf-sdk-go from v0.0.5 → v0.0.7 (and update go.mod/go.sum accordingly).
  • Add extractComputePoolPhase helper to read status["phase"] from the new untyped ComputePool.Status and use it in on-prem compute-pool list/describe/create + local conversion.
  • Update environment create/update paths (client + test server + commands) to use PostEnvironment.Name as *string via &environmentName writes and GetName() reads; add unit tests for phase extraction.

Reviewed changes

Copilot reviewed 10 out of 11 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
test/test-server/flink_onprem_handler.go Adjust test-server models to match SDK pointer/name and status-map changes.
pkg/flink/cmf_rest_client.go Switch PostEnvironment name reads to GetName(); fix UpdateEnvironment godoc.
internal/flink/command_environment_create.go Set PostEnvironment.Name as *string.
internal/flink/command_environment_update.go Set PostEnvironment.Name as *string.
internal/flink/command_compute_pool.go Add extractComputePoolPhase and update local conversion to use it.
internal/flink/command_compute_pool_list_onprem.go Use extractComputePoolPhase instead of typed status access.
internal/flink/command_compute_pool_describe_onprem.go Use extractComputePoolPhase instead of typed status access.
internal/flink/command_compute_pool_create_onprem.go Use extractComputePoolPhase for Phase output.
internal/flink/command_compute_pool_test.go Add table-driven tests for extractComputePoolPhase.
go.mod / go.sum Dependency bump for cmf-sdk-go.

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

Comment on lines 93 to 94
Name: sdkComputePool.GetMetadata().Name,
Type: sdkComputePool.GetSpec().Type,
Copy link

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the human output table, Name and Type are taken from the input (sdkComputePool) while CreationTime/Phase come from the server response (sdkOutputComputePool). This can print incorrect values if the server defaulted/normalized fields; it’s also inconsistent with describe/list which print response data. Prefer using sdkOutputComputePool for Name and Type as well.

Suggested change
Name: sdkComputePool.GetMetadata().Name,
Type: sdkComputePool.GetSpec().Type,
Name: sdkOutputComputePool.GetMetadata().Name,
Type: sdkOutputComputePool.GetSpec().Type,

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a valid concern.

Comment on lines 93 to 94
Name: sdkComputePool.GetMetadata().Name,
Type: sdkComputePool.GetSpec().Type,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a valid concern.

Comment thread internal/flink/command_compute_pool.go
Comment thread pkg/schemaregistry/schema_test.go Outdated
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

srsdk "github.com/confluentinc/schema-registry-sdk-go"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like we have merging conflict for this file, can we fix that?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

resolved

v0.0.7 introduces two breaking changes that needed CLI adaptation:

1. ComputePool.Status is now *map[string]interface{} (flattened from the
   earlier v0.0.6 nested map shape). Phase is no longer a typed struct
   field - callers read it from the untyped status map. Added
   extractComputePoolPhase helper used by the 4 compute-pool command
   sites; a non-string "phase" value (server/schema contract violation)
   is logged at debug level.

2. PostEnvironment.Name is now *string. Writes use &name; reads use the
   SDK-generated nil-safe GetName() accessor.

Also fixes a pre-existing godoc typo on UpdateEnvironment.
…d phase-extraction test

Addresses review feedback on #3333:
- Read Name/Type from sdkOutputComputePool so create matches describe/list
  and the human/serialized outputs agree on what was actually persisted.
- Add table-driven unit test for extractComputePoolPhase covering nil
  status, missing key, nil/string/empty/non-string phase values.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
// A missing or nil value is treated as "phase not yet populated" and returns "". A
// present value that isn't a string indicates a server/schema contract violation and
// is logged at debug level.
func extractComputePoolPhase(pool cmfsdk.ComputePool) string {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we expecting similar *map[string]interface{} as a general data struct for future CMF development?

We have an ongoing initiative to automate the CP CMF CLI source code generation, and your team doesn't need to spend time on the standard CRUDL feature development anymore, so we'd like to have the custom functions as few as possible.

Can we explore ways to make this function more generic so that we can try to generate it whenever we have a need to extract certain key from a map?

Copy link
Copy Markdown
Author

@paras-negi-flink Paras Negi (paras-negi-flink) Apr 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, it's realistically the only practical way to surface controller-driven values, and patterns like these will pop up from time to time.

Generalized to pkg/flink.GetMapField so auto-gen has a single helper to target.

….GetMapField

cmf-sdk-go v0.0.7 represents 9+ K8s/FKO-style fields as map[string]any
(Environment.Status, ApplicationInstanceStatus.Spec, FlinkApplicationAllOf
.{Metadata,Spec,Status}, ComputePoolDefaults, etc.). The pattern will
recur as more CMF resources surface controller-driven status.

Lift the compute-pool-specific extraction into a generic, type-parameterized
helper available CMF-wide. Replaces the local extractComputePoolPhase logic
with a one-line wrapper. Tests moved alongside the helper and expanded to
cover string, int64, nested map, and slice value types.

The auto-gen tooling for the CP CMF CLI can target this signature directly
when emitting accessors for untyped fields.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@sonarqube-confluent
Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants