feat: sanitizing store keys - store path v2

[cmwylie19]

Description

This PR Migrates the Store to a v2 data path, migrates keys from previous version before a read of the CR, patches them to v2, then starts a watch. It also adds fuzzing, property-based, more unit and journey tests.

Related Issue

Fixes #915

Relates to PR defenseunicorns/pepr-excellent-examples#46 updating the e2e test to expect the keys base64 encoded

Type of change

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Other (security config, docs update, etc)

Checklist before merging

• Test, docs, adr added or updated as needed
• Contributor Guide Steps followed

[mjnagel]

This definitely seems functional, I do have a few comments on things that might be painful from the core side with this path:

1. In the past I have relied on a k describe of the peprstore resource(s) to see what my currently store k/v pairs are. With the keys base64 encoded that's no longer the most transparent experience. I think that's okay since it's not dissimilar from the experience viewing something like a secret, but it may be nice to consider a way to view the store easily (whether that's just a documented kubectl command, k9s plugin, or a new npx pepr <store-viewer> type thing?
2. The migration script is definitely usable, and we could probably find a way to make the upgrade completely seamless (i.e. not require any end user to do more than just a zarf deploy). I don't know how much effort this would be on this side of things but it would definitely be a more pleasant experience if pepr completely handled the migration for us. When I was playing around with this issue early on I tried some basic migration tactics in our code like when calling getItem first check if the old format key existed (non-base64 in this case). If it did, perform a "migration" by deleting and setting the new key, before using the value. Unsure if something similar would fit well in pepr.
3. Related to ^ but not directly tied to these changes, it might be nice to explore adding a Store.migrate(oldKey, newKey) type function for end users if they change their code in what keys they use. Might just be a lack of knowledge on my part but in exploring changing the store keys it didn't feel like the most seamless experience.

[cmwylie19]

This definitely seems functional, I do have a few comments on things that might be painful from the core side with this path:

1. In the past I have relied on a k describe of the peprstore resource(s) to see what my currently store k/v pairs are. With the keys base64 encoded that's no longer the most transparent experience. I think that's okay since it's not dissimilar from the experience viewing something like a secret, but it may be nice to consider a way to view the store easily (whether that's just a documented kubectl command, k9s plugin, or a new npx pepr <store-viewer> type thing?
2. The migration script is definitely usable, and we could probably find a way to make the upgrade completely seamless (i.e. not require any end user to do more than just a zarf deploy). I don't know how much effort this would be on this side of things but it would definitely be a more pleasant experience if pepr completely handled the migration for us. When I was playing around with this issue early on I tried some basic migration tactics in our code like when calling getItem first check if the old format key existed (non-base64 in this case). If it did, perform a "migration" by deleting and setting the new key, before using the value. Unsure if something similar would fit well in pepr.
3. Related to ^ but not directly tied to these changes, it might be nice to explore adding a Store.migrate(oldKey, newKey) type function for end users if they change their code in what keys they use. Might just be a lack of knowledge on my part but in exploring changing the store keys it didn't feel like the most seamless experience.

1. For sure, could not agree more. Maybe we can even do both.

2 & 3 are good ideas too. I just have a lot of open questions around how to guarantee we will get the capability name right without user intervention in the code as it is today because the capability name is tightly coupled to the module. However, i definitely think we can come up with something together to see how we can make this upgrade a seamless experience without the user even knowing it happened.

Lets chat when you are freed up on current work and we will come up with a game plan. For now I am going to put this PR on old and do more investigation and research.

[cmwylie19]

This definitely seems functional, I do have a few comments on things that might be painful from the core side with this path:

1. In the past I have relied on a k describe of the peprstore resource(s) to see what my currently store k/v pairs are. With the keys base64 encoded that's no longer the most transparent experience. I think that's okay since it's not dissimilar from the experience viewing something like a secret, but it may be nice to consider a way to view the store easily (whether that's just a documented kubectl command, k9s plugin, or a new npx pepr <store-viewer> type thing?
2. The migration script is definitely usable, and we could probably find a way to make the upgrade completely seamless (i.e. not require any end user to do more than just a zarf deploy). I don't know how much effort this would be on this side of things but it would definitely be a more pleasant experience if pepr completely handled the migration for us. When I was playing around with this issue early on I tried some basic migration tactics in our code like when calling getItem first check if the old format key existed (non-base64 in this case). If it did, perform a "migration" by deleting and setting the new key, before using the value. Unsure if something similar would fit well in pepr.
3. Related to ^ but not directly tied to these changes, it might be nice to explore adding a Store.migrate(oldKey, newKey) type function for end users if they change their code in what keys they use. Might just be a lack of knowledge on my part but in exploring changing the store keys it didn't feel like the most seamless experience.

1. For sure, could not agree more. Maybe we can even do both.

2 & 3 are good ideas too. I just have a lot of open questions around how to guarantee we will get the capability name right without user intervention in the code as it is today because the capability name is tightly coupled to the module. However, i definitely think we can come up with something together to see how we can make this upgrade a seamless experience without the user even knowing it happened.

Lets chat when you are freed up on current work and we will come up with a game plan. For now I am going to put this PR on old and do more investigation and research.

Now thinking back on this, what if we:

• used Store.onReady Callback Store.onReady(()=>{})
• added a Store.keyRaw like feature
• When we map through Store items, check if they are base64Encoded
• if not, migrate and delete the old key

What are you thoughts on that? We could also roll out the store view feature at the same time

[socket-security]

New dependencies detected. Learn more about Socket for GitHub ↗︎

Package	New capabilities	Transitives	Size	Publisher
npm/@types/json-pointer@1.0.34	None	0	7.81 kB	types
npm/json-pointer@0.6.2	None	+1	109 kB	manuelstofer

View full report↗︎

[btlghrants]

Not much left to say after the in-depth, in-person walk-through -- seems to do what it says on the tin. 👍

[cmwylie19]

this runner is bjorked

[cmwylie19]

this runner is bjorked

[cmwylie19]

The GitHub Runners are failing for node/npm

Run npm ci
  npm ci
  shell: /usr/bin/bash -e {0}
npm error Exit handler never called!
npm error This is an error with npm itself. Please report this error at:
npm error   <https://github.com/npm/cli/issues>
npm error A complete log of this run can be found in: /home/runner/.npm/_logs/[2](https://github.com/defenseunicorns/pepr/actions/runs/9980001407/job/27580881374#step:4:2)024-07-17T19_05_0[3](https://github.com/defenseunicorns/pepr/actions/runs/9980001407/job/27580881374#step:4:3)_312Z-debug-0.log

Relevant Issues:

• Node 22.5.0 started to crash and hangs on different cases nodejs/node#53902

Relevant news:

• https://github.com/nodejs/node/releases/tag/v22.5.0 Just released, thanks @mjnagel

We need to talk about changing the matrix test from latest if this does not get fixed ASAP.

[btlghrants]

Couple minor suggestions & one Q.