Skip to content

Commit

Permalink
server/embed: address advertise URLs golangci var-naming issues
Browse files Browse the repository at this point in the history 
Deprecate AdvertisePeerUrls and AdvertiseClientUrls in favor of
AdvertisePeerURLs and AdvertiseClientURLs.

Signed-off-by: Ivan Valdes <[email protected]>
  • Loading branch information
@ivanvc
ivanvc committed Apr 5, 2024
1 parent bdbf8a2 commit e4dfc4f
Show file tree
Hide file tree
Showing 13 changed files with 134 additions and 99 deletions.
125 changes: 80 additions & 45 deletions server/embed/config.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -215,11 +215,26 @@ type Config struct {
MaxConcurrentStreams uint32 `json:"max-concurrent-streams"`

ListenPeerUrls, ListenClientUrls, ListenClientHttpUrls []url.URL
AdvertisePeerUrls, AdvertiseClientUrls []url.URL
ClientTLSInfo transport.TLSInfo
ClientAutoTLS bool
PeerTLSInfo transport.TLSInfo
PeerAutoTLS bool

// AdvertisePeerUrls specifies the host and port to advertise to the etcd peers.
// Deprecated: Please use AdvertisePeerURLs instead, to be decommissioned in
// 3.7.
//revive:disable-next-line:var-naming
AdvertisePeerUrls []url.URL
// AdvertisePeerURLs specifies the host and port to advertise to the etcd peers.
AdvertisePeerURLs []url.URL
// AdvertiseClientUrls specifies the host and port to advertise to the etcd clients.
// Deprecated: Please use AdvertiseClientURLs instead, to be decommissioned in
// 3.7.
//revive:disable-next-line:var-naming
AdvertiseClientUrls []url.URL
// AdvertiseClientURLs specifies the host and port to advertise to the etcd clients.
AdvertiseClientURLs []url.URL

ClientTLSInfo transport.TLSInfo
ClientAutoTLS bool
PeerTLSInfo transport.TLSInfo
PeerAutoTLS bool
// SelfSignedCertValidity specifies the validity period of the client and peer certificates
// that are automatically generated by etcd when you specify ClientAutoTLS and PeerAutoTLS,
// the unit is year, and the default is 1
Expand Down Expand Up @@ -447,8 +462,8 @@ type configJSON struct {
ListenPeerUrls string `json:"listen-peer-urls"`
ListenClientUrls string `json:"listen-client-urls"`
ListenClientHttpUrls string `json:"listen-client-http-urls"`
AdvertisePeerUrls string `json:"initial-advertise-peer-urls"`
AdvertiseClientUrls string `json:"advertise-client-urls"`
AdvertisePeerURLs string `json:"initial-advertise-peer-urls"`
AdvertiseClientURLs string `json:"advertise-client-urls"`

CORSJSON string `json:"cors"`
HostWhitelistJSON string `json:"host-whitelist"`
Expand Down Expand Up @@ -502,8 +517,8 @@ func NewConfig() *Config {

ListenPeerUrls: []url.URL{*lpurl},
ListenClientUrls: []url.URL{*lcurl},
AdvertisePeerUrls: []url.URL{*apurl},
AdvertiseClientUrls: []url.URL{*acurl},
AdvertisePeerURLs: []url.URL{*apurl},
AdvertiseClientURLs: []url.URL{*acurl},

ClusterState: ClusterStateFlagNew,
InitialClusterToken: "etcd-cluster",
Expand Down Expand Up @@ -558,6 +573,26 @@ func NewConfig() *Config {
return cfg
}

// getAdvertiseClientURLs returns `AdvertiseClientURLs` if set, otherwise
// `AdvertiseClientUrls`.
// TODO: remove once we remove the deprecated `AdvertiseClientUrls`.
func (cfg *Config) getAdvertiseClientURLs() []url.URL {
if len(cfg.AdvertiseClientURLs) > 0 {
return cfg.AdvertiseClientURLs
}
return cfg.AdvertiseClientUrls
}

// getAdvertisePeerURLs returns `AdvertisePeerURLs` if set, otherwise
// `AdvertisePeerUrls`.
// TODO: remove once we remove the deprecated `AdvertisePeerUrls`.
func (cfg *Config) getAdvertisePeerURLs() []url.URL {
if len(cfg.AdvertisePeerURLs) > 0 {
return cfg.AdvertisePeerURLs
}
return cfg.AdvertisePeerUrls
}

func (cfg *Config) AddFlags(fs *flag.FlagSet) {
// member
fs.StringVar(&cfg.Dir, "data-dir", cfg.Dir, "Path to the data directory.")
Expand Down Expand Up @@ -787,22 +822,22 @@ func (cfg *configYAML) configFromFile(path string) error {
cfg.Config.ListenClientHttpUrls = u
}

if cfg.configJSON.AdvertisePeerUrls != "" {
u, err := types.NewURLs(strings.Split(cfg.configJSON.AdvertisePeerUrls, ","))
if cfg.configJSON.AdvertisePeerURLs != "" {
u, err := types.NewURLs(strings.Split(cfg.configJSON.AdvertisePeerURLs, ","))
if err != nil {
fmt.Fprintf(os.Stderr, "unexpected error setting up initial-advertise-peer-urls: %v\n", err)
os.Exit(1)
}
cfg.Config.AdvertisePeerUrls = u
cfg.Config.AdvertisePeerURLs = u
}

if cfg.configJSON.AdvertiseClientUrls != "" {
u, err := types.NewURLs(strings.Split(cfg.configJSON.AdvertiseClientUrls, ","))
if cfg.configJSON.AdvertiseClientURLs != "" {
u, err := types.NewURLs(strings.Split(cfg.configJSON.AdvertiseClientURLs, ","))
if err != nil {
fmt.Fprintf(os.Stderr, "unexpected error setting up advertise-peer-urls: %v\n", err)
os.Exit(1)
}
cfg.Config.AdvertiseClientUrls = u
cfg.Config.AdvertiseClientURLs = u
}

if cfg.ListenMetricsUrlsJSON != "" {
Expand Down Expand Up @@ -895,12 +930,12 @@ func (cfg *Config) Validate() error {
if err := checkBindURLs(cfg.ListenMetricsUrls); err != nil {
return err
}
if err := checkHostURLs(cfg.AdvertisePeerUrls); err != nil {
addrs := cfg.getAdvertisePeerUrls()
if err := checkHostURLs(cfg.getAdvertisePeerURLs()); err != nil {
addrs := cfg.getAdvertisePeerURLsAsStrings()
return fmt.Errorf(`--initial-advertise-peer-urls %q must be "host:port" (%v)`, strings.Join(addrs, ","), err)
}
if err := checkHostURLs(cfg.AdvertiseClientUrls); err != nil {
addrs := cfg.getAdvertiseClientUrls()
if err := checkHostURLs(cfg.getAdvertiseClientURLs()); err != nil {
addrs := cfg.getAdvertiseClientURLsAsStrings()
return fmt.Errorf(`--advertise-client-urls %q must be "host:port" (%v)`, strings.Join(addrs, ","), err)
}
// Check if conflicting flags are passed.
Expand Down Expand Up @@ -955,7 +990,7 @@ func (cfg *Config) Validate() error {
}

// check this last since proxying in etcdmain may make this OK
if cfg.ListenClientUrls != nil && cfg.AdvertiseClientUrls == nil {
if cfg.ListenClientUrls != nil && cfg.AdvertiseClientURLs == nil {
return ErrUnsetAdvertiseClientURLsFlag
}

Expand Down Expand Up @@ -1028,14 +1063,14 @@ func (cfg *Config) PeerURLsMapAndToken(which string) (urlsmap types.URLsMap, tok
urlsmap = types.URLsMap{}
// If using v2 discovery, generate a temporary cluster based on
// self's advertised peer URLs
urlsmap[cfg.Name] = cfg.AdvertisePeerUrls
urlsmap[cfg.Name] = cfg.getAdvertisePeerURLs()
token = cfg.Durl

case len(cfg.DiscoveryCfg.Endpoints) > 0:
urlsmap = types.URLsMap{}
// If using v3 discovery, generate a temporary cluster based on
// self's advertised peer URLs
urlsmap[cfg.Name] = cfg.AdvertisePeerUrls
urlsmap[cfg.Name] = cfg.getAdvertisePeerURLs()
token = cfg.DiscoveryCfg.Token

case cfg.DNSCluster != "":
Expand Down Expand Up @@ -1089,7 +1124,7 @@ func (cfg *Config) GetDNSClusterNames() ([]string, error) {

// Use both etcd-server-ssl and etcd-server for discovery.
// Combine the results if both are available.
clusterStrs, cerr = getCluster("https", "etcd-server-ssl"+serviceNameSuffix, cfg.Name, cfg.DNSCluster, cfg.AdvertisePeerUrls)
clusterStrs, cerr = getCluster("https", "etcd-server-ssl"+serviceNameSuffix, cfg.Name, cfg.DNSCluster, cfg.getAdvertisePeerURLs())
if cerr != nil {
clusterStrs = make([]string, 0)
}
Expand All @@ -1099,12 +1134,12 @@ func (cfg *Config) GetDNSClusterNames() ([]string, error) {
zap.String("service-name", "etcd-server-ssl"+serviceNameSuffix),
zap.String("server-name", cfg.Name),
zap.String("discovery-srv", cfg.DNSCluster),
zap.Strings("advertise-peer-urls", cfg.getAdvertisePeerUrls()),
zap.Strings("advertise-peer-urls", cfg.getAdvertisePeerURLsAsStrings()),
zap.Strings("found-cluster", clusterStrs),
zap.Error(cerr),
)

defaultHTTPClusterStrs, httpCerr := getCluster("http", "etcd-server"+serviceNameSuffix, cfg.Name, cfg.DNSCluster, cfg.AdvertisePeerUrls)
defaultHTTPClusterStrs, httpCerr := getCluster("http", "etcd-server"+serviceNameSuffix, cfg.Name, cfg.DNSCluster, cfg.getAdvertisePeerURLs())
if httpCerr == nil {
clusterStrs = append(clusterStrs, defaultHTTPClusterStrs...)
}
Expand All @@ -1114,7 +1149,7 @@ func (cfg *Config) GetDNSClusterNames() ([]string, error) {
zap.String("service-name", "etcd-server"+serviceNameSuffix),
zap.String("server-name", cfg.Name),
zap.String("discovery-srv", cfg.DNSCluster),
zap.Strings("advertise-peer-urls", cfg.getAdvertisePeerUrls()),
zap.Strings("advertise-peer-urls", cfg.getAdvertisePeerURLsAsStrings()),
zap.Strings("found-cluster", clusterStrs),
zap.Error(httpCerr),
)
Expand All @@ -1123,15 +1158,15 @@ func (cfg *Config) GetDNSClusterNames() ([]string, error) {
}

func (cfg *Config) InitialClusterFromName(name string) (ret string) {
if len(cfg.AdvertisePeerUrls) == 0 {
if len(cfg.getAdvertisePeerURLs()) == 0 {
return ""
}
n := name
if name == "" {
n = DefaultName
}
for i := range cfg.AdvertisePeerUrls {
ret = ret + "," + n + "=" + cfg.AdvertisePeerUrls[i].String()
for _, u := range cfg.getAdvertisePeerURLs() {
ret = ret + "," + n + "=" + u.String()
}
return ret[1:]
}
Expand All @@ -1147,11 +1182,11 @@ func (cfg *Config) V2DeprecationEffective() config.V2DeprecationEnum {
}

func (cfg *Config) defaultPeerHost() bool {
return len(cfg.AdvertisePeerUrls) == 1 && cfg.AdvertisePeerUrls[0].String() == DefaultInitialAdvertisePeerURLs
return len(cfg.getAdvertisePeerURLs()) == 1 && cfg.getAdvertisePeerURLs()[0].String() == DefaultInitialAdvertisePeerURLs
}

func (cfg *Config) defaultClientHost() bool {
return len(cfg.AdvertiseClientUrls) == 1 && cfg.AdvertiseClientUrls[0].String() == DefaultAdvertiseClientURLs
return len(cfg.getAdvertiseClientURLs()) == 1 && cfg.getAdvertiseClientURLs()[0].String() == DefaultAdvertiseClientURLs
}

func (cfg *Config) ClientSelfCert() (err error) {
Expand Down Expand Up @@ -1215,7 +1250,7 @@ func (cfg *Config) UpdateDefaultClusterFromName(defaultInitialCluster string) (s
used := false
pip, pport := cfg.ListenPeerUrls[0].Hostname(), cfg.ListenPeerUrls[0].Port()
if cfg.defaultPeerHost() && pip == "0.0.0.0" {
cfg.AdvertisePeerUrls[0] = url.URL{Scheme: cfg.AdvertisePeerUrls[0].Scheme, Host: fmt.Sprintf("%s:%s", defaultHostname, pport)}
cfg.getAdvertisePeerURLs()[0] = url.URL{Scheme: cfg.getAdvertisePeerURLs()[0].Scheme, Host: fmt.Sprintf("%s:%s", defaultHostname, pport)}
used = true
}
// update 'initial-cluster' when only the name is specified (e.g. 'etcd --name=abc')
Expand All @@ -1225,7 +1260,7 @@ func (cfg *Config) UpdateDefaultClusterFromName(defaultInitialCluster string) (s

cip, cport := cfg.ListenClientUrls[0].Hostname(), cfg.ListenClientUrls[0].Port()
if cfg.defaultClientHost() && cip == "0.0.0.0" {
cfg.AdvertiseClientUrls[0] = url.URL{Scheme: cfg.AdvertiseClientUrls[0].Scheme, Host: fmt.Sprintf("%s:%s", defaultHostname, cport)}
cfg.getAdvertiseClientURLs()[0] = url.URL{Scheme: cfg.getAdvertiseClientURLs()[0].Scheme, Host: fmt.Sprintf("%s:%s", defaultHostname, cport)}
used = true
}
dhost := defaultHostname
Expand Down Expand Up @@ -1270,12 +1305,8 @@ func checkHostURLs(urls []url.URL) error {
return nil
}

func (cfg *Config) getAdvertisePeerUrls() (ss []string) {
ss = make([]string, len(cfg.AdvertisePeerUrls))
for i := range cfg.AdvertisePeerUrls {
ss[i] = cfg.AdvertisePeerUrls[i].String()
}
return ss
func (cfg *Config) getAdvertisePeerURLsAsStrings() []string {
return toStringSlice(cfg.getAdvertisePeerURLs())
}

func (cfg *Config) getListenPeerUrls() (ss []string) {
Expand All @@ -1286,12 +1317,8 @@ func (cfg *Config) getListenPeerUrls() (ss []string) {
return ss
}

func (cfg *Config) getAdvertiseClientUrls() (ss []string) {
ss = make([]string, len(cfg.AdvertiseClientUrls))
for i := range cfg.AdvertiseClientUrls {
ss[i] = cfg.AdvertiseClientUrls[i].String()
}
return ss
func (cfg *Config) getAdvertiseClientURLsAsStrings() []string {
return toStringSlice(cfg.getAdvertiseClientURLs())
}

func (cfg *Config) getListenClientUrls() (ss []string) {
Expand All @@ -1310,6 +1337,14 @@ func (cfg *Config) getMetricsURLs() (ss []string) {
return ss
}

func toStringSlice(urls []url.URL) []string {
ss := make([]string, len(urls))
for i := range urls {
ss[i] = urls[i].String()
}
return ss
}

func parseBackendFreelistType(freelistType string) bolt.FreelistType {
if freelistType == freelistArrayType {
return bolt.FreelistArrayType
Expand Down
26 changes: 13 additions & 13 deletions server/embed/config_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -92,9 +92,9 @@ func TestConfigFileOtherFields(t *testing.T) {
func TestUpdateDefaultClusterFromName(t *testing.T) {
cfg := NewConfig()
defaultInitialCluster := cfg.InitialCluster
oldscheme := cfg.AdvertisePeerUrls[0].Scheme
origpeer := cfg.AdvertisePeerUrls[0].String()
origadvc := cfg.AdvertiseClientUrls[0].String()
oldscheme := cfg.getAdvertisePeerURLs()[0].Scheme
origpeer := cfg.getAdvertisePeerURLs()[0].String()
origadvc := cfg.getAdvertiseClientURLs()[0].String()

cfg.Name = "abc"
lpport := cfg.ListenPeerUrls[0].Port()
Expand All @@ -106,12 +106,12 @@ func TestUpdateDefaultClusterFromName(t *testing.T) {
t.Fatalf("initial-cluster expected %q, got %q", exp, cfg.InitialCluster)
}
// advertise peer URL should not be affected
if origpeer != cfg.AdvertisePeerUrls[0].String() {
t.Fatalf("advertise peer url expected %q, got %q", origadvc, cfg.AdvertisePeerUrls[0].String())
if origpeer != cfg.getAdvertisePeerURLs()[0].String() {
t.Fatalf("advertise peer url expected %q, got %q", origadvc, cfg.getAdvertisePeerURLs()[0].String())
}
// advertise client URL should not be affected
if origadvc != cfg.AdvertiseClientUrls[0].String() {
t.Fatalf("advertise client url expected %q, got %q", origadvc, cfg.AdvertiseClientUrls[0].String())
if origadvc != cfg.getAdvertiseClientURLs()[0].String() {
t.Fatalf("advertise client url expected %q, got %q", origadvc, cfg.getAdvertiseClientURLs()[0].String())
}
}

Expand All @@ -124,8 +124,8 @@ func TestUpdateDefaultClusterFromNameOverwrite(t *testing.T) {

cfg := NewConfig()
defaultInitialCluster := cfg.InitialCluster
oldscheme := cfg.AdvertisePeerUrls[0].Scheme
origadvc := cfg.AdvertiseClientUrls[0].String()
oldscheme := cfg.getAdvertisePeerURLs()[0].Scheme
origadvc := cfg.getAdvertiseClientURLs()[0].String()

cfg.Name = "abc"
lpport := cfg.ListenPeerUrls[0].Port()
Expand All @@ -134,7 +134,7 @@ func TestUpdateDefaultClusterFromNameOverwrite(t *testing.T) {
if dhost != defaultHostname {
t.Fatalf("expected default host %q, got %q", defaultHostname, dhost)
}
aphost, apport := cfg.AdvertisePeerUrls[0].Hostname(), cfg.AdvertisePeerUrls[0].Port()
aphost, apport := cfg.getAdvertisePeerURLs()[0].Hostname(), cfg.getAdvertisePeerURLs()[0].Port()
if apport != lpport {
t.Fatalf("advertise peer url got different port %s, expected %s", apport, lpport)
}
Expand All @@ -147,8 +147,8 @@ func TestUpdateDefaultClusterFromNameOverwrite(t *testing.T) {
}

// advertise client URL should not be affected
if origadvc != cfg.AdvertiseClientUrls[0].String() {
t.Fatalf("advertise-client-url expected %q, got %q", origadvc, cfg.AdvertiseClientUrls[0].String())
if origadvc != cfg.getAdvertiseClientURLs()[0].String() {
t.Fatalf("advertise-client-url expected %q, got %q", origadvc, cfg.getAdvertiseClientURLs()[0].String())
}
}

Expand Down Expand Up @@ -287,7 +287,7 @@ func TestPeerURLsMapAndTokenFromSRV(t *testing.T) {
cfg.InitialCluster = ""
cfg.InitialClusterToken = ""
cfg.DNSCluster = "example.com"
cfg.AdvertisePeerUrls = types.MustNewURLs(tt.apurls)
cfg.AdvertisePeerURLs = types.MustNewURLs(tt.apurls)

if err := cfg.Validate(); err != nil {
t.Errorf("#%d: failed to validate test Config: %v", i, err)
Expand Down
16 changes: 8 additions & 8 deletions server/embed/etcd.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -168,8 +168,8 @@ func StartEtcd(inCfg *Config) (e *Etcd, err error) {

srvcfg := config.ServerConfig{
Name: cfg.Name,
ClientURLs: cfg.AdvertiseClientUrls,
PeerURLs: cfg.AdvertisePeerUrls,
ClientURLs: cfg.AdvertiseClientURLs,
PeerURLs: cfg.AdvertisePeerURLs,
DataDir: cfg.Dir,
DedicatedWALDir: cfg.WalDir,
SnapshotCount: cfg.SnapshotCount,
Expand Down Expand Up @@ -280,9 +280,9 @@ func StartEtcd(inCfg *Config) (e *Etcd, err error) {
e.cfg.logger.Info(
"now serving peer/client/metrics",
zap.String("local-member-id", e.Server.MemberID().String()),
zap.Strings("initial-advertise-peer-urls", e.cfg.getAdvertisePeerUrls()),
zap.Strings("initial-advertise-peer-urls", e.cfg.getAdvertisePeerURLsAsStrings()),
zap.Strings("listen-peer-urls", e.cfg.getListenPeerUrls()),
zap.Strings("advertise-client-urls", e.cfg.getAdvertiseClientUrls()),
zap.Strings("advertise-client-urls", e.cfg.getAdvertiseClientURLsAsStrings()),
zap.Strings("listen-client-urls", e.cfg.getListenClientUrls()),
zap.Strings("listen-metrics-urls", e.cfg.getMetricsURLs()),
)
Expand Down Expand Up @@ -331,9 +331,9 @@ func print(lg *zap.Logger, ec Config, sc config.ServerConfig, memberInitialized
zap.Uint("max-wals", sc.MaxWALFiles),
zap.Uint("max-snapshots", sc.MaxSnapFiles),
zap.Uint64("snapshot-catchup-entries", sc.SnapshotCatchUpEntries),
zap.Strings("initial-advertise-peer-urls", ec.getAdvertisePeerUrls()),
zap.Strings("initial-advertise-peer-urls", ec.getAdvertisePeerURLsAsStrings()),
zap.Strings("listen-peer-urls", ec.getListenPeerUrls()),
zap.Strings("advertise-client-urls", ec.getAdvertiseClientUrls()),
zap.Strings("advertise-client-urls", ec.getAdvertiseClientURLsAsStrings()),
zap.Strings("listen-client-urls", ec.getListenClientUrls()),
zap.Strings("listen-metrics-urls", ec.getMetricsURLs()),
zap.Strings("cors", cors),
Expand Down Expand Up @@ -386,8 +386,8 @@ func (e *Etcd) Close() {
fields := []zap.Field{
zap.String("name", e.cfg.Name),
zap.String("data-dir", e.cfg.Dir),
zap.Strings("advertise-peer-urls", e.cfg.getAdvertisePeerUrls()),
zap.Strings("advertise-client-urls", e.cfg.getAdvertiseClientUrls()),
zap.Strings("advertise-peer-urls", e.cfg.getAdvertisePeerURLsAsStrings()),
zap.Strings("advertise-client-urls", e.cfg.getAdvertiseClientURLsAsStrings()),
}
lg := e.GetLogger()
lg.Info("closing etcd server", fields...)
Expand Down
Loading

0 comments on commit e4dfc4f

Please sign in to comment.