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

Add ability to sign requests for all AWS services #2604

Merged
merged 1 commit into from
May 18, 2022

Conversation

transacid
Copy link
Contributor

@transacid transacid commented May 17, 2022

This adds the ability to utilize sigv4 signing for all AWS services not
just "aps". When the newly introduced property "service" is not set it
will default to "aps".
Some words about the reasoning behind this PR:
We are running VictoriaMetrics in an EKS cluster in AWS and would like to use API gateway for authentication. To sign requests for the API gateway the "service" needs to be "execute-api". However vmAgent hard codes it to be "aps.
this PR extends functionality added by #1287

@transacid
Copy link
Contributor Author

this would solve #2605

@transacid
Copy link
Contributor Author

I have opened a similar PR at prometheus prometheus/common#380

@codecov
Copy link

codecov bot commented May 17, 2022

Codecov Report

Merging #2604 (f54744d) into master (b68f0fe) will decrease coverage by 0.18%.
The diff coverage is 0.00%.

❗ Current head f54744d differs from pull request most recent head 654a5df. Consider uploading reports for the commit 654a5df to get more accurate results

@@            Coverage Diff             @@
##           master    #2604      +/-   ##
==========================================
- Coverage   59.31%   59.12%   -0.19%     
==========================================
  Files         255      255              
  Lines       42913    43035     +122     
==========================================
- Hits        25452    25445       -7     
- Misses      15772    15892     +120     
- Partials     1689     1698       +9     
Impacted Files Coverage Δ
lib/awsapi/config.go 9.92% <0.00%> (-0.18%) ⬇️
lib/promscrape/discovery/ec2/api.go 0.00% <0.00%> (ø)
app/vmalert/notifier/alert.go 44.44% <0.00%> (-11.12%) ⬇️
lib/mergeset/table.go 65.58% <0.00%> (-1.44%) ⬇️
app/vmalert/datasource/vm.go 85.71% <0.00%> (-0.29%) ⬇️
app/vmalert/alerting.go 77.56% <0.00%> (ø)
app/vmalert/remotewrite/init.go 0.00% <0.00%> (ø)
app/vmalert/datasource/vm_prom_api.go 84.82% <0.00%> (ø)
app/vmselect/promql/aggr_incremental.go 85.24% <0.00%> (ø)
app/vmalert/notifier/template_func.go
... and 4 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b68f0fe...654a5df. Read the comment docs.

@f41gh7 f41gh7 requested review from valyala and f41gh7 and removed request for valyala May 17, 2022 14:30
Copy link
Contributor

@f41gh7 f41gh7 left a comment

Choose a reason for hiding this comment

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

LGTM

This adds the ability to utilize sigv4 signing for all AWS services not
just "aps". When the newly introduced property "service" is not set it
will default to "aps".

Signed-off-by: Boris Petersen <boris.petersen@idealo.de>
@f41gh7 f41gh7 merged commit 3df8cac into VictoriaMetrics:master May 18, 2022
@f41gh7
Copy link
Contributor

f41gh7 commented May 18, 2022

Thanks for contribution!

valyala pushed a commit that referenced this pull request May 20, 2022
This adds the ability to utilize sigv4 signing for all AWS services not
just "aps". When the newly introduced property "service" is not set it
will default to "aps".

Signed-off-by: Boris Petersen <boris.petersen@idealo.de>
transacid pushed a commit to transacid/VictoriaMetrics that referenced this pull request Jul 13, 2022
Signed-off-by: Boris Petersen <boris.petersen@idealo.de>
valyala pushed a commit that referenced this pull request Jul 13, 2022
Signed-off-by: Boris Petersen <boris.petersen@idealo.de>
@transacid transacid deleted the sigv4_service branch July 13, 2022 13:21
valyala pushed a commit that referenced this pull request Jul 13, 2022
Signed-off-by: Boris Petersen <boris.petersen@idealo.de>
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

2 participants