Custom resource encryption #113015

ritazh · 2022-10-12T15:32:59Z

What type of PR is this?

/kind bug

What this PR does / why we need it:

Enable encryption of custom resources in ETCD

Which issue(s) this PR fixes:

Fixes #104165

Special notes for your reviewer:

Does this PR introduce a user-facing change?

kube-apiserver: APIs backed by CustomResourceDefinitions can now be specified in the `--encryption-provider-config` file; when specified, the data for those custom resources can now be encrypted in etcd.

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

Custom resources can now be encrypted in ETCD

cmd/kube-apiserver/app/apiextensions.go

test/integration/controlplane/transformation/all_transformation_test.go

test/integration/controlplane/transformation/transformation_testcase.go

test/integration/controlplane/transformation/all_transformation_test.go

leilajal · 2022-10-13T16:32:33Z

/remove-sig api-machinery

aramase · 2022-10-13T18:59:10Z

/cc

enj · 2022-10-18T12:44:15Z

New integration test from 0c2357af6aa40ec0f6b5cae1fc60c748eec997a1 applied to master fails as expected, with proper subtest breakdown:

=== RUN   TestEncryptSupportedForAllResourceTypes
    testserver.go:244: runtime-config=map[api/all:true]
    testserver.go:245: Starting kube-apiserver on port 36789...
    testserver.go:265: Waiting for /healthz to be ok...
=== RUN   TestEncryptSupportedForAllResourceTypes/configmaps
=== PAUSE TestEncryptSupportedForAllResourceTypes/configmaps
=== RUN   TestEncryptSupportedForAllResourceTypes/customresourcedefinitions
=== PAUSE TestEncryptSupportedForAllResourceTypes/customresourcedefinitions
=== RUN   TestEncryptSupportedForAllResourceTypes/pandas
=== PAUSE TestEncryptSupportedForAllResourceTypes/pandas
=== RUN   TestEncryptSupportedForAllResourceTypes/apiservices
=== PAUSE TestEncryptSupportedForAllResourceTypes/apiservices
=== RUN   TestEncryptSupportedForAllResourceTypes/pods
=== PAUSE TestEncryptSupportedForAllResourceTypes/pods
=== CONT  TestEncryptSupportedForAllResourceTypes/configmaps
=== CONT  TestEncryptSupportedForAllResourceTypes/apiservices
=== CONT  TestEncryptSupportedForAllResourceTypes/pandas
=== CONT  TestEncryptSupportedForAllResourceTypes/customresourcedefinitions
=== CONT  TestEncryptSupportedForAllResourceTypes/pods
=== CONT  TestEncryptSupportedForAllResourceTypes/customresourcedefinitions
    transformation_testcase.go:194: Get object with dynamic client
=== CONT  TestEncryptSupportedForAllResourceTypes/apiservices
    transformation_testcase.go:194: Get object with dynamic client
=== CONT  TestEncryptSupportedForAllResourceTypes/pandas
    transformation_testcase.go:135: expected data to be prefixed with k8s:enc:aescbc:v1:key1:, but got {"apiVersion":"awesome.bears.com/v1","kind":"Panda","metadata":{"creationTimestamp":"2022-10-18T12:39:44Z","generation":1,"managedFields":[{"apiVersion":"awesome.bears.com/v1","fieldsType":"FieldsV1","fieldsV1":{"f:spec":{".":{},"f:replicas":{}}},"manager":"transformation.test","operation":"Update","time":"2022-10-18T12:39:44Z"}],"name":"cr3panda","uid":"9e9d63b3-d206-4905-b539-cda35cc44f5b"},"spec":{"replicas":100}}
--- FAIL: TestEncryptSupportedForAllResourceTypes (6.39s)
    --- PASS: TestEncryptSupportedForAllResourceTypes/configmaps (0.03s)
    --- PASS: TestEncryptSupportedForAllResourceTypes/customresourcedefinitions (0.03s)
    --- PASS: TestEncryptSupportedForAllResourceTypes/pods (0.03s)
    --- PASS: TestEncryptSupportedForAllResourceTypes/apiservices (0.03s)
    --- FAIL: TestEncryptSupportedForAllResourceTypes/pandas (2.06s)
FAIL
FAIL	k8s.io/kubernetes/test/integration/controlplane/transformation	8.627s
FAIL

enj · 2022-10-18T12:50:18Z

/milestone v1.26
/triage accepted
/priority important-soon
/sig auth

test/integration/controlplane/transformation/all_transformation_test.go

staging/src/k8s.io/apiextensions-apiserver/pkg/cmd/server/options/options.go

test/integration/controlplane/transformation/transformation_testcase.go

liggitt · 2022-10-21T17:24:34Z

test/integration/controlplane/transformation/transformation_testcase.go

@@ -161,6 +216,23 @@ func (e *transformTest) getETCDPath() string {
 	return fmt.Sprintf("/%s/secrets/%s/%s", e.storageConfig.Prefix, e.ns.Name, e.secret.Name)
 }

+func (e *transformTest) getETCDPathForResource(group, resource, name, namespaceName string) string {
+	if e.secret != nil {


it's pretty hard to follow for state inside transformTest to override params... can we make this a pure function that takes storagePrefix, group, resource, name, namespace params?

also, what's the difference between getETCDPathForResource and getETCDPath? can we collapse to a single etcd path computation function?

also, what's the difference between getETCDPathForResource and getETCDPath? can we collapse to a single etcd path computation function?

I didn't want to touch getETCDPath to minimize changes on existing test files. If you prefer to combine them, then I will go ahead and update kms_transformation_test.go and kmsv2_transformation_test.go. LMK!

yeah, go ahead and clean up run and getETCDPath... the ripples don't look too bad and I think leave the test package in a less confusing state (mostly just makes the paths that assume they are dealing with secrets explicitly say so)

I got a clean run with this diff:

diff --git a/test/integration/controlplane/transformation/kms_transformation_test.go b/test/integration/controlplane/transformation/kms_transformation_test.go index 9773296ee29..3db46c22c3b 100644 --- a/test/integration/controlplane/transformation/kms_transformation_test.go +++ b/test/integration/controlplane/transformation/kms_transformation_test.go @@ -145,7 +145,7 @@ resources: // Since Data Encryption Key (DEK) is randomly generated (per encryption operation), we need to ask KMS Mock for it. plainTextDEK := pluginMock.LastEncryptRequest() - secretETCDPath := test.getETCDPath() + secretETCDPath := test.getETCDPathForResource("", "secrets", test.secret.Name, test.secret.Namespace) rawEnvelope, err := test.getRawSecretFromETCD() if err != nil { t.Fatalf("failed to read %s from etcd: %v", secretETCDPath, err) diff --git a/test/integration/controlplane/transformation/kmsv2_transformation_test.go b/test/integration/controlplane/transformation/kmsv2_transformation_test.go index dfb250a938f..f8b385adb24 100644 --- a/test/integration/controlplane/transformation/kmsv2_transformation_test.go +++ b/test/integration/controlplane/transformation/kmsv2_transformation_test.go @@ -154,7 +154,7 @@ resources: // Since Data Encryption Key (DEK) is randomly generated (per encryption operation), we need to ask KMS Mock for it. plainTextDEK := pluginMock.LastEncryptRequest() - secretETCDPath := test.getETCDPath() + secretETCDPath := test.getETCDPathForResource("", "secrets", test.secret.Name, test.secret.Namespace) rawEnvelope, err := test.getRawSecretFromETCD() if err != nil { t.Fatalf("failed to read %s from etcd: %v", secretETCDPath, err) diff --git a/test/integration/controlplane/transformation/secrets_transformation_test.go b/test/integration/controlplane/transformation/secrets_transformation_test.go index b899237a937..ccbd86ef0ff 100644 --- a/test/integration/controlplane/transformation/secrets_transformation_test.go +++ b/test/integration/controlplane/transformation/secrets_transformation_test.go @@ -95,7 +95,7 @@ func TestSecretsShouldBeTransformed(t *testing.T) { if err != nil { t.Fatalf("Failed to create test secret, error: %v", err) } - test.run(tt.unSealFunc, tt.transformerPrefix) + test.runResource(test.logger, tt.unSealFunc, tt.transformerPrefix, "", "v1", "secrets", test.secret.Name, test.secret.Namespace) test.cleanUp() } } diff --git a/test/integration/controlplane/transformation/transformation_testcase.go b/test/integration/controlplane/transformation/transformation_testcase.go index d5ca2b8fcc7..cea83135866 100644 --- a/test/integration/controlplane/transformation/transformation_testcase.go +++ b/test/integration/controlplane/transformation/transformation_testcase.go @@ -114,10 +114,6 @@ func (e *transformTest) cleanUp() { e.kubeAPIServer.TearDownFn() } -func (e *transformTest) run(unSealSecretFunc unSealSecret, expectedEnvelopePrefix string) { - e.runResource(e.logger, unSealSecretFunc, expectedEnvelopePrefix, "", "v1", "secrets", "", "") -} - func (e *transformTest) runResource(l kubeapiservertesting.Logger, unSealSecretFunc unSealSecret, expectedEnvelopePrefix, group, version, @@ -212,17 +208,7 @@ func (e *transformTest) benchmark(b *testing.B) { } } -func (e *transformTest) getETCDPath() string { - return fmt.Sprintf("/%s/secrets/%s/%s", e.storageConfig.Prefix, e.ns.Name, e.secret.Name) -} - func (e *transformTest) getETCDPathForResource(group, resource, name, namespaceName string) string { - if e.secret != nil { - group = "" - resource = "secrets" - name = testSecret - namespaceName = testNamespace - } groupResource := resource if group != "" { groupResource = fmt.Sprintf("%s/%s", group, resource) @@ -234,7 +220,7 @@ func (e *transformTest) getETCDPathForResource(group, resource, name, namespaceN } func (e *transformTest) getRawSecretFromETCD() ([]byte, error) { - secretETCDPath := e.getETCDPath() + secretETCDPath := e.getETCDPathForResource("", "secrets", e.secret.Name, e.ns.Name) etcdResponse, err := e.readRawRecordFromETCD(secretETCDPath) if err != nil { return nil, fmt.Errorf("failed to read %s from etcd: %v", secretETCDPath, err)

staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/customresource_handler.go

test/integration/controlplane/transformation/all_transformation_test.go

liggitt · 2022-10-26T18:38:10Z

/approve
squash, then lgtm

k8s-ci-robot · 2022-10-26T18:42:41Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: enj, liggitt, ritazh

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:

~~cmd/kube-apiserver/OWNERS~~ [liggitt]
~~staging/src/k8s.io/apiextensions-apiserver/OWNERS~~ [liggitt]
~~test/OWNERS~~ [enj,liggitt]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Signed-off-by: Rita Zhang <rita.z.zhang@gmail.com>

enj · 2022-10-26T20:40:51Z

/lgtm

sftim · 2022-11-16T04:51:14Z

My suggestion for the changelog text (which can be Markdown):

kube-apiserver: APIs backed by CustomResourceDefinitions can now be specified in the `--encryption-provider-config` file; when specified, the data for those custom resources can now be encrypted in etcd.

I like to put command line arguments inside backticks.

ritazh · 2022-11-16T06:42:24Z

Thanks for the suggestion @sftim! I've updated above.

ritazh changed the title ~~Crencryption~~ Customer resource encryption Oct 12, 2022

ritazh marked this pull request as draft October 12, 2022 15:33

k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 12, 2022

k8s-ci-robot requested review from caesarxuchao and jpbetz October 12, 2022 15:33

enj reviewed Oct 12, 2022

View reviewed changes

k8s-ci-robot removed the sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. label Oct 13, 2022

k8s-ci-robot requested a review from aramase October 13, 2022 18:59

k8s-ci-robot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. labels Oct 13, 2022

ritazh changed the title ~~Customer resource encryption~~ Custom resource encryption Oct 14, 2022

ritazh force-pushed the crencryption branch from c104c9d to 97df093 Compare October 14, 2022 22:05

k8s-ci-robot removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. do-not-merge/contains-merge-commits Indicates a PR which contains merge commits. labels Oct 14, 2022

ritazh force-pushed the crencryption branch from 97df093 to 905169f Compare October 14, 2022 22:09

k8s-ci-robot assigned enj Oct 18, 2022

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 18, 2022

k8s-ci-robot added the triage/accepted Indicates an issue or PR is ready to be actively worked on. label Oct 18, 2022

k8s-ci-robot added this to the v1.26 milestone Oct 18, 2022

enj mentioned this pull request Oct 18, 2022

Implements hot reload of the KMS EncryptionConfiguration #112050

Merged

aramase reviewed Oct 18, 2022

View reviewed changes

test/integration/controlplane/transformation/all_transformation_test.go Show resolved Hide resolved

test/integration/controlplane/transformation/all_transformation_test.go Show resolved Hide resolved

liggitt reviewed Oct 21, 2022

View reviewed changes

test/integration/controlplane/transformation/all_transformation_test.go Show resolved Hide resolved

ritazh mentioned this pull request Oct 21, 2022

Update godoc on how to specify group/resources to be encrypted #113272

Closed

k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 26, 2022

ritazh force-pushed the crencryption branch from 70fe0a2 to 6a8caa0 Compare October 26, 2022 05:28

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 26, 2022

Enable encryption for custom resources

c3df726

Signed-off-by: Rita Zhang <rita.z.zhang@gmail.com>

ritazh force-pushed the crencryption branch from 6a8caa0 to c3df726 Compare October 26, 2022 20:37

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 26, 2022

k8s-ci-robot merged commit 52f5816 into kubernetes:master Oct 26, 2022

ritazh deleted the crencryption branch October 27, 2022 13:19

reshnm mentioned this pull request Oct 28, 2022

Support End-User Configuration for set of encrypted resources gardener/gardener#4606

Closed

ritazh mentioned this pull request Nov 28, 2022

Document custom resource encryption support kubernetes/website#38109

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Custom resource encryption #113015

Custom resource encryption #113015

ritazh commented Oct 12, 2022 •

edited

leilajal commented Oct 13, 2022

aramase commented Oct 13, 2022

enj commented Oct 18, 2022

enj commented Oct 18, 2022

liggitt Oct 21, 2022

liggitt Oct 21, 2022

ritazh Oct 21, 2022

liggitt Oct 24, 2022 •

edited

liggitt commented Oct 26, 2022

k8s-ci-robot commented Oct 26, 2022

enj commented Oct 26, 2022

sftim commented Nov 16, 2022

ritazh commented Nov 16, 2022

Custom resource encryption #113015

Custom resource encryption #113015

Conversation

ritazh commented Oct 12, 2022 • edited

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR fixes:

Special notes for your reviewer:

Does this PR introduce a user-facing change?

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

leilajal commented Oct 13, 2022

aramase commented Oct 13, 2022

enj commented Oct 18, 2022

enj commented Oct 18, 2022

liggitt Oct 21, 2022

Choose a reason for hiding this comment

liggitt Oct 21, 2022

Choose a reason for hiding this comment

ritazh Oct 21, 2022

Choose a reason for hiding this comment

liggitt Oct 24, 2022 • edited

Choose a reason for hiding this comment

liggitt commented Oct 26, 2022

k8s-ci-robot commented Oct 26, 2022

enj commented Oct 26, 2022

sftim commented Nov 16, 2022

ritazh commented Nov 16, 2022

ritazh commented Oct 12, 2022 •

edited

liggitt Oct 24, 2022 •

edited