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
Allow encryption for all resources #115149
Conversation
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. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Drop the first commit since these do not appear to conflict.
staging/src/k8s.io/apiserver/pkg/apis/config/validation/validation.go
Outdated
Show resolved
Hide resolved
staging/src/k8s.io/apiserver/pkg/apis/config/validation/validation.go
Outdated
Show resolved
Hide resolved
test/integration/controlplane/transformation/kms_transformation_test.go
Outdated
Show resolved
Hide resolved
test/integration/controlplane/transformation/kms_transformation_test.go
Outdated
Show resolved
Hide resolved
test/integration/controlplane/transformation/kms_transformation_test.go
Outdated
Show resolved
Hide resolved
test/integration/controlplane/transformation/kms_transformation_test.go
Outdated
Show resolved
Hide resolved
test/integration/controlplane/transformation/kms_transformation_test.go
Outdated
Show resolved
Hide resolved
/kind api-change @liggitt this PR proposes adding encryption support for all resources without any schema changes by interpreting |
/remove-sig api-machinery |
If the format is For the "encrypt everything in API group foo except resource bar" use case, I think it's ok to require enumerating the resources they want to encrypt. I would suggest normalizing items that don't contain a Questions:
|
Both are acceptable and encryption works as expected.
Yes. We plan to add such additional support/validations down the line based on the feedback. |
/retest |
/lgtm |
LGTM label has been added. Git tree hash: bb3bb3ef6e23e4d66d5183733ca1ac392ea5c3e1
|
/retest |
Hi @nilekhc. I am from the Bug triage shadow team for K8s release 1.27. I am checking in to make sure this PR is on track for 1.27 release. Please note that Code Freeze begins on March 15, post this date, only critical and release-blocker issues/PRs will be accepted. |
/release-note-edit
|
/assign @liggitt |
return identity.NewEncryptCheckTransformer() | ||
} | ||
|
||
func grYAMLString(gr schema.GroupResource) string { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
producing "yaml" strings is weird... is this only used for error messages? if so, can we skip the single-quote wrapping and just quote the values in the messages using %q?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's primarily for handling *.
case. String method of schema.GroupResource parses it to *
instead of *.
if gr.Group == "" && gr.Resource == "*" {
return "'*.'"
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, a tweak to emit *.
is fine, but drop the single-quote wrapping
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
API bits lgtm, one comment on the yaml stringer, then lgtm |
Signed-off-by: Nilekh Chaudhari <1626598+nilekhc@users.noreply.github.com>
Thanks for the review. I have updated it. PTAL when you get a chance. cc @enj |
/lgtm |
LGTM label has been added. Git tree hash: 09348e636a5c2b2002d09553baae28fcd8af82c0
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: enj, liggitt, nilekhc The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/retest |
What type of PR is this?
/kind feature
What this PR does / why we need it:
Which issue(s) this PR fixes:
Fixes # #111977
Special notes for your reviewer:
Does this PR introduce a user-facing change?
Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.: