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
7 changes: 7 additions & 0 deletions contributors/devel/sig-architecture/api-conventions.md
Original file line number Diff line number Diff line change
Expand Up @@ -1982,6 +1982,13 @@ type definitions, or manually, by writing validation functions. For all new
APIs, declarative validation is the preferred approach for the validation rules
it supports. For more information see the
[declarative validation documentation](api_changes.md#declarative-validation).

Additionally, "Declarative Validation Native" (DV Native) is a mode that allows
net-new API fields and their validations to be defined exclusively using Go
comment tags, without requiring a parallel hand-written implementation in Go.
See the [DV Native documentation](api_changes.md#declarative-validation-native-dv-native)
for more details.

Validation errors are flagged and returned to the caller in a `Failure` status,
usually with `reason` set to `Invalid`.

Expand Down
39 changes: 39 additions & 0 deletions contributors/devel/sig-architecture/api_changes.md
Original file line number Diff line number Diff line change
Expand Up @@ -706,6 +706,45 @@ Users will need to write go unit tests similar to what is done for hand-written

While the goal is to express as much validation declaratively as possible, some complex or validation rules might still require manual implementation in `validation.go`.

#### Declarative Validation Native (DV Native)

"Declarative Validation Native" (DV Native) is a mode of operation for the declarative validation framework that allows net-new API fields and their validations to be defined exclusively using Go comment tags, without requiring a parallel hand-written implementation in Go.

This simplifies API development by making declarative tags the single source of truth for validation, reducing hand-written boilerplate code, and ensuring consistency between API definitions and enforcement logic.

To opt-in a field to DV Native mode, use the `+k8s:declarativeValidationNative` tag on the field in `types.go`. For example, if adding a net-new `Name` field and associated validation for an API, you would add the following in the associated `types.go` file. Note that this `+k8s:declarativeValidationNative` tag is added as well as the actual validation tags to be used (in this case `+k8s:required` and `+k8s:format=k8s-short-name`):

```go
// +k8s:required
// +k8s:format=k8s-short-name
// +k8s:declarativeValidationNative
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's a little unfortunate to have to add this to all DV-only fields for the future ... is there not a way to indicate this only in the strategy / tests and not need need to pollute the field docs with this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it is possible to indicate this in the strategy.go files. This will tie our hands, forcing us to use only stable validations tags for that kind. This means we will not be able to use alpha/beta tags mirrored with handwritten validation code in that kind. 

Secondarily, Eventually we don't want to restrict DV native for new API's. We want to also enable DV native validations for new fields added to existing kinds. This gives us flexibility to do that. 

We hear your feedback. If a Kind only has all stable validations. They we just need to put this new tag on one super field i.e., spec. This will automatically mark all validations of the spec field and its subfields as native.  This should not pollute the Kind that much.

type MyType {

// +k8s:declarativeNativeValidation
Spec  MySpec

Status MyStatus
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Does the DV test framework prevent adding a DV validation without a covering test? If not, that seems like a good gap to close.

  2. Are DV validation tests required to exercise in DV and non-DV mode OR explicitly indicate the validation it is testing is DV-native? If not, that seems like a good gap to close. If so, wouldn't we run DV-native tests with only stable DV validations enabled? That seems like it would cover what we need without requiring native-indication in the field docs.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does the DV test framework prevent adding a DV validation without a covering test? If not, that seems like a good gap to close.

There are no tests to test this. This responsibility is on the reviewers right now. There is an AI prompt to make sure all DV tags have tests. https://github.com/gke-labs/gemini-for-kubernetes-development/blob/main/commands/dv/review.toml

Are DV validation tests required to exercise in DV and non-DV mode OR explicitly indicate the validation it is testing is DV-native? If not, that seems like a good gap to close. If so, wouldn't we run DV-native tests with only stable DV validations enabled? That seems like it would cover what we need without requiring native-indication in the field doc

Yes, DV validation tests required to exercise in DV and non-DV mode. We segregate native validations errors from all dv errors, there is a bool present in the error struct to indicate whether it is from a native validation or not. Test cases need to explicitly mention which type of validations errors they are expecting for a given failure scenario. Mismatches in this will result an test failure and leaks will be caught.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are no tests to test this.

That might be a good improvement to add to the backlog. If adding a DV tag couldn't merge until a test that exercised it was added, that would be ideal.

If so, wouldn't we run DV-native tests with only stable DV validations enabled? That seems like it would cover what we need without requiring native-indication in the field doc

Yes, DV validation tests required to exercise in DV and non-DV mode.

So... can we scope the DV-native indicator to the test definition that is exercising the specific test? That would skip running the non-DV version, and would run the DV version with only stable tags enabled, and make sure the tested validation still works? That seems like it would give similar granularity without needing to pollute the type docs

Name string `json:"name" protobuf:"bytes,1,opt,name=name"`
```

By default, fields marked with `+k8s:declarativeValidationNative` are only allowed to use **stable** validation tags (e.g., `+k8s:required`, `+k8s:minimum`, `+k8s:maxLength`, `+k8s:format`, `+k8s:enum`, etc.). An up to date list of each tag and their associated Stability Level can be found at the kubernetes.io [Declarative Validation Tag Catalog](https://kubernetes.io/docs/reference/using-api/declarative-validation/#catalog).

To enable DV Native enforcement in your API strategy, you must pass the `rest.WithDeclarativeNative()` option to the declarative validation call in your `strategy.go`.

```go
func (myStrategy) Validate(ctx context.Context, obj runtime.Object) field.ErrorList {
myObj := obj.(*myapi.MyResource)
allErrs := validation.ValidateMyResource(myObj)
return rest.ValidateDeclarativelyWithMigrationChecks(ctx, legacyscheme.Scheme, obj, nil, allErrs, operation.Create, rest.WithDeclarativeNative())
}
```

When writing tests for DV Native fields, you must use `.MarkDeclarativeNative()` on the expected errors for fields that are DV Native.

```go
{
name: "missing native field",
obj: &MyResource{...},
expectedErrs: field.ErrorList{
field.Required(field.NewPath("spec", "myNativeField"), "").MarkDeclarativeNative(),
},
}
```

## Edit version conversions

At this point you have both the versioned API changes and the internal
Expand Down