agent-substrate / agent-substrate/substrate

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

Aberta
#1,359 0 comentários 0 reações 0 responsáveis Ver no GitHub
area/api area/api-machinery kind/bug
Linguagem predominante
Go
Estrelas
1.8k
Forks
316
Merge médio
2d 43min
PRs com merge (30d)
287

Descrição

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

Guia de contribuição

Abrir o guia de contribuição

Avaliação

Esta issue ainda não foi avaliada.

Receba novas issues na sua caixa de entrada

Um resumo curto de issues do GitHub para quem está começando.