Skip to content
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

packaging: Add IBM Z SE artifacts to main #6755

Merged
merged 12 commits into from Dec 8, 2023

Conversation

BbolroC
Copy link
Member

@BbolroC BbolroC commented Apr 28, 2023

This is to add artifacts for IBM Z SE(TEE) to main.

This PR MUST be merged after #6586.

Fixes: #6754

Signed-off-by: Hyounggyu Choi Hyounggyu.Choi@ibm.com

@katacontainersbot katacontainersbot added the size/tiny Smallest and simplest task label Apr 28, 2023
@BbolroC BbolroC force-pushed the add-se-artifacts-to-main branch 2 times, most recently from 6a43d6e to c92c911 Compare April 28, 2023 10:52
@katacontainersbot katacontainersbot added size/huge Largest and most complex task (probably needs breaking into small pieces) and removed size/tiny Smallest and simplest task labels Apr 28, 2023
ci/lib.sh Outdated Show resolved Hide resolved
ci/lib.sh Outdated Show resolved Hide resolved
ci/lib.sh Outdated Show resolved Hide resolved
@wainersm
Copy link
Contributor

wainersm commented May 1, 2023

hi @BbolroC , thanks for helping on the CCv0 -> main merge!

I left a few comments. Overall it looks good but it would be better if you could break it into smaller commits. Could you do that?

Copy link
Contributor

@fitzthum fitzthum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few comments

src/runtime/config/configuration-qemu-se.toml.in Outdated Show resolved Hide resolved
ci/lib.sh Outdated Show resolved Hide resolved
.github/workflows/build-kata-static-tarball-s390x.yaml Outdated Show resolved Hide resolved
ci/lib.sh Outdated Show resolved Hide resolved
tools/packaging/guest-image/build_se_image.sh Outdated Show resolved Hide resolved
@BbolroC
Copy link
Member Author

BbolroC commented May 22, 2023

hi @BbolroC , thanks for helping on the CCv0 -> main merge!

I left a few comments. Overall it looks good but it would be better if you could break it into smaller commits. Could you do that?

Yes, I will follow your suggestion. Thanks a lot!

@katacontainersbot katacontainersbot added size/tiny Smallest and simplest task and removed size/huge Largest and most complex task (probably needs breaking into small pieces) labels May 22, 2023
@BbolroC BbolroC force-pushed the add-se-artifacts-to-main branch 2 times, most recently from edccb79 to b8a1b3f Compare May 22, 2023 14:26
@katacontainersbot katacontainersbot added size/huge Largest and most complex task (probably needs breaking into small pieces) and removed size/tiny Smallest and simplest task labels May 22, 2023
@BbolroC BbolroC force-pushed the add-se-artifacts-to-main branch 2 times, most recently from 278ab7f to 0d09bc7 Compare May 22, 2023 16:47
@BbolroC BbolroC changed the title packaging: Add se artifacts to main packaging: Add IBM Z SE artifacts to main May 22, 2023
Copy link
Contributor

@fitzthum fitzthum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few more comments.

.github/workflows/build-kata-static-tarball-s390x.yaml Outdated Show resolved Hide resolved
src/runtime/Makefile Outdated Show resolved Hide resolved
@@ -54,6 +54,8 @@ docker run \
-v "${kata_dir}:${kata_dir}" \
--env CI="${CI:-}" \
--env USER=${USER} \
--env AA_KBC="${AA_KBC:-}" \
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we use the AA_KBC variable in main yet, but I guess this doesn't hurt.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the info.

src/runtime/config/configuration-qemu-se.toml.in Outdated Show resolved Hide resolved
tools/packaging/kata-deploy/local-build/Makefile Outdated Show resolved Hide resolved
@BbolroC BbolroC requested a review from a team as a code owner May 22, 2023 19:50
@BbolroC
Copy link
Member Author

BbolroC commented May 22, 2023

This PR should be merged after #6586 due to the cross-compilation.

@wainersm
Copy link
Contributor

On my workstation (x86_64) I rebased this PR on top of #6586 (enable cross build for non-x86) then I ran the following command to build the SE boot image:

CROSS_BUILD=true TARGET_ARCH=s390x ARCH=s390x make boot-image-se-tarball

It failed to build the kernel (pre-req of the boot image):

<OUTPUT OMITTED>

#7 naming to quay.io/kata-containers/builders:kernel-7b62aa69333117c07a066af8dd95c772fbeba5e1-x86_64-s390x-cross-build 0.0s done
#7 DONE 2.1s
INFO: Config version: 106
INFO: Kernel version: 5.19.2
INFO: kernel path does not exist, will download kernel
INFO: Download kernel checksum file: sha256sums.asc from https://cdn.kernel.org/pub/linux/kernel/v5.x/sha256sums.asc
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100  292k  100  292k    0     0   158k      0  0:00:01  0:00:01 --:--:--  158k
INFO: Download kernel version 5.19.2
INFO: Download kernel
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100   162  100   162    0     0    115      0  0:00:01  0:00:01 --:--:--   115
100  125M  100  125M    0     0   252k      0  0:08:28  0:08:28 --:--:--  287k
linux-5.19.2.tar.xz: OK
INFO: Apply patches from /home/wmoschet/go/src/github.com/kata-containers/kata-containers/tools/packaging/kernel/patches/5.19.x
INFO: Found 0 patches
INFO: Constructing config from fragments: /home/wmoschet/go/src/github.com/kata-containers/kata-containers/tools/packaging/kernel/configs/fragments/s390/.config
INFO: Some CONFIG elements failed to make the final .config:
INFO: Value requested for CONFIG_HAVE_EBPF_JIT not in final .config
INFO: Generated config file can be found in /home/wmoschet/go/src/github.com/kata-containers/kata-containers/tools/packaging/kernel/configs/fragments/s390/.config
ERROR: Failed to construct requested .config file
ERROR: failed to find default config 
make[1]: *** [tools/packaging/kata-deploy/local-build/Makefile:59: kernel-tarball-build] Error 1
make[1]: Leaving directory '/home/wmoschet/go/src/github.com/kata-containers/kata-containers'
make: *** [tools/packaging/kata-deploy/local-build/Makefile:83: kernel-tarball] Error 2

Have I done it wrong?

@jodh-intel jodh-intel added safe-to-test Add to PR after manually reviewing to allow certain extra checks to run ok-to-test labels Nov 7, 2023
@jodh-intel
Copy link
Contributor

/test

This is to rule out unnecessary build targets for s390x.

Signed-off-by: Hyounggyu Choi <Hyounggyu.Choi@ibm.com>
This is to increase resources for relaxing the limitation of hotplug for
SE.

Signed-off-by: Hyounggyu Choi <Hyounggyu.Choi@ibm.com>
This is to add SE configuration which is used by kata runtime.

Signed-off-by: Hyounggyu Choi <Hyounggyu.Choi@ibm.com>
This is add a build target boot-image-se with a host-key-document
config for s390x.

Signed-off-by: Hyounggyu Choi <Hyounggyu.Choi@ibm.com>
This is to install s390-tools including genprotimg during the docker
build.

Signed-off-by: Hyounggyu Choi <Hyounggyu.Choi@ibm.com>
This is to add a build target boot-image-se for s390x.

Signed-off-by: Hyounggyu Choi <Hyounggyu.Choi@ibm.com>
This is to add an artifact for IBM Z SE(TEE) to main.

Fixes: kata-containers#6754

Signed-off-by: Hyounggyu Choi <Hyounggyu.Choi@ibm.com>
Copy link
Contributor

@wainersm wainersm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hi @BbolroC ! Sorry for taking months to re-review it. I couldn't build the boot-image-se due the lack of host key document on my laptop, then I reviewed the code carefully and didn't find any problem. The shim-v2, on the other hand, I could build and I see the configuration-qemu-se.toml generated and it looks good.

This is to make a base builder image build genprotimg without a package
manager under the cross-compilation environment.

Signed-off-by: Hyounggyu Choi <Hyounggyu.Choi@ibm.com>
This is to make kernel parameters configurable during the secure image build by adding an environment variable SE_KERNEL_PARAMS.

Signed-off-by: Hyounggyu Choi <Hyounggyu.Choi@ibm.com>
This is to make `build_se_image.sh` incorporate the key verification originally supported by `genprotimg`.
It can be achieved by specifying two environment variables called `SIGNING_KEY_CERT_PATH` and `INTERMEDIATE_CA_CERT_PATH`.

Signed-off-by: Hyounggyu Choi <Hyounggyu.Choi@ibm.com>
It is to remove the build redundancy of `kernel` and `rootfs-initrd` by making `boot-image-se` built based on them at the second build stage.

Signed-off-by: Hyounggyu Choi <Hyounggyu.Choi@ibm.com>
This is to adjust a name of the binary `strip` to a target architecture for cross-compilation.

Signed-off-by: Hyounggyu Choi <Hyounggyu.Choi@ibm.com>
@BbolroC
Copy link
Member Author

BbolroC commented Dec 7, 2023

/test

@BbolroC BbolroC merged commit 588f639 into kata-containers:main Dec 8, 2023
164 of 172 checks passed
@BbolroC BbolroC deleted the add-se-artifacts-to-main branch December 8, 2023 04:17
BbolroC added a commit to BbolroC/kata-containers that referenced this pull request Jan 25, 2024
The existing confidential basic test titled `Test unencrypted
confidential container launch success and verify that we are
running in a secure enclave` has been updated to incorporate
IBM Secure Execution (`qemu-se`).
Previously, a secure image was absent from kata-deploy, hindering
the inclusion of IBM SE in the test.
Thanks to the kata-containers#6755 update, it is now possible to test the TEE.

This modification extends the existing test by introducing
`qemu-se`. The specific changes are outlined below:

- Add an additional test `cc-se-e2e-tests` to s390x nightly
- Expansion of `REMOTE_COMMAND_PER_HYPERVISOR` for `qemu-se`
- Temporary exclusion of two test cases currently incompatible with IBM SE
(`cpu-ns` is a common issue across all TEEs, while `inotify`
will be addressed in a subsequent pull request).

Fixes: kata-containers#8913

Signed-off-by: Hyounggyu Choi <Hyounggyu.Choi@ibm.com>
BbolroC added a commit to BbolroC/kata-containers that referenced this pull request Jan 25, 2024
The existing confidential basic test titled `Test unencrypted
confidential container launch success and verify that we are
running in a secure enclave` has been updated to incorporate
IBM Secure Execution (`qemu-se`).
Previously, a secure image was absent from kata-deploy, hindering
the inclusion of IBM SE in the test.
Thanks to the kata-containers#6755 update, it is now possible to test the TEE.

This modification extends the existing test by introducing
`qemu-se`. The specific changes are outlined below:

- Add an additional test `cc-se-e2e-tests` to s390x nightly
- Expansion of `REMOTE_COMMAND_PER_HYPERVISOR` for `qemu-se`
- Temporary exclusion of two test cases currently incompatible with IBM SE
(`cpu-ns` is a common issue across all TEEs, while `inotify`
will be addressed in a subsequent pull request).

Fixes: kata-containers#8913

Signed-off-by: Hyounggyu Choi <Hyounggyu.Choi@ibm.com>
BbolroC added a commit to BbolroC/kata-containers that referenced this pull request Jan 26, 2024
The existing confidential basic test titled `Test unencrypted
confidential container launch success and verify that we are
running in a secure enclave` has been updated to incorporate
IBM Secure Execution (`qemu-se`).
Previously, a secure image was absent from kata-deploy, hindering
the inclusion of IBM SE in the test.
Thanks to the kata-containers#6755 update, it is now possible to test the TEE.

This modification extends the existing test by introducing
`qemu-se`. The specific changes are outlined below:

- Add an additional test `cc-se-e2e-tests` to s390x nightly
- Expansion of `REMOTE_COMMAND_PER_HYPERVISOR` for `qemu-se`
- Temporary exclusion of two test cases currently incompatible with IBM SE
(`cpu-ns` is a common issue across all TEEs, while `inotify`
will be addressed in a subsequent pull request).

Fixes: kata-containers#8913

Signed-off-by: Hyounggyu Choi <Hyounggyu.Choi@ibm.com>
c3d pushed a commit to c3d/kata-containers that referenced this pull request Feb 23, 2024
The existing confidential basic test titled `Test unencrypted
confidential container launch success and verify that we are
running in a secure enclave` has been updated to incorporate
IBM Secure Execution (`qemu-se`).
Previously, a secure image was absent from kata-deploy, hindering
the inclusion of IBM SE in the test.
Thanks to the kata-containers#6755 update, it is now possible to test the TEE.

This modification extends the existing test by introducing
`qemu-se`. The specific changes are outlined below:

- Add an additional test `cc-se-e2e-tests` to s390x nightly
- Expansion of `REMOTE_COMMAND_PER_HYPERVISOR` for `qemu-se`
- Temporary exclusion of two test cases currently incompatible with IBM SE
(`cpu-ns` is a common issue across all TEEs, while `inotify`
will be addressed in a subsequent pull request).

Fixes: kata-containers#8913

Signed-off-by: Hyounggyu Choi <Hyounggyu.Choi@ibm.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ok-to-test safe-to-test Add to PR after manually reviewing to allow certain extra checks to run size/huge Largest and most complex task (probably needs breaking into small pieces)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

se: add se artifacts for osbuilder (main)
5 participants