GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — [connectors,types,manager] Keep connector credentials out of diagnostics by bestouff · Pull Request #7786 · feldera/feldera

Conversation

@bestouff

@bestouff bestouff commented Nov 2, 2026

Copy link
Copy Markdown
Contributor

Linked Issue

Part of feldera/cloud#2136 (connector credentials in errors, logs and support bundles). PR 1 of 3: stop the known emission sites. PR 2 adds a redaction net for resolved secret references, PR 3 scrubs stored diagnostics in the pipeline manager.

A read-role user could read a connector password from deployment_error, the pipeline log, the monitor events and the support bundle. The password got there through the connector's own error and log text, also when the user supplied it as a Kubernetes Secret reference, because the connector sees the resolved value.

One commit per site:

Site Before After
Postgres input connect error and log lines whole URI host:port/db from tokio_postgres::Config
Pipeline startup summary (before the logger is up) whole configuration one line per endpoint, allowlisted runtime fields, env variable names
Connector compile errors in the manager the rejected property value [REDACTED], category and corrective action, positions kept
Delta input and output errors and spans whole table URI URI without userinfo and query string
Kafka client option rejected by librdkafka option value in the error text option name, value masked
Format parse errors in adapterlib the format configuration dump and serde's rejected scalar [REDACTED], endpoint and error code kept
Debug prints of Kafka, Avro and controller configs {:?} of the resolved config endpoint, topic and transport names; safe Debug impls that destructure every field

The helper commit adds feldera_types::display_url (url_without_credentials, text_without_url, REDACTED) and the safe configuration error categories.

Describe Manual Test Plan

  • Each new unit test was broken in turn by reverting its fix and failed, then passed again after the restore.
  • cargo fmt --all --check and clippy on feldera-types, adapterlib, connectors, observability and pipeline-manager are clean.
  • Unit suites of the five crates pass in the feldera-dev container. The Delta test drives the real ensure_table_uri failure with credentials in the URI.
  • Not done yet: reproducing the original finding on a Kubernetes deployment with a Secret-backed Postgres URI; planned together with PR 2.

Checklist

  • Unit tests added/updated
  • Integration tests added/updated
  • Documentation updated
  • Changelog updated

Breaking Changes?

Mark if you think the answer is yes for any of these components:

  • OpenAPI / REST HTTP API / feldera-types / manager (What is a breaking change?)
  • Feldera SQL (Syntax, Semantics)
  • feldera-sqllib (incl. dependencies fxp, etc.) (What is a breaking change?)
  • Python SDK (What is a breaking change?)
  • fda (CLI arguments)
  • Adapters (including configuration)
  • Storage Format / Checkpoints
  • Others (specify)

Describe Incompatible Changes

Error and log texts change: they no longer contain the connection URI, the rejected option value or the configuration dump. No configuration or API field changes.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@bestouff bestouff mentioned this pull request Nov 2, 2026
[types,connectors] Show resolved secrets in diagnostics as references #7787
Open
mythical-fred-oss[bot]
mythical-fred-oss Bot reviewed Nov 2, 2026
View reviewed changes
Comment thread crates/feldera-types/src/config_diagnostics.rs Resolved

Uh oh!

There was an error while loading. Please reload this page.

mythical-fred-oss[bot]
mythical-fred-oss Bot reviewed Nov 2, 2026
View reviewed changes
Comment thread crates/connectors/src/format/avro/output.rs Resolved

Uh oh!

There was an error while loading. Please reload this page.

mythical-fred-oss[bot]
mythical-fred-oss Bot reviewed Nov 2, 2026
View reviewed changes
Comment thread crates/feldera-types/src/display_url.rs Resolved

Uh oh!

There was an error while loading. Please reload this page.

mythical-fred-oss[bot]
mythical-fred-oss Bot reviewed Nov 2, 2026
View reviewed changes
Comment thread crates/connectors/src/transport/kafka.rs Resolved

Uh oh!

There was an error while loading. Please reload this page.

@mythical-fred-oss

mythical-fred-oss Bot commented Nov 2, 2026

Copy link
Copy Markdown
Contributor

What I ran

Check Result
cargo test -p feldera-types (270 lib tests) ✅
cargo test -p feldera-adapterlib format_errors ✅
cargo test -p dbsp_connectors with the 6 new tests (Kafka, Delta, JSON output, Postgres input) ✅
cargo test -p pipeline-manager connectors_ ✅
pre-commit run on the changed files ✅
Probe of url_without_credentials and deserialization_error gaps found, see inline comments

Gate failed (high): actionable errors. The format-config and connectors compile errors no longer name the field, the missing option, or the allowed values. That is a regression for every user who mistypes an option, and it is avoidable because those parts come from the schema, not from user input. See the inline comment on config_diagnostics.rs. I'd fix this before merge.

Missing tests (medium):

  • URL edge cases: a scheme-less user:pw@host string, a #fragment, and Azure container@account URIs.
  • A Kafka secret shorter than 4 bytes.
  • A test that the Avro schema error still carries the parser's reason, once that is fixed.
  • Asserting what the errors still say, e.g. the field name for an unknown or missing field. The current tests mostly assert !contains("hunter2") and contains("check"), so they would pass even if the message were empty of useful detail.

Observation (medium): the "Diagnostics" table in secret-references.md says connection errors show "the endpoint name and option names only". Other connectors are not covered yet (Postgres CDC and output, Redis, Iceberg, and others), so the table promises more than this PR delivers. Please scope the wording to the connectors fixed here until PR 2 lands.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@bestouff
bestouff marked this pull request as draft November 2, 2026 19:47
@bestouff
bestouff added this pull request to stack #7791 November 2, 2026 20:09
@bestouff
bestouff force-pushed the cloud-2136-diagnostics branch 4 times, most recently from d0c965f to 9cb42fc Compare November 3, 2026 17:49
@bestouff
bestouff marked this pull request as ready for review November 3, 2026 18:44
@bestouff
bestouff requested a review from mihaibudiu November 3, 2026 18:44
mythical-fred-oss[bot]
mythical-fred-oss Bot reviewed Nov 3, 2026
View reviewed changes
Comment thread crates/connectors/src/integrated/delta_table.rs Resolved

Uh oh!

There was an error while loading. Please reload this page.

mythical-fred-oss[bot]
mythical-fred-oss Bot reviewed Nov 3, 2026
View reviewed changes
Comment thread crates/feldera-types/src/config_diagnostics.rs Resolved

Uh oh!

There was an error while loading. Please reload this page.

@mythical-fred-oss

mythical-fred-oss Bot commented Nov 3, 2026

Copy link
Copy Markdown
Contributor

What I ran (debug, --locked)

Command Result
cargo test -p feldera-types ✅ all pass
cargo test -p feldera-adapterlib format_errors ✅
cargo test -p dbsp_connectors --lib (the PR's new Kafka, Delta, Postgres, JSON, Avro and server tests, 9 in total) ✅
cargo test -p pipeline-manager --lib (the 2 new connectors property tests) ✅
QA probe: Delta library error that quotes the normalized URI ❌ the credentials leak (see inline comment)

Findings

  1. bug (high): Delta errors still leak credentials when delta-rs quotes the normalized URL. Details are inline. This PR's goal is to stop the known leak sites, so I would fix this before merge.
  2. missing (medium): Nothing covers URI variants that the library rewrites: a trailing slash before ?query, an upper-case scheme, // in the path. Please add the normalized-URI case as a regression test.
  3. pitfall (medium): Format config errors no longer say which field failed (inline). That goes against the actionable-errors rule.

Gates: the unit tests, the docs and changelog updates, and the commit messages are fine. No unsafe code, no new third-party dependencies (url is already a workspace crate), no CI changes. The PR's own test plan says the Kubernetes reproduction with a Secret-backed Postgres URI has not been done yet. That end-to-end check should happen before the series is called done.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@bestouff
bestouff force-pushed the cloud-2136-diagnostics branch from 9cb42fc to 3968c3f Compare November 3, 2026 20:27
@mihaibudiu

mihaibudiu commented Nov 4, 2026

Copy link
Copy Markdown
Contributor

I haven't yet reviewed everything, because I thought of the following:

Can you actually use the type system to make sure errors are always sanitized?
I would define a new type SanitizedError which wraps a string and require controllers to only accept sanitized errors. This way you cannot accidentally pass a string.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@bestouff

bestouff commented Nov 4, 2026

Copy link
Copy Markdown
Contributor Author

I guess you thought the stack was too shallow; let's add a PR on top of it !

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

mihaibudiu
mihaibudiu reviewed Nov 4, 2026
View reviewed changes
Comment thread crates/adapterlib/src/errors/controller.rs Outdated
ParserConfigParseError {
endpoint_name: String,
error: String,
/// Always `[REDACTED]`. A format config can hold credentials, so

@mihaibudiu mihaibudiu Nov 3, 2026

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this mean that this string is always exactly [REDACTED], or that it is obtained from a redaction algorithm of the real configuration?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@bestouff bestouff Nov 5, 2026

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Always [REDACTED] - in fact, it's a constant defined at the start of crates/feldera-types/src/display_url.rs

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Comment thread crates/adapterlib/src/errors/controller.rs Outdated
config: config.to_string(),
error: feldera_types::config_diagnostics::deserialization_error(&error.to_string())
.to_string(),
config: REDACTED.to_owned(),

@mihaibudiu mihaibudiu Nov 3, 2026

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This I don't understand. Why carry the field if it's a constant?
Is the error supposed to quote the part that is wrong?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@bestouff bestouff Nov 5, 2026

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Yes, good point, dropping it.
  2. The error is supposed to name the part that is wrong but not show its value; that's what feldera_types::config_diagnostics::deserialization_error does - even if, yes, it parses the serde error to do that, so it parses English because there's no structured way of getting to serde errors.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Comment thread crates/adapterlib/src/errors/controller.rs Outdated
}
}

/// The safe description of `error`, with the field it points at.

@mihaibudiu mihaibudiu Nov 3, 2026

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what makes it "Safe"?
Should the function have "Safe" in its name?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@bestouff bestouff Nov 5, 2026

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Safe" because it's constructed from its schema and name, not from the input. I'll expand on it.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@bestouff

bestouff commented Nov 4, 2026

Copy link
Copy Markdown
Contributor Author

How would this work if someone writes

error!("postgres {name}: connection error: {e}");

?

The type is useless here. There's no SanitizedError in sight AFAIK.
I guess we have to filter everything at the writer step.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@mihaibudiu

mihaibudiu commented Nov 4, 2026

Copy link
Copy Markdown
Contributor

How would this work if someone writes

error!("postgres {name}: connection error: {e}");

?

The type is useless here. There's no SanitizedError in sight AFAIK. I guess we have to filter everything at the writer step.

The sensitive part is e, so one should not be allowed to write this unless e is a SanitizedError.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@bestouff

bestouff commented Nov 5, 2026

Copy link
Copy Markdown
Contributor Author

Yeah that's not possible. error!() uses format!() under the hood, and this will print any type that implements Display. All errors implement Display - or at least, we can't control if an error type implements it or not.

What we could do (I know, one more layer):

  1. Wrap at the boundary, and let Display be the sanitized form (the legal trait).
  /// A library error as the pipeline may show it. The raw error stays inside;
  /// its text goes through the structural scrubs and the registry.
  pub struct LibraryError(Box<dyn std::error::Error + Send + Sync>);
 
  impl fmt::Display for LibraryError {
      fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
          f.write_str(&sanitize_library_text(&self.0.to_string()))
      }
  }   
  // Debug prints the same text, so `{e:?}` is safe too.

Then the log line at crates/connectors/src/integrated/postgres/input.rs:484 stays exactly as written and becomes safe, because e is a LibraryError:

  // before: conn is Result<(), tokio_postgres::Error>, e is the raw driver error
  fn spawn_pg_connection_monitor(name: String,
      conn: impl Future<Output = Result<(), tokio_postgres::Error>> + Send + 'static)
 
  // after: the raw type does not cross a signature
  fn spawn_pg_connection_monitor(name: String,
      conn: impl Future<Output = Result<(), LibraryError>> + Send + 'static)

The raw tokio_postgres::Error then exists only inside the one expression that wraps it, connect().await.map_err(LibraryError::from), not in any variable a log line can reach.

  1. Machnically forbid specific errors with clippy's disallowed_types. We would list the library error types in clippy.toml:
  disallowed-types = [
    "tokio_postgres::Error",
    "rdkafka::error::KafkaError",
    "deltalake::DeltaTableError",
  ] 

Every explicit mention of those types in our crates then fails CI. In that same file, the closure at line 472, |e: tokio_postgres::Error|, and the future type at line 481 are both explicit mentions, so both would be flagged, which is the point: they are the two places where the raw error is handed around today.

There would still be places like .map_err(|e| ...) (used to transform a specific error) which would need some manual #[allow] to pass CI, but I don't expect these to be a lot.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@bestouff
bestouff force-pushed the cloud-2136-diagnostics branch 2 times, most recently from 1ad643a to aaf46f9 Compare November 5, 2026 17:00
mihaibudiu
mihaibudiu approved these changes Nov 6, 2026
View reviewed changes
Comment thread crates/connectors/src/integrated/delta_table/output.rs
"delta_output",
endpoint = &*self.inner.endpoint_name,
table = &*self.inner.config.uri,
table = %self.inner.config.display_uri(),

@mihaibudiu mihaibudiu Nov 6, 2026

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these macros do the damndest things

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@bestouff bestouff Nov 12, 2026

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In tracing's info_span!, %expr records the field with Display (and ?expr with Debug). Before, the span recorded &*config.uri, the raw URI with its credentials. Now it records display_uri(), the URI without userinfo, query and fragment.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Comment thread crates/connectors/src/integrated/postgres/input.rs Outdated
}
});
}
/// Open a postgres connection for `config`, optionally with TLS.

@mihaibudiu mihaibudiu Nov 6, 2026

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can't really tell where the TLS bit is coming from

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@bestouff bestouff Nov 12, 2026 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

make_tls_connector(&config.tls) returns Some only when a CA certificate is configured.
I'll rewrite this.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Comment thread crates/connectors/src/integrated/postgres/input.rs
.unwrap_or_default();
debug!("postgres {endpoint_name}: opening connection{target}");

let connector = make_tls_connector(&config.tls, endpoint_name).map_err(|e| {

@mihaibudiu mihaibudiu Nov 6, 2026

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this must be the bit. But why single it in the docs?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@bestouff bestouff Nov 12, 2026

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because it is the only branch in the function: a TLS and a plain connect. The reworded comment says what decides it.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Comment thread crates/connectors/src/integrated/postgres/input.rs
use super::*;
use feldera_types::transport::postgres::PostgresTlsConfig;

const PASSWORD: &str = "hunter2-do-not-print";

@mihaibudiu mihaibudiu Nov 6, 2026

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hm. Is this Hunter Biden?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

bestouff reacted with laugh emoji
All reactions
  • 😄 1 reaction

@bestouff bestouff Nov 12, 2026

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like you're one of the lucky 10000 today: https://knowyourmeme.com/memes/hunter2

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@mihaibudiu mihaibudiu Nov 12, 2026

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I learned 2 more things now.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Comment thread crates/connectors/src/transport/kafka.rs Outdated
match error {
KafkaError::ClientConfig(_, description, key, value) => {
// librdkafka quotes the value in its description. The quoted
// token goes whatever its length; a bare copy of a longer value

@mihaibudiu mihaibudiu Nov 6, 2026

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

goes whatever it's length?
can it go in any other way?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@mihaibudiu mihaibudiu Nov 6, 2026

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so this replaces ALL values? Isn't this a little extreme?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@bestouff bestouff Nov 12, 2026

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only one value is replaced: the value of the option that librdkafka rejected. No other option appears in this error. "Goes" means it is replaced. librdkafka quotes that value, so the quoted copy is always replaced. A simple copy is also replaced when it is 4 bytes or longer. A shorter one is skipped: with a value like on, replacing every bare on would mangle words like configuration. Will be rewritten.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Comment thread crates/feldera-types/src/format/avro.rs
.map(|url| url_without_credentials(url))
.collect();
let header_names: Vec<&str> = registry_headers.keys().map(String::as_str).collect();
f.debug_struct("AvroSchemaRegistryConfig")

@mihaibudiu mihaibudiu Nov 6, 2026

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is maybe misleading, since it does not include all the information. should the output say that?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@bestouff bestouff Nov 12, 2026

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Credential fields already show [REDACTED], but the headers show as a bare list of names. I'll print them as {name: "[REDACTED]"}, so the output says that a value exists and is hidden.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Comment thread crates/pipeline-manager/src/db/types/program.rs
let properties = std::collections::BTreeMap::from([(
"connectors".to_string(),
PropertyValue {
value: value.to_string(),

@mihaibudiu mihaibudiu Nov 6, 2026

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we have to do this in the compiler too?
Maybe one of your subsequent PRs does?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@bestouff bestouff Nov 12, 2026

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, probably.
I'll stack one more PR for the road.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@bestouff bestouff Nov 12, 2026 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(kidding, it'll be 1 more commit: the parse error will no longer quote the rejected token)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Comment thread docs.feldera.com/docs/connectors/secret-references.md

The following limits apply:

- A value that is part of SQL, UDF code, an endpoint name, or table data is

@mihaibudiu mihaibudiu Nov 6, 2026

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

connector configuration is part of SQL, so that sounds wrong.
Moreover, compiler warnings may include connector configuration, which can be saved into bundles.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

bestouff reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
Comment thread crates/feldera-types/src/transport/kafka.rs Outdated
oauth_provider,
} = self;
// Header values are user data and can be credentials too.
let header_keys: Vec<&str> = headers.iter().map(|h| h.key.as_str()).collect();

@mihaibudiu mihaibudiu Nov 6, 2026

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same question about this being incomplete

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

bestouff reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
Comment thread crates/feldera-types/src/config_diagnostics.rs
/// fields, the name of a missing or repeated field, and the position that
/// serde_json appends. Callers supply the endpoint or source position
/// separately.
pub fn deserialization_error(error: &impl std::fmt::Display) -> String {

@mihaibudiu mihaibudiu Nov 6, 2026

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hard for me to figure out if this is missing important parterns...

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@bestouff bestouff Nov 12, 2026

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The patterns that won't be recognized will fail with "could not be parsed; check its syntax and documented options". So it can miss details but not leak a value.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@bestouff
bestouff force-pushed the cloud-2136-diagnostics branch from aaf46f9 to ed84b9c Compare November 12, 2026 12:51
bestouff added 2 commits November 12, 2026 14:07
@bestouff
[types] Add helpers for safe configuration diagnostics
79e5796
Connection URLs and deserializer messages can include credentials. Give
callers helpers that remove URL userinfo, query strings and fragments,
and describe configuration failures without copying rejected values or
map keys. The latter keeps the error category and the schema-defined
parts of serde's message, such as the expected values; callers supply
endpoint identity and source positions separately.

Test URL credentials, repeated URLs, rejected scalars and fields, and
custom deserializer errors. The following commits use these helpers at
the emission sites.

Related: feldera/cloud#2136

Signed-off-by: Xavier Bestel <[email protected]>
@bestouff
[connectors] Keep the Postgres URI out of the input's diagnostics
6634416
The Postgres input connector put the resolved connection URI into its
connect error and into two log lines. The URI holds the password, also
when the user supplied it as a Kubernetes Secret reference, because the
connector sees the resolved value. The error reached the pipeline log,
deployment_error, the monitor events and the support bundle, where any
user with read access to the pipeline could read the password.

Show the hosts, ports and database instead, parsed with
tokio_postgres::Config, and nothing when the URI does not parse. The
endpoint name in the error already identifies the connector.

Part of feldera/cloud#2136.

Signed-off-by: Xavier Bestel <[email protected]>
bestouff added 8 commits November 12, 2026 14:07
@bestouff
[types] Keep connector options and credentials out of the startup log
da0d358
The pipeline printed its whole configuration on startup, before it
resolved secret references and before the logger was up. Inline
connector options such as sasl.password, environment values and storage
sync keys went to the pipeline log and from there to the support bundle.

Print one line per endpoint with its stream, transport and format names,
a reviewed list of runtime fields, the storage backend name and the
environment variable names. A field that is not on the list stays out,
so a new field that holds a secret is safe by default.

Part of feldera/cloud#2136.

Signed-off-by: Xavier Bestel <[email protected]>
@bestouff
[pipeline-manager] Keep credentials out of connector compile errors
75a976c
A malformed connectors property was copied into the compile error.
Deserializer reasons could also echo a rejected scalar containing an
inline credential, even after the whole property was omitted.

Preserve the relation, property, source position, and JSON line/column.
Replace the property value with [REDACTED] and report a safe error
category and corrective action. Test malformed JSON and rejected
scalars in both displayed and serialized errors.

Related: feldera/cloud#2136

Signed-off-by: Xavier Bestel <[email protected]>
@bestouff
[connectors] Show Delta table URIs without userinfo and query string
f1aa017
The Delta input connector put the table URI into its errors. A URI can
carry credentials in its userinfo or a signed-URL token in its query
string.

Strip both before formatting. A local path shows as is; a string that
does not parse and holds either part shows as [REDACTED].

Part of feldera/cloud#2136.

Signed-off-by: Xavier Bestel <[email protected]>
@bestouff
[connectors] Report a rejected Kafka option without its value
1e6856f
When librdkafka rejects a client option, rdkafka's error carries the
option's value and repeats it in the description. The connectors turned
that error into text with {e}, so a rejected sasl.password, inline or
resolved from a Secret reference, went to the pipeline log and to
deployment_error.

Map the error at every client construction: name the option, keep the
description with the value masked, drop the value.

Part of feldera/cloud#2136.

Signed-off-by: Xavier Bestel <[email protected]>
@bestouff
[adapterlib] Keep credentials out of format parse errors
50258b0
Format configurations can contain resolved secret values. Both the
configuration dump and serde's rejected scalar or map key exposed those
values through initialization errors. Short secrets also bypass the
runtime registry, so the source must be safe independently.

Keep the endpoint identity, error code, and serialized field types.
Replace the configuration with [REDACTED] and describe the failure using
a fixed category and corrective action. Test parser and encoder errors
with rejected scalar values, fields, and enum variants.

Related: feldera/cloud#2136

Signed-off-by: Xavier Bestel <[email protected]>
@bestouff
[connectors] Stop printing whole connector configs at debug level
cabd055
The Kafka input and output connectors, the Avro encoder and the
controller printed a connector's complete config with {:?} at debug
level. For Kafka and Avro this is the resolved config, so a password
supplied as a Kubernetes Secret reference was in the log as soon as a
pipeline author set runtime_config.logging to debug.

Print the endpoint, topic and transport names instead. Give the Kafka
configs and the Avro schema registry config a Debug implementation that
names the options and headers and hides their values, so a later {:?}
of these types stays safe.

Part of feldera/cloud#2136.

Signed-off-by: Xavier Bestel <[email protected]>
@bestouff
[connectors] Keep rejected values out of the pipeline config parse error
5a0c51d
The pipeline reports a configuration file that does not deserialize with
serde's message, which quotes the rejected token. A credential in a
wrong-shaped field, such as a NATS `credentials` given as a plain string,
is rejected here before any redaction can run, so the value reached the
startup error and `deployment_error`.

Keep the error category, the expected values and the line and column,
and drop the token, with the helper the other configuration errors use.

Related: feldera/cloud#2136
Signed-off-by: Xavier Bestel <[email protected]>
@bestouff
[sql-to-dbsp] Keep the rejected JSON token out of connector errors
469134e
Jackson quotes an unrecognized token in full: a 'connectors' value
like "password": hunter2 gave "Unrecognized token 'hunter2'". The
token can be an inline credential, and the message is stored in the
program error, so the value also reached the support bundle. A bundle
collected without source code drops the SQL excerpt, but it kept this
message.

Drop the quoted token from the message. The source position and the
excerpt still point at the bad text.

Related: feldera/cloud#2136
Signed-off-by: Xavier Bestel <[email protected]>
@bestouff
bestouff force-pushed the cloud-2136-diagnostics branch from ed84b9c to 469134e Compare November 12, 2026 13:46

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mihaibudiu mihaibudiu mihaibudiu approved these changes
@mythical-fred-oss mythical-fred-oss[bot] mythical-fred-oss[bot] left review comments

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

2 participants

@bestouff @mihaibudiu
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.