Add helm chart deployment by Xynnn007 · Pull Request #1150 · confidential-containers/trustee

Xynnn007 · 2026-01-15T10:03:30Z

This PR mainly introduces the helm chart for trustee, and also some fixes met during tests. Please see the concrete commits for details.

Two issues to be created once merged

Helm | Have a strategy for release, image tags
HTTPS support for Helm

Updated:

fitzthum

This is great. cc @lmilleri

I made a few notes. Mostly they are just notes for myself about Trustee in CoCo. We can ignore those for now.

lmilleri · 2026-01-16T11:54:23Z

Very nice. Any plans to add support for single image with all the components (kbs-as-rvps)? Or do you think deploying separate services is definitely the way to go?

Xynnn007 · 2026-01-19T01:12:43Z

Very nice. Any plans to add support for single image with all the components (kbs-as-rvps)? Or do you think deploying separate services is definitely the way to go?

Yes, we can add it later if needed. Currently, it completely mimics the logic of docker-compose as first draft. Personally I prefer micro service state.

vans163 · 2026-01-19T04:05:49Z

Autocreate seems not working

helm install trustee ./deployment/helm-chart --namespace coco-trustee --create-namespace
Error: INSTALLATION FAILED: StatefulSet.apps "attestation-service" is invalid: [spec.template.spec.volumes[4].secret.secretName: Required value, spec.template.spec.containers[0].volumeMounts[3].name: Not found: "user-keys"]

Xynnn007 · 2026-01-19T04:41:57Z

Autocreate seems not working

helm install trustee ./deployment/helm-chart --namespace coco-trustee --create-namespace
Error: INSTALLATION FAILED: StatefulSet.apps "attestation-service" is invalid: [spec.template.spec.volumes[4].secret.secretName: Required value, spec.template.spec.containers[0].volumeMounts[3].name: Not found: "user-keys"]

The work is still under testing. I will turn to green when test locally successfully

pawelpros · 2026-03-25T13:56:57Z

@Xynnn007 nice work - I was able to verify basic setup (needed some fixes here and there) but works quite nice in current state.

Xynnn007 · 2026-03-26T02:08:12Z

Let me get #1190 in and make more changes together

Xynnn007 · 2026-04-12T12:14:45Z

Folks. Update this PR and test locally with default (localfs) and postgres on alibaba cloud container service and KIND

PTAL

cc @fidencio @beraldoleal

Xynnn007 · 2026-04-12T12:16:34Z

@fitzthum #1239 will influence this PR a lot. It makes sense for either to get merged first and I can help to take rebase in another.

Copilot

Pull request overview

Adds a Helm chart for deploying the Trustee stack (KBS + gRPC Attestation Service + RVPS, optionally bundled Postgres) and aligns unified-storage namespaces to underscore-separated identifiers (better suited for backends like Postgres that treat namespaces as table names).

Changes:

Introduce deployment/helm-chart with templates, values, scenario overrides, and embedded config templates for KBS/AS/RVPS (+ optional Postgres).
Rename several unified-storage namespaces from hyphenated to underscore-separated variants across RVPS/KBS/AS code + docs.
Update documentation/config examples to reflect the new namespaces.

Reviewed changes

Copilot reviewed 36 out of 36 changed files in this pull request and generated 11 comments.

Show a summary per file

File	Description
rvps/src/lib.rs	Rename RVPS unified-storage namespace constant.
rvps/README.md	Update documented namespace name.
kbs/src/config.rs	Doc comment reference update for AS policy namespace constant.
kbs/src/attestation/backend.rs	Rename KBS session storage namespace constant + comment.
kbs/docs/config.md	Update documented unified-storage namespaces and examples.
kbs/config/kbs-config.toml	Update comment for session namespace.
attestation-service/src/lib.rs	Rename AS policy unified-storage namespace constant.
attestation-service/src/config.rs	Update doc comments for AS namespaces.
attestation-service/docs/config.md	Update documented namespaces.
deployment/helm-chart/Chart.yaml	New Helm chart metadata.
deployment/helm-chart/values.yaml	Default chart values (images, services, storage, secrets, Postgres).
deployment/helm-chart/README.md	Helm chart usage + scenarios + configuration reference.
deployment/helm-chart/templates/*	New templates for KBS/AS/RVPS services/deployments/configmaps + optional Postgres + secrets + bootstrap job/RBAC + helpers.
deployment/helm-chart/files/*	Embedded config templates for KBS/AS/RVPS and SGX QCNL config.
deployment/helm-chart/scenarios/*	Example override scenario for bundled Postgres + memory sessions.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Xynnn007 · 2026-04-13T02:15:25Z

Let me play with Copilot for a while.

Copilot

Pull request overview

Copilot reviewed 37 out of 37 changed files in this pull request and generated 11 comments.

Comments suppressed due to low confidence (1)

rvps/src/lib.rs:1

Changing the storage namespace string is a breaking change for any existing deployments (LocalFs/LocalJson/Postgres) because previously persisted data under reference-value will no longer be read. If this change is required for Postgres/table-name compatibility, consider providing a migration path (e.g., attempt read/write fallback/alias for the old namespace, or document/automate a one-time migration) and call it out explicitly in upgrade notes.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Replace the kustomize-based Trustee deployment with the Helm chart from confidential-containers/trustee#1150, using a fork that adds health probes, generic extension points, and standard ingress/NodePort support. Signed-off-by: Fabiano Fidêncio <ffidencio@nvidia.com>

fidencio · 2026-06-02T11:43:12Z

@Xynnn007, I've pushed 4 commits atop of yours (all helm related) and that's what I used to get it to work on Kata Containers side.

Please, review those, and then squash those into your original commit, adding also my Signed-off-by: ....

This series LGTM with those changes and was tested end-to-end on the Kata Containers side.
I'd love to get it merged ASAP, so we can switch Kata Containers to using it, and ensure others will have an easier time improving it than I did. :-)

fidencio · 2026-06-02T11:44:04Z

cc @fitzthum as well

fitzthum

A few comments. I think we gotta have all the security features that we can turned on. That might not be necessary for testing, but the main purpose of the Helm chart is for people to use it.

I had been thinking about enabling HTTPS by default for the Helm chart. I think we should do that, but not in this PR.

Xynnn007 · 2026-06-03T04:01:05Z

Note: DO NOT MERGE. TESTING ON KATA SIDE WITH ADMIN TOKEN THINGS.

mythi · 2026-06-03T04:30:38Z

+trusted_certs_paths = ["/opt/confidential-containers/kbs/user-keys/token-cert-chain.pem"]
+
+[attestation_service]
+type = "coco_as_grpc"


is ITA configurable?

it can but this PR we do not support yet. Can we delay it in a following PR?

That's probably OK given it's not used by Kata anymore (it was until recently still)

Is there a reason we move away from builtin to as_grpc? I had understood builtin was the preferred mode.

There is always nowhere to mention that builtin is preferred actually and this just follows what docker-compose is doing. Are there any hard blockers for grpc as?

+1 for @fitzthum . In both way, we still need extra work. So from my personal preference, this PR can be merged firstly, and then do ITA patch.

We do not need to make helm perfect on this one PR. And we can start kata side integration work later. We are in between development cycles so it's fine to make fixups later. @fidencio what's your opinion for this?

@mythi I understand the concerns, and I already get some of the stuff ready in this PR. For ITA it's hard for me to test as I do not have access to it.

Given that this PR has already been open for quite some time, we would prefer to move forward with a multi-PR way so that the current work can land first, and then address the remaining items iteratively.

This is not a sign that we are unwilling to support Intel TDX-related work. On the contrary, we remain committed to supporting it - as always. and I am happy to continue contributing in subsequent PRs.

I hope this makes sense to you, and shall we proceed with this PR first and follow up on the remaining enhancements in later PRs?

ITA is not my concern here. My concern is the model that we are ready accept changes that we know need to be changed/removed and have user impact. I'd rather get the baseline correct (e.g., that this needs to run within a single pod) and make incremental (small) additions on top of it.

@mythi Putting all containers in one pod and separating them is a trade-off.

Separating them allows for fine-grained disaster recovery, while keeping them together ensures that all traffic flows within the pod.

I believe that means extra work on Kata side because the deployment is going to be different (the current Kata implementation checks all pods are there which is not the case in the future). Would be good to get this in place before merge

In either way, it's enough for the kata side to probe the kbs health api, as helm logic has dependency launch logic, which can ensure other services are ready before kbs is health.

I already have it working on kata with this in the way it is here. I even got fixes for the health API merged exactly due to this. :-)

Xynnn007 · 2026-06-10T11:48:36Z

@fidencio @fitzthum @mythi

Updated the PR:

Reorganize the codes
Add DCAP verifier config
Use bitnamicharts to provide internal postgres deployment
add deletion logic to ephemeral keys

I'd prefer to leave other work items given by the reviews in later PRs. All of them are listed in the #1150 (comment) box. I will open issues once this PR gets merged.

Test locally with sample kbs-client.

Btw I'd like to help @fidencio with kata-ci integration work, who has made VERY HUGE EFFORT to the work already and I really appreciate it.

fitzthum

Ok, one more comment.

Also, what is the plan for testing the Helm chart in this repo? Is that going to be part of this PR?

fitzthum · 2026-06-10T19:20:08Z

+# incremented each time you make changes to the application. Versions are not expected to
+# follow Semantic Versioning. They should reflect the version the application is using.
+# It is recommended to use it with quotes.
+appVersion: "0.18.0"


Are we planning to keep this in sync with the Trustee version? It's a bit tricky to do that with our current release process.

It's tricky from my side too. So I added a TODO mark in the code

I think this is somehow coupled with the discussion

My initial idea is that if we can integrate Trustee's Helm deployment and testing into an end-to-end CI within the Trustee repository, then we can solve the puzzle of "first Kata CI, then release Trustee," enabling independent releases of Trustee, with Kata referencing the tag?

Before that, it may be fine to leave the appVersion number some number, as it does not make much sense?

ok. i guess Kata tracks the versions of the components separately so we don't have to figure this out quite yet.

Xynnn007 · 2026-06-11T01:45:25Z

Also, what is the plan for testing the Helm chart in this repo? Is that going to be part of this PR?

I tried to use kind to set-up a light kubernetes cluster on GHA runner to test with kbs-client. However, the commit seems large, and also there is some timeout issues. I prefer to add it in a following PR and needs some optimization.

mythi · 2026-06-11T06:41:07Z

2. Add DCAP verifier config

thanks! The commit message looks odd. I don't think they need all to be set (at least I was testing air-gapped with just collateral_service = file:// ... added).

Xynnn007 · 2026-06-11T07:28:23Z

thanks! The commit message looks odd. I don't think they need all to be set (at least I was testing air-gapped with just collateral_service = file:// ... added).

@mythi

Fixed, PTAL. Also I think #1150 (comment) is resolved.

Also, to push forward the work, let's make a due time of the next trustee meeting (until 6.15). If no other points raised, I will get this PR merged. Feel free to give a review until that time.

Also, keep in mind that we can fix things any time before actually integrate this into kata-ci.

This patch introduces helm deployment for Trustee, including the basic 1. values.yaml definition, default with localFs backend 2. Optionally BYOK mechanism 3. Optionally automatically ephemeral key generation Helped with Cursor vibe coding, and test manually with kind cluster. Assisted-by: Cursor Signed-off-by: Xynnn007 <xynnn@linux.alibaba.com>

Switch chart configuration documentation to helm-docs: add README.md.gotmpl, generate the Values table from values.yaml `# --` comments, and keep prose sections (install, scenarios, testing) in the template. Initial import covers only settings present at chart introduction; later commits extend both values.yaml and the generated table. Assisted-by: Cursor <cursoragent@cursor.com> Signed-off-by: Xynnn007 <xynnn@linux.alibaba.com>

Xynnn007 · 2026-06-15T02:37:04Z

@mythi helm-docs updated. use_secure_cert deleted. PTAL.

The base chart only exposed KBS through an in-cluster ClusterIP plus an optional LoadBalancer. Add first-class templates for a Kubernetes Ingress and a NodePort Service so the KBS can be reached in environments without a cloud LoadBalancer, both gated behind values and disabled by default. Also fix the optional LoadBalancer toggle: `exposeLoadBalancer: false` was ignored because the template used `default true`, which treats an explicit false the same as unset. Check for the key explicitly so users can opt out. Signed-off-by: Fabiano Fidêncio <ffidencio@nvidia.com> Assisted-by: Cursor <cursoragent@cursor.com> Signed-off-by: Xynnn007 <xynnn@linux.alibaba.com>

Add the knobs needed to run the chart in real clusters rather than only the demo defaults: - extraEnvVars on KBS/AS/RVPS (e.g. HTTP(S)_PROXY/NO_PROXY for collateral fetching), plus extraVolumes/extraVolumeMounts on KBS, - imagePullPolicy switched to Always so :latest images are refreshed, - higher CPU/memory requests and limits. Attestation is CPU-bound and the all-verifier AS image loads TDX/SGX DCAP and NVIDIA libraries; the old tight limits got the AS OOMKilled/throttled into probe failures mid attest, dropping it from the Service and breaking KBS->AS calls. Signed-off-by: Fabiano Fidêncio <ffidencio@nvidia.com> Assisted-by: Cursor <cursoragent@cursor.com> Signed-off-by: Xynnn007 <xynnn@linux.alibaba.com>

Wire the new health endpoints into Kubernetes probes so unhealthy pods are restarted and only ready pods receive traffic: HTTP /healthz for KBS, gRPC probes for AS and RVPS. The AS probes are deliberately tolerant. Intel TDX/SGX DCAP verification makes blocking FFI and collateral calls that can stall the health endpoint for several seconds under load, so the AS gets a startupProbe plus relaxed liveness/readiness timeouts and failure thresholds. This keeps a busy but alive AS in the Service instead of letting kubelet kill it mid attest. Signed-off-by: Fabiano Fidêncio <ffidencio@nvidia.com> Assisted-by: Cursor <cursoragent@cursor.com> Signed-off-by: Xynnn007 <xynnn@linux.alibaba.com>

For AS: optionally emit `verifier_config.nvidia_verifier` (Local or Remote NRAS), driven by a new `as.verifier.nvidia` value. Other verifiers (SNP, Intel DCAP) keep working with their built-in defaultsm and can be extended later. Note that if the nvidia config's type is not given, it will default to `Local`. If `Remote` is selected, `verifierUrl` is required. Signed-off-by: Fabiano Fidêncio <ffidencio@nvidia.com> Assisted-by: Cursor <cursoragent@cursor.com> Signed-off-by: Xynnn007 <xynnn@linux.alibaba.com>

An optional item for `verifier_config.dcap_verifier`, driven by a new `as.verifier.dcap` value. If DCAP config is configured, all fields are required to be configure, or there will be an error. Assisted-by: Cursor <cursoragent@cursor.com> Signed-off-by: Xynnn007 <xynnn@linux.alibaba.com>

Enable Postgres as the unified KV backend for KBS, AS, and RVPS via POSTGRES_URL, with optional bundled Bitnami PostgreSQL (including KV table init) or an external database referenced by an existing Secret. Assisted-by: Cursor <cursoragent@cursor.com> Signed-off-by: Xynnn007 <xynnn@linux.alibaba.com>

When use `secrets.useEphemeralGeneratedKeys`, an admin token and asymmetric key pair is generated for auth. Thus commit will automatically delete the auth secrets via a post-delete job. Assisted-by: Cursor <cursoragent@cursor.com> Signed-off-by: Xynnn007 <xynnn@linux.alibaba.com>

fitzthum

LGTM

We still have work to do after this PR, but I think we should probably get this in and iterate on it.

Xynnn007 · 2026-06-24T03:35:18Z

I think most comments have been resolved. Others are recorded as issues with Helm Chart | name prefix. Let me get this in and start with other work.

fitzthum reviewed Jan 15, 2026

View reviewed changes

Comment thread deployment/helm-chart/Chart.yaml Outdated

Comment thread deployment/helm-chart/README.md Outdated

Comment thread deployment/helm-chart/README.md Outdated

Comment thread deployment/helm-chart/README.md Outdated

Comment thread deployment/helm-chart/README.md Outdated

fitzthum mentioned this pull request Jan 16, 2026

[RFC] Helm Chart vs Operator for Trustee Deployment #904

Open

lmilleri mentioned this pull request Jan 16, 2026

Introducing the trustee chart confidential-containers/trustee-operator#118

Draft

fitzthum added this to CoCo Releases Feb 3, 2026

fitzthum moved this to 🏗 In progress in CoCo Releases Feb 3, 2026

fitzthum mentioned this pull request Mar 3, 2026

General Technical Review: Confidential Containers - Incubation cncf/toc#2051

Open

Xynnn007 force-pushed the feat-helm-chart branch 2 times, most recently from 1f67640 to 19c0d71 Compare April 12, 2026 12:12

Xynnn007 marked this pull request as ready for review April 12, 2026 12:14

Xynnn007 requested a review from a team as a code owner April 12, 2026 12:14

Xynnn007 changed the title ~~WIP : Add helm chart deployment~~ Add helm chart deployment Apr 12, 2026

fidencio requested a review from Copilot April 12, 2026 12:31

Copilot started reviewing on behalf of fidencio April 12, 2026 12:31 View session

Copilot AI reviewed Apr 12, 2026

View reviewed changes

Xynnn007 marked this pull request as draft April 13, 2026 02:15

Xynnn007 force-pushed the feat-helm-chart branch from 19c0d71 to ef95473 Compare April 13, 2026 09:58

Xynnn007 requested a review from Copilot April 13, 2026 09:58

Copilot started reviewing on behalf of Xynnn007 April 13, 2026 09:59 View session

Copilot AI reviewed Apr 13, 2026

View reviewed changes

Xynnn007 force-pushed the feat-helm-chart branch from ef95473 to 503d5a1 Compare April 15, 2026 05:31

github-advanced-security AI found potential problems Apr 15, 2026

View reviewed changes

Comment thread kbs/src/token/jwk.rs Fixed

mythi reviewed Jun 2, 2026

View reviewed changes

Comment thread deployment/helm-chart/templates/as-deployment.yaml Outdated

fitzthum reviewed Jun 2, 2026

View reviewed changes

mythi reviewed Jun 2, 2026

View reviewed changes

Comment thread deployment/helm-chart/README.md Outdated

Comment thread deployment/helm-chart/templates/postgres-deployment.yaml Outdated

mythi reviewed Jun 3, 2026

View reviewed changes

Comment thread deployment/helm-chart/templates/bootstrap-user-keys-job.yaml

mythi reviewed Jun 3, 2026

View reviewed changes

fitzthum reviewed Jun 10, 2026

View reviewed changes

mythi reviewed Jun 12, 2026

View reviewed changes

Comment thread deployment/helm-chart/README.md Outdated

mythi reviewed Jun 12, 2026

View reviewed changes

Comment thread deployment/helm-chart/templates/_helpers.tpl Outdated

Xynnn007 added 2 commits June 15, 2026 09:56

Xynnn007 added 7 commits June 15, 2026 11:12

Xynnn007 mentioned this pull request Jun 16, 2026

Deployment model for trustee helm chart #1426

Open

fitzthum approved these changes Jun 24, 2026

View reviewed changes

Uh oh!

Conversation

Xynnn007 commented Jan 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fitzthum left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lmilleri commented Jan 16, 2026

Uh oh!

Xynnn007 commented Jan 19, 2026

Uh oh!

vans163 commented Jan 19, 2026

Uh oh!

Xynnn007 commented Jan 19, 2026

Uh oh!

pawelpros commented Mar 25, 2026

Uh oh!

Xynnn007 commented Mar 26, 2026

Uh oh!

Xynnn007 commented Apr 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Xynnn007 commented Apr 12, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Xynnn007 commented Apr 13, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

fidencio commented Jun 2, 2026

Uh oh!

fidencio commented Jun 2, 2026

Uh oh!

Uh oh!

fitzthum left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Xynnn007 commented Jun 3, 2026

Uh oh!

Uh oh!

Xynnn007 commented Jan 15, 2026 •

edited

Loading

Xynnn007 commented Apr 12, 2026 •

edited

Loading

mythi Jun 3, 2026 •

edited

Loading

mythi Jun 3, 2026 •

edited

Loading