-
Notifications
You must be signed in to change notification settings - Fork 41.5k
support optional listMapKeys in server-side apply #133020
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
support optional listMapKeys in server-side apply #133020
Conversation
test/integration/apiserver/apply covers the behavior of server-side-apply (SSA) for official APIs. But there seem to be no integration tests which cover the semantic of SSA like adding/removing/updating entries in a list map. This adds such a test. It needs an API which is under control of the test and uses k8s.io/apimachinery/pkg/apis/testapigroup for that purpose, with some issues fixed (OpenAPI code generation complained) and a new list map added. Registering that API group in the apiserver needs a REST storage and strategy. The API group only gets added in the test. However, the production code has to know about it. In particular, pkg/generated/openapi/zz_generated.openapi.go has to describe it.
As of structured-merge-diff v6.3.0, list map keys may be optional, as long as at least one key is provided.
This issue is currently awaiting triage. If a SIG or subproject determines this is a relevant issue, they will accept it by applying the The Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
This PR may require API review. If so, when the changes are ready, complete the pre-review checklist and request an API review. Status of requested reviews is tracked in the API Review project. |
/lgtm |
LGTM label has been added. Git tree hash: 0a7b0eaf56c33f5bfd88b904fc0fbde6b0fa1ff9
|
What type of PR is this?
/kind feature
What this PR does / why we need it:
When an API uses
+listType=map
with a certain set of+listMapKeys
, it should be possible to extend the API such that new, optional fields can be used as keys. This used to fail withassociative list with keys has an element that omits key field "shareUID" (and doesn''t have default value)
for a new keyshareUID
. As of structured-merged-diff v6.3.0 it's supported, see kubernetes-sigs/structured-merge-diff#296 (merged via kubernetes-sigs/structured-merge-diff#298).Also includes an integration test for SSA with such an optional key.
Which issue(s) this PR is related to:
Fixes #132524
Special notes for your reviewer:
Based on #132998, which in turn was based on my branch.
Diff against #132998 is just some test and commit message cleanup:
Does this PR introduce a user-facing change?
/assign @jpbetz @liggitt