vmsdk/python/tests: add tests for TDX #57
Conversation
dfe1870 to
afe6941
Compare
vmsdk/python/tests/tdx/test_sdk.py
Outdated
|
|
||
| def test_get_eventlog(self): | ||
| """Test get_eventlog result.""" | ||
| #TODO: verify the eventlog value. |
There was a problem hiding this comment.
Please remove the test if not implemented yet, otherwise the blank test will fail to run, correct?
There was a problem hiding this comment.
Please let me remove it to avoid confusion (although it doesn't fail the test). I'll add them back when the TODO is done. Thanks!
vmsdk/python/tests/tdx/test_sdk.py
Outdated
| rtmrs[event.imr_index] = sha384_algo.digest() | ||
| return rtmrs | ||
|
|
||
| def check_imr(self, imr_index: int, alg_id: int, rtmr: bytes): |
There was a problem hiding this comment.
If it is not a test case with the prefix of "test_", then please add "_" to indicate it as an internal function.
vmsdk/python/tests/tdx/test_sdk.py
Outdated
| will measure boot_aggregate by default. | ||
| """ | ||
|
|
||
| RTMR_LENGTH_BY_BYTES = 48 |
There was a problem hiding this comment.
why this is not a constant value from cctrusted_base package?
vmsdk/python/tests/tdx/test_sdk.py
Outdated
| assert cmdline is not None | ||
|
|
||
| ima_policy = None | ||
| if "ima_hash=sha384" in cmdline: |
There was a problem hiding this comment.
why measurement is related to ima_policy? @ruomengh
There was a problem hiding this comment.
This is my misunderstanding. I'll resolve this and update PR.
vmsdk/python/tests/tdx/test_sdk.py
Outdated
| count = CCTrustedVmSdk.inst().get_measurement_count() | ||
| assert count == TestCCTrustedVmSdkTdx.MEASUREMENT_COUNT | ||
|
|
||
| def replay_eventlog(self): |
There was a problem hiding this comment.
please add "_" as the prefix for internal function.
OK, I'll resolve the issue and update this PR per review feedbacks. |
36cba8e to
a587769
Compare
| def get_quote(self, nonce: bytearray, data: bytearray, extraArgs=None) -> Quote: | ||
| return self._cvm.imrs[imr_index] | ||
|
|
||
| def get_quote( |
There was a problem hiding this comment.
why change to this style, looks wierd.
There was a problem hiding this comment.
After adding default values for parameters "nonce" and "data", the line became too long. So I followed the https://google.github.io/styleguide/pyguide.html#3192-line-breaking 3.19.2 Line Breaking for that style.
9529220 to
704fe53
Compare
59b9598 to
ff0b6ab
Compare
Please review the latest version which has the changes per review. Thanks! |
vmsdk/python/tests/run.py
Outdated
| if args.no_cacheprovider is True: | ||
| pytest_options += ["-p", "no:cacheprovider"] | ||
|
|
||
| cc_type = ConfidentialVM.detect_cc_type() |
There was a problem hiding this comment.
@dongx1x @ruomengh @intelzhongjie do you have better approach to make filter in conftest?
I know you already add mark.basic mark.tdx. But it looks wierd...
There was a problem hiding this comment.
I thought about this for a while. A rough idea on one alternative might be to specify the type of confidential VM we want to test and start that explicitly. It needs some change on the existing workflow (currently the test will be run in existing VM directly, we need to change this to that start VM and run the tests).
The current way leverages the existing running confidential VM as the action runner. So it needs to detect the type of that existing VM where it runs to decide if it needs to run TDX specific tests.
There was a problem hiding this comment.
Discussed with Ruomeng and Xiaocheng, I'll modify and update this per feedbacks.
6154b91 to
4cc0cc4
Compare
This patch mainly adds some tests for TDX. And it refactors some corresponding code accordingly. Signed-off-by: zhongjie <zhongjie.shi@intel.com>
No description provided.