fix: [CODE-2728]: fix webhook internal url to not read from db (#2953)

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,

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)
 }

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
+}

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)
+}

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)
+}

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
 	}