Add cli command to show CMF system-info#3316
Add cli command to show CMF system-info#3316Paras Negi (paras-negi-flink) wants to merge 2 commits intomainfrom
Conversation
|
🎉 All Contributor License Agreements have been signed. Ready to merge. |
There was a problem hiding this comment.
Pull request overview
Adds a new on-prem Flink CLI subcommand to display CMF “system information” (version/revision), backed by a new CMF REST client call and test-server route/fixtures.
Changes:
- Introduce
flink system-infocommand with human + JSON/YAML output. - Add CMF client support for
/cmf/api/v1/system-informationand corresponding test-server handler/route. - Add integration tests and golden fixtures for
system-infooutput and update on-premflinkhelp output.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
internal/flink/command.go |
Registers the new system-info subcommand under flink (on-prem specific commands). |
internal/flink/command_system_info.go |
Implements flink system-info command, parsing and output formatting. |
pkg/flink/cmf_rest_client.go |
Adds GetSystemInformation method to call CMF system-information endpoint. |
pkg/flink/test/mock/cmf_client_mock.go |
Updates gomock client interface with GetSystemInformation. |
test/test-server/flink_onprem_router.go |
Wires new system-information route into the on-prem test router. |
test/test-server/flink_onprem_handler.go |
Adds handler returning stubbed system info payload for tests. |
test/flink_onprem_test.go |
Adds integration test coverage for flink system-info across output formats. |
test/fixtures/output/flink/system-info.golden |
Golden fixture for human/table output. |
test/fixtures/output/flink/system-info-json.golden |
Golden fixture for JSON output. |
test/fixtures/output/flink/system-info-yaml.golden |
Golden fixture for YAML output. |
test/fixtures/output/flink/help-onprem.golden |
Updates on-prem flink --help to include system-info. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| table := output.NewTable(cmd) | ||
| table.Add(&systemInfoOut{ | ||
| Version: derefString(sysInfo.Status.Version), | ||
| Revision: derefString(sysInfo.Status.Revision), |
There was a problem hiding this comment.
sysInfo.Status can be nil when the CMF response is missing/invalid status, but the human-output branch unconditionally dereferences sysInfo.Status.Version/Revision, which will panic. Guard against sysInfo.Status == nil (or initialize a non-nil status before dereferencing) so the command fails gracefully instead of crashing.
| table := output.NewTable(cmd) | |
| table.Add(&systemInfoOut{ | |
| Version: derefString(sysInfo.Status.Version), | |
| Revision: derefString(sysInfo.Status.Revision), | |
| status := &localSystemInformationStatus{} | |
| if sysInfo.Status != nil { | |
| status = sysInfo.Status | |
| } | |
| table := output.NewTable(cmd) | |
| table.Add(&systemInfoOut{ | |
| Version: derefString(status.Version), | |
| Revision: derefString(status.Revision), |
| func (c *command) newSystemInfoCommand() *cobra.Command { | ||
| cmd := &cobra.Command{ | ||
| Use: "system-info", | ||
| Short: "Display CMF system information.", | ||
| Args: cobra.NoArgs, | ||
| RunE: c.systemInfo, | ||
| Annotations: map[string]string{pcmd.RunRequirement: pcmd.RequireCloudLogout}, | ||
| } | ||
|
|
||
| addCmfFlagSet(cmd) | ||
| pcmd.AddOutputFlag(cmd) | ||
|
|
||
| return cmd | ||
| } |
There was a problem hiding this comment.
The recursive help golden tests expect a fixture for the leaf command help output (e.g., test/fixtures/output/flink/system-info-help-onprem.golden). Adding this command without the corresponding help fixture will cause TestHelp to fail for the on-prem configuration.
| } | ||
|
|
||
| func (cmfClient *CmfRestClient) GetSystemInformation(ctx context.Context) (map[string]interface{}, error) { | ||
| baseURL := cmfClient.GetConfig().Servers[0].URL |
There was a problem hiding this comment.
Building the request URL via string concatenation can produce an invalid path when the configured base URL ends with / (resulting in //cmf/api/...). Prefer joining/normalizing the base URL (e.g., trimming the trailing slash or using url.JoinPath) to ensure the request is well-formed.
| baseURL := cmfClient.GetConfig().Servers[0].URL | |
| baseURL := strings.TrimRight(cmfClient.GetConfig().Servers[0].URL, "/") |
- Remove quotes around version in YAML golden (YAML serializer doesn't quote version-like strings) - Reorder help-onprem.golden so system-info sorts after statement (cobra alphabetical ordering) - Add missing system-info-help-onprem.golden for auto-generated help tests Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Release Notes
Breaking Changes
New Features
Bug Fixes
Checklist
Whatsection below whether this PR applies to Confluent Cloud, Confluent Platform, or both.Test & Reviewsection below.Blast Radiussection below.What
Blast Radius
References
Test & Review