From 10da8d931ceffbaed50be2b5e7b90fcce6fe52be Mon Sep 17 00:00:00 2001 From: Weiming Xu Date: Thu, 21 May 2026 11:20:42 -0700 Subject: [PATCH 01/15] Add GNMI audit logging for Get() and Set() Signed-off-by: Weiming --- gnmi_server/server.go | 48 ++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 45 insertions(+), 3 deletions(-) diff --git a/gnmi_server/server.go b/gnmi_server/server.go index 55af5efc1..e0c545ebf 100644 --- a/gnmi_server/server.go +++ b/gnmi_server/server.go @@ -950,7 +950,28 @@ func IsNativeOrigin(origin string) bool { } // Get implements the Get RPC in gNMI spec. -func (s *Server) Get(ctx context.Context, req *gnmipb.GetRequest) (*gnmipb.GetResponse, error) { +func (s *Server) Get(ctx context.Context, req *gnmipb.GetRequest) (resp *gnmipb.GetResponse, err error) { + // GNMI-AUDIT logging + start := time.Now() + user := extractUser(ctx) + peer := extractPeer(ctx) + + log.Infof("[GNMI-AUDIT] GetRequest user=%s peer=%s prefix=%v paths=%v type=%v", + user, peer, req.GetPrefix(), req.GetPath(), req.GetType()) + + // defer logs automatically when function returns + defer func() { + duration := time.Since(start) + + if err != nil { + log.Errorf("[GNMI-AUDIT] GetResponse user=%s peer=%s status=FAIL err=%v duration=%v", + user, peer, err, duration) + } else { + log.Infof("[GNMI-AUDIT] GetResponse user=%s peer=%s status=OK duration=%v", + user, peer, duration) + } + }() + common_utils.IncCounter(common_utils.GNMI_GET) if req.GetType() != gnmipb.GetRequest_ALL { @@ -1091,8 +1112,19 @@ func SaveOnSetEnabled() error { func saveOnSetDisabled() error { return nil } func (s *Server) Set(ctx context.Context, req *gnmipb.SetRequest) (*gnmipb.SetResponse, error) { + // GNMI-AUDIT logging + start := time.Now() + user := extractUser(ctx) // from auth metadata + peer := extractPeer(ctx) // client address + + log.Infof("[GNMI-AUDIT] SetRequest user=%s peer=%s prefix=%v updates=%d replaces=%d deletes=%d", + user, peer, req.GetPrefix(), len(req.GetUpdate()), len(req.GetReplace()), len(req.GetDelete())) + e := s.ReqFromMaster(req, &s.masterEID) if e != nil { + duration := time.Since(start) + log.Errorf("[GNMI-AUDIT] SetResponse user=%s peer=%s status=FAIL err=%v duration=%v", + user, peer, e, duration) return nil, e } @@ -1232,11 +1264,21 @@ func (s *Server) Set(ctx context.Context, req *gnmipb.SetRequest) (*gnmipb.SetRe s.SaveStartupConfig() } - return &gnmipb.SetResponse{ + resp := &gnmipb.SetResponse{ Prefix: req.GetPrefix(), Response: results, - }, err + } + + duration := time.Since(start) + if err != nil { + log.Errorf("[GNMI-AUDIT] SetResponse user=%s peer=%s status=FAIL err=%v duration=%v", + user, peer, err, duration) + } else { + log.Infof("[GNMI-AUDIT] SetResponse user=%s peer=%s status=OK duration=%v", + user, peer, duration) + } + return resp, err } func (s *Server) Capabilities(ctx context.Context, req *gnmipb.CapabilityRequest) (*gnmipb.CapabilityResponse, error) { From 813464cfcb6410e76d44166025a4add070396f7a Mon Sep 17 00:00:00 2001 From: Weiming Xu Date: Mon, 29 Jun 2026 11:55:40 -0700 Subject: [PATCH 02/15] [gnmi_server]: fix Get/Set audit logging and err scoping Signed-off-by: Weiming Xu Signed-off-by: Weiming --- gnmi_server/server.go | 69 +++++++++++++++++++++---------------------- 1 file changed, 33 insertions(+), 36 deletions(-) diff --git a/gnmi_server/server.go b/gnmi_server/server.go index e0c545ebf..3a6c48734 100644 --- a/gnmi_server/server.go +++ b/gnmi_server/server.go @@ -981,14 +981,14 @@ func (s *Server) Get(ctx context.Context, req *gnmipb.GetRequest) (resp *gnmipb. // gNMI path based authorization if s.config.PathzPolicy && len(req.GetPath()) != 0 { newPaths := []*gnmipb.Path{} - user, err := getUsername(ctx) - if err != nil { - log.V(1).Infof("GetRequest User not found: %s", err.Error()) - return nil, err + pathzUser, userErr := getUsername(ctx) + if userErr != nil { + log.V(1).Infof("GetRequest User not found: %s", userErr.Error()) + return nil, userErr } for _, path := range req.GetPath() { // Only process the authorized paths in the request. - s.gnsiPathz.pathzProcessor.AuthorizeWithPrefix(user, req.GetPrefix(), path, gnsi_pathz_pb.Mode_MODE_READ) + s.gnsiPathz.pathzProcessor.AuthorizeWithPrefix(pathzUser, req.GetPrefix(), path, gnsi_pathz_pb.Mode_MODE_READ) } if len(newPaths) == 0 { return nil, status.Error(codes.PermissionDenied, "Unauthorized request. Rejected by pathz policy.") @@ -996,7 +996,8 @@ func (s *Server) Get(ctx context.Context, req *gnmipb.GetRequest) (resp *gnmipb. req.Path = newPaths } - if err := s.checkEncodingAndModel(req.GetEncoding(), req.GetUseModels()); err != nil { + err = s.checkEncodingAndModel(req.GetEncoding(), req.GetUseModels()) + if err != nil { common_utils.IncCounter(common_utils.GNMI_GET_FAIL) return nil, status.Error(codes.Unimplemented, err.Error()) } @@ -1015,7 +1016,6 @@ func (s *Server) Get(ctx context.Context, req *gnmipb.GetRequest) (resp *gnmipb. log.V(2).Infof("GetRequest paths: %v", paths) var dc sdc.Client - var err error // Handle OPERATIONAL target directly without SONiC routing if target == "OPERATIONAL" { return s.handleOperationalGet(ctx, req, paths, prefix) @@ -1111,7 +1111,7 @@ func SaveOnSetEnabled() error { // SaveOnSetDisabeld does nothing. func saveOnSetDisabled() error { return nil } -func (s *Server) Set(ctx context.Context, req *gnmipb.SetRequest) (*gnmipb.SetResponse, error) { +func (s *Server) Set(ctx context.Context, req *gnmipb.SetRequest) (resp *gnmipb.SetResponse, err error) { // GNMI-AUDIT logging start := time.Now() user := extractUser(ctx) // from auth metadata @@ -1120,12 +1120,20 @@ func (s *Server) Set(ctx context.Context, req *gnmipb.SetRequest) (*gnmipb.SetRe log.Infof("[GNMI-AUDIT] SetRequest user=%s peer=%s prefix=%v updates=%d replaces=%d deletes=%d", user, peer, req.GetPrefix(), len(req.GetUpdate()), len(req.GetReplace()), len(req.GetDelete())) - e := s.ReqFromMaster(req, &s.masterEID) - if e != nil { + defer func() { duration := time.Since(start) - log.Errorf("[GNMI-AUDIT] SetResponse user=%s peer=%s status=FAIL err=%v duration=%v", - user, peer, e, duration) - return nil, e + if err != nil { + log.Errorf("[GNMI-AUDIT] SetResponse user=%s peer=%s status=FAIL err=%v duration=%v", + user, peer, err, duration) + } else { + log.Infof("[GNMI-AUDIT] SetResponse user=%s peer=%s status=OK duration=%v", + user, peer, duration) + } + }() + + err = s.ReqFromMaster(req, &s.masterEID) + if err != nil { + return nil, err } common_utils.IncCounter(common_utils.GNMI_SET) @@ -1135,20 +1143,20 @@ func (s *Server) Set(ctx context.Context, req *gnmipb.SetRequest) (*gnmipb.SetRe } // gNMI path based authorization if s.config.PathzPolicy { - user, err := getUsername(ctx) - if err != nil { - log.V(1).Infof("SetRequest User not found: %s", err.Error()) - return nil, err + pathzUser, userErr := getUsername(ctx) + if userErr != nil { + log.V(1).Infof("SetRequest User not found: %s", userErr.Error()) + return nil, userErr } permitted := true for _, path := range req.GetDelete() { - s.gnsiPathz.pathzProcessor.AuthorizeWithPrefix(user, req.GetPrefix(), path, gnsi_pathz_pb.Mode_MODE_WRITE) + s.gnsiPathz.pathzProcessor.AuthorizeWithPrefix(pathzUser, req.GetPrefix(), path, gnsi_pathz_pb.Mode_MODE_WRITE) } for _, update := range req.GetReplace() { - s.gnsiPathz.pathzProcessor.AuthorizeWithPrefix(user, req.GetPrefix(), update.GetPath(), gnsi_pathz_pb.Mode_MODE_WRITE) + s.gnsiPathz.pathzProcessor.AuthorizeWithPrefix(pathzUser, req.GetPrefix(), update.GetPath(), gnsi_pathz_pb.Mode_MODE_WRITE) } for _, update := range req.GetUpdate() { - s.gnsiPathz.pathzProcessor.AuthorizeWithPrefix(user, req.GetPrefix(), update.GetPath(), gnsi_pathz_pb.Mode_MODE_WRITE) + s.gnsiPathz.pathzProcessor.AuthorizeWithPrefix(pathzUser, req.GetPrefix(), update.GetPath(), gnsi_pathz_pb.Mode_MODE_WRITE) } if !permitted { return nil, status.Error(codes.PermissionDenied, "Unauthorized request. Rejected by pathz policy.") @@ -1166,7 +1174,6 @@ func (s *Server) Set(ctx context.Context, req *gnmipb.SetRequest) (*gnmipb.SetRe encoding := gnmipb.Encoding_JSON_IETF var dc sdc.Client - var err error paths := req.GetDelete() for _, path := range req.GetReplace() { paths = append(paths, path.GetPath()) @@ -1189,13 +1196,13 @@ func (s *Server) Set(ctx context.Context, req *gnmipb.SetRequest) (*gnmipb.SetRe // Fast path: bypass validation for allowed tables/SKUs allUpdates := append(req.GetReplace(), req.GetUpdate()...) - if resp, used, err := bypass.TrySet(ctx, prefix, req.GetDelete(), allUpdates); used { - if err != nil { + if bypassResp, used, bypassErr := bypass.TrySet(ctx, prefix, req.GetDelete(), allUpdates); used { + if bypassErr != nil { common_utils.IncCounter(common_utils.GNMI_SET_FAIL) - return nil, status.Error(codes.Internal, err.Error()) + return nil, status.Error(codes.Internal, bypassErr.Error()) } common_utils.IncCounter(common_utils.GNMI_SET_BYPASS) - return resp, nil + return bypassResp, nil } var targetDbName string @@ -1264,20 +1271,10 @@ func (s *Server) Set(ctx context.Context, req *gnmipb.SetRequest) (*gnmipb.SetRe s.SaveStartupConfig() } - resp := &gnmipb.SetResponse{ + resp = &gnmipb.SetResponse{ Prefix: req.GetPrefix(), Response: results, } - - duration := time.Since(start) - - if err != nil { - log.Errorf("[GNMI-AUDIT] SetResponse user=%s peer=%s status=FAIL err=%v duration=%v", - user, peer, err, duration) - } else { - log.Infof("[GNMI-AUDIT] SetResponse user=%s peer=%s status=OK duration=%v", - user, peer, duration) - } return resp, err } From 6230aa93721d71b54b7ea81f727102861d76e25d Mon Sep 17 00:00:00 2001 From: Weiming Xu Date: Mon, 29 Jun 2026 16:03:10 -0700 Subject: [PATCH 03/15] [gnmi_server]: add extractUser/extractPeer helpers for audit logs Signed-off-by: Weiming Xu Signed-off-by: Weiming --- gnmi_server/server.go | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/gnmi_server/server.go b/gnmi_server/server.go index 3a6c48734..4d15be498 100644 --- a/gnmi_server/server.go +++ b/gnmi_server/server.go @@ -53,6 +53,7 @@ import ( "google.golang.org/grpc/authz" "google.golang.org/grpc/codes" "google.golang.org/grpc/credentials/tls/certprovider" + "google.golang.org/grpc/metadata" "google.golang.org/grpc/peer" "google.golang.org/grpc/reflection" "google.golang.org/grpc/security/advancedtls" @@ -1321,6 +1322,35 @@ func (s *Server) Capabilities(ctx context.Context, req *gnmipb.CapabilityRequest Extension: exts}, nil } +func extractUser(ctx context.Context) string { + rc, _ := common_utils.GetContext(ctx) + if rc != nil && rc.Auth.User != "" { + return rc.Auth.User + } + + if md, ok := metadata.FromIncomingContext(ctx); ok { + for _, key := range []string{"username", "user", "x-remote-user"} { + if values := md.Get(key); len(values) > 0 && values[0] != "" { + return values[0] + } + } + } + + if username, err := getUsername(ctx); err == nil && username != "" { + return username + } + + return "unknown" +} + +func extractPeer(ctx context.Context) string { + if pr, ok := peer.FromContext(ctx); ok && pr.Addr != nil { + return pr.Addr.String() + } + + return "unknown" +} + // Obtain the user name as the last element of the SPIFFE ID. func getUsername(ctx context.Context) (string, error) { pr, ok := peer.FromContext(ctx) From f77d273546aa21c5e8aee31dbe14f0461a226ffb Mon Sep 17 00:00:00 2001 From: Weiming Xu Date: Mon, 29 Jun 2026 16:42:45 -0700 Subject: [PATCH 04/15] [gnmi_server]: keep existing user/err names and scope audit vars Signed-off-by: Weiming Xu Signed-off-by: Weiming --- gnmi_server/server.go | 44 +++++++++++++++++++++---------------------- 1 file changed, 22 insertions(+), 22 deletions(-) diff --git a/gnmi_server/server.go b/gnmi_server/server.go index 4d15be498..a3ab9d928 100644 --- a/gnmi_server/server.go +++ b/gnmi_server/server.go @@ -954,11 +954,11 @@ func IsNativeOrigin(origin string) bool { func (s *Server) Get(ctx context.Context, req *gnmipb.GetRequest) (resp *gnmipb.GetResponse, err error) { // GNMI-AUDIT logging start := time.Now() - user := extractUser(ctx) - peer := extractPeer(ctx) + auditUser := extractUser(ctx) + auditPeer := extractPeer(ctx) log.Infof("[GNMI-AUDIT] GetRequest user=%s peer=%s prefix=%v paths=%v type=%v", - user, peer, req.GetPrefix(), req.GetPath(), req.GetType()) + auditUser, auditPeer, req.GetPrefix(), req.GetPath(), req.GetType()) // defer logs automatically when function returns defer func() { @@ -966,10 +966,10 @@ func (s *Server) Get(ctx context.Context, req *gnmipb.GetRequest) (resp *gnmipb. if err != nil { log.Errorf("[GNMI-AUDIT] GetResponse user=%s peer=%s status=FAIL err=%v duration=%v", - user, peer, err, duration) + auditUser, auditPeer, err, duration) } else { log.Infof("[GNMI-AUDIT] GetResponse user=%s peer=%s status=OK duration=%v", - user, peer, duration) + auditUser, auditPeer, duration) } }() @@ -982,14 +982,14 @@ func (s *Server) Get(ctx context.Context, req *gnmipb.GetRequest) (resp *gnmipb. // gNMI path based authorization if s.config.PathzPolicy && len(req.GetPath()) != 0 { newPaths := []*gnmipb.Path{} - pathzUser, userErr := getUsername(ctx) - if userErr != nil { - log.V(1).Infof("GetRequest User not found: %s", userErr.Error()) - return nil, userErr + user, err := getUsername(ctx) + if err != nil { + log.V(1).Infof("GetRequest User not found: %s", err.Error()) + return nil, err } for _, path := range req.GetPath() { // Only process the authorized paths in the request. - s.gnsiPathz.pathzProcessor.AuthorizeWithPrefix(pathzUser, req.GetPrefix(), path, gnsi_pathz_pb.Mode_MODE_READ) + s.gnsiPathz.pathzProcessor.AuthorizeWithPrefix(user, req.GetPrefix(), path, gnsi_pathz_pb.Mode_MODE_READ) } if len(newPaths) == 0 { return nil, status.Error(codes.PermissionDenied, "Unauthorized request. Rejected by pathz policy.") @@ -1115,20 +1115,20 @@ func saveOnSetDisabled() error { return nil } func (s *Server) Set(ctx context.Context, req *gnmipb.SetRequest) (resp *gnmipb.SetResponse, err error) { // GNMI-AUDIT logging start := time.Now() - user := extractUser(ctx) // from auth metadata - peer := extractPeer(ctx) // client address + auditUser := extractUser(ctx) // from auth metadata + auditPeer := extractPeer(ctx) // client address log.Infof("[GNMI-AUDIT] SetRequest user=%s peer=%s prefix=%v updates=%d replaces=%d deletes=%d", - user, peer, req.GetPrefix(), len(req.GetUpdate()), len(req.GetReplace()), len(req.GetDelete())) + auditUser, auditPeer, req.GetPrefix(), len(req.GetUpdate()), len(req.GetReplace()), len(req.GetDelete())) defer func() { duration := time.Since(start) if err != nil { log.Errorf("[GNMI-AUDIT] SetResponse user=%s peer=%s status=FAIL err=%v duration=%v", - user, peer, err, duration) + auditUser, auditPeer, err, duration) } else { log.Infof("[GNMI-AUDIT] SetResponse user=%s peer=%s status=OK duration=%v", - user, peer, duration) + auditUser, auditPeer, duration) } }() @@ -1144,20 +1144,20 @@ func (s *Server) Set(ctx context.Context, req *gnmipb.SetRequest) (resp *gnmipb. } // gNMI path based authorization if s.config.PathzPolicy { - pathzUser, userErr := getUsername(ctx) - if userErr != nil { - log.V(1).Infof("SetRequest User not found: %s", userErr.Error()) - return nil, userErr + user, err := getUsername(ctx) + if err != nil { + log.V(1).Infof("SetRequest User not found: %s", err.Error()) + return nil, err } permitted := true for _, path := range req.GetDelete() { - s.gnsiPathz.pathzProcessor.AuthorizeWithPrefix(pathzUser, req.GetPrefix(), path, gnsi_pathz_pb.Mode_MODE_WRITE) + s.gnsiPathz.pathzProcessor.AuthorizeWithPrefix(user, req.GetPrefix(), path, gnsi_pathz_pb.Mode_MODE_WRITE) } for _, update := range req.GetReplace() { - s.gnsiPathz.pathzProcessor.AuthorizeWithPrefix(pathzUser, req.GetPrefix(), update.GetPath(), gnsi_pathz_pb.Mode_MODE_WRITE) + s.gnsiPathz.pathzProcessor.AuthorizeWithPrefix(user, req.GetPrefix(), update.GetPath(), gnsi_pathz_pb.Mode_MODE_WRITE) } for _, update := range req.GetUpdate() { - s.gnsiPathz.pathzProcessor.AuthorizeWithPrefix(pathzUser, req.GetPrefix(), update.GetPath(), gnsi_pathz_pb.Mode_MODE_WRITE) + s.gnsiPathz.pathzProcessor.AuthorizeWithPrefix(user, req.GetPrefix(), update.GetPath(), gnsi_pathz_pb.Mode_MODE_WRITE) } if !permitted { return nil, status.Error(codes.PermissionDenied, "Unauthorized request. Rejected by pathz policy.") From 83bb47f0acdad813a3f9ec7f0fea8b4646c6d0cb Mon Sep 17 00:00:00 2001 From: Weiming Xu Date: Mon, 29 Jun 2026 16:48:03 -0700 Subject: [PATCH 05/15] [gnmi_server]: restore Set pathz variable naming block Signed-off-by: Weiming Xu Signed-off-by: Weiming --- gnmi_server/server.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/gnmi_server/server.go b/gnmi_server/server.go index a3ab9d928..920d8f8b5 100644 --- a/gnmi_server/server.go +++ b/gnmi_server/server.go @@ -1144,20 +1144,20 @@ func (s *Server) Set(ctx context.Context, req *gnmipb.SetRequest) (resp *gnmipb. } // gNMI path based authorization if s.config.PathzPolicy { - user, err := getUsername(ctx) - if err != nil { - log.V(1).Infof("SetRequest User not found: %s", err.Error()) - return nil, err + pathzUser, userErr := getUsername(ctx) + if userErr != nil { + log.V(1).Infof("SetRequest User not found: %s", userErr.Error()) + return nil, userErr } permitted := true for _, path := range req.GetDelete() { - s.gnsiPathz.pathzProcessor.AuthorizeWithPrefix(user, req.GetPrefix(), path, gnsi_pathz_pb.Mode_MODE_WRITE) + s.gnsiPathz.pathzProcessor.AuthorizeWithPrefix(pathzUser, req.GetPrefix(), path, gnsi_pathz_pb.Mode_MODE_WRITE) } for _, update := range req.GetReplace() { - s.gnsiPathz.pathzProcessor.AuthorizeWithPrefix(user, req.GetPrefix(), update.GetPath(), gnsi_pathz_pb.Mode_MODE_WRITE) + s.gnsiPathz.pathzProcessor.AuthorizeWithPrefix(pathzUser, req.GetPrefix(), update.GetPath(), gnsi_pathz_pb.Mode_MODE_WRITE) } for _, update := range req.GetUpdate() { - s.gnsiPathz.pathzProcessor.AuthorizeWithPrefix(user, req.GetPrefix(), update.GetPath(), gnsi_pathz_pb.Mode_MODE_WRITE) + s.gnsiPathz.pathzProcessor.AuthorizeWithPrefix(pathzUser, req.GetPrefix(), update.GetPath(), gnsi_pathz_pb.Mode_MODE_WRITE) } if !permitted { return nil, status.Error(codes.PermissionDenied, "Unauthorized request. Rejected by pathz policy.") From cde92196259d7230ab9a111c0a7b01f53535bc8c Mon Sep 17 00:00:00 2001 From: Weiming Xu Date: Mon, 29 Jun 2026 16:54:19 -0700 Subject: [PATCH 06/15] [gnmi_server]: restore requested unchanged line ranges Signed-off-by: Weiming Xu Signed-off-by: Weiming --- gnmi_server/server.go | 30 ++++++------------------------ 1 file changed, 6 insertions(+), 24 deletions(-) diff --git a/gnmi_server/server.go b/gnmi_server/server.go index 920d8f8b5..337846136 100644 --- a/gnmi_server/server.go +++ b/gnmi_server/server.go @@ -1112,29 +1112,10 @@ func SaveOnSetEnabled() error { // SaveOnSetDisabeld does nothing. func saveOnSetDisabled() error { return nil } -func (s *Server) Set(ctx context.Context, req *gnmipb.SetRequest) (resp *gnmipb.SetResponse, err error) { - // GNMI-AUDIT logging - start := time.Now() - auditUser := extractUser(ctx) // from auth metadata - auditPeer := extractPeer(ctx) // client address - - log.Infof("[GNMI-AUDIT] SetRequest user=%s peer=%s prefix=%v updates=%d replaces=%d deletes=%d", - auditUser, auditPeer, req.GetPrefix(), len(req.GetUpdate()), len(req.GetReplace()), len(req.GetDelete())) - - defer func() { - duration := time.Since(start) - if err != nil { - log.Errorf("[GNMI-AUDIT] SetResponse user=%s peer=%s status=FAIL err=%v duration=%v", - auditUser, auditPeer, err, duration) - } else { - log.Infof("[GNMI-AUDIT] SetResponse user=%s peer=%s status=OK duration=%v", - auditUser, auditPeer, duration) - } - }() - - err = s.ReqFromMaster(req, &s.masterEID) - if err != nil { - return nil, err +func (s *Server) Set(ctx context.Context, req *gnmipb.SetRequest) (*gnmipb.SetResponse, error) { + e := s.ReqFromMaster(req, &s.masterEID) + if e != nil { + return nil, e } common_utils.IncCounter(common_utils.GNMI_SET) @@ -1175,6 +1156,7 @@ func (s *Server) Set(ctx context.Context, req *gnmipb.SetRequest) (resp *gnmipb. encoding := gnmipb.Encoding_JSON_IETF var dc sdc.Client + var err error paths := req.GetDelete() for _, path := range req.GetReplace() { paths = append(paths, path.GetPath()) @@ -1272,7 +1254,7 @@ func (s *Server) Set(ctx context.Context, req *gnmipb.SetRequest) (resp *gnmipb. s.SaveStartupConfig() } - resp = &gnmipb.SetResponse{ + resp := &gnmipb.SetResponse{ Prefix: req.GetPrefix(), Response: results, } From 74e972eaa68b925f67647931d1442ac92f41033d Mon Sep 17 00:00:00 2001 From: Weiming Xu Date: Mon, 29 Jun 2026 17:09:45 -0700 Subject: [PATCH 07/15] [gnmi_server]: rework Set audit logging with minimal logic changes Signed-off-by: Weiming Xu Signed-off-by: Weiming --- gnmi_server/server.go | 254 +++++++++++++++++++++++------------------- 1 file changed, 138 insertions(+), 116 deletions(-) diff --git a/gnmi_server/server.go b/gnmi_server/server.go index 337846136..4767b73cb 100644 --- a/gnmi_server/server.go +++ b/gnmi_server/server.go @@ -1113,151 +1113,173 @@ func SaveOnSetEnabled() error { func saveOnSetDisabled() error { return nil } func (s *Server) Set(ctx context.Context, req *gnmipb.SetRequest) (*gnmipb.SetResponse, error) { - e := s.ReqFromMaster(req, &s.masterEID) - if e != nil { - return nil, e - } + start := time.Now() + auditUser := extractUser(ctx) + auditPeer := extractPeer(ctx) - common_utils.IncCounter(common_utils.GNMI_SET) - if s.config.EnableTranslibWrite == false && s.config.EnableNativeWrite == false { - common_utils.IncCounter(common_utils.GNMI_SET_FAIL) - return nil, grpc.Errorf(codes.Unimplemented, "GNMI is in read-only mode") - } - // gNMI path based authorization - if s.config.PathzPolicy { - pathzUser, userErr := getUsername(ctx) - if userErr != nil { - log.V(1).Infof("SetRequest User not found: %s", userErr.Error()) - return nil, userErr + log.Infof("[GNMI-AUDIT] SetRequest user=%s peer=%s prefix=%v updates=%d replaces=%d deletes=%d", + auditUser, auditPeer, req.GetPrefix(), len(req.GetUpdate()), len(req.GetReplace()), len(req.GetDelete())) + + resp, err := func() (*gnmipb.SetResponse, error) { + ctx := ctx + + e := s.ReqFromMaster(req, &s.masterEID) + if e != nil { + return nil, e } - permitted := true - for _, path := range req.GetDelete() { - s.gnsiPathz.pathzProcessor.AuthorizeWithPrefix(pathzUser, req.GetPrefix(), path, gnsi_pathz_pb.Mode_MODE_WRITE) + + common_utils.IncCounter(common_utils.GNMI_SET) + if s.config.EnableTranslibWrite == false && s.config.EnableNativeWrite == false { + common_utils.IncCounter(common_utils.GNMI_SET_FAIL) + return nil, grpc.Errorf(codes.Unimplemented, "GNMI is in read-only mode") } - for _, update := range req.GetReplace() { - s.gnsiPathz.pathzProcessor.AuthorizeWithPrefix(pathzUser, req.GetPrefix(), update.GetPath(), gnsi_pathz_pb.Mode_MODE_WRITE) + // gNMI path based authorization + if s.config.PathzPolicy { + pathzUser, userErr := getUsername(ctx) + if userErr != nil { + log.V(1).Infof("SetRequest User not found: %s", userErr.Error()) + return nil, userErr + } + permitted := true + for _, path := range req.GetDelete() { + s.gnsiPathz.pathzProcessor.AuthorizeWithPrefix(pathzUser, req.GetPrefix(), path, gnsi_pathz_pb.Mode_MODE_WRITE) + } + for _, update := range req.GetReplace() { + s.gnsiPathz.pathzProcessor.AuthorizeWithPrefix(pathzUser, req.GetPrefix(), update.GetPath(), gnsi_pathz_pb.Mode_MODE_WRITE) + } + for _, update := range req.GetUpdate() { + s.gnsiPathz.pathzProcessor.AuthorizeWithPrefix(pathzUser, req.GetPrefix(), update.GetPath(), gnsi_pathz_pb.Mode_MODE_WRITE) + } + if !permitted { + return nil, status.Error(codes.PermissionDenied, "Unauthorized request. Rejected by pathz policy.") + } } - for _, update := range req.GetUpdate() { - s.gnsiPathz.pathzProcessor.AuthorizeWithPrefix(pathzUser, req.GetPrefix(), update.GetPath(), gnsi_pathz_pb.Mode_MODE_WRITE) + var results []*gnmipb.UpdateResult + + /* Fetch the prefix. */ + prefix := req.GetPrefix() + origin := "" + if prefix != nil { + origin = prefix.Origin } - if !permitted { - return nil, status.Error(codes.PermissionDenied, "Unauthorized request. Rejected by pathz policy.") + extensions := req.GetExtension() + encoding := gnmipb.Encoding_JSON_IETF + + var dc sdc.Client + var err error + paths := req.GetDelete() + for _, path := range req.GetReplace() { + paths = append(paths, path.GetPath()) } - } - var results []*gnmipb.UpdateResult - - /* Fetch the prefix. */ - prefix := req.GetPrefix() - origin := "" - if prefix != nil { - origin = prefix.Origin - } - extensions := req.GetExtension() - encoding := gnmipb.Encoding_JSON_IETF - - var dc sdc.Client - var err error - paths := req.GetDelete() - for _, path := range req.GetReplace() { - paths = append(paths, path.GetPath()) - } - for _, path := range req.GetUpdate() { - paths = append(paths, path.GetPath()) - } - if origin == "" { - origin, err = ParseOrigin(paths) - if err != nil { - return nil, err + for _, path := range req.GetUpdate() { + paths = append(paths, path.GetPath()) } - } - authTarget := "gnmi" - if check := IsNativeOrigin(origin); check { - if s.config.EnableNativeWrite == false { - common_utils.IncCounter(common_utils.GNMI_SET_FAIL) - return nil, grpc.Errorf(codes.Unimplemented, "GNMI native write is disabled") + if origin == "" { + origin, err = ParseOrigin(paths) + if err != nil { + return nil, err + } } + authTarget := "gnmi" + if check := IsNativeOrigin(origin); check { + if s.config.EnableNativeWrite == false { + common_utils.IncCounter(common_utils.GNMI_SET_FAIL) + return nil, grpc.Errorf(codes.Unimplemented, "GNMI native write is disabled") + } + + // Fast path: bypass validation for allowed tables/SKUs + allUpdates := append(req.GetReplace(), req.GetUpdate()...) + if bypassResp, used, bypassErr := bypass.TrySet(ctx, prefix, req.GetDelete(), allUpdates); used { + if bypassErr != nil { + common_utils.IncCounter(common_utils.GNMI_SET_FAIL) + return nil, status.Error(codes.Internal, bypassErr.Error()) + } + common_utils.IncCounter(common_utils.GNMI_SET_BYPASS) + return bypassResp, nil + } - // Fast path: bypass validation for allowed tables/SKUs - allUpdates := append(req.GetReplace(), req.GetUpdate()...) - if bypassResp, used, bypassErr := bypass.TrySet(ctx, prefix, req.GetDelete(), allUpdates); used { - if bypassErr != nil { + var targetDbName string + dc, err = sdc.NewMixedDbClient(paths, prefix, origin, encoding, s.config.ZmqPort, s.config.Vrf, &targetDbName) + authTarget = "gnmi_" + targetDbName + } else { + if s.config.EnableTranslibWrite == false { common_utils.IncCounter(common_utils.GNMI_SET_FAIL) - return nil, status.Error(codes.Internal, bypassErr.Error()) + return nil, grpc.Errorf(codes.Unimplemented, "Translib write is disabled") } - common_utils.IncCounter(common_utils.GNMI_SET_BYPASS) - return bypassResp, nil + /* Create Transl client. */ + dc, err = sdc.NewTranslClient(prefix, nil, ctx, extensions) } - var targetDbName string - dc, err = sdc.NewMixedDbClient(paths, prefix, origin, encoding, s.config.ZmqPort, s.config.Vrf, &targetDbName) - authTarget = "gnmi_" + targetDbName - } else { - if s.config.EnableTranslibWrite == false { + if err != nil { common_utils.IncCounter(common_utils.GNMI_SET_FAIL) - return nil, grpc.Errorf(codes.Unimplemented, "Translib write is disabled") + return nil, status.Error(codes.NotFound, err.Error()) } - /* Create Transl client. */ - dc, err = sdc.NewTranslClient(prefix, nil, ctx, extensions) - } + defer dc.Close() - if err != nil { - common_utils.IncCounter(common_utils.GNMI_SET_FAIL) - return nil, status.Error(codes.NotFound, err.Error()) - } - defer dc.Close() + ctx, err = authenticate(s.config, ctx, authTarget, true) + if err != nil { + common_utils.IncCounter(common_utils.GNMI_SET_FAIL) + return nil, err + } + /* DELETE */ + for _, path := range req.GetDelete() { + log.V(2).Infof("Delete path: %v", path) - ctx, err = authenticate(s.config, ctx, authTarget, true) - if err != nil { - common_utils.IncCounter(common_utils.GNMI_SET_FAIL) - return nil, err - } - /* DELETE */ - for _, path := range req.GetDelete() { - log.V(2).Infof("Delete path: %v", path) + res := gnmipb.UpdateResult{ + Path: path, + Op: gnmipb.UpdateResult_DELETE, + } - res := gnmipb.UpdateResult{ - Path: path, - Op: gnmipb.UpdateResult_DELETE, + /* Add to Set response results. */ + results = append(results, &res) } - /* Add to Set response results. */ - results = append(results, &res) - } - - /* REPLACE */ - for _, path := range req.GetReplace() { - log.V(2).Infof("Replace path: %v ", path) + /* REPLACE */ + for _, path := range req.GetReplace() { + log.V(2).Infof("Replace path: %v ", path) - res := gnmipb.UpdateResult{ - Path: path.GetPath(), - Op: gnmipb.UpdateResult_REPLACE, + res := gnmipb.UpdateResult{ + Path: path.GetPath(), + Op: gnmipb.UpdateResult_REPLACE, + } + /* Add to Set response results. */ + results = append(results, &res) } - /* Add to Set response results. */ - results = append(results, &res) - } - /* UPDATE */ - for _, path := range req.GetUpdate() { - log.V(2).Infof("Update path: %v ", path) + /* UPDATE */ + for _, path := range req.GetUpdate() { + log.V(2).Infof("Update path: %v ", path) + + res := gnmipb.UpdateResult{ + Path: path.GetPath(), + Op: gnmipb.UpdateResult_UPDATE, + } + /* Add to Set response results. */ + results = append(results, &res) + } + err = dc.Set(req.GetDelete(), req.GetReplace(), req.GetUpdate()) + if err != nil { + common_utils.IncCounter(common_utils.GNMI_SET_FAIL) + } else { + s.SaveStartupConfig() + } - res := gnmipb.UpdateResult{ - Path: path.GetPath(), - Op: gnmipb.UpdateResult_UPDATE, + resp := &gnmipb.SetResponse{ + Prefix: req.GetPrefix(), + Response: results, } - /* Add to Set response results. */ - results = append(results, &res) - } - err = dc.Set(req.GetDelete(), req.GetReplace(), req.GetUpdate()) + return resp, err + }() + + duration := time.Since(start) if err != nil { - common_utils.IncCounter(common_utils.GNMI_SET_FAIL) + log.Errorf("[GNMI-AUDIT] SetResponse user=%s peer=%s status=FAIL err=%v duration=%v", + auditUser, auditPeer, err, duration) } else { - s.SaveStartupConfig() + log.Infof("[GNMI-AUDIT] SetResponse user=%s peer=%s status=OK duration=%v", + auditUser, auditPeer, duration) } - resp := &gnmipb.SetResponse{ - Prefix: req.GetPrefix(), - Response: results, - } return resp, err } From f3f7247e6a59ff3116768485341d404572b75b90 Mon Sep 17 00:00:00 2001 From: Weiming Xu Date: Mon, 29 Jun 2026 17:16:06 -0700 Subject: [PATCH 08/15] [gnmi_server]: simplify Set audit logging to match Get style Signed-off-by: Weiming Xu Signed-off-by: Weiming --- gnmi_server/server.go | 259 +++++++++++++++++++++--------------------- 1 file changed, 127 insertions(+), 132 deletions(-) diff --git a/gnmi_server/server.go b/gnmi_server/server.go index 4767b73cb..11619b560 100644 --- a/gnmi_server/server.go +++ b/gnmi_server/server.go @@ -1112,7 +1112,7 @@ func SaveOnSetEnabled() error { // SaveOnSetDisabeld does nothing. func saveOnSetDisabled() error { return nil } -func (s *Server) Set(ctx context.Context, req *gnmipb.SetRequest) (*gnmipb.SetResponse, error) { +func (s *Server) Set(ctx context.Context, req *gnmipb.SetRequest) (resp *gnmipb.SetResponse, err error) { start := time.Now() auditUser := extractUser(ctx) auditPeer := extractPeer(ctx) @@ -1120,166 +1120,161 @@ func (s *Server) Set(ctx context.Context, req *gnmipb.SetRequest) (*gnmipb.SetRe log.Infof("[GNMI-AUDIT] SetRequest user=%s peer=%s prefix=%v updates=%d replaces=%d deletes=%d", auditUser, auditPeer, req.GetPrefix(), len(req.GetUpdate()), len(req.GetReplace()), len(req.GetDelete())) - resp, err := func() (*gnmipb.SetResponse, error) { - ctx := ctx - - e := s.ReqFromMaster(req, &s.masterEID) - if e != nil { - return nil, e + defer func() { + duration := time.Since(start) + if err != nil { + log.Errorf("[GNMI-AUDIT] SetResponse user=%s peer=%s status=FAIL err=%v duration=%v", + auditUser, auditPeer, err, duration) + } else { + log.Infof("[GNMI-AUDIT] SetResponse user=%s peer=%s status=OK duration=%v", + auditUser, auditPeer, duration) } + }() - common_utils.IncCounter(common_utils.GNMI_SET) - if s.config.EnableTranslibWrite == false && s.config.EnableNativeWrite == false { - common_utils.IncCounter(common_utils.GNMI_SET_FAIL) - return nil, grpc.Errorf(codes.Unimplemented, "GNMI is in read-only mode") - } - // gNMI path based authorization - if s.config.PathzPolicy { - pathzUser, userErr := getUsername(ctx) - if userErr != nil { - log.V(1).Infof("SetRequest User not found: %s", userErr.Error()) - return nil, userErr - } - permitted := true - for _, path := range req.GetDelete() { - s.gnsiPathz.pathzProcessor.AuthorizeWithPrefix(pathzUser, req.GetPrefix(), path, gnsi_pathz_pb.Mode_MODE_WRITE) - } - for _, update := range req.GetReplace() { - s.gnsiPathz.pathzProcessor.AuthorizeWithPrefix(pathzUser, req.GetPrefix(), update.GetPath(), gnsi_pathz_pb.Mode_MODE_WRITE) - } - for _, update := range req.GetUpdate() { - s.gnsiPathz.pathzProcessor.AuthorizeWithPrefix(pathzUser, req.GetPrefix(), update.GetPath(), gnsi_pathz_pb.Mode_MODE_WRITE) - } - if !permitted { - return nil, status.Error(codes.PermissionDenied, "Unauthorized request. Rejected by pathz policy.") - } - } - var results []*gnmipb.UpdateResult + e := s.ReqFromMaster(req, &s.masterEID) + if e != nil { + return nil, e + } - /* Fetch the prefix. */ - prefix := req.GetPrefix() - origin := "" - if prefix != nil { - origin = prefix.Origin + common_utils.IncCounter(common_utils.GNMI_SET) + if s.config.EnableTranslibWrite == false && s.config.EnableNativeWrite == false { + common_utils.IncCounter(common_utils.GNMI_SET_FAIL) + return nil, grpc.Errorf(codes.Unimplemented, "GNMI is in read-only mode") + } + // gNMI path based authorization + if s.config.PathzPolicy { + pathzUser, userErr := getUsername(ctx) + if userErr != nil { + log.V(1).Infof("SetRequest User not found: %s", userErr.Error()) + return nil, userErr } - extensions := req.GetExtension() - encoding := gnmipb.Encoding_JSON_IETF - - var dc sdc.Client - var err error - paths := req.GetDelete() - for _, path := range req.GetReplace() { - paths = append(paths, path.GetPath()) + permitted := true + for _, path := range req.GetDelete() { + s.gnsiPathz.pathzProcessor.AuthorizeWithPrefix(pathzUser, req.GetPrefix(), path, gnsi_pathz_pb.Mode_MODE_WRITE) } - for _, path := range req.GetUpdate() { - paths = append(paths, path.GetPath()) + for _, update := range req.GetReplace() { + s.gnsiPathz.pathzProcessor.AuthorizeWithPrefix(pathzUser, req.GetPrefix(), update.GetPath(), gnsi_pathz_pb.Mode_MODE_WRITE) } - if origin == "" { - origin, err = ParseOrigin(paths) - if err != nil { - return nil, err - } + for _, update := range req.GetUpdate() { + s.gnsiPathz.pathzProcessor.AuthorizeWithPrefix(pathzUser, req.GetPrefix(), update.GetPath(), gnsi_pathz_pb.Mode_MODE_WRITE) } - authTarget := "gnmi" - if check := IsNativeOrigin(origin); check { - if s.config.EnableNativeWrite == false { - common_utils.IncCounter(common_utils.GNMI_SET_FAIL) - return nil, grpc.Errorf(codes.Unimplemented, "GNMI native write is disabled") - } - - // Fast path: bypass validation for allowed tables/SKUs - allUpdates := append(req.GetReplace(), req.GetUpdate()...) - if bypassResp, used, bypassErr := bypass.TrySet(ctx, prefix, req.GetDelete(), allUpdates); used { - if bypassErr != nil { - common_utils.IncCounter(common_utils.GNMI_SET_FAIL) - return nil, status.Error(codes.Internal, bypassErr.Error()) - } - common_utils.IncCounter(common_utils.GNMI_SET_BYPASS) - return bypassResp, nil - } - - var targetDbName string - dc, err = sdc.NewMixedDbClient(paths, prefix, origin, encoding, s.config.ZmqPort, s.config.Vrf, &targetDbName) - authTarget = "gnmi_" + targetDbName - } else { - if s.config.EnableTranslibWrite == false { - common_utils.IncCounter(common_utils.GNMI_SET_FAIL) - return nil, grpc.Errorf(codes.Unimplemented, "Translib write is disabled") - } - /* Create Transl client. */ - dc, err = sdc.NewTranslClient(prefix, nil, ctx, extensions) + if !permitted { + return nil, status.Error(codes.PermissionDenied, "Unauthorized request. Rejected by pathz policy.") } + } + var results []*gnmipb.UpdateResult - if err != nil { - common_utils.IncCounter(common_utils.GNMI_SET_FAIL) - return nil, status.Error(codes.NotFound, err.Error()) - } - defer dc.Close() + /* Fetch the prefix. */ + prefix := req.GetPrefix() + origin := "" + if prefix != nil { + origin = prefix.Origin + } + extensions := req.GetExtension() + encoding := gnmipb.Encoding_JSON_IETF - ctx, err = authenticate(s.config, ctx, authTarget, true) + var dc sdc.Client + paths := req.GetDelete() + for _, path := range req.GetReplace() { + paths = append(paths, path.GetPath()) + } + for _, path := range req.GetUpdate() { + paths = append(paths, path.GetPath()) + } + if origin == "" { + origin, err = ParseOrigin(paths) if err != nil { - common_utils.IncCounter(common_utils.GNMI_SET_FAIL) return nil, err } - /* DELETE */ - for _, path := range req.GetDelete() { - log.V(2).Infof("Delete path: %v", path) + } + authTarget := "gnmi" + if check := IsNativeOrigin(origin); check { + if s.config.EnableNativeWrite == false { + common_utils.IncCounter(common_utils.GNMI_SET_FAIL) + return nil, grpc.Errorf(codes.Unimplemented, "GNMI native write is disabled") + } - res := gnmipb.UpdateResult{ - Path: path, - Op: gnmipb.UpdateResult_DELETE, + // Fast path: bypass validation for allowed tables/SKUs + allUpdates := append(req.GetReplace(), req.GetUpdate()...) + if bypassResp, used, bypassErr := bypass.TrySet(ctx, prefix, req.GetDelete(), allUpdates); used { + if bypassErr != nil { + common_utils.IncCounter(common_utils.GNMI_SET_FAIL) + return nil, status.Error(codes.Internal, bypassErr.Error()) } + common_utils.IncCounter(common_utils.GNMI_SET_BYPASS) + return bypassResp, nil + } - /* Add to Set response results. */ - results = append(results, &res) + var targetDbName string + dc, err = sdc.NewMixedDbClient(paths, prefix, origin, encoding, s.config.ZmqPort, s.config.Vrf, &targetDbName) + authTarget = "gnmi_" + targetDbName + } else { + if s.config.EnableTranslibWrite == false { + common_utils.IncCounter(common_utils.GNMI_SET_FAIL) + return nil, grpc.Errorf(codes.Unimplemented, "Translib write is disabled") } + /* Create Transl client. */ + dc, err = sdc.NewTranslClient(prefix, nil, ctx, extensions) + } + + if err != nil { + common_utils.IncCounter(common_utils.GNMI_SET_FAIL) + return nil, status.Error(codes.NotFound, err.Error()) + } + defer dc.Close() - /* REPLACE */ - for _, path := range req.GetReplace() { - log.V(2).Infof("Replace path: %v ", path) + ctx, err = authenticate(s.config, ctx, authTarget, true) + if err != nil { + common_utils.IncCounter(common_utils.GNMI_SET_FAIL) + return nil, err + } + /* DELETE */ + for _, path := range req.GetDelete() { + log.V(2).Infof("Delete path: %v", path) - res := gnmipb.UpdateResult{ - Path: path.GetPath(), - Op: gnmipb.UpdateResult_REPLACE, - } - /* Add to Set response results. */ - results = append(results, &res) + res := gnmipb.UpdateResult{ + Path: path, + Op: gnmipb.UpdateResult_DELETE, } - /* UPDATE */ - for _, path := range req.GetUpdate() { - log.V(2).Infof("Update path: %v ", path) + /* Add to Set response results. */ + results = append(results, &res) + } - res := gnmipb.UpdateResult{ - Path: path.GetPath(), - Op: gnmipb.UpdateResult_UPDATE, - } - /* Add to Set response results. */ - results = append(results, &res) - } - err = dc.Set(req.GetDelete(), req.GetReplace(), req.GetUpdate()) - if err != nil { - common_utils.IncCounter(common_utils.GNMI_SET_FAIL) - } else { - s.SaveStartupConfig() - } + /* REPLACE */ + for _, path := range req.GetReplace() { + log.V(2).Infof("Replace path: %v ", path) - resp := &gnmipb.SetResponse{ - Prefix: req.GetPrefix(), - Response: results, + res := gnmipb.UpdateResult{ + Path: path.GetPath(), + Op: gnmipb.UpdateResult_REPLACE, } - return resp, err - }() + /* Add to Set response results. */ + results = append(results, &res) + } + + /* UPDATE */ + for _, path := range req.GetUpdate() { + log.V(2).Infof("Update path: %v ", path) - duration := time.Since(start) + res := gnmipb.UpdateResult{ + Path: path.GetPath(), + Op: gnmipb.UpdateResult_UPDATE, + } + /* Add to Set response results. */ + results = append(results, &res) + } + err = dc.Set(req.GetDelete(), req.GetReplace(), req.GetUpdate()) if err != nil { - log.Errorf("[GNMI-AUDIT] SetResponse user=%s peer=%s status=FAIL err=%v duration=%v", - auditUser, auditPeer, err, duration) + common_utils.IncCounter(common_utils.GNMI_SET_FAIL) } else { - log.Infof("[GNMI-AUDIT] SetResponse user=%s peer=%s status=OK duration=%v", - auditUser, auditPeer, duration) + s.SaveStartupConfig() } + resp = &gnmipb.SetResponse{ + Prefix: req.GetPrefix(), + Response: results, + } return resp, err } From ea688a6e7dd2a8a01e07e6a0c4bc84e3e7375da7 Mon Sep 17 00:00:00 2001 From: Weiming Xu Date: Mon, 29 Jun 2026 17:19:56 -0700 Subject: [PATCH 09/15] [gnmi_server]: restore Set core block while keeping audit logs Signed-off-by: Weiming Xu Signed-off-by: Weiming --- gnmi_server/server.go | 29 +++++++++++++++-------------- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/gnmi_server/server.go b/gnmi_server/server.go index 11619b560..7e9201833 100644 --- a/gnmi_server/server.go +++ b/gnmi_server/server.go @@ -1112,7 +1112,7 @@ func SaveOnSetEnabled() error { // SaveOnSetDisabeld does nothing. func saveOnSetDisabled() error { return nil } -func (s *Server) Set(ctx context.Context, req *gnmipb.SetRequest) (resp *gnmipb.SetResponse, err error) { +func (s *Server) Set(ctx context.Context, req *gnmipb.SetRequest) (resp *gnmipb.SetResponse, retErr error) { start := time.Now() auditUser := extractUser(ctx) auditPeer := extractPeer(ctx) @@ -1122,9 +1122,9 @@ func (s *Server) Set(ctx context.Context, req *gnmipb.SetRequest) (resp *gnmipb. defer func() { duration := time.Since(start) - if err != nil { + if retErr != nil { log.Errorf("[GNMI-AUDIT] SetResponse user=%s peer=%s status=FAIL err=%v duration=%v", - auditUser, auditPeer, err, duration) + auditUser, auditPeer, retErr, duration) } else { log.Infof("[GNMI-AUDIT] SetResponse user=%s peer=%s status=OK duration=%v", auditUser, auditPeer, duration) @@ -1143,20 +1143,20 @@ func (s *Server) Set(ctx context.Context, req *gnmipb.SetRequest) (resp *gnmipb. } // gNMI path based authorization if s.config.PathzPolicy { - pathzUser, userErr := getUsername(ctx) - if userErr != nil { - log.V(1).Infof("SetRequest User not found: %s", userErr.Error()) - return nil, userErr + user, err := getUsername(ctx) + if err != nil { + log.V(1).Infof("SetRequest User not found: %s", err.Error()) + return nil, err } permitted := true for _, path := range req.GetDelete() { - s.gnsiPathz.pathzProcessor.AuthorizeWithPrefix(pathzUser, req.GetPrefix(), path, gnsi_pathz_pb.Mode_MODE_WRITE) + s.gnsiPathz.pathzProcessor.AuthorizeWithPrefix(user, req.GetPrefix(), path, gnsi_pathz_pb.Mode_MODE_WRITE) } for _, update := range req.GetReplace() { - s.gnsiPathz.pathzProcessor.AuthorizeWithPrefix(pathzUser, req.GetPrefix(), update.GetPath(), gnsi_pathz_pb.Mode_MODE_WRITE) + s.gnsiPathz.pathzProcessor.AuthorizeWithPrefix(user, req.GetPrefix(), update.GetPath(), gnsi_pathz_pb.Mode_MODE_WRITE) } for _, update := range req.GetUpdate() { - s.gnsiPathz.pathzProcessor.AuthorizeWithPrefix(pathzUser, req.GetPrefix(), update.GetPath(), gnsi_pathz_pb.Mode_MODE_WRITE) + s.gnsiPathz.pathzProcessor.AuthorizeWithPrefix(user, req.GetPrefix(), update.GetPath(), gnsi_pathz_pb.Mode_MODE_WRITE) } if !permitted { return nil, status.Error(codes.PermissionDenied, "Unauthorized request. Rejected by pathz policy.") @@ -1174,6 +1174,7 @@ func (s *Server) Set(ctx context.Context, req *gnmipb.SetRequest) (resp *gnmipb. encoding := gnmipb.Encoding_JSON_IETF var dc sdc.Client + var err error paths := req.GetDelete() for _, path := range req.GetReplace() { paths = append(paths, path.GetPath()) @@ -1196,13 +1197,13 @@ func (s *Server) Set(ctx context.Context, req *gnmipb.SetRequest) (resp *gnmipb. // Fast path: bypass validation for allowed tables/SKUs allUpdates := append(req.GetReplace(), req.GetUpdate()...) - if bypassResp, used, bypassErr := bypass.TrySet(ctx, prefix, req.GetDelete(), allUpdates); used { - if bypassErr != nil { + if resp, used, err := bypass.TrySet(ctx, prefix, req.GetDelete(), allUpdates); used { + if err != nil { common_utils.IncCounter(common_utils.GNMI_SET_FAIL) - return nil, status.Error(codes.Internal, bypassErr.Error()) + return nil, status.Error(codes.Internal, err.Error()) } common_utils.IncCounter(common_utils.GNMI_SET_BYPASS) - return bypassResp, nil + return resp, nil } var targetDbName string From 680b3189c0db137254659d5c1fdbe4279b02b08f Mon Sep 17 00:00:00 2001 From: Weiming Xu Date: Mon, 29 Jun 2026 17:30:42 -0700 Subject: [PATCH 10/15] gnmi_server: restore inline Set response return Signed-off-by: Weiming Xu Signed-off-by: Weiming --- gnmi_server/server.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/gnmi_server/server.go b/gnmi_server/server.go index 7e9201833..98333f82e 100644 --- a/gnmi_server/server.go +++ b/gnmi_server/server.go @@ -1272,11 +1272,10 @@ func (s *Server) Set(ctx context.Context, req *gnmipb.SetRequest) (resp *gnmipb. s.SaveStartupConfig() } - resp = &gnmipb.SetResponse{ + return &gnmipb.SetResponse{ Prefix: req.GetPrefix(), Response: results, - } - return resp, err + }, err } func (s *Server) Capabilities(ctx context.Context, req *gnmipb.CapabilityRequest) (*gnmipb.CapabilityResponse, error) { From 6865bf3a4167c92d11d0d864c9efc34ab7b61c25 Mon Sep 17 00:00:00 2001 From: Weiming Xu Date: Mon, 29 Jun 2026 17:49:29 -0700 Subject: [PATCH 11/15] gnmi_server: unify Get/Set audit defer logging helper Signed-off-by: Weiming Xu Signed-off-by: Weiming --- gnmi_server/server.go | 36 ++++++++++++++---------------------- 1 file changed, 14 insertions(+), 22 deletions(-) diff --git a/gnmi_server/server.go b/gnmi_server/server.go index 98333f82e..52040cc42 100644 --- a/gnmi_server/server.go +++ b/gnmi_server/server.go @@ -960,18 +960,7 @@ func (s *Server) Get(ctx context.Context, req *gnmipb.GetRequest) (resp *gnmipb. log.Infof("[GNMI-AUDIT] GetRequest user=%s peer=%s prefix=%v paths=%v type=%v", auditUser, auditPeer, req.GetPrefix(), req.GetPath(), req.GetType()) - // defer logs automatically when function returns - defer func() { - duration := time.Since(start) - - if err != nil { - log.Errorf("[GNMI-AUDIT] GetResponse user=%s peer=%s status=FAIL err=%v duration=%v", - auditUser, auditPeer, err, duration) - } else { - log.Infof("[GNMI-AUDIT] GetResponse user=%s peer=%s status=OK duration=%v", - auditUser, auditPeer, duration) - } - }() + defer logGnmiAuditResponse("Get", auditUser, auditPeer, start, &err) common_utils.IncCounter(common_utils.GNMI_GET) @@ -1120,16 +1109,7 @@ func (s *Server) Set(ctx context.Context, req *gnmipb.SetRequest) (resp *gnmipb. log.Infof("[GNMI-AUDIT] SetRequest user=%s peer=%s prefix=%v updates=%d replaces=%d deletes=%d", auditUser, auditPeer, req.GetPrefix(), len(req.GetUpdate()), len(req.GetReplace()), len(req.GetDelete())) - defer func() { - duration := time.Since(start) - if retErr != nil { - log.Errorf("[GNMI-AUDIT] SetResponse user=%s peer=%s status=FAIL err=%v duration=%v", - auditUser, auditPeer, retErr, duration) - } else { - log.Infof("[GNMI-AUDIT] SetResponse user=%s peer=%s status=OK duration=%v", - auditUser, auditPeer, duration) - } - }() + defer logGnmiAuditResponse("Set", auditUser, auditPeer, start, &retErr) e := s.ReqFromMaster(req, &s.masterEID) if e != nil { @@ -1350,6 +1330,18 @@ func extractPeer(ctx context.Context) string { return "unknown" } +func logGnmiAuditResponse(method, user, peerAddr string, start time.Time, errPtr *error) { + duration := time.Since(start) + + if errPtr != nil && *errPtr != nil { + log.Errorf("[GNMI-AUDIT] %sResponse user=%s peer=%s status=FAIL err=%v duration=%v", + method, user, peerAddr, *errPtr, duration) + } else { + log.Infof("[GNMI-AUDIT] %sResponse user=%s peer=%s status=OK duration=%v", + method, user, peerAddr, duration) + } +} + // Obtain the user name as the last element of the SPIFFE ID. func getUsername(ctx context.Context) (string, error) { pr, ok := peer.FromContext(ctx) From 191ccbc5661b925cb03d04d0ec232417ca7a00af Mon Sep 17 00:00:00 2001 From: Weiming Xu Date: Mon, 29 Jun 2026 17:53:48 -0700 Subject: [PATCH 12/15] gnmi_server: restore Set trailing blank line layout Signed-off-by: Weiming Xu Signed-off-by: Weiming --- gnmi_server/server.go | 1 + 1 file changed, 1 insertion(+) diff --git a/gnmi_server/server.go b/gnmi_server/server.go index 52040cc42..bb598fea3 100644 --- a/gnmi_server/server.go +++ b/gnmi_server/server.go @@ -1256,6 +1256,7 @@ func (s *Server) Set(ctx context.Context, req *gnmipb.SetRequest) (resp *gnmipb. Prefix: req.GetPrefix(), Response: results, }, err + } func (s *Server) Capabilities(ctx context.Context, req *gnmipb.CapabilityRequest) (*gnmipb.CapabilityResponse, error) { From d00b7f17a8df667186a49410456a5d97279aae64 Mon Sep 17 00:00:00 2001 From: Weiming Xu Date: Mon, 29 Jun 2026 18:23:49 -0700 Subject: [PATCH 13/15] gnmi_server: rename audit helper peer parameter Signed-off-by: Weiming Xu Signed-off-by: Weiming --- gnmi_server/server.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/gnmi_server/server.go b/gnmi_server/server.go index bb598fea3..45fcc584d 100644 --- a/gnmi_server/server.go +++ b/gnmi_server/server.go @@ -1331,15 +1331,15 @@ func extractPeer(ctx context.Context) string { return "unknown" } -func logGnmiAuditResponse(method, user, peerAddr string, start time.Time, errPtr *error) { +func logGnmiAuditResponse(method, user, peer string, start time.Time, errPtr *error) { duration := time.Since(start) if errPtr != nil && *errPtr != nil { log.Errorf("[GNMI-AUDIT] %sResponse user=%s peer=%s status=FAIL err=%v duration=%v", - method, user, peerAddr, *errPtr, duration) + method, user, peer, *errPtr, duration) } else { log.Infof("[GNMI-AUDIT] %sResponse user=%s peer=%s status=OK duration=%v", - method, user, peerAddr, duration) + method, user, peer, duration) } } From 8f41c18a118f37a48674af8c613bc66f456ea891 Mon Sep 17 00:00:00 2001 From: Weiming Xu Date: Fri, 3 Jul 2026 19:32:34 -0700 Subject: [PATCH 14/15] gnmi_server: add TestGnmiAuditLogging Signed-off-by: Weiming Xu (cherry picked from commit 5a20feab9a9b79f4b93a88342d190e769b776c67) Signed-off-by: Weiming --- gnmi_server/server_test.go | 119 +++++++++++++++++++++++++++++++++++++ 1 file changed, 119 insertions(+) diff --git a/gnmi_server/server_test.go b/gnmi_server/server_test.go index a054958b7..e2141806e 100644 --- a/gnmi_server/server_test.go +++ b/gnmi_server/server_test.go @@ -7269,6 +7269,125 @@ func TestSrvAdvConfig(t *testing.T) { } } +// TestGnmiAuditLogging verifies that [GNMI-AUDIT] log entries are produced +// with the correct method, user, peer, status, and error for Get and Set RPCs. +func TestGnmiAuditLogging(t *testing.T) { + type auditCall struct { + method string + user string + peer string + status string + err error + } + + tests := []struct { + desc string + setupServer func(t *testing.T) *Server + makeRequest func(t *testing.T, gClient pb.GNMIClient, ctx context.Context) + wantMethod string + wantStatus string + wantErrMsg string + }{ + { + desc: "Get request logged as FAIL on auth failure", + setupServer: func(t *testing.T) *Server { return createAuthServer(t, 8081) }, + makeRequest: func(t *testing.T, gClient pb.GNMIClient, ctx context.Context) { + gClient.Get(ctx, &pb.GetRequest{}) + }, + wantMethod: "Get", + wantStatus: "FAIL", + wantErrMsg: "Unauthenticated", + }, + { + desc: "Set request logged as FAIL on read-only server", + setupServer: func(t *testing.T) *Server { return createReadServer(t, 8081) }, + makeRequest: func(t *testing.T, gClient pb.GNMIClient, ctx context.Context) { + gClient.Set(ctx, &pb.SetRequest{}) + }, + wantMethod: "Set", + wantStatus: "FAIL", + wantErrMsg: "read-only", + }, + { + desc: "Set request logged as FAIL on auth failure", + setupServer: func(t *testing.T) *Server { return createAuthServer(t, 8081) }, + makeRequest: func(t *testing.T, gClient pb.GNMIClient, ctx context.Context) { + gClient.Set(ctx, &pb.SetRequest{}) + }, + wantMethod: "Set", + wantStatus: "FAIL", + wantErrMsg: "Unauthenticated", + }, + } + + for _, tt := range tests { + t.Run(tt.desc, func(t *testing.T) { + var mu sync.Mutex + var captured auditCall + + mock := gomonkey.ApplyFunc(logGnmiAuditResponse, + func(method, user, peer string, start time.Time, errPtr *error) { + mu.Lock() + defer mu.Unlock() + captured.method = method + captured.user = user + captured.peer = peer + if errPtr != nil && *errPtr != nil { + captured.status = "FAIL" + captured.err = *errPtr + } else { + captured.status = "OK" + } + }) + defer mock.Reset() + + s := tt.setupServer(t) + go runServer(t, s) + defer s.Stop() + + tlsConfig := &tls.Config{InsecureSkipVerify: true} + opts := []grpc.DialOption{grpc.WithTransportCredentials(credentials.NewTLS(tlsConfig))} + conn, err := grpc.Dial("127.0.0.1:8081", opts...) + if err != nil { + t.Fatalf("Dialing failed: %v", err) + } + defer conn.Close() + + gClient := pb.NewGNMIClient(conn) + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + + tt.makeRequest(t, gClient, ctx) + // Allow time for the deferred audit log to fire after RPC returns. + time.Sleep(200 * time.Millisecond) + + mu.Lock() + gotMethod := captured.method + gotStatus := captured.status + gotErr := captured.err + gotPeer := captured.peer + mu.Unlock() + + if gotMethod != tt.wantMethod { + t.Errorf("audit method: got %q, want %q", gotMethod, tt.wantMethod) + } + if gotStatus != tt.wantStatus { + t.Errorf("audit status: got %q, want %q", gotStatus, tt.wantStatus) + } + if gotPeer == "" || gotPeer == "unknown" { + t.Errorf("audit peer should be populated, got %q", gotPeer) + } + if tt.wantErrMsg != "" { + if gotErr == nil { + t.Errorf("expected audit error containing %q but got nil", tt.wantErrMsg) + } else if !strings.Contains(gotErr.Error(), tt.wantErrMsg) { + t.Errorf("audit error %q does not contain %q", gotErr.Error(), tt.wantErrMsg) + } + } + }) + } +} + func TestSrvTestConfigLogsAndReturns(t *testing.T) { // 1. Force error in the FIRST WriteFile (Server Certificate) t.Run("CoverageCertWriteError", func(t *testing.T) { From f2de4d7faafcdbaa85bc64801a7452e35da649e1 Mon Sep 17 00:00:00 2001 From: Weiming Xu Date: Sun, 12 Jul 2026 05:59:22 +0000 Subject: [PATCH 15/15] gofmt gnmi_server/server.go Signed-off-by: Weiming Xu --- gnmi_server/server.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gnmi_server/server.go b/gnmi_server/server.go index 45fcc584d..fa9502f66 100644 --- a/gnmi_server/server.go +++ b/gnmi_server/server.go @@ -958,7 +958,7 @@ func (s *Server) Get(ctx context.Context, req *gnmipb.GetRequest) (resp *gnmipb. auditPeer := extractPeer(ctx) log.Infof("[GNMI-AUDIT] GetRequest user=%s peer=%s prefix=%v paths=%v type=%v", - auditUser, auditPeer, req.GetPrefix(), req.GetPath(), req.GetType()) + auditUser, auditPeer, req.GetPrefix(), req.GetPath(), req.GetType()) defer logGnmiAuditResponse("Get", auditUser, auditPeer, start, &err)