Skip to content

Commit 955d241

Browse files
committed
Make address field optional for providers that generate URLs internally
This change removes the generic address validation from event_handlers.go that was preventing address-optional providers from functioning without specifying a dummy address value. Some providers generate URLs internally and don't require external address configuration. This allows providers that generate URLs internally to work without requiring dummy address values in the provider configuration. Signed-off-by: cappyzawa <[email protected]>
1 parent e8a909f commit 955d241

File tree

11 files changed

+89
-77
lines changed

11 files changed

+89
-77
lines changed

docs/spec/v1beta3/providers.md

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -544,7 +544,7 @@ stringData:
544544
##### Telegram
545545

546546
When `.spec.type` is set to `telegram`, the controller will send a payload for
547-
an [Event](events.md#event-structure) to the provided Telegram [Address](#address).
547+
an [Event](events.md#event-structure) to the Telegram Bot API.
548548

549549
The Event will be formatted into a message string, with the metadata attached
550550
as a list of key-value pairs.
@@ -556,12 +556,14 @@ or the username (`@username`) of the target channel.
556556

557557
This Provider type does not support the configuration of a [certificate secret reference](#certificate-secret-reference).
558558

559+
**Note:** The Telegram provider always ignores the `address` field and uses the default
560+
Telegram Bot API endpoint (`https://api.telegram.org`).
561+
559562
###### Telegram example
560563

561564
To configure a Provider for Telegram, create a Secret with [the `token`](#token-example)
562565
obtained from [the BotFather](https://core.telegram.org/bots#how-do-i-create-a-bot),
563-
and a `telegram` Provider with a [Secret reference](#secret-reference), and the
564-
`address` set to `https://api.telegram.org`.
566+
and a `telegram` Provider with a [Secret reference](#secret-reference).
565567

566568
```yaml
567569
---
@@ -572,7 +574,6 @@ metadata:
572574
namespace: default
573575
spec:
574576
type: telegram
575-
address: https://api.telegram.org
576577
channel: "@fluxcd" # or "-1557265138" (channel id) or "-1552289257:1" (forum chat id with topic id)
577578
secretRef:
578579
name: telegram-token

internal/notifier/azure_eventhub.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -179,6 +179,10 @@ func newManagedIdentityToken(ctx context.Context, proxy, serviceAccountName, pro
179179

180180
// validateAuthOptions checks if the authentication options are valid
181181
func validateAuthOptions(endpointURL, token, serviceAccountName string) error {
182+
if endpointURL == "" {
183+
return fmt.Errorf("endpoint URL cannot be empty")
184+
}
185+
182186
if isSASAuth(endpointURL) {
183187
if err := validateSASAuth(token, serviceAccountName); err != nil {
184188
return err

internal/notifier/azure_eventhub_test.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,13 @@ func TestNewAzureEventHub(t *testing.T) {
7272
eventHubNamespace: "namespace",
7373
err: errors.New("invalid authentication options: serviceAccountName and jwt token authentication cannot be set at the same time"),
7474
},
75+
{
76+
name: "empty endpoint URL",
77+
endpointURL: "",
78+
token: "test-token",
79+
eventHubNamespace: "namespace",
80+
err: errors.New("invalid authentication options: endpoint URL cannot be empty"),
81+
},
7582
}
7683

7784
for _, tt := range tests {

internal/notifier/factory.go

Lines changed: 13 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -203,11 +203,17 @@ func WithServiceAccount(serviceAccountName string) Option {
203203
}
204204
}
205205

206-
// NewFactory creates a new notifier factory with the given URL and optional configurations.
207-
func NewFactory(ctx context.Context, url string, opts ...Option) *Factory {
206+
// WithURL sets the webhook URL for the notifier.
207+
func WithURL(url string) Option {
208+
return func(o *notifierOptions) {
209+
o.URL = url
210+
}
211+
}
212+
213+
// NewFactory creates a new notifier factory with optional configurations.
214+
func NewFactory(ctx context.Context, opts ...Option) *Factory {
208215
options := notifierOptions{
209216
Context: ctx,
210-
URL: url,
211217
}
212218

213219
for _, opt := range opts {
@@ -220,24 +226,12 @@ func NewFactory(ctx context.Context, url string, opts ...Option) *Factory {
220226
}
221227

222228
func (f Factory) Notifier(provider string) (Interface, error) {
223-
if f.URL == "" {
224-
return &NopNotifier{}, nil
229+
notifier, ok := notifiers[provider]
230+
if !ok {
231+
return nil, fmt.Errorf("provider %s not supported", provider)
225232
}
226233

227-
var (
228-
n Interface
229-
err error
230-
)
231-
if notifier, ok := notifiers[provider]; ok {
232-
n, err = notifier(f.notifierOptions)
233-
} else {
234-
err = fmt.Errorf("provider %s not supported", provider)
235-
}
236-
237-
if err != nil {
238-
n = &NopNotifier{}
239-
}
240-
return n, err
234+
return notifier(f.notifierOptions)
241235
}
242236

243237
func genericNotifierFunc(opts notifierOptions) (Interface, error) {

internal/notifier/nop.go

Lines changed: 0 additions & 29 deletions
This file was deleted.

internal/notifier/sentry.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,10 @@ type Sentry struct {
3434

3535
// NewSentry creates a Sentry client from the provided Data Source Name (DSN)
3636
func NewSentry(certPool *x509.CertPool, dsn string, environment string) (*Sentry, error) {
37+
if dsn == "" {
38+
return nil, fmt.Errorf("DSN cannot be empty")
39+
}
40+
3741
tr := &http.Transport{}
3842
if certPool != nil {
3943
tr = &http.Transport{

internal/notifier/sentry_test.go

Lines changed: 34 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ limitations under the License.
1717
package notifier
1818

1919
import (
20+
"errors"
2021
"testing"
2122
"time"
2223

@@ -25,15 +26,42 @@ import (
2526
"github.com/stretchr/testify/assert"
2627
corev1 "k8s.io/api/core/v1"
2728
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
28-
29-
"github.com/stretchr/testify/require"
3029
)
3130

3231
func TestNewSentry(t *testing.T) {
33-
s, err := NewSentry(nil, "https://test@localhost/1", "foo")
34-
require.NoError(t, err)
35-
assert.Equal(t, s.Client.Options().Dsn, "https://test@localhost/1")
36-
assert.Equal(t, s.Client.Options().Environment, "foo")
32+
tests := []struct {
33+
name string
34+
dsn string
35+
environment string
36+
err error
37+
}{
38+
{
39+
name: "valid DSN",
40+
dsn: "https://test@localhost/1",
41+
environment: "foo",
42+
err: nil,
43+
},
44+
{
45+
name: "empty DSN",
46+
dsn: "",
47+
environment: "foo",
48+
err: errors.New("DSN cannot be empty"),
49+
},
50+
}
51+
52+
for _, tt := range tests {
53+
t.Run(tt.name, func(t *testing.T) {
54+
s, err := NewSentry(nil, tt.dsn, tt.environment)
55+
if tt.err != nil {
56+
assert.Error(t, err)
57+
assert.Equal(t, tt.err, err)
58+
} else {
59+
assert.NoError(t, err)
60+
assert.Equal(t, s.Client.Options().Dsn, tt.dsn)
61+
assert.Equal(t, s.Client.Options().Environment, tt.environment)
62+
}
63+
})
64+
}
3765
}
3866

3967
func TestToSentryEvent(t *testing.T) {

internal/notifier/telegram.go

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -11,12 +11,12 @@ import (
1111
)
1212

1313
const (
14-
defaultTelegramBaseURL = "https://api.telegram.org/bot%s"
15-
sendMessageMethodName = "sendMessage"
14+
telegramBaseURL = "https://api.telegram.org/bot%s"
15+
sendMessageMethodName = "sendMessage"
1616
)
1717

1818
type Telegram struct {
19-
URL string
19+
url string
2020
ProxyURL string
2121
Channel string
2222
Token string
@@ -39,10 +39,8 @@ func NewTelegram(proxyURL, channel, token string) (*Telegram, error) {
3939
return nil, errors.New("empty Telegram token")
4040
}
4141

42-
apiURL := fmt.Sprintf(defaultTelegramBaseURL, token)
43-
4442
return &Telegram{
45-
URL: apiURL,
43+
url: fmt.Sprintf(telegramBaseURL, token),
4644
ProxyURL: proxyURL,
4745
Channel: channel,
4846
Token: token,
@@ -74,7 +72,7 @@ func (t *Telegram) Post(ctx context.Context, event eventv1.Event) error {
7472
ParseMode: "MarkdownV2", // https://core.telegram.org/bots/api#markdownv2-style
7573
}
7674

77-
apiURL, err := url.JoinPath(t.URL, sendMessageMethodName)
75+
apiURL, err := url.JoinPath(t.url, sendMessageMethodName)
7876
if err != nil {
7977
return fmt.Errorf("failed to construct API URL: %w", err)
8078
}

internal/notifier/telegram_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ func TestTelegram_Post(t *testing.T) {
6363
telegram, err := NewTelegram("", "channel", "token")
6464
require.NoError(t, err)
6565

66-
telegram.URL = ts.URL
66+
telegram.url = ts.URL
6767

6868
ev := testEvent()
6969
ev.Metadata["kubernetes.io/somekey"] = "some.value"

internal/server/event_handlers.go

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -358,7 +358,6 @@ func extractAuthFromSecret(ctx context.Context, kubeClient client.Client, provid
358358
// handling authentication, proxy settings, and TLS configuration.
359359
func createNotifier(ctx context.Context, kubeClient client.Client, provider *apiv1beta3.Provider,
360360
commitStatus string, tokenCache *cache.TokenCache) (notifier.Interface, string, error) {
361-
362361
options := []notifier.Option{
363362
notifier.WithTokenClient(kubeClient),
364363
notifier.WithProviderName(provider.Name),
@@ -439,11 +438,11 @@ func createNotifier(ctx context.Context, kubeClient client.Client, provider *api
439438
options = append(options, notifier.WithTLSConfig(tlsConfig))
440439
}
441440

442-
if webhook == "" {
443-
return nil, "", fmt.Errorf("provider has no address")
441+
if webhook != "" {
442+
options = append(options, notifier.WithURL(webhook))
444443
}
445444

446-
factory := notifier.NewFactory(ctx, webhook, options...)
445+
factory := notifier.NewFactory(ctx, options...)
447446
sender, err := factory.Notifier(provider.Spec.Type)
448447
if err != nil {
449448
return nil, "", fmt.Errorf("failed to initialize notifier: %w", err)

0 commit comments

Comments
 (0)