Skip to content

feat(BA-2806): Implement WebSocket connectionParams to HTTP headers conversion#6379

Merged
HyeockJinKim merged 3 commits into
mainfrom
feat/add-subscription-feature-hive
Oct 24, 2025
Merged

feat(BA-2806): Implement WebSocket connectionParams to HTTP headers conversion#6379
HyeockJinKim merged 3 commits into
mainfrom
feat/add-subscription-feature-hive

Conversation

@HyeockJinKim
Copy link
Copy Markdown
Collaborator

resolves #6378 (BA-2806)

Checklist: (if applicable)

  • Milestone metadata specifying the target backport version
  • Mention to the original issue
  • Installer updates including:
    • Fixtures for db schema changes
    • New mandatory config options
  • Update of end-to-end CLI integration tests in ai.backend.test
  • API server-client counterparts (e.g., manager API -> client SDK)
  • Test case(s) to:
    • Demonstrate the difference of before/after
    • Demonstrate the flow of abstract/conceptual models with a concrete implementation
  • Documentation
    • Contents in the docs directory
    • docstrings in public interfaces and type annotations

… and remove deprecated router and supergraph configurations
@HyeockJinKim HyeockJinKim self-assigned this Oct 24, 2025
Copilot AI review requested due to automatic review settings October 24, 2025 02:34
@github-actions github-actions Bot added the size:M 30~100 LoC label Oct 24, 2025
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR implements WebSocket connectionParams to HTTP headers conversion for the GraphQL gateway, enabling authentication parameters sent via WebSocket connection_init to be forwarded as HTTP headers in federation requests to subgraphs.

Key Changes:

  • Added a custom plugin (useConnectionParamsToHeadersPlugin) that intercepts subgraph execution to inject WebSocket connectionParams as HTTP headers
  • Updated propagateHeaders.fromClientToSubgraphs to use context-based headers instead of request headers
  • Configured WebSocket subscriptions with connectionParams that map context headers

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

File Description
configs/graphql/supergraph.yaml Removed legacy Apollo Router supergraph configuration
configs/graphql/router.yaml Removed legacy Apollo Router configuration
configs/graphql/gateway.config.ts Implemented WebSocket connectionParams to headers conversion plugin and updated gateway configuration

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

if (connectionParams && typeof connectionParams === 'object') {
// Wrap executor to inject connectionParams as HTTP headers
const originalExecutor = executor;
const wrappedExecutor = async (execRequest: any) => {
Copy link

Copilot AI Oct 24, 2025

Choose a reason for hiding this comment

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

The execRequest parameter uses any type, which bypasses TypeScript's type safety. Consider using a proper type from the gateway library or defining an appropriate interface for the execution request.

Suggested change
const wrappedExecutor = async (execRequest: any) => {
interface ExecutorRequest {
context: {
headers?: Record<string, string>;
[key: string]: any;
};
extensions?: Record<string, any>;
[key: string]: any;
}
const wrappedExecutor = async (execRequest: ExecutorRequest) => {

Copilot uses AI. Check for mistakes.
const originalExecutor = executor;
const wrappedExecutor = async (execRequest: any) => {
// Extract Authorization header from context if it exists
let reqHeaders = execRequest.context.headers || {};
Copy link

Copilot AI Oct 24, 2025

Choose a reason for hiding this comment

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

Mutating the original headers object with delete could cause unintended side effects. Create a copy of the headers object before modification: const reqHeaders = { ...(execRequest.context.headers || {}) }; followed by delete reqHeaders['content-length'];

Suggested change
let reqHeaders = execRequest.context.headers || {};
const reqHeaders = { ...(execRequest.context.headers || {}) };

Copilot uses AI. Check for mistakes.
Comment thread configs/graphql/gateway.config.ts Outdated
Comment on lines +73 to +75
connectionParams: {
token: '{context.headers.authorization}'
}
Copy link

Copilot AI Oct 24, 2025

Choose a reason for hiding this comment

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

The string literal '{context.headers.authorization}' appears to be a template string but may not be processed as expected. Add a comment explaining the expected format or mechanism by which this placeholder is resolved at runtime.

Suggested change
connectionParams: {
token: '{context.headers.authorization}'
}
// Set connectionParams as a function to dynamically inject the Authorization header at runtime
connectionParams: (context: any) => ({
token: context.headers?.authorization
})

Copilot uses AI. Check for mistakes.
@HyeockJinKim HyeockJinKim added this pull request to the merge queue Oct 24, 2025
Merged via the queue into main with commit 2123a02 Oct 24, 2025
28 checks passed
@HyeockJinKim HyeockJinKim deleted the feat/add-subscription-feature-hive branch October 24, 2025 02:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:M 30~100 LoC

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support WebSocket connectionParams to HTTP headers conversion for GraphQL subscriptions in Hive Router

2 participants