etcdserver: allow non-admin to fetch member list and alarms

In some environments, etcd members do not have stable hostnames or IP
addresses. During maintenance, all etcd nodes may be replaced, resulting in
new hostnames and IPs for every member. In that case, clients such as Patroni
can lose access to the cluster entirely if they are not allowed to refresh the
member list.

Allow non-admin users to fetch the member list so they can rediscover updated
member endpoints after such topology changes.

Signed-off-by: Wei Fu <fuweid89@gmail.com>
(cherry picked from commit a2987fd)
Signed-off-by: Wei Fu <fuweid89@gmail.com>

Author: fuweid

server/etcdserver/api/v3rpc/auth.go

@@ -185,3 +185,19 @@ func (aa *AuthAdmin) isPermitted(ctx context.Context) error {
 
 	return aa.ag.AuthStore().IsAdminPermitted(authInfo)
 }
+
+func (aa *AuthAdmin) requireAuthInfo(ctx context.Context) error {
+	if !aa.ag.AuthStore().IsAuthEnabled() {
+		return nil
+	}
+
+	authInfo, err := aa.ag.AuthInfoFromCtx(ctx)
+	if err != nil {
+		return err
+	}
+
+	if authInfo == nil {
+		return auth.ErrUserEmpty
+	}
+	return nil
+}

server/etcdserver/api/v3rpc/maintenance.go

@@ -350,8 +350,15 @@ func (ams *authMaintenanceServer) HashKV(ctx context.Context, r *pb.HashKVReques
 }
 
 func (ams *authMaintenanceServer) Alarm(ctx context.Context, ar *pb.AlarmRequest) (*pb.AlarmResponse, error) {
-	if err := ams.isPermitted(ctx); err != nil {
-		return nil, togRPCError(err)
+	switch ar.GetAction() {
+	case pb.AlarmRequest_GET:
+		if err := ams.requireAuthInfo(ctx); err != nil {
+			return nil, togRPCError(err)
+		}
+	default:
+		if err := ams.isPermitted(ctx); err != nil {
+			return nil, togRPCError(err)
+		}
 	}
 	return ams.maintenanceServer.Alarm(ctx, ar)
 }

server/etcdserver/server.go

@@ -1677,7 +1677,7 @@ func (s *EtcdServer) MemberList(ctx context.Context, r *pb.MemberListRequest) ([
 		}
 	}
 
-	if err := s.checkMembershipOperationPermission(ctx); err != nil {
+	if err := s.requireAuthInfo(ctx); err != nil {
 		return nil, err
 	}
 	return s.cluster.Members(), nil

tests/common/auth_test.go

@@ -691,9 +691,14 @@ func TestAuthMemberList(t *testing.T) {
 		require.NoErrorf(t, setupAuth(cc, []authRole{testRole}, []authUser{rootUser, testUser}), "failed to enable auth")
 		rootAuthClient := testutils.MustClient(clus.Client(WithAuth(rootUserName, rootPassword)))
 		testUserAuthClient := testutils.MustClient(clus.Client(WithAuth(testUserName, testPassword)))
+		anonAuthClient := testutils.MustClient(clus.Client())
 
 		_, err := testUserAuthClient.MemberList(ctx, false)
-		require.ErrorContains(t, err, PermissionDenied)
+		require.NoError(t, err)
+
+		_, err = anonAuthClient.MemberList(ctx, false)
+		require.Error(t, err)
+		require.ErrorContains(t, err, "etcdserver: user name is empty")
 
 		_, err = rootAuthClient.MemberList(ctx, false)
 		require.NoError(t, err)
@@ -994,6 +999,7 @@ func TestAuthAlarm(t *testing.T) {
 		require.NoErrorf(t, setupAuth(cc, []authRole{testRole}, []authUser{rootUser, testUser}), "failed to enable auth")
 		rootAuthClient := testutils.MustClient(clus.Client(WithAuth(rootUserName, rootPassword)))
 		testUserAuthClient := testutils.MustClient(clus.Client(WithAuth(testUserName, testPassword)))
+		anonAuthClient := testutils.MustClient(clus.Client())
 
 		for i := 0; ; i++ {
 			err := rootAuthClient.Put(ctx,
@@ -1006,9 +1012,6 @@ func TestAuthAlarm(t *testing.T) {
 			break
 		}
 
-		_, err := testUserAuthClient.AlarmList(ctx)
-		require.ErrorContains(t, err, PermissionDenied)
-
 		memberID := uint64(0)
 
 		for i := 0; i < 10; i++ {
@@ -1023,6 +1026,12 @@ func TestAuthAlarm(t *testing.T) {
 		}
 		require.NotEqualf(t, uint64(0), memberID, "expect to find alarm with non-zero member ID")
 
+		_, err := anonAuthClient.AlarmList(ctx)
+		require.ErrorContains(t, err, "etcdserver: user name is empty")
+
+		_, err = testUserAuthClient.AlarmList(ctx)
+		require.NoError(t, err)
+
 		_, err = testUserAuthClient.AlarmDisarm(ctx, &clientv3.AlarmMember{
 			MemberID: memberID,
 			Alarm:    pb.AlarmType_NOSPACE,

tests/e2e/ctl_v3_auth_test.go

@@ -15,7 +15,6 @@
 package e2e
 
 import (
-	"context"
 	"fmt"
 	"os"
 	"strings"
@@ -296,26 +295,7 @@ func authTestEndpointHealth(cx ctlCtx) {
 	require.NoError(cx.t, ctlV3RoleGrantPermission(cx, "test-role", grantingPerm{true, true, "health", "", false}))
 
 	cx.user, cx.pass = "test-user", "pass"
-	func(cx ctlCtx) {
-		cmdArgs := append(cx.PrefixArgs(), "endpoint", "health")
-		lines := make([]expect.ExpectedResponse, cx.epc.Cfg.ClusterSize)
-		for i := range lines {
-			lines[i] = expect.ExpectedResponse{
-				Value: cx.epc.Procs[i].EndpointsGRPC()[0] + " is unhealthy: failed to commit proposal: Unable to fetch the alarm list",
-			}
-		}
-
-		proc, err := e2e.SpawnCmd(cmdArgs, cx.envMap)
-		require.NoErrorf(cx.t, err, "failed to spawn endpoint health command")
-		defer func() {
-			require.Errorf(cx.t, proc.Close(), "endpoint health command should reject all non-root users")
-		}()
-
-		for _, line := range lines {
-			_, lerr := proc.ExpectWithContext(context.TODO(), line)
-			require.NoErrorf(cx.t, lerr, "endpoint health should fail with permission denied error")
-		}
-	}(cx)
+	require.NoErrorf(cx.t, ctlV3EndpointHealth(cx), "endpointStatusTest ctlV3EndpointHealth error")
 
 	cmdArgs := append(cx.PrefixArgs(), "endpoint", "health", "--user=root:root", "--cluster")
 	proc, err := e2e.SpawnCmd(cmdArgs, cx.envMap)