-
Notifications
You must be signed in to change notification settings - Fork 713
Update openconfig-system-grpc.yang #1531
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: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -23,10 +23,17 @@ module openconfig-system-grpc { | |||||
| to be included in the list."; | ||||||
|
|
||||||
|
|
||||||
| oc-ext:openconfig-version "1.1.0"; | ||||||
| oc-ext:openconfig-version "1.2.0"; | ||||||
| oc-ext:catalog-organization "openconfig"; | ||||||
| oc-ext:origin "openconfig"; | ||||||
|
|
||||||
| revision "2026-08-03" { | ||||||
| description | ||||||
| "Add support for mutual TLS, keepalive settings, and | ||||||
| max concurrent streams configuration on gRPC servers."; | ||||||
| reference "1.2.0"; | ||||||
| } | ||||||
|
|
||||||
| revision "2024-05-29" { | ||||||
| description | ||||||
| "Add support for gRPC connections."; | ||||||
|
|
@@ -104,6 +111,7 @@ module openconfig-system-grpc { | |||||
| "Operational state relating to the gRPC service."; | ||||||
| uses grpc-server-config; | ||||||
| } | ||||||
| uses grpc-server-keepalive-top; | ||||||
| uses connections-top; | ||||||
| } | ||||||
| } | ||||||
|
|
@@ -167,6 +175,18 @@ module openconfig-system-grpc { | |||||
| are not supported, such as lab testing."; | ||||||
| } | ||||||
|
|
||||||
| leaf tls-mutual { | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In two of the referenced implementations (Nokia and Arista), mTLS is configured exclusively at the TLS-profile level. Although a case can be made for placing it at the per-server level, I don’t see any rationale for choosing this approach in the description. |
||||||
| type boolean; | ||||||
| default false; | ||||||
| must "not(../transport-security) or ../transport-security = 'true'" { | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This might be the time to question This should be able to be conveyed with just:
Suggested change
However default=true is imo problematic in reality |
||||||
| error-message "Mutual TLS requires transport-security to be enabled."; | ||||||
| } | ||||||
| description | ||||||
| "When set to true, mutual TLS (mTLS) client certificate verification | ||||||
| is enforced on the gRPC server. The client must present a certificate | ||||||
| validated against the trusted certificate authority."; | ||||||
| } | ||||||
|
|
||||||
| leaf certificate-id { | ||||||
| type string; | ||||||
| description | ||||||
|
|
@@ -184,6 +204,15 @@ module openconfig-system-grpc { | |||||
| https://github.com/openconfig/reference/blob/master/rpc/gnmi/gnmi-authentication.md#credentials-and-authentication."; | ||||||
| } | ||||||
|
|
||||||
| leaf max-concurrent-streams { | ||||||
| type uint32 { | ||||||
| range "1..max"; | ||||||
| } | ||||||
| description | ||||||
| "Maximum number of concurrent streams permitted per gRPC session | ||||||
| or connection."; | ||||||
| } | ||||||
|
|
||||||
| leaf-list listen-addresses { | ||||||
| type union { | ||||||
| type oc-inet:ip-address; | ||||||
|
|
@@ -208,6 +237,70 @@ module openconfig-system-grpc { | |||||
| } | ||||||
| } | ||||||
|
|
||||||
| grouping grpc-server-keepalive-config { | ||||||
| description | ||||||
| "Configuration parameters relating to gRPC server keepalive."; | ||||||
|
|
||||||
| leaf time { | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
| type uint32 { | ||||||
| range "1..max"; | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This probably can start at 0 (where |
||||||
| } | ||||||
| units "seconds"; | ||||||
| description | ||||||
| "Interval in seconds at which the server will send keepalive PING | ||||||
| frames to the client to verify connection health when no activity | ||||||
| is detected."; | ||||||
| } | ||||||
|
|
||||||
| leaf min-keepalive-interval { | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Already under |
||||||
| type uint32 { | ||||||
| range "1..max"; | ||||||
| } | ||||||
| units "seconds"; | ||||||
| description | ||||||
| "Minimum interval in seconds that the server permits between client | ||||||
| keepalive PING frames. If a client sends PING frames more frequently | ||||||
| than this interval, the server will send a GOAWAY frame with | ||||||
| 'too_many_pings' and close the connection."; | ||||||
| } | ||||||
|
|
||||||
| leaf timeout { | ||||||
| type uint32 { | ||||||
| range "1..max"; | ||||||
| } | ||||||
| units "seconds"; | ||||||
| must "not(../time) or . < ../time" { | ||||||
| error-message "Keepalive timeout must be less than the keepalive time interval."; | ||||||
| } | ||||||
| description | ||||||
| "Time in seconds the server waits for a keepalive PING response | ||||||
| before closing the connection."; | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| grouping grpc-server-keepalive-top { | ||||||
| description | ||||||
| "Structural grouping for gRPC keepalive parameters."; | ||||||
|
|
||||||
| container keepalive { | ||||||
| description | ||||||
| "Parameters for gRPC keepalive probes and rate limits."; | ||||||
|
|
||||||
| container config { | ||||||
| description | ||||||
| "Configuration parameters for gRPC keepalive."; | ||||||
| uses grpc-server-keepalive-config; | ||||||
| } | ||||||
|
|
||||||
| container state { | ||||||
| config false; | ||||||
| description | ||||||
| "Operational state parameters for gRPC keepalive."; | ||||||
| uses grpc-server-keepalive-config; | ||||||
| } | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| grouping grpc-counters { | ||||||
| description | ||||||
| "Top-level container for gRPC counters."; | ||||||
|
|
||||||
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.
i would add one more container "tls" under config so that we can extend it to add ssl/tls-profile(certz reference) and other TLS specific configs which can help me control version, ciphers, signature algorithms etc. Please check the feasibility of this approach
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.
@navaneethyv What do you think?
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.
As it stands, this leaf is not enough for mutual auth. The schema needs to be mindful of both any gNSI approaches as well as schema-driven profiles/references (the latter being the predominant deployment across secured gRPC transports today)
Agree that TLS is in need of its own hierarchy to consolidate all TLS related parameters
e.g. JUNOS/EVO