From 7cfc0ea361a435b77430f9b17ffd58b0ad688b10 Mon Sep 17 00:00:00 2001 From: isliao613 Date: Fri, 5 Jun 2026 11:33:04 +0800 Subject: [PATCH] fix(connect): handle null value sections in connector config validation Debezium Oracle 3.5.x returns null value sections for deprecated XStream properties (e.g. database.out.server.name). Three crash sites fixed across the connector UI: - createCustomProperties in state.ts: filter null-value entries before mapping, add errors ?? [] guard - validate() in state.ts: null guards on recommended_values and errors before isEqual comparisons - transitionConditionMet and getDataSource in create-connector.tsx: value?.errors ?? [] guard on errorCount sum and filter Also updates ConnectorProperty type to allow value: null, adds ConnectorPropertyWithValue predicate type, and adds unit tests covering all three null-value cases. --- .../pages/connect/create-connector.test.ts | 115 ++++++++++++++++++ .../pages/connect/create-connector.tsx | 6 +- .../connect/dynamic-ui/property-component.tsx | 2 +- frontend/src/state/connect/state.test.ts | 107 +++++++++++++++- frontend/src/state/connect/state.ts | 29 +++-- frontend/src/state/rest-interfaces.ts | 18 ++- 6 files changed, 257 insertions(+), 20 deletions(-) create mode 100644 frontend/src/components/pages/connect/create-connector.test.ts diff --git a/frontend/src/components/pages/connect/create-connector.test.ts b/frontend/src/components/pages/connect/create-connector.test.ts new file mode 100644 index 0000000000..99b1961a9a --- /dev/null +++ b/frontend/src/components/pages/connect/create-connector.test.ts @@ -0,0 +1,115 @@ +/** + * Copyright 2026 Redpanda Data, Inc. + * + * Use of this software is governed by the Business Source License + * included in the file https://github.com/redpanda-data/redpanda/blob/dev/licenses/bsl.md + * + * As of the Change Date specified in that file, in accordance with + * the Business Source License, use of this software will be governed + * by the Apache License, Version 2.0 + */ + +import { describe, expect, it } from 'vitest'; + +import type { ConnectorProperty, ConnectorValidationResult } from '../../../state/rest-interfaces'; +import { DataType, PropertyImportance, PropertyWidth } from '../../../state/rest-interfaces'; +import { getDataSource } from './create-connector'; + +function makeProperty(name: string, errors: string[]): ConnectorProperty { + return { + definition: { + name, + type: DataType.String as ConnectorProperty['definition']['type'], + required: false, + default_value: null, + importance: PropertyImportance.High, + documentation: '', + width: PropertyWidth.Medium, + display_name: name, + dependents: [], + order: 0, + }, + value: { + name, + value: null, + recommended_values: [], + errors, + visible: true, + }, + metadata: {}, + }; +} + +function makeNullValueProperty(name: string): ConnectorProperty { + return { + definition: { + name, + type: DataType.String as ConnectorProperty['definition']['type'], + required: false, + default_value: null, + importance: PropertyImportance.High, + documentation: '', + width: PropertyWidth.Medium, + display_name: name, + dependents: [], + order: 0, + }, + value: null, + metadata: {}, + }; +} + +function makeValidationResult(configs: ConnectorProperty[]): ConnectorValidationResult { + return { + name: 'test-connector', + configs, + steps: [], + }; +} + +describe('getDataSource', () => { + it('returns only configs that have errors', () => { + const result = makeValidationResult([ + makeProperty('topic.prefix', ['required field']), + makeProperty('database.hostname', []), + ]); + + const rows = getDataSource(result); + expect(rows).toHaveLength(1); + expect(rows[0]?.name).toBe('topic.prefix'); + }); + + it('does not throw when value is null (Debezium Oracle 3.5.x deprecated properties)', () => { + const result = makeValidationResult([ + makeProperty('topic.prefix', ['required field']), + makeNullValueProperty('database.out.server.name'), + ]); + + expect(() => getDataSource(result)).not.toThrow(); + }); + + it('excludes null-value configs from results even when mixing with erroring configs', () => { + const result = makeValidationResult([ + makeProperty('topic.prefix', ['required field']), + makeNullValueProperty('database.out.server.name'), + makeProperty('database.hostname', []), + ]); + + const rows = getDataSource(result); + expect(rows).toHaveLength(1); + expect(rows[0]?.name).toBe('topic.prefix'); + }); + + it('returns empty array when all configs have no errors', () => { + const result = makeValidationResult([ + makeProperty('topic.prefix', []), + makeProperty('database.hostname', []), + ]); + + expect(getDataSource(result)).toHaveLength(0); + }); + + it('returns empty array when configs list is empty', () => { + expect(getDataSource(makeValidationResult([]))).toHaveLength(0); + }); +}); diff --git a/frontend/src/components/pages/connect/create-connector.tsx b/frontend/src/components/pages/connect/create-connector.tsx index 370460be91..cb16bd1dec 100644 --- a/frontend/src/components/pages/connect/create-connector.tsx +++ b/frontend/src/components/pages/connect/create-connector.tsx @@ -439,7 +439,7 @@ const ConnectorWizard = ({ connectClusters, activeCluster }: ConnectorWizardProp propertiesObject ?? {} ); - const errorCount = validationResult.configs.sum((x) => x.value.errors.length); + const errorCount = validationResult.configs.sum((x) => (x.value?.errors ?? []).length); if (errorCount > 0) { setInvalidValidationResult(validationResult); @@ -693,9 +693,9 @@ function Review({ ); } -function getDataSource(validationResult: ConnectorValidationResult) { +export function getDataSource(validationResult: ConnectorValidationResult) { return validationResult.configs - .filter((connectorProperty) => connectorProperty.value.errors.length > 0) + .filter((connectorProperty) => (connectorProperty.value?.errors ?? []).length > 0) .map((cp) => cp.value); } diff --git a/frontend/src/components/pages/connect/dynamic-ui/property-component.tsx b/frontend/src/components/pages/connect/dynamic-ui/property-component.tsx index dc651f6240..a233f3829a 100644 --- a/frontend/src/components/pages/connect/dynamic-ui/property-component.tsx +++ b/frontend/src/components/pages/connect/dynamic-ui/property-component.tsx @@ -51,7 +51,7 @@ export const PropertyComponent = (props: { property: Property }) => { switch (def.type) { case 'STRING': case 'CLASS': { - const recValues = p.entry.value.recommended_values; + const recValues = p.entry.value.recommended_values ?? []; if (metadata?.component_type === 'RADIO_GROUP') { const options = metadata.recommended_values && metadata.recommended_values?.length > 0 diff --git a/frontend/src/state/connect/state.test.ts b/frontend/src/state/connect/state.test.ts index 5e1d460ab1..3a7b880d4c 100644 --- a/frontend/src/state/connect/state.test.ts +++ b/frontend/src/state/connect/state.test.ts @@ -31,6 +31,7 @@ function createMockProperty(overrides: { custom_default_value?: string; value?: string | null; required?: boolean; + errors?: string[]; }): ConnectorProperty { return { definition: { @@ -50,13 +51,35 @@ function createMockProperty(overrides: { name: overrides.name, value: overrides.value ?? null, recommended_values: [], - errors: [], + errors: overrides.errors ?? [], visible: true, }, metadata: {}, }; } +// Creates a ConnectorProperty whose entire value section is null. +// Kafka Connect returns this for deprecated/inapplicable properties, +// e.g. database.out.server.name in Debezium Oracle 3.5.x (LogMiner mode). +function createNullValueProperty(name: string): ConnectorProperty { + return { + definition: { + name, + type: DataType.String as ConnectorProperty['definition']['type'], + required: false, + default_value: null, + importance: PropertyImportance.High, + documentation: '', + width: PropertyWidth.Medium, + display_name: name, + dependents: [], + order: 0, + }, + value: null, + metadata: {}, + }; +} + function createMockValidationResult(properties: ConnectorProperty[]): ConnectorValidationResult { return { name: 'test-connector', @@ -219,6 +242,88 @@ describe('ConnectorPropertiesStore', () => { }); }); + describe('null value section handling', () => { + // Regression tests for Debezium Oracle 3.5.x returning "value": null for + // deprecated XStream properties (e.g. database.out.server.name). + + it('excludes properties with a null value section from initConfig', async () => { + const properties = [ + createMockProperty({ name: 'topic.prefix', value: 'cdc_oracle' }), + createNullValueProperty('database.out.server.name'), + ]; + + mockValidateConnectorConfig.mockResolvedValue(createMockValidationResult(properties)); + + const store = new ConnectorPropertiesStore('test-cluster', 'io.example.Connector', 'source', undefined); + await vi.waitFor(() => expect(store.initPending).toBe(false)); + + // The null-value property must be absent; the valid one must be present. + expect(store.propsByName.has('database.out.server.name')).toBe(false); + expect(store.propsByName.has('topic.prefix')).toBe(true); + }); + + it('does not throw when validate() response contains a null value section', async () => { + // initConfig: only valid properties + mockValidateConnectorConfig.mockResolvedValue( + createMockValidationResult([createMockProperty({ name: 'topic.prefix', value: 'cdc_oracle' })]) + ); + + const store = new ConnectorPropertiesStore('test-cluster', 'io.example.Connector', 'source', undefined); + await vi.waitFor(() => expect(store.initPending).toBe(false)); + + // validate(): mix of valid and null-value property + mockValidateConnectorConfig.mockResolvedValue( + createMockValidationResult([ + createMockProperty({ name: 'topic.prefix', value: 'cdc_oracle', errors: ['required field'] }), + createNullValueProperty('database.out.server.name'), + ]) + ); + + await expect(store.validate({ 'topic.prefix': 'cdc_oracle' })).resolves.not.toThrow(); + + expect(store.propsByName.has('database.out.server.name')).toBe(false); + // Errors reported by the validate response should be reflected on the property. + expect(store.propsByName.get('topic.prefix')?.showErrors).toBe(true); + }); + + it('does not throw when validate() response contains null recommended_values or errors', async () => { + mockValidateConnectorConfig.mockResolvedValue( + createMockValidationResult([createMockProperty({ name: 'topic.prefix', value: 'cdc_oracle' })]) + ); + + const store = new ConnectorPropertiesStore('test-cluster', 'io.example.Connector', 'source', undefined); + await vi.waitFor(() => expect(store.initPending).toBe(false)); + + // validate(): property with null recommended_values and null errors inside value + const propertyWithNullFields: ConnectorProperty = { + definition: { + name: 'topic.prefix', + type: DataType.String as ConnectorProperty['definition']['type'], + required: false, + default_value: null, + importance: PropertyImportance.High, + documentation: '', + width: PropertyWidth.Medium, + display_name: 'topic.prefix', + dependents: [], + order: 0, + }, + value: { + name: 'topic.prefix', + value: 'cdc_oracle', + recommended_values: null, + errors: null, + visible: true, + }, + metadata: {}, + }; + + mockValidateConnectorConfig.mockResolvedValue(createMockValidationResult([propertyWithNullFields])); + + await expect(store.validate({ 'topic.prefix': 'cdc_oracle' })).resolves.not.toThrow(); + }); + }); + describe('SecretsStore', () => { it('filters out secrets with empty values', () => { const store = new SecretsStore(); diff --git a/frontend/src/state/connect/state.ts b/frontend/src/state/connect/state.ts index f648289650..acfd90b7dd 100644 --- a/frontend/src/state/connect/state.ts +++ b/frontend/src/state/connect/state.ts @@ -18,6 +18,7 @@ import { type ConnectorGroup, type ConnectorPossibleStatesLiteral, type ConnectorProperty, + type ConnectorPropertyWithValue, type ConnectorStep, DataType, PropertyImportance, @@ -762,9 +763,9 @@ export class ConnectorPropertiesStore { } // Update: recommended values - const suggestedSrc = source.entry.value.recommended_values; + const suggestedSrc = source.entry.value.recommended_values ?? []; const suggestedTar = target.entry.value.recommended_values; - if (!suggestedSrc.isEqual(suggestedTar)) { + if (suggestedTar != null && !suggestedSrc.isEqual(suggestedTar)) { suggestedTar.updateWith(suggestedSrc); } @@ -772,28 +773,31 @@ export class ConnectorPropertiesStore { target.entry.value.visible = source.entry.value.visible; // Update: errors - if (!target.errors.isEqual(source.errors)) { - if (source.errors.length > 0) { - target.lastErrors = [...source.errors]; // create copy + const targetErrors = target.errors ?? []; + const sourceErrors = source.errors ?? []; + if (!targetErrors.isEqual(sourceErrors)) { + if (sourceErrors.length > 0) { + target.lastErrors = [...sourceErrors]; // create copy } // Update - target.errors.updateWith(source.errors); + targetErrors.updateWith(sourceErrors); + target.errors = targetErrors; - target.showErrors = target.errors.length > 0; + target.showErrors = targetErrors.length > 0; // Show first error target.currentErrorIndex = 0; - if (target.errors.length > 1) { + if (targetErrors.length > 1) { // Skip over simple / unhelpful messages - const betterStartValue = target.errors.findIndex((x) => !x.includes('which has no default value')); + const betterStartValue = targetErrors.findIndex((x) => !x.includes('which has no default value')); if (betterStartValue > -1) { target.currentErrorIndex = betterStartValue; } } // Add or remove from parent group - const hasErrors = target.errors.length > 0; + const hasErrors = targetErrors.length > 0; if (hasErrors) { target.propertyGroup.propertiesWithErrors.pushDistinct(target); } else { @@ -849,6 +853,7 @@ export class ConnectorPropertiesStore { // Create our own properties const allProps = properties + .filter((p): p is ConnectorPropertyWithValue => p.value != null) // skip properties the API returned with a null value section .map((p) => { const name = p.definition.name; const definitionType = p.definition.type; @@ -865,7 +870,7 @@ export class ConnectorPropertiesStore { isHidden: hiddenProperties.includes(name), errors: p.value.errors ?? [], lastErrors: [], - showErrors: p.value.errors.length > 0, + showErrors: (p.value.errors ?? []).length > 0, currentErrorIndex: 0, lastErrorValue: undefined as unknown, propertyGroup: undefined as unknown as PropertyGroup, @@ -938,7 +943,7 @@ export type PropertyGroup = { export type Property = { name: string; - entry: ConnectorProperty; + entry: ConnectorPropertyWithValue; value: null | string | number | boolean | string[]; isHidden: boolean; // currently only used for "connector.class" errors: string[]; // current errors diff --git a/frontend/src/state/rest-interfaces.ts b/frontend/src/state/rest-interfaces.ts index b3f9e01fd9..bb9ce4f1fc 100644 --- a/frontend/src/state/rest-interfaces.ts +++ b/frontend/src/state/rest-interfaces.ts @@ -1161,19 +1161,31 @@ export type ConnectorProperty = { // added by backend custom_default_value?: string; }; + // Kafka Connect may return null for deprecated/inapplicable properties (e.g. + // database.out.server.name in Debezium Oracle 3.5.x when using LogMiner). value: { name: string; value: null | string; - recommended_values: string[]; - errors: string[]; + // Kafka Connect may return null for these fields under some conditions + recommended_values: string[] | null; + errors: string[] | null; visible: boolean; - }; + } | null; metadata: { component_type?: 'RADIO_GROUP'; recommended_values?: ConnectorRecommendedValueEntry[]; }; }; +/** + * A ConnectorProperty whose value section is guaranteed non-null. + * All Property objects produced by createCustomProperties() satisfy this + * because null-value entries are filtered out before the map. + */ +export type ConnectorPropertyWithValue = ConnectorProperty & { + value: NonNullable; +}; + export const PropertyImportance = { Low: 'LOW', Medium: 'MEDIUM',