-
Notifications
You must be signed in to change notification settings - Fork 60
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
WIP tests/e2e: Add encrypted image test for operator #446
base: main
Are you sure you want to change the base?
Conversation
uses: docker/login-action@v3 | ||
with: | ||
registry: ghcr.io | ||
username: ${{ github.actor }} |
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.
@stevenhorsman I think that we need to create this secrets and have that info, is this something that you can help me to set up?
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 think the gitub.actor value is automatically generated based on the last person to modify the flow, rather than a secret
tests/e2e/nginx-encrypted.yaml
Outdated
spec: | ||
runtimeClassName: RUNTIMECLASS | ||
containers: | ||
- image: docker://ghcr.io/confidential/nginx:latest |
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.
The name of the image can change depending of the secrets that we will have for the GHA
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.
Same comment as above
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.
changes applied
- name: Install skopeo | ||
shell: | | ||
sudo apt-get install -y build-essential libgpgme-dev libassuan-dev libbtrfs-dev pkg-config go-md2man | ||
git clone https://github.com/containers/skopeo |
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 think @fidencio found in confidential-containers/guest-components#669 that we needed a specific version of skopeo in order to get the round trip encryption working? It might be we can get a newer one going, but maybe it's safest to start with that one?
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.
well I tried to use the skopeo version f64a376
but seems like it does not work as when looking for the logs to see that the key is being retrieved by the kbs when doing kubectl logs -f "$POD" | grep "$KEY_PATH"
[2024-09-23T17:30:00Z INFO kbs] Using config file /etc/kbs/kbs-config.toml
[2024-09-23T17:30:00Z WARN attestation_service::rvps] No RVPS address provided and will launch a built-in rvps
[2024-09-23T17:30:00Z INFO attestation_service::token::simple] No Token Signer key in config file, create an ephemeral key and without CA pubkey cert
[2024-09-23T17:30:00Z INFO kbs] Starting HTTP server at [0.0.0.0:8080]
[2024-09-23T17:30:00Z INFO actix_server::builder] starting 256 workers
[2024-09-23T17:30:00Z INFO actix_server::server] Tokio runtime found; starting in existing Tokio runtime
tests/e2e/encrypted.sh
Outdated
export kbs_svc_name="kbs" | ||
export kbs_ingress_name="kbs" | ||
export runtimeclass="kata-qemu" | ||
export encrypted_image="ghcr.io/confidential/nginx:latest" |
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.
export encrypted_image="ghcr.io/confidential/nginx:latest" | |
export encrypted_image="ghcr.io/confidential-containers/nginx:encrypted" |
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.
If we can pass in the repo via an envar and set it in the workflow that would be even better
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.
changes applied
tests/e2e/nginx-encrypted.yaml
Outdated
spec: | ||
runtimeClassName: RUNTIMECLASS | ||
containers: | ||
- image: docker://ghcr.io/confidential/nginx:latest |
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.
Same comment as above
with: | ||
registry: ghcr.io | ||
username: ${{ github.actor }} | ||
password: ${{ secrets.GITHUB_TOKEN }} |
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.
@stevenhorsman I think here we will need to create the secret
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.
The GITHUB_TOKEN is automatically created, so we don't need to make it.
popd | ||
} | ||
|
||
deploy_k8s_kbs() { |
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.
Do we need something else for the kbs configuration?
kubectl delete "${pod_name}" | ||
} | ||
|
||
check_image_key() { |
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.
This is the part that currently is failing... seems like we can't find the secret...not sure if a configuration is missing @wainersm
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.
Looks good. I made a few suggestions.
Does this pass as is?
tests/e2e/encrypted.sh
Outdated
guest_components_repo="https://github.com/confidential-containers/guest-components.git" | ||
guest_components_path="${script_dir}/guest-components" | ||
if [ -d "${guest_components_path}" ]; then | ||
echo "Guest components repo directory exists" |
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.
Maybe we should always clone guest components in case the version on the host is out of date
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.
changes applied
tests/e2e/encrypted.sh
Outdated
tag_name="coco-keyprovider" | ||
docker build -t "${tag_name}" -f ./attestation-agent/docker/Dockerfile.keyprovider . | ||
mkdir -p oci/{input,output} | ||
skopeo copy docker-daemon:unencrypted:latest dir:./oci/input |
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.
Once we get the basic flow working, let's think about adding a random secret to the image (rather than "something confidential") and checking for that value inside the guest. Also, we can grep for that string in the encrypted layer and make sure it doesn't show up.
tests/e2e/encrypted.sh
Outdated
fi | ||
|
||
pushd "${script_dir}/trustee/kbs/config/kubernetes" | ||
echo "somesecret" > overlays/$(uname -m)/key.bin |
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.
btw you could probably simplify things by providing the image decryption key here (instead of "somesecret"). in that case you could probably remove provide_image_key
.
force: yes | ||
retries: 3 | ||
delay: 10 | ||
- name: Build and install the kbs client |
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.
It doesn't look like you actually use the kbs-client
in this test. You can probably skip installing and uninstalling it.
0523d88
to
ac44ba4
Compare
tests/e2e/encrypted.sh
Outdated
export aa_kbc="${aa_kbc:-cc_kbc}" | ||
export kbs_svc_name="${kbs_svc_name:-kbs}" | ||
export kbs_ingress_name="${kbs_ingress_name:-kbs}" | ||
export runtimeclass="${runtimeclass:-kata-qemu}" |
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.
Hi @GabyCT !
I started reviewing and, more importantly, running this script. I'm executing function by function, as of today I ran the build_encryption_key()
.
The first thing caught my attention was the default value of runtimeclass
. It should be kata-qemu-coco-dev
and the encrypted test should not run on kata-qemu
or kata-clh
.
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.
@wainersm the kata-qemu-coco-dev
does not exists as we can see
runtimeclass: |
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.
yes, not yet. It's in my TODO list to create a new job for kata-qemu-coco-dev.
tests/e2e/encrypted.sh
Outdated
export encrypted_image="${encrypted_image:-ghcr.io/$username/nginx:encrypted}" | ||
|
||
build_encryption_key() { | ||
docker build -t unencrypted . |
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.
Side note: On v0 of this test I think it's fine to build the encrypted image all the time, but long term we will need to catch up with @portersrc 's work to have these test images all built once and shared amongst tests.
tests/e2e/encrypted.sh
Outdated
|
||
pushd "${guest_components_path}" | ||
export tag_name="coco-keyprovider" | ||
docker build -t "${tag_name}" -f ./attestation-agent/docker/Dockerfile.keyprovider . |
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.
guest-components published an image of the keyprovider for the v0.10.0. There is a workflow to keep publishing that image for the next releases so I think we can consume the built one instead for build ourselves.
export kbs_ingress_name="${kbs_ingress_name:-kbs}" | ||
export runtimeclass="${runtimeclass:-kata-qemu}" | ||
export username="${username:-}" | ||
export encrypted_image="${encrypted_image:-ghcr.io/$username/nginx:encrypted}" |
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.
The operator e2e CI install and launch a container registry at localhost:5000. One option that we could explore later is to push the encrypted image to that registry rather than ghcr.io
. I managed to push an encrypted image to localhost:5000 but I had to disable TLS on all operations with skopeo because the local registry is configured with HTTP only. Thus, the image-rs on kata's guest may not be able to talk with the local registry....
I wanted to record that idea, but it should not be priority now :D
echo "somesecret" > overlays/$(uname -m)/key.bin | ||
export DEPLOYMENT_DIR=nodeport | ||
./deploy-kbs.sh | ||
kbs_pod=$(kubectl -n "${kbs_namespace}" get pods -o NAME) |
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.
The kbs pod may not had started and kbs_pod
will be empty. This happened here in my env.
On kata-containers we use the waitForProcess
function to wait the pod show up. But a simple solution would be to use the kubectl rollout
command (and drop the next kubectl wait
call): kubectl rollout status -w --timeout=30s deployment/kbs -n coco-tenant
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.
changes applied
yq -i ".${annotation_key} = \"${value}\"" "${yaml}" | ||
} | ||
|
||
set_aa_kbc() { |
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.
The added kernel_params metadata is not quoted:
apiVersion: apps/v1
kind: Deployment
metadata:
labels:
app: nginx
name: nginx-encrypted
annotations:
io.katacontainers.config.hypervisor.kernel_params: agent.guest_components_rest_api=resource agent.aa_kbc_params=cc_kbc::http://192.168.122.180:31231
spec:
replicas: 1
selector:
matchLabels:
app: nginx
<snip>
Don't know whether it will be a problem or not but we better quote it to avoid any parsing issues. Something like io.katacontainers.config.hypervisor.kernel_params: ' agent.guest_components_rest_api=resource agent.aa_kbc_params=cc_kbc::http://192.168.122.180:31231'
tests/e2e/nginx-encrypted.yaml
Outdated
spec: | ||
runtimeClassName: RUNTIMECLASS | ||
containers: | ||
- image: docker://ghcr.io/USERNAME/nginx:encrypted |
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.
docker://
isn't needed. It even refuses to start the container.
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.
changes applied
tests/e2e/encrypted.sh
Outdated
|
||
launch_pod() { | ||
kubectl apply -f "${script_dir}/nginx-encrypted.yaml" | ||
export pod_name=$(kubectl -n default get pods -o NAME) |
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.
Use kubectl rollout
here too.
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.
changes applied
Hi @GabyCT , I ran your code and have some results to share. With the generated
Then I decided to apply the following pod yaml which is pretty similar to what is used on Kata CI. Notice that I'm using
This time it is able to connect to kbs:
But notice the error Ok, then I tried to set the "allow all" policy but hit an error, maybe a bug on latest kbs-client:
@fitzthum ^ is it a known issue? |
4489846
to
d193795
Compare
Yeah. I thought we had fixed this, but we have seen problems parsing the policies in the past. Maybe try randomly changing the format :'( |
Actually let me be more clear. I wouldn't expect to run into a parsing problem with that policy in particular and actually I just tested it on my machine and it works fine. We have seen some issues parsing policies in general but not with this file. |
0186de8
to
e53c18d
Compare
This PR adds an encrypted image test for the operator repository. Signed-off-by: Gabriela Cervantes <[email protected]>
This PR adds an encrypted image test for the operator repository.