Skip to content

Fixing acctz tests for Nokia#5354

Merged
ram-mac merged 9 commits into
openconfig:mainfrom
ram-mac:nokia_acctz_fix
Apr 21, 2026
Merged

Fixing acctz tests for Nokia#5354
ram-mac merged 9 commits into
openconfig:mainfrom
ram-mac:nokia_acctz_fix

Conversation

@ram-mac
Copy link
Copy Markdown
Contributor

@ram-mac ram-mac commented Apr 17, 2026

changes include as below

  1. For nokia acctz tests to work, the grpc-server metadata.authentication need to be set to 'true'. However when we set this to true the gnmi session is terminated and hence the gnmi connection lost specifically for gnmi/gnoi/gnsi services running on 9339. To fix this we have been able t
  2. Added a expectedAuthzStatus helper function to conditionally expect AUTHZ_STATUS_ERROR for Nokia devices where AUTHZ_STATUS_DENY is normally expected.
  3. For GNOI Vendor-Specific RPC switched GNOI record testing for Nokia from Ping (streaming) to Time (unary) to ensure reliable record capture.
  4. Added following deviation for Nokia for record_subscribe_non_grpc test

@ram-mac ram-mac requested review from a team as code owners April 17, 2026 10:52
@gemini-code-assist
Copy link
Copy Markdown
Contributor

Summary of Changes

Hello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request introduces necessary adjustments to the acctz test suite to support Nokia devices. The changes primarily address compatibility issues with gRPC metadata authentication and authorization status reporting, ensuring that tests can run reliably on Nokia hardware without triggering unintended session losses or incorrect status expectations.

Highlights

  • Nokia gRPC Metadata Authentication: Implemented a workaround for Nokia devices by creating a separate gRPC server for testing, as enabling metadata authentication on the primary server caused session termination.
  • Conditional Authorization Status: Introduced the expectedAuthzStatus helper function to correctly handle Nokia-specific authorization responses, mapping AUTHZ_STATUS_DENY to AUTHZ_STATUS_ERROR where appropriate.
  • GNOI RPC Testing: Switched GNOI record testing for Nokia from Ping (streaming) to Time (unary) to ensure reliable record capture.
  • Test Deviations: Added specific platform deviations for Nokia in the record_subscribe_non_grpc test.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

@OpenConfigBot
Copy link
Copy Markdown

OpenConfigBot commented Apr 17, 2026

Pull Request Functional Test Report for #5354 / ffba8fe

Virtual Devices

Device Test Test Documentation Job Raw Log
Arista cEOS status
ACCTZ-3.1: Record Subscribe Non-gRPC
462d3a8c Log
Cisco 8000E status
ACCTZ-3.1: Record Subscribe Non-gRPC
571ddb1a Log
Cisco XRd status
ACCTZ-3.1: Record Subscribe Non-gRPC
6745602b Log
Juniper ncPTX status
ACCTZ-3.1: Record Subscribe Non-gRPC
945fb3c7 Log
Nokia SR Linux status
ACCTZ-3.1: Record Subscribe Non-gRPC
00ba6c5c Log
Openconfig Lemming status
ACCTZ-3.1: Record Subscribe Non-gRPC
f5c353bc Log

Hardware Devices

Device Test Test Documentation Raw Log
Arista 7808 status
ACCTZ-3.1: Record Subscribe Non-gRPC
Cisco 8808 status
ACCTZ-3.1: Record Subscribe Non-gRPC
Juniper PTX10008 status
ACCTZ-3.1: Record Subscribe Non-gRPC
Nokia 7250 IXR-10e status
ACCTZ-3.1: Record Subscribe Non-gRPC

Help

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request adds support for Nokia devices to the acctz security tests, including platform-specific deviations, custom gRPC server configuration, and vendor-specific RPC handling for gNMI, gNOI, and gNSI. The feedback suggests improving code maintainability by defining the hardcoded port number as a constant and refactoring duplicated dialing logic into a shared helper function.

Comment thread internal/security/acctz/acctz.go
Comment thread internal/security/acctz/acctz.go
Copy link
Copy Markdown
Member

@mihirpitale-googler mihirpitale-googler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@ram-mac ram-mac merged commit 1d0405b into openconfig:main Apr 21, 2026
15 of 16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants