fix(webhook): Don't try to deserialize fields we don't really need #3202
Conversation
|
Fixes spinnaker/spinnaker#4881 |
An assumption was made that body is a can be jackon parsed into a JSON object. But that's not always the case, in fact, we actually account for it in the original parse: https://github.com/spinnaker/orca/blob/master/orca-webhook/src/main/groovy/com/netflix/spinnaker/orca/webhook/tasks/CreateWebhookTask.groovy#L71 But then disregard it in StageContext mapping This change removes those fields from being parsed since we don't actually need them for anything inside the stage (they were added in the attempt to be fully typed)
marchello2000
force-pushed
the
mark/issue_4881
branch
from
ae5474a
to
40279af
Compare
| WebhookMonitorResponseStageData monitor | ||
| String error | ||
|
|
||
| // NOTE: The fields below exist in the context because they are inserted by the CreateWebhookTask but they aren't |
|
What do you mean "they aren't consumed by spinnaker" - did they used to be? |
We don't ever "read" them in orca (or anywhere) we store them for the user/deck to observe in the context. In my previous change, i wanted to be complete and define everything in the stagecontext for webhook, but I think there is some marginal value in having them there... even if they are commented out... but i am happy to also remove them |
|
I'd say leave them for now. I like having as much information as possible when you're in orca trying to fix a stage. |
|
LGTM |
|
@marchello2000 is this okay to merge? |
|
We'd love to get this one in and cut 1.16.3 asap :) |
|
@ethanfrogers yes, thank you! feel free to merge when tRavis does it's thing... or I can if I see it finish first |
|
@spinnakerbot cherry-pick 1.16 |
|
fyi, I merged/cherry-picked because @ethanfrogers told me to since he was |
|
Thanks, @kevinawoo !!!, CC: @louisjimenez as FYI that this should go to |
* fix(webhook): Don't try to deserialize fields we don't really need An assumption was made that body is a can be jackon parsed into a JSON object. But that's not always the case, in fact, we actually account for it in the original parse: https://github.com/spinnaker/orca/blob/master/orca-webhook/src/main/groovy/com/netflix/spinnaker/orca/webhook/tasks/CreateWebhookTask.groovy#L71 But then disregard it in StageContext mapping This change removes those fields from being parsed since we don't actually need them for anything inside the stage (they were added in the attempt to be fully typed) * fixup! fix(webhook): Don't try to deserialize fields we don't really need Co-authored-by: Emily Burns <emjeburns@gmail.com> Co-authored-by: Ethan Rogers <ethanfrogers@users.noreply.github.com>
|
Cherry pick successful: #3207 |
…) (#3207) * fix(webhook): Don't try to deserialize fields we don't really need An assumption was made that body is a can be jackon parsed into a JSON object. But that's not always the case, in fact, we actually account for it in the original parse: https://github.com/spinnaker/orca/blob/master/orca-webhook/src/main/groovy/com/netflix/spinnaker/orca/webhook/tasks/CreateWebhookTask.groovy#L71 But then disregard it in StageContext mapping This change removes those fields from being parsed since we don't actually need them for anything inside the stage (they were added in the attempt to be fully typed) * fixup! fix(webhook): Don't try to deserialize fields we don't really need Co-authored-by: Emily Burns <emjeburns@gmail.com> Co-authored-by: Ethan Rogers <ethanfrogers@users.noreply.github.com>
An assumption was made that body is a can be jackon parsed into a JSON object.
But that's not always the case, in fact, we actually account for it in the original parse:
https://github.com/spinnaker/orca/blob/master/orca-webhook/src/main/groovy/com/netflix/spinnaker/orca/webhook/tasks/CreateWebhookTask.groovy#L71
But then disregard it in StageContext mapping
This change removes those fields from being parsed since we don't actually need them for anything
inside the stage (they were added in the attempt to be fully typed)