fix panic caused by an invalid type assertion #1738
Conversation
Signed-off-by: Benedikt Bongartz <[email protected]>
frzifus
force-pushed
the
feat/validate_jaeger_options_#1733
branch
from
217ab67 to
2535c01
Compare
Codecov Report
@@ Coverage Diff @@
## master #1738 +/- ##
==========================================
+ Coverage 87.46% 87.48% +0.02%
==========================================
Files 99 99
Lines 5926 5937 +11
==========================================
+ Hits 5183 5194 +11
Misses 570 570
Partials 173 173
Continue to review full report at Codecov.
|
|
Didn't we talk about using validating webhook to catch this? |
| @@ -63,7 +63,9 @@ func (o *Options) UnmarshalJSON(b []byte) error { | |||
| if err := d.Decode(&entries); err != nil { | |||
| return err | |||
| } | |||
| o.parse(entries) | |||
| if err := o.parse(entries); err != nil { | |||
| return err | |||
There was a problem hiding this comment.
is the return error propagated to the user at object creation?
There was a problem hiding this comment.
Not this particular one. But an error that says your body of your request is invalid.
Its the "code": 400 in the webhook response.
There was a problem hiding this comment.
If the error is wrapped here would that be propagated to user?
We should tell the user what was wrong in the CR if that is possible.
There was a problem hiding this comment.
oh seems i mixed things up. When you create Jeager with invalid Options it tells you whats wrong.
$ k apply -f invalid.yaml
Error from server: error when creating "invalid.yaml": admission webhook "mjaeger.kb.io" denied the request: v1.Jaeger.Spec: v1.JaegerSpec.Strategy: Storage: v1.JaegerStorageSpec.Type: Options: unmarshalerDecoder: invalid option type, expect: string, got: map[string]interface {}, error found in #10 byte of ...|":"test"}},"type":"c|..., bigger context ...|"datacenter":"dev","enabled":false,"mode":"test"}},"type":"cassandra"},"strategy":"production"}}|...Looking at the logs, we could explain error 400 a bit better. But placing a description in the "reason": "" field needs to be done in controller-runtime.webhook.webhooks.
@pavolloffay you are right. In course of doing this, i noticed that the |
Signed-off-by: Benedikt Bongartz [email protected]
Closes #1733
Which problem is this PR solving?
Short description of the changes
(*Options).UnmarshalJSONmethod.string.Optionscant be decoded error 400 is returned.controller-runtime.webhook.webhooks received request {"webhook": "/mutate-jaegertracing-io-v1-jaeger", "UID": "6c77a050-a28d-4172-abc4-56f61670baef", "kind": "jaegertracing.io/v1, Kind=Jaeger", "resource": {"group":"jaegertracing.io","version":"v1","resource":"jaegers"}} controller-runtime.webhook.webhooks wrote response {"webhook": "/mutate-jaegertracing-io-v1-jaeger", "code": 400, "reason": "", "UID": "6c77a050-a28d-4172-abc4-56f61670baef", "allowed": false}