Merge pull request #1141 from cappyzawa/feat/optional-address-field

Make address field optional for providers that generate URLs internally

Author: matheuscscp

docs/spec/v1beta3/providers.md

@@ -544,7 +544,7 @@ stringData:
 ##### Telegram
 
 When `.spec.type` is set to `telegram`, the controller will send a payload for
-an [Event](events.md#event-structure) to the provided Telegram [Address](#address).
+an [Event](events.md#event-structure) to the Telegram Bot API.
 
 The Event will be formatted into a message string, with the metadata attached
 as a list of key-value pairs.
@@ -556,12 +556,14 @@ or the username (`@username`) of the target channel.
 
 This Provider type does not support the configuration of a [certificate secret reference](#certificate-secret-reference).
 
+**Note:** The Telegram provider always ignores the `address` field and uses the default 
+Telegram Bot API endpoint (`https://api.telegram.org`).
+
 ###### Telegram example
 
 To configure a Provider for Telegram, create a Secret with [the `token`](#token-example)
 obtained from [the BotFather](https://core.telegram.org/bots#how-do-i-create-a-bot),
-and a `telegram` Provider with a [Secret reference](#secret-reference), and the
-`address` set to `https://api.telegram.org`.
+and a `telegram` Provider with a [Secret reference](#secret-reference).
 
 ```yaml
 ---
@@ -572,7 +574,6 @@ metadata:
   namespace: default
 spec:
   type: telegram
-  address: https://api.telegram.org
   channel: "@fluxcd" # or "-1557265138" (channel id) or "-1552289257:1" (forum chat id with topic id)
   secretRef:
     name: telegram-token

internal/notifier/azure_eventhub.go

@@ -179,6 +179,10 @@ func newManagedIdentityToken(ctx context.Context, proxy, serviceAccountName, pro
 
 // validateAuthOptions checks if the authentication options are valid
 func validateAuthOptions(endpointURL, token, serviceAccountName string) error {
+	if endpointURL == "" {
+		return fmt.Errorf("endpoint URL cannot be empty")
+	}
+
 	if isSASAuth(endpointURL) {
 		if err := validateSASAuth(token, serviceAccountName); err != nil {
 			return err

internal/notifier/azure_eventhub_test.go

@@ -72,6 +72,13 @@ func TestNewAzureEventHub(t *testing.T) {
 			eventHubNamespace:  "namespace",
 			err:                errors.New("invalid authentication options: serviceAccountName and jwt token authentication cannot be set at the same time"),
 		},
+		{
+			name:              "empty endpoint URL",
+			endpointURL:       "",
+			token:             "test-token",
+			eventHubNamespace: "namespace",
+			err:               errors.New("invalid authentication options: endpoint URL cannot be empty"),
+		},
 	}
 
 	for _, tt := range tests {

internal/notifier/factory.go

@@ -203,11 +203,17 @@ func WithServiceAccount(serviceAccountName string) Option {
 	}
 }
 
-// NewFactory creates a new notifier factory with the given URL and optional configurations.
-func NewFactory(ctx context.Context, url string, opts ...Option) *Factory {
+// WithURL sets the webhook URL for the notifier.
+func WithURL(url string) Option {
+	return func(o *notifierOptions) {
+		o.URL = url
+	}
+}
+
+// NewFactory creates a new notifier factory with optional configurations.
+func NewFactory(ctx context.Context, opts ...Option) *Factory {
 	options := notifierOptions{
 		Context: ctx,
-		URL:     url,
 	}
 
 	for _, opt := range opts {
@@ -220,24 +226,12 @@ func NewFactory(ctx context.Context, url string, opts ...Option) *Factory {
 }
 
 func (f Factory) Notifier(provider string) (Interface, error) {
-	if f.URL == "" {
-		return &NopNotifier{}, nil
+	notifier, ok := notifiers[provider]
+	if !ok {
+		return nil, fmt.Errorf("provider %s not supported", provider)
 	}
 
-	var (
-		n   Interface
-		err error
-	)
-	if notifier, ok := notifiers[provider]; ok {
-		n, err = notifier(f.notifierOptions)
-	} else {
-		err = fmt.Errorf("provider %s not supported", provider)
-	}
-
-	if err != nil {
-		n = &NopNotifier{}
-	}
-	return n, err
+	return notifier(f.notifierOptions)
 }
 
 func genericNotifierFunc(opts notifierOptions) (Interface, error) {

internal/notifier/nop.go

@@ -34,6 +34,10 @@ type Sentry struct {
 
 // NewSentry creates a Sentry client from the provided Data Source Name (DSN)
 func NewSentry(certPool *x509.CertPool, dsn string, environment string) (*Sentry, error) {
+	if dsn == "" {
+		return nil, fmt.Errorf("DSN cannot be empty")
+	}
+
 	tr := &http.Transport{}
 	if certPool != nil {
 		tr = &http.Transport{

internal/notifier/sentry.go

@@ -17,6 +17,7 @@ limitations under the License.
 package notifier
 
 import (
+	"errors"
 	"testing"
 	"time"
 
@@ -25,15 +26,42 @@ import (
 	"github.com/stretchr/testify/assert"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-
-	"github.com/stretchr/testify/require"
 )
 
 func TestNewSentry(t *testing.T) {
-	s, err := NewSentry(nil, "https://test@localhost/1", "foo")
-	require.NoError(t, err)
-	assert.Equal(t, s.Client.Options().Dsn, "https://test@localhost/1")
-	assert.Equal(t, s.Client.Options().Environment, "foo")
+	tests := []struct {
+		name        string
+		dsn         string
+		environment string
+		err         error
+	}{
+		{
+			name:        "valid DSN",
+			dsn:         "https://test@localhost/1",
+			environment: "foo",
+			err:         nil,
+		},
+		{
+			name:        "empty DSN",
+			dsn:         "",
+			environment: "foo",
+			err:         errors.New("DSN cannot be empty"),
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			s, err := NewSentry(nil, tt.dsn, tt.environment)
+			if tt.err != nil {
+				assert.Error(t, err)
+				assert.Equal(t, tt.err, err)
+			} else {
+				assert.NoError(t, err)
+				assert.Equal(t, s.Client.Options().Dsn, tt.dsn)
+				assert.Equal(t, s.Client.Options().Environment, tt.environment)
+			}
+		})
+	}
 }
 
 func TestToSentryEvent(t *testing.T) {

internal/notifier/sentry_test.go

@@ -11,12 +11,12 @@ import (
 )
 
 const (
-	defaultTelegramBaseURL = "https://api.telegram.org/bot%s"
-	sendMessageMethodName  = "sendMessage"
+	telegramBaseURL       = "https://api.telegram.org/bot%s"
+	sendMessageMethodName = "sendMessage"
 )
 
 type Telegram struct {
-	URL      string
+	url      string
 	ProxyURL string
 	Channel  string
 	Token    string
@@ -39,10 +39,8 @@ func NewTelegram(proxyURL, channel, token string) (*Telegram, error) {
 		return nil, errors.New("empty Telegram token")
 	}
 
-	apiURL := fmt.Sprintf(defaultTelegramBaseURL, token)
-
 	return &Telegram{
-		URL:      apiURL,
+		url:      fmt.Sprintf(telegramBaseURL, token),
 		ProxyURL: proxyURL,
 		Channel:  channel,
 		Token:    token,
@@ -74,7 +72,7 @@ func (t *Telegram) Post(ctx context.Context, event eventv1.Event) error {
 		ParseMode: "MarkdownV2", // https://core.telegram.org/bots/api#markdownv2-style
 	}
 
-	apiURL, err := url.JoinPath(t.URL, sendMessageMethodName)
+	apiURL, err := url.JoinPath(t.url, sendMessageMethodName)
 	if err != nil {
 		return fmt.Errorf("failed to construct API URL: %w", err)
 	}

internal/notifier/telegram.go

@@ -63,7 +63,7 @@ func TestTelegram_Post(t *testing.T) {
 	telegram, err := NewTelegram("", "channel", "token")
 	require.NoError(t, err)
 
-	telegram.URL = ts.URL
+	telegram.url = ts.URL
 
 	ev := testEvent()
 	ev.Metadata["kubernetes.io/somekey"] = "some.value"

internal/notifier/telegram_test.go

@@ -358,7 +358,6 @@ func extractAuthFromSecret(ctx context.Context, kubeClient client.Client, provid
 // handling authentication, proxy settings, and TLS configuration.
 func createNotifier(ctx context.Context, kubeClient client.Client, provider *apiv1beta3.Provider,
 	commitStatus string, tokenCache *cache.TokenCache) (notifier.Interface, string, error) {
-
 	options := []notifier.Option{
 		notifier.WithTokenClient(kubeClient),
 		notifier.WithProviderName(provider.Name),
@@ -439,11 +438,11 @@ func createNotifier(ctx context.Context, kubeClient client.Client, provider *api
 		options = append(options, notifier.WithTLSConfig(tlsConfig))
 	}
 
-	if webhook == "" {
-		return nil, "", fmt.Errorf("provider has no address")
+	if webhook != "" {
+		options = append(options, notifier.WithURL(webhook))
 	}
 
-	factory := notifier.NewFactory(ctx, webhook, options...)
+	factory := notifier.NewFactory(ctx, options...)
 	sender, err := factory.Notifier(provider.Spec.Type)
 	if err != nil {
 		return nil, "", fmt.Errorf("failed to initialize notifier: %w", err)