sql, server: add new system privileges for observability

docs/generated/sql/bnf/stmt_block.bnf

@@ -1359,7 +1359,9 @@ unreserved_keyword ::=
 	| 'VIEW'
 	| 'VIEWACTIVITY'
 	| 'VIEWACTIVITYREDACTED'
+	| 'VIEWCLUSTERMETADATA'
 	| 'VIEWCLUSTERSETTING'
+	| 'VIEWDEBUG'
 	| 'VISIBLE'
 	| 'VOLATILE'
 	| 'VOTERS'

pkg/server/admin.go

@@ -1515,14 +1515,17 @@ func (s *adminServer) RangeLog(
 	ctx = s.server.AnnotateCtx(ctx)
 
 	// Range keys, even when pretty-printed, contain PII.
-	userName, err := s.requireAdminUser(ctx)
+	user, _, err := s.getUserAndRole(ctx)
+	if err != nil {
+		return nil, err
+	}
+
+	err = s.requireViewClusterMetadataPermission(ctx)
 	if err != nil {
-		// NB: not using serverError() here since the priv checker
-		// already returns a proper gRPC error status.
 		return nil, err
 	}
 
-	r, err := s.rangeLogHelper(ctx, req, userName)
+	r, err := s.rangeLogHelper(ctx, req, user)
 	if err != nil {
 		return nil, serverError(ctx, err)
 	}
@@ -3543,6 +3546,50 @@ func (c *adminPrivilegeChecker) requireViewActivityAndNoViewActivityRedactedPerm
 	return nil
 }
 
+// requireViewClusterMetadataPermission requires the user have admin or the VIEWCLUSTERMETADATA
+// system privilege and returns an error if the user does not have it.
+func (c *adminPrivilegeChecker) requireViewClusterMetadataPermission(
+	ctx context.Context,
+) (err error) {
+	userName, isAdmin, err := c.getUserAndRole(ctx)
+	if err != nil {
+		return serverError(ctx, err)
+	}
+	if !isAdmin {
+		if c.st.Version.IsActive(ctx, clusterversion.SystemPrivilegesTable) {
+			if hasViewClusterMetadata := c.checkHasSystemPrivilege(ctx, userName, privilege.VIEWCLUSTERMETADATA); !hasViewClusterMetadata {
+				return status.Errorf(
+					codes.PermissionDenied, "this operation requires the %s system privilege",
+					privilege.VIEWCLUSTERMETADATA)
+			}
+		} else {
+			return status.Error(codes.PermissionDenied, "this operation requires admin privilege")
+		}
+	}
+	return nil
+}
+
+// requireViewDebugPermission requires the user have admin or the VIEWDEBUG system privilege
+// and returns an error if the user does not have it.
+func (c *adminPrivilegeChecker) requireViewDebugPermission(ctx context.Context) (err error) {
+	userName, isAdmin, err := c.getUserAndRole(ctx)
+	if err != nil {
+		return serverError(ctx, err)
+	}
+	if !isAdmin {
+		if c.st.Version.IsActive(ctx, clusterversion.SystemPrivilegesTable) {
+			if hasViewDebug := c.checkHasSystemPrivilege(ctx, userName, privilege.VIEWDEBUG); !hasViewDebug {
+				return status.Errorf(
+					codes.PermissionDenied, "this operation requires the %s system privilege",
+					privilege.VIEWDEBUG)
+			}
+		} else {
+			return status.Error(codes.PermissionDenied, "this operation requires admin privilege")
+		}
+	}
+	return nil
+}
+
 // Note that the function returns plain errors, and it is the caller's
 // responsibility to convert them to serverErrors.
 func (c *adminPrivilegeChecker) getUserAndRole(
@@ -3638,7 +3685,7 @@ func (s *adminServer) ListTracingSnapshots(
 	ctx context.Context, req *serverpb.ListTracingSnapshotsRequest,
 ) (*serverpb.ListTracingSnapshotsResponse, error) {
 	ctx = s.server.AnnotateCtx(ctx)
-	_, err := s.requireAdminUser(ctx)
+	err := s.requireViewDebugPermission(ctx)
 	if err != nil {
 		return nil, err
 	}
@@ -3665,7 +3712,7 @@ func (s *adminServer) TakeTracingSnapshot(
 	ctx context.Context, req *serverpb.TakeTracingSnapshotRequest,
 ) (*serverpb.TakeTracingSnapshotResponse, error) {
 	ctx = s.server.AnnotateCtx(ctx)
-	_, err := s.requireAdminUser(ctx)
+	err := s.requireViewDebugPermission(ctx)
 	if err != nil {
 		return nil, err
 	}
@@ -3709,7 +3756,7 @@ func (s *adminServer) GetTracingSnapshot(
 	ctx context.Context, req *serverpb.GetTracingSnapshotRequest,
 ) (*serverpb.GetTracingSnapshotResponse, error) {
 	ctx = s.server.AnnotateCtx(ctx)
-	_, err := s.requireAdminUser(ctx)
+	err := s.requireViewDebugPermission(ctx)
 	if err != nil {
 		return nil, err
 	}
@@ -3768,7 +3815,7 @@ func (s *adminServer) GetTrace(
 	ctx context.Context, req *serverpb.GetTraceRequest,
 ) (*serverpb.GetTraceResponse, error) {
 	ctx = s.server.AnnotateCtx(ctx)
-	_, err := s.requireAdminUser(ctx)
+	err := s.requireViewDebugPermission(ctx)
 	if err != nil {
 		return nil, err
 	}

pkg/server/admin_test.go

@@ -2834,6 +2834,20 @@ func TestAdminPrivilegeChecker(t *testing.T) {
 				withAdmin: false, withVa: false, withVaRedacted: true, withVaAndRedacted: true, withoutPrivs: true,
 			},
 		},
+		{
+			"requireViewClusterMetadataPermission",
+			underTest.requireViewClusterMetadataPermission,
+			map[username.SQLUsername]bool{
+				withAdmin: false, withoutPrivs: true,
+			},
+		},
+		{
+			"requireViewDebugPermission",
+			underTest.requireViewDebugPermission,
+			map[username.SQLUsername]bool{
+				withAdmin: false, withoutPrivs: true,
+			},
+		},
 	}
 	// test system privileges if valid version
 	if s.ClusterSettings().Version.IsActive(ctx, clusterversion.SystemPrivilegesTable) {
@@ -2844,10 +2858,16 @@ func TestAdminPrivilegeChecker(t *testing.T) {
 		sqlDB.Exec(t, "CREATE USER withvaandredactedsystemprivilege")
 		sqlDB.Exec(t, "GRANT SYSTEM VIEWACTIVITY TO withvaandredactedsystemprivilege")
 		sqlDB.Exec(t, "GRANT SYSTEM VIEWACTIVITYREDACTED TO withvaandredactedsystemprivilege")
+		sqlDB.Exec(t, "CREATE USER withviewclustermetadata")
+		sqlDB.Exec(t, "GRANT SYSTEM VIEWCLUSTERMETADATA TO withviewclustermetadata")
+		sqlDB.Exec(t, "CREATE USER withviewdebug")
+		sqlDB.Exec(t, "GRANT SYSTEM VIEWDEBUG TO withviewdebug")
 
 		withVaSystemPrivilege := username.MakeSQLUsernameFromPreNormalizedString("withvasystemprivilege")
 		withVaRedactedSystemPrivilege := username.MakeSQLUsernameFromPreNormalizedString("withvaredactedsystemprivilege")
 		withVaAndRedactedSystemPrivilege := username.MakeSQLUsernameFromPreNormalizedString("withvaandredactedsystemprivilege")
+		withviewclustermetadata := username.MakeSQLUsernameFromPreNormalizedString("withviewclustermetadata")
+		withViewDebug := username.MakeSQLUsernameFromPreNormalizedString("withviewdebug")
 
 		tests[0].usernameWantErr[withVaSystemPrivilege] = false
 		tests[1].usernameWantErr[withVaSystemPrivilege] = false
@@ -2858,6 +2878,8 @@ func TestAdminPrivilegeChecker(t *testing.T) {
 		tests[0].usernameWantErr[withVaAndRedactedSystemPrivilege] = false
 		tests[1].usernameWantErr[withVaAndRedactedSystemPrivilege] = false
 		tests[2].usernameWantErr[withVaAndRedactedSystemPrivilege] = true
+		tests[3].usernameWantErr[withviewclustermetadata] = false
+		tests[4].usernameWantErr[withViewDebug] = false
 
 	}
 	for _, tt := range tests {

pkg/server/server_http.go

@@ -185,13 +185,9 @@ func makeAdminAuthzCheckHandler(
 		md := forwardAuthenticationMetadata(req.Context(), req)
 		authCtx := metadata.NewIncomingContext(req.Context(), md)
 		// Check the privileges of the requester.
-		_, err := adminAuthzCheck.requireAdminUser(authCtx)
-		if errors.Is(err, errRequiresAdmin) {
-			http.Error(w, "admin privilege required", http.StatusUnauthorized)
-			return
-		} else if err != nil {
-			log.Ops.Infof(authCtx, "web session error: %s", err)
-			http.Error(w, "error checking authentication", http.StatusInternalServerError)
+		err := adminAuthzCheck.requireViewDebugPermission(authCtx)
+		if err != nil {
+			http.Error(w, "admin privilege or VIEWDEBUG system privilege required", http.StatusUnauthorized)
 			return
 		}
 		// Forward the request to the inner handler.

pkg/server/status.go

@@ -800,9 +800,8 @@ func (s *statusServer) AllocatorRange(
 	ctx = propagateGatewayMetadata(ctx)
 	ctx = s.AnnotateCtx(ctx)
 
-	if _, err := s.privilegeChecker.requireAdminUser(ctx); err != nil {
-		// NB: not using serverError() here since the priv checker
-		// already returns a proper gRPC error status.
+	err := s.privilegeChecker.requireViewClusterMetadataPermission(ctx)
+	if err != nil {
 		return nil, err
 	}
 
@@ -1453,7 +1452,7 @@ func (s *statusServer) Nodes(
 	ctx = propagateGatewayMetadata(ctx)
 	ctx = s.AnnotateCtx(ctx)
 
-	err := s.privilegeChecker.requireViewActivityPermission(ctx)
+	err := s.privilegeChecker.requireViewClusterMetadataPermission(ctx)
 	if err != nil {
 		return nil, err
 	}
@@ -1471,14 +1470,14 @@ func (s *statusServer) NodesUI(
 	ctx = propagateGatewayMetadata(ctx)
 	ctx = s.AnnotateCtx(ctx)
 
-	hasViewActivity := false
-	err := s.privilegeChecker.requireViewActivityPermission(ctx)
+	hasViewClusterMetadata := false
+	err := s.privilegeChecker.requireViewClusterMetadataPermission(ctx)
 	if err != nil {
 		if !grpcutil.IsAuthError(err) {
 			return nil, err
 		}
 	} else {
-		hasViewActivity = true
+		hasViewClusterMetadata = true
 	}
 
 	internalResp, _, err := s.nodesHelper(ctx, 0 /* limit */, 0 /* offset */)
@@ -1490,13 +1489,13 @@ func (s *statusServer) NodesUI(
 		LivenessByNodeID: internalResp.LivenessByNodeID,
 	}
 	for i, nodeStatus := range internalResp.Nodes {
-		resp.Nodes[i] = nodeStatusToResp(&nodeStatus, hasViewActivity)
+		resp.Nodes[i] = nodeStatusToResp(&nodeStatus, hasViewClusterMetadata)
 	}
 
 	return resp, nil
 }
 
-func nodeStatusToResp(n *statuspb.NodeStatus, hasViewActivity bool) serverpb.NodeResponse {
+func nodeStatusToResp(n *statuspb.NodeStatus, hasViewClusterMetadata bool) serverpb.NodeResponse {
 	tiers := make([]serverpb.Tier, len(n.Desc.Locality.Tiers))
 	for j, t := range n.Desc.Locality.Tiers {
 		tiers[j] = serverpb.Tier{
@@ -1552,7 +1551,7 @@ func nodeStatusToResp(n *statuspb.NodeStatus, hasViewActivity bool) serverpb.Nod
 			sfsprops := &roachpb.FileStoreProperties{
 				FsType: fsprops.FsType,
 			}
-			if hasViewActivity {
+			if hasViewClusterMetadata {
 				sfsprops.Path = fsprops.Path
 				sfsprops.BlockDevice = fsprops.BlockDevice
 				sfsprops.MountPoint = fsprops.MountPoint
@@ -1577,7 +1576,7 @@ func nodeStatusToResp(n *statuspb.NodeStatus, hasViewActivity bool) serverpb.Nod
 		NumCpus:           n.NumCpus,
 	}
 
-	if hasViewActivity {
+	if hasViewClusterMetadata {
 		resp.Args = n.Args
 		resp.Env = n.Env
 		resp.Desc.Attrs = n.Desc.Attrs
@@ -1916,7 +1915,8 @@ func (s *statusServer) rangesHelper(
 	ctx = propagateGatewayMetadata(ctx)
 	ctx = s.AnnotateCtx(ctx)
 
-	if _, err := s.privilegeChecker.requireAdminUser(ctx); err != nil {
+	err := s.privilegeChecker.requireViewClusterMetadataPermission(ctx)
+	if err != nil {
 		return nil, 0, err
 	}
 

pkg/sql/parser/sql.y

@@ -932,8 +932,8 @@ func (u *sqlSymUnion) functionObjs() tree.FuncObjs {
 %token <str> UNBOUNDED UNCOMMITTED UNION UNIQUE UNKNOWN UNLOGGED UNSPLIT
 %token <str> UPDATE UPSERT UNSET UNTIL USE USER USERS USING UUID
 
-%token <str> VALID VALIDATE VALUE VALUES VARBIT VARCHAR VARIADIC VIEW VARYING VIEWACTIVITY VIEWACTIVITYREDACTED
-%token <str> VIEWCLUSTERSETTING VIRTUAL VISIBLE VOLATILE VOTERS
+%token <str> VALID VALIDATE VALUE VALUES VARBIT VARCHAR VARIADIC VIEW VARYING VIEWACTIVITY VIEWACTIVITYREDACTED VIEWDEBUG
+%token <str> VIEWCLUSTERMETADATA VIEWCLUSTERSETTING VIRTUAL VISIBLE VOLATILE VOTERS
 
 %token <str> WHEN WHERE WINDOW WITH WITHIN WITHOUT WORK WRITE
 
@@ -15198,7 +15198,9 @@ unreserved_keyword:
 | VIEW
 | VIEWACTIVITY
 | VIEWACTIVITYREDACTED
+| VIEWCLUSTERMETADATA
 | VIEWCLUSTERSETTING
+| VIEWDEBUG
 | VISIBLE
 | VOLATILE
 | VOTERS

pkg/sql/privilege/privilege.go

@@ -53,6 +53,8 @@ const (
 	CANCELQUERY          Kind = 18
 	NOSQLLOGIN           Kind = 19
 	EXECUTE              Kind = 20
+	VIEWCLUSTERMETADATA  Kind = 21
+	VIEWDEBUG            Kind = 22
 )
 
 // Privilege represents a privilege parsed from an Access Privilege Inquiry
@@ -120,7 +122,7 @@ var (
 	// certain privileges unavailable after upgrade migration.
 	// Note that "CREATE, INSERT, DELETE, ZONECONFIG" are no-op privileges on sequences.
 	SequencePrivileges           = List{ALL, USAGE, SELECT, UPDATE, CREATE, DROP, INSERT, DELETE, ZONECONFIG}
-	SystemPrivileges             = List{ALL, MODIFYCLUSTERSETTING, EXTERNALCONNECTION, VIEWACTIVITY, VIEWACTIVITYREDACTED, VIEWCLUSTERSETTING, CANCELQUERY, NOSQLLOGIN}
+	SystemPrivileges             = List{ALL, MODIFYCLUSTERSETTING, EXTERNALCONNECTION, VIEWACTIVITY, VIEWACTIVITYREDACTED, VIEWCLUSTERSETTING, CANCELQUERY, NOSQLLOGIN, VIEWCLUSTERMETADATA, VIEWDEBUG}
 	VirtualTablePrivileges       = List{ALL, SELECT}
 	ExternalConnectionPrivileges = List{ALL, USAGE}
 )
@@ -137,8 +139,7 @@ func (k Kind) IsSetIn(bits uint32) bool {
 
 // ByValue is just an array of privilege kinds sorted by value.
 var ByValue = [...]Kind{
-	ALL, CREATE, DROP, SELECT, INSERT, DELETE, UPDATE, USAGE, ZONECONFIG, CONNECT, RULE, MODIFYCLUSTERSETTING,
-	EXTERNALCONNECTION, VIEWACTIVITY, VIEWACTIVITYREDACTED, VIEWCLUSTERSETTING, CANCELQUERY, NOSQLLOGIN, EXECUTE,
+	ALL, CREATE, DROP, SELECT, INSERT, DELETE, UPDATE, USAGE, ZONECONFIG, CONNECT, RULE, MODIFYCLUSTERSETTING, EXTERNALCONNECTION, VIEWACTIVITY, VIEWACTIVITYREDACTED, VIEWCLUSTERSETTING, CANCELQUERY, NOSQLLOGIN, EXECUTE, VIEWCLUSTERMETADATA, VIEWDEBUG,
 }
 
 // ByName is a map of string -> kind value.
@@ -162,6 +163,8 @@ var ByName = map[string]Kind{
 	"CANCELQUERY":          CANCELQUERY,
 	"NOSQLLOGIN":           NOSQLLOGIN,
 	"EXECUTE":              EXECUTE,
+	"VIEWCLUSTERMETADATA":  VIEWCLUSTERMETADATA,
+	"VIEWDEBUG":            VIEWDEBUG,
 }
 
 // List is a list of privileges.