Merge pull request #9712 from gyuho/unparam

*: test with "unparam", fix "v2v3" store stored get

etcdctl/ctlv3/command/del_command.go

@@ -43,7 +43,7 @@ func NewDelCommand() *cobra.Command {
 
 // delCommandFunc executes the "del" command.
 func delCommandFunc(cmd *cobra.Command, args []string) {
-	key, opts := getDelOp(cmd, args)
+	key, opts := getDelOp(args)
 	ctx, cancel := commandCtx(cmd)
 	resp, err := mustClientFromCmd(cmd).Delete(ctx, key, opts...)
 	cancel()
@@ -53,7 +53,7 @@ func delCommandFunc(cmd *cobra.Command, args []string) {
 	display.Del(*resp)
 }
 
-func getDelOp(cmd *cobra.Command, args []string) (string, []clientv3.OpOption) {
+func getDelOp(args []string) (string, []clientv3.OpOption) {
 	if len(args) == 0 || len(args) > 2 {
 		ExitWithError(ExitBadArgs, fmt.Errorf("del command needs one argument as key and an optional argument as range_end."))
 	}

etcdctl/ctlv3/command/get_command.go

@@ -56,7 +56,7 @@ func NewGetCommand() *cobra.Command {
 
 // getCommandFunc executes the "get" command.
 func getCommandFunc(cmd *cobra.Command, args []string) {
-	key, opts := getGetOp(cmd, args)
+	key, opts := getGetOp(args)
 	ctx, cancel := commandCtx(cmd)
 	resp, err := mustClientFromCmd(cmd).Get(ctx, key, opts...)
 	cancel()
@@ -74,7 +74,7 @@ func getCommandFunc(cmd *cobra.Command, args []string) {
 	display.Get(*resp)
 }
 
-func getGetOp(cmd *cobra.Command, args []string) (string, []clientv3.OpOption) {
+func getGetOp(args []string) (string, []clientv3.OpOption) {
 	if len(args) == 0 {
 		ExitWithError(ExitBadArgs, fmt.Errorf("range command needs arguments."))
 	}

etcdctl/ctlv3/command/put_command.go

@@ -65,7 +65,7 @@ will store the content of the file to <key>.
 
 // putCommandFunc executes the "put" command.
 func putCommandFunc(cmd *cobra.Command, args []string) {
-	key, value, opts := getPutOp(cmd, args)
+	key, value, opts := getPutOp(args)
 
 	ctx, cancel := commandCtx(cmd)
 	resp, err := mustClientFromCmd(cmd).Put(ctx, key, value, opts...)
@@ -76,7 +76,7 @@ func putCommandFunc(cmd *cobra.Command, args []string) {
 	display.Put(*resp)
 }
 
-func getPutOp(cmd *cobra.Command, args []string) (string, string, []clientv3.OpOption) {
+func getPutOp(args []string) (string, string, []clientv3.OpOption) {
 	if len(args) == 0 {
 		ExitWithError(ExitBadArgs, fmt.Errorf("put command needs 1 argument and input from stdin or 2 arguments."))
 	}

etcdctl/ctlv3/command/txn_command.go

@@ -28,9 +28,7 @@ import (
 	"github.com/spf13/cobra"
 )
 
-var (
-	txnInteractive bool
-)
+var txnInteractive bool
 
 // NewTxnCommand returns the cobra command for "txn".
 func NewTxnCommand() *cobra.Command {
@@ -129,17 +127,17 @@ func parseRequestUnion(line string) (*clientv3.Op, error) {
 
 	put := NewPutCommand()
 	put.Run = func(cmd *cobra.Command, args []string) {
-		key, value, opts := getPutOp(cmd, args)
+		key, value, opts := getPutOp(args)
 		opc <- clientv3.OpPut(key, value, opts...)
 	}
 	get := NewGetCommand()
 	get.Run = func(cmd *cobra.Command, args []string) {
-		key, opts := getGetOp(cmd, args)
+		key, opts := getGetOp(args)
 		opc <- clientv3.OpGet(key, opts...)
 	}
 	del := NewDelCommand()
 	del.Run = func(cmd *cobra.Command, args []string) {
-		key, opts := getDelOp(cmd, args)
+		key, opts := getDelOp(args)
 		opc <- clientv3.OpDelete(key, opts...)
 	}
 	cmds := &cobra.Command{SilenceErrors: true}

etcdctl/ctlv3/command/watch_command.go

@@ -258,9 +258,9 @@ func parseWatchArgs(osArgs, commandArgs []string, envKey, envRange string, inter
 		}
 
 		flagset := NewWatchCommand().Flags()
-		if err := flagset.Parse(watchArgs); err != nil {
+		if perr := flagset.Parse(watchArgs); perr != nil {
 			watchPrefix, watchRev, watchPrevKey = false, 0, false
-			return nil, nil, err
+			return nil, nil, perr
 		}
 		pArgs := flagset.Args()
 
@@ -298,8 +298,8 @@ func parseWatchArgs(osArgs, commandArgs []string, envKey, envRange string, inter
 
 	if interactive {
 		flagset := NewWatchCommand().Flags()
-		if err := flagset.Parse(argsWithSep); err != nil {
-			return nil, nil, err
+		if perr := flagset.Parse(argsWithSep); perr != nil {
+			return nil, nil, perr
 		}
 		watchArgs = flagset.Args()
 

etcdmain/gateway.go

@@ -106,8 +106,8 @@ func startGateway(cmd *cobra.Command, args []string) {
 	srvs.Endpoints = stripSchema(srvs.Endpoints)
 	if len(srvs.SRVs) == 0 {
 		for _, ep := range srvs.Endpoints {
-			h, p, err := net.SplitHostPort(ep)
-			if err != nil {
+			h, p, serr := net.SplitHostPort(ep)
+			if serr != nil {
 				fmt.Printf("error parsing endpoint %q", ep)
 				os.Exit(1)
 			}

etcdserver/api/v2http/capability.go

@@ -22,10 +22,10 @@ import (
 	"github.com/coreos/etcd/etcdserver/api/v2http/httptypes"
 )
 
-func capabilityHandler(c api.Capability, fn func(http.ResponseWriter, *http.Request)) http.HandlerFunc {
+func authCapabilityHandler(fn func(http.ResponseWriter, *http.Request)) http.HandlerFunc {
 	return func(w http.ResponseWriter, r *http.Request) {
-		if !api.IsCapabilityEnabled(c) {
-			notCapable(w, r, c)
+		if !api.IsCapabilityEnabled(api.AuthCapability) {
+			notCapable(w, r, api.AuthCapability)
 			return
 		}
 		fn(w, r)

etcdserver/api/v2http/client.go

@@ -155,7 +155,7 @@ func (h *keysHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 				plog.Errorf("error writing event (%v)", err)
 			}
 		}
-		reportRequestCompleted(rr, resp, startTime)
+		reportRequestCompleted(rr, startTime)
 	case resp.Watcher != nil:
 		ctx, cancel := context.WithTimeout(context.Background(), defaultWatchTimeout)
 		defer cancel()

etcdserver/api/v2http/client_auth.go

@@ -228,11 +228,11 @@ func writeNoAuth(lg *zap.Logger, w http.ResponseWriter, r *http.Request) {
 }
 
 func handleAuth(mux *http.ServeMux, sh *authHandler) {
-	mux.HandleFunc(authPrefix+"/roles", capabilityHandler(api.AuthCapability, sh.baseRoles))
-	mux.HandleFunc(authPrefix+"/roles/", capabilityHandler(api.AuthCapability, sh.handleRoles))
-	mux.HandleFunc(authPrefix+"/users", capabilityHandler(api.AuthCapability, sh.baseUsers))
-	mux.HandleFunc(authPrefix+"/users/", capabilityHandler(api.AuthCapability, sh.handleUsers))
-	mux.HandleFunc(authPrefix+"/enable", capabilityHandler(api.AuthCapability, sh.enableDisable))
+	mux.HandleFunc(authPrefix+"/roles", authCapabilityHandler(sh.baseRoles))
+	mux.HandleFunc(authPrefix+"/roles/", authCapabilityHandler(sh.handleRoles))
+	mux.HandleFunc(authPrefix+"/users", authCapabilityHandler(sh.baseUsers))
+	mux.HandleFunc(authPrefix+"/users/", authCapabilityHandler(sh.handleUsers))
+	mux.HandleFunc(authPrefix+"/enable", authCapabilityHandler(sh.enableDisable))
 }
 
 func (sh *authHandler) baseRoles(w http.ResponseWriter, r *http.Request) {

etcdserver/api/v2http/client_auth_test.go

@@ -438,17 +438,17 @@ func TestGetUserGrantedWithNonexistingRole(t *testing.T) {
 	}
 }
 
-func mustAuthRequest(method, username, password string) *http.Request {
-	req, err := http.NewRequest(method, "path", strings.NewReader(""))
+func mustAuthRequest(username, password string) *http.Request {
+	req, err := http.NewRequest(http.MethodGet, "path", strings.NewReader(""))
 	if err != nil {
 		panic("Cannot make auth request: " + err.Error())
 	}
 	req.SetBasicAuth(username, password)
 	return req
 }
 
-func unauthedRequest(method string) *http.Request {
-	req, err := http.NewRequest(method, "path", strings.NewReader(""))
+func unauthedRequest() *http.Request {
+	req, err := http.NewRequest(http.MethodGet, "path", strings.NewReader(""))
 	if err != nil {
 		panic("Cannot make request: " + err.Error())
 	}
@@ -484,7 +484,7 @@ func TestPrefixAccess(t *testing.T) {
 	}{
 		{
 			key: "/foo",
-			req: mustAuthRequest("GET", "root", "good"),
+			req: mustAuthRequest("root", "good"),
 			store: &mockAuthStore{
 				users: map[string]*v2auth.User{
 					"root": {
@@ -506,7 +506,7 @@ func TestPrefixAccess(t *testing.T) {
 		},
 		{
 			key: "/foo",
-			req: mustAuthRequest("GET", "user", "good"),
+			req: mustAuthRequest("user", "good"),
 			store: &mockAuthStore{
 				users: map[string]*v2auth.User{
 					"user": {
@@ -534,7 +534,7 @@ func TestPrefixAccess(t *testing.T) {
 		},
 		{
 			key: "/foo",
-			req: mustAuthRequest("GET", "user", "good"),
+			req: mustAuthRequest("user", "good"),
 			store: &mockAuthStore{
 				users: map[string]*v2auth.User{
 					"user": {
@@ -562,7 +562,7 @@ func TestPrefixAccess(t *testing.T) {
 		},
 		{
 			key: "/foo",
-			req: mustAuthRequest("GET", "user", "bad"),
+			req: mustAuthRequest("user", "bad"),
 			store: &mockAuthStore{
 				users: map[string]*v2auth.User{
 					"user": {
@@ -590,7 +590,7 @@ func TestPrefixAccess(t *testing.T) {
 		},
 		{
 			key: "/foo",
-			req: mustAuthRequest("GET", "user", "good"),
+			req: mustAuthRequest("user", "good"),
 			store: &mockAuthStore{
 				users:   map[string]*v2auth.User{},
 				err:     errors.New("Not the user"),
@@ -659,7 +659,7 @@ func TestPrefixAccess(t *testing.T) {
 		// check access for multiple roles
 		{
 			key: "/foo",
-			req: mustAuthRequest("GET", "user", "good"),
+			req: mustAuthRequest("user", "good"),
 			store: &mockAuthStore{
 				users: map[string]*v2auth.User{
 					"user": {
@@ -815,20 +815,20 @@ func TestUserFromClientCertificate(t *testing.T) {
 	}{
 		{
 			// non tls request
-			req:        unauthedRequest("GET"),
+			req:        unauthedRequest(),
 			userExists: false,
 			store:      witherror,
 		},
 		{
 			// cert with cn of existing user
-			req:        tlsAuthedRequest(unauthedRequest("GET"), "user"),
+			req:        tlsAuthedRequest(unauthedRequest(), "user"),
 			userExists: true,
 			username:   "user",
 			store:      noerror,
 		},
 		{
 			// cert with cn of non-existing user
-			req:        tlsAuthedRequest(unauthedRequest("GET"), "otheruser"),
+			req:        tlsAuthedRequest(unauthedRequest(), "otheruser"),
 			userExists: false,
 			store:      witherror,
 		},
@@ -871,30 +871,30 @@ func TestUserFromBasicAuth(t *testing.T) {
 		{
 			// valid user, valid pass
 			username:   "user",
-			req:        mustAuthRequest("GET", "user", "password"),
+			req:        mustAuthRequest("user", "password"),
 			userExists: true,
 		},
 		{
 			// valid user, bad pass
 			username:   "user",
-			req:        mustAuthRequest("GET", "user", "badpass"),
+			req:        mustAuthRequest("user", "badpass"),
 			userExists: false,
 		},
 		{
 			// valid user, no pass
 			username:   "user",
-			req:        mustAuthRequest("GET", "user", ""),
+			req:        mustAuthRequest("user", ""),
 			userExists: false,
 		},
 		{
 			// missing user
 			username:   "missing",
-			req:        mustAuthRequest("GET", "missing", "badpass"),
+			req:        mustAuthRequest("missing", "badpass"),
 			userExists: false,
 		},
 		{
 			// no basic auth
-			req:        unauthedRequest("GET"),
+			req:        unauthedRequest(),
 			userExists: false,
 		},
 	}

etcdserver/api/v2http/metrics.go

@@ -20,7 +20,6 @@ import (
 
 	"net/http"
 
-	"github.com/coreos/etcd/etcdserver"
 	"github.com/coreos/etcd/etcdserver/api/v2http/httptypes"
 	"github.com/coreos/etcd/etcdserver/etcdserverpb"
 	"github.com/coreos/etcd/etcdserver/v2error"
@@ -64,7 +63,7 @@ func reportRequestReceived(request etcdserverpb.Request) {
 	incomingEvents.WithLabelValues(methodFromRequest(request)).Inc()
 }
 
-func reportRequestCompleted(request etcdserverpb.Request, response etcdserver.Response, startTime time.Time) {
+func reportRequestCompleted(request etcdserverpb.Request, startTime time.Time) {
 	method := methodFromRequest(request)
 	successfulEventsHandlingTime.WithLabelValues(method).Observe(time.Since(startTime).Seconds())
 }

etcdserver/api/v2v3/store.go

@@ -18,6 +18,7 @@ import (
 	"context"
 	"fmt"
 	"path"
+	"sort"
 	"strings"
 	"time"
 
@@ -94,6 +95,9 @@ func (s *v2v3Store) Get(nodePath string, recursive, sorted bool) (*v2store.Event
 func (s *v2v3Store) getDir(nodePath string, recursive, sorted bool, rev int64) ([]*v2store.NodeExtern, error) {
 	rootNodes, err := s.getDirDepth(nodePath, 1, rev)
 	if err != nil || !recursive {
+		if sorted {
+			sort.Sort(v2store.NodeExterns(rootNodes))
+		}
 		return rootNodes, err
 	}
 	nextNodes := rootNodes
@@ -110,6 +114,10 @@ func (s *v2v3Store) getDir(nodePath string, recursive, sorted bool, rev int64) (
 			return nil, err
 		}
 	}
+
+	if sorted {
+		sort.Sort(v2store.NodeExterns(rootNodes))
+	}
 	return rootNodes, nil
 }
 

etcdserver/v2auth/auth.go

@@ -171,7 +171,7 @@ func (_ passwordStore) HashPassword(password string) (string, error) {
 }
 
 func (s *store) AllUsers() ([]string, error) {
-	resp, err := s.requestResource("/users/", false, false)
+	resp, err := s.requestResource("/users/", false)
 	if err != nil {
 		if e, ok := err.(*v2error.Error); ok {
 			if e.ErrorCode == v2error.EcodeKeyNotFound {
@@ -245,7 +245,7 @@ func (s *store) DeleteUser(name string) error {
 	if s.AuthEnabled() && name == "root" {
 		return authErr(http.StatusForbidden, "Cannot delete root user while auth is enabled.")
 	}
-	_, err := s.deleteResource("/users/" + name)
+	err := s.deleteResource("/users/" + name)
 	if err != nil {
 		if e, ok := err.(*v2error.Error); ok {
 			if e.ErrorCode == v2error.EcodeKeyNotFound {
@@ -293,7 +293,7 @@ func (s *store) UpdateUser(user User) (User, error) {
 
 func (s *store) AllRoles() ([]string, error) {
 	nodes := []string{RootRoleName}
-	resp, err := s.requestResource("/roles/", false, false)
+	resp, err := s.requestResource("/roles/", false)
 	if err != nil {
 		if e, ok := err.(*v2error.Error); ok {
 			if e.ErrorCode == v2error.EcodeKeyNotFound {
@@ -338,7 +338,7 @@ func (s *store) DeleteRole(name string) error {
 	if name == RootRoleName {
 		return authErr(http.StatusForbidden, "Cannot modify role %s: is root role.", name)
 	}
-	_, err := s.deleteResource("/roles/" + name)
+	err := s.deleteResource("/roles/" + name)
 	if err != nil {
 		if e, ok := err.(*v2error.Error); ok {
 			if e.ErrorCode == v2error.EcodeKeyNotFound {
@@ -696,7 +696,7 @@ func attachRootRole(u User) User {
 }
 
 func (s *store) getUser(name string, quorum bool) (User, error) {
-	resp, err := s.requestResource("/users/"+name, false, quorum)
+	resp, err := s.requestResource("/users/"+name, quorum)
 	if err != nil {
 		if e, ok := err.(*v2error.Error); ok {
 			if e.ErrorCode == v2error.EcodeKeyNotFound {
@@ -721,7 +721,7 @@ func (s *store) getRole(name string, quorum bool) (Role, error) {
 	if name == RootRoleName {
 		return rootRole, nil
 	}
-	resp, err := s.requestResource("/roles/"+name, false, quorum)
+	resp, err := s.requestResource("/roles/"+name, quorum)
 	if err != nil {
 		if e, ok := err.(*v2error.Error); ok {
 			if e.ErrorCode == v2error.EcodeKeyNotFound {