cloudnative-pg / cloudnative-pg/cloudnative-pg

[Bug]: Plugin with isWALArchiver: false writes into the source object store, corrupting the timeline history of the origin cluster

Open
#11,351 1 comment 1 reaction 1 assignee Claimed by @gbartolini View on GitHub
bug :bug: triage
Dominant language
Go
Stars
9.3k
Forks
759
Avg merge
2d 6h
Merged PRs (30d)
44

Description

### Is there an existing issue already for this bug?

- [x] I have searched for an existing issue, and could not find anything. I believe this is a new bug.

### I have read the troubleshooting guide

- [x] I have read the troubleshooting guide and I think this is a new bug.

### I am running a supported version of CloudNativePG

- [x] I have read the troubleshooting guide and I think this is a new bug.

### Version

1.28 (latest patch)

### What version of Kubernetes are you using?

1.34

### What is your Kubernetes environment?

Cloud: Other

### How did you install the operator?

Helm

### What happened?

**The documentation states the opposite of what the code actually does.** In `docs/src/wal_archiving.md`

(section **Plugin-Based Architecture**, `release-1.28`):

> "Only **one plugin at a time** can be responsible for WAL archiving. This is configured by setting the `isWALArchiver` field to `true` within the plugin configuration."

`spec.plugins[].isWALArchiver: false` does **not** prevent a plugin from receiving WAL archive
requests. Any enabled plugin that advertises the CNPG-I `ARCHIVE_WAL` capability is called on every
WAL archive, regardless of the value of `isWALArchiver`.

This is easy to mistake for an authorization flag. It is not: it only selects which plugin
*suppresses the legacy barman path* and which one the operator waits for at startup. The actual
dispatch is a broadcast over every loaded plugin:

```go
// internal/cnpi/plugin/client/wal.go (release-1.28)
for idx := range data.plugins {
plugin := data.plugins[idx]
if !slices.Contains(plugin.WALCapabilities(), wal.WALCapability_RPC_TYPE_ARCHIVE_WAL) {
continue
}
_, err := plugin.WALClient().Archive(ctx, &request) // <- isWALArchiver never consulted
}
```

and the caller deliberately runs it even when no archiver is selected:

```go
// pkg/management/postgres/archiver/archiver.go:165 (release-1.28)
// We allow plugins to archive WALs even if there is no plugin
// directly enabled by the user, to retain compatibility with
// the old API.
if err := archiveWALViaPlugins(ctx, cluster, pgData, walName); err != nil {
```

`GetEnabledWALArchivePluginName()` is used only as a *guard* (error if the named archiver is not
loaded). When it returns `""` — i.e. every plugin has `isWALArchiver: false` — the guard is skipped
and the broadcast happens anyway.

**Why this is more than cosmetic.** The common migration pattern is to declare a plugin purely as a
*read* path for `externalClusters` (restore/replica source) pointing at a **third-party, production**
object store, while a different plugin does the writing. With `isWALArchiver: false` this looks safe,
but the read-only plugin is handed every WAL and writes into the source archive.

It is also silent for a long time: WAL-G skips objects that already exist, so while the standby
replays segments already present in the source archive, every push is a no-op. The first writes
appear only when the cluster produces WAL the source archive does not have — i.e. **at promotion**,
when a new timeline is created. In our case that injected a timeline-62 `.history` plus 7 segments
into a shared WAL-G catalog. Nothing reports an error, no existing backup is corrupted, but later
restores of the *real* cluster fail, because `recovery_target_timeline = latest` follows the injected
timeline and then panics:

```
PANIC: online backup was canceled, recovery cannot continue
pg_control: Min recovery ending loc's timeline: 62 (backup is on timeline 61)
```

Replica clusters make this more likely: they get `archive_mode = 'always'`
(`pkg/postgres/configuration.go:729`), so even a standby archives the WAL it receives.

## To Reproduce

1. Install any CNPG-I plugin that implements `ARCHIVE_WAL` (we used `cnpg-plugin-wal-g` 0.3.0).
2. Create a **replica** cluster whose only declared plugin is that one, marked as *not* the archiver,
pointing at an existing third-party archive:
```yaml
spec:
replica:
enabled: true
source: external-archive
plugins:
- name: cnpg-extensions.yandex.cloud
enabled: true
isWALArchiver: false # <- expected: this plugin never writes
parameters: { backupConfig: source-archive }
externalClusters:
- name: external-archive
plugin:
name: cnpg-extensions.yandex.cloud
parameters: { backupConfig: source-archive }
```
3. Let the standby replay past the end of what the source archive already contains (or promote it).
4. Inspect the source object store.

## Expected behavior

With `isWALArchiver: false` on every declared plugin, the instance should not hand WAL to any plugin
for archiving. At minimum, a plugin explicitly marked `isWALArchiver: false` should be excluded from
the `ArchiveWAL` broadcast even when it is the only plugin loaded.

If the broadcast must stay for backward compatibility with the pre-`isWALArchiver` API, then an
explicit `isWALArchiver: false` should still opt the plugin out — the field is only meaningful if it
can say "no".

## Logs

Generated configuration (single plugin, `isWALArchiver: false`):

```
archive_mode = 'always'
archive_command = '/controller/manager wal-archive --log-destination /controller/log/postgres.json %p'
```

Plugin sidecar, 107 archive calls on a cluster with no enabled WAL archiver:

```
{"level":"info","logger":"plugin_wal","msg":"Successful run wal-g wal-push /var/lib/postgresql/data/pgdata/pg_wal/0000003D0000017F00000085"}
```

Objects the read-only-intended plugin wrote into the source archive (WAL-G writes brotli; the
upstream Spilo/WAL-G producer writes lz4, so the origin is unambiguous):

```
0000003D0000017F00000030.lz4 <- written by the source cluster
0000003E.history.br <- written by the CNPG cluster (timeline 62)
0000003E0000017F00000031.br
... 6 more
```

## Environment

- CNPG version: 1.28
- Kubernetes version: v1.34.2 (OKE)
- Cloud provider: OCI (S3-compatible object storage)
- Plugin: `cnpg-plugin-wal-g` 0.3.0 (bundles WAL-G v3.0.8)

## Additional context

Workarounds we found:

- annotate the cluster with `cnpg.io/skipWalArchiving: enabled`, which forces `archive_mode = off`
(`pkg/utils/labels_annotations.go:487`). This is the only reliable off switch, but it also disables
legitimate archiving, so it is unusable for a standby that must archive to a *different* store;
- always declare an explicit archiver (e.g. `barman-cloud` with `isWALArchiver: true`) so the
read-path plugin is not the one wired to `archive_command`. Note this does **not** stop the
broadcast — the read-path plugin is still called — it only adds a second writer;
- use a **read-only credential** for the source archive. This is what actually protects the source,
and it is what we now consider mandatory.

Suggestion: documenting that `isWALArchiver: false` is not a write barrier would already help;
honouring it in `innerArchiveWAL` would be better.

### Code of Conduct

- [x] I agree to follow this project's Code of Conduct

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.