chore: add internal package called @unkey/vault #2279
Conversation
harshsbhat
requested review from
perkinsjr,
chronark,
mcstepp and
MichaelUnkey
as code owners
|
|
Caution Review failedThe pull request is closed. 📝 Walkthrough📝 WalkthroughWalkthroughThe changes in this pull request involve the addition of a new package, Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant VaultService
Client->>VaultService: encrypt(EncryptRequest)
VaultService-->>Client: EncryptResponse
Client->>VaultService: encryptBulk(EncryptBulkRequest)
VaultService-->>Client: EncryptBulkResponse
Client->>VaultService: decrypt(DecryptRequest)
VaultService-->>Client: DecryptResponse
Possibly related PRs
Suggested labels
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
@harshsbhat is attempting to deploy a commit to the Unkey Team on Vercel. A member of the Team first needs to authorize it. |
|
Thank you for following the naming conventions for pull request titles! 🙏 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (3)
- apps/dashboard/package.json (1 hunks)
- internal/vault/package.json (1 hunks)
- internal/vault/src/index.ts (1 hunks)
✅ Files skipped from review due to trivial changes (1)
- internal/vault/package.json
🧰 Additional context used
🔇 Additional comments (1)
apps/dashboard/package.json (1)
57-57: LGTM! Verify the package exists in the workspace.The addition of
"@unkey/vault": "workspace:^"as a dependency is consistent with the PR objectives to introduce a new internal library for encryption and decryption. Theworkspace:^version correctly indicates that this is a local package within the same monorepo.To ensure the package exists in the workspace, run the following script:
✅ Verification successful
Verification Successful!
The addition of
"@unkey/vault": "workspace:^"inapps/dashboard/package.jsonhas been confirmed. The@unkey/vaultpackage exists in the workspace and itspackage.jsoncorrectly names it as@unkey/vault.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the existence of @unkey/vault package in the workspace # Test: Check if the package.json for @unkey/vault exists if fd -t f "package.json" | rg "/vault/package.json"; then echo "✅ @unkey/vault package found in the workspace" else echo "❌ @unkey/vault package not found in the workspace" fi # Test: Verify the package name in vault's package.json fd -t f "package.json" | rg "/vault/package.json" | xargs cat | jq -r '.name' | rg "^@unkey/vault$" && echo "✅ Package name in vault's package.json is correct" || echo "❌ Package name in vault's package.json is incorrect or not found"Length of output: 397
There was a problem hiding this comment.
This is essentially a copy of https://github.com/unkeyed/unkey/blob/main/apps/api/src/pkg/vault.ts right?
As part of this PR, could you remove it in the api and hook it up to use the shared library?
Makes sense. That would reduce repeated code. |
@chronark what about metrics? We probably don't need it for the dashboard. But as the library is shared we would need to include them regardless right? That would pretty much mean moving the metrics into the internal too right? |
|
https://github.com/unkeyed/unkey/blob/main/apps/api/src/pkg/metrics/interface.ts it's a very small interface, we can just copy that as well and make it optional |
why optional? we don't need it for the dashboard but having it won't hurt right? |
|
Yes, we won't emit metrics from the dashboard, only in the api |
|
@chronark I was planning to work on this after we were done with the hackathon. Considering this one is going to require a lot of reviews from you guys which might not be possible right now during oss.gg. Hope that is fine/ |
|
ah, you did the requested changes and merged it lol. My bad. |
What does this PR do?
Adds a new internal library called
@unkey/vaultwith encryption and decryption so that we can connect dashboard to the vaultType of change
How should this be tested?
You can import this using the following line and use it in the dashboard
import { type EncryptRequest, type RequestContext, Vault } from "@unkey/vault";Checklist
Required
pnpm buildpnpm fmtconsole.logsgit pull origin mainAppreciated
Summary by CodeRabbit
New Features
@unkey/vaultpackage for encryption and decryption operations.Vaultclass, including methods for single and bulk operations.Chores
@unkey/dashboardproject to include the new@unkey/vaultdependency.