From ea9f3af325e6b9b87da519ab6e66fd36ec5743a1 Mon Sep 17 00:00:00 2001 From: Abhinav Singh Date: Sat, 9 Nov 2024 00:46:41 +0000 Subject: [PATCH] fix: [CODE-2728]: fix webhook internal url to not read from db (#2953) --- app/services/webhook/service.go | 8 +++-- app/services/webhook/trigger.go | 19 +++-------- app/services/webhook/url_provider.go | 34 +++++++++++++++++++ .../webhook/url_provider_interface.go | 25 ++++++++++++++ app/services/webhook/wire.go | 7 ++++ cmd/gitness/wire_gen.go | 3 +- 6 files changed, 78 insertions(+), 18 deletions(-) create mode 100644 app/services/webhook/url_provider.go create mode 100644 app/services/webhook/url_provider_interface.go diff --git a/app/services/webhook/service.go b/app/services/webhook/service.go index 9e74265a8..7eece64ee 100644 --- a/app/services/webhook/service.go +++ b/app/services/webhook/service.go @@ -100,7 +100,8 @@ type Service struct { secureHTTPClientInternal *http.Client insecureHTTPClientInternal *http.Client - config Config + config Config + webhookURLProvider URLProvider } func NewService( @@ -120,7 +121,7 @@ func NewService( git git.Interface, encrypter encrypt.Encrypter, labelStore store.LabelStore, - + webhookURLProvider URLProvider, ) (*Service, error) { if err := config.Prepare(); err != nil { return nil, fmt.Errorf("provided webhook service config is invalid: %w", err) @@ -146,7 +147,8 @@ func NewService( config: config, - labelStore: labelStore, + labelStore: labelStore, + webhookURLProvider: webhookURLProvider, } _, err := gitReaderFactory.Launch(ctx, eventsReaderGroupName, config.EventReaderName, diff --git a/app/services/webhook/trigger.go b/app/services/webhook/trigger.go index 0edf89469..57d830eff 100644 --- a/app/services/webhook/trigger.go +++ b/app/services/webhook/trigger.go @@ -287,7 +287,11 @@ func (s *Service) executeWebhook(ctx context.Context, webhook *types.Webhook, tr // NOTE: if the body is an io.Reader, the value is used as response body as is, otherwise it'll be JSON serialized. func (s *Service) prepareHTTPRequest(ctx context.Context, execution *types.WebhookExecution, triggerType enum.WebhookTrigger, webhook *types.Webhook, body any) (*http.Request, error) { - execution.Request.URL = s.getWebhookURL(ctx, webhook) + url, err := s.webhookURLProvider.GetWebhookURL(ctx, webhook) + if err != nil { + return nil, fmt.Errorf("webhook url is not resolvable: %w", err) + } + execution.Request.URL = url // Serialize body before anything else. // This allows the user to retrigger the execution even in case of bad URL. @@ -370,19 +374,6 @@ func (s *Service) prepareHTTPRequest(ctx context.Context, execution *types.Webho return req, nil } -func (s *Service) getWebhookURL(ctx context.Context, webhook *types.Webhook) string { - // in case a webhook is internal use the specified URL if not empty instead of using one saved in db. - if webhook.Internal && s.config.InternalWebhooksURL != "" { - return s.config.InternalWebhooksURL - } - - if webhook.Internal && s.config.InternalWebhooksURL == "" { - log.Ctx(ctx).Error().Msg("internal webhook URL is empty, falling back to one in db") - } - // set URL as is (already has been validated, any other error will be caught in request creation) - return webhook.URL -} - func (s *Service) toXHeader(name string) string { return fmt.Sprintf("X-%s-%s", s.config.HeaderIdentity, name) } diff --git a/app/services/webhook/url_provider.go b/app/services/webhook/url_provider.go new file mode 100644 index 000000000..a1975843a --- /dev/null +++ b/app/services/webhook/url_provider.go @@ -0,0 +1,34 @@ +// Copyright 2023 Harness, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package webhook + +import ( + "context" + + "github.com/harness/gitness/types" +) + +var _ URLProvider = (*GitnessURLProvider)(nil) + +type GitnessURLProvider struct{} + +func NewURLProvider(_ context.Context) *GitnessURLProvider { + return &GitnessURLProvider{} +} + +func (u *GitnessURLProvider) GetWebhookURL(_ context.Context, webhook *types.Webhook) (string, error) { + // set URL as is (already has been validated, any other error will be caught in request creation) + return webhook.URL, nil +} diff --git a/app/services/webhook/url_provider_interface.go b/app/services/webhook/url_provider_interface.go new file mode 100644 index 000000000..75d2b3ab7 --- /dev/null +++ b/app/services/webhook/url_provider_interface.go @@ -0,0 +1,25 @@ +// Copyright 2023 Harness, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package webhook + +import ( + "context" + + "github.com/harness/gitness/types" +) + +type URLProvider interface { + GetWebhookURL(ctx context.Context, webhook *types.Webhook) (string, error) +} diff --git a/app/services/webhook/wire.go b/app/services/webhook/wire.go index d64316466..fe6ebd60c 100644 --- a/app/services/webhook/wire.go +++ b/app/services/webhook/wire.go @@ -32,6 +32,7 @@ import ( // WireSet provides a wire set for this package. var WireSet = wire.NewSet( ProvideService, + ProvideURLProvider, ) func ProvideService( @@ -51,6 +52,7 @@ func ProvideService( git git.Interface, encrypter encrypt.Encrypter, labelStore store.LabelStore, + webhookURLProvider URLProvider, ) (*Service, error) { return NewService( ctx, @@ -68,5 +70,10 @@ func ProvideService( git, encrypter, labelStore, + webhookURLProvider, ) } + +func ProvideURLProvider(ctx context.Context) URLProvider { + return NewURLProvider(ctx) +} diff --git a/cmd/gitness/wire_gen.go b/cmd/gitness/wire_gen.go index 5cd528cbe..a9f151bc0 100644 --- a/cmd/gitness/wire_gen.go +++ b/cmd/gitness/wire_gen.go @@ -365,7 +365,8 @@ func initSystem(ctx context.Context, config *types.Config) (*server.System, erro webhookConfig := server.ProvideWebhookConfig(config) webhookStore := database.ProvideWebhookStore(db) webhookExecutionStore := database.ProvideWebhookExecutionStore(db) - webhookService, err := webhook.ProvideService(ctx, webhookConfig, transactor, readerFactory, eventsReaderFactory, webhookStore, webhookExecutionStore, spaceStore, repoStore, pullReqStore, pullReqActivityStore, provider, principalStore, gitInterface, encrypter, labelStore) + urlProvider := webhook.ProvideURLProvider(ctx) + webhookService, err := webhook.ProvideService(ctx, webhookConfig, transactor, readerFactory, eventsReaderFactory, webhookStore, webhookExecutionStore, spaceStore, repoStore, pullReqStore, pullReqActivityStore, provider, principalStore, gitInterface, encrypter, labelStore, urlProvider) if err != nil { return nil, err }