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

app/vmbackup: introduce flags to take basic auth credentials and snapshot authkey #6223

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

wasim-nihal
Copy link
Contributor

Describe Your Changes

This PR includes changes in vmbackup to take the basic auth credentials (username and password) and the snapshot authKey via cli flags for the connections towards vmsingle/vmstorage.

Related issue: #5973

A few points related to the changes:

  • The basic auth credentials and snapshot authKey are taken as cli clags in vmbackup. (But still user can provide this info within the URL itself for backward compatibility. For example, http://username:password@localhost:8428/snapshot/create?authKey=mykey)
  • If the user sets the newly introduced flag for snapshot authKey -snapshot.authKey, then the authKey will be passed via http header X-AuthKey towards vmsimgle/storage. For more context refer app/vmbackup: introduce new flag type URL #6152 (comment)
  • For now, X-AuthKey is supported only for snapshotAuthKey flag. Later if needed, this can be modified for other authKeys like -reloadAuthKey, -configAuthKey etc.
  • [Masking for existing users passing credentials within URL] A new function RedactedURL is added under httputils package for masking sensitive info from the URLs. Note that this will just mask the URLs within the Victoria-Metrics logs. The sensitive info can still be exposed via the standard net/http package. See vmbackup:Facilitate Flag-Based Provisioning for Sensitive Data in snapshot.createURL #5973 (comment) and app/vmbackup: introduce new flag type URL #6152 (comment)

@hagen1778 @zekker6 please let me know your thoughts/comments on this change. Will address them.

Checklist

The following checks are mandatory:

…lags and pass the snapshot authKey and http header instead of query parameter. See VictoriaMetrics#5973

Signed-off-by: Syed Nihal <syed.nihal@nokia.com>
Signed-off-by: Syed Nihal <syed.nihal@nokia.com>
Copy link

codecov bot commented May 6, 2024

Codecov Report

Attention: Patch coverage is 69.23077% with 12 lines in your changes are missing coverage. Please review.

Project coverage is 57.22%. Comparing base (8aaa828) to head (85b2f90).
Report is 641 commits behind head on master.

Files Patch % Lines
lib/snapshot/snapshot.go 66.66% 6 Missing and 2 partials ⚠️
lib/httpserver/httpserver.go 0.00% 1 Missing and 1 partial ⚠️
lib/httputils/url.go 84.61% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #6223      +/-   ##
==========================================
- Coverage   60.37%   57.22%   -3.15%     
==========================================
  Files         411      528     +117     
  Lines       76609    72483    -4126     
==========================================
- Hits        46253    41482    -4771     
- Misses      27794    28096     +302     
- Partials     2562     2905     +343     

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

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.

None yet

1 participant