Skip to content

atelet: introduce declarative validation for the AteomSupport RPCs - #1903

Merged
Julian Gutierrez Oschmann (juli4n) merged 25 commits into
agent-substrate:mainfrom
shrutiyam-glitch:atelet-dv
Sep 29, 2026
Merged

Julian Gutierrez Oschmann (juli4n) merged 25 commits into
agent-substrate:mainfrom
shrutiyam-glitch:atelet-dv

Conversation

@shrutiyam-glitch

@shrutiyam-glitch Shrutiya Mohan (shrutiyam-glitch) commented Sep 25, 2026 •

Copy link
Copy Markdown
Collaborator

Fixes a part of #1709

This adds declarative validation to atelet's AteomSupport RPCs. Malformed requests are now rejected at atelet instead of being forwarded to the control plane. Validation runs after authentication.

RPC Rules
MintActorCertificate actor atespace/name are required short names; actor UID is a required UUID; the CSR is required and at most 16 KiB
RequestActorSuspend actor atespace/name are required short names; actor UID is a required UUID
SetWorkerCapacity capacity is required and must follow the WorkerResources rules ateapi declares, shared through internal/resources

The AteomHerder RPCs and WorkloadSpec get their validation in a follow-up PR.

  • Tests pass
  • Appropriate changes to documentation are included in the PR

  Generate DV for the atelet request messages into a new ateletvalidation
  package (atelet's handlers live in package main, so the generated code
  cannot sit with them as it does in controlapi). Fields carry their
  zero-value gates at parity with the hand-written validators; opaqueType
  defers the deep trees to follow-ups. Not wired to handlers yet.
  MintActorCertificate forwarded requests unvalidated and relied on the
  control plane to reject malformed ones after a round trip; now they
  fail at the node, after the certificate check so an unauthenticated
  caller still learns nothing but Unauthenticated. The CSR takes the
  same 16KiB maxBytes bound as the control-plane request it is
  forwarded into.
  docs/api-validation.md unions are flat optional fields tagged
  unionMember rather than proto oneofs, which declarative validation
  cannot see. The conversion is wire-compatible; only the Go wrapper
  types disappear. The unionMember tags land with the WorkloadSpec
  validation tags.
  Tag the flattened union members with unionMember, so a volume with no
  source or several sources is now rejected rather than silently
  tolerated, and bound SystemInfoVolume's data sources with the same
  paths-unique rule the control plane enforces. Deeper source messages
  stay opaque for follow-ups; Volume carries a per-type opt-in because
  no request descends into the WorkloadSpec tree yet.
  Bound ExternalVolumeSource and ImageVolumeSource with the same rules
  the control plane enforces on their template counterparts, and stop
  marking them opaque under Volume. storage_volume_id is required here,
  unlike the actor-status field it mirrors: the control plane only sends
  volumes that finished provisioning.
  Bound Capabilities with the control plane's grammar (uppercase, no
  CAP_ prefix, ALL only in drop, unique within each set) and floor the
  cgroup limits at zero. Container.name and the containers list are
  tagged ahead of the rest of Container: the list is keyed by name, and
  the generator requires presence tags on fields whose types now carry
  validations.
  Mirror the control plane's rules: env names are printable ASCII
  without '=', set at most once; mount paths are clean absolute Unix
  paths, unique per container, and must not nest, while a volume may be
  mounted at multiple paths. The env and volume_mounts lists on
  Container take their keys and bounds from the template counterparts.
  Mirror the template probe's rules: an RFC 3986 path capped at 1024
  characters, a port in 1..65535, and a polling timeout of at most an
  hour. The control plane always sends concrete values, so requiring
  them here is parity, not a tightening.
  The image must be a well-formed reference pinned by digest, matching
  the template rule it mirrors, and argv strings take the template's
  guardrail bounds. The container name hook rejects the reserved
  "pause", the last ValidateContainerNames rule without a declarative
  equivalent, so the herder handlers can drop that check as they wire up.
  ActorMetadataItem and TrustBundleDataSource take the control plane's
  rules: enum-bounded fields, each identity field projected at most
  once, and projected paths checked against the shared
  internal/volumepath rule that atelet also re-applies at host-write
  time. This removes the last opaque markers inside the WorkloadSpec
  tree.
  Give the request header its formats, open spec so the whole validated
  WorkloadSpec tree runs on a live RPC, and delete the hand-written
  validator: every check it made now has a declarative equivalent,
  including the reserved pause container name. actor_uid tightens from
  DNS label to UUID, matching the values the control plane mints. The
  lifecycle test fixture switches to a digest-pinned image, which is
  what validated specs actually carry.
atelet checked only that a capacity report was present. It now applies
the rules ateapi declares on WorkerResources, so bad reports are
rejected at atelet's edge instead of being forwarded.

validation-gen cannot follow ateapipb types into atelet, so the field
stays opaque and a custom hook calls resources.ValidateWorkerResources,
a hand-written copy of those rules. The Resources.limits rule moves to
resources.ValidateLimits so both packages share it, and a parity test
keeps the copy matching the generated validator. ateapi behavior is
unchanged.
Test each nested message through the message that owns it (Volume,
Container, WorkloadSpec) so errors are asserted at their real paths and
each shared type is covered once. Request tables also check the
handler-facing wrapper instead of separate edge tests.
Keep this change to the AteomSupport RPCs: MintActorCertificate,
SetWorkerCapacity, and RequestActorSuspend. AteomHerder validation, moves to a folloup.
@shrutiyam-glitch Shrutiya Mohan (shrutiyam-glitch) changed the title atelet: introduce declarative validation for API requests atelet: introduce declarative validation for the AteomSupport RPCs Sep 28, 2026
SetWorkerCapacityRequest reused ateapi's WorkerResources, which made the
atelet proto import the public control-plane API and differed from the
other atelet methods, which own their messages. Add atelet-owned
WorkerResources, Resources and Limits that mirror the control plane's
field for field, and convert them when the atelet forwards the request,
as it already does for WorkloadSpec.
The mirrored messages carry the same validation tags. The limit rules
move to resources.ValidateLimits, generic over either Limits type, so
the control plane and atelet hooks share one implementation.
The SetWorkerCapacity handler tests are folded into one table-driven
TestSetWorkerCapacity that checks the returned code and exactly what is
forwarded to the control plane.
Comment thread internal/resources/validate.go Outdated
type Limit interface {
GetName() string
GetQuantity() string
ProtoReflect() protoreflect.Message

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This is a bit too complicated. How about we keep the one validator for each api message (under controlapi and apivalidation) packages. If there is something we can reuse (like we do already today with resource.ParseQuantity) we can extract it.

Eventually this will all go away once we can validate these fields with DV tags.

// proto messages, and reflect.DeepEqual otherwise. This is called by
// declarative validation's generated code. It is a copy of controlapi's
// ateDeepEqual: the generated code calls it as a package-level identifier, so
// each generating package carries its own.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This is already defined in cmd/ateapi/internal/controlapi. How about moving it under /internal and then just have a wrapper ateDeepEqual that just calls the one shared?


func toInvalidArgument(errs field.ErrorList) error {
if len(errs) == 0 {
return nil

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The idiom that we follow in ate-apiserver for errors is that the validator functions return plain field.ErrorList errors, and then the handler does:

if errs := validateFoo(ctx, req); len(errors > 0 {
  return nil, toGRPCStatusError(errs)
}

Can we follow the same pattern? That way we can a) reuse toGRPCStatusError (we might need to move it under internal and b) the tests get simplified as they only need to assert using field.ErrorMatcher.


var createOp = operation.Operation{Type: operation.Create}

func expectErrors(t *testing.T, want, got field.ErrorList) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Nit: rename to assertValidateErr to be consistent with other validation tests?


// expectEdge checks the handler-facing wrapper: a valid request passes and an
// invalid one comes back as InvalidArgument.
func expectEdge(t *testing.T, err error, wantInvalid bool) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

See my comment about making validations return field.ErrorList which would remove the need for this function.

Merged via the queue into agent-substrate:main with commit 8205915 Sep 29, 2026
10 checks passed
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.

2 participants