linode-blockstorage-csi-driver icon indicating copy to clipboard operation
linode-blockstorage-csi-driver copied to clipboard

Luks e2e

Open avestuk opened this issue 1 year ago • 3 comments

General:

  • [x] Have you removed all sensitive information, including but not limited to access keys and passwords?
  • [x] Have you checked to ensure there aren't other open or closed Pull Requests for the same bug/feature/question?

Pull Request Guidelines:

  1. [x] Does your submission pass tests?
  2. [x] Have you added tests?
  3. [x] Are you addressing a single feature in this PR?
  4. [x] Are your commits atomic, addressing one change per commit?
  5. [x] Are you following the conventions of the language?
  6. [x] Have you saved your large formatting changes for a different PR, so we can focus on your work?
  7. [x] Have you explained your rationale for why this feature is needed?
  8. [ ] Have you linked your PR to an open issue

Addresses the lack of tests in #159 - Creating loopback devices in docker was sadly not as straight forward as I had hoped and I wasn't able to get something working. Given the ease with which we can now write e2e tests thanks to #163 and @mhmxs an e2e test should afford us a better level of comfort.

Currently this PR is set as a draft to get some initial commentary back on the overall approach. Mainly I'm not enamoured with using static names for the LUKS storageClass and Secrets and the subsequent cleanup that has to happen. I don't think that there is an issue in modifying the "global" storageClass as ginkgo does parallelism using separate processes so mutation of storageClass in one process will not affect others.

Breaking this out into a separate Describe node would work but would require copying JustBeforeEach or moving it to a function and substantially altering the tests. I would like to add additional LUKS tests to show that LUKS volumes require a proper AES-XTS key in order to function, there may well be other tests we would like to add such as LUKS volume expansion etc.

avestuk avatar Apr 10 '24 13:04 avestuk

@nesv @eljohnson92 Hello from the other side 👋 Tagging the two of you as you've been here recently.

Do you want to take this PR over? It's just an E2E test for the LUKS encrypted volumes, at least one user was interested in LUKS encrypted volumes because they can manage their own keys and having an e2e test seems to make sense. The reason that I statically defined the LUKS key was that I never quite figured out how the keys should be generated. You'd think it'd just be 256/512 bytes of random stuff but it didn't quite seem to work for me.

Anyway if you want to throw this in the bin then by all means go ahead :)

avestuk avatar Jun 13 '24 07:06 avestuk

Hey @avestuk thanks for the context here! Yes, we will take this effort over. We have a separate initiative to revamp the e2e tests. We'll likely roll this in with those changes

eljohnson92 avatar Jun 14 '24 03:06 eljohnson92

@eljohnson92 🫡

avestuk avatar Jun 14 '24 12:06 avestuk