-
Notifications
You must be signed in to change notification settings - Fork 7
WIP: Finish "IP sloppy" removal #142
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
thockin
wants to merge
190
commits into
jpbetz:main
Choose a base branch
from
thockin:vg_finish_ip_sloppy_removal
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
1651b06 to
fea39ac
Compare
Collaborator
Author
|
Rebased |
0c6a924 to
1b17c5f
Compare
…gs and missing selector.go import
…d options test + generate validations
This reverts commit 178b781a6d6cc8a3324aeb9c9e177d86a62c9b65.
Also fix boilerplate errors
This crashes during codegen with a pretty useless panic(), which doesn't
make sense any more.
```
$ pushd; ./hack/update-codegen.sh valid; pushd
~/src/kubernetes ~/src/kubernetes/staging/src/k8s.io/code-generator/cmd/validation-gen
+++ [0418 23:56:03] validation: 69 targets
panic: alias type should already have been unwrapped
goroutine 1 [running]:
k8s.io/code-generator/cmd/validation-gen/validators.requirednessTagValidator.GetValidations({{0x86cb10?, 0xc0000c0b60?}}, {{0x86ca5a, 0xd}, 0xc01ff9efc0, 0xc01ff9f0a0, 0xc02a3f0cd0, 0xc02a4092c0}, {0x0, 0x0, ...}, ...)
/home/thockin/src/kubernetes/staging/src/k8s.io/code-generator/cmd/validation-gen/validators/required.go:69 +0x40f
k8s.io/code-generator/cmd/validation-gen/validators.(*registry).ExtractValidations(0xc0a4e0, {{0x86ca5a, 0xd}, 0xc01ff9efc0, 0xc01ff9f0a0, 0xc02a3f0cd0, 0xc02a4092c0}, {0xc0200640e0, 0x2, 0x2})
```
Subsequent commits will clean this area up more.
…nd tags/required tests
Before this, pointer fields would fail at codegen.
Prior to this it would look at the .Kind, see "Alias", and generate calls to (e.g.) `RequiredValue()`. Now calls `RequiredSlice()`.
…nd tags/required tests
This commit introduces a new format validator for Kubernetes label values and adds a comprehensive set of test cases.
feat(validation): Add k8s-label-value format validation
Adds comprehensive tests for the `+k8s:unique` validation tag, covering combinations with `+k8s:listType` and various edge cases.
…ation - Use single `semantic` field instead of multiple boolean fields - Make listType tags non-idempotent - Fix validation logic to not require all lists to have listType - Improve error messages and documentation
- Require listType=atomic when using unique=set or unique=map - Allow item tags to work with both listType=map and unique=map - Prevent invalid combinations like listType=map + unique=map - Improve validation error messages and comment consistency - Add test case for atomic + unique=map + item validation - Fix existing tests to use valid listType + unique combinations
This commit refactors the list validation logic for clarity and correctness. - The internal representation now distinguishes between `ownership` (atomic, shared) and `semantic` (set, map) for clearer intent. - Validation for `k8s:listType` and `k8s:unique` tags is now order-independent, correctly enforcing that `unique` can only be used with `listType=atomic`. - Uniqueness checks for `listType=map` are temporarily disabled unless explicitly requested with `unique=map` to support existing use cases.
rename and consolidate unique list fields in Struct and update validations
This reverts commit a44d3e3. For some reason it was using a private fork of gengo
This reverts commit 3de883c. This seems wrong
This reverts commit b7e6fa8. Not needed anymore, now that gengo calls 'gofmt -s'
* Also add origin for k8s:enum tag.
Declarative validation's ratcheting support (on for all fields) will already allow previously-valid-but-now-invalid IPs to remain in place, as long as they don't change. For "special" semantics like lists, we need to define the general ratcheting mechanism that solves it, rather than something special for IPs.
a8aba8a to
5a6906d
Compare
c4ad0d7 to
346c86a
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In a previous PR we renamed format "k8s-ip-sloppy" to "k8s-ip", but we did not fix the actual validation. This set of commits changes the validation to follow the strict rules in the handwritten validation. We can't convert it all at the same time because there is a gate, so this adds some comments.