From b8e403eedd14eb4b4090a3dd8b0fec374da41a56 Mon Sep 17 00:00:00 2001 From: Julian Gutierrez Oschmann Date: Fri, 11 Sep 2026 13:22:28 -0400 Subject: [PATCH] Add missing validation to actor template container image. (#1614) The container image must include the image digest. This validation was dropped during the migration of the ActorTemplate resource from CRD to Substrate API. --- .../internal/controlapi/actor_template.go | 19 ++++++++++++------ .../controlapi/actor_template_test.go | 20 ++++++++++++------- .../functionaltest/actor_template_test.go | 8 ++++---- .../controlapi/functionaltest/actor_test.go | 4 ++-- .../controlapi/zz_generated.validation.go | 4 ++++ docs/api-guide.md | 6 +++--- .../generated/ate.dev_sandboxconfigs.yaml | 3 +-- pkg/api/v1alpha1/sandboxconfig_types.go | 2 +- .../v1alpha1/sandboxconfig_validation_test.go | 2 +- pkg/proto/ateapipb/ateapi.pb.go | 12 +++++++---- pkg/proto/ateapipb/ateapi.proto | 12 +++++++---- 11 files changed, 58 insertions(+), 34 deletions(-) diff --git a/cmd/ateapi/internal/controlapi/actor_template.go b/cmd/ateapi/internal/controlapi/actor_template.go index 707b2572a..30d9f9958 100644 --- a/cmd/ateapi/internal/controlapi/actor_template.go +++ b/cmd/ateapi/internal/controlapi/actor_template.go @@ -269,16 +269,23 @@ func ValidateCustom_SystemInfoVolumeSource_DataSources(_ context.Context, _ oper return errs } -// ValidateCustom_ImageVolumeSource_Reference requires image references to -// be pinned by digest, because changing the image content under a fixed -// reference invalidates snapshots. -func ValidateCustom_ImageVolumeSource_Reference(_ context.Context, _ operation.Operation, fldPath *field.Path, value, _ *string) field.ErrorList { - if !strings.Contains(*value, "@") { - return field.ErrorList{field.Invalid(fldPath, *value, "must be pinned by digest (changing the image invalidates snapshots)")} +// validatePinnedImage requires an image reference to include a digest +// (e.g. "name@sha256:..."). +func validatePinnedImage(fldPath *field.Path, value string) field.ErrorList { + if !strings.Contains(value, "@") { + return field.ErrorList{field.Invalid(fldPath, value, "must include a digest")} } return nil } +func ValidateCustom_ImageVolumeSource_Reference(_ context.Context, _ operation.Operation, fldPath *field.Path, value, _ *string) field.ErrorList { + return validatePinnedImage(fldPath, *value) +} + +func ValidateCustom_Container_Image(_ context.Context, _ operation.Operation, fldPath *field.Path, value, _ *string) field.ErrorList { + return validatePinnedImage(fldPath, *value) +} + func ValidateCustom_ExternalVolumeTemplate_Capacity(_ context.Context, _ operation.Operation, fldPath *field.Path, value, _ *string) field.ErrorList { if _, err := resource.ParseQuantity(*value); err != nil { return field.ErrorList{field.Invalid(fldPath, *value, fmt.Sprintf("must be a Kubernetes resource quantity: %v", err))} diff --git a/cmd/ateapi/internal/controlapi/actor_template_test.go b/cmd/ateapi/internal/controlapi/actor_template_test.go index dabcc6fa6..2b88f4ad7 100644 --- a/cmd/ateapi/internal/controlapi/actor_template_test.go +++ b/cmd/ateapi/internal/controlapi/actor_template_test.go @@ -40,7 +40,7 @@ import ( func validActorTemplate(mutations ...func(*ateapipb.ActorTemplate)) *ateapipb.ActorTemplate { template := &ateapipb.ActorTemplate{ Metadata: &ateapipb.ResourceMetadata{Atespace: "ns1", Name: "tmpl-a"}, - Containers: []*ateapipb.Container{{Name: "main", Image: "example.com/app:v1"}}, + Containers: []*ateapipb.Container{{Name: "main", Image: "example.com/app:v1@sha256:abc"}}, SnapshotsConfig: &ateapipb.SnapshotsConfig{StorageLocation: "gs://my-bucket/snapshots"}, SandboxConfig: &ateapipb.SandboxConfig{SandboxClass: ateapipb.SandboxClass_SANDBOX_CLASS_GVISOR, ConfigName: "gvisor-default"}, } @@ -308,7 +308,7 @@ func TestCreateActorTemplateIgnoresServerOwnedFields(t *testing.T) { tmpl.Metadata.Uid = "11111111-1111-1111-1111-111111111111" tmpl.Metadata.Version = 42 tmpl.WorkerSelector = &ateapipb.Selector{MatchLabels: map[string]string{"pool": "default"}} - tmpl.Containers = []*ateapipb.Container{{Name: "main", Image: "example.com/app:v1"}} + tmpl.Containers = []*ateapipb.Container{{Name: "main", Image: "example.com/app:v1@sha256:abc"}} tmpl.SnapshotsConfig = &ateapipb.SnapshotsConfig{StorageLocation: "gs://my-bucket/snapshots"} tmpl.Resources = &ateapipb.Resources{Limits: []*ateapipb.Limits{{Name: "memory", Quantity: "1Gi"}}} // Server-owned status a client must not be able to set. @@ -550,14 +550,14 @@ func TestValidateActorTemplate(t *testing.T) { name: "too many containers", mutate: func(tmpl *ateapipb.ActorTemplate) { for i := 0; i < 10; i++ { - tmpl.Containers = append(tmpl.Containers, &ateapipb.Container{Name: fmt.Sprintf("c-%d", i), Image: "example.com/app:v1"}) + tmpl.Containers = append(tmpl.Containers, &ateapipb.Container{Name: fmt.Sprintf("c-%d", i), Image: "example.com/app:v1@sha256:abc"}) } }, want: field.ErrorList{field.TooMany(field.NewPath("containers"), 11, 10).WithOrigin("maxItems")}, }, { name: "duplicate container name", mutate: func(tmpl *ateapipb.ActorTemplate) { - tmpl.Containers = append(tmpl.Containers, &ateapipb.Container{Name: "main", Image: "example.com/other:v1"}) + tmpl.Containers = append(tmpl.Containers, &ateapipb.Container{Name: "main", Image: "example.com/other:v1@sha256:abc"}) }, want: field.ErrorList{field.Duplicate(field.NewPath("containers").Index(1), nil)}, }, { @@ -631,7 +631,7 @@ func TestValidateActorTemplate(t *testing.T) { }, { name: "the same path in different containers is allowed", mutate: func(tmpl *ateapipb.ActorTemplate) { - tmpl.Containers = append(tmpl.Containers, &ateapipb.Container{Name: "sidecar", Image: "example.com/side:v1"}) + tmpl.Containers = append(tmpl.Containers, &ateapipb.Container{Name: "sidecar", Image: "example.com/side:v1@sha256:abc"}) tmpl.Containers[0].VolumeMounts = []*ateapipb.VolumeMount{{Name: "data", MountPath: "/var/data"}} tmpl.Containers[1].VolumeMounts = []*ateapipb.VolumeMount{{Name: "data", MountPath: "/var/data"}} }, @@ -712,9 +712,15 @@ func TestValidateActorTemplate(t *testing.T) { }, { name: "image too long", mutate: func(tmpl *ateapipb.ActorTemplate) { - tmpl.Containers[0].Image = strings.Repeat("x", 513) + tmpl.Containers[0].Image = strings.Repeat("x", 513) + "@sha256:abc" }, want: field.ErrorList{field.TooLong(field.NewPath("containers").Index(0).Child("image"), nil, 512).WithOrigin("maxLength")}, + }, { + name: "container image missing digest", + mutate: func(tmpl *ateapipb.ActorTemplate) { + tmpl.Containers[0].Image = "example.com/app:v1" + }, + want: field.ErrorList{field.Invalid(field.NewPath("containers").Index(0).Child("image"), nil, "")}, }, { name: "container missing name", mutate: func(tmpl *ateapipb.ActorTemplate) { tmpl.Containers[0].Name = "" }, @@ -959,7 +965,7 @@ func TestValidateActorTemplate(t *testing.T) { }, want: field.ErrorList{field.Required(field.NewPath("volumes").Index(0).Child("image", "reference"), "")}, }, { - name: "image volume reference not pinned by digest", + name: "image volume reference missing digest", mutate: func(tmpl *ateapipb.ActorTemplate) { tmpl.Volumes = []*ateapipb.Volume{{Name: "tools", Image: &ateapipb.ImageVolumeSource{Reference: "example.com/app:v1"}}} }, diff --git a/cmd/ateapi/internal/controlapi/functionaltest/actor_template_test.go b/cmd/ateapi/internal/controlapi/functionaltest/actor_template_test.go index 4783d1ee0..d7b8216cf 100644 --- a/cmd/ateapi/internal/controlapi/functionaltest/actor_template_test.go +++ b/cmd/ateapi/internal/controlapi/functionaltest/actor_template_test.go @@ -34,7 +34,7 @@ func TestActorTemplateCRUD(t *testing.T) { created, err := tc.client.CreateActorTemplate(ctx, &ateapipb.CreateActorTemplateRequest{ ActorTemplate: &ateapipb.ActorTemplate{ Metadata: &ateapipb.ResourceMetadata{Atespace: testAtespace, Name: "tmpl-a"}, - Containers: []*ateapipb.Container{{Name: "main", Image: "example.com/app:v1"}}, + Containers: []*ateapipb.Container{{Name: "main", Image: "example.com/app:v1@sha256:abc"}}, SnapshotsConfig: &ateapipb.SnapshotsConfig{StorageLocation: "gs://my-bucket/snapshots"}, SandboxConfig: &ateapipb.SandboxConfig{ SandboxClass: ateapipb.SandboxClass_SANDBOX_CLASS_GVISOR, @@ -54,7 +54,7 @@ func TestActorTemplateCRUD(t *testing.T) { } want := &ateapipb.ActorTemplate{ Metadata: &ateapipb.ResourceMetadata{Atespace: testAtespace, Name: "tmpl-a", Version: 1}, - Containers: []*ateapipb.Container{{Name: "main", Image: "example.com/app:v1"}}, + Containers: []*ateapipb.Container{{Name: "main", Image: "example.com/app:v1@sha256:abc"}}, SnapshotsConfig: &ateapipb.SnapshotsConfig{StorageLocation: "gs://my-bucket/snapshots"}, SandboxConfig: &ateapipb.SandboxConfig{ SandboxClass: ateapipb.SandboxClass_SANDBOX_CLASS_GVISOR, @@ -70,7 +70,7 @@ func TestActorTemplateCRUD(t *testing.T) { _, err = tc.client.CreateActorTemplate(ctx, &ateapipb.CreateActorTemplateRequest{ ActorTemplate: &ateapipb.ActorTemplate{ Metadata: &ateapipb.ResourceMetadata{Atespace: testAtespace, Name: "tmpl-a"}, - Containers: []*ateapipb.Container{{Name: "main", Image: "example.com/app:v1"}}, + Containers: []*ateapipb.Container{{Name: "main", Image: "example.com/app:v1@sha256:abc"}}, SnapshotsConfig: &ateapipb.SnapshotsConfig{StorageLocation: "gs://my-bucket/snapshots"}, SandboxConfig: &ateapipb.SandboxConfig{SandboxClass: ateapipb.SandboxClass_SANDBOX_CLASS_GVISOR, ConfigName: "gvisor-default"}, }, @@ -126,7 +126,7 @@ func TestActorTemplateCRUD(t *testing.T) { _, err = tc.client.CreateActorTemplate(ctx, &ateapipb.CreateActorTemplateRequest{ ActorTemplate: &ateapipb.ActorTemplate{ Metadata: &ateapipb.ResourceMetadata{Atespace: testAtespace, Name: "tmpl-unnamed-config"}, - Containers: []*ateapipb.Container{{Name: "main", Image: "example.com/app:v1"}}, + Containers: []*ateapipb.Container{{Name: "main", Image: "example.com/app:v1@sha256:abc"}}, SnapshotsConfig: &ateapipb.SnapshotsConfig{StorageLocation: "gs://my-bucket/snapshots"}, SandboxConfig: &ateapipb.SandboxConfig{SandboxClass: ateapipb.SandboxClass_SANDBOX_CLASS_GVISOR}, }, diff --git a/cmd/ateapi/internal/controlapi/functionaltest/actor_test.go b/cmd/ateapi/internal/controlapi/functionaltest/actor_test.go index 855d7ad63..9617c4c48 100644 --- a/cmd/ateapi/internal/controlapi/functionaltest/actor_test.go +++ b/cmd/ateapi/internal/controlapi/functionaltest/actor_test.go @@ -184,7 +184,7 @@ func TestCreateActor_SubstrateTemplateRef(t *testing.T) { if _, err := tc.client.CreateActorTemplate(ctx, &ateapipb.CreateActorTemplateRequest{ ActorTemplate: &ateapipb.ActorTemplate{ Metadata: &ateapipb.ResourceMetadata{Atespace: testAtespace, Name: "sub-tmpl"}, - Containers: []*ateapipb.Container{{Name: "main", Image: "example.com/app:v1"}}, + Containers: []*ateapipb.Container{{Name: "main", Image: "example.com/app:v1@sha256:abc"}}, SnapshotsConfig: &ateapipb.SnapshotsConfig{StorageLocation: "gs://my-bucket/snapshots"}, SandboxConfig: &ateapipb.SandboxConfig{SandboxClass: ateapipb.SandboxClass_SANDBOX_CLASS_GVISOR, ConfigName: "gvisor-default"}, }, @@ -753,7 +753,7 @@ func TestUpdateActor_RepointTemplate(t *testing.T) { Metadata: &ateapipb.ResourceMetadata{Atespace: testAtespace, Name: name}, Containers: []*ateapipb.Container{{ Name: "main", - Image: "example.com/app:v1", + Image: "example.com/app:v1@sha256:abc", VolumeMounts: []*ateapipb.VolumeMount{{Name: "data", MountPath: tmpl.mountPath}}, }}, Volumes: tmpl.volumes, diff --git a/cmd/ateapi/internal/controlapi/zz_generated.validation.go b/cmd/ateapi/internal/controlapi/zz_generated.validation.go index 3bbc3dbc9..82ce14ec8 100644 --- a/cmd/ateapi/internal/controlapi/zz_generated.validation.go +++ b/cmd/ateapi/internal/controlapi/zz_generated.validation.go @@ -1214,6 +1214,10 @@ func Validate_Container( if earlyReturn { return // do not proceed } + // custom validation + if e := ValidateCustom_Container_Image(ctx, op, fldPath, obj, oldObj); len(e) != 0 { + errs = append(errs, e...) + } if e := validate.MaxLength(ctx, op, fldPath, obj, oldObj, 512); len(e) != 0 { errs = append(errs, e...) } diff --git a/docs/api-guide.md b/docs/api-guide.md index d76bf4c27..afd5dfc59 100644 --- a/docs/api-guide.md +++ b/docs/api-guide.md @@ -240,7 +240,7 @@ Each entry in `containers` describes one process to run in the actor's sandbox. | Field | Type | Description | | :--- | :--- | :--- | | `name` | `string` | **Required.** DNS-label-safe container name. | -| `image` | `string` | **Required.** Must be pinned by digest (`...@sha256:...`) — changing the image invalidates snapshots. | +| `image` | `string` | **Required.** Container image name; must include a digest (`name@sha256:...`). | | `command` | `[]string` | Optional. Entrypoint array. If unset, the image's `ENTRYPOINT` is used. If set, it replaces **both** the image's `ENTRYPOINT` and `CMD`. | | `args` | `[]string` | Optional. Arguments to the entrypoint. If unset, the image's `CMD` is used (unless `command` is set, which discards the image's `CMD`). If set, it replaces the image's `CMD`. | | `env` | `[]EnvVar` | Optional. Literal `value` entries. | @@ -330,7 +330,7 @@ metadata: name: secret-agent containers: - name: agent - image: gcr.io/my-project/my-agent:latest + image: gcr.io/my-project/my-agent@sha256:7f28ab0e... # Optional: gate Run/Restore on the agent's HTTP readiness endpoint. # See "Container Readiness Probe (readyz)" above. readyz: @@ -396,7 +396,7 @@ This means a single, cluster-managed config pins the sandbox runtime version for | Field | Type | Description | | :--- | :--- | :--- | | `sandboxClass` | `string` | **Required.** Runtime family this config applies to: `gvisor` (default) or `microvm`. An `ActorTemplate` only uses `SandboxConfig`s whose `sandboxClass` matches its own. | -| `pauseImage` | `string` | **Required.** The image for the sandbox's root container (e.g. `registry.k8s.io/pause`, or `gcr.io/gke-release/pause` on GKE). Must be pinned by digest (`...@sha256:...`) — it is recorded in each snapshot's manifest so a restore rebuilds the sandbox from the same image. | +| `pauseImage` | `string` | **Required.** The image for the sandbox's root container (e.g. `registry.k8s.io/pause`, or `gcr.io/gke-release/pause` on GKE). Must include a digest (`...@sha256:...`) — it is recorded in each snapshot's manifest so a restore rebuilds the sandbox from the same image. | | `assets` | `map[arch]map[name]AssetFile` | Optional. Content-addressed files atelet fetches, keyed by architecture (`amd64`, `arm64`) then asset name. gVisor expects a `gvisor` asset (the release's `gvisor.tar.zstd`), which atelet auto-extracts. A micro-VM backend expects several. Each `AssetFile` is a `{ url, sha256 }` pair. | A cluster-wide gVisor `SandboxConfig` (`gvisor-default`) is installed with the platform, so gVisor templates can name it via `sandboxConfig.configName` without any extra setup. diff --git a/manifests/ate-install/generated/ate.dev_sandboxconfigs.yaml b/manifests/ate-install/generated/ate.dev_sandboxconfigs.yaml index 76161dcc0..583edd592 100644 --- a/manifests/ate-install/generated/ate.dev_sandboxconfigs.yaml +++ b/manifests/ate-install/generated/ate.dev_sandboxconfigs.yaml @@ -117,8 +117,7 @@ spec: - [2] registry.k8s.io/pause:3.10.2@sha256:f548e0e8e3dc1896ca956272154dde3314e8cc4fde0a57577ee9fa1c63f5baf4 type: string x-kubernetes-validations: - - message: All images must be pinned (changing the image invalidates - snapshots) + - message: All images must include a digest rule: self.contains('@') sandboxClass: default: gvisor diff --git a/pkg/api/v1alpha1/sandboxconfig_types.go b/pkg/api/v1alpha1/sandboxconfig_types.go index 3c8c5ad5f..b63634f63 100644 --- a/pkg/api/v1alpha1/sandboxconfig_types.go +++ b/pkg/api/v1alpha1/sandboxconfig_types.go @@ -73,7 +73,7 @@ type SandboxConfigSpec struct { // - [2] registry.k8s.io/pause:3.10.2@sha256:f548e0e8e3dc1896ca956272154dde3314e8cc4fde0a57577ee9fa1c63f5baf4 // // +required - // +kubebuilder:validation:XValidation:rule="self.contains('@')",message="All images must be pinned (changing the image invalidates snapshots)" + // +kubebuilder:validation:XValidation:rule="self.contains('@')",message="All images must include a digest" PauseImage string `json:"pauseImage"` // Assets is the set of files atelet fetches for this runtime, keyed first by diff --git a/pkg/api/v1alpha1/sandboxconfig_validation_test.go b/pkg/api/v1alpha1/sandboxconfig_validation_test.go index 374edf966..91d233a3c 100644 --- a/pkg/api/v1alpha1/sandboxconfig_validation_test.go +++ b/pkg/api/v1alpha1/sandboxconfig_validation_test.go @@ -202,7 +202,7 @@ func TestSandboxConfigValidation(t *testing.T) { name: "unpinned pauseImage", sc: withPauseImage(sandboxConfig("bad-unpinned-pause", SandboxClassGvisor, map[string]map[string]AssetFile{"amd64": {"gvisor": gvisorAsset()}}), "registry.k8s.io/pause:3.10.2"), wantErr: true, - errMsg: "All images must be pinned", + errMsg: "All images must include a digest", }} for _, tt := range tests { diff --git a/pkg/proto/ateapipb/ateapi.pb.go b/pkg/proto/ateapipb/ateapi.pb.go index 8f04416d2..6e50a50d1 100644 --- a/pkg/proto/ateapipb/ateapi.pb.go +++ b/pkg/proto/ateapipb/ateapi.pb.go @@ -2593,9 +2593,12 @@ type Container struct { // +k8s:required // +k8s:format=k8s-short-name Name string `protobuf:"bytes,1,opt,name=name,proto3" json:"name,omitempty"` + // image is the container image name. Must include a digest + // (e.g. "name@sha256:..."). + // // +k8s:required // +k8s:maxLength=512 # matches ImageVolumeSource.reference's bound - // TODO: validate that this is a well-formed image reference + // +k8s:customValidation Image string `protobuf:"bytes,2,opt,name=image,proto3" json:"image,omitempty"` // Entrypoint array; when set, the image's ENTRYPOINT and CMD are both // ignored and the process argv is command + args. Unlike Kubernetes, @@ -3144,15 +3147,16 @@ func (x *Volume) GetImage() *ImageVolumeSource { } // ImageVolumeSource mounts the contents of an OCI image, read-only. The -// reference must be pinned by digest: changing the image invalidates +// reference must include a digest: changing the image invalidates // snapshots. type ImageVolumeSource struct { state protoimpl.MessageState `protogen:"open.v1"` - // reference is the OCI image reference, pinned by digest. + // reference is the OCI image reference. Must include a digest + // (e.g. "name@sha256:..."). // // +k8s:required // +k8s:maxLength=512 - // +k8s:customValidation # must be pinned by digest; no contains tag exists + // +k8s:customValidation Reference string `protobuf:"bytes,1,opt,name=reference,proto3" json:"reference,omitempty"` unknownFields protoimpl.UnknownFields sizeCache protoimpl.SizeCache diff --git a/pkg/proto/ateapipb/ateapi.proto b/pkg/proto/ateapipb/ateapi.proto index 00a3d35b3..a18f52c9a 100644 --- a/pkg/proto/ateapipb/ateapi.proto +++ b/pkg/proto/ateapipb/ateapi.proto @@ -898,9 +898,12 @@ message Container { // +k8s:format=k8s-short-name string name = 1; + // image is the container image name. Must include a digest + // (e.g. "name@sha256:..."). + // // +k8s:required // +k8s:maxLength=512 # matches ImageVolumeSource.reference's bound - // TODO: validate that this is a well-formed image reference + // +k8s:customValidation string image = 2; // Entrypoint array; when set, the image's ENTRYPOINT and CMD are both @@ -1073,14 +1076,15 @@ message Volume { } // ImageVolumeSource mounts the contents of an OCI image, read-only. The -// reference must be pinned by digest: changing the image invalidates +// reference must include a digest: changing the image invalidates // snapshots. message ImageVolumeSource { - // reference is the OCI image reference, pinned by digest. + // reference is the OCI image reference. Must include a digest + // (e.g. "name@sha256:..."). // // +k8s:required // +k8s:maxLength=512 - // +k8s:customValidation # must be pinned by digest; no contains tag exists + // +k8s:customValidation string reference = 1; }