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

k8s: docs: DCAP kustomization + non-release images #375

Merged
merged 2 commits into from
May 1, 2024

Conversation

mythi
Copy link
Contributor

@mythi mythi commented Apr 24, 2024

The KBS crash problem (ee1a8ce) was difficult to debug with the "built-in-as" image since all the logs from AS were lost.

Make the k8s sample deployment to use AS separately. This also makes it aligned with what docker-compose offers by default.

@mythi mythi changed the title k8s: use KBS and AS separately k8s: use KBS and AS separately + DCAP kustomization Apr 24, 2024
@fitzthum
Copy link
Member

Unfortunately I strongly prefer using the builtin AS. When the KBS and AS are separate the attack surface increases because they are communicating over the network. We have already found one issue with the API between the two and I suspect there could be more.

@mythi
Copy link
Contributor Author

mythi commented Apr 24, 2024

Unfortunately I strongly prefer using the builtin AS. When the KBS and AS are separate the attack surface increases because they are communicating over the network. We have already found one issue with the API between the two and I suspect there could be more.

Do you mean this is not meant to be just a test setup for CI but something people would use for real? Previously the only argument for a built-in was just that it's easier for developers to use...

@bpradipt
Copy link
Member

Using an all-in-one KBS image is useful in real scenarios where one want to run it as a single binary or container image using docker/podman without using compose. The same can also be run via systemd. So I would also prefer to have the inbuilt AS support to be tested in CI.

@fitzthum
Copy link
Member

Do you mean this is not meant to be just a test setup for CI but something people would use for real? Previously the only argument for a built-in was just that it's easier for developers to use...

We support both but I would prefer having our sample deployment use the builtin AS because it will make users more likely to adopt that model.

@mythi
Copy link
Contributor Author

mythi commented Apr 24, 2024

Do you mean this is not meant to be just a test setup for CI but something people would use for real? Previously the only argument for a built-in was just that it's easier for developers to use...

We support both but I would prefer having our sample deployment use the builtin AS because it will make users more likely to adopt that model.

As noted in the commit message, this aligns with the docker-compose sample we have. I'm fine moving this to an overlay (i.e., away from base). Would that work?

@fitzthum
Copy link
Member

As noted in the commit message, this aligns with the docker-compose sample we have. I'm fine moving this to an overlay (i.e., away from base). Would that work?

I would be fine with adding an overlay that allows people to use a separate AS if they want to.

@mythi mythi changed the title k8s: use KBS and AS separately + DCAP kustomization k8s: docs: DCAP kustomization + non-release images Apr 25, 2024
@mythi
Copy link
Contributor Author

mythi commented Apr 25, 2024

As noted in the commit message, this aligns with the docker-compose sample we have. I'm fine moving this to an overlay (i.e., away from base). Would that work?

I would be fine with adding an overlay that allows people to use a separate AS if they want to.

I decided to drop this idea for now since it's not super critical. I proposed the changes only because I had the work done while I was debugging the crash. Instead, I'm now proposing to add an optional doc entry on how users can use custom images if needed.

I've now verified the crash is gone with the latest staged images following the steps documented in this PR.

Copy link
Member

@fidencio fidencio left a comment

Choose a reason for hiding this comment

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

lgtm, thanks @mythi!

@fidencio fidencio added the test_e2e Authorize TEE e2e test run label May 1, 2024
Copy link
Member

@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.

NIce

@fitzthum fitzthum merged commit 6adb838 into confidential-containers:main May 1, 2024
21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
test_e2e Authorize TEE e2e test run
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants