Skip to content

Commit

Permalink
server/embed: address golangci var-naming issues
Browse files Browse the repository at this point in the history 
Addresses issues in TLSMinVersion, TLSMaxVersion, WALDir, and
MaxWALFiles.

Signed-off-by: Ivan Valdes <[email protected]>
  • Loading branch information
@ivanvc
ivanvc committed Apr 2, 2024
1 parent a22ae62 commit 4bd3d28
Show file tree
Hide file tree
Showing 5 changed files with 85 additions and 31 deletions.
82 changes: 68 additions & 14 deletions server/embed/config.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -149,9 +149,13 @@ func init() {

// Config holds the arguments for configuring an etcd server.
type Config struct {
Name string `json:"name"`
Dir string `json:"data-dir"`
WalDir string `json:"wal-dir"`
Name string `json:"name"`
Dir string `json:"data-dir"`
// WalDir is the directory to save the write ahead log files.
// Deprecated: please use WALDir instead, to be decommissioned in 3.7
WalDir string
// WALDir is the directory to save the write ahead log files.
WALDir string `json:"wal-dir"`

SnapshotCount uint64 `json:"snapshot-count"`

Expand All @@ -163,7 +167,11 @@ type Config struct {
SnapshotCatchUpEntries uint64 `json:"experimental-snapshot-catch-up-entries"`

MaxSnapFiles uint `json:"max-snapshots"`
MaxWalFiles uint `json:"max-wals"`
// MaxWalFiles is the maximum number of write ahead log files stored in the wal directory.
// Deprecated: please use MaxWALFiles instead, to be decommissioned in 3.7
MaxWalFiles uint
// MaxWALFiles is the maximum number of write ahead log files stored in the wal directory.
MaxWALFiles uint `json:"max-wals"`

// TickMs is the number of milliseconds between heartbeat ticks.
// TODO: decouple tickMs and heartbeat tick (current heartbeat tick = 1).
Expand Down Expand Up @@ -230,10 +238,16 @@ type Config struct {
// Note that cipher suites are prioritized in the given order.
CipherSuites []string `json:"cipher-suites"`

// TLSMinVersion is the minimum accepted TLS version between client/server and peers.
TLSMinVersion string `json:"tls-min-version"`
// TlsMinVersion is the minimum accepted TLS version between client/server and peers.
TlsMinVersion string `json:"tls-min-version"`
// Deprecated: please use TLSMinVersion instead, to be decommissioned in 3.7
TlsMinVersion string
// TLSMaxVersion is the maximum accepted TLS version between client/server and peers.
TLSMaxVersion string `json:"tls-max-version"`
// TlsMaxVersion is the maximum accepted TLS version between client/server and peers.
TlsMaxVersion string `json:"tls-max-version"`
// Deprecated: please use TLSMaxVersion instead, to be decommissioned in 3.7
TlsMaxVersion string

ClusterState string `json:"initial-cluster-state"`
DNSCluster string `json:"discovery-srv"`
Expand Down Expand Up @@ -475,7 +489,7 @@ func NewConfig() *Config {
acurl, _ := url.Parse(DefaultAdvertiseClientURLs)
cfg := &Config{
MaxSnapFiles: DefaultMaxSnapshots,
MaxWalFiles: DefaultMaxWALs,
MaxWALFiles: DefaultMaxWALs,

Name: DefaultName,

Expand Down Expand Up @@ -558,10 +572,50 @@ func NewConfig() *Config {
return cfg
}

// getWALDir returns the `WALDir` if not empty, otherwise `WalDir`.
// TODO: remove this method once we remove the deprecated `WalDir`.
func (cfg *Config) getWALDir() string {
if len(cfg.WALDir) > 0 {
return cfg.WALDir
}

return cfg.WalDir
}

// getMaxWALFiles returns the `MaxWALFiles` if not empty, otherwise `MaxWalFiles`.
// TODO: remove this method once we remove the deprecated `MaxWalFiles`.
func (cfg *Config) getMaxWALFiles() uint {
if cfg.MaxWALFiles > 0 {
return cfg.MaxWALFiles
}

return cfg.MaxWalFiles
}

// getTLSMinVersion returns the `TLSMinVersion` if not empty, otherwise `TlsMinVersion`.
// TODO: remove this method once we remove the deprecated `TlsMinVersion`.
func (cfg *Config) getTLSMinVersion() string {
if len(cfg.TLSMinVersion) > 0 {
return cfg.TLSMinVersion
}

return cfg.TlsMinVersion
}

// getTLSMaxVersion returns the `TLSMaxVersion` if not empty, otherwise `TlsMaxVersion`.
// TODO: remove this method once we remove the deprecated `TlsMaxVersion`.
func (cfg *Config) getTLSMaxVersion() string {
if len(cfg.TLSMaxVersion) > 0 {
return cfg.TLSMaxVersion
}

return cfg.TlsMaxVersion
}

func (cfg *Config) AddFlags(fs *flag.FlagSet) {
// member
fs.StringVar(&cfg.Dir, "data-dir", cfg.Dir, "Path to the data directory.")
fs.StringVar(&cfg.WalDir, "wal-dir", cfg.WalDir, "Path to the dedicated wal directory.")
fs.StringVar(&cfg.WALDir, "wal-dir", cfg.WALDir, "Path to the dedicated wal directory.")
fs.Var(
flags.NewUniqueURLsWithExceptions(DefaultListenPeerURLs, ""),
"listen-peer-urls",
Expand All @@ -581,7 +635,7 @@ func (cfg *Config) AddFlags(fs *flag.FlagSet) {
"List of URLs to listen on for the metrics and health endpoints.",
)
fs.UintVar(&cfg.MaxSnapFiles, "max-snapshots", cfg.MaxSnapFiles, "Maximum number of snapshot files to retain (0 is unlimited).")
fs.UintVar(&cfg.MaxWalFiles, "max-wals", cfg.MaxWalFiles, "Maximum number of wal files to retain (0 is unlimited).")
fs.UintVar(&cfg.MaxWALFiles, "max-wals", cfg.MaxWALFiles, "Maximum number of wal files to retain (0 is unlimited).")
fs.StringVar(&cfg.Name, "name", cfg.Name, "Human-readable name for this member.")
fs.Uint64Var(&cfg.SnapshotCount, "snapshot-count", cfg.SnapshotCount, "Number of committed transactions to trigger a snapshot to disk.")
fs.UintVar(&cfg.TickMs, "heartbeat-interval", cfg.TickMs, "Time (in milliseconds) of a heartbeat interval.")
Expand Down Expand Up @@ -669,8 +723,8 @@ func (cfg *Config) AddFlags(fs *flag.FlagSet) {
fs.StringVar(&cfg.PeerTLSInfo.AllowedHostname, "peer-cert-allowed-hostname", "", "Allowed TLS hostname for inter peer authentication.")
fs.Var(flags.NewStringsValue(""), "cipher-suites", "Comma-separated list of supported TLS cipher suites between client/server and peers (empty will be auto-populated by Go).")
fs.BoolVar(&cfg.PeerTLSInfo.SkipClientSANVerify, "experimental-peer-skip-client-san-verification", false, "Skip verification of SAN field in client certificate for peer connections.")
fs.StringVar(&cfg.TlsMinVersion, "tls-min-version", string(tlsutil.TLSVersion12), "Minimum TLS version supported by etcd. Possible values: TLS1.2, TLS1.3.")
fs.StringVar(&cfg.TlsMaxVersion, "tls-max-version", string(tlsutil.TLSVersionDefault), "Maximum TLS version supported by etcd. Possible values: TLS1.2, TLS1.3 (empty defers to Go).")
fs.StringVar(&cfg.TLSMinVersion, "tls-min-version", string(tlsutil.TLSVersion12), "Minimum TLS version supported by etcd. Possible values: TLS1.2, TLS1.3.")
fs.StringVar(&cfg.TLSMaxVersion, "tls-max-version", string(tlsutil.TLSVersionDefault), "Maximum TLS version supported by etcd. Possible values: TLS1.2, TLS1.3 (empty defers to Go).")

fs.Var(
flags.NewUniqueURLsWithExceptions("*", "*"),
Expand Down Expand Up @@ -998,18 +1052,18 @@ func (cfg *Config) Validate() error {
zap.String("name", cfg.Name))
}

minVersion, err := tlsutil.GetTLSVersion(cfg.TlsMinVersion)
minVersion, err := tlsutil.GetTLSVersion(cfg.getTLSMinVersion())
if err != nil {
return err
}
maxVersion, err := tlsutil.GetTLSVersion(cfg.TlsMaxVersion)
maxVersion, err := tlsutil.GetTLSVersion(cfg.getTLSMaxVersion())
if err != nil {
return err
}

// maxVersion == 0 means that Go selects the highest available version.
if maxVersion != 0 && minVersion > maxVersion {
return fmt.Errorf("min version (%s) is greater than max version (%s)", cfg.TlsMinVersion, cfg.TlsMaxVersion)
return fmt.Errorf("min version (%s) is greater than max version (%s)", cfg.getTLSMinVersion(), cfg.getTLSMaxVersion())
}

// Check if user attempted to configure ciphers for TLS1.3 only: Go does not support that currently.
Expand Down
8 changes: 4 additions & 4 deletions server/embed/config_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -492,8 +492,8 @@ func TestTLSVersionMinMax(t *testing.T) {
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
cfg := NewConfig()
cfg.TlsMinVersion = tt.givenTLSMinVersion
cfg.TlsMaxVersion = tt.givenTLSMaxVersion
cfg.TLSMinVersion = tt.givenTLSMinVersion
cfg.TLSMaxVersion = tt.givenTLSMaxVersion
cfg.CipherSuites = tt.givenCipherSuites

err := cfg.Validate()
Expand All @@ -502,8 +502,8 @@ func TestTLSVersionMinMax(t *testing.T) {
return
}

updateMinMaxVersions(&cfg.PeerTLSInfo, cfg.TlsMinVersion, cfg.TlsMaxVersion)
updateMinMaxVersions(&cfg.ClientTLSInfo, cfg.TlsMinVersion, cfg.TlsMaxVersion)
updateMinMaxVersions(&cfg.PeerTLSInfo, cfg.TLSMinVersion, cfg.TLSMaxVersion)
updateMinMaxVersions(&cfg.ClientTLSInfo, cfg.TLSMinVersion, cfg.TLSMaxVersion)

assert.Equal(t, tt.expectedMinTLSVersion, cfg.PeerTLSInfo.MinVersion)
assert.Equal(t, tt.expectedMaxTLSVersion, cfg.PeerTLSInfo.MaxVersion)
Expand Down
10 changes: 5 additions & 5 deletions server/embed/etcd.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -171,11 +171,11 @@ func StartEtcd(inCfg *Config) (e *Etcd, err error) {
ClientURLs: cfg.AdvertiseClientUrls,
PeerURLs: cfg.AdvertisePeerUrls,
DataDir: cfg.Dir,
DedicatedWALDir: cfg.WalDir,
DedicatedWALDir: cfg.getWALDir(),
SnapshotCount: cfg.SnapshotCount,
SnapshotCatchUpEntries: cfg.SnapshotCatchUpEntries,
MaxSnapFiles: cfg.MaxSnapFiles,
MaxWALFiles: cfg.MaxWalFiles,
MaxWALFiles: cfg.getMaxWALFiles(),
InitialPeerURLsMap: urlsmap,
InitialClusterToken: token,
DiscoveryURL: cfg.Durl,
Expand Down Expand Up @@ -320,7 +320,7 @@ func print(lg *zap.Logger, ec Config, sc config.ServerConfig, memberInitialized
zap.Bool("member-initialized", memberInitialized),
zap.String("name", sc.Name),
zap.String("data-dir", sc.DataDir),
zap.String("wal-dir", ec.WalDir),
zap.String("wal-dir", ec.getWALDir()),
zap.String("wal-dir-dedicated", sc.DedicatedWALDir),
zap.String("member-dir", sc.MemberDir()),
zap.Bool("force-new-cluster", sc.ForceNewCluster),
Expand Down Expand Up @@ -505,7 +505,7 @@ func configurePeerListeners(cfg *Config) (peers []*peerListener, err error) {
if err = cfg.PeerSelfCert(); err != nil {
cfg.logger.Fatal("failed to get peer self-signed certs", zap.Error(err))
}
updateMinMaxVersions(&cfg.PeerTLSInfo, cfg.TlsMinVersion, cfg.TlsMaxVersion)
updateMinMaxVersions(&cfg.PeerTLSInfo, cfg.getTLSMinVersion(), cfg.getTLSMinVersion())
if !cfg.PeerTLSInfo.Empty() {
cfg.logger.Info(
"starting with peer TLS",
Expand Down Expand Up @@ -618,7 +618,7 @@ func configureClientListeners(cfg *Config) (sctxs map[string]*serveCtx, err erro
if err = cfg.ClientSelfCert(); err != nil {
cfg.logger.Fatal("failed to get client self-signed certs", zap.Error(err))
}
updateMinMaxVersions(&cfg.ClientTLSInfo, cfg.TlsMinVersion, cfg.TlsMaxVersion)
updateMinMaxVersions(&cfg.ClientTLSInfo, cfg.getTLSMinVersion(), cfg.getTLSMaxVersion())
if cfg.EnablePprof {
cfg.logger.Info("pprof is enabled", zap.String("path", debugutil.HTTPPrefixPProf))
}
Expand Down
8 changes: 4 additions & 4 deletions server/embed/util.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,9 @@ import (
)

func isMemberInitialized(cfg *Config) bool {
waldir := cfg.WalDir
if waldir == "" {
waldir = filepath.Join(cfg.Dir, "member", "wal")
walDir := cfg.getWALDir()
if walDir == "" {
walDir = filepath.Join(cfg.Dir, "member", "wal")
}
return wal.Exist(waldir)
return wal.Exist(walDir)
}
8 changes: 4 additions & 4 deletions server/etcdmain/config_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ func TestConfigFileMemberFields(t *testing.T) {
yc := struct {
Dir string `json:"data-dir"`
MaxSnapFiles uint `json:"max-snapshots"`
MaxWalFiles uint `json:"max-wals"`
MaxWALFiles uint `json:"max-wals"`
Name string `json:"name"`
SnapshotCount uint64 `json:"snapshot-count"`
SnapshotCatchUpEntries uint64 `json:"experimental-snapshot-catch-up-entries"`
Expand Down Expand Up @@ -420,7 +420,7 @@ func validateMemberFlags(t *testing.T, cfg *config) {
ListenClientUrls: []url.URL{{Scheme: "http", Host: "localhost:7000"}, {Scheme: "https", Host: "localhost:7001"}},
ListenClientHttpUrls: []url.URL{{Scheme: "http", Host: "localhost:7002"}, {Scheme: "https", Host: "localhost:7003"}},
MaxSnapFiles: 10,
MaxWalFiles: 10,
MaxWALFiles: 10,
Name: "testname",
SnapshotCount: 10,
SnapshotCatchUpEntries: 1000,
Expand All @@ -432,8 +432,8 @@ func validateMemberFlags(t *testing.T, cfg *config) {
if cfg.ec.MaxSnapFiles != wcfg.MaxSnapFiles {
t.Errorf("maxsnap = %v, want %v", cfg.ec.MaxSnapFiles, wcfg.MaxSnapFiles)
}
if cfg.ec.MaxWalFiles != wcfg.MaxWalFiles {
t.Errorf("maxwal = %v, want %v", cfg.ec.MaxWalFiles, wcfg.MaxWalFiles)
if cfg.ec.MaxWALFiles != wcfg.MaxWALFiles {
t.Errorf("maxwal = %v, want %v", cfg.ec.MaxWALFiles, wcfg.MaxWALFiles)
}
if cfg.ec.Name != wcfg.Name {
t.Errorf("name = %v, want %v", cfg.ec.Name, wcfg.Name)
Expand Down

0 comments on commit 4bd3d28

Please sign in to comment.