[ResponseOps][Connectors] Allow additional methods for the Webhook connector

[adcoelho]

Closes #236398

Summary

This PR allows the webhook connector to be configured with the GET, PATCH, and DELETE methods.

If the method is one of GET or DELETE, a rule action using this connector will have body parameter as optional.

Although the original issue says otherwise, I decided to exclude the body for DELETE requests too. The documentation says:

The DELETE method has no defined semantics for the message body, so this should be empty.

Release Notes

The webhook connector now allows the following HTTP request methods: POST(default), PUT, PATCH, GET, or DELETE.

[elasticmachine]

Pinging @elastic/response-ops (Team:ResponseOps)

[jcger]

I've created a connector with POST and used it as rule action with a body. I then updated the webhook to use GET instead. The webhook is not being send (which makes sense), but I would have expected to see an error message in the logs and there is nothing. Also, updating back from GET to POST, still contains the body. Shouldn't the body have been deleted when updating to GET? Seems like we might have GET configs with a body. At the same time (thinking out loud) deleting the body when updating from POST to GET could be a huge disaster if someone updates by mistake or not really thinking about it. If we end up deleting the body when updating from POST to GET, should we show a warning message?

[adcoelho]

I've created a connector with POST and used it as rule action with a body. I then updated the webhook to use GET instead. The webhook is not being send (which makes sense), but I would have expected to see an error message in the logs and there is nothing. Also, updating back from GET to POST, still contains the body. Shouldn't the body have been deleted when updating to GET? Seems like we might have GET configs with a body. At the same time (thinking out loud) deleting the body when updating from POST to GET could be a huge disaster if someone updates by mistake or not really thinking about it. If we end up deleting the body when updating from POST to GET, should we show a warning message?

Thanks for this @jcger. After your message I had a chat with tia offline.

Changing all configured actions if the connector changed was not feasible so we opted for the following solution:

1. Always display the configured method in the rule action of a webhook connector.
2. The body is optional in the form for GET and DELETE but always shown.
3. The body is ignored when running execute for GET and DELETE.

[jcger]

1. Should we also indicate that the body will be ignored?

2. I did not test in latest main, so maybe there is something already in place. What about an intermediate release? I configured a GET and a PATCH webhook, when rolling back, the GET request stops working (GET with body error in the console), PATCH works, so far so good. The problem is that when updating these connectors, I see POST, which is incorrect and would confuse the users

code LGTM

[adcoelho]

Should we also indicate that the body will be ignored?

We decided not to. It looked a bit awkward with the subtext there.

I did not test in latest main, so maybe there is something already in place. What about an intermediate release? I configured a GET and a PATCH webhook, when rolling back, the GET request stops working (GET with body error in the console), PATCH works, so far so good. The problem is that when updating these connectors, I see POST, which is incorrect and would confuse the users

I created an intermediate release PR only with the changes from the BE #239847

[cnasikas]

We need to make the same changes for the components in the triggers_actions_ui, otherwise, I get an error for the DELETE/GET methods in security solution.

[cnasikas]

Should we use ActionConfig from the template literal?

[e40pud]

Same question. I think this will allow to avoid type casting in the x-pack/platform/plugins/shared/stack_connectors/public/connector_types/webhook/webhook.tsx where connectorConfig is of a WebhookConfig type.

[e40pud]

Thanks for addressing all feedback! 🚀

[dplumlee]

Tested all new methods in the rule edit and create forms, rule management changes LGTM

[cnasikas]

LGTM!

[janmonschke]

kibana-cases changes lgtm

[peteharverson]

Changes to the AI Infra owned tests LGTM

[semd]

Workflows code LGTM

[arturoliduena]

LGTM - Obs AI Assistant code review

[elasticmachine]

💚 Build Succeeded

• Buildkite Build
• Commit: 28247e2

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
securitySolution	8352	8378	+26

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id	before	after	diff
@kbn/alerts-ui-shared	306	307	+1
triggersActionsUi	500	501	+1
total			+2

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
apm	2.8MB	2.8MB	+111.0B
datasetQuality	426.9KB	427.0KB	+111.0B
discover	1.2MB	1.2MB	+111.0B
infra	1.1MB	1.1MB	+111.0B
ml	5.4MB	5.4MB	+111.0B
monitoring	631.7KB	631.8KB	+111.0B
observability	1.7MB	1.7MB	+112.0B
securitySolution	11.1MB	11.1MB	+2.9KB
slo	984.4KB	984.5KB	+111.0B
stackConnectors	819.8KB	820.0KB	+210.0B
synthetics	1.0MB	1.0MB	+111.0B
transform	624.5KB	624.6KB	+111.0B
triggersActionsUi	1.5MB	1.5MB	+230.0B
uptime	490.6KB	490.7KB	+111.0B
total			+4.5KB

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
stackConnectors	75.8KB	76.3KB	+514.0B

Unknown metric groups

API count

id	before	after	diff
@kbn/alerts-ui-shared	325	326	+1
triggersActionsUi	504	505	+1
total			+2

History

• 💔 Build #361383 failed 92ddad9
• 💔 Build #361293 failed 8f23d2c
• 💔 Build #361281 failed 43d96de
• 💚 Build #360435 succeeded aaf7286
• 💔 Build #360425 failed b80ceca

cc @adcoelho @js-jankisalvi