Skip to content

fix(cluster): fail closed on unresolved TEE runtimes - #425

Draft
chalabi2 wants to merge 2 commits into
akash-network:mainfrom
chalabi2:jc/tee-runtime-fail-closed
Draft

fix(cluster): fail closed on unresolved TEE runtimes#425
chalabi2 wants to merge 2 commits into
akash-network:mainfrom
chalabi2:jc/tee-runtime-fail-closed

Conversation

@chalabi2

@chalabi2 chalabi2 commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

Why

A confidential workload whose TEE type or node platform cannot be resolved must not fall through to the ordinary OCI runtime. Startup label races, unmanaged nodes, mixed TEE platforms, and unknown values could otherwise silently remove the confidentiality boundary.

What changed

  • make TEE type-to-runtime selection typed and exhaustive
  • resolve the runtime before mutating inventory state
  • reject invalid or conflicting stored TEE state
  • scope platform discovery to Akash-managed nodes
  • delay discovery until managed nodes are ready
  • cover supported combinations and every fail-closed case

Validation

CGO was disabled because the local Xcode/clang installation is broken. These focused tests passed:

CGO_ENABLED=0 GOWORK=off go test ./cluster/kube/builder ./cluster/kube ./cluster/types/v1beta3/clients/inventory ./cluster/kube/operators/clients/inventory ./cluster -count=1

All current GitHub checks pass; one non-applicable check is skipped.

@chalabi2
chalabi2 requested a review from a team as a code owner August 1, 2026 06:46
@coderabbitai

coderabbitai Bot commented Aug 1, 2026

Copy link
Copy Markdown

Review Change Stack

Walkthrough

The change adds TEE validation, improves managed-node platform detection, moves detection after operator readiness, and resolves runtime classes before confidential-compute inventory allocation in both inventory implementations.

Changes

TEE validation and confidential workload scheduling

Layer / File(s) Summary
TEE input validation
cluster/inventory.go, cluster/inventory_test.go
TEE values from resource groups and cluster parameters now return errors for invalid, duplicate, or conflicting declarations. Reservation restoration and requests propagate validation errors.
Managed-node platform detection
cluster/client.go, cluster/inventory.go, cluster/kube/client_attestation.go, cluster/kube/client_attestation_test.go
Detection now examines managed Kubernetes nodes, rejects mixed platforms, handles list failures, and runs after operators become ready.
Runtime-class resolution
cluster/kube/builder/workload.go, cluster/kube/builder/runtime_class_test.go
RuntimeClassForTEEType accepts typed TEE values, validates inputs, and returns SNP or TDX CPU/GPU runtime classes with errors for unsupported values.
Inventory runtime propagation
cluster/kube/operators/clients/inventory/*, cluster/types/v1beta3/clients/inventory/*
Both inventory implementations resolve runtime classes before allocation and pass them through CPU and GPU confidential-compute scheduling. Tests verify errors occur without inventory mutation.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Reservation
  participant Inventory
  participant RuntimeClassForTEEType
  participant WorkloadAllocation
  Reservation->>Inventory: submit TEE request
  Inventory->>RuntimeClassForTEEType: resolve TEE type and platform
  RuntimeClassForTEEType-->>Inventory: return RuntimeClass or error
  Inventory->>WorkloadAllocation: pass resolved RuntimeClass
  WorkloadAllocation-->>Inventory: adjust CPU or GPU inventory
Loading

Possibly related PRs

Suggested reviewers: cloud-j-luna, troian

Poem

A rabbit checks each TEE with care,
Finds mixed labels in the air.
Runtime classes hop in line,
CPU and GPU scheduling align.
Errors stop before resources move.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly summarizes the main change: fail-closed handling for unresolved TEE runtimes.
Description check ✅ Passed The description directly explains the TEE runtime changes, fail-closed behavior, validation, and test coverage.
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Reject unknown or conflicting TEE requests before inventory mutation, resolve the runtime class exhaustively, and detect platforms only from managed nodes after inventory discovery is ready.

Signed-off-by: Joseph Chalabi <chalabi.joseph@gmail.com>
@chalabi2
chalabi2 force-pushed the jc/tee-runtime-fail-closed branch from bc2507b to 1801dff Compare August 1, 2026 06:52
@chalabi2 chalabi2 changed the title cluster: fail closed on unresolved TEE runtimes fix(cluster): fail closed on unresolved TEE runtimes Aug 1, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@cluster/inventory.go`:
- Around line 150-157: Update the reservation restoration loop in
newInventoryService so an error from teeTypeFromClusterParams for one deployment
is logged and isolated to that reservation instead of being returned. Skip the
invalid reservation (excluding it from reservations) and continue restoring the
remaining deployments so go is.run(ctx, reservations) can start.
- Around line 514-544: Update teeTypeFromClusterParams to handle
crd.ReservationClusterSettings in its type switch, extracting its
SchedulerParams into sparams like the existing ClusterSettings cases. Preserve
the current parsing, invalid-type errors, and conflicting-type detection for
restored reservation settings.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: c0e83eea-58df-42bf-bdc6-6c56dc9f11b4

📥 Commits

Reviewing files that changed from the base of the PR and between 707ab90 and bc2507b.

📒 Files selected for processing (11)
  • cluster/client.go
  • cluster/inventory.go
  • cluster/inventory_test.go
  • cluster/kube/builder/runtime_class_test.go
  • cluster/kube/builder/workload.go
  • cluster/kube/client_attestation.go
  • cluster/kube/client_attestation_test.go
  • cluster/kube/operators/clients/inventory/inventory.go
  • cluster/kube/operators/clients/inventory/tee_runtime_test.go
  • cluster/types/v1beta3/clients/inventory/inventory.go
  • cluster/types/v1beta3/clients/inventory/tee_runtime_test.go

Comment thread cluster/inventory.go
Comment thread cluster/inventory.go
Signed-off-by: Joseph Chalabi <chalabi.joseph@gmail.com>
@chalabi2
chalabi2 marked this pull request as draft August 1, 2026 16:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant