From 7753b97cc7dd597a7ed3a4fbb14a6906e36ab108 Mon Sep 17 00:00:00 2001 From: Christian Muehlhaeuser Date: Sat, 20 Jul 2019 15:37:19 +0200 Subject: [PATCH] Simplified code in various places (#6176) All these changes should have no side-effects or change behavior: - Use bytes.Buffer's String() instead of a conversion - Use time.Since and time.Until where fitting - Drop unnecessary returns and assignment --- agent/agent_endpoint.go | 2 +- agent/agent_endpoint_test.go | 4 ++-- agent/cache/cache.go | 2 +- agent/catalog_endpoint.go | 2 +- agent/connect/ca/provider_consul_test.go | 2 +- agent/connect/ca/provider_vault_test.go | 2 +- agent/consul/acl_replication_types.go | 2 +- agent/consul/kvs_endpoint_test.go | 6 +----- agent/consul/leader.go | 2 +- agent/consul/server_lookup.go | 3 +-- agent/consul/server_test.go | 1 - agent/consul/state/acl_test.go | 4 ++-- agent/http.go | 2 +- agent/router/manager.go | 2 -- agent/ui_endpoint_test.go | 2 +- command/debug/debug.go | 2 +- lib/telemetry.go | 2 +- sdk/testutil/retry/retry.go | 2 +- 18 files changed, 18 insertions(+), 26 deletions(-) diff --git a/agent/agent_endpoint.go b/agent/agent_endpoint.go index e25f1ea6b1..4cecd1d41f 100644 --- a/agent/agent_endpoint.go +++ b/agent/agent_endpoint.go @@ -141,7 +141,7 @@ func (s *HTTPServer) AgentReload(resp http.ResponseWriter, req *http.Request) (i } // Trigger the reload - errCh := make(chan error, 0) + errCh := make(chan error) select { case <-s.agent.shutdownCh: return nil, fmt.Errorf("Agent was shutdown before reload could be completed") diff --git a/agent/agent_endpoint_test.go b/agent/agent_endpoint_test.go index b0fc8eb8ba..397c70d529 100644 --- a/agent/agent_endpoint_test.go +++ b/agent/agent_endpoint_test.go @@ -532,7 +532,7 @@ func TestAgent_Service(t *testing.T) { } start := time.Now() obj, err := a.srv.AgentService(resp, req) - elapsed := time.Now().Sub(start) + elapsed := time.Since(start) if tt.wantErr != "" { require.Error(err) @@ -5298,7 +5298,7 @@ func TestAgentConnectProxyConfig_Blocking(t *testing.T) { } start := time.Now() obj, err := a.srv.AgentConnectProxyConfig(resp, req) - elapsed := time.Now().Sub(start) + elapsed := time.Since(start) if tt.wantErr { require.Error(err) diff --git a/agent/cache/cache.go b/agent/cache/cache.go index ee032bbb99..7892272451 100644 --- a/agent/cache/cache.go +++ b/agent/cache/cache.go @@ -703,7 +703,7 @@ func (c *Cache) runExpiryLoop() { c.entriesLock.RLock() if len(c.entriesExpiryHeap.Entries) > 0 { entry = c.entriesExpiryHeap.Entries[0] - expiryTimer = time.NewTimer(entry.Expires.Sub(time.Now())) + expiryTimer = time.NewTimer(time.Until(entry.Expires)) expiryCh = expiryTimer.C } c.entriesLock.RUnlock() diff --git a/agent/catalog_endpoint.go b/agent/catalog_endpoint.go index 7381cdf642..6f4b428abf 100644 --- a/agent/catalog_endpoint.go +++ b/agent/catalog_endpoint.go @@ -151,7 +151,7 @@ RETRY_ONCE: // Use empty map instead of nil if out.Services == nil { - out.Services = make(structs.Services, 0) + out.Services = make(structs.Services) } metrics.IncrCounterWithLabels([]string{"client", "api", "success", "catalog_services"}, 1, []metrics.Label{{Name: "node", Value: s.nodeName()}}) diff --git a/agent/connect/ca/provider_consul_test.go b/agent/connect/ca/provider_consul_test.go index 479d460e6a..6ec9b56752 100644 --- a/agent/connect/ca/provider_consul_test.go +++ b/agent/connect/ca/provider_consul_test.go @@ -178,7 +178,7 @@ func TestConsulCAProvider_SignLeaf(t *testing.T) { require.Equal(parsed.SerialNumber.Uint64(), uint64(2)) // Ensure the cert is valid now and expires within the correct limit. - require.True(parsed.NotAfter.Sub(time.Now()) < 3*24*time.Hour) + require.True(time.Until(parsed.NotAfter) < 3*24*time.Hour) require.True(parsed.NotBefore.Before(time.Now())) } diff --git a/agent/connect/ca/provider_vault_test.go b/agent/connect/ca/provider_vault_test.go index 10be1befb4..b0ddaa4112 100644 --- a/agent/connect/ca/provider_vault_test.go +++ b/agent/connect/ca/provider_vault_test.go @@ -186,7 +186,7 @@ func TestVaultCAProvider_SignLeaf(t *testing.T) { require.NotEqual(firstSerial, parsed.SerialNumber.Uint64()) // Ensure the cert is valid now and expires within the correct limit. - require.True(parsed.NotAfter.Sub(time.Now()) < time.Hour) + require.True(time.Until(parsed.NotAfter) < time.Hour) require.True(parsed.NotBefore.Before(time.Now())) } } diff --git a/agent/consul/acl_replication_types.go b/agent/consul/acl_replication_types.go index 3009bec983..e0222e1443 100644 --- a/agent/consul/acl_replication_types.go +++ b/agent/consul/acl_replication_types.go @@ -316,7 +316,7 @@ func (r *aclRoleReplicator) FetchUpdated(srv *Server, updates []string) (int, er delete(keep, role.ID) } missing := make([]string, 0, len(keep)) - for id, _ := range keep { + for id := range keep { missing = append(missing, id) } return 0, fmt.Errorf("role replication trying to replicated uncached roles with IDs: %v", missing) diff --git a/agent/consul/kvs_endpoint_test.go b/agent/consul/kvs_endpoint_test.go index a91985c725..e3ba380df5 100644 --- a/agent/consul/kvs_endpoint_test.go +++ b/agent/consul/kvs_endpoint_test.go @@ -596,11 +596,7 @@ key "zip" { t.Fatalf("err: %v", err) } - actualKeys = []string{} - - for _, key := range keyList.Keys { - actualKeys = append(actualKeys, key) - } + actualKeys = keyList.Keys verify.Values(t, "", actualKeys, expectedKeys) diff --git a/agent/consul/leader.go b/agent/consul/leader.go index 21d96c3607..28acd7c312 100644 --- a/agent/consul/leader.go +++ b/agent/consul/leader.go @@ -1192,7 +1192,7 @@ func (s *Server) pruneCARoots() error { var newRoots structs.CARoots for _, r := range roots { - if !r.Active && !r.RotatedOutAt.IsZero() && time.Now().Sub(r.RotatedOutAt) > common.LeafCertTTL*2 { + if !r.Active && !r.RotatedOutAt.IsZero() && time.Since(r.RotatedOutAt) > common.LeafCertTTL*2 { s.logger.Printf("[INFO] connect: pruning old unused root CA (ID: %s)", r.ID) continue } diff --git a/agent/consul/server_lookup.go b/agent/consul/server_lookup.go index e163856d71..f40b573770 100644 --- a/agent/consul/server_lookup.go +++ b/agent/consul/server_lookup.go @@ -51,8 +51,7 @@ func (sl *ServerLookup) ServerAddr(id raft.ServerID) (raft.ServerAddress, error) func (sl *ServerLookup) Server(addr raft.ServerAddress) *metadata.Server { sl.lock.RLock() defer sl.lock.RUnlock() - svr, _ := sl.addressToServer[addr] - return svr + return sl.addressToServer[addr] } func (sl *ServerLookup) Servers() []*metadata.Server { diff --git a/agent/consul/server_test.go b/agent/consul/server_test.go index 32eadb232c..47278b78c3 100644 --- a/agent/consul/server_test.go +++ b/agent/consul/server_test.go @@ -820,7 +820,6 @@ func TestServer_BadExpect(t *testing.T) { type fakeGlobalResp struct{} func (r *fakeGlobalResp) Add(interface{}) { - return } func (r *fakeGlobalResp) New() interface{} { diff --git a/agent/consul/state/acl_test.go b/agent/consul/state/acl_test.go index 58c69f7433..563f6bde08 100644 --- a/agent/consul/state/acl_test.go +++ b/agent/consul/state/acl_test.go @@ -3824,11 +3824,11 @@ func stripIrrelevantTokenFields(token *structs.ACLToken) *structs.ACLToken { // When comparing the tokens disregard the policy link names. This // data is not cleanly updated in a variety of scenarios and should not // be relied upon. - for i, _ := range tokenCopy.Policies { + for i := range tokenCopy.Policies { tokenCopy.Policies[i].Name = "" } // Also do the same for Role links. - for i, _ := range tokenCopy.Roles { + for i := range tokenCopy.Roles { tokenCopy.Roles[i].Name = "" } // The raft indexes won't match either because the requester will not diff --git a/agent/http.go b/agent/http.go index 9025cc088e..bd876f46b4 100644 --- a/agent/http.go +++ b/agent/http.go @@ -293,7 +293,7 @@ func (s *HTTPServer) handler(enableDebug bool) http.Handler { mux.HandleFunc("/", s.Index) for pattern, fn := range endpoints { thisFn := fn - methods, _ := allowedMethods[pattern] + methods := allowedMethods[pattern] bound := func(resp http.ResponseWriter, req *http.Request) (interface{}, error) { return thisFn(s, resp, req) } diff --git a/agent/router/manager.go b/agent/router/manager.go index 081893c5e9..ae764087c4 100644 --- a/agent/router/manager.go +++ b/agent/router/manager.go @@ -342,8 +342,6 @@ func (m *Manager) RebalanceServers() { // continue to use the existing connection until the next // rebalance occurs. } - - return } // reconcileServerList returns true when the first server in serverList diff --git a/agent/ui_endpoint_test.go b/agent/ui_endpoint_test.go index 940e3ec36b..51d19a0bcf 100644 --- a/agent/ui_endpoint_test.go +++ b/agent/ui_endpoint_test.go @@ -60,7 +60,7 @@ func TestUiIndex(t *testing.T) { // Verify the body out := bytes.NewBuffer(nil) io.Copy(out, resp.Body) - if string(out.Bytes()) != "test" { + if out.String() != "test" { t.Fatalf("bad: %s", out.Bytes()) } } diff --git a/command/debug/debug.go b/command/debug/debug.go index 36a8ae3662..a9697b624a 100644 --- a/command/debug/debug.go +++ b/command/debug/debug.go @@ -302,7 +302,7 @@ func (c *cmd) captureStatic() error { var errors error // Collect the named outputs here - outputs := make(map[string]interface{}, 0) + outputs := make(map[string]interface{}) // Capture host information if c.configuredTarget("host") { diff --git a/lib/telemetry.go b/lib/telemetry.go index a335d6cc85..d815f4b587 100644 --- a/lib/telemetry.go +++ b/lib/telemetry.go @@ -225,7 +225,7 @@ func (c *TelemetryConfig) MergeDefaults(defaults *TelemetryConfig) { continue } case reflect.Bool: - if f.Bool() != false { + if f.Bool() { continue } default: diff --git a/sdk/testutil/retry/retry.go b/sdk/testutil/retry/retry.go index 2ef3c4c0eb..53c05a2b05 100644 --- a/sdk/testutil/retry/retry.go +++ b/sdk/testutil/retry/retry.go @@ -110,7 +110,7 @@ func dedup(a []string) string { delete(m, s) } } - return string(b.Bytes()) + return b.String() } func run(r Retryer, t Failer, f func(r *R)) {