-
Notifications
You must be signed in to change notification settings - Fork 198
Enable the creation of mTLS encrypted gRPC channels leveraging NI TLS via nitlsconfig (Python) #1019
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
Open
alexdubois-ni
wants to merge
9
commits into
master
Choose a base branch
from
users/adubois/integrateNITLSConfigForGRPC
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Enable the creation of mTLS encrypted gRPC channels leveraging NI TLS via nitlsconfig (Python) #1019
Changes from all commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
1aa5cb6
Commit template as well
alexdubois-ni 71ca05a
Add the newly generated code as well
alexdubois-ni 38ec344
Consume latest nitlsconfig and run poetry lock as well
alexdubois-ni a11d222
Add the missing poetry.toml and unit tests
alexdubois-ni c2e3b6b
Fix and flesh out unit tests
alexdubois-ni b44a817
Update changelog
alexdubois-ni f51dcd1
Addback whitespaces, add nitlsconfig to CSpell exception and change t…
alexdubois-ni 7f16f82
Add a brief mention of what nitlsconfig is as well
alexdubois-ni bdf3b89
Add a comment as well to clarify
alexdubois-ni File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
|
alexdubois-ni marked this conversation as resolved.
alexdubois-ni marked this conversation as resolved.
bkeryan marked this conversation as resolved.
|
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
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
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
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,133 @@ | ||
| from __future__ import annotations | ||
|
|
||
| from unittest.mock import Mock | ||
|
|
||
| import pytest | ||
| from pytest_mock import MockerFixture | ||
|
|
||
| import nidaqmx | ||
| from nidaqmx import Task | ||
| from tests.unit._grpc_utils import create_grpc_options | ||
| from tests.unit._task_utils import expect_create_task, expect_get_task_name | ||
|
|
||
| # _FakeRpcError subclasses grpc.RpcError, so it must be defined here or importing this | ||
| # module raises NameError when the grpc extra isn't installed. | ||
| try: | ||
| import grpc | ||
| import nitlsconfig | ||
|
|
||
| from nidaqmx._grpc_interpreter import GrpcStubInterpreter | ||
|
|
||
| class _FakeRpcError(grpc.RpcError): | ||
| def __init__(self, code): | ||
| self._code = code | ||
|
|
||
| def code(self): | ||
| return self._code | ||
|
|
||
| def details(self): | ||
| return "original details" | ||
|
|
||
| def trailing_metadata(self): | ||
| return [] | ||
|
|
||
| except ImportError: | ||
| grpc = None # type: ignore | ||
|
|
||
|
|
||
| @pytest.fixture | ||
| def nitlsconfig_tagged_channel(): | ||
| """A gRPC channel tagged the way nitlsconfig.create_grpc_device_channel tags one. | ||
|
|
||
| create_grpc_device_channel needs the nitlsconfig CLI installed on the system, so tagging a | ||
| plain channel is the closest we can get to one here. Nothing connects over it. | ||
| """ | ||
| if grpc is None: | ||
| pytest.skip("The grpc module is not available.") | ||
| target = "localhost:31763" | ||
| with grpc.insecure_channel(target) as channel: | ||
| nitlsconfig.channel_tag.tag_channel_target(channel, target) | ||
| yield channel | ||
|
|
||
|
|
||
| def _create_interpreter(mocker: MockerFixture, grpc_options, version_error=None): | ||
| mocker.patch("nidaqmx._grpc_interpreter.nidaqmx_grpc.NiDAQmxStub", autospec=True) | ||
| mocker.patch.object( | ||
| GrpcStubInterpreter, | ||
| "get_system_info_attribute_uint32", | ||
| side_effect=version_error, | ||
| return_value=None if version_error else 1, | ||
| ) | ||
| return GrpcStubInterpreter(grpc_options) | ||
|
|
||
|
|
||
| def test___untagged_channel___handle_unavailable___raises_failed_to_connect( | ||
| mocker: MockerFixture, | ||
| ): | ||
| interpreter = _create_interpreter(mocker, create_grpc_options(mocker)) | ||
|
|
||
| with pytest.raises(nidaqmx.errors.RpcError) as exc_info: | ||
| interpreter._handle_rpc_error(_FakeRpcError(grpc.StatusCode.UNAVAILABLE)) | ||
|
|
||
| assert exc_info.value.rpc_code == grpc.StatusCode.UNAVAILABLE | ||
| assert exc_info.value.description == "Failed to connect to server" | ||
|
|
||
|
|
||
| def test___nitlsconfig_tagged_channel___handle_unavailable___raises_tls_elaboration( | ||
| mocker: MockerFixture, nitlsconfig_tagged_channel | ||
| ): | ||
| # Derived from nitlsconfig itself, so this fails if it stops recognizing our channel. | ||
| expected_message = nitlsconfig.get_tls_connection_error_elaboration(nitlsconfig_tagged_channel) | ||
| assert expected_message is not None | ||
| assert expected_message != "Failed to connect to server" | ||
| grpc_options = nidaqmx.GrpcSessionOptions(nitlsconfig_tagged_channel, "") | ||
| interpreter = _create_interpreter(mocker, grpc_options) | ||
|
|
||
| with pytest.raises(nidaqmx.errors.RpcError) as exc_info: | ||
| interpreter._handle_rpc_error(_FakeRpcError(grpc.StatusCode.UNAVAILABLE)) | ||
|
|
||
| assert exc_info.value.rpc_code == grpc.StatusCode.UNAVAILABLE | ||
| assert exc_info.value.description == expected_message | ||
|
|
||
|
|
||
| def test___nitlsconfig_tagged_channel___handle_other_status_code___preserves_original_details( | ||
| mocker: MockerFixture, nitlsconfig_tagged_channel | ||
| ): | ||
| # Tagged, so an elaboration is available: this fails if we stop limiting it to UNAVAILABLE. | ||
| grpc_options = nidaqmx.GrpcSessionOptions(nitlsconfig_tagged_channel, "") | ||
| interpreter = _create_interpreter(mocker, grpc_options) | ||
|
|
||
| with pytest.raises(nidaqmx.errors.RpcError) as exc_info: | ||
| interpreter._handle_rpc_error(_FakeRpcError(grpc.StatusCode.INTERNAL)) | ||
|
|
||
| assert exc_info.value.rpc_code == grpc.StatusCode.INTERNAL | ||
| assert exc_info.value.description == "original details" | ||
|
|
||
|
|
||
| def test___server_reachable___create_interpreter___audits_connected(mocker: MockerFixture): | ||
| patched_audit = mocker.patch("nitlsconfig.audit_session_connect", autospec=True) | ||
| grpc_options = create_grpc_options(mocker) | ||
|
|
||
| _create_interpreter(mocker, grpc_options) | ||
|
|
||
| patched_audit.assert_called_once_with("NI-DAQmx", grpc_options.grpc_channel, True) | ||
|
|
||
|
|
||
| def test___server_unreachable___create_interpreter___audits_not_connected(mocker: MockerFixture): | ||
| patched_audit = mocker.patch("nitlsconfig.audit_session_connect", autospec=True) | ||
| grpc_options = create_grpc_options(mocker) | ||
|
|
||
| _create_interpreter(mocker, grpc_options, version_error=Exception("unreachable")) | ||
|
|
||
| patched_audit.assert_called_once_with("NI-DAQmx", grpc_options.grpc_channel, False) | ||
|
|
||
|
|
||
| def test___no_grpc_options___create_task___does_not_audit(interpreter: Mock, mocker: MockerFixture): | ||
| patched_audit = mocker.patch("nitlsconfig.audit_session_connect", autospec=True) | ||
| expect_create_task(interpreter) | ||
| expect_get_task_name(interpreter, "MyTask") | ||
|
|
||
| with Task("MyTask"): | ||
| pass | ||
|
|
||
| patched_audit.assert_not_called() |
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.
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.
Does this require any new software to be installed for existing non-TLS use cases? I see that the nitlsconfig Python package shells out to a new
nitlsconfigcommand: https://github.com/ni/nitlsconfig-python/blob/main/src/nitlsconfig/cli.pyI saw the comment about "create_grpc_device_channel needs the nitlsconfig CLI installed on the system." I just want to check that the auditing part works without the nitlsconfig CLI. Otherwise, this will break existing applications with older drivers that install the latest
nidaqmxPython package.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 see the system tests passed, which is a good sign, but I'm not sure which version of NI-DAQmx we're using for testing or whether it includes the
nitlsconfigcommand.