Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions cmd/ateapi/internal/apivalidation/actor.go
Original file line number Diff line number Diff line change
Expand Up @@ -134,9 +134,9 @@ func ValidateCustom_WorkerAssignment_WorkerPodIps(_ context.Context, _ operation
return validateDualStackIPs(fldPath, value)
}

// ValidateCustom_ExternalVolume_VolumeType checks that a volume type string is well-formed.
// ValidateCustom_ActorVolumeStatus_VolumeType checks that a volume type string is well-formed.
// It allows an optional "substrate.io/" prefix, followed by a valid DNS-1123 subdomain.
func ValidateCustom_ExternalVolume_VolumeType(_ context.Context, _ operation.Operation, fldPath *field.Path, value, _ *string) field.ErrorList {
func ValidateCustom_ActorVolumeStatus_VolumeType(_ context.Context, _ operation.Operation, fldPath *field.Path, value, _ *string) field.ErrorList {
if value == nil || *value == "" {
return nil
}
Expand All @@ -148,9 +148,9 @@ func ValidateCustom_ExternalVolume_VolumeType(_ context.Context, _ operation.Ope
return errs
}

// ValidateCustom_ExternalVolume_StorageVolumeId checks that an external volume's storage ID does not
// ValidateCustom_ActorVolumeStatus_StorageVolumeId checks that an external volume's storage ID does not
// contain control characters (U+0000-U+0008, U+000B, U+000C, U+000E-U+001F, U+007F-U+009F).
func ValidateCustom_ExternalVolume_StorageVolumeId(_ context.Context, _ operation.Operation, fldPath *field.Path, value, _ *string) field.ErrorList {
func ValidateCustom_ActorVolumeStatus_StorageVolumeId(_ context.Context, _ operation.Operation, fldPath *field.Path, value, _ *string) field.ErrorList {
if value == nil || *value == "" {
return nil
}
Expand Down
14 changes: 7 additions & 7 deletions cmd/ateapi/internal/apivalidation/actor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -643,9 +643,9 @@ func TestValidateActorUpdate(t *testing.T) {
"too many external_volumes",
validInput(),
validOutput(withStatus(func(s *ateapipb.ActorStatus) {
vols := make([]*ateapipb.ExternalVolume, 33)
vols := make([]*ateapipb.ActorVolumeStatus, 33)
for i := range vols {
vols[i] = &ateapipb.ExternalVolume{Name: fmt.Sprintf("vol-%d", i), VolumeType: "substrate.io/mock"}
vols[i] = &ateapipb.ActorVolumeStatus{Name: fmt.Sprintf("vol-%d", i), VolumeType: "substrate.io/mock"}
}
s.ExternalVolumes = vols
})),
Expand All @@ -656,14 +656,14 @@ func TestValidateActorUpdate(t *testing.T) {
"adding a volume on update is allowed",
validInput(withStatus()),
validOutput(withStatus(func(s *ateapipb.ActorStatus) {
s.ExternalVolumes = []*ateapipb.ExternalVolume{{Name: "vol-a", VolumeType: "substrate.io/mock"}}
s.ExternalVolumes = []*ateapipb.ActorVolumeStatus{{Name: "vol-a", VolumeType: "substrate.io/mock"}}
})),
nil,
}, {
"duplicate external_volumes name",
validInput(withStatus()),
validOutput(withStatus(func(s *ateapipb.ActorStatus) {
s.ExternalVolumes = []*ateapipb.ExternalVolume{
s.ExternalVolumes = []*ateapipb.ActorVolumeStatus{
{Name: "vol-a", VolumeType: "substrate.io/mock"},
{Name: "vol-a", VolumeType: "substrate.io/mock"},
}
Expand All @@ -672,14 +672,14 @@ func TestValidateActorUpdate(t *testing.T) {
}, {
"provisioning transition on an existing volume is valid",
validInput(withStatus(func(s *ateapipb.ActorStatus) {
s.ExternalVolumes = []*ateapipb.ExternalVolume{{Name: "vol-a", VolumeType: "substrate.io/mock", Status: ateapipb.ExternalVolume_STATUS_PENDING}}
s.ExternalVolumes = []*ateapipb.ActorVolumeStatus{{Name: "vol-a", VolumeType: "substrate.io/mock", Status: ateapipb.ActorVolumeStatus_STATUS_PENDING}}
})),
validOutput(withStatus(func(s *ateapipb.ActorStatus) {
s.ExternalVolumes = []*ateapipb.ExternalVolume{{
s.ExternalVolumes = []*ateapipb.ActorVolumeStatus{{
Name: "vol-a",
VolumeType: "substrate.io/mock",
StorageVolumeId: "csi-426d29b7",
Status: ateapipb.ExternalVolume_STATUS_CREATED,
Status: ateapipb.ActorVolumeStatus_STATUS_CREATED,
VolumeContext: map[string]string{"attachment": "iqn.2026-08.io.ate:vol-a"},
}}
})),
Expand Down
96 changes: 49 additions & 47 deletions cmd/ateapi/internal/apivalidation/validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -911,12 +911,12 @@ func TestValidateTrustBundleDataSource(t *testing.T) {
}
}

func validExternalVolume(mutate ...func(*ateapipb.ExternalVolume)) *ateapipb.ExternalVolume {
v := &ateapipb.ExternalVolume{
func validExternalVolume(mutate ...func(*ateapipb.ActorVolumeStatus)) *ateapipb.ActorVolumeStatus {
v := &ateapipb.ActorVolumeStatus{
Name: "my-vol",
StorageVolumeId: "valid-storage-id",
VolumeType: "mock",
Status: ateapipb.ExternalVolume_STATUS_CREATED,
Status: ateapipb.ActorVolumeStatus_STATUS_CREATED,
}
for _, m := range mutate {
m(v)
Expand All @@ -929,96 +929,96 @@ func TestValidateExternalVolume(t *testing.T) {

tests := []struct {
name string
obj *ateapipb.ExternalVolume
obj *ateapipb.ActorVolumeStatus
want field.ErrorList
}{{
name: "valid external volume",
obj: valid(),
}, {
name: "missing volume name",
obj: valid(func(v *ateapipb.ExternalVolume) { v.Name = "" }),
obj: valid(func(v *ateapipb.ActorVolumeStatus) { v.Name = "" }),
want: field.ErrorList{field.Required(field.NewPath("name"), "")},
}, {
name: "invalid volume name",
obj: valid(func(v *ateapipb.ExternalVolume) { v.Name = "NOT A VOLUME" }),
obj: valid(func(v *ateapipb.ActorVolumeStatus) { v.Name = "NOT A VOLUME" }),
want: field.ErrorList{field.Invalid(field.NewPath("name"), nil, "").WithOrigin("format=k8s-short-name")},
}, {
name: "valid external volume with empty storage volume id",
obj: valid(func(v *ateapipb.ExternalVolume) { v.StorageVolumeId = "" }),
obj: valid(func(v *ateapipb.ActorVolumeStatus) { v.StorageVolumeId = "" }),
}, {
name: "invalid storage volume id with null U+0000",
obj: valid(func(v *ateapipb.ExternalVolume) { v.StorageVolumeId = "vol\x00id" }),
obj: valid(func(v *ateapipb.ActorVolumeStatus) { v.StorageVolumeId = "vol\x00id" }),
want: field.ErrorList{field.Invalid(field.NewPath("storage_volume_id"), nil, "")},
}, {
name: "invalid storage volume id with unit separator U+001F",
obj: valid(func(v *ateapipb.ExternalVolume) { v.StorageVolumeId = "vol\x1fid" }),
obj: valid(func(v *ateapipb.ActorVolumeStatus) { v.StorageVolumeId = "vol\x1fid" }),
want: field.ErrorList{field.Invalid(field.NewPath("storage_volume_id"), nil, "")},
}, {
name: "invalid storage volume id with DEL U+007F",
obj: valid(func(v *ateapipb.ExternalVolume) { v.StorageVolumeId = "vol\x7fid" }),
obj: valid(func(v *ateapipb.ActorVolumeStatus) { v.StorageVolumeId = "vol\x7fid" }),
want: field.ErrorList{field.Invalid(field.NewPath("storage_volume_id"), nil, "")},
}, {
name: "invalid storage volume id with C1 control U+0080",
obj: valid(func(v *ateapipb.ExternalVolume) { v.StorageVolumeId = "vol\u0080id" }),
obj: valid(func(v *ateapipb.ActorVolumeStatus) { v.StorageVolumeId = "vol\u0080id" }),
want: field.ErrorList{field.Invalid(field.NewPath("storage_volume_id"), nil, "")},
}, {
name: "invalid storage volume id with C1 control U+009F",
obj: valid(func(v *ateapipb.ExternalVolume) { v.StorageVolumeId = "vol\u009fid" }),
obj: valid(func(v *ateapipb.ActorVolumeStatus) { v.StorageVolumeId = "vol\u009fid" }),
want: field.ErrorList{field.Invalid(field.NewPath("storage_volume_id"), nil, "")},
}, {
name: "storage volume id too long",
obj: valid(func(v *ateapipb.ExternalVolume) { v.StorageVolumeId = strings.Repeat("x", 257) }),
obj: valid(func(v *ateapipb.ActorVolumeStatus) { v.StorageVolumeId = strings.Repeat("x", 257) }),
want: field.ErrorList{field.TooLong(field.NewPath("storage_volume_id"), nil, 256).WithOrigin("maxLength")},
}, {
name: "valid csi volume type",
obj: valid(func(v *ateapipb.ExternalVolume) { v.VolumeType = "pd.csi.storage.gke.io" }),
obj: valid(func(v *ateapipb.ActorVolumeStatus) { v.VolumeType = "pd.csi.storage.gke.io" }),
}, {
name: "missing volume type",
obj: valid(func(v *ateapipb.ExternalVolume) { v.VolumeType = "" }),
obj: valid(func(v *ateapipb.ActorVolumeStatus) { v.VolumeType = "" }),
want: field.ErrorList{field.Required(field.NewPath("volume_type"), "")},
}, {
name: "invalid volume type with uppercase",
obj: valid(func(v *ateapipb.ExternalVolume) { v.VolumeType = "MockPlugin" }),
obj: valid(func(v *ateapipb.ActorVolumeStatus) { v.VolumeType = "MockPlugin" }),
want: field.ErrorList{field.Invalid(field.NewPath("volume_type"), nil, "")},
}, {
name: "valid volume type with 253 characters",
obj: valid(func(v *ateapipb.ExternalVolume) { v.VolumeType = strings.Repeat("a", 253) }),
obj: valid(func(v *ateapipb.ActorVolumeStatus) { v.VolumeType = strings.Repeat("a", 253) }),
}, {
name: "invalid volume type exceeding 253 characters",
obj: valid(func(v *ateapipb.ExternalVolume) { v.VolumeType = strings.Repeat("a", 254) }),
obj: valid(func(v *ateapipb.ActorVolumeStatus) { v.VolumeType = strings.Repeat("a", 254) }),
want: field.ErrorList{
field.Invalid(field.NewPath("volume_type"), nil, ""),
field.TooLong(field.NewPath("volume_type"), nil, 253).WithOrigin("maxLength"),
},
}, {
name: "valid volume with substrate.io prefixed volume type",
obj: valid(func(v *ateapipb.ExternalVolume) { v.VolumeType = "substrate.io/mock" }),
obj: valid(func(v *ateapipb.ActorVolumeStatus) { v.VolumeType = "substrate.io/mock" }),
}, {
name: "invalid volume type with empty plugin after substrate.io prefix",
obj: valid(func(v *ateapipb.ExternalVolume) { v.VolumeType = "substrate.io/" }),
obj: valid(func(v *ateapipb.ActorVolumeStatus) { v.VolumeType = "substrate.io/" }),
want: field.ErrorList{field.Invalid(field.NewPath("volume_type"), nil, "")},
}, {
name: "invalid volume type with invalid plugin name after substrate.io prefix",
obj: valid(func(v *ateapipb.ExternalVolume) { v.VolumeType = "substrate.io/Mock_Plugin" }),
obj: valid(func(v *ateapipb.ActorVolumeStatus) { v.VolumeType = "substrate.io/Mock_Plugin" }),
want: field.ErrorList{field.Invalid(field.NewPath("volume_type"), nil, "")},
}, {
name: "invalid volume type with non-substrate prefix",
obj: valid(func(v *ateapipb.ExternalVolume) { v.VolumeType = "other.io/mock" }),
obj: valid(func(v *ateapipb.ActorVolumeStatus) { v.VolumeType = "other.io/mock" }),
want: field.ErrorList{field.Invalid(field.NewPath("volume_type"), nil, "")},
}, {
name: "negative status",
obj: valid(func(v *ateapipb.ExternalVolume) { v.Status = ateapipb.ExternalVolume_Status(-1) }),
obj: valid(func(v *ateapipb.ActorVolumeStatus) { v.Status = ateapipb.ActorVolumeStatus_Status(-1) }),
want: field.ErrorList{field.Invalid(field.NewPath("status"), nil, "").WithOrigin("minimum")},
}, {
name: "status outside the enum",
obj: valid(func(v *ateapipb.ExternalVolume) { v.Status = ateapipb.ExternalVolume_Status(4) }),
obj: valid(func(v *ateapipb.ActorVolumeStatus) { v.Status = ateapipb.ActorVolumeStatus_Status(4) }),
want: field.ErrorList{field.Invalid(field.NewPath("status"), nil, "").WithOrigin("maximum")},
}, {
name: "storage volume id at the bound",
obj: valid(func(v *ateapipb.ExternalVolume) { v.StorageVolumeId = strings.Repeat("x", 256) }),
obj: valid(func(v *ateapipb.ActorVolumeStatus) { v.StorageVolumeId = strings.Repeat("x", 256) }),
}, {
name: "too many volume_context entries",
obj: valid(func(v *ateapipb.ExternalVolume) {
obj: valid(func(v *ateapipb.ActorVolumeStatus) {
ctxMap := make(map[string]string, 33)
for i := 0; i < 33; i++ {
ctxMap[fmt.Sprintf("key-%d", i)] = "v"
Expand All @@ -1028,74 +1028,76 @@ func TestValidateExternalVolume(t *testing.T) {
want: field.ErrorList{field.TooMany(field.NewPath("volume_context"), 33, 32).WithOrigin("maxProperties")},
}, {
name: "volume_context key too long",
obj: valid(func(v *ateapipb.ExternalVolume) { v.VolumeContext = map[string]string{strings.Repeat("k", 129): "v"} }),
obj: valid(func(v *ateapipb.ActorVolumeStatus) {
v.VolumeContext = map[string]string{strings.Repeat("k", 129): "v"}
}),
want: field.ErrorList{field.TooLong(field.NewPath("volume_context"), nil, 128).WithOrigin("maxLength")},
}, {
name: "volume_context value too long",
obj: valid(func(v *ateapipb.ExternalVolume) {
obj: valid(func(v *ateapipb.ActorVolumeStatus) {
v.VolumeContext = map[string]string{"attachment": strings.Repeat("v", 257)}
}),
want: field.ErrorList{field.TooLong(field.NewPath("volume_context").Key("attachment"), nil, 256).WithOrigin("maxLength")},
}}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
op := operation.Operation{Type: operation.Create}
assertValidateErr(t, Validate_ExternalVolume(context.Background(), op, nil, tt.obj, nil), tt.want)
assertValidateErr(t, Validate_ActorVolumeStatus(context.Background(), op, nil, tt.obj, nil), tt.want)
})
}
}

func TestValidateExternalVolume_Update(t *testing.T) {
func TestValidateActorVolumeStatus_Update(t *testing.T) {
valid := validExternalVolume

tests := []struct {
name string
oldObj *ateapipb.ExternalVolume
newObj *ateapipb.ExternalVolume
oldObj *ateapipb.ActorVolumeStatus
newObj *ateapipb.ActorVolumeStatus
want field.ErrorList
}{{
name: "unchanged volume is valid",
oldObj: valid(),
newObj: valid(),
}, {
name: "name changed is invalid",
oldObj: valid(func(v *ateapipb.ExternalVolume) { v.Name = "vol1" }),
newObj: valid(func(v *ateapipb.ExternalVolume) { v.Name = "vol2" }),
oldObj: valid(func(v *ateapipb.ActorVolumeStatus) { v.Name = "vol1" }),
newObj: valid(func(v *ateapipb.ActorVolumeStatus) { v.Name = "vol2" }),
want: field.ErrorList{field.Invalid(field.NewPath("name"), nil, "").WithOrigin("update")},
}, {
name: "storage_volume_id transition from empty to non-empty is valid",
oldObj: valid(func(v *ateapipb.ExternalVolume) { v.StorageVolumeId = "" }),
newObj: valid(func(v *ateapipb.ExternalVolume) { v.StorageVolumeId = "vol-id-1" }),
oldObj: valid(func(v *ateapipb.ActorVolumeStatus) { v.StorageVolumeId = "" }),
newObj: valid(func(v *ateapipb.ActorVolumeStatus) { v.StorageVolumeId = "vol-id-1" }),
}, {
name: "storage_volume_id changed once set is invalid",
oldObj: valid(func(v *ateapipb.ExternalVolume) { v.StorageVolumeId = "vol-id-1" }),
newObj: valid(func(v *ateapipb.ExternalVolume) { v.StorageVolumeId = "vol-id-2" }),
oldObj: valid(func(v *ateapipb.ActorVolumeStatus) { v.StorageVolumeId = "vol-id-1" }),
newObj: valid(func(v *ateapipb.ActorVolumeStatus) { v.StorageVolumeId = "vol-id-2" }),
want: field.ErrorList{field.Invalid(field.NewPath("storage_volume_id"), nil, "").WithOrigin("update")},
}, {
name: "storage_volume_id unset once set is invalid",
oldObj: valid(func(v *ateapipb.ExternalVolume) { v.StorageVolumeId = "vol-id-1" }),
newObj: valid(func(v *ateapipb.ExternalVolume) { v.StorageVolumeId = "" }),
oldObj: valid(func(v *ateapipb.ActorVolumeStatus) { v.StorageVolumeId = "vol-id-1" }),
newObj: valid(func(v *ateapipb.ActorVolumeStatus) { v.StorageVolumeId = "" }),
want: field.ErrorList{field.Invalid(field.NewPath("storage_volume_id"), nil, "").WithOrigin("update")},
}, {
name: "volume_type changed is invalid",
oldObj: valid(func(v *ateapipb.ExternalVolume) { v.VolumeType = "mock" }),
newObj: valid(func(v *ateapipb.ExternalVolume) { v.VolumeType = "pd.csi.storage.gke.io" }),
oldObj: valid(func(v *ateapipb.ActorVolumeStatus) { v.VolumeType = "mock" }),
newObj: valid(func(v *ateapipb.ActorVolumeStatus) { v.VolumeType = "pd.csi.storage.gke.io" }),
want: field.ErrorList{field.Invalid(field.NewPath("volume_type"), nil, "").WithOrigin("update")},
}, {
name: "status and volume_context changed is valid",
oldObj: valid(func(v *ateapipb.ExternalVolume) {
v.Status = ateapipb.ExternalVolume_STATUS_PENDING
oldObj: valid(func(v *ateapipb.ActorVolumeStatus) {
v.Status = ateapipb.ActorVolumeStatus_STATUS_PENDING
v.VolumeContext = nil
}),
newObj: valid(func(v *ateapipb.ExternalVolume) {
v.Status = ateapipb.ExternalVolume_STATUS_CREATED
newObj: valid(func(v *ateapipb.ActorVolumeStatus) {
v.Status = ateapipb.ActorVolumeStatus_STATUS_CREATED
v.VolumeContext = map[string]string{"foo": "bar"}
}),
}}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
op := operation.Operation{Type: operation.Update}
assertValidateErr(t, Validate_ExternalVolume(context.Background(), op, nil, tt.newObj, tt.oldObj), tt.want)
assertValidateErr(t, Validate_ActorVolumeStatus(context.Background(), op, nil, tt.newObj, tt.oldObj), tt.want)
})
}
}
Expand Down
Loading
Loading