agent-substrate / agent-substrate/substrate

The way we use DV makes it impossible to set immutable status fields

Đang mở
#1,359 0 bình luận 0 reaction 0 người được giao Xem trên GitHub
area/api area/api-machinery kind/bug
Ngôn ngữ chính
Go
Star
1.8k
Fork
316
Merge trung bình
2 ngày 43 phút
Pull request đã merge (30 ngày)
287

Mô tả

https://github.com/agent-substrate/substrate/pull/1244#discussion_r3890745561

The way we do it today is:
* Scrub status
* Validate user input
* Populate status
* Validate user input+status against the raw user input

The last step is the problem.

The struct in question is like:

```
type Obj struct {
// +optional
Status *Status
}

type Status struct {
// +k8s:immutable
Field int
}
```

In the old value, status is nil. In the new value it is not. DV "loses track" of that by the time it reaches `Field` and it passes a pointer to the newval and a nil pointer (oldval) to `validate.Immutable()` which, obviously, fails.

What we SHOULD do is somehow keep track of the fact that the status was nil and cut off any "oldval" checks.

Something like:

```diff
fn := func(
fldPath *field.Path,
obj, oldObj *ateapipb.ActorStatus,
oldValueCorrelated bool) (errs field.ErrorList) {
// don't revalidate unchanged data
if oldValueCorrelated && op.Type == operation.Update {
if ateDeepEqual(obj, oldObj) {
return nil
}
}
+ op := op
+ if oldObj == nil {
+ op.Type = operation.Create
+ }
// call field-attached validations
earlyReturn := false
if e := validate.OptionalPointer(ctx, op, fldPath, obj, oldObj).MarkShortCircuit(); len(e) != 0 {
earlyReturn = true
}
if earlyReturn {
return // do not proceed
}
// call the type's validation function
errs = append(errs, Validate_ActorStatus(ctx, op, fldPath, obj, oldObj)...)
```

That's not quite right but it is close.

@yongruilin @jpbetz

Hướng dẫn đóng góp

Mở hướng dẫn đóng góp

Đánh giá

Issue này chưa được đánh giá.

Nhận issue mới trong hộp thư của bạn

Bản tóm tắt ngắn những issue GitHub phù hợp với người mới.