apache / apache/iggy

fix(connectors): restart loads the newest connector config, not the active one

Open
#3,848 1 comment 0 reactions 0 assignees View on GitHub
bug connectors rust
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.