RPCN - Support for optional fields#2560
Conversation
The v4.53→v4.73 benthos bump in cloudv2 (PR #27724) re-serialized the Connect config schema, restoring per-field is_optional/is_advanced/ is_secret/is_deprecated. Wire those signals through: - enrichComponentsWithConfigSchema stamps requiredBySchema/secret onto proto specs from the raw GetPipelineServiceConfigSchema JSON (the ListComponents proto still drops non-string defaults and has no secret field at all). - checkRequired prefers the stamp; the proto-flag ladder remains as a documented degraded fallback for pre-4.59 dataplanes. - generateDefaultValue stops zero-filling fields whose default was lost in proto serialization, so engine defaults apply. - Deprecated fields are excluded from forms and generated configs; secret masking is the schema flag unioned with the name heuristic. - useEnrichedComponents consolidates the enrichment recipe for the pipeline editor, onboarding wizard, and template form panel, gating spec consumption until the config schema settles. - Ground-truth fixtures captured from the served schema (RPCN 4.100.0) back integration tests for kafka/generate/http_client/aws_s3/mapping/ chunker required-vs-optional behavior. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Four templates drifted from the components Connect actually ships, and the drift was invisible because the schema snapshot the regression tests validate against was frozen at RPCN 4.65 with the affected components allowlisted as known-missing: - dynamodb-cdc: aws_dynamodb_stream never shipped; rebuilt on aws_dynamodb_cdc (tables list, start_from/snapshot_mode knobs). - oracle-cdc: oracle_cdc → oracledb_cdc (connection_string/include). - sqlserver-cdc: sql_server_cdc → microsoft_sql_server_cdc (connection_string/include). - iceberg: catalog_uri/warehouse moved to catalog.url and the now-optional catalog.warehouse. This one was user-visible broken — the component exists, so the template showed in the gallery and its YAML failed dataplane lint on deploy. Refresh the snapshot to the RPCN 4.100 cloud schema cloudv2 actually serves, which also surfaced that redpanda_migrator dropped its batching field (removed from the mirror template). Empty the KNOWN_MISSING_COMPONENTS allowlist: every template is now fully validated against the snapshot. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
✅ Clean — no registry drift, off-token colours, or ad-hoc classesApp:
Generated by lookout audit-changes. |
…n fix The isSchemaLoading gate in useEnrichedComponents held specs empty until the multi-MB GetPipelineServiceConfigSchema settled, while pickers and forms rendered from the instantly-available proto componentList. Every confirmed regression traced to that window: silent no-op connector inserts, the serverless create timeout firing then a late resolution clobbering user-typed YAML, wizard YAML regeneration failing on deep-linked steps, and view mode rendering schema-less (forever, on a hung request). Specs are now available as soon as ListComponents resolves and re-emit with enrichment stamps when the schema settles — the interim proto-flag fallback is the same degraded mode used for pre-4.59 dataplanes. The template form's one-shot defaults reset still waits for full fidelity via isLoading, and enrichment results are memoized module-wide instead of recomputed per mount. Separately, harden the serverless create flow against the pre-existing race: after the init timeout has surfaced the editor, a late resolution no longer replaces the buffer. Fix a deterministic generation bug: components whose root config is an array of objects (switch, group_by) can never be "required", so the required-only collection seeding emitted undefined → `switch:` YAML null, which fails lint. Component roots now always produce a type-correct value. Cleanups from the same review: fold the deprecated-field exclusion into isFormField, deduplicate the SASL array generation into generateObjectArrayValue, and share a findConnectComponent lookup. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@claude review |
|
Claude finished @SpicyPete's task in 3m 24s —— View job Code Review — RPCN optional field support
Reviewed the full diff against Findings below are all minor / non-blocking. 1. Template-form default reset now depends on the config-schema query settling
2. List-entry scalars are force-set to
|
|
@claude review |
|
Claude finished @SpicyPete's task in 3m 32s —— View job Code Review — RPCN optional field support (re-review)
Re-reviewed against the current branch tip, closely reading the logic files (schema enrichment/
Findings below are all minor / non-blocking — nothing new that rises above the last round. 1. Container
|
| { | ||
| new: 'Enter a name for the new topic.', | ||
| existing: 'Choose an existing topic to read or write data from.', | ||
| both: 'Choose an existing topic to read or write data from, or create a new topic.', | ||
| }[selectionMode] |
There was a problem hiding this comment.
nit: why not hoist? kind of weird to see an object in the render body like this
| const { data: componentListResponse, isLoading: isComponentListLoading } = useListComponentsQuery(); | ||
| const components = useMemo( | ||
| () => (componentListResponse?.components ? parseSchema(componentListResponse.components) : []), | ||
| [componentListResponse] | ||
| ); | ||
| const { components, componentList, isLoading: isComponentListLoading } = useEnrichedComponents(); |
| /** | ||
| * Anchors a node's lint hints to the inspector form fields they belong to, so an error renders | ||
| * under the field (or inside the group section) it's about instead of as "(line 12)" in a banner | ||
| * the user has to decode. See `resolveHint` for the anchoring tiers. | ||
| */ |
| * the user has to decode. See `resolveHint` for the anchoring tiers. | ||
| */ | ||
|
|
||
| /** Field/group key (`path.join('/')`) → the lint messages anchored to it. */ |
There was a problem hiding this comment.
i'm gonna guess something weird happened wrt comments in this file?
|
|
||
| function coerceArrayItems(spec: RawFieldSpec, text: string): unknown[] { | ||
| const lines = text | ||
| const arrayFieldLines = (text: string): string[] => |
There was a problem hiding this comment.
nit: for the functions in this file I wonder if they already exist in a util file. some look a little familiar
| // The "Create new topic" affordance next to topic pickers; opens the Add-topic dialog. The created | ||
| // topic is written into the YAML, which re-syncs into the (clean) form. | ||
| const CreateTopicButton = () => { | ||
| const { onCreateTopic } = useContext(ResourceFieldContext); | ||
| if (!onCreateTopic) { | ||
| return null; | ||
| } | ||
| return ( | ||
| <Button className="self-start" onClick={onCreateTopic} size="sm" type="button" variant="outline"> | ||
| <Plus className="size-3.5" /> Create new topic | ||
| </Button> | ||
| ); | ||
| }; |
There was a problem hiding this comment.
[code smell]: any particular reason why you're using context to pass ar[ound event handlers like this?
| // A select can't be cleared by deleting text the way inputs can, so without an explicit unset | ||
| // item a value (often seeded by the insert template) is stuck in the YAML forever. Maps to '' | ||
| // which the save path treats as "remove the key". Sentinel because '' can't be an item value. | ||
| const UNSET_OPTION = '__unset__'; |
There was a problem hiding this comment.
[nit]: a lot of these comments discuss business logic but seem to live in the wrong places... like a sentinel like this is self-explanatory based on the constant name... hwo it's used should not be relevant here, rather when rendering it if there are nuances to comment about that would imrpove future contributors' devex.. then comment verbosely
| {invalid ? ( | ||
| <div className="text-body-sm text-destructive"> | ||
| Invalid YAML — these settings won't be saved until fixed. | ||
| </div> | ||
| ) : null} |
| 'sql_server_cdc', | ||
| 'iceberg', | ||
| ]); | ||
| export const KNOWN_MISSING_COMPONENTS: ReadonlySet<string> = new Set([]); |
There was a problem hiding this comment.
[minor]: remove if orphaned? or no use in future?
| // Stamped by enrichComponentsWithConfigSchema from the raw config schema JSON, which carries | ||
| // signals the proto FieldSpec drops (is_secret entirely; required-ness for fields whose | ||
| // non-string default is lost in serialization). Both stay undefined when the dataplane's | ||
| // schema predates flag serialization (benthos < 4.59) — consumers then fall back to proto flags. |
| // Stamps from the raw schema; returns the input untouched when the schema is missing, unparsable, | ||
| // or carries no signal. `required` arrays are the load-bearing signal and benthos has emitted them | ||
| // (computed knowing every default) far longer than the per-field flags (`is_optional`/`is_secret`, | ||
| // 4.59+) — so a mid-generation document still fixes required-ness even though it can't flag | ||
| // secrets (the name heuristic still covers those). A document with no `required` keys at all | ||
| // predates the arrays; stamping `requiredBySchema: false` everywhere from it would be a lie. |
There was a problem hiding this comment.
[nit]: would be nice to stick to jsdoc comments
eblairmckee
left a comment
eblairmckee
left a comment
There was a problem hiding this comment.
lgtm. left some minor nits that can be addressed in a follow
2 participants
RPCN: correct optional-field support, lint-anchored editing, and topic selection in the visual editor
Background
cloudv2 #27724 (merged 2026-07-14) bumped
benthos/v44.53 → 4.73 inredpanda-connect-api, which re-serializes the Connect config schema before serving it. Benthos only began emitting per-fieldis_optional/is_advanced/is_secret/is_deprecatedat v4.59, so until that bump the console never received them and the frontend compensated with a defensive heuristic ladder (checkRequired) that guessed required-ness from proto flags and name patterns.The two RPCs still differ in what they carry (verified against cloudv2 source and the embedded RPCN 4.100 schema):
ListComponents(protoFieldSpec)GetPipelineServiceConfigSchema(raw JSON)optional/advanced/deprecatedis_secret''requiredarrays computed by the backend with full default knowledgeSo required/secret truth must come from the raw JSON path; the proto alone cannot distinguish "required" from "default lost in serialization".
This branch adopts those restored signals end-to-end, then fixes the editing friction they exposed: lint errors referencing invisible line numbers, edits that only saved on node-deselect, and topic fields that were plain text boxes.
What's changed
Schema-signal enrichment
enrichComponentsWithConfigSchema(utils/schema-enrichment.ts) stampsrequiredBySchemaandsecretonto proto-derived specs by walking the raw config-schema JSON (object/array/map wrappers unwrapped so JSON-Schema nodes line up with proto children).requiredarrays (long-standing in the JSON) rather thanis_optional(4.59+ only), so mid-generation dataplanes that serverequiredarrays without per-field flags still get correct required/optional grouping.LINT_REQUIRED_FIELDSoverrides for requirements the schema can't express (input:redpandatopics/consumer_groupare.Optional()in the engine but enforced conditionally by lint) — the form shows them as required instead of contradicting the linter.useEnrichedComponentshook consolidates the recipe for all three YAML-generating consumers (pipeline editor, onboarding wizard, template form panel) and keepscomponentsempty until the schema query settles, so YAML is never generated from un-enriched specs. Display-only consumers (gallery grid, tiles, command palette) intentionally stay on plainparseSchema.Required-field detection
checkRequiredis now: deprecated guard → trustrequiredBySchemastamp → documented degraded proto ladder (for pre-4.59 dataplanes only). Dead heuristic branches deleted.generateDefaultValueno longer zero-fills fields whose default was dropped by the proto — they're omitted so engine defaults apply (previously e.g.auto_replay_nacksdefaulting totruewas rendered asfalse).drop, etc.) generate{}instead of nothing, so they render as canvas nodes and produce valid YAML.Lint errors anchored to form fields
lint-field-mapping.tsmaps a node's lint hints onto the inspector's form fields via three tiers: explicitfield xtokens in the message, the hint's line number resolved through the parsed document's node ranges (deepest field wins, walking up to the nearest rendered ancestor), and loose name mentions (consumer groupfindsconsumer_group).expected object value, got !!null (batching)inside their section — and are hidden while the field is dirty so stale complaints don't fight live edits.Field-level commits
Topic selection on topic fields
topic/topicsfields on Redpanda-family components (kafka,kafka_franz,redpanda*— same gate as the template flow) get cluster-aware pickers: a creatable combobox on scalar fields (existing topics, free text for interpolations/topic:partition) and an append-only picker under the topics textarea that preserves free-text lines.topicslist in place (dedup, comments and non-string entries preserved).mqtt,gcp_pubsub,pulsar, …) stay plain inputs — their topics are not cluster topics.Editor polish
Template fixes (drift caught by this work)
Four templates referenced components/fields the engine never shipped or has since renamed — invisible because the test snapshot was frozen at RPCN 4.65 with the components allowlisted as known-missing:
aws_dynamodb_stream→aws_dynamodb_cdc(tableslist,start_from/snapshot_mode)oracle_cdc→oracledb_cdc(connection_string,include)sql_server_cdc→microsoft_sql_server_cdc(connection_string,include)catalog_uri/warehouse→catalog.url/ optionalcatalog.warehouse— this one was user-visible broken: the component exists, so the template showed in the gallery and its YAML failed dataplane lint on every deployassets/rp-connect-schema-full.json4.65 → RPCN 4.100 (the cloud schema cloudv2 actually serves) also caughtredpanda_migratorhaving dropped itsbatchingfield; removed from the mirror template.KNOWN_MISSING_COMPONENTSis now empty — every template is fully validated by the regression tests (schemaField paths resolve, stitched YAML contains only known fields, required fields always emitted).Docs links module
docs.redpanda.comURLs across the app now live insrc/utils/docs-links.ts(~25 call sites migrated). Every entry is canonical against the restructured docs site (direct 200s, no redirects).Behavior changes to be aware of
seed_brokersis no longer marked required (genuinely.Optional()in the engine);topics/consumer_groupremain required via the curated lint override. The wizard still auto-populates them.requiredvalues were audited against the served schema: templates stricter than the engine (e.g. S3 region/credentials) are deliberate curation; the one looser case (postgres-cdcslot_name) is safe becausedefaultWhenBlankalways supplies a value.topicslist instead of replacing it.