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 摘要。