Skip to content

Commit 585045b

Browse files
f0sself0ssel
authored
feat: support nested structs, structured arrays, and better secret value handling in config (#4727)
1 parent f9c6220 commit 585045b

File tree

11 files changed

+740
-560
lines changed

11 files changed

+740
-560
lines changed

‎cli/deployment/config.go

+401-344
Large diffs are not rendered by default.

‎cli/deployment/config_test.go

+42-42
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ func TestConfig(t *testing.T) {
2121
for _, tc := range []struct {
2222
Name string
2323
Env map[string]string
24-
Valid func(config codersdk.DeploymentConfig)
24+
Valid func(config *codersdk.DeploymentConfig)
2525
}{{
2626
Name: "Deployment",
2727
Env: map[string]string{
@@ -39,19 +39,19 @@ func TestConfig(t *testing.T) {
3939
"CODER_TELEMETRY_TRACE": "false",
4040
"CODER_WILDCARD_ACCESS_URL": "something-wildcard.com",
4141
},
42-
Valid: func(config codersdk.DeploymentConfig) {
42+
Valid: func(config *codersdk.DeploymentConfig) {
4343
require.Equal(t, config.Address.Value, "0.0.0.0:8443")
4444
require.Equal(t, config.AccessURL.Value, "https://dev.coder.com")
4545
require.Equal(t, config.PostgresURL.Value, "some-url")
46-
require.Equal(t, config.PprofAddress.Value, "something")
47-
require.Equal(t, config.PprofEnable.Value, true)
48-
require.Equal(t, config.PrometheusAddress.Value, "hello-world")
49-
require.Equal(t, config.PrometheusEnable.Value, true)
46+
require.Equal(t, config.Pprof.Address.Value, "something")
47+
require.Equal(t, config.Pprof.Enable.Value, true)
48+
require.Equal(t, config.Prometheus.Address.Value, "hello-world")
49+
require.Equal(t, config.Prometheus.Enable.Value, true)
5050
require.Equal(t, config.ProvisionerDaemons.Value, 5)
5151
require.Equal(t, config.SecureAuthCookie.Value, true)
5252
require.Equal(t, config.SSHKeygenAlgorithm.Value, "potato")
53-
require.Equal(t, config.TelemetryEnable.Value, false)
54-
require.Equal(t, config.TelemetryTrace.Value, false)
53+
require.Equal(t, config.Telemetry.Enable.Value, false)
54+
require.Equal(t, config.Telemetry.Trace.Value, false)
5555
require.Equal(t, config.WildcardAccessURL.Value, "something-wildcard.com")
5656
},
5757
}, {
@@ -66,15 +66,15 @@ func TestConfig(t *testing.T) {
6666
"CODER_DERP_SERVER_RELAY_URL": "1.1.1.1",
6767
"CODER_DERP_SERVER_STUN_ADDRESSES": "google.org",
6868
},
69-
Valid: func(config codersdk.DeploymentConfig) {
70-
require.Equal(t, config.DERPConfigPath.Value, "/example/path")
71-
require.Equal(t, config.DERPConfigURL.Value, "https://google.com")
72-
require.Equal(t, config.DERPServerEnable.Value, false)
73-
require.Equal(t, config.DERPServerRegionCode.Value, "something")
74-
require.Equal(t, config.DERPServerRegionID.Value, 123)
75-
require.Equal(t, config.DERPServerRegionName.Value, "Code-Land")
76-
require.Equal(t, config.DERPServerRelayURL.Value, "1.1.1.1")
77-
require.Equal(t, config.DERPServerSTUNAddresses.Value, []string{"google.org"})
69+
Valid: func(config *codersdk.DeploymentConfig) {
70+
require.Equal(t, config.DERP.Config.Path.Value, "/example/path")
71+
require.Equal(t, config.DERP.Config.URL.Value, "https://google.com")
72+
require.Equal(t, config.DERP.Server.Enable.Value, false)
73+
require.Equal(t, config.DERP.Server.RegionCode.Value, "something")
74+
require.Equal(t, config.DERP.Server.RegionID.Value, 123)
75+
require.Equal(t, config.DERP.Server.RegionName.Value, "Code-Land")
76+
require.Equal(t, config.DERP.Server.RelayURL.Value, "1.1.1.1")
77+
require.Equal(t, config.DERP.Server.STUNAddresses.Value, []string{"google.org"})
7878
},
7979
}, {
8080
Name: "Enterprise",
@@ -84,7 +84,7 @@ func TestConfig(t *testing.T) {
8484
"CODER_SCIM_API_KEY": "some-key",
8585
"CODER_USER_WORKSPACE_QUOTA": "10",
8686
},
87-
Valid: func(config codersdk.DeploymentConfig) {
87+
Valid: func(config *codersdk.DeploymentConfig) {
8888
require.Equal(t, config.AuditLogging.Value, false)
8989
require.Equal(t, config.BrowserOnly.Value, true)
9090
require.Equal(t, config.SCIMAPIKey.Value, "some-key")
@@ -100,19 +100,19 @@ func TestConfig(t *testing.T) {
100100
"CODER_TLS_ENABLE": "true",
101101
"CODER_TLS_MIN_VERSION": "tls10",
102102
},
103-
Valid: func(config codersdk.DeploymentConfig) {
104-
require.Len(t, config.TLSCertFiles.Value, 2)
105-
require.Equal(t, config.TLSCertFiles.Value[0], "/etc/acme-sh/dev.coder.com")
106-
require.Equal(t, config.TLSCertFiles.Value[1], "/etc/acme-sh/*.dev.coder.com")
103+
Valid: func(config *codersdk.DeploymentConfig) {
104+
require.Len(t, config.TLS.CertFiles.Value, 2)
105+
require.Equal(t, config.TLS.CertFiles.Value[0], "/etc/acme-sh/dev.coder.com")
106+
require.Equal(t, config.TLS.CertFiles.Value[1], "/etc/acme-sh/*.dev.coder.com")
107107

108-
require.Len(t, config.TLSKeyFiles.Value, 2)
109-
require.Equal(t, config.TLSKeyFiles.Value[0], "/etc/acme-sh/dev.coder.com")
110-
require.Equal(t, config.TLSKeyFiles.Value[1], "/etc/acme-sh/*.dev.coder.com")
108+
require.Len(t, config.TLS.KeyFiles.Value, 2)
109+
require.Equal(t, config.TLS.KeyFiles.Value[0], "/etc/acme-sh/dev.coder.com")
110+
require.Equal(t, config.TLS.KeyFiles.Value[1], "/etc/acme-sh/*.dev.coder.com")
111111

112-
require.Equal(t, config.TLSClientAuth.Value, "/some/path")
113-
require.Equal(t, config.TLSClientCAFile.Value, "/some/path")
114-
require.Equal(t, config.TLSEnable.Value, true)
115-
require.Equal(t, config.TLSMinVersion.Value, "tls10")
112+
require.Equal(t, config.TLS.ClientAuth.Value, "/some/path")
113+
require.Equal(t, config.TLS.ClientCAFile.Value, "/some/path")
114+
require.Equal(t, config.TLS.Enable.Value, true)
115+
require.Equal(t, config.TLS.MinVersion.Value, "tls10")
116116
},
117117
}, {
118118
Name: "OIDC",
@@ -124,13 +124,13 @@ func TestConfig(t *testing.T) {
124124
"CODER_OIDC_ALLOW_SIGNUPS": "false",
125125
"CODER_OIDC_SCOPES": "something,here",
126126
},
127-
Valid: func(config codersdk.DeploymentConfig) {
128-
require.Equal(t, config.OIDCIssuerURL.Value, "https://accounts.google.com")
129-
require.Equal(t, config.OIDCEmailDomain.Value, "coder.com")
130-
require.Equal(t, config.OIDCClientID.Value, "client")
131-
require.Equal(t, config.OIDCClientSecret.Value, "secret")
132-
require.Equal(t, config.OIDCAllowSignups.Value, false)
133-
require.Equal(t, config.OIDCScopes.Value, []string{"something", "here"})
127+
Valid: func(config *codersdk.DeploymentConfig) {
128+
require.Equal(t, config.OIDC.IssuerURL.Value, "https://accounts.google.com")
129+
require.Equal(t, config.OIDC.EmailDomain.Value, "coder.com")
130+
require.Equal(t, config.OIDC.ClientID.Value, "client")
131+
require.Equal(t, config.OIDC.ClientSecret.Value, "secret")
132+
require.Equal(t, config.OIDC.AllowSignups.Value, false)
133+
require.Equal(t, config.OIDC.Scopes.Value, []string{"something", "here"})
134134
},
135135
}, {
136136
Name: "GitHub",
@@ -141,12 +141,12 @@ func TestConfig(t *testing.T) {
141141
"CODER_OAUTH2_GITHUB_ALLOWED_TEAMS": "coder",
142142
"CODER_OAUTH2_GITHUB_ALLOW_SIGNUPS": "true",
143143
},
144-
Valid: func(config codersdk.DeploymentConfig) {
145-
require.Equal(t, config.OAuth2GithubClientID.Value, "client")
146-
require.Equal(t, config.OAuth2GithubClientSecret.Value, "secret")
147-
require.Equal(t, []string{"coder"}, config.OAuth2GithubAllowedOrgs.Value)
148-
require.Equal(t, []string{"coder"}, config.OAuth2GithubAllowedTeams.Value)
149-
require.Equal(t, config.OAuth2GithubAllowSignups.Value, true)
144+
Valid: func(config *codersdk.DeploymentConfig) {
145+
require.Equal(t, config.OAuth2.Github.ClientID.Value, "client")
146+
require.Equal(t, config.OAuth2.Github.ClientSecret.Value, "secret")
147+
require.Equal(t, []string{"coder"}, config.OAuth2.Github.AllowedOrgs.Value)
148+
require.Equal(t, []string{"coder"}, config.OAuth2.Github.AllowedTeams.Value)
149+
require.Equal(t, config.OAuth2.Github.AllowSignups.Value, true)
150150
},
151151
}} {
152152
tc := tc

‎cli/server.go

+48-48
Original file line numberDiff line numberDiff line change
@@ -117,11 +117,11 @@ func Server(vip *viper.Viper, newAPI func(context.Context, *coderd.Options) (*co
117117

118118
// Coder tracing should be disabled if telemetry is disabled unless
119119
// --telemetry-trace was explicitly provided.
120-
shouldCoderTrace := cfg.TelemetryEnable.Value && !isTest()
120+
shouldCoderTrace := cfg.Telemetry.Enable.Value && !isTest()
121121
// Only override if telemetryTraceEnable was specifically set.
122122
// By default we want it to be controlled by telemetryEnable.
123123
if cmd.Flags().Changed("telemetry-trace") {
124-
shouldCoderTrace = cfg.TelemetryTrace.Value
124+
shouldCoderTrace = cfg.Telemetry.Trace.Value
125125
}
126126

127127
if cfg.TraceEnable.Value || shouldCoderTrace {
@@ -174,13 +174,13 @@ func Server(vip *viper.Viper, newAPI func(context.Context, *coderd.Options) (*co
174174
defer listener.Close()
175175

176176
var tlsConfig *tls.Config
177-
if cfg.TLSEnable.Value {
177+
if cfg.TLS.Enable.Value {
178178
tlsConfig, err = configureTLS(
179-
cfg.TLSMinVersion.Value,
180-
cfg.TLSClientAuth.Value,
181-
cfg.TLSCertFiles.Value,
182-
cfg.TLSKeyFiles.Value,
183-
cfg.TLSClientCAFile.Value,
179+
cfg.TLS.MinVersion.Value,
180+
cfg.TLS.ClientAuth.Value,
181+
cfg.TLS.CertFiles.Value,
182+
cfg.TLS.KeyFiles.Value,
183+
cfg.TLS.ClientCAFile.Value,
184184
)
185185
if err != nil {
186186
return xerrors.Errorf("configure tls: %w", err)
@@ -202,7 +202,7 @@ func Server(vip *viper.Viper, newAPI func(context.Context, *coderd.Options) (*co
202202
Scheme: "http",
203203
Host: tcpAddr.String(),
204204
}
205-
if cfg.TLSEnable.Value {
205+
if cfg.TLS.Enable.Value {
206206
localURL.Scheme = "https"
207207
}
208208

@@ -297,22 +297,22 @@ func Server(vip *viper.Viper, newAPI func(context.Context, *coderd.Options) (*co
297297

298298
defaultRegion := &tailcfg.DERPRegion{
299299
EmbeddedRelay: true,
300-
RegionID: cfg.DERPServerRegionID.Value,
301-
RegionCode: cfg.DERPServerRegionCode.Value,
302-
RegionName: cfg.DERPServerRegionName.Value,
300+
RegionID: cfg.DERP.Server.RegionID.Value,
301+
RegionCode: cfg.DERP.Server.RegionCode.Value,
302+
RegionName: cfg.DERP.Server.RegionName.Value,
303303
Nodes: []*tailcfg.DERPNode{{
304-
Name: fmt.Sprintf("%db", cfg.DERPServerRegionID.Value),
305-
RegionID: cfg.DERPServerRegionID.Value,
304+
Name: fmt.Sprintf("%db", cfg.DERP.Server.RegionID.Value),
305+
RegionID: cfg.DERP.Server.RegionID.Value,
306306
HostName: accessURLParsed.Hostname(),
307307
DERPPort: accessURLPort,
308308
STUNPort: -1,
309309
ForceHTTP: accessURLParsed.Scheme == "http",
310310
}},
311311
}
312-
if !cfg.DERPServerEnable.Value {
312+
if !cfg.DERP.Server.Enable.Value {
313313
defaultRegion = nil
314314
}
315-
derpMap, err := tailnet.NewDERPMap(ctx, defaultRegion, cfg.DERPServerSTUNAddresses.Value, cfg.DERPConfigURL.Value, cfg.DERPConfigPath.Value)
315+
derpMap, err := tailnet.NewDERPMap(ctx, defaultRegion, cfg.DERP.Server.STUNAddresses.Value, cfg.DERP.Config.URL.Value, cfg.DERP.Config.Path.Value)
316316
if err != nil {
317317
return xerrors.Errorf("create derp map: %w", err)
318318
}
@@ -350,35 +350,35 @@ func Server(vip *viper.Viper, newAPI func(context.Context, *coderd.Options) (*co
350350
MetricsCacheRefreshInterval: cfg.MetricsCacheRefreshInterval.Value,
351351
AgentStatsRefreshInterval: cfg.AgentStatRefreshInterval.Value,
352352
Experimental: ExperimentalEnabled(cmd),
353-
DeploymentConfig: &cfg,
353+
DeploymentConfig: cfg,
354354
}
355355
if tlsConfig != nil {
356356
options.TLSCertificates = tlsConfig.Certificates
357357
}
358358

359-
if cfg.OAuth2GithubClientSecret.Value != "" {
359+
if cfg.OAuth2.Github.ClientSecret.Value != "" {
360360
options.GithubOAuth2Config, err = configureGithubOAuth2(accessURLParsed,
361-
cfg.OAuth2GithubClientID.Value,
362-
cfg.OAuth2GithubClientSecret.Value,
363-
cfg.OAuth2GithubAllowSignups.Value,
364-
cfg.OAuth2GithubAllowedOrgs.Value,
365-
cfg.OAuth2GithubAllowedTeams.Value,
366-
cfg.OAuth2GithubEnterpriseBaseURL.Value,
361+
cfg.OAuth2.Github.ClientID.Value,
362+
cfg.OAuth2.Github.ClientSecret.Value,
363+
cfg.OAuth2.Github.AllowSignups.Value,
364+
cfg.OAuth2.Github.AllowedOrgs.Value,
365+
cfg.OAuth2.Github.AllowedTeams.Value,
366+
cfg.OAuth2.Github.EnterpriseBaseURL.Value,
367367
)
368368
if err != nil {
369369
return xerrors.Errorf("configure github oauth2: %w", err)
370370
}
371371
}
372372

373-
if cfg.OIDCClientSecret.Value != "" {
374-
if cfg.OIDCClientID.Value == "" {
373+
if cfg.OIDC.ClientSecret.Value != "" {
374+
if cfg.OIDC.ClientID.Value == "" {
375375
return xerrors.Errorf("OIDC client ID be set!")
376376
}
377-
if cfg.OIDCIssuerURL.Value == "" {
377+
if cfg.OIDC.IssuerURL.Value == "" {
378378
return xerrors.Errorf("OIDC issuer URL must be set!")
379379
}
380380

381-
oidcProvider, err := oidc.NewProvider(ctx, cfg.OIDCIssuerURL.Value)
381+
oidcProvider, err := oidc.NewProvider(ctx, cfg.OIDC.IssuerURL.Value)
382382
if err != nil {
383383
return xerrors.Errorf("configure oidc provider: %w", err)
384384
}
@@ -388,17 +388,17 @@ func Server(vip *viper.Viper, newAPI func(context.Context, *coderd.Options) (*co
388388
}
389389
options.OIDCConfig = &coderd.OIDCConfig{
390390
OAuth2Config: &oauth2.Config{
391-
ClientID: cfg.OIDCClientID.Value,
392-
ClientSecret: cfg.OIDCClientSecret.Value,
391+
ClientID: cfg.OIDC.ClientID.Value,
392+
ClientSecret: cfg.OIDC.ClientSecret.Value,
393393
RedirectURL: redirectURL.String(),
394394
Endpoint: oidcProvider.Endpoint(),
395-
Scopes: cfg.OIDCScopes.Value,
395+
Scopes: cfg.OIDC.Scopes.Value,
396396
},
397397
Verifier: oidcProvider.Verifier(&oidc.Config{
398-
ClientID: cfg.OIDCClientID.Value,
398+
ClientID: cfg.OIDC.ClientID.Value,
399399
}),
400-
EmailDomain: cfg.OIDCEmailDomain.Value,
401-
AllowSignups: cfg.OIDCAllowSignups.Value,
400+
EmailDomain: cfg.OIDC.EmailDomain.Value,
401+
AllowSignups: cfg.OIDC.AllowSignups.Value,
402402
}
403403
}
404404

@@ -461,26 +461,26 @@ func Server(vip *viper.Viper, newAPI func(context.Context, *coderd.Options) (*co
461461
}
462462

463463
// Parse the raw telemetry URL!
464-
telemetryURL, err := parseURL(ctx, cfg.TelemetryURL.Value)
464+
telemetryURL, err := parseURL(ctx, cfg.Telemetry.URL.Value)
465465
if err != nil {
466466
return xerrors.Errorf("parse telemetry url: %w", err)
467467
}
468468
// Disable telemetry if the in-memory database is used unless explicitly defined!
469-
if cfg.InMemoryDatabase.Value && !cmd.Flags().Changed(cfg.TelemetryEnable.Flag) {
470-
cfg.TelemetryEnable.Value = false
469+
if cfg.InMemoryDatabase.Value && !cmd.Flags().Changed(cfg.Telemetry.Enable.Flag) {
470+
cfg.Telemetry.Enable.Value = false
471471
}
472-
if cfg.TelemetryEnable.Value {
472+
if cfg.Telemetry.Enable.Value {
473473
options.Telemetry, err = telemetry.New(telemetry.Options{
474474
BuiltinPostgres: builtinPostgres,
475475
DeploymentID: deploymentID,
476476
Database: options.Database,
477477
Logger: logger.Named("telemetry"),
478478
URL: telemetryURL,
479-
GitHubOAuth: cfg.OAuth2GithubClientID.Value != "",
480-
OIDCAuth: cfg.OIDCClientID.Value != "",
481-
OIDCIssuerURL: cfg.OIDCIssuerURL.Value,
482-
Prometheus: cfg.PrometheusEnable.Value,
483-
STUN: len(cfg.DERPServerSTUNAddresses.Value) != 0,
479+
GitHubOAuth: cfg.OAuth2.Github.ClientID.Value != "",
480+
OIDCAuth: cfg.OIDC.ClientID.Value != "",
481+
OIDCIssuerURL: cfg.OIDC.IssuerURL.Value,
482+
Prometheus: cfg.Prometheus.Enable.Value,
483+
STUN: len(cfg.DERP.Server.STUNAddresses.Value) != 0,
484484
Tunnel: tunnel != nil,
485485
})
486486
if err != nil {
@@ -491,11 +491,11 @@ func Server(vip *viper.Viper, newAPI func(context.Context, *coderd.Options) (*co
491491

492492
// This prevents the pprof import from being accidentally deleted.
493493
_ = pprof.Handler
494-
if cfg.PprofEnable.Value {
494+
if cfg.Pprof.Enable.Value {
495495
//nolint:revive
496-
defer serveHandler(ctx, logger, nil, cfg.PprofAddress.Value, "pprof")()
496+
defer serveHandler(ctx, logger, nil, cfg.Pprof.Address.Value, "pprof")()
497497
}
498-
if cfg.PrometheusEnable.Value {
498+
if cfg.Prometheus.Enable.Value {
499499
options.PrometheusRegistry = prometheus.NewRegistry()
500500
closeUsersFunc, err := prometheusmetrics.ActiveUsers(ctx, options.PrometheusRegistry, options.Database, 0)
501501
if err != nil {
@@ -512,7 +512,7 @@ func Server(vip *viper.Viper, newAPI func(context.Context, *coderd.Options) (*co
512512
//nolint:revive
513513
defer serveHandler(ctx, logger, promhttp.InstrumentMetricHandler(
514514
options.PrometheusRegistry, promhttp.HandlerFor(options.PrometheusRegistry, promhttp.HandlerOpts{}),
515-
), cfg.PrometheusAddress.Value, "prometheus")()
515+
), cfg.Prometheus.Address.Value, "prometheus")()
516516
}
517517

518518
// We use a separate coderAPICloser so the Enterprise API
@@ -524,7 +524,7 @@ func Server(vip *viper.Viper, newAPI func(context.Context, *coderd.Options) (*co
524524
}
525525

526526
client := codersdk.New(localURL)
527-
if cfg.TLSEnable.Value {
527+
if cfg.TLS.Enable.Value {
528528
// Secure transport isn't needed for locally communicating!
529529
client.HTTPClient.Transport = &http.Transport{
530530
TLSClientConfig: &tls.Config{

‎coderd/deploymentconfig_test.go

+5-5
Original file line numberDiff line numberDiff line change
@@ -26,22 +26,22 @@ func TestDeploymentConfig(t *testing.T) {
2626
// values should be returned
2727
cfg.AccessURL.Value = hi
2828
// values should not be returned
29-
cfg.OAuth2GithubClientSecret.Value = hi
30-
cfg.OIDCClientSecret.Value = hi
29+
cfg.OAuth2.Github.ClientSecret.Value = hi
30+
cfg.OIDC.ClientSecret.Value = hi
3131
cfg.PostgresURL.Value = hi
3232
cfg.SCIMAPIKey.Value = hi
3333

3434
client := coderdtest.New(t, &coderdtest.Options{
35-
DeploymentConfig: &cfg,
35+
DeploymentConfig: cfg,
3636
})
3737
_ = coderdtest.CreateFirstUser(t, client)
3838
scrubbed, err := client.DeploymentConfig(ctx)
3939
require.NoError(t, err)
4040
// ensure normal values pass through
4141
require.EqualValues(t, hi, scrubbed.AccessURL.Value)
4242
// ensure secrets are removed
43-
require.Empty(t, scrubbed.OAuth2GithubClientSecret.Value)
44-
require.Empty(t, scrubbed.OIDCClientSecret.Value)
43+
require.Empty(t, scrubbed.OAuth2.Github.ClientSecret.Value)
44+
require.Empty(t, scrubbed.OIDC.ClientSecret.Value)
4545
require.Empty(t, scrubbed.PostgresURL.Value)
4646
require.Empty(t, scrubbed.SCIMAPIKey.Value)
4747
}

0 commit comments

Comments
 (0)