Add ni-tls-config integration for server TLS configuration#1249
Merged
texasaggie97 merged 14 commits intoni:mainfrom Apr 1, 2026
Merged
Conversation
Signed-off-by: Mark Silva <mark.silva@emerson.com>
Signed-off-by: Mark Silva <mark.silva@emerson.com>
Signed-off-by: Mark Silva <mark.silva@emerson.com>
Signed-off-by: Mark Silva <mark.silva@emerson.com>
texasaggie97
requested review from
astarche,
maxxboehme and
reckenro
as code owners
Signed-off-by: Mark Silva <mark.silva@emerson.com>
dmondrik
approved these changes
Mar 30, 2026
When security_mode is 'ni-tls-config', the 'security' key in the JSON config is a string (not an object), so parse_server_cert/key/root_cert all return empty strings. Constructing ServerSecurityConfiguration with empty strings produces insecure credentials before they are overwritten. Restructure to default-construct server_security_config and assign it in the correct branch (ni-tls-config or cert-based), ensuring insecure credentials are never constructed unnecessarily. Signed-off-by: Mark Silva <texasaggie97@gmail.com>
Avoid splitting the declaration of listeningPort from its assignment in AddListeningPort by declaring it immediately above that call, after the ServerSecurityConfiguration block. Signed-off-by: Mark Silva <texasaggie97@gmail.com>
Contributor
Author
|
I do not believe the failed test is due to my changes, but rerunning it is not an option for me. Can one of the maintainers re-run it? |
Collaborator
Yeah, I think it's just an intermittent test. I kicked off the test runs again. Also, added you to https://github.com/orgs/ni/teams/grpc-device-developers which has write permissions to this repo. |
reckenro
reviewed
Mar 31, 2026
reckenro
reviewed
Mar 31, 2026
reckenro
reviewed
Mar 31, 2026
astarche
approved these changes
Mar 31, 2026
reckenro
reviewed
Mar 31, 2026
reckenro
reviewed
Mar 31, 2026
reckenro
reviewed
Mar 31, 2026
reckenro
approved these changes
Mar 31, 2026
Drop the 'ni_' prefix to match the convention used by other wrappers in this repo (e.g. nisyscfg). Files renamed: source/server/ni_tls_config_loader.h -> tls_config_loader.h source/server/ni_tls_config_loader.cpp -> tls_config_loader.cpp source/tests/unit/ni_tls_config_loader_tests.cpp -> tls_config_loader_tests.cpp Class NiTlsConfigLoader -> TlsConfigLoader. All references in CMakeLists.txt and core_server.cpp updated accordingly. Also converts the typedef function pointer declarations to the 'using' style consistent with the rest of the codebase. Signed-off-by: Mark Silva <texasaggie97@gmail.com>
When 'security' is the string 'ni-tls-config', parse_server_cert/key/ root_cert would call parse_key_from_security_section() on a non-object JSON value. While nlohmann::json handles this gracefully (find() on a string returns end()), the behavior is confusing and relies on an implementation detail. Fix by moving parse_security_mode() before the cert calls and guarding parse_server_cert/key/root_cert so they are skipped entirely when the mode is 'ni-tls-config'. Add a unit test documenting the graceful parser behavior when 'security' is a string value. Signed-off-by: Mark Silva <texasaggie97@gmail.com>
Resolves two previously unused API entry points: nitlsconfig_server_readClientMode nitlsconfig_server_readTrustedCertificatesLocation When the client mode is anything other than Disabled, read the trusted certificates location and pass it as root_cert to ServerSecurityConfiguration, enabling mutual TLS. When client mode is Disabled, root_cert remains empty (server-side TLS only). Also adds ClientMode enum mirroring nitlsconfig_server_client_mode_* constants, ReadClientModeFunc type alias, and the corresponding member variables. Both new function pointers are now required for is_available() to return true. Signed-off-by: Mark Silva <texasaggie97@gmail.com>
Replace the single if-check for CertificateMode_Disabled with a switch statement that explicitly handles all known enum values: - Disabled -> return insecure credentials - Unmanaged - ManagedSelfSigned -> proceed to load cert files - Unknown/default -> throw std::runtime_error with the mode value This ensures unrecognised or future certificate modes fail loud rather than silently attempting to read certificate locations. Signed-off-by: Mark Silva <texasaggie97@gmail.com>
reckenro
reviewed
Mar 31, 2026
Add 'ni-tls-config' as a kNextRelease feature toggle. parse_feature_toggles()
is now called first in GetConfiguration so that subsequent parsing decisions
can depend on it, following the recommended pattern.
parse_security_mode() and the ni-tls-config branch in RunServer are both
guarded by is_feature_enabled('ni-tls-config', kNextRelease). When the
toggle is absent or disabled, security_mode stays empty and the server
behaves exactly as before.
To enable during testing or deployment:
{ 'feature_toggles': { 'ni-tls-config': true }, 'security': 'ni-tls-config' }
Or use FeatureToggles({}, CodeReadiness::kNextRelease) in test code.
Signed-off-by: Mark Silva <texasaggie97@gmail.com>
In the else branch of the security mode dispatch in RunServer, add an explicit check: if security_mode is non-empty but not 'ni-tls-config', log an error and exit rather than silently ignoring the unknown value and falling back to cert-based TLS. Only an empty security_mode (the default when the key is absent or the feature toggle is off) continues to the cert-based ServerSecurityConfiguration path as before. Signed-off-by: Mark Silva <texasaggie97@gmail.com>
Add source/config/ni_tls_config.json as an example configuration file showing the ni-tls-config security mode alongside the other example configs in that folder. Includes the feature_toggles key required to enable the ni-tls-config mode. Update the NI TLS Config Integration section of README.md to show the feature_toggles key in the example JSON, matching the actual required configuration. Signed-off-by: Mark Silva <texasaggie97@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this Pull Request accomplish?
Adds support for delegating TLS configuration to the
ni-tls-configpackage by introducing a new"security": "ni-tls-config"mode inserver_config.json. When this mode is set, the server dynamically loadsnitlsconfig.dll/libnitlsconfig.soat startup and reads per-service TLS settings from it, rather than requiring certificate paths to be specified directly in the config file.Changes include:
NiTlsConfigLoader: a new class that dynamically loads theni-tls-configlibrary and resolves TLS credentials for a named service usingnierr_Statusfor error handling (consistent with the rest of the repo).ServerConfigurationParser::parse_security_mode(): reads the top-level"security"string value from the config file.core_server: wires the new mode into server startup; logs an error and exits cleanly ifni-tls-configis requested but not installed.ni-grpc-device.conf.yml: a template configuration file forni-tls-config, copied to the binary output directory alongside the server.ni-grpc-device.caps.yml: a capabilities declaration file forni-tls-config, copied to the binary output directory alongside the server.Why should this Pull Request be merged?
Allows the grpc-device server to participate in the NI-managed TLS certificate lifecycle provided by the
ni-tls-configpackage, without introducing a link-time dependency on that package. Ifni-tls-configis not installed the server continues to work exactly as before.What testing has been done?
UnitTestsRunner) all pass (40 tests).parse_security_mode(3 tests covering string mode, object-valued security key, and missing key) andNiTlsConfigLoader(2 tests covering library-not-installed behavior:is_available()returns false andget_server_credentials()throws rather than crashing).