[USTORAGE-27286] Add metadata-column-naming-scheme flag to tableflow topic commands - #3410
Conversation
…opic commands Add a --metadata-column-naming-scheme flag to `tableflow topic enable` (create) and `tableflow topic update`, sending it on the config only when the user provides it, and surface the value in describe/list/enable/update output (omitted when unset). Bumps ccloud-sdk-go-v2/tableflow to v0.7.0, which adds the field to the topic config spec. Adds integration test cases for enable/update with the flag (with a mock update handler that echoes the scheme) and regenerates the affected help goldens.
|
🎉 All Contributor License Agreements have been signed. Ready to merge. |
There was a problem hiding this comment.
Pull request overview
This PR extends the Tableflow topic enable/update commands with a new --metadata-column-naming-scheme flag to let users choose the naming scheme for Tableflow’s internal metadata columns, and surfaces that value in topic outputs. It also updates the Tableflow SDK dependency and refreshes integration test fixtures/goldens to cover the new flag.
Changes:
- Add
--metadata-column-naming-schemetoconfluent tableflow topic enableandconfluent tableflow topic update, passing it through only when explicitly provided. - Surface
metadata_column_naming_schemein human/JSON/YAML output for topic list/describe/enable/update. - Bump
ccloud-sdk-go-v2/tableflowtov0.7.0(andgolang.org/x/oauth2tov0.36.0) and add/refresh integration test coverage + help/output goldens.
Reviewed changes
Copilot reviewed 11 out of 12 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
internal/tableflow/command_topic_enable.go |
Adds the new flag and sends it on create requests when provided. |
internal/tableflow/command_topic_update.go |
Adds the new flag and sends it on patch requests when provided. |
internal/tableflow/command_topic.go |
Adds a new output field to the shared topic output struct and populates it for describe/enable/update output. |
internal/tableflow/command_topic_list.go |
Includes the new field in list output. |
test/test-server/tableflow_handlers.go |
Updates the mock update handler to echo back the metadata naming scheme in responses. |
test/tableflow_test.go |
Adds integration test cases for enable/update with the new flag. |
test/fixtures/output/tableflow/topic/enable-topic-managed-metadata-column-naming-scheme.golden |
New golden output for enable with metadata naming scheme set. |
test/fixtures/output/tableflow/topic/update-topic-managed-metadata-column-naming-scheme.golden |
New golden output for update with metadata naming scheme set. |
test/fixtures/output/tableflow/topic/enable-help.golden |
Updates help output to include the new flag. |
test/fixtures/output/tableflow/topic/update-help.golden |
Updates help output to include the new flag. |
go.mod |
Bumps ccloud-sdk-go-v2/tableflow and golang.org/x/oauth2 dependency versions. |
go.sum |
Updates module checksums for the dependency bumps. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Address review feedback: - Uppercase-normalize --metadata-column-naming-scheme before sending, so values like "portable" work (consistent with --storage-type/--error-handling). The field is x-extensible-enum, so we deliberately don't hard-reject unknown values client-side; the server validates and returns a clear error. - Fix the pre-existing trailing space in the storage_region serialized tag ("storage_region ,omitempty"), which produced a malformed JSON/YAML key; regen the affected JSON goldens. - Regenerate list-topic.golden to include the Metadata Column Naming Scheme column (list output shows all columns).
…ame linter The flag name exceeds the default 20-char / 2-delimiter limits. Add it to both ExcludeFlag allowlists (length and delimiter), matching the sibling record-failure-strategy flag, so the name stays aligned with the API/SDK field metadata_column_naming_scheme rather than abbreviating it.
…st coverage Revert the storage_region serialized-tag change: renaming a serialized field is a breaking change per AGENTS.md and is unrelated to this feature, so it shouldn't ride along in a minor PR. Set metadata_column_naming_scheme on the Azure mock topic so the describe and list goldens exercise a non-empty value read back from the API (previously only the enable/update echo paths covered it, leaving command_topic_list.go's read untested).
Rajendra Dangwal (rajdangwal)
changed the title
|
| } | ||
|
|
||
| if cmd.Flags().Changed("metadata-column-naming-scheme") { | ||
| createTopic.Spec.Config.SetMetadataColumnNamingScheme(strings.ToUpper(metadataColumnNamingScheme)) |
There was a problem hiding this comment.
Question: is ToUpper required? I believe most of our APIs accept lower or upper case even for extensible enum fields.
There was a problem hiding this comment.
The Tableflow API is strictly case-sensitive here. validateMetadataColumnNamingScheme in cc-cts-api-service is an exact-match switch on the uppercase constants (codec.go#L108-L133)
So --metadata-column-naming-scheme portable would get a 400 without the ToUpper. Nothing normalizes case earlier in the request path either (no ToUpper/EqualFold anywhere in internal/api/tableflow/).
Not unique to this field: validateRecordFailureStrategy is an exact != comparison too, so the case-sensitivity is the norm on this API rather than a one-off.
There was a problem hiding this comment.
Update: I've gone the other way on this and made it strict. The ToUpper is removed, so the CLI now sends the value as typed and portable is rejected instead of silently normalized.
Correcting my reply above: the CLI no longer normalizes case. The API behavior I described is unchanged (exact match on DEFAULT/PORTABLE), the CLI just surfaces that now rather than papering over it.
For your TF generator, this means the CLI and the confluent_tableflow_topic resource now agree on this field: both require the canonical uppercase value, so no case translation is needed between them. They differ only in where the error appears (TF at plan time, the CLI as a 400 from the API).
The Tableflow API matches DEFAULT/PORTABLE exactly, and the confluent_tableflow_topic Terraform resource validates the same field case-sensitively. Uppercasing in the CLI meant the same value succeeded via the CLI and failed at terraform plan, so pass the flag value through unchanged and let the API reject anything non-canonical. Reverses the normalization added in fb1d886. No client-side allow-list is added, since the API field is x-extensible-enum.
2ad221a to
9fa7a02
Compare
|
Release Notes
Breaking Changes
New Features
--metadata-column-naming-schemeflag toconfluent tableflow topic createandconfluent tableflow topic updateto select the naming scheme (DEFAULTorPORTABLE) for a Tableflow topic's internal metadata columns.PORTABLEuses BigQuery-compatiblecflt_metadata_-prefixed names.Bug Fixes
Checklist
Whatsection below whether this PR applies to Confluent Cloud, Confluent Platform, or both.Test & Reviewsection below.Blast Radiussection below.What
Applies to Confluent Cloud only (Tableflow).
Adds a
--metadata-column-naming-schemeflag toconfluent tableflow topic create(aliasenable) andupdate, letting customers choose how a Tableflow topic's internal metadata columns are named in the materialized table:DEFAULT— the existing$-prefixed names.PORTABLE— BigQuery-compatiblecflt_metadata_-prefixed names.The value is sent as typed, only when the flag is provided, and surfaced in
describe/list/create/updateoutput (omitted when unset, matching the API). Bumpsccloud-sdk-go-v2/tableflowtov0.7.0, which adds the field to the topic config spec. No client-side allow-list validation, since the API field isx-extensible-enumand the server validates; consistent with--table-formats/--record-failure-strategy. The value is case-sensitive: the API matchesDEFAULT/PORTABLEexactly and the CLI does not normalize, so a lowercase value is rejected. This keeps the CLI consistent with theconfluent_tableflow_topicTerraform resource, which validates the same field case-sensitively.Blast Radius
Additive, opt-in flag; default behavior is unchanged for anyone not using it.
--metadata-column-naming-schemeare unaffected — no change to existingtableflow topiccommands or output.--metadata-column-naming-schemeontableflow topic create/updatecould be affected. The value is validated server-side, andPORTABLEis gated behind a LaunchDarkly flag (rejected unless enabled for the org), so a bad value fails the single request rather than affecting existing topics.References
Test & Review
tableflow topic enable/updatewith--metadata-column-naming-scheme(mock update handler echoes the scheme), plus regeneratedenable-help/update-helpgoldens.TestTableflowTopicandTestHelppass;gofmt,go vet, and the CLI flag linter clean.tableflow topic enable/update+describeboth with and without--metadata-column-naming-scheme. ConfirmedPORTABLEis accepted, persisted, and echoed back ondescribe(and in-o json), while topics without the flag show no scheme.Existing commands without
metadata-column-naming-schemeworks fine and the config is not shown during describe:Commands with
metadata-column-naming-schemeworks withDEFAULTvalue and shown during describe:Commands with
metadata-column-naming-schemefails withPORTABLEvalue if it is not enabled via LD, works when enabled:Similarly, the update commands work for the DEFAULT <-> PORTABLE switch, if the LD allows it.