Add encryption support #514

paravibe · 2023-06-22T11:47:21Z

This is refactored version of #337 based on the master.

I've added 2 more StorageClasses:
do-block-storage-luks use one key per volume
do-block-storage-luks-global use one key per cluster

Also resize of LUKS partition is supported now. Code was taken from cloudscale-ch/csi-cloudscale.

paravibe · 2023-06-22T19:24:58Z

@timoreimann I'd be very happy if you could review this PR

okamidash

This is a good PR.
Have you built and deployed these changes on a DOKS cluster to test their functionality?

okamidash · 2023-07-13T10:32:58Z

deploy/kubernetes/releases/csi-digitalocean-v4.7.0/crds.yaml

This, along with the other files within this directory aren't needed. We'll push a release when we're ready to do so, rather than as a result of this PR.

okamidash · 2023-07-13T10:43:37Z

driver/mounter.go

+		}
+		return nil
+	} else {
+		err := luksContext.validate()


Are you able to return a formatted error instead using fmt.Errorf, stating something like validation failed : %s?

sure, updated

okamidash · 2023-07-13T10:44:00Z

driver/mounter.go

-	return nil
+		err = luksFormat(source, mkfsCmd, mkfsArgs, luksContext, m.log)
+		if err != nil {
+			return err


Same here, stating instead that formatting failed?

luksFormat already return fmt.Errorf so I think this is OK

okamidash · 2023-07-13T10:46:33Z

deploy/kubernetes/releases/csi-digitalocean-dev/driver.yaml

@@ -38,6 +38,36 @@ deletionPolicy: Delete

 ---

+kind: StorageClass


Given that we're already defining quite a few storageclasses, I think it might be better to instead document this feature, rather than explicitly define it for all future clusters.

Ok, I've removed those LUKS storage classes and updated README.md a bit.

VERSION

paravibe · 2023-07-13T11:30:45Z

@okamidash yes, I'm using it currently on a DOKS with 144 volumes attached. No issues so far.

okamidash · 2023-07-17T16:55:15Z

Unit tests passed.

1.24

Ran 11 of 7233 Specs in 836.475 seconds
SUCCESS! -- 11 Passed | 0 Failed | 0 Pending | 7222 Skipped
PASS

Ginkgo ran 1 suite in 13m57.271159009s
Test Suite Passed
Cleaned up cluster 5792124c-e66a-4335-aa05-1bc39eff3470 (csi-e2e-1-24-test-htxxq)
--- PASS: TestE2E (1667.67s)
    --- PASS: TestE2E/1.24 (1667.67s)
PASS
ok  	github.com/digitalocean/csi-digitalocean/test/e2e	1667.679s

1.25

Ran 11 of 7333 Specs in 868.294 seconds
SUCCESS! -- 11 Passed | 0 Failed | 0 Pending | 7322 Skipped
PASS

Ginkgo ran 1 suite in 14m28.390392693s
Test Suite Passed
Cleaned up cluster 16548020-d7c2-490c-bd38-1acd6606f3e7 (csi-e2e-1-25-test-4rljg)
--- PASS: TestE2E (1679.32s)
    --- PASS: TestE2E/1.25 (1679.32s)
PASS
ok  	github.com/digitalocean/csi-digitalocean/test/e2e	1679.322s

1.26

Ran 11 of 7345 Specs in 879.964 seconds
SUCCESS! -- 11 Passed | 0 Failed | 0 Pending | 7334 Skipped
PASS

Ginkgo ran 1 suite in 14m40.079153607s
Test Suite Passed
Cleaned up cluster 17e83a25-559c-4689-bb88-89ae008c9265 (csi-e2e-1-26-test-wfxnp)
--- PASS: TestE2E (1719.72s)
    --- PASS: TestE2E/1.26 (1719.72s)
PASS
ok  	github.com/digitalocean/csi-digitalocean/test/e2e	1719.727s

1.27

Ran 12 of 7486 Specs in 767.612 seconds
SUCCESS! -- 12 Passed | 0 Failed | 0 Pending | 7474 Skipped
PASS

Ginkgo ran 1 suite in 12m47.757203948s
Test Suite Passed
Cleaned up cluster 0b4c838b-12eb-4482-8367-48daedf13c99 (csi-e2e-1-27-test-fq4rv)
--- PASS: TestE2E (1558.06s)
    --- PASS: TestE2E/1.27 (1558.06s)
PASS
ok  	github.com/digitalocean/csi-digitalocean/test/e2e	1558.064s

timoreimann

Thanks for the contribution @paravibe!

Before doing a review on specific code portions, I'd have two more general questions/requests:

DO volumes are already encrypted at block storage layer. I wanted to confirm that you are aware of this and that you see a need to have another encryption layer on top of it regardless. Appreciate if you could speak to what your use case for LUKS is in light of this context.
Would you be open to writing an end-to-end test? If we intend to support LUKS in the CSI driver, then I feel we need something beyond unit tests (e.g., to verify that the tooling installed in the container continues to be compatible and fit the needs of the driver). The good news is that we already have a pretty elaborate e2e test scaffolding in place; the bad news is that it is currently hard-wired to running the official Kubernetes e2e storage tests, so this would be the first time we would have an e2e test of our own. This is certainly possible but would require a bit of refactoring work (and, related, some effort to get familiar with the not super trivial test setup).

Thanks again.

paravibe · 2023-07-22T15:39:56Z

DO volumes are already encrypted at block storage layer. I wanted to confirm that you are aware of this and that you see a need to have another encryption layer on top of it regardless. Appreciate if you could speak to what your use case for LUKS is in light of this context.

Yes, I know it. This is requirement from our customers. The main reason they are asking about this is because they want to control encryption key.

Would you be open to writing an end-to-end test? If we intend to support LUKS in the CSI driver, then I feel we need something beyond unit tests (e.g., to verify that the tooling installed in the container continues to be compatible and fit the needs of the driver). The good news is that we already have a pretty elaborate e2e test scaffolding in place; the bad news is that it is currently hard-wired to running the official Kubernetes e2e storage tests, so this would be the first time we would have an e2e test of our own. This is certainly possible but would require a bit of refactoring work (and, related, some effort to get familiar with the not super trivial test setup).

I can't promise anything on this as this require some time (which I don't have enough right now) and skills (which I also don't have enough). But I'll take a look at it ASAP.

paravibe · 2023-08-04T16:20:16Z

@timoreimann
I've added a test for LUKS volume (pls check last commit). This is still WIP but I need your confirm that this the right direction.
So basically it will create a pod with PVC attached and check if it has a proper filesystem endpoint. If it's /dev/mapper/pvc-xxx then it's luks encrypted volume otherwise it will fail.

TODO:

Build a test with make command and copy it into docker image.
Add a command to execute it later with run-versioned-e2e-tests.sh

paravibe · 2023-08-07T17:21:28Z

@timoreimann @okamidash
I've finished with my todos so now luks test executed as well. Please take a look.

okamidash · 2023-09-05T09:48:11Z

1.25

Ran 11 of 7333 Specs in 833.723 seconds
SUCCESS! -- 11 Passed | 0 Failed | 0 Pending | 7322 Skipped
PASS

Ginkgo ran 1 suite in 13m53.838931618s
Test Suite Passed
Cleaned up cluster 71d2d002-257f-43c8-97ab-81f9964e1154 (csi-e2e-1-25-test-c8bd5)
--- PASS: TestE2E (1622.49s)
    --- PASS: TestE2E/1.25 (1622.49s)
PASS
ok  	github.com/digitalocean/csi-digitalocean/test/e2e	1622.505s

1.26

Ran 11 of 7345 Specs in 839.761 seconds
SUCCESS! -- 11 Passed | 0 Failed | 0 Pending | 7334 Skipped
PASS

Ginkgo ran 1 suite in 13m59.989531433s
Test Suite Passed
Cleaned up cluster 39fff0bc-89c5-44fe-bb1e-5fc1cac799e3 (csi-e2e-1-26-test-bm422)
--- PASS: TestE2E (1703.92s)
    --- PASS: TestE2E/1.26 (1703.92s)
PASS
ok  	github.com/digitalocean/csi-digitalocean/test/e2e	1703.926s

1.27


Ran 12 of 7486 Specs in 801.764 seconds
SUCCESS! -- 12 Passed | 0 Failed | 0 Pending | 7474 Skipped
PASS

Ginkgo ran 1 suite in 13m22.002781584s
Test Suite Passed
Cleaned up cluster f8a86460-ef4f-4b5c-b089-3ce3ac515424 (csi-e2e-1-27-test-dlgrm)
--- PASS: TestE2E (1732.95s)
    --- PASS: TestE2E/1.27 (1732.95s)
PASS
ok  	github.com/digitalocean/csi-digitalocean/test/e2e	1732.965s

paravibe · 2023-10-17T10:36:26Z

Any updates?

paravibe · 2023-10-27T13:33:57Z

As discussed, this won't be integrated into DOKS. Closing PR.

paravibe force-pushed the luks branch from af5cfa5 to 9088197 Compare June 23, 2023 10:24

Add LUKS encryption support.

916c067

paravibe force-pushed the luks branch from 9088197 to 916c067 Compare June 23, 2023 10:35

okamidash requested changes Jul 13, 2023

View reviewed changes

paravibe added 3 commits July 13, 2023 14:39

Revert bump-version

b121391

Remove LUKS storage class.

7dfa6c6

Return formatted error in case LUKS validation is failed.

ec5f145

timoreimann reviewed Jul 22, 2023

View reviewed changes

Add e2e test for LUKS volume.

3db00e4

paravibe force-pushed the luks branch from 3d65413 to 3db00e4 Compare August 7, 2023 17:17

paravibe closed this Oct 27, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add encryption support #514

Add encryption support #514

paravibe commented Jun 22, 2023 •

edited

Loading

paravibe commented Jun 22, 2023

okamidash left a comment

okamidash Jul 13, 2023 •

edited

Loading

paravibe Jul 13, 2023

okamidash Jul 13, 2023

paravibe Jul 13, 2023

okamidash Jul 13, 2023

paravibe Jul 13, 2023

okamidash Jul 13, 2023

paravibe Jul 13, 2023

paravibe commented Jul 13, 2023

okamidash commented Jul 17, 2023

timoreimann left a comment

paravibe commented Jul 22, 2023

paravibe commented Aug 4, 2023

paravibe commented Aug 7, 2023

okamidash commented Sep 5, 2023 •

edited

Loading

paravibe commented Oct 17, 2023

paravibe commented Oct 27, 2023

		@@ -38,6 +38,36 @@ deletionPolicy: Delete

		---

		kind: StorageClass

Add encryption support #514

Add encryption support #514

Conversation

paravibe commented Jun 22, 2023 • edited Loading

paravibe commented Jun 22, 2023

okamidash left a comment

Choose a reason for hiding this comment

okamidash Jul 13, 2023 • edited Loading

Choose a reason for hiding this comment

paravibe Jul 13, 2023

Choose a reason for hiding this comment

okamidash Jul 13, 2023

Choose a reason for hiding this comment

paravibe Jul 13, 2023

Choose a reason for hiding this comment

okamidash Jul 13, 2023

Choose a reason for hiding this comment

paravibe Jul 13, 2023

Choose a reason for hiding this comment

okamidash Jul 13, 2023

Choose a reason for hiding this comment

paravibe Jul 13, 2023

Choose a reason for hiding this comment

paravibe commented Jul 13, 2023

okamidash commented Jul 17, 2023

1.24

1.25

1.26

1.27

timoreimann left a comment

Choose a reason for hiding this comment

paravibe commented Jul 22, 2023

paravibe commented Aug 4, 2023

paravibe commented Aug 7, 2023

okamidash commented Sep 5, 2023 • edited Loading

paravibe commented Oct 17, 2023

paravibe commented Oct 27, 2023

paravibe commented Jun 22, 2023 •

edited

Loading

okamidash Jul 13, 2023 •

edited

Loading

okamidash commented Sep 5, 2023 •

edited

Loading