From 0691c39f6a8f50797e1084d61ff5259d8c951f31 Mon Sep 17 00:00:00 2001 From: shrutiyam-glitch Date: Thu, 10 Sep 2026 10:02:14 -0700 Subject: [PATCH 1/2] Add custom validation for create/update time fields --- cmd/ateapi/internal/controlapi/validate.go | 25 ++++++++++++++++ .../internal/controlapi/validation_test.go | 29 +++++++++++++++++-- .../controlapi/zz_generated.validation.go | 5 ++++ pkg/proto/ateapipb/ateapi.pb.go | 5 ++-- pkg/proto/ateapipb/ateapi.proto | 5 ++-- 5 files changed, 61 insertions(+), 8 deletions(-) diff --git a/cmd/ateapi/internal/controlapi/validate.go b/cmd/ateapi/internal/controlapi/validate.go index 2bb767f415..3de5e62892 100644 --- a/cmd/ateapi/internal/controlapi/validate.go +++ b/cmd/ateapi/internal/controlapi/validate.go @@ -77,6 +77,31 @@ func ateDeepEqual[T any](a, b T) bool { return reflect.DeepEqual(a, b) } +// ValidateCustom_ResourceMetadata checks the server-stamped timestamps: each, +// when set, must be a valid google.protobuf.Timestamp, and update_time must +// not precede create_time. Both fields are scrubbed from input, so a +// violation here is a server stamping bug surfaced by the final-object +// validation pass, not a client error. +func ValidateCustom_ResourceMetadata(_ context.Context, _ operation.Operation, fldPath *field.Path, obj, _ *ateapipb.ResourceMetadata) field.ErrorList { + var errs field.ErrorList + createTimeValid := false + if ct := obj.GetCreateTime(); ct != nil { + if err := ct.CheckValid(); err != nil { + errs = append(errs, field.Invalid(fldPath.Child("create_time"), ct.String(), err.Error())) + } else { + createTimeValid = true + } + } + if ut := obj.GetUpdateTime(); ut != nil { + if err := ut.CheckValid(); err != nil { + errs = append(errs, field.Invalid(fldPath.Child("update_time"), ut.String(), err.Error())) + } else if createTimeValid && ut.AsTime().Before(obj.GetCreateTime().AsTime()) { + errs = append(errs, field.Invalid(fldPath.Child("update_time"), ut.String(), "must not precede create_time")) + } + } + return errs +} + // This is needed because DV doesn't have a standard format for IP addresses yet. func ValidateCustom_WorkerAssignment_WorkerPodIp(_ context.Context, _ operation.Operation, fldPath *field.Path, value, _ *string) field.ErrorList { return validation.IsValidIP(fldPath, *value) diff --git a/cmd/ateapi/internal/controlapi/validation_test.go b/cmd/ateapi/internal/controlapi/validation_test.go index ac94ccba3b..5e8953c18a 100644 --- a/cmd/ateapi/internal/controlapi/validation_test.go +++ b/cmd/ateapi/internal/controlapi/validation_test.go @@ -93,6 +93,26 @@ func TestValidateResourceMetadataCreate(t *testing.T) { name: "unspecified updateTime", obj: valid(func(rm *ateapipb.ResourceMetadata) { rm.UpdateTime = nil }), want: nil, + }, { + name: "invalid createTime: seconds out of range", + obj: valid(func(rm *ateapipb.ResourceMetadata) { rm.CreateTime = ×tamppb.Timestamp{Seconds: 253402300800} }), + want: field.ErrorList{field.Invalid(field.NewPath("create_time"), nil, "")}, + }, { + name: "invalid updateTime: negative nanos", + obj: valid(func(rm *ateapipb.ResourceMetadata) { rm.UpdateTime = ×tamppb.Timestamp{Seconds: 5309, Nanos: -1} }), + want: field.ErrorList{field.Invalid(field.NewPath("update_time"), nil, "")}, + }, { + name: "invalid updateTime: precedes createTime", + obj: valid(func(rm *ateapipb.ResourceMetadata) { rm.UpdateTime = ×tamppb.Timestamp{Seconds: 866} }), + want: field.ErrorList{field.Invalid(field.NewPath("update_time"), nil, "")}, + }, { + name: "valid updateTime: equals createTime", + obj: valid(func(rm *ateapipb.ResourceMetadata) { rm.UpdateTime = ×tamppb.Timestamp{Seconds: 867} }), + want: nil, + }, { + name: "valid updateTime: set without createTime", + obj: valid(func(rm *ateapipb.ResourceMetadata) { rm.CreateTime = nil }), + want: nil, }} for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -201,9 +221,14 @@ func TestValidateResourceMetadataUpdate(t *testing.T) { want: field.ErrorList{field.Invalid(field.NewPath("update_time"), nil, "").WithOrigin("update")}, }, { name: "update_time: changed to valid", - oldObj: valid(func(rm *ateapipb.ResourceMetadata) { rm.UpdateTime.Seconds = 123 }), - newObj: valid(func(rm *ateapipb.ResourceMetadata) { rm.UpdateTime.Seconds = 456 }), + oldObj: valid(func(rm *ateapipb.ResourceMetadata) { rm.UpdateTime.Seconds = 1000 }), + newObj: valid(func(rm *ateapipb.ResourceMetadata) { rm.UpdateTime.Seconds = 6000 }), want: nil, + }, { + name: "update_time: changed to precede create_time", + oldObj: valid(), + newObj: valid(func(rm *ateapipb.ResourceMetadata) { rm.UpdateTime.Seconds = 866 }), + want: field.ErrorList{field.Invalid(field.NewPath("update_time"), nil, "")}, }} for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/cmd/ateapi/internal/controlapi/zz_generated.validation.go b/cmd/ateapi/internal/controlapi/zz_generated.validation.go index 3bbc3dbc9e..3484915c9d 100644 --- a/cmd/ateapi/internal/controlapi/zz_generated.validation.go +++ b/cmd/ateapi/internal/controlapi/zz_generated.validation.go @@ -5080,6 +5080,11 @@ func Validate_ResourceMetadata( ctx context.Context, op operation.Operation, fldPath *field.Path, obj, oldObj *ateapipb.ResourceMetadata) (errs field.ErrorList) { + // custom validation + if e := ValidateCustom_ResourceMetadata(ctx, op, fldPath, obj, oldObj); len(e) != 0 { + errs = append(errs, e...) + } + { // field ateapipb.ResourceMetadata.Atespace fn := func( fldPath *field.Path, diff --git a/pkg/proto/ateapipb/ateapi.pb.go b/pkg/proto/ateapipb/ateapi.pb.go index 8f04416d2e..21016981be 100644 --- a/pkg/proto/ateapipb/ateapi.pb.go +++ b/pkg/proto/ateapipb/ateapi.pb.go @@ -717,6 +717,8 @@ func (x *Selector) GetMatchLabels() map[string]string { } // ResourceMetadata holds the common fields carried by every Substrate resource. +// +// +k8s:customValidation # timestamps must be valid, and update_time must not precede create_time type ResourceMetadata struct { state protoimpl.MessageState `protogen:"open.v1"` // atespace is the namespace the resource belongs to. Empty for global-scoped @@ -761,7 +763,6 @@ type ResourceMetadata struct { // // +k8s:optional // +k8s:immutable - // TODO: validate that this is a valid timestamp CreateTime *timestamppb.Timestamp `protobuf:"bytes,5,opt,name=create_time,json=createTime,proto3" json:"create_time,omitempty"` // update_time is the time the resource was last updated. // @@ -769,8 +770,6 @@ type ResourceMetadata struct { // // +k8s:optional // +k8s:update=NoUnset - // TODO: validate that this is a valid timestamp - // TODO: validate that UpdateTime >= CreateTime UpdateTime *timestamppb.Timestamp `protobuf:"bytes,6,opt,name=update_time,json=updateTime,proto3" json:"update_time,omitempty"` unknownFields protoimpl.UnknownFields sizeCache protoimpl.SizeCache diff --git a/pkg/proto/ateapipb/ateapi.proto b/pkg/proto/ateapipb/ateapi.proto index 00a3d35b3b..0624d61ebe 100644 --- a/pkg/proto/ateapipb/ateapi.proto +++ b/pkg/proto/ateapipb/ateapi.proto @@ -207,6 +207,8 @@ message Selector { } // ResourceMetadata holds the common fields carried by every Substrate resource. +// +// +k8s:customValidation # timestamps must be valid, and update_time must not precede create_time message ResourceMetadata { // atespace is the namespace the resource belongs to. Empty for global-scoped // resources. Caller-specified at creation and immutable thereafter. @@ -254,7 +256,6 @@ message ResourceMetadata { // // +k8s:optional // +k8s:immutable - // TODO: validate that this is a valid timestamp google.protobuf.Timestamp create_time = 5; // update_time is the time the resource was last updated. @@ -263,8 +264,6 @@ message ResourceMetadata { // // +k8s:optional // +k8s:update=NoUnset - // TODO: validate that this is a valid timestamp - // TODO: validate that UpdateTime >= CreateTime google.protobuf.Timestamp update_time = 6; } From 5803f5ea52ef6a0dc62e0f71093960422e1b48e9 Mon Sep 17 00:00:00 2001 From: shrutiyam-glitch Date: Thu, 10 Sep 2026 11:29:22 -0700 Subject: [PATCH 2/2] Add dv for Container.image --- .../internal/controlapi/actor_template.go | 30 ++++++++++-- .../controlapi/actor_template_test.go | 49 +++++++++++++++++-- .../controlapi/functionaltest/actor_test.go | 4 +- .../controlapi/functionaltest/common_test.go | 6 +-- .../controlapi/zz_generated.validation.go | 4 ++ go.mod | 2 +- pkg/proto/ateapipb/ateapi.pb.go | 7 ++- pkg/proto/ateapipb/ateapi.proto | 7 ++- 8 files changed, 92 insertions(+), 17 deletions(-) diff --git a/cmd/ateapi/internal/controlapi/actor_template.go b/cmd/ateapi/internal/controlapi/actor_template.go index 707b2572a1..17180b3691 100644 --- a/cmd/ateapi/internal/controlapi/actor_template.go +++ b/cmd/ateapi/internal/controlapi/actor_template.go @@ -21,6 +21,8 @@ import ( "regexp" "strings" + "github.com/distribution/reference" + "github.com/agent-substrate/substrate/cmd/ateapi/internal/store" "github.com/agent-substrate/substrate/internal/resources" "github.com/agent-substrate/substrate/internal/volumepath" @@ -269,11 +271,31 @@ 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. +// ValidateCustom_Container_Image checks that the image is a well-formed +// OCI image reference, using the same grammar the container runtimes parse +// with. Unlike image volumes, a tag reference is allowed. +func ValidateCustom_Container_Image(_ context.Context, _ operation.Operation, fldPath *field.Path, value, _ *string) field.ErrorList { + if *value == "" { + return nil // required is enforced by tags + } + if _, err := reference.ParseNormalizedNamed(*value); err != nil { + return field.ErrorList{field.Invalid(fldPath, *value, fmt.Sprintf("must be a well-formed image reference: %v", err))} + } + return nil +} + +// ValidateCustom_ImageVolumeSource_Reference requires a well-formed image +// reference 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, "@") { + if *value == "" { + return nil // required is enforced by tags + } + ref, err := reference.ParseNormalizedNamed(*value) + if err != nil { + return field.ErrorList{field.Invalid(fldPath, *value, fmt.Sprintf("must be a well-formed image reference: %v", err))} + } + if _, ok := ref.(reference.Digested); !ok { return field.ErrorList{field.Invalid(fldPath, *value, "must be pinned by digest (changing the image invalidates snapshots)")} } return nil diff --git a/cmd/ateapi/internal/controlapi/actor_template_test.go b/cmd/ateapi/internal/controlapi/actor_template_test.go index dabcc6fa69..f1e39f5c49 100644 --- a/cmd/ateapi/internal/controlapi/actor_template_test.go +++ b/cmd/ateapi/internal/controlapi/actor_template_test.go @@ -714,7 +714,38 @@ func TestValidateActorTemplate(t *testing.T) { mutate: func(tmpl *ateapipb.ActorTemplate) { tmpl.Containers[0].Image = strings.Repeat("x", 513) }, - want: field.ErrorList{field.TooLong(field.NewPath("containers").Index(0).Child("image"), nil, 512).WithOrigin("maxLength")}, + want: field.ErrorList{ + field.Invalid(field.NewPath("containers").Index(0).Child("image"), nil, ""), + field.TooLong(field.NewPath("containers").Index(0).Child("image"), nil, 512).WithOrigin("maxLength"), + }, + }, { + name: "valid image: bare repository", + mutate: func(tmpl *ateapipb.ActorTemplate) { + tmpl.Containers[0].Image = "ubuntu" + }, + }, { + name: "valid image: pinned by digest", + mutate: func(tmpl *ateapipb.ActorTemplate) { + tmpl.Containers[0].Image = "example.com/app@sha256:e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855" + }, + }, { + name: "invalid image: uppercase repository", + 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: "invalid image: malformed digest", + mutate: func(tmpl *ateapipb.ActorTemplate) { + tmpl.Containers[0].Image = "example.com/app@sha256:abc" + }, + want: field.ErrorList{field.Invalid(field.NewPath("containers").Index(0).Child("image"), nil, "")}, + }, { + name: "invalid image: empty tag", + mutate: func(tmpl *ateapipb.ActorTemplate) { + tmpl.Containers[0].Image = "example.com/app:" + }, + 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 = "" }, @@ -943,14 +974,14 @@ func TestValidateActorTemplate(t *testing.T) { tmpl.Volumes = []*ateapipb.Volume{{ Name: "scratch", DurableDir: &ateapipb.DurableDirVolumeSource{}, - Image: &ateapipb.ImageVolumeSource{Reference: "example.com/app@sha256:abc"}, + Image: &ateapipb.ImageVolumeSource{Reference: "example.com/app@sha256:e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855"}, }} }, want: field.ErrorList{field.Invalid(field.NewPath("volumes").Index(0), nil, "one of").WithOrigin("union")}, }, { name: "valid image volume", mutate: func(tmpl *ateapipb.ActorTemplate) { - tmpl.Volumes = []*ateapipb.Volume{{Name: "tools", Image: &ateapipb.ImageVolumeSource{Reference: "example.com/app@sha256:abc"}}} + tmpl.Volumes = []*ateapipb.Volume{{Name: "tools", Image: &ateapipb.ImageVolumeSource{Reference: "example.com/app@sha256:e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855"}}} }, }, { name: "image volume missing reference", @@ -964,6 +995,18 @@ func TestValidateActorTemplate(t *testing.T) { tmpl.Volumes = []*ateapipb.Volume{{Name: "tools", Image: &ateapipb.ImageVolumeSource{Reference: "example.com/app:v1"}}} }, want: field.ErrorList{field.Invalid(field.NewPath("volumes").Index(0).Child("image", "reference"), nil, "")}, + }, { + name: "image volume reference with malformed digest", + mutate: func(tmpl *ateapipb.ActorTemplate) { + tmpl.Volumes = []*ateapipb.Volume{{Name: "tools", Image: &ateapipb.ImageVolumeSource{Reference: "example.com/app@sha256:abc"}}} + }, + want: field.ErrorList{field.Invalid(field.NewPath("volumes").Index(0).Child("image", "reference"), nil, "")}, + }, { + name: "image volume reference not a reference at all", + mutate: func(tmpl *ateapipb.ActorTemplate) { + tmpl.Volumes = []*ateapipb.Volume{{Name: "tools", Image: &ateapipb.ImageVolumeSource{Reference: "@"}}} + }, + want: field.ErrorList{field.Invalid(field.NewPath("volumes").Index(0).Child("image", "reference"), nil, "")}, }, { name: "valid external volume template", mutate: func(tmpl *ateapipb.ActorTemplate) { diff --git a/cmd/ateapi/internal/controlapi/functionaltest/actor_test.go b/cmd/ateapi/internal/controlapi/functionaltest/actor_test.go index 855d7ad633..d9d0a3d2bf 100644 --- a/cmd/ateapi/internal/controlapi/functionaltest/actor_test.go +++ b/cmd/ateapi/internal/controlapi/functionaltest/actor_test.go @@ -313,7 +313,7 @@ func TestCreateActor_RejectsSnapshotWithExternalVolumes(t *testing.T) { ConfigName: "gvisor-default", }, Containers: []*ateapipb.Container{{ - Name: "main", Image: "main@sha256:abc", VolumeMounts: []*ateapipb.VolumeMount{{Name: "data", MountPath: "/data"}}, + Name: "main", Image: "main@sha256:e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855", VolumeMounts: []*ateapipb.VolumeMount{{Name: "data", MountPath: "/data"}}, }}, Volumes: []*ateapipb.Volume{{ Name: "data", @@ -1865,7 +1865,7 @@ func TestResumeActorPassesLiteralEnv(t *testing.T) { createTemplateWithContainers(t, tc, ns, []*ateapipb.Container{ { Name: "main", - Image: "main@sha256:abc", + Image: "main@sha256:e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855", Command: []string{"/main"}, Env: []*ateapipb.EnvVar{ { diff --git a/cmd/ateapi/internal/controlapi/functionaltest/common_test.go b/cmd/ateapi/internal/controlapi/functionaltest/common_test.go index 05d7a3a615..5ddc2cc233 100644 --- a/cmd/ateapi/internal/controlapi/functionaltest/common_test.go +++ b/cmd/ateapi/internal/controlapi/functionaltest/common_test.go @@ -343,7 +343,7 @@ func createTemplate(t *testing.T, tc *testContext, ns string) *ateapipb.ActorTem return createTemplateWithContainers(t, tc, ns, []*ateapipb.Container{ { Name: "main", - Image: "main@sha256:abc", + Image: "main@sha256:e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855", Command: []string{"/main"}, }, }) @@ -369,7 +369,7 @@ func createTemplateWithVolumes(t *testing.T, tc *testContext, ns string, volumes return createTemplateWithContainersAndVolumes(t, tc, ns, []*ateapipb.Container{ { Name: "main", - Image: "main@sha256:abc", + Image: "main@sha256:e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855", Command: []string{"/main"}, VolumeMounts: mounts, }, @@ -529,7 +529,7 @@ func createTemplateWithSelector(t *testing.T, tc *testContext, name string, sele ConfigName: "gvisor-default", }, Containers: []*ateapipb.Container{ - {Name: "main", Image: "main@sha256:abc", Command: []string{"/main"}}, + {Name: "main", Image: "main@sha256:e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855", Command: []string{"/main"}}, }, WorkerSelector: selector, }, diff --git a/cmd/ateapi/internal/controlapi/zz_generated.validation.go b/cmd/ateapi/internal/controlapi/zz_generated.validation.go index 3484915c9d..a664116eaa 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/go.mod b/go.mod index 02ac3b84b3..4edf49121d 100644 --- a/go.mod +++ b/go.mod @@ -18,6 +18,7 @@ require ( github.com/aws/smithy-go v1.25.1 github.com/container-storage-interface/spec v1.12.0 github.com/containerd/ttrpc v1.2.8 + github.com/distribution/reference v0.6.0 github.com/envoyproxy/go-control-plane v0.14.0 github.com/envoyproxy/go-control-plane/envoy v1.37.1-0.20260812071801-353463cc7248 github.com/fsnotify/fsnotify v1.9.0 @@ -111,7 +112,6 @@ require ( github.com/containerd/platforms v0.2.1 // indirect github.com/cpuguy83/dockercfg v0.3.2 // indirect github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc // indirect - github.com/distribution/reference v0.6.0 // indirect github.com/docker/cli v29.5.3+incompatible // indirect github.com/docker/docker-credential-helpers v0.9.3 // indirect github.com/docker/go-connections v0.7.0 // indirect diff --git a/pkg/proto/ateapipb/ateapi.pb.go b/pkg/proto/ateapipb/ateapi.pb.go index 21016981be..8ce712b7cd 100644 --- a/pkg/proto/ateapipb/ateapi.pb.go +++ b/pkg/proto/ateapipb/ateapi.pb.go @@ -2592,9 +2592,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 OCI image reference the container runs: + // [registry/]repository[:tag][@digest]. + // // +k8s:required // +k8s:maxLength=512 # matches ImageVolumeSource.reference's bound - // TODO: validate that this is a well-formed image reference + // +k8s:customValidation # must be a well-formed image reference 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, @@ -3151,7 +3154,7 @@ type ImageVolumeSource struct { // // +k8s:required // +k8s:maxLength=512 - // +k8s:customValidation # must be pinned by digest; no contains tag exists + // +k8s:customValidation # must be a well-formed image reference, pinned by digest 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 0624d61ebe..4016a323b8 100644 --- a/pkg/proto/ateapipb/ateapi.proto +++ b/pkg/proto/ateapipb/ateapi.proto @@ -897,9 +897,12 @@ message Container { // +k8s:format=k8s-short-name string name = 1; + // image is the OCI image reference the container runs: + // [registry/]repository[:tag][@digest]. + // // +k8s:required // +k8s:maxLength=512 # matches ImageVolumeSource.reference's bound - // TODO: validate that this is a well-formed image reference + // +k8s:customValidation # must be a well-formed image reference string image = 2; // Entrypoint array; when set, the image's ENTRYPOINT and CMD are both @@ -1079,7 +1082,7 @@ message ImageVolumeSource { // // +k8s:required // +k8s:maxLength=512 - // +k8s:customValidation # must be pinned by digest; no contains tag exists + // +k8s:customValidation # must be a well-formed image reference, pinned by digest string reference = 1; }