Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit576e1f4

Browse files
DanielleMaywooddannykoppingmafredri
authored
feat!: allow disabling notifications (#15509)
Resolves#15513Disables notifications when both `$CODER_NOTIFICATIONS_WEBHOOK_ENDPOINT` and `$CODER_EMAIL_SMARTHOST` are unset.Breaking change: `$CODER_EMAIL_SMARTHOST` is no longer set by default as `localhost:587`, meaning any deployments that make use of this default value will need to add it back.---------Co-authored-by: Danny Kopping <danny@coder.com>Co-authored-by: Mathias Fredriksson <mafredri@gmail.com>
1 parent1c08580 commit576e1f4

File tree

15 files changed

+153
-99
lines changed

15 files changed

+153
-99
lines changed

‎cli/server.go‎

Lines changed: 42 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -876,31 +876,39 @@ func (r *RootCmd) Server(newAPI func(context.Context, *coderd.Options) (*coderd.
876876
}
877877

878878
// Manage notifications.
879-
cfg:=options.DeploymentValues.Notifications
880-
metrics:=notifications.NewMetrics(options.PrometheusRegistry)
881-
helpers:=templateHelpers(options)
879+
var (
880+
notificationsCfg=options.DeploymentValues.Notifications
881+
notificationsManager*notifications.Manager
882+
)
882883

883-
// The enqueuer is responsible for enqueueing notifications to the given store.
884-
enqueuer,err:=notifications.NewStoreEnqueuer(cfg,options.Database,helpers,logger.Named("notifications.enqueuer"),quartz.NewReal())
885-
iferr!=nil {
886-
returnxerrors.Errorf("failed to instantiate notification store enqueuer: %w",err)
887-
}
888-
options.NotificationsEnqueuer=enqueuer
884+
ifnotificationsCfg.Enabled() {
885+
metrics:=notifications.NewMetrics(options.PrometheusRegistry)
886+
helpers:=templateHelpers(options)
889887

890-
// The notification manager is responsible for:
891-
// - creating notifiers and managing their lifecycles (notifiers are responsible for dequeueing/sending notifications)
892-
// - keeping the store updated with status updates
893-
notificationsManager,err:=notifications.NewManager(cfg,options.Database,helpers,metrics,logger.Named("notifications.manager"))
894-
iferr!=nil {
895-
returnxerrors.Errorf("failed to instantiate notification manager: %w",err)
896-
}
888+
// The enqueuer is responsible for enqueueing notifications to the given store.
889+
enqueuer,err:=notifications.NewStoreEnqueuer(notificationsCfg,options.Database,helpers,logger.Named("notifications.enqueuer"),quartz.NewReal())
890+
iferr!=nil {
891+
returnxerrors.Errorf("failed to instantiate notification store enqueuer: %w",err)
892+
}
893+
options.NotificationsEnqueuer=enqueuer
894+
895+
// The notification manager is responsible for:
896+
// - creating notifiers and managing their lifecycles (notifiers are responsible for dequeueing/sending notifications)
897+
// - keeping the store updated with status updates
898+
notificationsManager,err=notifications.NewManager(notificationsCfg,options.Database,helpers,metrics,logger.Named("notifications.manager"))
899+
iferr!=nil {
900+
returnxerrors.Errorf("failed to instantiate notification manager: %w",err)
901+
}
897902

898-
// nolint:gocritic // We need to run the manager in a notifier context.
899-
notificationsManager.Run(dbauthz.AsNotifier(ctx))
903+
// nolint:gocritic // We need to run the manager in a notifier context.
904+
notificationsManager.Run(dbauthz.AsNotifier(ctx))
900905

901-
// Run report generator to distribute periodic reports.
902-
notificationReportGenerator:=reports.NewReportGenerator(ctx,logger.Named("notifications.report_generator"),options.Database,options.NotificationsEnqueuer,quartz.NewReal())
903-
defernotificationReportGenerator.Close()
906+
// Run report generator to distribute periodic reports.
907+
notificationReportGenerator:=reports.NewReportGenerator(ctx,logger.Named("notifications.report_generator"),options.Database,options.NotificationsEnqueuer,quartz.NewReal())
908+
defernotificationReportGenerator.Close()
909+
}else {
910+
cliui.Info(inv.Stdout,"Notifications are currently disabled as there are no configured delivery methods. See https://coder.com/docs/admin/monitoring/notifications#delivery-methods for more details.")
911+
}
904912

905913
// Since errCh only has one buffered slot, all routines
906914
// sending on it must be wrapped in a select/default to
@@ -1077,17 +1085,19 @@ func (r *RootCmd) Server(newAPI func(context.Context, *coderd.Options) (*coderd.
10771085
// Cancel any remaining in-flight requests.
10781086
shutdownConns()
10791087

1080-
// Stop the notification manager, which will cause any buffered updates to the store to be flushed.
1081-
// If the Stop() call times out, messages that were sent but not reflected as such in the store will have
1082-
// their leases expire after a period of time and will be re-queued for sending.
1083-
// See CODER_NOTIFICATIONS_LEASE_PERIOD.
1084-
cliui.Info(inv.Stdout,"Shutting down notifications manager..."+"\n")
1085-
err=shutdownWithTimeout(notificationsManager.Stop,5*time.Second)
1086-
iferr!=nil {
1087-
cliui.Warnf(inv.Stderr,"Notifications manager shutdown took longer than 5s, "+
1088-
"this may result in duplicate notifications being sent: %s\n",err)
1089-
}else {
1090-
cliui.Info(inv.Stdout,"Gracefully shut down notifications manager\n")
1088+
ifnotificationsManager!=nil {
1089+
// Stop the notification manager, which will cause any buffered updates to the store to be flushed.
1090+
// If the Stop() call times out, messages that were sent but not reflected as such in the store will have
1091+
// their leases expire after a period of time and will be re-queued for sending.
1092+
// See CODER_NOTIFICATIONS_LEASE_PERIOD.
1093+
cliui.Info(inv.Stdout,"Shutting down notifications manager..."+"\n")
1094+
err=shutdownWithTimeout(notificationsManager.Stop,5*time.Second)
1095+
iferr!=nil {
1096+
cliui.Warnf(inv.Stderr,"Notifications manager shutdown took longer than 5s, "+
1097+
"this may result in duplicate notifications being sent: %s\n",err)
1098+
}else {
1099+
cliui.Info(inv.Stdout,"Gracefully shut down notifications manager\n")
1100+
}
10911101
}
10921102

10931103
// Shut down provisioners before waiting for WebSockets

‎cli/testdata/coder_server_--help.golden‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,7 @@ Configure how emails are sent.
118118
--email-hello string, $CODER_EMAIL_HELLO (default: localhost)
119119
The hostname identifying the SMTP server.
120120

121-
--email-smarthosthost:port, $CODER_EMAIL_SMARTHOST (default: localhost:587)
121+
--email-smarthoststring, $CODER_EMAIL_SMARTHOST
122122
The intermediary SMTP host through which emails are sent.
123123

124124
EMAIL / EMAIL AUTHENTICATION OPTIONS:
@@ -413,7 +413,7 @@ Configure how email notifications are sent.
413413
The hostname identifying the SMTP server.
414414
DEPRECATED: Use --email-hello instead.
415415

416-
--notifications-email-smarthosthost:port, $CODER_NOTIFICATIONS_EMAIL_SMARTHOST
416+
--notifications-email-smarthoststring, $CODER_NOTIFICATIONS_EMAIL_SMARTHOST
417417
The intermediary SMTP host through which emails are sent.
418418
DEPRECATED: Use --email-smarthost instead.
419419

‎cli/testdata/server-config.yaml.golden‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -524,8 +524,8 @@ email:
524524
# (default: <unset>, type: string)
525525
from: ""
526526
# The intermediary SMTP host through which emails are sent.
527-
# (default:localhost:587, type:host:port)
528-
smarthost:localhost:587
527+
# (default:<unset>, type:string)
528+
smarthost:""
529529
# The hostname identifying the SMTP server.
530530
# (default: localhost, type: string)
531531
hello: localhost
@@ -577,8 +577,8 @@ notifications:
577577
# (default: <unset>, type: string)
578578
from: ""
579579
# The intermediary SMTP host through which emails are sent.
580-
# (default: <unset>, type:host:port)
581-
smarthost:localhost:587
580+
# (default: <unset>, type:string)
581+
smarthost:""
582582
# The hostname identifying the SMTP server.
583583
# (default: <unset>, type: string)
584584
hello: localhost

‎coderd/apidoc/docs.go‎

Lines changed: 1 addition & 5 deletions
Some generated files are not rendered by default. Learn more aboutcustomizing how changed files appear on GitHub.

‎coderd/apidoc/swagger.json‎

Lines changed: 1 addition & 5 deletions
Some generated files are not rendered by default. Learn more aboutcustomizing how changed files appear on GitHub.

‎coderd/notifications/dispatch/smtp.go‎

Lines changed: 11 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -34,11 +34,10 @@ import (
3434
)
3535

3636
var (
37-
ValidationNoFromAddressErr=xerrors.New("no 'from' address defined")
38-
ValidationNoToAddressErr=xerrors.New("no 'to' address(es) defined")
39-
ValidationNoSmarthostHostErr=xerrors.New("smarthost 'host' is not defined, or is invalid")
40-
ValidationNoSmarthostPortErr=xerrors.New("smarthost 'port' is not defined, or is invalid")
41-
ValidationNoHelloErr=xerrors.New("'hello' not defined")
37+
ValidationNoFromAddressErr=xerrors.New("'from' address not defined")
38+
ValidationNoToAddressErr=xerrors.New("'to' address(es) not defined")
39+
ValidationNoSmarthostErr=xerrors.New("'smarthost' address not defined")
40+
ValidationNoHelloErr=xerrors.New("'hello' not defined")
4241

4342
//go:embed smtp/html.gotmpl
4443
htmlTemplatestring
@@ -521,15 +520,14 @@ func (s *SMTPHandler) validateToAddrs(to string) ([]string, error) {
521520
// Does not allow overriding.
522521
// nolint:revive // documented.
523522
func (s*SMTPHandler)smarthost() (string,string,error) {
524-
host:=s.cfg.Smarthost.Host
525-
port:=s.cfg.Smarthost.Port
526-
527-
// We don't validate the contents themselves; this will be done by the underlying SMTP library.
528-
ifhost=="" {
529-
return"","",ValidationNoSmarthostHostErr
523+
smarthost:=strings.TrimSpace(string(s.cfg.Smarthost))
524+
ifsmarthost=="" {
525+
return"","",ValidationNoSmarthostErr
530526
}
531-
ifport=="" {
532-
return"","",ValidationNoSmarthostPortErr
527+
528+
host,port,err:=net.SplitHostPort(string(s.cfg.Smarthost))
529+
iferr!=nil {
530+
return"","",xerrors.Errorf("split host port: %w",err)
533531
}
534532

535533
returnhost,port,nil

‎coderd/notifications/dispatch/smtp_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -440,7 +440,7 @@ func TestSMTP(t *testing.T) {
440440

441441
varhp serpent.HostPort
442442
require.NoError(t,hp.Set(listen.Addr().String()))
443-
tc.cfg.Smarthost=hp
443+
tc.cfg.Smarthost=serpent.String(hp.String())
444444

445445
handler:=dispatch.NewSMTPHandler(tc.cfg,logger.Named("smtp"))
446446

‎coderd/notifications/notifications_test.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,7 @@ func TestSMTPDispatch(t *testing.T) {
154154
cfg:=defaultNotificationsConfig(method)
155155
cfg.SMTP= codersdk.NotificationsEmailConfig{
156156
From:from,
157-
Smarthost: serpent.HostPort{Host:"localhost",Port:fmt.Sprintf("%d",mockSMTPSrv.PortNumber())},
157+
Smarthost:serpent.String(fmt.Sprintf("localhost:%d",mockSMTPSrv.PortNumber())),
158158
Hello:"localhost",
159159
}
160160
handler:=newDispatchInterceptor(dispatch.NewSMTPHandler(cfg.SMTP,logger.Named("smtp")))
@@ -1112,7 +1112,7 @@ func TestNotificationTemplates_Golden(t *testing.T) {
11121112

11131113
varhp serpent.HostPort
11141114
require.NoError(t,hp.Set(listen.Addr().String()))
1115-
smtpConfig.Smarthost=hp
1115+
smtpConfig.Smarthost=serpent.String(hp.String())
11161116

11171117
// Start mock SMTP server in the background.
11181118
varwg sync.WaitGroup
@@ -1523,7 +1523,7 @@ func TestCustomNotificationMethod(t *testing.T) {
15231523
cfg.SMTP= codersdk.NotificationsEmailConfig{
15241524
From:"danny@coder.com",
15251525
Hello:"localhost",
1526-
Smarthost: serpent.HostPort{Host:"localhost",Port:fmt.Sprintf("%d",mockSMTPSrv.PortNumber())},
1526+
Smarthost:serpent.String(fmt.Sprintf("localhost:%d",mockSMTPSrv.PortNumber())),
15271527
}
15281528
cfg.Webhook= codersdk.NotificationsWebhookConfig{
15291529
Endpoint:*serpent.URLOf(endpoint),

‎codersdk/deployment.go‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -686,11 +686,15 @@ type NotificationsConfig struct {
686686
WebhookNotificationsWebhookConfig`json:"webhook" typescript:",notnull"`
687687
}
688688

689+
func (n*NotificationsConfig)Enabled()bool {
690+
returnn.SMTP.Smarthost!=""||n.Webhook.Endpoint!= serpent.URL{}
691+
}
692+
689693
typeNotificationsEmailConfigstruct {
690694
// The sender's address.
691695
From serpent.String`json:"from" typescript:",notnull"`
692696
// The intermediary SMTP host through which emails are sent (host:port).
693-
Smarthost serpent.HostPort`json:"smarthost" typescript:",notnull"`
697+
Smarthost serpent.String`json:"smarthost" typescript:",notnull"`
694698
// The hostname identifying the SMTP server.
695699
Hello serpent.String`json:"hello" typescript:",notnull"`
696700

@@ -1028,7 +1032,6 @@ when required by your organization's security policy.`,
10281032
Description:"The intermediary SMTP host through which emails are sent.",
10291033
Flag:"email-smarthost",
10301034
Env:"CODER_EMAIL_SMARTHOST",
1031-
Default:"localhost:587",// To pass validation.
10321035
Value:&c.Notifications.SMTP.Smarthost,
10331036
Group:&deploymentGroupEmail,
10341037
YAML:"smarthost",

‎codersdk/deployment_test.go‎

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -568,3 +568,69 @@ func TestPremiumSuperSet(t *testing.T) {
568568
require.NotContains(t,enterprise.Features(),"","enterprise should not contain empty string")
569569
require.NotContains(t,premium.Features(),"","premium should not contain empty string")
570570
}
571+
572+
funcTestNotificationsCanBeDisabled(t*testing.T) {
573+
t.Parallel()
574+
575+
tests:= []struct {
576+
namestring
577+
expectNotificationsEnabledbool
578+
environment []serpent.EnvVar
579+
}{
580+
{
581+
name:"NoDeliveryMethodSet",
582+
environment: []serpent.EnvVar{},
583+
expectNotificationsEnabled:false,
584+
},
585+
{
586+
name:"SMTP_DeliveryMethodSet",
587+
environment: []serpent.EnvVar{
588+
{
589+
Name:"CODER_EMAIL_SMARTHOST",
590+
Value:"localhost:587",
591+
},
592+
},
593+
expectNotificationsEnabled:true,
594+
},
595+
{
596+
name:"Webhook_DeliveryMethodSet",
597+
environment: []serpent.EnvVar{
598+
{
599+
Name:"CODER_NOTIFICATIONS_WEBHOOK_ENDPOINT",
600+
Value:"https://example.com/webhook",
601+
},
602+
},
603+
expectNotificationsEnabled:true,
604+
},
605+
{
606+
name:"WebhookAndSMTP_DeliveryMethodSet",
607+
environment: []serpent.EnvVar{
608+
{
609+
Name:"CODER_NOTIFICATIONS_WEBHOOK_ENDPOINT",
610+
Value:"https://example.com/webhook",
611+
},
612+
{
613+
Name:"CODER_EMAIL_SMARTHOST",
614+
Value:"localhost:587",
615+
},
616+
},
617+
expectNotificationsEnabled:true,
618+
},
619+
}
620+
621+
for_,tt:=rangetests {
622+
tt:=tt
623+
624+
t.Run(tt.name,func(t*testing.T) {
625+
t.Parallel()
626+
627+
dv:= codersdk.DeploymentValues{}
628+
opts:=dv.Options()
629+
630+
err:=opts.ParseEnv(tt.environment)
631+
require.NoError(t,err)
632+
633+
require.Equal(t,tt.expectNotificationsEnabled,dv.Notifications.Enabled())
634+
})
635+
}
636+
}

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp