Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pepr Store Keys Need Sanitization #915

Closed
cmwylie19 opened this issue Jun 25, 2024 · 3 comments · Fixed by #918
Closed

Pepr Store Keys Need Sanitization #915

cmwylie19 opened this issue Jun 25, 2024 · 3 comments · Fixed by #918
Assignees

Comments

@cmwylie19
Copy link
Collaborator

cmwylie19 commented Jun 25, 2024

Describe what should be investigated or refactored

Certain keys are not making it into the store, the patch operation is returning 422.

image

We need a way to sanitize the key so that the patch operation succeeds and users of the store can get their data in a consistent manner.

We ran into an issue in a few spots where we accept arbitrary keycloak client ids in our Package CR and then use those in other resources without "sanitizing" them. One of those places was the pepr store - we were setting weird client IDs like https://google.com/ for testing and then our pepr store key had that in it and failed during set. Just want to make sure we know what is allowed for those key names.

Reproduce:

When(a.Pod)
  .IsCreatedOrUpdated()
  .Mutate(po => {
    Log.info("Pod created or updated", po);
    //Store.setItem(`${po.Raw.metadata!.name}`, "a");
    Store.setItem(`https://google.com-${po.Raw.metadata!.name}`, "bcd");
  });

Definition of done:

  • Folks can put in keys to the store like https://google.com-${po.Raw.metadata!.name}, the store sanitizes them before the pack, and they can get them back out by the same key (the store sanitizes the Store.getItem).

Links to any relevant code

/src/lib/controller/store.ts

Additional context

Add any other context or screenshots about the technical debt here.

@cmwylie19 cmwylie19 added this to the v0.33.0 milestone Jun 26, 2024
@mjnagel
Copy link
Contributor

mjnagel commented Jun 26, 2024

This would be a pretty big value add for uds-core. We started to go down the route of figuring out our own sanitization but were struggling with:

  • What names are valid for the store
  • How should we handle sanitization in such a way that keys maintain uniqueness (i.e. if there is a length limit and you just trim to 4 characters how do you ensure that foobar ends up different from foobaz

If pepr handled this in a way that we could just set and keys and be guaranteed that set/get calls would succeed and not retrieve the wrong value that would be great for our use case.

@cmwylie19 cmwylie19 self-assigned this Jun 27, 2024
@cmwylie19
Copy link
Collaborator Author

In researching other edge-cases, size does not seem to be a limitation.

    let longString = 'a'.repeat(253);
    const longerString = longString.repeat(1000);
    Store.setItem(longerString, longString);

@cmwylie19 cmwylie19 removed this from the v0.32.4 milestone Jun 27, 2024
@cmwylie19
Copy link
Collaborator Author

Moving to the next milestone since we need to plan with UDS Core how to roll this out to prod users

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants