Add Gnmi Audit-Logging to Get() and Set()#714
Conversation
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
Hi, there are workflow run(s) waiting for approval, you may be first-time contributor. I will notify maintainers to help approve once PR is approved. Thanks! ---Powered by SONiC BuildBot
|
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
=== RUN TestGnmiAuditLogging |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Signed-off-by: Weiming <wmxu2000@gmail.com>
Signed-off-by: Weiming Xu <wmxu2000@gmail.com> Signed-off-by: Weiming <wmxu2000@gmail.com>
Signed-off-by: Weiming Xu <wmxu2000@gmail.com> Signed-off-by: Weiming <wmxu2000@gmail.com>
Signed-off-by: Weiming Xu <wmxu2000@gmail.com> Signed-off-by: Weiming <wmxu2000@gmail.com>
Signed-off-by: Weiming Xu <wmxu2000@gmail.com> Signed-off-by: Weiming <wmxu2000@gmail.com>
Signed-off-by: Weiming Xu <wmxu2000@gmail.com> Signed-off-by: Weiming <wmxu2000@gmail.com>
Signed-off-by: Weiming Xu <wmxu2000@gmail.com> Signed-off-by: Weiming <wmxu2000@gmail.com>
Signed-off-by: Weiming Xu <wmxu2000@gmail.com> Signed-off-by: Weiming <wmxu2000@gmail.com>
Signed-off-by: Weiming Xu <wmxu2000@gmail.com> Signed-off-by: Weiming <wmxu2000@gmail.com>
Signed-off-by: Weiming Xu <wmxu2000@gmail.com> Signed-off-by: Weiming <wmxu2000@gmail.com>
Signed-off-by: Weiming Xu <wmxu2000@gmail.com> Signed-off-by: Weiming <wmxu2000@gmail.com>
Signed-off-by: Weiming Xu <wmxu2000@gmail.com> Signed-off-by: Weiming <wmxu2000@gmail.com>
Signed-off-by: Weiming Xu <wmxu2000@gmail.com> Signed-off-by: Weiming <wmxu2000@gmail.com>
Signed-off-by: Weiming Xu <wmxu2000@gmail.com> (cherry picked from commit 5a20fea) Signed-off-by: Weiming <wmxu2000@gmail.com>
985bdad to
8f41c18
Compare
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run |
|
Commenter does not have sufficient privileges for PR 714 in repo sonic-net/sonic-gnmi |
|
/azp run |
|
Commenter does not have sufficient privileges for PR 714 in repo sonic-net/sonic-gnmi |
|
Maintainers: this PR is currently blocked by workflow approval and I do not have permission to trigger pipelines. Could someone please approve and run workflows, then trigger /azp run (or re-run required jobs) on the latest commit 8f41c18? DCO is green and the diff is scoped to two files. Also requesting one approving review to unblock merge. |
Leyza
left a comment
Leyza
left a comment
There was a problem hiding this comment.
Can you include test outputs in description?
Added. |
Signed-off-by: Weiming Xu <wmxu2000@gmail.com>
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
3 participants
Summary
This PR fixes GNMI audit logging in server.go for both Get() and Set().
Changes
Get():
keeps deferred request/response audit logs
removes problematic error-variable shadowing/redeclaration paths
uses clearer local variable names in pathz handling
Set():
switches to named returns so deferred response logging can cover all exits
adds a single deferred SetResponse audit log for success/failure
removes duplicated end-of-function response logging
keeps existing functional behavior and counters
Why
Previous changes had error-scope issues and non-uniform logging behavior on early returns. This update makes logging consistent and safer.
Validation
diagnostics show no errors in server.go
branch pushed: fix/gnmi-audit-logging-get-set
commit: 172f204
Test Results
TestGnmiAuditLoggingadded in gnmi_server/server_test.go — 3 subtests all PASS:The complete TestGnmiAuditLogging is as follows:
root@docker-desktop:/src/sonic-gnmi# cd /src/sonic-gnmi
Logging -count=1root@docker-desktop:/src/sonic-gnmi# grep -n "TestGnmiAuditLogging" gnmi_server/server_test.go
7118:// TestGnmiAuditLogging verifies that [GNMI-AUDIT] log entries are produced
7120:func TestGnmiAuditLogging(t testing.T) {
root@docker-desktop:/src/sonic-gnmi# go test -mod=vendor -v ./gnmi_server -run TestGnmiAuditLogging -count=1
#github.com/sonic-net/sonic-gnmi/sonic_data_client
cgo-gcc-prolog: In function ‘_cgo_09acf087b92e_Cfunc_Py_Finalize’:
cgo-gcc-prolog:69:49: warning: unused variable ‘_cgo_a’ [-Wunused-variable]
cgo-gcc-prolog: In function ‘_cgo_09acf087b92e_Cfunc_Py_Initialize’:
cgo-gcc-prolog:81:49: warning: unused variable ‘_cgo_a’ [-Wunused-variable]
libyang[0]: Unable to use search directory "schema/" (No such file or directory)
ERROR: logging before flag.Parse: I0703 06:20:00.944570 65664 util.go:332] [CVL] : Could not read platform schema location or no platform specific schema exists.
ERROR: logging before flag.Parse: W0703 06:20:00.949425 65664 db_redis_opts.go:165] _DBRedisOptsConfig:reconfigure: flags not parsed!
ERROR: logging before flag.Parse: I0703 06:20:00.949477 65664 db_redis_opts.go:175] _DBRedisOptsConfig:reconfigure: Handling signal.
ERROR: logging before flag.Parse: W0703 06:20:00.949483 65664 db_redis_opts.go:165] _DBRedisOptsConfig:reconfigure: flags not parsed!
ERROR: logging before flag.Parse: W0703 06:20:00.963029 65664 db_redis_opts.go:165] _DBRedisOptsConfig:reconfigure: flags not parsed!
ERROR: logging before flag.Parse: W0703 06:20:00.967376 65664 db_redis_opts.go:165] _DBRedisOptsConfig:reconfigure: flags not parsed!
ERROR: logging before flag.Parse: W0703 06:20:00.968744 65664 db_redis_opts.go:165] _DBRedisOptsConfig:reconfigure: flags not parsed!
ERROR: logging before flag.Parse: W0703 06:20:00.969878 65664 db_redis_opts.go:165] _DBRedisOptsConfig:reconfigure: flags not parsed!
ERROR: logging before flag.Parse: W0703 06:20:00.971500 65664 db_redis_opts.go:165] _DBRedisOptsConfig:reconfigure: flags not parsed!
ERROR: logging before flag.Parse: W0703 06:20:00.973013 65664 db_redis_opts.go:165] _DBRedisOptsConfig:reconfigure: flags not parsed!
ERROR: logging before flag.Parse: W0703 06:20:00.974517 65664 db_redis_opts.go:165] _DBRedisOptsConfig:reconfigure: flags not parsed!
ERROR: logging before flag.Parse: W0703 06:20:00.979949 65664 db_redis_opts.go:165] _DBRedisOptsConfig:reconfigure: flags not parsed!
ERROR: logging before flag.Parse: W0703 06:20:00.980008 65664 db_redis_opts.go:119] Database instance not present for the Db name: ERROR_DB
ERROR: logging before flag.Parse: W0703 06:20:01.003601 65664 db_redis_opts.go:165] _DBRedisOptsConfig:reconfigure: flags not parsed!
ERROR: logging before flag.Parse: W0703 06:20:01.003658 65664 db_redis_opts.go:119] Database instance not present for the Db name: EVENT_DB
ERROR: logging before flag.Parse: W0703 06:20:01.006957 65664 db_redis_opts.go:165] _DBRedisOptsConfig:reconfigure: flags not parsed!
ERROR: logging before flag.Parse: I0703 06:20:01.013269 65664 db_lock.go:234] ConfigDBClearLock:
ERROR: logging before flag.Parse: W0703 06:20:01.013312 65664 db_redis_opts.go:165] _DBRedisOptsConfig:reconfigure: flags not parsed!
ERROR: logging before flag.Parse: I0703 06:20:01.016736 65664 db_lock.go:134] unlock: Already Unlocked
ERROR: logging before flag.Parse: I0703 06:20:01.016816 65664 db_lock.go:242] ConfigDBClearLock: Lock Absent
ERROR: logging before flag.Parse: W0703 06:20:01.016864 65664 db_redis_opts.go:165] _DBRedisOptsConfig:reconfigure: flags not parsed!
ERROR: logging before flag.Parse: I0703 06:20:01.019066 65664 emitjson_select.go:68] Marker file /var/run/gnmi_server.test/use_ygot_emitjson not found (err = stat /var/run/gnmi_server.test/use_ygot_emitjson: no such file or directory). Will use internal EmitJSON
ERROR: logging before flag.Parse: I0703 06:20:01.109238 65664 xlate_utils.go:1366] [transformer.go:92]init.2 Yang model List: []
ERROR: logging before flag.Parse: I0703 06:20:01.178385 65664 xspec.go:150] xspec init done ...
ERROR: logging before flag.Parse: I0703 06:20:01.178468 65664 app_interface.go:117] Registering for path =/openconfig-acl:acl
ERROR: logging before flag.Parse: I0703 06:20:01.178475 65664 app_interface.go:117] Registering for path =/sonic-
ERROR: logging before flag.Parse: I0703 06:20:01.178479 65664 app_interface.go:117] Registering for path =
ERROR: logging before flag.Parse: W0703 06:20:01.178484 65664 common_app.go:75] Failure in fetching model capabilities data.
ERROR: logging before flag.Parse: I0703 06:20:01.178497 65664 lldp_app.go:62] Init called for LLDP modules module
ERROR: logging before flag.Parse: I0703 06:20:01.178502 65664 app_interface.go:117] Registering for path =/openconfig-lldp:lldp
ERROR: logging before flag.Parse: I0703 06:20:01.178514 65664 pfm_app.go:42] Init called for Platform module
ERROR: logging before flag.Parse: I0703 06:20:01.178518 65664 app_interface.go:117] Registering for path =/openconfig-platform:components
ERROR: logging before flag.Parse: I0703 06:20:01.178545 65664 sys_app.go:54] SysApp: Init called for System module
ERROR: logging before flag.Parse: I0703 06:20:01.178549 65664 app_interface.go:117] Registering for path =/openconfig-system:systemERROR: logging before flag.Parse: E0703 06:20:01.178587 65664 version.go:141] Error loading version config file; err=open /usr/models/yang/version.xml: no such file or directory
ERROR: logging before flag.Parse: E0703 06:20:01.178602 65664 version.go:142] API VERSION CHECK IS DISABLED
ERROR: logging before flag.Parse: I0703 06:20:01.178607 65664 app_interface.go:117] Registering for path =/ietf-yang-library:modules-state
=== RUN TestGnmiAuditLogging
=== RUN TestGnmiAuditLogging/Get_request_logged_as_FAIL_on_auth_failure
=== RUN TestGnmiAuditLogging/Set_request_logged_as_FAIL_on_read-only_server
=== RUN TestGnmiAuditLogging/Set_request_logged_as_FAIL_on_auth_failure
--- PASS: TestGnmiAuditLogging (6.17s)
--- PASS: TestGnmiAuditLogging/Get_request_logged_as_FAIL_on_auth_failure (0.60s)
--- PASS: TestGnmiAuditLogging/Set_request_logged_as_FAIL_on_read-only_server (2.16s)
--- PASS: TestGnmiAuditLogging/Set_request_logged_as_FAIL_on_auth_failure (3.41s)
PASS
ok github.com/sonic-net/sonic-gnmi/gnmi_server 6.667s
The dedicated audit test passed in Docker.
The important part of your output is:
• === RUN TestGnmiAuditLogging
• three subtests all PASS
• final PASS
• package ok
So the new audit-logging unit test in server_test.go:7120 is good. The earlier ERROR: logging before flag.Parse, version.xml warnings, and cgo warnings did not fail the test run.