agent-substrate / agent-substrate/substrate

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

未關閉
#1,359 0 則留言 0 個 reaction 已指派 0 人 在 GitHub 檢視
area/api area/api-machinery kind/bug
主要語言
Go
星號
1.8k
分支
316
平均合併
2 天 43 分鐘
30 天內合併 PR
287

描述

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

貢獻指南

開啟貢獻指南

評估

這個 Issue 還沒有評估資料。

把新 issue 寄到你的電子郵件信箱

精選適合新手參與的 GitHub issue 摘要。