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

feat(multitenant)! Account prefixed API topics and manifest storage #133

Merged
merged 3 commits into from
Jul 19, 2023

Conversation

brooksmtownsend
Copy link
Member

@brooksmtownsend brooksmtownsend commented Jul 18, 2023

Feature or Problem

This PR ensures we are storing and querying manifests based on an account prefix if running in multitenant mode. This ensures that accounts can only access and modify manifests that are specifically bound to that account, rather than simply using a global lattice ID.

Due to possible edge cases, it's still recommended to keep lattice prefixes globally unique between accounts, as there are not e2e tests at this time to verify that functionality works without issues. This PR at least implements the portion of that that constrains manifests from conflicting between accounts.

Related Issues

#132

Release Information

v0.5.0

Consumer Impact

Multitenancy hasn't released yet so this shouldn't affect consumers, however they will need to examine the account prefixed topic if manually looking up manifests in NATS.

Testing

Built on platform(s)

  • x86_64-linux
  • aarch64-linux
  • x86_64-darwin
  • aarch64-darwin
  • x86_64-windows

Tested on platform(s)

  • x86_64-linux
  • aarch64-linux
  • x86_64-darwin
  • aarch64-darwin
  • x86_64-windows

Unit Test(s)

Modified a few unit tests that were hitting deprecation warnings

Acceptance or Integration

Will have to modify the e2e tests to properly import/export account prefixes

Manual Verification

Using the multitenant docker compose, I was able to verify that if I put a manifest using one account, I cannot query that manifest in another account even if I use the correct lattice prefix 😄

@brooksmtownsend brooksmtownsend changed the base branch from main to feat/multitenant-mode July 18, 2023 15:47
Base automatically changed from feat/multitenant-mode to main July 18, 2023 17:00
Signed-off-by: Brooks Townsend <brooks@cosmonic.com>
@brooksmtownsend brooksmtownsend force-pushed the feat/multitenant-api-manifest-storage branch from 61f0d86 to 9b650d1 Compare July 18, 2023 17:00
@brooksmtownsend brooksmtownsend force-pushed the feat/multitenant-api-manifest-storage branch from 9b650d1 to e73023b Compare July 18, 2023 17:13
@brooksmtownsend brooksmtownsend marked this pull request as ready for review July 18, 2023 17:19
src/server/mod.rs Outdated Show resolved Hide resolved
src/server/mod.rs Show resolved Hide resolved
src/server/mod.rs Outdated Show resolved Hide resolved
@brooksmtownsend brooksmtownsend force-pushed the feat/multitenant-api-manifest-storage branch from e73023b to eec783a Compare July 18, 2023 19:24
Signed-off-by: Brooks Townsend <brooks@cosmonic.com>

reworked e2e account import/exports

Signed-off-by: Brooks Townsend <brooks@cosmonic.com>

removed unnecessary stream response type

Signed-off-by: Brooks Townsend <brooks@cosmonic.com>

addressed PR concerns

Signed-off-by: Brooks Townsend <brooks@cosmonic.com>
@brooksmtownsend brooksmtownsend force-pushed the feat/multitenant-api-manifest-storage branch from eec783a to ecbc0df Compare July 18, 2023 19:29
Signed-off-by: Brooks Townsend <brooks@cosmonic.com>
@brooksmtownsend brooksmtownsend merged commit 01b1115 into main Jul 19, 2023
3 checks passed
@brooksmtownsend brooksmtownsend deleted the feat/multitenant-api-manifest-storage branch July 19, 2023 17:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants