initdata: migrate key release test cases to initdata #2006
Merged
Conversation
This file contains 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
|
Tested for libvirt provider against sample TEE: |
huoqifeng
force-pushed
the
e2e
branch
7 times, most recently
from
e7b28c8 to
3feaa88
Compare
|
I'll retest after the refactoring, I guess we can start reviewing also. |
huoqifeng
requested review from
bpradipt,
mkulke,
liudalibj,
stevenhorsman,
genjuro214 and
wainersm
|
Test and passed:
|
|
@mkulke may you help check whether the changes are OK for azure? |
There was a problem hiding this comment.
Generally it looks good to me. A couple of points that would make the review/future commit checks easier:
- Adding commit body explaining the why/how of the changes
- Squashing some of the commits together e.g. the current 4th commit and quite a lot of the 3rd commit, could be fixed up into the first commit for more clarity and grouping of the code.
Thanks for the great work!
Just re-thinking about it from a ux standpoint. Is it really needed to remove the support for AA_KBC_PARAMS ? |
@bpradipt any specific reason to keep |
That would be another configuration that we'd have to support and test. beyond a URI AA_KBC_PARAMs doesn't support certs or any current or future option for AA and CDH. I suspect AA_KBC_PARAMS will also be retired for kata-qemu (at least for cc_kbc). If we want to have a per-installation configuration for CAA to improve the UX, I suggest to (optionally) provide a default initdata.toml o via configmap, similar to the auth.json and use that one if no initdata is set as annotation on a pod. |
|
right, more config are adding in cdh.toml like |
|
@huoqifeng @mkulke thanks for clarifying. Providing default initData via configMap is a good option. |
8 tasks
It was not in my original plan, I added it as an item in #1985, hopefully, I can have time to do that. |
added 2 commits
August 20, 2024 14:46
- migrate key release test cases to initdata - remove AA_KBC_PARAMS and aaKBCParams - use allow-all rego policy to make key release test run correctly Fixes: confidential-containers#1985 Signed-off-by: Qi Feng Huo <[email protected]>
- add global-initdata in configmap and parameters Fixes: confidential-containers#1985 Signed-off-by: Qi Feng Huo <[email protected]>
@bpradipt the |
mkulke
reviewed
Aug 20, 2024
- rename GLOBAL_INITDATA to INITDATA - rename CdhFilePath to CDHConfigPath - rename AaFilePath to AAConfigPath Fixes: confidential-containers#1985 Signed-off-by: Qi Feng Huo <[email protected]>
I'll keep monitoring the PR in kata-qemu. kata-containers/kata-containers#10163 |
- Validate the initdata passed in both from configmap and annotation Fixes: confidential-containers#1985 Signed-off-by: Qi Feng Huo <[email protected]>
|
if invalid initdata used, the error looks like: |
bpradipt
reviewed
Aug 24, 2024
bpradipt
reviewed
Aug 24, 2024
bpradipt
reviewed
Sep 2, 2024
Fixes: confidential-containers#1985 Signed-off-by: Qi Feng Huo <[email protected]>
mkulke
reviewed
Sep 2, 2024
mkulke
approved these changes
Sep 2, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Fixes: #1985
AA_KBC_PARAMSandpkg/aa,pkg/cdhetc.AA_KBC_PARAMSINITDATAin cmpeer-pods-cmfor default initdata for every Pod, Annotation in Pod will overwrite the settings inINITDATA.because aa, cdh certificates might need kbs upgrade, so certificates and others support will be in separate PRs.