fix(connectors): restart loads the newest connector config, not the active one
- Dominant language
- Rust
- Stars
- 4.9k
- Forks
- 432
- Avg merge
- 2d 10h
- Merged PRs (30d)
- 173
Description
## Summary
On the default local configuration provider, a connector runs its **active** configuration after a process start and its **highest-numbered** configuration after `POST /{sinks,sources}/{key}/restart`. Rolling back with `PUT .../configs/active` therefore appears to work until the next restart, which silently reinstates the newest version.
The HTTP provider does not have this split.
## Where the two paths diverge
Startup resolves the active configuration:
```rust
// core/connectors/runtime/src/main.rs:153
let connectors_config = connectors_config_provider.get_active_configs().await?;
```
Restart asks for `None` instead:
```rust
// core/connectors/runtime/src/manager/sink.rs:261 (source.rs:280 is identical)
.get_sink_config(key, None)
```
and the local provider resolves `None` to the highest version number rather than the active one:
```rust
// core/connectors/runtime/src/configs/connectors/local_provider.rs
async fn get_sink_config(&self, key: &str, version: Option) -> ... {
if let Some(version) = version {
...
} else {
Ok(self
.get_sink_configs(key)
.await?
.into_iter()
.max_by_key(|config| config.version))
}
}
```
`get_source_config` is the same. Meanwhile the HTTP provider resolves the identical `None` to the active config:
```rust
// core/connectors/runtime/src/configs/connectors/http_provider.rs:291
None => self.url_builder.build(TemplateKeys::GET_ACTIVE_SINK_CONFIG, &vars),
```
So `version: None` means "active" in one provider and "newest" in the other, and the restart path is the caller that notices.
## The local provider already tracks this
It is not that the local provider lacks the concept. `set_active_sink_version` persists it, and `get_active_configs` reads it back and selects on it:
```rust
let active_config = if let Some(&version) = active_versions.sinks.get(key) {
config_files.iter().find(|c| c.config.version == version)
```
Only the `get_{sink,source}_config(key, None)` accessor skips it, which is exactly the one `restart_connector` uses.
## Why it matters
The intended rollback flow does not survive a restart:
1. `POST /sinks/{key}/configs` publishes v2, which turns out to be bad.
2. `PUT /sinks/{key}/configs/active` sets v1 active. `GET .../configs/active` confirms v1, and a process restart runs v1.
3. `POST /sinks/{key}/restart` runs **v2** again.
An operator rolling back an incident sees the rollback take effect, then get undone by the very call they would reach for to apply it.
## Suggested fix
Make `None` mean "active" in the local provider, matching the HTTP provider and `get_active_configs`. If "newest" is wanted somewhere, it deserves an explicit accessor rather than an overloaded `None`.
Happy to send a PR if the direction is agreed. Found while documenting the control API surface for #3804.
Contributor guide
Research direction
Start in core/connectors/runtime/src/configs/connectors/local_provider.rs and compare get_sink_config and get_source_config with the HTTP provider's handling of None. Then trace the restart callers in manager/sink.rs and manager/source.rs, alongside get_active_configs in main.rs. Confirm that rolling back the active version and restarting a connector continues to use the active configuration rather than the highest-numbered one.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- backend
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 72/100