Skip to content

Fix certificate reload issue when files are moved - #210

Merged
harshavardhana merged 2 commits into
minio:mainfrom
ramondeklein:fix-move-cert
Dec 1, 2025
Merged

harshavardhana merged 2 commits into
minio:mainfrom
ramondeklein:fix-move-cert

Conversation

@ramondeklein

@ramondeklein ramondeklein commented Dec 1, 2025 •

Copy link
Copy Markdown
Contributor

Certificates were not properly reloaded in Kubernetes, because it uses a slightly different method of replacing certificates:

  1. Write the new certificate to /tmp/minio/certs/public.crt~.
  2. Rename the file /tmp/minio/certs/public.crt~ to /tmp/minio/certs/public.crt.

Since event_linux.go is only watching notify.InCloseWrite it did trigger when public.crt~ is written, but the actual certificate hasn't been updated yet. After adding notify.InMovedTo it also gets triggered after the rename. This caused a race condition between reloading the certificate and the rename operation.

The PR also adds a unit test to check this behavior.

PS: This doesn't seem to be caused by #198. The previous certificate manager also didn't watch the notify.InMovedTo event.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes certificate reload issues in Kubernetes environments by adding support for file rename operations. Kubernetes updates certificates using a write-then-rename pattern that wasn't being detected by the existing InCloseWrite event watcher on Linux. The fix adds notify.InMovedTo to the watched events and includes comprehensive integration tests to verify the new behavior.

  • Adds notify.InMovedTo event to Linux file watcher to detect certificate updates via rename operations
  • Introduces integration tests for certificate reload via rename operations
  • Increases test timeout to accommodate file system event processing delays

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
certs/event_linux.go Adds notify.InMovedTo to the list of watched file system events for certificate files on Linux
certs/certificate2_test.go Adds new test cases and helper function parameter to test certificate reload via rename operations, updates all existing test calls with the new parameter, and increases test timeout for reliability

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread certs/certificate2_test.go Outdated
Comment thread certs/certificate2_test.go Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread certs/certificate2_test.go Outdated
Comment thread certs/certificate2_test.go
@ramondeklein
ramondeklein requested a review from Copilot December 1, 2025 13:52

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@harshavardhana
harshavardhana merged commit 98761f4 into minio:main Dec 1, 2025
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants