Merge pull request #7064 from heyitsanthony/fix-health-perms

etcdctl: treat permission denied as healthy endpoint

e2e/ctl_v3_endpoint_test.go

@@ -40,15 +40,6 @@ func ctlV3EndpointHealth(cx ctlCtx) error {
 	return spawnWithExpects(cmdArgs, lines...)
 }
 
-func ctlV3EndpointHealthWithKey(cx ctlCtx, key string) error {
-	cmdArgs := append(cx.PrefixArgs(), "endpoint", "health", "--health-check-key", key)
-	lines := make([]string, cx.epc.cfg.clusterSize)
-	for i := range lines {
-		lines[i] = "is healthy"
-	}
-	return spawnWithExpects(cmdArgs, lines...)
-}
-
 func endpointStatusTest(cx ctlCtx) {
 	if err := ctlV3EndpointStatus(cx); err != nil {
 		cx.t.Fatalf("endpointStatusTest ctlV3EndpointStatus error (%v)", err)
@@ -65,15 +56,6 @@ func ctlV3EndpointStatus(cx ctlCtx) error {
 	return spawnWithExpects(cmdArgs, eps...)
 }
 
-func ctlV3EndpointHealthFailPermissionDenied(cx ctlCtx) error {
-	cmdArgs := append(cx.PrefixArgs(), "endpoint", "health")
-	lines := make([]string, cx.epc.cfg.clusterSize)
-	for i := range lines {
-		lines[i] = "is unhealthy: failed to commit proposal: etcdserver: permission denied"
-	}
-	return spawnWithExpects(cmdArgs, lines...)
-}
-
 func endpointHealthTestWithAuth(cx ctlCtx) {
 	if err := authEnable(cx); err != nil {
 		cx.t.Fatal(err)
@@ -86,19 +68,19 @@ func endpointHealthTestWithAuth(cx ctlCtx) {
 		cx.t.Fatalf("endpointStatusTest ctlV3EndpointHealth error (%v)", err)
 	}
 
-	// health checking with an ordinal user must fail because the user isn't granted a permission of the key "health"
+	// health checking with an ordinal user "succeeds" since permission denial goes through consensus
 	cx.user, cx.pass = "test-user", "pass"
-	if err := ctlV3EndpointHealthFailPermissionDenied(cx); err != nil {
+	if err := ctlV3EndpointHealth(cx); err != nil {
 		cx.t.Fatalf("endpointStatusTest ctlV3EndpointHealth error (%v)", err)
 	}
 
+	// succeed if permissions granted for ordinary user
 	cx.user, cx.pass = "root", "root"
-	if err := ctlV3RoleGrantPermission(cx, "test-role", grantingPerm{true, true, "custom-key", "", false}); err != nil {
+	if err := ctlV3RoleGrantPermission(cx, "test-role", grantingPerm{true, true, "health", "", false}); err != nil {
 		cx.t.Fatal(err)
 	}
-
 	cx.user, cx.pass = "test-user", "pass"
-	if err := ctlV3EndpointHealthWithKey(cx, "custom-key"); err != nil {
+	if err := ctlV3EndpointHealth(cx); err != nil {
 		cx.t.Fatalf("endpointStatusTest ctlV3EndpointHealth error (%v)", err)
 	}
 }

etcdctl/ctlv3/command/ep_command.go

@@ -21,12 +21,10 @@ import (
 	"time"
 
 	v3 "github.com/coreos/etcd/clientv3"
+	"github.com/coreos/etcd/etcdserver/api/v3rpc/rpctypes"
 	"github.com/coreos/etcd/pkg/flags"
-	"github.com/spf13/cobra"
-)
 
-var (
-	healthCheckKey string
+	"github.com/spf13/cobra"
 )
 
 // NewEndpointCommand returns the cobra command for "endpoint".
@@ -49,8 +47,6 @@ func newEpHealthCommand() *cobra.Command {
 		Run:   epHealthCommandFunc,
 	}
 
-	cmd.Flags().StringVar(&healthCheckKey, "health-check-key", "health", "The key used to perform the health check. Makes sure the user can access the key.")
-
 	return cmd
 }
 
@@ -101,12 +97,13 @@ func epHealthCommandFunc(cmd *cobra.Command, args []string) {
 			// get a random key. As long as we can get the response without an error, the
 			// endpoint is health.
 			ctx, cancel := commandCtx(cmd)
-			_, err = cli.Get(ctx, healthCheckKey)
+			_, err = cli.Get(ctx, "health")
 			cancel()
-			if err != nil {
-				fmt.Printf("%s is unhealthy: failed to commit proposal: %v\n", ep, err)
-			} else {
+			// permission denied is OK since proposal goes through consensus to get it
+			if err == nil || err == rpctypes.ErrPermissionDenied {
 				fmt.Printf("%s is healthy: successfully committed proposal: took = %v\n", ep, time.Since(st))
+			} else {
+				fmt.Printf("%s is unhealthy: failed to commit proposal: %v\n", ep, err)
 			}
 		}(cfg)
 	}