[WIP] Remove Webhook#22617
Open
RensR wants to merge 9 commits into
Open
Conversation
Contributor
CORA - Pending ReviewersAll codeowners have approved! ✅ Legend: ✅ Approved | ❌ Changes Requested | 💬 Commented | 🚫 Dismissed | ⏳ Pending | ❓ Unknown For more details, see the full review summary. |
Contributor
|
✅ No conflicts with other open PRs targeting |
RensR
force-pushed
the
rm-webhook
branch
2 times, most recently
from
b3c06ab to
7b2e580
Compare
RensR
commented
May 28, 2026
| cltest.AssertServerResponse(t, resp, http.StatusOK) | ||
| } | ||
|
|
||
| type OperatorContracts struct { |
Contributor
Author
There was a problem hiding this comment.
Some tests in this file have been moved to core/services/pipeline/eth_tx_integration_test.go, but without relying on the jobspecs that we're slowly removing. This means we can keep stable tests even if we mess around with jobs. This is the most significant change in this PR that's not directly removing webhook.
Should also fix quarantine.Flaky(t, "DX-1767")
the internal feature tests are better places at the pipeline level for these specific ones.
RensR
requested review from
b-gopalswami,
chris-de-leon-cll,
makramkd,
ogtownsend and
tt-cll
product-security-plaid-production
Bot
requested review from
DimitriosNaikopoulos,
agparadiso,
archseer,
dhaidashenko and
mchain0
Contributor
There was a problem hiding this comment.
Pull request overview
Removes the runtime Webhook job type: deletes the entire core/services/webhook package (validator, delegate, in-memory runner, EI manager, authorizer, mocks, tests), wires job.Webhook to job.DeprecatedDelegate, and adds rejection paths at the ORM, REST controller, and GraphQL mutation. DB tables, persisted models, presenters, and GraphQL spec resolvers are retained so existing rows remain readable/deletable.
Changes:
- Delete
core/services/webhookpackage and all references (Application.GetExternalInitiatorManager,RunWebhookJobV2,webhookJobRunner,ExternalInitiatorManageropt, EI manager construction incmd/shell.go, mockery entries, cltest helpers/integration tests). - Reject webhook job creation in
JobsController.validateJobSpec, GraphQLCreateJob, andjob.ORM.CreateJob; reject UUID-based runs inPipelineRunsController.Create; simplifyRunJobPayloadResolveraccordingly. - Update tests and
testspecs.WebhookSpecParamsto use a local TOML type, replace EI/webhook integration tests with rejection-focused tests, and add a new asyncethtxintegration test undercore/services/pipeline(separate cron-based coverage that previously relied on webhook).
Reviewed changes
Copilot reviewed 33 out of 34 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| docs/CONFIG.md, core/config/docs/core.toml | Reword ExternalInitiatorsEnabled doc to mention legacy webhook context. |
| deployment/utils/nodetestutils/node.go | Drop ExternalInitiatorManager from ApplicationOpts. |
| core/web/resolver/mutation.go, job_run.go, job_run_test.go, resolver_test.go | GraphQL: reject webhook creation, simplify RunJobPayload, remove webhook mocks/tests. |
| core/web/pipeline_runs_controller{,_test}.go | Reject UUID job runs; replace happy-path tests with rejection tests. |
| core/web/jobs_controller{,_test}.go | Reject webhook in validateJobSpec; assert 422. |
| core/testdata/testspecs/v2_specs.go | Move TOMLWebhookSpecExternalInitiator locally. |
| core/services/webhook/** | Entire package deleted (delegate, validator, EI manager, authorizer, mocks, tests). |
| core/services/pipeline/eth_tx_integration_test.go | New cron-based async ethtx integration test replacing webhook-driven version. |
| core/services/job/{orm,job_orm_test,runner_integration_test}.go | ORM rejects webhook creation; tests reworked accordingly. |
| core/services/chainlink/application.go | Drop EI manager/runner wiring; use DeprecatedDelegate for webhook. |
| core/internal/mocks/application.go | Remove GetExternalInitiatorManager/RunWebhookJobV2 mocks. |
| core/internal/features/features_test.go | Remove webhook EI integration and related helpers. |
| core/internal/cltest/{cltest,job_factories}.go | Remove EI manager plumbing/helpers; add optional UUID arg to MustInsertWebhookSpec; add CreateJobRunViaUserByID. |
| core/cmd/shell.go | Drop EI manager construction. |
| .mockery.yaml | Remove webhook interface mocks. |
| .changeset/fuzzy-falcons-thank.md | Changeset entry. |
Files not reviewed (1)
- core/internal/mocks/application.go: Language not supported
Comment on lines
+34
to
49
| func TestPipelineRunsController_CreateWebhookJobRejected(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| ctx := testutils.Context(t) | ||
| ethClient := cltest.NewEthMocksWithStartupAssertions(t) | ||
| ethClient.On("BalanceAt", mock.Anything, mock.Anything, mock.Anything).Maybe().Return(big.NewInt(0), nil) | ||
| cfg := configtest.NewGeneralConfig(t, func(c *chainlink.Config, s *chainlink.Secrets) { | ||
| c.JobPipeline.HTTPRequest.DefaultTimeout = commonconfig.MustNewDuration(2 * time.Second) | ||
| c.Database.Listener.FallbackPollInterval = commonconfig.MustNewDuration(10 * time.Millisecond) | ||
| }) | ||
|
|
||
| app := cltest.NewApplicationWithConfig(t, cfg, ethClient) | ||
| app := cltest.NewApplicationEVMDisabled(t) | ||
| require.NoError(t, app.Start(testutils.Context(t))) | ||
|
|
||
| // Setup the bridge | ||
| mockServer := cltest.NewHTTPMockServerWithRequest(t, 200, `{}`, func(r *http.Request) { | ||
| defer r.Body.Close() | ||
| bs, err := io.ReadAll(r.Body) | ||
| require.NoError(t, err) | ||
| require.JSONEq(t, `{"result":"12345"}`, string(bs)) | ||
| }) | ||
|
|
||
| _, bridge := cltest.MustCreateBridge(t, app.GetDB(), cltest.BridgeOpts{URL: mockServer.URL}) | ||
|
|
||
| // Add the job | ||
| uuid := uuid.New() | ||
| { | ||
| tomlStr := fmt.Sprintf(testspecs.WebhookSpecWithBodyTemplate, uuid, bridge.Name.String()) | ||
| jb, err := webhook.ValidatedWebhookSpec(ctx, tomlStr, app.GetExternalInitiatorManager()) | ||
| require.NoError(t, err) | ||
|
|
||
| err = app.AddJobV2(testutils.Context(t), &jb) | ||
| require.NoError(t, err) | ||
| } | ||
|
|
||
| // Give the job.Spawner ample time to discover the job and start its service | ||
| // (because Postgres events don't seem to work here) | ||
| time.Sleep(3 * time.Second) | ||
|
|
||
| // Make the request | ||
| { | ||
| client := app.NewHTTPClient(nil) | ||
| body := strings.NewReader(`{"data":{"result":"123.45"}}`) | ||
| response, cleanup := client.Post("/v2/jobs/"+uuid.String()+"/runs", body) | ||
| defer cleanup() | ||
| cltest.AssertServerResponse(t, response, http.StatusOK) | ||
|
|
||
| var parsedResponse presenters.PipelineRunResource | ||
| err := web.ParseJSONAPIResponse(cltest.ParseResponseBody(t, response), &parsedResponse) | ||
| assert.NoError(t, err) | ||
| assert.NotNil(t, parsedResponse.ID) | ||
| assert.NotNil(t, parsedResponse.CreatedAt) | ||
| assert.NotNil(t, parsedResponse.FinishedAt) | ||
| require.Len(t, parsedResponse.TaskRuns, 3) | ||
| } | ||
| client := app.NewHTTPClient(nil) | ||
| // Bridge names are arbitrary; webhook creation is rejected before bridge validation. | ||
| tomlStr := testspecs.GetWebhookSpecNoBody(uuid.New(), uuid.NewString(), uuid.NewString()) | ||
| body, err := json.Marshal(web.CreateJobRequest{TOML: tomlStr}) | ||
| require.NoError(t, err) | ||
| response, cleanup := client.Post("/v2/jobs", bytes.NewReader(body)) | ||
| defer cleanup() | ||
| cltest.AssertServerResponse(t, response, http.StatusUnprocessableEntity) | ||
| require.Contains(t, string(cltest.ParseResponseBody(t, response)), "job type webhook has been removed") | ||
| } |
| ExternalInitiatorsEnabled = false # Default | ||
| ``` | ||
| ExternalInitiatorsEnabled enables the External Initiator feature. If disabled, `webhook` jobs can ONLY be initiated by a logged-in user. If enabled, `webhook` jobs can be initiated by a whitelisted external initiator. | ||
| ExternalInitiatorsEnabled enables the External Initiator feature for legacy webhook job runs via external initiators. Webhook jobs can no longer be created; existing rows are read-only and runs are rejected. |
pavel-raykov
previously approved these changes
May 29, 2026
|
pavel-raykov
approved these changes
May 29, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR removes the Webhook job, but keeps the db tables and the UI elements, but removes all webhook job logic.