-
Notifications
You must be signed in to change notification settings - Fork 80
Fix parsing of gNMI response without namespaces #2250
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
JIRA: LIGHTY-294 Signed-off-by: tobias.pobocik <[email protected]> (cherry picked from commit e7a2725)
Start next development iteration towards lighty.io 20.1.0. Signed-off-by: Ivan Hrasko <[email protected]>
JIRA: LIGHTY-294 Signed-off-by: tobias.pobocik <[email protected]> (cherry picked from commit 76a31ff)
This timeout caused tests to be unreliable an inconsistent. JIRA: LIGHTY-299 Signed-off-by: tobias.pobocik <[email protected]> (cherry picked from commit 70346b0)
The addCallback methods do not need to be implemented since they are default. Also change the future method to return completed future since it's not chained. JIRA: LIGHTY-298 Signed-off-by: tobias.pobocik <[email protected]> (cherry picked from commit 24c3dbe)
Just use .namespace() to return namespace, and if namespace is null, just use name. JIRA: LIGHTY-298 Signed-off-by: tobias.pobocik <[email protected]> (cherry picked from commit 04d6fcd)
No need to import these classes when ImmutableNodes can beused for same purpose. JIRA: LIGHTY-298 Signed-off-by: tobias.pobocik <[email protected]> Signed-off-by: Ivan Hrasko <[email protected]> (cherry picked from commit 8b7fe7f)
This service is no longer required as a part of this rework: opendaylight/controller@9917911 JIRA: LIGHTY-298 Signed-off-by: tobias.pobocik <[email protected]> (cherry picked from commit 33b9022)
Use these changes to modernize RPC implementation registration: opendaylight/lispflowmapping@27c0004 JIRA: LIGHTY-298 Signed-off-by: tobias.pobocik <[email protected]> (cherry picked from commit 57e185e)
There is no need to bind it again, since it is already bound from DOMSchemaService. JIRA: LIGHTY-298 Signed-off-by: tobias.pobocik <[email protected]> (cherry picked from commit 600b033)
Reformat the code a little to improve its readability. JIRA: LIGHTY-298 Signed-off-by: tobias.pobocik <[email protected]> (cherry picked from commit 6f16e5e)
Adopt: - odlparent-13.1.2 - infrautils-6.0.8 - yangtools-13.0.5 - mdsal-13.0.3 - controller-9.0.3 - aaa-0.19.4 - netconf-7.0.5 - bgpcep-0.21.5 JIRA: LIGHTY-302 Signed-off-by: tobias.pobocik <[email protected]> (cherry picked from commit 85d16a5)
Rename these components according to: opendaylight/mdsal@c4c6619 JIRA: LIGHTY-302 Signed-off-by: tobias.pobocik <[email protected]> (cherry picked from commit f7bdd24)
The usage of TokenStore was removed with OAuth2 implementations. https://git.opendaylight.org/gerrit/c/aaa/+/104968 JIRA: LIGHTY-302 Signed-off-by: tobias.pobocik <[email protected]> (cherry picked from commit 152d4e5)
Introduce these parameters as a result of: opendaylight/netconf@4e2e5be#diff-8eddc437230f04d9d277415bdf718dd998114e916aea8bfcba5a2f9bb69e2174R72 and: opendaylight/netconf@2bb1482#diff-8eddc437230f04d9d277415bdf718dd998114e916aea8bfcba5a2f9bb69e2174R71 Let's currently just hardcode these parameters and resolve their configuration in LIGHTY-305. JIRA: LIGHTY-302 Signed-off-by: tobias.pobocik <[email protected]> (cherry picked from commit 9719f72)
Adjust imports based on: opendaylight/netconf@68f748b JIRA: LIGHTY-302 Signed-off-by: tobias.pobocik <[email protected]> (cherry picked from commit 7a604dc)
One constructor of this class got removed, lets just use a workaround for this test where the localRpcs parameter is not required: opendaylight/netconf@b372a54#diff-7de3450feabdc63b9c2b21ce40182a3d784e788ae85c31927a36615f2e24080eL66 JIRA: LIGHTY-302 Signed-off-by: tobias.pobocik <[email protected]> (cherry picked from commit 1a034c6)
This reverts commit 48ce203. This issue was resolved in netconf 7.0.5. JIRA: LIGHTY-304 Signed-off-by: tobias.pobocik <[email protected]> (cherry picked from commit 3d830d5)
Comment these tests out until YANGTOOLS-1575 is resolved. JIRA: LIGHTY-303 Signed-off-by: tobias.pobocik <[email protected]> (cherry picked from commit c1c5286)
https://checkstyle.org/releasenotes.html#Release_10.16.0 JIRA: LIGHTY-304 Signed-off-by: tobias.pobocik <[email protected]> (cherry picked from commit fc94ad8)
https://issues.apache.org/jira/secure/ReleaseNote.jspa?projectId=12317524&version=12353548 JIRA: LIGHTY-304 Signed-off-by: tobias.pobocik <[email protected]> (cherry picked from commit b4f87c1)
https://issues.apache.org/jira/secure/ReleaseNote.jspa?projectId=12317526&version=12354551 JIRA: LIGHTY-304 Signed-off-by: tobias.pobocik <[email protected]> (cherry picked from commit 84b0827)
https://github.com/testng-team/testng/releases/tag/7.10.2 Signed-off-by: Ivan Hrasko <[email protected]> (cherry picked from commit 1780357)
Adopt: odlparent-13.1.3 infrautils-6.0.9 yangtools-13.0.6 mdsal-13.0.4 JIRA: LIGHTY-309 Signed-off-by: Ivan Hrasko <[email protected]> (cherry picked from commit 19e85c3)
https://mvnrepository.com/artifact/org.springframework.boot/spring-boot-dependencies JIRA: LIGHTY-306 Signed-off-by: tobias.pobocik <[email protected]> (cherry picked from commit 002b153)
Snyk has created this PR to upgrade io.grpc:grpc-netty from 1.63.0 to 1.64.0. https://github.com/grpc/grpc-java/releases/tag/v1.64.0 Signed-off-by: Ivan Hrasko <[email protected]> (cherry picked from commit 0b36799)
https://mvnrepository.com/artifact/org.springframework.boot/spring-boot-dependencies/3.3.0 JIRA: LIGHTY-303 Signed-off-by: tobias.pobocik <[email protected]> (cherry picked from commit 46424a9)
https://github.com/spotbugs/spotbugs/releases/tag/4.8.4 https://github.com/spotbugs/spotbugs/releases/tag/4.8.5 https://github.com/spotbugs/spotbugs-maven-plugin/releases/tag/spotbugs-maven-plugin-4.8.4.0 https://github.com/spotbugs/spotbugs-maven-plugin/releases/tag/spotbugs-maven-plugin-4.8.5.0 JIRA: LIGHTY-303 Signed-off-by: tobias.pobocik <[email protected]> (cherry picked from commit a598163)
https://github.com/mojohaus/build-helper-maven-plugin/releases/tag/3.6.0 JIRA: LIGHTY-303 Signed-off-by: tobias.pobocik <[email protected]> (cherry picked from commit a577039)
Snyk has created this PR to upgrade io.grpc:grpc-netty from 1.69.0 to 1.69.1 https://github.com/grpc/grpc-java/releases/tag/v1.69.1 Signed-off-by: tobias.pobocik <[email protected]> (cherry picked from commit 6dd4bcf)
Snyk has created this PR to upgrade com.google.protobuf:protobuf-java from 3.25.5 to 3.25.6. https://github.com/protocolbuffers/protobuf/releases/tag/v3.25.6 Signed-off-by: Ivan Hrasko <[email protected]> (cherry picked from commit ac1aa11)
Snyk has created this PR to upgrade io.grpc:grpc-netty from 1.69.1 to 1.70.0. https://github.com/grpc/grpc-java/releases/tag/v1.70.0 Signed-off-by: Ivan Hrasko <[email protected]> (cherry picked from commit 6656ea5)
Snyk has created this PR to upgrade org.casbin:jcasbin from 1.78.0 to 1.79.0. https://github.com/casbin/jcasbin/releases/tag/v1.79.0 Signed-off-by: Ivan Hrasko <[email protected]> (cherry picked from commit 1b9a7d2)
Snyk has created this PR to upgrade org.slf4j:slf4j-api from 2.0.16 to 2.0.17. https://www.slf4j.org/news.html#2.0.17 Signed-off-by: tobias.pobocik <[email protected]> (cherry picked from commit b9c33cc)
Snyk has created this PR to upgrade org.casbin:jcasbin from 1.79.0 to 1.80.0. https://github.com/casbin/jcasbin/releases/tag/v1.80.0 Signed-off-by: tobias.pobocik <[email protected]> (cherry picked from commit 5ecc888)
https://github.com/spring-projects/spring-boot/releases/tag/v3.3.9 Signed-off-by: tobias.pobocik <[email protected]> (cherry picked from commit d0f0f49)
This issue was detected by sonarcloud. Resolve it by sanitising the LOG message using StringEscapeUtils. JIRA: LIGHTY-362 Signed-off-by: tobias.pobocik <[email protected]> (cherry picked from commit 5e19c91)
https://github.com/spring-projects/spring-boot/releases/tag/v3.3.10 Signed-off-by: tobias.pobocik <[email protected]> (cherry picked from commit 0401fad)
Snyk has created this PR to upgrade org.casbin:jcasbin from 1.80.0 to 1.81.0. https://github.com/casbin/jcasbin/releases/tag/v1.81.0 Signed-off-by: tobias.pobocik <[email protected]> (cherry picked from commit c96411a)
Snyk has created this PR to upgrade io.grpc:grpc-netty from 1.70.0 to 1.71.0. https://github.com/grpc/grpc-java/releases/tag/v1.71.0 Signed-off-by: tobias.pobocik <[email protected]> (cherry picked from commit a5ac897)
https://github.com/spring-projects/spring-boot/releases/tag/v3.3.11 Signed-off-by: tobias.pobocik <[email protected]> (cherry picked from commit f9bbaa1)
Snyk has created this PR to upgrade commons-io:commons-io from 2.18.0 to 2.19.0. https://commons.apache.org/proper/commons-io/changes.html#a2.19.0 Signed-off-by: tobias.pobocik <[email protected]> (cherry picked from commit 82a5ea7)
Use the latest minikube setup and bump minikube and kubernetes accordingly. https://github.com/manusa/actions-setup-minikube/releases/tag/v2.14.0 JIRA: LIGHTY-367 Signed-off-by: tobias.pobocik <[email protected]> (cherry picked from commit 33f8f6f)
Snyk has created this PR to upgrade io.grpc:grpc-netty from 1.71.0 to 1.72.0. https://github.com/grpc/grpc-java/releases/tag/v1.72.0 Signed-off-by: tobias.pobocik <[email protected]> (cherry picked from commit 50f47ae)
Snyk has created this PR to upgrade com.google.protobuf:protobuf-java from 3.25.6 to 3.25.7. https://github.com/protocolbuffers/protobuf/releases/tag/v3.25.7 Signed-off-by: tobias.pobocik <[email protected]> (cherry picked from commit 89f6772)
Snyk has created this PR to upgrade org.apache.commons:commons-collections4 from 4.4 to 4.5.0. https://commons.apache.org/proper/commons-collections/changes.html#a4.5.0 Signed-off-by: tobias.pobocik <[email protected]> (cherry picked from commit 7dee18e)
Update the Dockerfile to use JSON array syntax to ensure the entrypoint works properly. Also use uppercase AS and FROM to resolve warnings. https://hackernoon.com/fix-dockerfile-error-warn-fromascasing-as-and-from-keywords-casing-do-not-match https://docs.docker.com/reference/build-checks/json-args-recommended/ This resolves previous issues where Controller could not connect to simulator. JIRA: LIGHTY-367 Signed-off-by: tobias.pobocik <[email protected]> (cherry picked from commit 78efdc2)
Running netconf-simulator using docker while running controller using kubernetes created all sorts of networking separation issues. Running both of them using kubernetes makes the code more robust and reduces the chance of experiencing networking separation issues. JIRA: LIGHTY-367 Signed-off-by: tobias.pobocik <[email protected]> (cherry picked from commit f3d0f6d)
https://github.com/spring-projects/spring-boot/releases/tag/v3.3.12 Signed-off-by: tobias.pobocik <[email protected]> (cherry picked from commit cbb2c66)
https://github.com/spring-projects/spring-boot/releases/tag/v3.3.13 Signed-off-by: Ivan Hrasko <[email protected]> (cherry picked from commit 70f5bfa)
Snyk has created this PR to upgrade org.apache.logging.log4j:log4j-api from 2.24.3 to 2.25.0. https://github.com/apache/logging-log4j2/releases/tag/rel%2F2.25.0 Signed-off-by: tobias.pobocik <[email protected]> (cherry picked from commit b062989)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Summary of Changes
Hello @tokarenko, 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 addresses a specific issue where gNMI responses lacking explicit namespaces were not being correctly parsed by the system. It introduces robust logic to detect and automatically add the necessary namespace information to such responses, ensuring their proper conversion into a normalized node representation for further processing.
Highlights
- Improved gNMI Response Parsing: Enhanced the
GetResponseToNormalizedNodeCodecto correctly handle gNMI responses that do not include namespace prefixes in their JSON structure, preventing parsing failures. - Namespace Auto-Correction: Implemented new functionality to detect if a gNMI JSON response is missing a namespace and, if so, to dynamically add the correct module namespace based on the requested identifier, ensuring proper data interpretation.
- Code Refactoring: Refactored the
isResponseJsonDeeperThanRequestedmethod to improve efficiency by parsing the response JSON into aJsonElementonce and passing it directly, avoiding redundant string parsing.
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 in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| 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 issue 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 is currently in preview and 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 to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
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. ↩
There was a problem hiding this 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 fixes an issue with parsing gNMI responses lacking namespaces. The changes introduce logic to detect such responses and prepend the appropriate module name as a namespace to the JSON keys. There are potential issues in the new helper methods that could lead to runtime exceptions and data loss.
.../main/java/io/lighty/gnmi/southbound/mountpoint/codecs/GetResponseToNormalizedNodeCodec.java
Outdated
Show resolved
Hide resolved
.../main/java/io/lighty/gnmi/southbound/mountpoint/codecs/GetResponseToNormalizedNodeCodec.java
Outdated
Show resolved
Hide resolved
.../main/java/io/lighty/gnmi/southbound/mountpoint/codecs/GetResponseToNormalizedNodeCodec.java
Outdated
Show resolved
Hide resolved
.../main/java/io/lighty/gnmi/southbound/mountpoint/codecs/GetResponseToNormalizedNodeCodec.java
Outdated
Show resolved
Hide resolved
.../main/java/io/lighty/gnmi/southbound/mountpoint/codecs/GetResponseToNormalizedNodeCodec.java
Outdated
Show resolved
Hide resolved
.../main/java/io/lighty/gnmi/southbound/mountpoint/codecs/GetResponseToNormalizedNodeCodec.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
See #2197