Skip to content

Conversation

@Raubzeug
Copy link
Contributor

@Raubzeug Raubzeug commented Oct 28, 2025

closes #2935
Stand - only this one cluster is supported for now.

Greptile Overview

Updated On: 2025-10-28 10:47:53 UTC

Greptile Summary

This PR introduces environment-based routing to support different authentication services based on cluster environment. The implementation adds an optional environment parameter to all major routes (/:environment?/cluster, /:environment?/tenant, etc.) and extracts the environment from the URL's first path segment.

Key Changes:

  • Routes now support optional environment prefix (e.g., /cloud-prod/cluster)
  • Environment is extracted from URL when it matches the allowedEnvironments list
  • Created React hooks (useVDiskPagePath, useStorageGroupPath, useTabletPagePath) to ensure database context is properly included in generated paths
  • API calls prepend environment prefix to meta proxy paths
  • Refactored path generation across 69 files for consistency

Architecture:

  • Environment extraction happens at app initialization in getUrlData
  • Environment is stored globally and used in createHref to maintain consistent routing
  • Components use hooks instead of direct path generation to avoid prop drilling

The changes are well-structured and include comprehensive test coverage for environment extraction logic.

Confidence Score: 4/5

  • This PR is safe to merge with minor risk related to potential edge cases in route matching
  • The implementation is well-structured with comprehensive test coverage for URL parsing logic. The changes follow consistent patterns across the codebase. However, the extensive refactoring (69 files) and introduction of optional route parameters could have edge cases. The use of hooks for path generation is a good architectural choice that avoids prop drilling.
  • Pay close attention to src/routes.ts for route matching logic with optional environment parameter

Important Files Changed

File Analysis

Filename Score Overview
src/routes.ts 4/5 Added environment parameter to route definitions and created hooks (useVDiskPagePath, useStorageGroupPath, useTabletPagePath) to ensure database context is properly included in paths
src/store/getUrlData.ts 5/5 Extracts environment from first URL segment when in allowedEnvironments list for multi-cluster mode
src/store/configureStore.ts 5/5 Passes environments array to getUrlData and exposes environment to window object
src/services/api/meta.ts 5/5 Prepends environment prefix to meta proxy paths when environment is defined
src/utils/parseBalancer.ts 5/5 Prepends environment prefix when building backend URL from balancer and meta_backend
src/components/VDiskPopup/VDiskPopup.tsx 4/5 Refactored to use useVDiskPagePath hook instead of passing database query parameter directly

Sequence Diagram

sequenceDiagram
    participant App as App Init
    participant URL as getUrlData
    participant Store as configureStore
    participant Routes as Route Handler
    participant API as MetaAPI
    participant Component as React Component

    App->>URL: Extract URL data
    Note over URL: Parse pathname<br/>Check first segment<br/>against allowedEnvironments
    URL-->>Store: {environment, basename, backend, clusterName}
    Store->>Store: Set global environment variable
    Store->>Store: window.environment = environment
    
    Note over Component: User navigates to page
    Component->>Component: useVDiskPagePath() hook
    Note over Component: Hook captures database<br/>from query params
    Component->>Routes: createHref(route, params, query)
    Routes->>Routes: Add environment to params if set
    Routes->>Routes: Add database to query
    Routes-->>Component: /environment/vDisk?nodeId=X&vDiskId=Y&database=Z
    
    Component->>API: API request with path
    API->>API: getPath(path, clusterName)
    Note over API: Prepend environment<br/>if proxyMeta enabled
    API-->>API: /environment/api/meta3/proxy/cluster/name/path
    API->>API: Make HTTP request
    API-->>Component: Response data
Loading

CI Results

Test Status: ⚠️ FLAKY

📊 Full Report

Total Passed Failed Flaky Skipped
378 374 0 2 2
Test Changes Summary ⏭️2

⏭️ Skipped Tests (2)

  1. Scroll to row, get shareable link, navigate to URL and verify row is scrolled into view (tenant/diagnostics/tabs/queries.test.ts)
  2. Copy result button copies to clipboard (tenant/queryEditor/queryEditor.test.ts)

Bundle Size: 🔺

Current: 47.08 MB | Main: 47.08 MB
Diff: +6.20 KB (0.01%)

⚠️ Bundle size increased. Please review.

ℹ️ CI Information
  • Test recordings for failed tests are available in the full report.
  • Bundle size is measured for the entire 'dist' directory.
  • 📊 indicates links to detailed reports.
  • 🔺 indicates increase, 🔽 decrease, and ✅ no change in bundle size.

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

66 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

artemmufazalov
artemmufazalov previously approved these changes Oct 30, 2025
@Raubzeug Raubzeug enabled auto-merge October 30, 2025 09:58
@Raubzeug Raubzeug added this pull request to the merge queue Oct 30, 2025
Merged via the queue into main with commit 44dd3b4 Oct 30, 2025
5 checks passed
@Raubzeug Raubzeug deleted the env-in-path branch October 30, 2025 10:04
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.

Rework auth in YDB EM internal installation

4 participants