Skip to content

Commit d1c85df

Browse files
cappyzawacappyzawa
committed
Unify BasicAuth processing using pkg/runtime/secrets
This commit refactors the createNotifier function to use pkg/runtime/secrets.BasicAuthFromSecret for standardized BasicAuth handling while maintaining token-first authentication precedence. Signed-off-by: cappyzawa <[email protected]>
1 parent 9e15025 commit d1c85df

File tree

1 file changed

+89
-101
lines changed

1 file changed

+89
-101
lines changed

‎internal/server/event_handlers.go‎

Lines changed: 89 additions & 101 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@ package server
1818

1919
import (
2020
"context"
21-
"crypto/tls"
2221
"errors"
2322
"fmt"
2423
"net/http"
@@ -306,147 +305,136 @@ func createCommitStatus(ctx context.Context, provider *apiv1beta3.Provider, even
306305
return commitStatus, nil
307306
}
308307

309-
// createNotifier returns a notifier.Interface for the given Provider.
310-
func createNotifier(ctx context.Context, kubeClient client.Client, provider *apiv1beta3.Provider, commitStatus string, tokenCache *pkgcache.TokenCache) (notifier.Interface, string, error) {
308+
// extractAuthFromSecret extracts notification-controller specific fields from the provider's secret
309+
// that are not handled by pkg/runtime/secrets (address, proxy, token, headers).
310+
// StandardizedSecret fields like BasicAuth, TLS, and ProxySecretRef are handled separately.
311+
func extractAuthFromSecret(ctx context.Context, kubeClient client.Client, provider *apiv1beta3.Provider) ([]notifier.Option, map[string][]byte, error) {
312+
options := []notifier.Option{}
311313

312-
webhook := provider.Spec.Address
313-
username := provider.Spec.Username
314-
// TODO: Remove deprecated proxy handling when Provider v1 is released.
315-
deprecatedProxy := provider.Spec.Proxy
316-
if deprecatedProxy != "" {
317-
log.FromContext(ctx).Error(nil, "warning: spec.proxy is deprecated, please use spec.proxySecretRef instead. Support for this field will be removed in v1.")
318-
}
319-
token := ""
320-
password := ""
321-
headers := make(map[string]string)
314+
secretName := types.NamespacedName{Namespace: provider.Namespace, Name: provider.Spec.SecretRef.Name}
322315
var secret corev1.Secret
323-
if provider.Spec.SecretRef != nil {
324-
secretName := types.NamespacedName{Namespace: provider.Namespace, Name: provider.Spec.SecretRef.Name}
325-
326-
err := kubeClient.Get(ctx, secretName, &secret)
327-
if err != nil {
328-
return nil, "", fmt.Errorf("failed to read secret: %w", err)
329-
}
330-
331-
if val, ok := secret.Data["address"]; ok {
332-
if len(val) > 2048 {
333-
return nil, "", fmt.Errorf("invalid address in secret: address exceeds maximum length of %d bytes", 2048)
334-
}
335-
webhook = strings.TrimSpace(string(val))
336-
}
337-
338-
if val, ok := secret.Data["password"]; ok {
339-
password = strings.TrimSpace(string(val))
340-
}
341-
342-
if val, ok := secret.Data["proxy"]; ok {
343-
deprecatedProxy = strings.TrimSpace(string(val))
344-
_, err := url.Parse(deprecatedProxy)
345-
if err != nil {
346-
return nil, "", fmt.Errorf("invalid 'proxy' in secret '%s/%s'", secret.Namespace, secret.Name)
347-
}
348-
log.FromContext(ctx).Error(nil, "warning: specifying proxy with 'proxy' key in the referenced secret is deprecated, use spec.proxySecretRef with 'address' key instead. Support for the 'proxy' key will be removed in v1.")
349-
}
316+
if err := kubeClient.Get(ctx, secretName, &secret); err != nil {
317+
return nil, nil, fmt.Errorf("failed to read secret: %w", err)
318+
}
350319

351-
if val, ok := secret.Data["token"]; ok {
352-
token = strings.TrimSpace(string(val))
320+
if val, ok := secret.Data["address"]; ok {
321+
if len(val) > 2048 {
322+
return nil, nil, fmt.Errorf("invalid address in secret: address exceeds maximum length of %d bytes", 2048)
353323
}
324+
}
354325

355-
if val, ok := secret.Data["username"]; ok {
356-
username = strings.TrimSpace(string(val))
357-
}
358-
359-
if h, ok := secret.Data["headers"]; ok {
360-
err := yaml.Unmarshal(h, &headers)
361-
if err != nil {
362-
return nil, "", fmt.Errorf("failed to read headers from secret: %w", err)
363-
}
326+
if val, ok := secret.Data["proxy"]; ok {
327+
deprecatedProxy := strings.TrimSpace(string(val))
328+
if _, err := url.Parse(deprecatedProxy); err != nil {
329+
return nil, nil, fmt.Errorf("invalid 'proxy' in secret '%s'", secretName.String())
364330
}
331+
log.FromContext(ctx).Error(nil, "warning: specifying proxy with 'proxy' key in the referenced secret is deprecated, use spec.proxySecretRef with 'address' key instead. Support for the 'proxy' key will be removed in v1.")
332+
options = append(options, notifier.WithProxyURL(deprecatedProxy))
365333
}
366334

367-
var proxy string
368-
if provider.Spec.ProxySecretRef != nil {
369-
proxyURL, err := secrets.ProxyURLFromSecret(ctx, kubeClient, provider.Spec.ProxySecretRef.Name, provider.Namespace)
370-
if err != nil {
371-
return nil, "", fmt.Errorf("failed to get proxy URL from secret: %w", err)
372-
}
373-
proxy = proxyURL.String()
374-
} else {
375-
proxy = deprecatedProxy
335+
if val, ok := secret.Data[secrets.TokenKey]; ok {
336+
options = append(options, notifier.WithToken(strings.TrimSpace(string(val))))
376337
}
377338

378-
var tlsConfig *tls.Config
379-
if provider.Spec.CertSecretRef != nil {
380-
var err error
381-
tlsConfig, err = secrets.TLSConfigFromSecret(
382-
ctx,
383-
kubeClient,
384-
provider.Spec.CertSecretRef.Name,
385-
provider.Namespace,
386-
)
387-
if err != nil {
388-
return nil, "", fmt.Errorf("failed to get TLS config: %w", err)
339+
if h, ok := secret.Data["headers"]; ok {
340+
headers := make(map[string]string)
341+
if err := yaml.Unmarshal(h, &headers); err != nil {
342+
return nil, nil, fmt.Errorf("failed to read headers from secret: %w", err)
389343
}
344+
options = append(options, notifier.WithHeaders(headers))
390345
}
391346

392-
if webhook == "" {
393-
return nil, "", fmt.Errorf("provider has no address")
394-
}
347+
return options, secret.Data, nil
348+
}
395349

350+
// createNotifier constructs a notifier interface from the provider configuration,
351+
// handling authentication, proxy settings, and TLS configuration.
352+
func createNotifier(ctx context.Context, kubeClient client.Client, provider *apiv1beta3.Provider, commitStatus string, tokenCache *pkgcache.TokenCache) (notifier.Interface, string, error) {
396353
options := []notifier.Option{
397354
notifier.WithTokenClient(kubeClient),
355+
notifier.WithProviderName(provider.Name),
356+
notifier.WithProviderNamespace(provider.Namespace),
398357
}
399358

400359
if commitStatus != "" {
401360
options = append(options, notifier.WithCommitStatus(commitStatus))
402361
}
403362

404-
if proxy != "" {
405-
options = append(options, notifier.WithProxyURL(proxy))
406-
}
407-
408-
if username != "" {
409-
options = append(options, notifier.WithUsername(username))
410-
}
411-
412363
if provider.Spec.Channel != "" {
413364
options = append(options, notifier.WithChannel(provider.Spec.Channel))
414365
}
415366

416-
if token != "" {
417-
options = append(options, notifier.WithToken(token))
367+
if provider.Spec.Username != "" {
368+
options = append(options, notifier.WithUsername(provider.Spec.Username))
418369
}
419370

420-
if len(headers) > 0 {
421-
options = append(options, notifier.WithHeaders(headers))
371+
if provider.Spec.ServiceAccountName != "" {
372+
options = append(options, notifier.WithServiceAccount(provider.Spec.ServiceAccountName))
422373
}
423374

424-
if tlsConfig != nil {
425-
options = append(options, notifier.WithTLSConfig(tlsConfig))
375+
if tokenCache != nil {
376+
options = append(options, notifier.WithTokenCache(tokenCache))
426377
}
427378

428-
if password != "" {
429-
options = append(options, notifier.WithPassword(password))
379+
// TODO: Remove deprecated proxy handling when Provider v1 is released.
380+
if provider.Spec.Proxy != "" {
381+
log.FromContext(ctx).Error(nil, "warning: spec.proxy is deprecated, please use spec.proxySecretRef instead. Support for this field will be removed in v1.")
382+
options = append(options, notifier.WithProxyURL(provider.Spec.Proxy))
430383
}
431384

432-
if provider.Name != "" {
433-
options = append(options, notifier.WithProviderName(provider.Name))
434-
}
385+
webhook := provider.Spec.Address
386+
var token string
387+
var secretData map[string][]byte
388+
389+
if provider.Spec.SecretRef != nil {
390+
secretOptions, sData, err := extractAuthFromSecret(ctx, kubeClient, provider)
391+
if err != nil {
392+
return nil, "", err
393+
}
394+
secretData = sData
395+
options = append(options, secretOptions...)
396+
397+
if secretData != nil {
398+
options = append(options, notifier.WithSecretData(secretData))
399+
}
400+
401+
if val, ok := secretData["address"]; ok {
402+
webhook = strings.TrimSpace(string(val))
403+
}
404+
if val, ok := secretData[secrets.TokenKey]; ok {
405+
token = strings.TrimSpace(string(val))
406+
}
435407

436-
if provider.Namespace != "" {
437-
options = append(options, notifier.WithProviderNamespace(provider.Namespace))
408+
user, pass, err := secrets.BasicAuthFromSecret(ctx, kubeClient, provider.Spec.SecretRef.Name, provider.Namespace)
409+
if err == nil {
410+
options = append(options, notifier.WithUsername(user))
411+
options = append(options, notifier.WithPassword(pass))
412+
}
438413
}
439414

440-
if secret.Data != nil {
441-
options = append(options, notifier.WithSecretData(secret.Data))
415+
if provider.Spec.ProxySecretRef != nil {
416+
proxyURL, err := secrets.ProxyURLFromSecret(ctx, kubeClient, provider.Spec.ProxySecretRef.Name, provider.Namespace)
417+
if err != nil {
418+
return nil, "", fmt.Errorf("failed to get proxy URL: %w", err)
419+
}
420+
options = append(options, notifier.WithProxyURL(proxyURL.String()))
442421
}
443422

444-
if tokenCache != nil {
445-
options = append(options, notifier.WithTokenCache(tokenCache))
423+
if provider.Spec.CertSecretRef != nil {
424+
tlsConfig, err := secrets.TLSConfigFromSecret(
425+
ctx,
426+
kubeClient,
427+
provider.Spec.CertSecretRef.Name,
428+
provider.Namespace,
429+
)
430+
if err != nil {
431+
return nil, "", fmt.Errorf("failed to get TLS config: %w", err)
432+
}
433+
options = append(options, notifier.WithTLSConfig(tlsConfig))
446434
}
447435

448-
if provider.Spec.ServiceAccountName != "" {
449-
options = append(options, notifier.WithServiceAccount(provider.Spec.ServiceAccountName))
436+
if webhook == "" {
437+
return nil, "", fmt.Errorf("provider has no address")
450438
}
451439

452440
factory := notifier.NewFactory(ctx, webhook, options...)

0 commit comments

Comments
 (0)