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.

Sign up for GitHub

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

Jump to bottom

fix(multitenancy): consistently use tenant nomenclature everywhere #6389

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

fix(multitenancy): consistently use tenant nomenclature everywhere #6389

wants to merge 5 commits into from
+111 −43

Conversation

dracarys18
Copy link
Member

@dracarys18 dracarys18 commented Oct 22, 2024 •
edited
Loading

Type of Change

  • Enhancement

Description

This fixes the inconsistent nomenclature used for tenants in router and drainer code

Additional Changes

  • This PR modifies application configuration/environment variables

Motivation and Context

This will eradicate confusion between name and tenant_id names in the tenant config. In the config below
public = { base_url = "http://localhost:8080/", schema = "public", redis_key_prefix = "", clickhouse_database = "default"}

The key public will be the tenant_id. This PR adds a custom deserializer for this part

How did you test it?

This cannot be tested on sandbox without enabling Multi Tenancy

  1. Application runs with the changed config and so does drainer
  2. Do a request with a valid x-tenant-id
curl --location 'http://localhost:8080/accounts' \
--header 'Content-Type: application/json' \
--header 'Accept: application/json' \
--header 'x-tenant-id: public' \
--header 'api-key: test_admin' \
--data-raw '{
  "merchant_id": "1729601216",
  "locker_id": "m0010",
  "merchant_name": "NewAge Retailer",
  "merchant_details": {
    "primary_contact_person": "John Test",
    "primary_email": "[email protected]",
    "primary_phone": "sunt laborum",
    "secondary_contact_person": "John Test2",
    "secondary_email": "[email protected]",
    "secondary_phone": "cillum do dolor id",
    "website": "www.example.com",
    "about_business": "Online Retail with a wide selection of organic products for North America",
    "address": {
      "line1": "1467",
      "line2": "Harrison Street",
      "line3": "Harrison Street",
      "city": "San Fransico",
      "state": "California",
      "zip": "94122",
      "country": "US"
    }
  },
  "return_url": "https://google.com/success",
  "webhook_details": {
    "webhook_version": "1.0.1",
    "webhook_username": "ekart_retail",
    "webhook_password": "password_ekart@123",
    "payment_created_enabled": true,
    "payment_succeeded_enabled": true,
    "payment_failed_enabled": true
  },
  "sub_merchants_enabled": false,
  "metadata": {
    "city": "NY",
    "unit": "245"
  },
  "primary_business_details": [
    {
      "country": "US",
      "business": "default"
    }
  ]
}'

It should return 200 and tenant-id should be the which has been passed

  1. Try the request with invalid tenant-id, It should return an error

Checklist

  • I formatted the code cargo +nightly fmt --all
  • I addressed lints thrown by cargo clippy
  • I reviewed the submitted code

@dracarys18 dracarys18 added A-core Area: Core flows S-waiting-on-review Status: This PR has been implemented and needs to be reviewed labels Oct 22, 2024
@dracarys18 dracarys18 added this to the October 2024 Release milestone Oct 22, 2024
@dracarys18 dracarys18 self-assigned this Oct 22, 2024
@dracarys18 dracarys18 requested review from a team as code owners October 22, 2024 09:27
@semanticdiff-com SemanticDiff.com
Copy link

semanticdiff-com bot commented Oct 22, 2024 •
edited
Loading

Review changes with SemanticDiff.

Analyzed 7 of 12 files.

Overall, the semantic diff is 23% smaller than the GitHub diff.

Filename Status
❔ loadtest/config/development.toml Unsupported file format
✔️ crates/router/src/types/storage/payment_attempt.rs Analyzed
✔️ crates/router/src/services/api.rs 34.32% smaller
✔️ crates/router/src/routes/app.rs Analyzed
✔️ crates/router/src/core/routing/helpers.rs Analyzed
✔️ crates/router/src/core/payment_methods/cards.rs Analyzed
✔️ crates/router/src/configs/settings.rs 20.17% smaller
✔️ crates/drainer/src/settings.rs 20.26% smaller
❔ config/config.example.toml Unsupported file format
❔ config/development.toml Unsupported file format
❔ config/docker_compose.toml Unsupported file format
❔ config/deployments/env_specific.toml Unsupported file format

@dracarys18 dracarys18 changed the title Multitenancy config fix(multitenancy): consistently use tenant nomenclature everywhere Oct 22, 2024
@dracarys18 dracarys18 linked an issue Oct 22, 2024 that may be closed by this pull request
[FIX] fix tenant nomenclature in the code #6390
Open
jagan-jaya
jagan-jaya previously approved these changes Oct 22, 2024
View reviewed changes
apoorvdixit88
apoorvdixit88 reviewed Oct 22, 2024
View reviewed changes
Copy link
Contributor

@apoorvdixit88 apoorvdixit88 left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we please update following as well?

  • evn_specific.toml
  • config.example.toml

Also we have to make corresponding config changes in Cloud as well?

@dracarys18
Copy link
Member Author

Done @apoorvdixit88

@SanchithHegde
Copy link
Member

Not a blocker, can you also update the tenant ID being logged in the middleware?

hyperswitch/crates/router/src/middleware.rs

Line 169 in 829a20c

tenant_id = "ta"

@apoorvdixit88
Copy link
Contributor

Not a blocker, can you also update the tenant ID being logged in the middleware?

hyperswitch/crates/router/src/middleware.rs

Line 169 in 829a20c

tenant_id = "ta"

@SanchithHegde I will cover this is my upcoming PRs related to tenancy. If this works?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jagan-jaya jagan-jaya jagan-jaya approved these changes

@jarnura jarnura jarnura approved these changes

@apoorvdixit88 apoorvdixit88 apoorvdixit88 approved these changes

@Chethan-rao Chethan-rao Chethan-rao approved these changes

@racnan racnan Awaiting requested review from racnan

Assignees

@dracarys18 dracarys18

Labels
A-core Area: Core flows S-waiting-on-review Status: This PR has been implemented and needs to be reviewed
Projects
None yet
Milestone
October 2024 Release
Development

Successfully merging this pull request may close these issues.

[FIX] fix tenant nomenclature in the code
6 participants
@dracarys18 @SanchithHegde @apoorvdixit88 @jagan-jaya @jarnura @Chethan-rao