Add CMEK E2E Test #218

davidz627 · 2019-01-10T01:38:39Z

A CMEK E2E test that provisions with CMEK, tests lifecycle, revokes key, makes sure disk usage fails, activates key, makes sure lifecycle succeeds again.

msau42 · 2019-01-10T18:50:39Z

test/e2e/tests/single_zone_e2e_test.go

+			},
+		}
+		key, err := kmsClient.CreateCryptoKey(ctx, keyReq)
+		if !gce.IsGCEError(err, "alreadyExists") {


should it already exist if we're generating uuid every time?

no. I guess it should be a hard failure. this code exists from before I was creating UUID's or cleaning up crypto keys

msau42 · 2019-01-10T18:53:20Z

test/e2e/tests/single_zone_e2e_test.go

+
+		// The resource name of the key rings.
+		parentName := fmt.Sprintf("projects/%s/locations/%s", p, locationID)
+		keyRingId := "gce-pd-csi-test-ring"


Is there any possible issue of multiple tests sharing the same key ring? Or leaving a key ring around at the end of the test?

key ring resource names are immutable.. all the tests will share the same key ring and the key ring will exist for the forseeable future. Talked to the CMEK TL and this is intended behavior

msau42 · 2019-01-10T18:54:06Z

test/e2e/tests/single_zone_e2e_test.go

+		defer func() {
+			// Delete Disk
+			controllerClient.DeleteVolume(volID)
+			Expect(err).To(BeNil(), "DeleteVolume failed")


Where is err coming from?

previous line but it got lost somehow

msau42 · 2019-01-10T18:55:58Z

test/e2e/utils/utils.go

+
+	// Set Cloud KMS permissions on compute service account
+	// TODO: Use the API to make this call instead of exec-ing gcloud command
+	computeSystemSA := fmt.Sprintf("service-%[email protected]", resp.ProjectNumber)


i'm not sure if this is the proper way we should be adding iam roles to project wide system accounts

agreed, looking into alternatives now

msau42 · 2019-01-10T18:56:50Z

test/e2e/tests/multi_zone_e2e_test.go

 		// Detach Disk
-		err = client.ControllerUnpublishVolume(volID, instance.GetNodeID())
-		Expect(err).To(BeNil(), "ControllerUnpublishVolume failed with error")
+		_ = client.ControllerUnpublishVolume(volID, instance.GetNodeID())


why do we not check for err?

k8s-ci-robot · 2019-01-17T01:20:54Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: davidz627

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:

~~OWNERS~~ [davidz627]

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

davidz627 · 2019-02-23T00:26:26Z

/retest

davidz627 · 2019-02-23T01:12:26Z

/retest

davidz627 · 2019-02-23T01:12:51Z

/test pull-gcp-compute-persistent-disk-csi-driver-e2e

davidz627 · 2019-02-23T01:34:20Z

/test pull-gcp-compute-persistent-disk-csi-driver-e2e

davidz627 · 2019-02-28T01:11:21Z

/test pull-gcp-compute-persistent-disk-csi-driver-e2e

davidz627 · 2019-02-28T20:52:05Z

/test pull-gcp-compute-persistent-disk-csi-driver-e2e

msau42 · 2019-02-28T21:53:47Z

test/e2e/tests/single_zone_e2e_test.go

+
+		// Defer deletion of all key versions
+		// https://cloud.google.com/kms/docs/destroy-restore
+		// Temporarily disable revokation of CMEK key because of test infra CI permissions


this is still not resolved? will we be leaking resources?

I'll add the permission for this. we are already leaking keyrings by design anyway (keyrings don't support deletion).

msau42 · 2019-02-28T21:54:59Z

test/run-e2e-local.sh

is this necessary to keep?

accidental git add

davidz627 · 2019-03-01T01:33:59Z

/test pull-gcp-compute-persistent-disk-csi-driver-e2e

msau42 · 2019-03-01T19:40:09Z

/lgtm

984feece Merge pull request kubernetes-sigs#234 from siddhikhapare/csi-tools 1f7e6059 fixed broken links of testgrid dashboard de2fba88 Merge pull request kubernetes-sigs#233 from andyzhangx/andyzhangx-patch-1 cee895e1 remove windows 20H2 build since it's EOL long time ago 670bb0ef Merge pull request kubernetes-sigs#229 from marosset/fix-codespell-errors 35d5e783 Merge pull request kubernetes-sigs#219 from yashsingh74/update-registry 63473cc9 Merge pull request kubernetes-sigs#231 from coulof/bump-go-version-1.20.5 29a5c76c Merge pull request kubernetes-sigs#228 from mowangdk/chore/adopt_kubernetes_recommand_labels 8dd28211 Update cloudbuild image with go 1.20.5 1df23dba Merge pull request kubernetes-sigs#230 from msau42/prow 1f92b7e7 Add ginkgo timeout to e2e tests to help catch any stuck tests 2b8b80ea fixing some codespell errors c10b6780 Merge pull request kubernetes-sigs#227 from coulof/check-sidecar-supported-versions 72984ec0 chore: adopt kubernetes recommand label b0555351 Header bd0a10b6 typo c39d73c3 Add comments f6491af0 Script to verify EOL sidecar version 4133d1df Merge pull request kubernetes-sigs#226 from msau42/cloudbuild 8d519d23 Pin buildkit to v0.10.6 to workaround v0.11 bug with docker manifest 6e04a030 Merge pull request kubernetes-sigs#224 from msau42/cloudbuild 26fdfffd Update cloudbuild image 6613c398 Merge pull request kubernetes-sigs#223 from sunnylovestiramisu/update 0e7ae993 Update k8s image repo url 77e47cce Merge pull request kubernetes-sigs#222 from xinydev/fix-dep-version 155854b0 Fix dep version mismatch 8f839056 Merge pull request kubernetes-sigs#221 from sunnylovestiramisu/go-update 1d3f94dd Update go version to 1.20 to match k/k v1.27 e322ce5e Merge pull request kubernetes-sigs#220 from andyzhangx/fix-golint-error b74a5120 test: fix golint error 901bcb5a Update registry k8s.gcr.io -> registry.k8s.io aa61bfd0 Merge pull request kubernetes-sigs#218 from xing-yang/update_csi_driver 7563d196 Update CSI_PROW_DRIVER_VERSION to v1.11.0 a2171bef Merge pull request kubernetes-sigs#216 from msau42/process cb987826 Merge pull request kubernetes-sigs#217 from msau42/owners a11216e4 add new reviewers and remove inactive reviewers dd986754 Add step for checking builds b66c0824 Merge pull request kubernetes-sigs#214 from pohly/junit-fixes b9b6763b filter-junit.go: fix loss of testcases when parsing Ginkgo v2 JUnit d4277839 filter-junit.go: preserve system error log 38e11468 prow.sh: publish individual JUnit files as separate artifacts git-subtree-dir: release-tools git-subtree-split: 984feece4bafac3aad74deeed76a500a0c485fb1

k8s-ci-robot assigned msau42 and saad-ali Jan 10, 2019

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 10, 2019

k8s-ci-robot requested review from jingxu97 and verult January 10, 2019 01:38

k8s-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Jan 10, 2019

msau42 reviewed Jan 10, 2019

View reviewed changes

davidz627 force-pushed the feature/CMEKTest branch from 48245d2 to 7721771 Compare January 17, 2019 01:20

davidz627 force-pushed the feature/CMEKTest branch from 7721771 to 5ffca02 Compare January 17, 2019 01:24

davidz627 force-pushed the feature/CMEKTest branch 2 times, most recently from d247734 to 5b8b082 Compare February 28, 2019 20:40

msau42 reviewed Feb 28, 2019

View reviewed changes

davidz627 force-pushed the feature/CMEKTest branch from 5b8b082 to d042fcf Compare February 28, 2019 22:05

davidz627 added 2 commits February 28, 2019 14:15

Add vendor dependencies for cloud kms apis

f3894db

Add Cloud KMS encrypted disk lifecycle test

3b8add3

davidz627 force-pushed the feature/CMEKTest branch from d042fcf to 3b8add3 Compare February 28, 2019 22:16

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 1, 2019

k8s-ci-robot merged commit 88e4af6 into kubernetes-sigs:master Mar 1, 2019

davidz627 deleted the feature/CMEKTest branch March 1, 2019 19:56

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add CMEK E2E Test #218

Add CMEK E2E Test #218

davidz627 commented Jan 10, 2019

msau42 Jan 10, 2019

davidz627 Jan 11, 2019

msau42 Jan 10, 2019

davidz627 Jan 11, 2019

msau42 Jan 10, 2019

davidz627 Jan 17, 2019

msau42 Jan 10, 2019

davidz627 Jan 17, 2019

msau42 Jan 10, 2019

davidz627 Jan 17, 2019

k8s-ci-robot commented Jan 17, 2019

davidz627 commented Feb 23, 2019

davidz627 commented Feb 23, 2019

davidz627 commented Feb 23, 2019

davidz627 commented Feb 23, 2019

davidz627 commented Feb 28, 2019

davidz627 commented Feb 28, 2019

msau42 Feb 28, 2019

davidz627 Feb 28, 2019

msau42 Feb 28, 2019

davidz627 Feb 28, 2019

davidz627 commented Mar 1, 2019

msau42 commented Mar 1, 2019

Add CMEK E2E Test #218

Add CMEK E2E Test #218

Conversation

davidz627 commented Jan 10, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

k8s-ci-robot commented Jan 17, 2019

davidz627 commented Feb 23, 2019

davidz627 commented Feb 23, 2019

davidz627 commented Feb 23, 2019

davidz627 commented Feb 23, 2019

davidz627 commented Feb 28, 2019

davidz627 commented Feb 28, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

davidz627 commented Mar 1, 2019

msau42 commented Mar 1, 2019