Skip to content

Commit 4747e8d

Browse files
authored
fix: connection editor form values and config display (#288)
- Align S3 config key from force_path_style to use_path_style so the toggle reflects the actual stored value - Strip runtime-injected keys (elicitation, progress_enabled) from connection config API responses so only user-configured values appear - Fall back to config.description for file-only connections where description lives inside the config map - Fix dirty tracking to use the same fallback - Add Default Schema field to Trino connection editor form
1 parent 8e09751 commit 4747e8d

File tree

3 files changed

+51
-13
lines changed

3 files changed

+51
-13
lines changed

pkg/admin/connection_handler.go

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -371,8 +371,15 @@ var connectionSensitiveKeys = []string{
371371
"token", "access_token", "refresh_token", "api_key",
372372
}
373373

374+
// platformInternalKeys lists config keys injected by the platform at runtime
375+
// (e.g., elicitation, progress) that should not be exposed in admin API responses.
376+
var platformInternalKeys = []string{
377+
"elicitation", "progress_enabled",
378+
}
379+
374380
// redactConnectionConfig returns a copy of config with sensitive fields replaced
375-
// by "[REDACTED]". Non-sensitive fields are copied as-is.
381+
// by "[REDACTED]" and platform-internal keys removed. Non-sensitive fields are
382+
// copied as-is.
376383
func redactConnectionConfig(config map[string]any) map[string]any {
377384
if config == nil {
378385
return nil
@@ -384,6 +391,9 @@ func redactConnectionConfig(config map[string]any) map[string]any {
384391
result[key] = redactedValue
385392
}
386393
}
394+
for _, key := range platformInternalKeys {
395+
delete(result, key)
396+
}
387397
return result
388398
}
389399

pkg/admin/connection_handler_test.go

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -508,6 +508,21 @@ func TestRedactConnectionConfig(t *testing.T) {
508508
_ = redactConnectionConfig(config)
509509
assert.Equal(t, "secret", config["password"])
510510
})
511+
512+
t.Run("removes platform-internal keys", func(t *testing.T) {
513+
config := map[string]any{
514+
"host": "trino.local",
515+
"elicitation": map[string]any{"enabled": true},
516+
"progress_enabled": true,
517+
}
518+
519+
redacted := redactConnectionConfig(config)
520+
assert.Equal(t, "trino.local", redacted["host"])
521+
_, hasElicitation := redacted["elicitation"]
522+
assert.False(t, hasElicitation, "elicitation should be removed")
523+
_, hasProgress := redacted["progress_enabled"]
524+
assert.False(t, hasProgress, "progress_enabled should be removed")
525+
})
511526
}
512527

513528
func TestMergeRedactedFields(t *testing.T) {

ui/src/pages/settings/ConnectionsPanel.tsx

Lines changed: 25 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -430,7 +430,9 @@ function ConnectionEditor({ connection, onSave, onCancel, onDirtyChange }: Edito
430430
const setMutation = useSetConnectionInstance();
431431
const [kind, setKind] = useState(connection?.kind ?? "trino");
432432
const [name, setName] = useState(connection?.name ?? "");
433-
const [description, setDescription] = useState(connection?.description ?? "");
433+
const [description, setDescription] = useState(
434+
connection?.description || (connection?.config?.description as string) || "",
435+
);
434436
const [configObj, setConfigObj] = useState<Record<string, any>>(
435437
connection?.config ? { ...connection.config } : {},
436438
);
@@ -443,9 +445,10 @@ function ConnectionEditor({ connection, onSave, onCancel, onDirtyChange }: Edito
443445
if (isCreate) {
444446
onDirtyChange(!!name.trim());
445447
} else {
448+
const origDesc = connection?.description || (connection?.config?.description as string) || "";
446449
const origJson = JSON.stringify(connection?.config ?? {});
447450
onDirtyChange(
448-
description !== (connection?.description ?? "") || configJson !== origJson,
451+
description !== origDesc || configJson !== origJson,
449452
);
450453
}
451454
}, [kind, name, description, configJson, connection, isCreate, onDirtyChange]);
@@ -745,14 +748,24 @@ function TrinoConfigForm({ config, onChange }: ConfigFormProps) {
745748
help="Leave blank to keep existing password"
746749
/>
747750
</div>
748-
<ConfigField
749-
label="Default Catalog"
750-
value={String(config.catalog ?? "")}
751-
onChange={(v) => onChange(update(config, "catalog", v))}
752-
placeholder="iceberg"
753-
mono
754-
help="Default Trino catalog for queries (e.g. iceberg, hive, memory)"
755-
/>
751+
<div className="grid grid-cols-2 gap-4">
752+
<ConfigField
753+
label="Default Catalog"
754+
value={String(config.catalog ?? "")}
755+
onChange={(v) => onChange(update(config, "catalog", v))}
756+
placeholder="iceberg"
757+
mono
758+
help="Default Trino catalog for queries (e.g. iceberg, hive, memory)"
759+
/>
760+
<ConfigField
761+
label="Default Schema"
762+
value={String(config.schema ?? "")}
763+
onChange={(v) => onChange(update(config, "schema", v))}
764+
placeholder="public"
765+
mono
766+
help="Default Trino schema within the catalog"
767+
/>
768+
</div>
756769
<ConfigToggle
757770
label="SSL / TLS"
758771
checked={!!config.ssl}
@@ -836,8 +849,8 @@ function S3ConfigForm({ config, onChange }: ConfigFormProps) {
836849
</div>
837850
<ConfigToggle
838851
label="Force Path Style"
839-
checked={!!config.force_path_style}
840-
onChange={(v) => onChange(update(config, "force_path_style", v))}
852+
checked={!!config.use_path_style}
853+
onChange={(v) => onChange(update(config, "use_path_style", v))}
841854
help="Use path-style URLs (bucket in path, not subdomain). Required for MinIO and most S3-compatible stores."
842855
/>
843856
<div className="border-t pt-4 mt-2">

0 commit comments

Comments
 (0)