Skip to content
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

Jump to bottom

crypto/tls: Categorize advanced configuration parameters #124

Merged
merged 2 commits into from
Jan 31, 2022
Merged

crypto/tls: Categorize advanced configuration parameters #124

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9f33e66
crypto/tls: Categorize advanced configuration parameters
aknuds1 Jan 31, 2022
8fb7519
Categorize tls_enabled as advanced
aknuds1 Jan 31, 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
10 changes: 5 additions & 5 deletions crypto/tls/tls.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,11 @@ import (

// ClientConfig is the config for client TLS.
type ClientConfig struct {
CertPath string `yaml:"tls_cert_path"`
KeyPath string `yaml:"tls_key_path"`
CAPath string `yaml:"tls_ca_path"`
ServerName string `yaml:"tls_server_name"`
InsecureSkipVerify bool `yaml:"tls_insecure_skip_verify"`
CertPath string `yaml:"tls_cert_path" category:"advanced"`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see some discrepancy. The EnableTLS is basic (eg. in grpcclient or etcd) but the settings for TLS are not. Either TLS is basic (and then all paths to cert/key/CA are basic as well) or the whole TLS setup is advanced, but I think having a mix is inconsistent.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pracucci I categorized all tls_enabled parameters as advanced in the meantime. Is there anything else that needs categorizing as well?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TLS config should have been fully addressed in this PR 👍

KeyPath string `yaml:"tls_key_path" category:"advanced"`
CAPath string `yaml:"tls_ca_path" category:"advanced"`
ServerName string `yaml:"tls_server_name" category:"advanced"`
InsecureSkipVerify bool `yaml:"tls_insecure_skip_verify" category:"advanced"`
}

var (
Expand Down
2 changes: 1 addition & 1 deletion grpcclient/grpcclient.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ type Config struct {
BackoffOnRatelimits bool `yaml:"backoff_on_ratelimits"`
BackoffConfig backoff.Config `yaml:"backoff_config"`

TLSEnabled bool `yaml:"tls_enabled"`
TLSEnabled bool `yaml:"tls_enabled" category:"advanced"`
TLS tls.ClientConfig `yaml:",inline"`
}

Expand Down
2 changes: 1 addition & 1 deletion kv/etcd/etcd.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ type Config struct {
Endpoints []string `yaml:"endpoints"`
DialTimeout time.Duration `yaml:"dial_timeout" category:"advanced"`
MaxRetries int `yaml:"max_retries" category:"advanced"`
EnableTLS bool `yaml:"tls_enabled"`
EnableTLS bool `yaml:"tls_enabled" category:"advanced"`
TLS dstls.ClientConfig `yaml:",inline"`

UserName string `yaml:"username"`
Expand Down
2 changes: 1 addition & 1 deletion kv/memberlist/tcp_transport.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ type TCPTransportConfig struct {
MetricsRegisterer prometheus.Registerer `yaml:"-"`
MetricsNamespace string `yaml:"-"`

TLSEnabled bool `yaml:"tls_enabled"`
TLSEnabled bool `yaml:"tls_enabled" category:"advanced"`
TLS dstls.ClientConfig `yaml:",inline"`
}

Expand Down