Add support for reading VictorOps API key from file #3111
Conversation
Signed-off-by: Oktarian Tilney-Bassett <oktariantilneybassett@improbable.io>
Signed-off-by: Oktarian Tilney-Bassett <oktariantilneybassett@improbable.io>
Signed-off-by: Oktarian Tilney-Bassett <oktariantilneybassett@improbable.io>
Signed-off-by: Oktarian Tilney-Bassett <oktariantilneybassett@improbable.io>
Signed-off-by: Oktarian Tilney-Bassett <oktariantilneybassett@improbable.io>
Signed-off-by: Oktarian Tilney-Bassett <oktariantilneybassett@improbable.io>
| if n.conf.APIKey != "" { | ||
| apiKey = string(n.conf.APIKey) | ||
| } else { | ||
| content, fileErr := os.ReadFile(n.conf.APIKeyFile) |
There was a problem hiding this comment.
I might miss the reason why we need a new variable name but if there's none, I'd prefer to keep err :)
| content, fileErr := os.ReadFile(n.conf.APIKeyFile) | |
| content, err := os.ReadFile(n.conf.APIKeyFile) |
There was a problem hiding this comment.
Declaring it as err would be a shadow declaration of a previously defined err variable used for templating errors: 🙂
alertmanager/notify/victorops/victorops.go
Lines 70 to 75 in 3f561cd
There was a problem hiding this comment.
Hmm it shouldn't cause a direct problem as long as tmpl isn't called in this branch but I don't mind keeping it explicit.
Other notifiers declare a tmplErrvariable to make it more obvious that the error variable shouldn't be reused for anything else but I feel that it's still a bit obfuscated (it's easy to call the templating function and forget checking the error). The general approach with a anonymous function might need to be revised but this isn't relevant to this change :)
There was a problem hiding this comment.
That makes sense! I agree that the templating error handling is not the best, just need to be careful I guess :)
There was a problem hiding this comment.
Are there any other changes needed for this PR btw? 🏁
Co-authored-by: Simon Pasquier <spasquie@redhat.com> Signed-off-by: Oktarian T-B <oktariantilneybassett@improbable.io>
Co-authored-by: Simon Pasquier <spasquie@redhat.com> Signed-off-by: Oktarian T-B <oktariantilneybassett@improbable.io>
* Read VictorOps API key from file * Update documentation * Improve tests Signed-off-by: Oktarian T-B <oktariantilneybassett@improbable.io> Signed-off-by: Yijie Qin <qinyijie@amazon.com>
* Read VictorOps API key from file * Update documentation * Improve tests Signed-off-by: Oktarian T-B <oktariantilneybassett@improbable.io> Signed-off-by: Yijie Qin <qinyijie@amazon.com>
Related to issue #2498. Basically a copy of #2728.
#2554 can be closed as a result.
Signed-off-by: Oktarian Tilney-Bassett 😄