Setting FailureMode from ENV #1030
Conversation
didierofrivia
requested review from
eguzki,
guicassolato and
adam-cattermole
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1030 +/- ##
==========================================
+ Coverage 76.15% 83.94% +7.78%
==========================================
Files 111 81 -30
Lines 8986 6694 -2292
==========================================
- Hits 6843 5619 -1224
+ Misses 1852 860 -992
+ Partials 291 215 -76
Flags with carried forward coverage won't be shown. Click here to find out more.
|
didierofrivia
force-pushed
the
failure-mode
branch
from
b59b48e
to
cb6bed7
Compare
pkg/wasm/utils.go
Outdated
| case string(FailureModeAllow), string(FailureModeDeny): | ||
| return FailureModeType(value) | ||
| default: | ||
| fmt.Printf("Warning: Invalid value '%s' for %s. Using default value '%s'.\n", value, envVarName, defaultValue) |
There was a problem hiding this comment.
Seems slightly odd to use standard printing here instead of using a logger but I guess that's because it's not easy to get the logger here?
There was a problem hiding this comment.
Yup, I wanted to avoid having to create an instance of the logger just for this... but I could if desired.
There was a problem hiding this comment.
I could also remove the message xD
There was a problem hiding this comment.
I think it's nice to have the message at least to know that if you set it with a typo there's a reason why it's not being set.. but it's also going to look weird with interleaved Printf messages with the operator logs every time there's an event that triggers modification to the plugin that goes through this code. I don't want to hold up this PR though not a big issue
There was a problem hiding this comment.
I would panic, it is wrong to have an invalid value
There was a problem hiding this comment.
But happy to merge this as it is as well
There was a problem hiding this comment.
Take a look at 161d965, an alternative would be creating a different instance just for the logging that setting or leave it to fmt.Println
didierofrivia
force-pushed
the
failure-mode
branch
from
cb6bed7
to
34b069c
Compare
didierofrivia
force-pushed
the
failure-mode
branch
5 times, most recently
from
57d6c8d
to
3220fbd
Compare
Signed-off-by: dd di cesare <[email protected]>
Signed-off-by: dd di cesare <[email protected]>
Signed-off-by: dd di cesare <[email protected]>
didierofrivia
force-pushed
the
failure-mode
branch
from
3220fbd
to
161d965
Compare
Closes #866
Reading the values from ENV, with the following defaults:
Auth => Deny
RateLimit => Allow