Skip to content

feat: support nested structs, structured arrays, and better secret value handling in config #4727

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 16 commits into from
Oct 25, 2022
Merged

feat: support nested structs, structured arrays, and better secret value handling in config #4727

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
bb494dc
feat: support nested structs and arrays in config
f0ssel Oct 24, 2022
d84d377
update to main
f0ssel Oct 24, 2022
b1876bb
fix tests
f0ssel Oct 24, 2022
426e638
fix server
f0ssel Oct 24, 2022
faadaa0
fix ent
f0ssel Oct 24, 2022
4bcdd0a
get reflect working with ptr
f0ssel Oct 24, 2022
e6522cb
support slices
f0ssel Oct 24, 2022
429f91b
make tests pass
f0ssel Oct 24, 2022
552e438
move to default
f0ssel Oct 24, 2022
24bdf1b
safely return secrets
f0ssel Oct 24, 2022
d71ef7b
lol
f0ssel Oct 24, 2022
cead44f
fix tests
f0ssel Oct 24, 2022
c280692
lint
f0ssel Oct 24, 2022
a43640d
make gen
f0ssel Oct 24, 2022
1a37ac4
fis js
f0ssel Oct 24, 2022
b5bbd5f
make fmt
f0ssel Oct 24, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
745 changes: 401 additions & 344 deletions cli/deployment/config.go
View file
Open in desktop

Large diffs are not rendered by default.

84 changes: 42 additions & 42 deletions cli/deployment/config_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ func TestConfig(t *testing.T) {
for _, tc := range []struct {
Name string
Env map[string]string
Valid func(config codersdk.DeploymentConfig)
Valid func(config *codersdk.DeploymentConfig)
}{{
Name: "Deployment",
Env: map[string]string{
Expand All @@ -39,19 +39,19 @@ func TestConfig(t *testing.T) {
"CODER_TELEMETRY_TRACE": "false",
"CODER_WILDCARD_ACCESS_URL": "something-wildcard.com",
},
Valid: func(config codersdk.DeploymentConfig) {
Valid: func(config *codersdk.DeploymentConfig) {
require.Equal(t, config.Address.Value, "0.0.0.0:8443")
require.Equal(t, config.AccessURL.Value, "https://dev.coder.com")
require.Equal(t, config.PostgresURL.Value, "some-url")
require.Equal(t, config.PprofAddress.Value, "something")
require.Equal(t, config.PprofEnable.Value, true)
require.Equal(t, config.PrometheusAddress.Value, "hello-world")
require.Equal(t, config.PrometheusEnable.Value, true)
require.Equal(t, config.Pprof.Address.Value, "something")
require.Equal(t, config.Pprof.Enable.Value, true)
require.Equal(t, config.Prometheus.Address.Value, "hello-world")
require.Equal(t, config.Prometheus.Enable.Value, true)
require.Equal(t, config.ProvisionerDaemons.Value, 5)
require.Equal(t, config.SecureAuthCookie.Value, true)
require.Equal(t, config.SSHKeygenAlgorithm.Value, "potato")
require.Equal(t, config.TelemetryEnable.Value, false)
require.Equal(t, config.TelemetryTrace.Value, false)
require.Equal(t, config.Telemetry.Enable.Value, false)
require.Equal(t, config.Telemetry.Trace.Value, false)
require.Equal(t, config.WildcardAccessURL.Value, "something-wildcard.com")
},
}, {
Expand All @@ -66,15 +66,15 @@ func TestConfig(t *testing.T) {
"CODER_DERP_SERVER_RELAY_URL": "1.1.1.1",
"CODER_DERP_SERVER_STUN_ADDRESSES": "google.org",
},
Valid: func(config codersdk.DeploymentConfig) {
require.Equal(t, config.DERPConfigPath.Value, "/example/path")
require.Equal(t, config.DERPConfigURL.Value, "https://google.com")
require.Equal(t, config.DERPServerEnable.Value, false)
require.Equal(t, config.DERPServerRegionCode.Value, "something")
require.Equal(t, config.DERPServerRegionID.Value, 123)
require.Equal(t, config.DERPServerRegionName.Value, "Code-Land")
require.Equal(t, config.DERPServerRelayURL.Value, "1.1.1.1")
require.Equal(t, config.DERPServerSTUNAddresses.Value, []string{"google.org"})
Valid: func(config *codersdk.DeploymentConfig) {
require.Equal(t, config.DERP.Config.Path.Value, "/example/path")
require.Equal(t, config.DERP.Config.URL.Value, "https://google.com")
require.Equal(t, config.DERP.Server.Enable.Value, false)
require.Equal(t, config.DERP.Server.RegionCode.Value, "something")
require.Equal(t, config.DERP.Server.RegionID.Value, 123)
require.Equal(t, config.DERP.Server.RegionName.Value, "Code-Land")
require.Equal(t, config.DERP.Server.RelayURL.Value, "1.1.1.1")
require.Equal(t, config.DERP.Server.STUNAddresses.Value, []string{"google.org"})
},
}, {
Name: "Enterprise",
Expand All @@ -84,7 +84,7 @@ func TestConfig(t *testing.T) {
"CODER_SCIM_API_KEY": "some-key",
"CODER_USER_WORKSPACE_QUOTA": "10",
},
Valid: func(config codersdk.DeploymentConfig) {
Valid: func(config *codersdk.DeploymentConfig) {
require.Equal(t, config.AuditLogging.Value, false)
require.Equal(t, config.BrowserOnly.Value, true)
require.Equal(t, config.SCIMAPIKey.Value, "some-key")
Expand All @@ -100,19 +100,19 @@ func TestConfig(t *testing.T) {
"CODER_TLS_ENABLE": "true",
"CODER_TLS_MIN_VERSION": "tls10",
},
Valid: func(config codersdk.DeploymentConfig) {
require.Len(t, config.TLSCertFiles.Value, 2)
require.Equal(t, config.TLSCertFiles.Value[0], "/etc/acme-sh/dev.coder.com")
require.Equal(t, config.TLSCertFiles.Value[1], "/etc/acme-sh/*.dev.coder.com")
Valid: func(config *codersdk.DeploymentConfig) {
require.Len(t, config.TLS.CertFiles.Value, 2)
require.Equal(t, config.TLS.CertFiles.Value[0], "/etc/acme-sh/dev.coder.com")
require.Equal(t, config.TLS.CertFiles.Value[1], "/etc/acme-sh/*.dev.coder.com")

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

require.Equal(t, config.TLSClientAuth.Value, "/some/path")
require.Equal(t, config.TLSClientCAFile.Value, "/some/path")
require.Equal(t, config.TLSEnable.Value, true)
require.Equal(t, config.TLSMinVersion.Value, "tls10")
require.Equal(t, config.TLS.ClientAuth.Value, "/some/path")
require.Equal(t, config.TLS.ClientCAFile.Value, "/some/path")
require.Equal(t, config.TLS.Enable.Value, true)
require.Equal(t, config.TLS.MinVersion.Value, "tls10")
},
}, {
Name: "OIDC",
Expand All @@ -124,13 +124,13 @@ func TestConfig(t *testing.T) {
"CODER_OIDC_ALLOW_SIGNUPS": "false",
"CODER_OIDC_SCOPES": "something,here",
},
Valid: func(config codersdk.DeploymentConfig) {
require.Equal(t, config.OIDCIssuerURL.Value, "https://accounts.google.com")
require.Equal(t, config.OIDCEmailDomain.Value, "coder.com")
require.Equal(t, config.OIDCClientID.Value, "client")
require.Equal(t, config.OIDCClientSecret.Value, "secret")
require.Equal(t, config.OIDCAllowSignups.Value, false)
require.Equal(t, config.OIDCScopes.Value, []string{"something", "here"})
Valid: func(config *codersdk.DeploymentConfig) {
require.Equal(t, config.OIDC.IssuerURL.Value, "https://accounts.google.com")
require.Equal(t, config.OIDC.EmailDomain.Value, "coder.com")
require.Equal(t, config.OIDC.ClientID.Value, "client")
require.Equal(t, config.OIDC.ClientSecret.Value, "secret")
require.Equal(t, config.OIDC.AllowSignups.Value, false)
require.Equal(t, config.OIDC.Scopes.Value, []string{"something", "here"})
},
}, {
Name: "GitHub",
Expand All @@ -141,12 +141,12 @@ func TestConfig(t *testing.T) {
"CODER_OAUTH2_GITHUB_ALLOWED_TEAMS": "coder",
"CODER_OAUTH2_GITHUB_ALLOW_SIGNUPS": "true",
},
Valid: func(config codersdk.DeploymentConfig) {
require.Equal(t, config.OAuth2GithubClientID.Value, "client")
require.Equal(t, config.OAuth2GithubClientSecret.Value, "secret")
require.Equal(t, []string{"coder"}, config.OAuth2GithubAllowedOrgs.Value)
require.Equal(t, []string{"coder"}, config.OAuth2GithubAllowedTeams.Value)
require.Equal(t, config.OAuth2GithubAllowSignups.Value, true)
Valid: func(config *codersdk.DeploymentConfig) {
require.Equal(t, config.OAuth2.Github.ClientID.Value, "client")
require.Equal(t, config.OAuth2.Github.ClientSecret.Value, "secret")
require.Equal(t, []string{"coder"}, config.OAuth2.Github.AllowedOrgs.Value)
require.Equal(t, []string{"coder"}, config.OAuth2.Github.AllowedTeams.Value)
require.Equal(t, config.OAuth2.Github.AllowSignups.Value, true)
},
}} {
tc := tc
Expand Down
96 changes: 48 additions & 48 deletions cli/server.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,11 +117,11 @@ func Server(vip *viper.Viper, newAPI func(context.Context, *coderd.Options) (*co

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

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

var tlsConfig *tls.Config
if cfg.TLSEnable.Value {
if cfg.TLS.Enable.Value {
tlsConfig, err = configureTLS(
cfg.TLSMinVersion.Value,
cfg.TLSClientAuth.Value,
cfg.TLSCertFiles.Value,
cfg.TLSKeyFiles.Value,
cfg.TLSClientCAFile.Value,
cfg.TLS.MinVersion.Value,
cfg.TLS.ClientAuth.Value,
cfg.TLS.CertFiles.Value,
cfg.TLS.KeyFiles.Value,
cfg.TLS.ClientCAFile.Value,
)
if err != nil {
return xerrors.Errorf("configure tls: %w", err)
Expand All @@ -202,7 +202,7 @@ func Server(vip *viper.Viper, newAPI func(context.Context, *coderd.Options) (*co
Scheme: "http",
Host: tcpAddr.String(),
}
if cfg.TLSEnable.Value {
if cfg.TLS.Enable.Value {
localURL.Scheme = "https"
}

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

defaultRegion := &tailcfg.DERPRegion{
EmbeddedRelay: true,
RegionID: cfg.DERPServerRegionID.Value,
RegionCode: cfg.DERPServerRegionCode.Value,
RegionName: cfg.DERPServerRegionName.Value,
RegionID: cfg.DERP.Server.RegionID.Value,
RegionCode: cfg.DERP.Server.RegionCode.Value,
RegionName: cfg.DERP.Server.RegionName.Value,
Nodes: []*tailcfg.DERPNode{{
Name: fmt.Sprintf("%db", cfg.DERPServerRegionID.Value),
RegionID: cfg.DERPServerRegionID.Value,
Name: fmt.Sprintf("%db", cfg.DERP.Server.RegionID.Value),
RegionID: cfg.DERP.Server.RegionID.Value,
HostName: accessURLParsed.Hostname(),
DERPPort: accessURLPort,
STUNPort: -1,
ForceHTTP: accessURLParsed.Scheme == "http",
}},
}
if !cfg.DERPServerEnable.Value {
if !cfg.DERP.Server.Enable.Value {
defaultRegion = nil
}
derpMap, err := tailnet.NewDERPMap(ctx, defaultRegion, cfg.DERPServerSTUNAddresses.Value, cfg.DERPConfigURL.Value, cfg.DERPConfigPath.Value)
derpMap, err := tailnet.NewDERPMap(ctx, defaultRegion, cfg.DERP.Server.STUNAddresses.Value, cfg.DERP.Config.URL.Value, cfg.DERP.Config.Path.Value)
if err != nil {
return xerrors.Errorf("create derp map: %w", err)
}
Expand Down Expand Up @@ -350,35 +350,35 @@ func Server(vip *viper.Viper, newAPI func(context.Context, *coderd.Options) (*co
MetricsCacheRefreshInterval: cfg.MetricsCacheRefreshInterval.Value,
AgentStatsRefreshInterval: cfg.AgentStatRefreshInterval.Value,
Experimental: ExperimentalEnabled(cmd),
DeploymentConfig: &cfg,
DeploymentConfig: cfg,
}
if tlsConfig != nil {
options.TLSCertificates = tlsConfig.Certificates
}

if cfg.OAuth2GithubClientSecret.Value != "" {
if cfg.OAuth2.Github.ClientSecret.Value != "" {
options.GithubOAuth2Config, err = configureGithubOAuth2(accessURLParsed,
cfg.OAuth2GithubClientID.Value,
cfg.OAuth2GithubClientSecret.Value,
cfg.OAuth2GithubAllowSignups.Value,
cfg.OAuth2GithubAllowedOrgs.Value,
cfg.OAuth2GithubAllowedTeams.Value,
cfg.OAuth2GithubEnterpriseBaseURL.Value,
cfg.OAuth2.Github.ClientID.Value,
cfg.OAuth2.Github.ClientSecret.Value,
cfg.OAuth2.Github.AllowSignups.Value,
cfg.OAuth2.Github.AllowedOrgs.Value,
cfg.OAuth2.Github.AllowedTeams.Value,
cfg.OAuth2.Github.EnterpriseBaseURL.Value,
)
if err != nil {
return xerrors.Errorf("configure github oauth2: %w", err)
}
}

if cfg.OIDCClientSecret.Value != "" {
if cfg.OIDCClientID.Value == "" {
if cfg.OIDC.ClientSecret.Value != "" {
if cfg.OIDC.ClientID.Value == "" {
return xerrors.Errorf("OIDC client ID be set!")
}
if cfg.OIDCIssuerURL.Value == "" {
if cfg.OIDC.IssuerURL.Value == "" {
return xerrors.Errorf("OIDC issuer URL must be set!")
}

oidcProvider, err := oidc.NewProvider(ctx, cfg.OIDCIssuerURL.Value)
oidcProvider, err := oidc.NewProvider(ctx, cfg.OIDC.IssuerURL.Value)
if err != nil {
return xerrors.Errorf("configure oidc provider: %w", err)
}
Expand All @@ -388,17 +388,17 @@ func Server(vip *viper.Viper, newAPI func(context.Context, *coderd.Options) (*co
}
options.OIDCConfig = &coderd.OIDCConfig{
OAuth2Config: &oauth2.Config{
ClientID: cfg.OIDCClientID.Value,
ClientSecret: cfg.OIDCClientSecret.Value,
ClientID: cfg.OIDC.ClientID.Value,
ClientSecret: cfg.OIDC.ClientSecret.Value,
RedirectURL: redirectURL.String(),
Endpoint: oidcProvider.Endpoint(),
Scopes: cfg.OIDCScopes.Value,
Scopes: cfg.OIDC.Scopes.Value,
},
Verifier: oidcProvider.Verifier(&oidc.Config{
ClientID: cfg.OIDCClientID.Value,
ClientID: cfg.OIDC.ClientID.Value,
}),
EmailDomain: cfg.OIDCEmailDomain.Value,
AllowSignups: cfg.OIDCAllowSignups.Value,
EmailDomain: cfg.OIDC.EmailDomain.Value,
AllowSignups: cfg.OIDC.AllowSignups.Value,
}
}

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

// Parse the raw telemetry URL!
telemetryURL, err := parseURL(ctx, cfg.TelemetryURL.Value)
telemetryURL, err := parseURL(ctx, cfg.Telemetry.URL.Value)
if err != nil {
return xerrors.Errorf("parse telemetry url: %w", err)
}
// Disable telemetry if the in-memory database is used unless explicitly defined!
if cfg.InMemoryDatabase.Value && !cmd.Flags().Changed(cfg.TelemetryEnable.Flag) {
cfg.TelemetryEnable.Value = false
if cfg.InMemoryDatabase.Value && !cmd.Flags().Changed(cfg.Telemetry.Enable.Flag) {
cfg.Telemetry.Enable.Value = false
}
if cfg.TelemetryEnable.Value {
if cfg.Telemetry.Enable.Value {
options.Telemetry, err = telemetry.New(telemetry.Options{
BuiltinPostgres: builtinPostgres,
DeploymentID: deploymentID,
Database: options.Database,
Logger: logger.Named("telemetry"),
URL: telemetryURL,
GitHubOAuth: cfg.OAuth2GithubClientID.Value != "",
OIDCAuth: cfg.OIDCClientID.Value != "",
OIDCIssuerURL: cfg.OIDCIssuerURL.Value,
Prometheus: cfg.PrometheusEnable.Value,
STUN: len(cfg.DERPServerSTUNAddresses.Value) != 0,
GitHubOAuth: cfg.OAuth2.Github.ClientID.Value != "",
OIDCAuth: cfg.OIDC.ClientID.Value != "",
OIDCIssuerURL: cfg.OIDC.IssuerURL.Value,
Prometheus: cfg.Prometheus.Enable.Value,
STUN: len(cfg.DERP.Server.STUNAddresses.Value) != 0,
Tunnel: tunnel != nil,
})
if err != nil {
Expand All @@ -491,11 +491,11 @@ func Server(vip *viper.Viper, newAPI func(context.Context, *coderd.Options) (*co

// This prevents the pprof import from being accidentally deleted.
_ = pprof.Handler
if cfg.PprofEnable.Value {
if cfg.Pprof.Enable.Value {
//nolint:revive
defer serveHandler(ctx, logger, nil, cfg.PprofAddress.Value, "pprof")()
defer serveHandler(ctx, logger, nil, cfg.Pprof.Address.Value, "pprof")()
}
if cfg.PrometheusEnable.Value {
if cfg.Prometheus.Enable.Value {
options.PrometheusRegistry = prometheus.NewRegistry()
closeUsersFunc, err := prometheusmetrics.ActiveUsers(ctx, options.PrometheusRegistry, options.Database, 0)
if err != nil {
Expand All @@ -512,7 +512,7 @@ func Server(vip *viper.Viper, newAPI func(context.Context, *coderd.Options) (*co
//nolint:revive
defer serveHandler(ctx, logger, promhttp.InstrumentMetricHandler(
options.PrometheusRegistry, promhttp.HandlerFor(options.PrometheusRegistry, promhttp.HandlerOpts{}),
), cfg.PrometheusAddress.Value, "prometheus")()
), cfg.Prometheus.Address.Value, "prometheus")()
}

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

client := codersdk.New(localURL)
if cfg.TLSEnable.Value {
if cfg.TLS.Enable.Value {
// Secure transport isn't needed for locally communicating!
client.HTTPClient.Transport = &http.Transport{
TLSClientConfig: &tls.Config{
Expand Down
10 changes: 5 additions & 5 deletions coderd/deploymentconfig_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,22 +26,22 @@ func TestDeploymentConfig(t *testing.T) {
// values should be returned
cfg.AccessURL.Value = hi
// values should not be returned
cfg.OAuth2GithubClientSecret.Value = hi
cfg.OIDCClientSecret.Value = hi
cfg.OAuth2.Github.ClientSecret.Value = hi
cfg.OIDC.ClientSecret.Value = hi
cfg.PostgresURL.Value = hi
cfg.SCIMAPIKey.Value = hi

client := coderdtest.New(t, &coderdtest.Options{
DeploymentConfig: &cfg,
DeploymentConfig: cfg,
})
_ = coderdtest.CreateFirstUser(t, client)
scrubbed, err := client.DeploymentConfig(ctx)
require.NoError(t, err)
// ensure normal values pass through
require.EqualValues(t, hi, scrubbed.AccessURL.Value)
// ensure secrets are removed
require.Empty(t, scrubbed.OAuth2GithubClientSecret.Value)
require.Empty(t, scrubbed.OIDCClientSecret.Value)
require.Empty(t, scrubbed.OAuth2.Github.ClientSecret.Value)
require.Empty(t, scrubbed.OIDC.ClientSecret.Value)
require.Empty(t, scrubbed.PostgresURL.Value)
require.Empty(t, scrubbed.SCIMAPIKey.Value)
}
Loading