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

Support secrets for notification_url #1300

Merged
merged 3 commits into from
Aug 1, 2022

Conversation

jlaska
Copy link
Contributor

@jlaska jlaska commented Jun 6, 2022

Since the NOTIFICATION_URL contains app and user defined tokens, this PR adds support for the parameter to be loaded from a secret file. Relevant documentation updated to note that a file path is supported.

Allow loading WATCHTOWER_NOTIFICATION_URL from a secret file.  Also
updates relevant documentation.
@piksel
Copy link
Member

piksel commented Jun 7, 2022

This used to not be possible to do due to a bug in cobra/viper/pflags. It should have been fixed now, so this might work as expected. Have you tried it with multiple values etc.?

@piksel
Copy link
Member

piksel commented Aug 1, 2022

No, the problem is still there. The StringArray flags only appends the values passed to Set, which means that additionally to the file contents, there will always be the file name (which probably isn't a valid notification URL).
There is a way to update this correctly though, I will make a new PR for this amend the PR.

@codecov
Copy link

codecov bot commented Aug 1, 2022

Codecov Report

Merging #1300 (88d7277) into main (e983beb) will increase coverage by 0.58%.
The diff coverage is 89.47%.

@@            Coverage Diff             @@
##             main    #1300      +/-   ##
==========================================
+ Coverage   63.65%   64.23%   +0.58%     
==========================================
  Files          23       23              
  Lines        1527     1552      +25     
==========================================
+ Hits          972      997      +25     
  Misses        465      465              
  Partials       90       90              
Impacted Files Coverage Δ
internal/flags/flags.go 89.60% <89.47%> (+0.48%) ⬆️
pkg/notifications/slack.go 95.12% <0.00%> (+0.25%) ⬆️
pkg/filters/filters.go 83.63% <0.00%> (+2.38%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us.

@piksel piksel merged commit bd2adf6 into containrrr:main Aug 1, 2022
@jlaska
Copy link
Contributor Author

jlaska commented Aug 1, 2022

This is great, thank you @piksel !

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