Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(htpasswd): add autoreload for htpasswd #2933

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

vooon
Copy link
Contributor

@vooon vooon commented Jan 31, 2025

What type of PR is this?

feature

Which issue does this PR fix:

Fix #2925

What does this PR do / Why do we need it:

If an issue # is not available please add repro steps and logs showing the issue:

Testing done on this change:

Automation added to e2e:

Will this break upgrades or downgrades?

Does this PR introduce any user-facing change?:


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@vooon
Copy link
Contributor Author

vooon commented Jan 31, 2025

Unsure, htpasswd change do not require full reload with server restarts. But putting the logic into existing watcher much easier.
And in my case the event is not very often.

@vooon
Copy link
Contributor Author

vooon commented Jan 31, 2025

Looks like it works, but not as fast as file updates.

@vooon vooon force-pushed the htpasswd-reload branch 6 times, most recently from 1f35ccb to e4605a5 Compare February 5, 2025 14:57
Copy link

codecov bot commented Feb 5, 2025

Codecov Report

Attention: Patch coverage is 84.25926% with 17 lines in your changes missing coverage. Please review.

Project coverage is 91.02%. Comparing base (d0ad935) to head (259df17).

Files with missing lines Patch % Lines
pkg/cli/server/config_reloader.go 22.22% 12 Missing and 2 partials ⚠️
pkg/api/htpasswd.go 94.33% 2 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2933      +/-   ##
==========================================
- Coverage   91.06%   91.02%   -0.04%     
==========================================
  Files         174      175       +1     
  Lines       32655    32725      +70     
==========================================
+ Hits        29736    29789      +53     
- Misses       2207     2221      +14     
- Partials      712      715       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@vooon vooon marked this pull request as ready for review February 5, 2025 17:33
@rchincha
Copy link
Contributor

rchincha commented Feb 6, 2025

Ideally, we need the watching logic to move outside the server/controller logic, so it is just another config file we watch (along with zot config)

That could be second step. Let other folks take a look also.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feat]: Autoreload htpasswd on changes
3 participants