Skip to content

fix: Keep published secrets on repeated NodePublishVolume - #773

Merged
lfrancke merged 4 commits into
mainfrom
fix/keep-published-secrets
Nov 10, 2026
Merged

lfrancke merged 4 commits into
mainfrom
fix/keep-published-secrets

Conversation

@lfrancke

@lfrancke lfrancke commented Nov 6, 2026 •

Copy link
Copy Markdown
Member

Description

Fixes #772.

It was a coincidence that I was working on this at the same time as Razvan in #772.

When the kubelet calls NodePublishVolume again for a volume that is already published (for example after a kubelet restart), secret-operator issued a new secret and moved the restarter.stackable.tech/expires-at.* annotation to it.
The running process keeps the secret it loaded at startup, so the restarter acted on the wrong expiry and could evict the Pod only after the secret in use had expired.

A completely published volume now contains an empty .secret-operator-published marker, written after the secret files.
A repeated publish returns early when it exists: no backend call, no Pod patch, no mount.

Tested on k3d (k3s v1.37.2) by enabling requiresRepublish for 20 s:

26.7.0 this PR
cert in container vs. host path differ same, unchanged
expires-at annotation moved unchanged
tmpfs layers 21 1

Independent of #770 but "related".

Author

  • Integration tests passed (for non trivial changes)
  • Release note snippet added

Reviewer

  • Code contains useful comments
  • Code contains useful logging statements
  • (Integration-)Test cases added
  • Documentation added or updated. Follows the style guide.
  • Changelog updated
  • Cargo.toml only contains references to git tags (not specific commits or branches)

Acceptance

  • Feature Tracker has been updated
  • Proper release label has been added
  • Links to generated (nightly) docs added
  • Release note snippet added

The kubelet calls NodePublishVolume again for volumes it already published,
for example after a kubelet restart. Every call issued a new secret and moved
the restarter's expiry annotation to it. A process that loaded its secret at
startup kept using the old one, so the restarter acted on the wrong expiry and
could restart the Pod only after the secret in use had expired.

A completely published volume now contains an empty marker file, written
after the secret files, and a repeated publish returns early when it exists.
@lfrancke

lfrancke commented Nov 6, 2026

Copy link
Copy Markdown
Member Author

This should not be merged before coordingating with #774

@lfrancke lfrancke moved this to Development: Waiting for Review in Stackable Engineering Nov 9, 2026
@Techassi
Techassi self-requested a review November 9, 2026 08:44
@Techassi Techassi moved this from Development: Waiting for Review to Development: In Review in Stackable Engineering Nov 9, 2026
@lfrancke

lfrancke commented Nov 9, 2026

Copy link
Copy Markdown
Member Author

I'll fix the conflict.
And on the weekend I looked at this + #774 and decided to keep this unchanged because the marker file is written at the very end. The #774 PR "protects" againts failures halfway through at the cost of more complexity. In the case of this PR that is handled by "just" retrying as is the Kubernetes way.

@Techassi Techassi left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The idea itself sounds good, I just have a few remarks on some security related points.

Processes in the Pod can write to the volume, because the kubelet applies
the fsGroup with group write permissions. Checking for the marker no longer
follows a symlink, and any entry under its name counts as provisioned:
returning early writes nothing, while reissuing on a marker that cannot be
written would issue a new secret on every kubelet retry. Writing the marker
refuses to follow a symlink. The marker is renamed to
.stackable-secret-operator-data-provisioned to avoid collisions with files
from other tools.
@lfrancke
lfrancke requested a review from Techassi November 9, 2026 13:54
Counting any entry as provisioned hid tampering. Failing the publish
instead stops before the backend is called, so a tampered marker still
cannot make every kubelet retry issue a new secret, and the error shows up
in the Pod's events. Only the Pod that changed its own volume is affected.
@lfrancke
lfrancke added this pull request to the merge queue Nov 10, 2026
@lfrancke lfrancke moved this from Development: In Review to Development: Done in Stackable Engineering Nov 10, 2026
Merged via the queue into main with commit 60a5fcf Nov 10, 2026
13 checks passed
@lfrancke
lfrancke deleted the fix/keep-published-secrets branch November 10, 2026 14:15
@lfrancke lfrancke moved this from Development: Done to Done in Stackable Engineering Nov 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Development

Successfully merging this pull request may close these issues.

Mounted secrets and the restarter.stackable.tech/expires-at can drift apart

2 participants