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.

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

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

Merged
merged 15 commits into from
May 10, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 2 additions & 2 deletions etcdctl/ctlv3/command/del_command.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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."))
}
Expand Down
4 changes: 2 additions & 2 deletions etcdctl/ctlv3/command/get_command.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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."))
}
Expand Down
4 changes: 2 additions & 2 deletions etcdctl/ctlv3/command/put_command.go
Original file line number Diff line number Diff line change
Expand Up @@ -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...)
Expand All @@ -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."))
}
Expand Down
10 changes: 4 additions & 6 deletions etcdctl/ctlv3/command/txn_command.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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}
Expand Down
8 changes: 4 additions & 4 deletions etcdctl/ctlv3/command/watch_command.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Expand Down Expand Up @@ -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()

Expand Down
4 changes: 2 additions & 2 deletions etcdmain/gateway.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
6 changes: 3 additions & 3 deletions etcdserver/api/v2http/capability.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion etcdserver/api/v2http/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
10 changes: 5 additions & 5 deletions etcdserver/api/v2http/client_auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
36 changes: 18 additions & 18 deletions etcdserver/api/v2http/client_auth_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
}
Expand Down Expand Up @@ -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": {
Expand All @@ -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": {
Expand Down Expand Up @@ -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": {
Expand Down Expand Up @@ -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": {
Expand Down Expand Up @@ -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"),
Expand Down Expand Up @@ -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": {
Expand Down Expand Up @@ -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,
},
Expand Down Expand Up @@ -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,
},
}
Expand Down
3 changes: 1 addition & 2 deletions etcdserver/api/v2http/metrics.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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())
}
Expand Down
8 changes: 8 additions & 0 deletions etcdserver/api/v2v3/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
"context"
"fmt"
"path"
"sort"
"strings"
"time"

Expand Down Expand Up @@ -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
Expand All @@ -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
}

Expand Down
12 changes: 6 additions & 6 deletions etcdserver/v2auth/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand All @@ -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 {
Expand Down
Loading