CONSOLE-5426: remove GraphQL proxy endpoint#16769
Conversation
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
|
@logonoff: This pull request references CONSOLE-5139 which is a valid jira issue. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
@logonoff: This pull request references CONSOLE-5426 which is a valid jira issue. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: openshift/coderabbit/.coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: ⛔ Files ignored due to path filters (62)
📒 Files selected for processing (37)
💤 Files with no reviewable changes (18)
🚧 Files skipped from review as they are similar to previous changes (16)
WalkthroughThe change removes GraphQL server and frontend integrations, adds authenticated Kubernetes API discovery, migrates feature and resource detection to REST APIs, updates Helm repository detection, and adjusts frontend build tooling and container images. ChangesKubernetes REST migration
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Frontend
participant ConsoleServer
participant KubernetesAPI
Frontend->>ConsoleServer: GET /api/api-discovery
ConsoleServer->>KubernetesAPI: Fetch API groups and resource lists
KubernetesAPI-->>ConsoleServer: Discovery data
ConsoleServer-->>Frontend: Combined resource discovery response
Suggested reviewers: 🚥 Pre-merge checks | ✅ 14 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (14 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Trivy (0.72.0)Trivy execution failed: 2026-07-17T13:15:12Z FATAL Fatal error run error: fs scan error: scan error: scan failed: failed analysis: post analysis error: post analysis error: cloudformation scan error: fs filter error: fs filter error: walk error range error: stat .golangci.yml: no such file or directory: range error: stat .golangci.yml: no such file or directory Comment |
1dda7cb to
f3cc85c
Compare
|
/label px-approved |
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pkg/server/api_discovery.go`:
- Around line 133-142: Update the header-forwarding logic in the discovery
request handler to parse X-Console-Impersonate-Groups using the existing proxy
semantics, convert its comma-separated values into Impersonate-Group headers,
and include system:authenticated as required. Adjust the related test to provide
the browser-visible header instead of repeated Impersonate-Group headers.
- Line 57: Update k8sFetch and its request flow to accept and propagate
context.Context from the caller, deriving an overall timeout context that covers
connection, response-header, and response-body waits. Create the HTTP request
with that context instead of a detached request, ensure the context is canceled,
and pass it through all discovery goroutines so browser disconnects and timeout
expiry allow wg.Wait() to complete.
- Around line 107-117: Initialize resourceLists as an empty non-nil slice before
filtering results in the discovery response construction, so all failed fetches
serialize as [] rather than null. Update the relevant API discovery handler and
add a regression test covering the case where every resource-list request fails,
asserting the response contains an empty resourceLists array.
- Around line 45-55: Update the transport construction in the API discovery
setup to honor proxy.Config.UseProxyFromEnvironment: use
http.ProxyFromEnvironment only when the setting is enabled, and disable proxy
selection otherwise. Preserve the existing TLS, dialing, timeout, and HTTP/2
configuration.
In `@pkg/server/server.go`:
- Line 361: Update the discovery request flow in api_discovery.go to create
outbound requests with http.NewRequestWithContext using the incoming request
context, preserving cancellation when the caller disconnects. Configure the HTTP
client used by k8sFetch with a bounded Timeout so stalled discovery requests
cannot run indefinitely.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 0becf735-ca61-44a5-8468-911c32f14171
⛔ Files ignored due to path filters (62)
frontend/yarn.lockis excluded by!**/yarn.lock,!**/*.lockgo.sumis excluded by!**/*.sumvendor/github.com/graph-gophers/graphql-go/.gitignoreis excluded by!**/vendor/**,!vendor/**vendor/github.com/graph-gophers/graphql-go/.golangci.ymlis excluded by!**/vendor/**,!vendor/**vendor/github.com/graph-gophers/graphql-go/CHANGELOG.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/graph-gophers/graphql-go/CODE_OF_CONDUCT.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/graph-gophers/graphql-go/CONTRIBUTING.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/graph-gophers/graphql-go/LICENSEis excluded by!**/vendor/**,!vendor/**vendor/github.com/graph-gophers/graphql-go/README.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/graph-gophers/graphql-go/SECURITY.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/graph-gophers/graphql-go/ast/argument.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/graph-gophers/graphql-go/ast/directive.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/graph-gophers/graphql-go/ast/doc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/graph-gophers/graphql-go/ast/enum.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/graph-gophers/graphql-go/ast/extension.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/graph-gophers/graphql-go/ast/field.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/graph-gophers/graphql-go/ast/fragment.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/graph-gophers/graphql-go/ast/input.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/graph-gophers/graphql-go/ast/interface.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/graph-gophers/graphql-go/ast/object.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/graph-gophers/graphql-go/ast/query.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/graph-gophers/graphql-go/ast/scalar.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/graph-gophers/graphql-go/ast/schema.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/graph-gophers/graphql-go/ast/types.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/graph-gophers/graphql-go/ast/union.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/graph-gophers/graphql-go/ast/value.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/graph-gophers/graphql-go/ast/variable.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/graph-gophers/graphql-go/decode/decode.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/graph-gophers/graphql-go/errors/errors.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/graph-gophers/graphql-go/errors/panic_handler.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/graph-gophers/graphql-go/graphql.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/graph-gophers/graphql-go/id.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/graph-gophers/graphql-go/internal/common/blockstring.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/graph-gophers/graphql-go/internal/common/directive.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/graph-gophers/graphql-go/internal/common/lexer.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/graph-gophers/graphql-go/internal/common/literals.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/graph-gophers/graphql-go/internal/common/types.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/graph-gophers/graphql-go/internal/common/values.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/graph-gophers/graphql-go/internal/exec/exec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/graph-gophers/graphql-go/internal/exec/packer/packer.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/graph-gophers/graphql-go/internal/exec/resolvable/meta.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/graph-gophers/graphql-go/internal/exec/resolvable/resolvable.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/graph-gophers/graphql-go/internal/exec/selected/selected.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/graph-gophers/graphql-go/internal/exec/subscribe.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/graph-gophers/graphql-go/internal/exec/validate.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/graph-gophers/graphql-go/internal/query/query.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/graph-gophers/graphql-go/internal/schema/meta.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/graph-gophers/graphql-go/internal/schema/schema.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/graph-gophers/graphql-go/internal/validation/suggestion.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/graph-gophers/graphql-go/internal/validation/validation.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/graph-gophers/graphql-go/introspection.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/graph-gophers/graphql-go/introspection/introspection.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/graph-gophers/graphql-go/log/log.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/graph-gophers/graphql-go/nullable_types.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/graph-gophers/graphql-go/relay/relay.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/graph-gophers/graphql-go/subscriptions.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/graph-gophers/graphql-go/time.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/graph-gophers/graphql-go/trace/noop/trace.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/graph-gophers/graphql-go/trace/tracer/tracer.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/rawagner/graphql-transport-ws/graphqlws/http.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/rawagner/graphql-transport-ws/graphqlws/internal/connection/connection.gois excluded by!**/vendor/**,!vendor/**vendor/modules.txtis excluded by!**/vendor/**,!vendor/**
📒 Files selected for processing (36)
DockerfileDockerfile.devDockerfile.plugins.demofrontend/.eslintrc.jsfrontend/@types/console/index.d.tsfrontend/@types/console/window.d.tsfrontend/graphql-codegen.ymlfrontend/package.jsonfrontend/packages/console-dynamic-plugin-sdk/package.jsonfrontend/packages/eslint-plugin-console/package.jsonfrontend/packages/helm-plugin/src/providers/__tests__/helm-detection-provider.spec.tsfrontend/packages/helm-plugin/src/providers/helm-detection-provider.tsfrontend/public/actions/features.gqlfrontend/public/actions/features.tsfrontend/public/actions/flags.tsfrontend/public/actions/ui.tsfrontend/public/components/app.tsxfrontend/public/components/useImpersonateRefreshFeatures.tsfrontend/public/graphql/client.gqlfrontend/public/graphql/client.tsfrontend/public/locales/en/public.jsonfrontend/public/models/index.tsfrontend/public/module/k8s/get-resources.tsfrontend/scripts/jest-transform-graphql.jsfrontend/webpack.config.tsgo.modpkg/graphql/README.mdpkg/graphql/httphandler.gopkg/graphql/resolver/k8s.gopkg/graphql/resolver/root.gopkg/graphql/resolver/utils.gopkg/graphql/schema.graphqlpkg/server/api_discovery.gopkg/server/api_discovery_test.gopkg/server/server.gotest-frontend.sh
💤 Files with no reviewable changes (17)
- frontend/public/actions/features.gql
- frontend/graphql-codegen.yml
- frontend/public/graphql/client.gql
- frontend/@types/console/index.d.ts
- pkg/graphql/README.md
- pkg/graphql/resolver/utils.go
- frontend/@types/console/window.d.ts
- pkg/graphql/schema.graphql
- Dockerfile
- frontend/scripts/jest-transform-graphql.js
- frontend/packages/eslint-plugin-console/package.json
- pkg/graphql/resolver/root.go
- Dockerfile.dev
- frontend/public/graphql/client.ts
- pkg/graphql/httphandler.go
- go.mod
- pkg/graphql/resolver/k8s.go
f3cc85c to
3aacef0
Compare
Performance Benchmark: GraphQL RemovalAutomated benchmark comparing before (GraphQL/WebSocket, Methodology: 60 page loads via Playwright CDP — 5 iterations × 3 CPU throttle rates × 2 cache modes. Metrics collected: Navigation Timing API, CDP Loaded CPU 1x (no throttle) — Cold Cache
CPU 4x — Cold Cache
CPU 8x — Cold Cache
CPU 8x — Warm Cache
SummaryThe GraphQL removal is a net performance improvement that scales with CPU constraint:
Note Benchmark ran in headless Chromium via Playwright CDP with Automated performance benchmark by Claude Code |
3aacef0 to
560743f
Compare
|
/label acknowledge-critical-fixes-only |
QA Verification Evidence
Verification Steps
Step 2: Projects list - detectCanCreateProject (pass)
Step 4: Deployments list - resource discovery (pass)
Step 6: ClusterVersion - detectClusterVersion (pass)
Warning This verification was performed by an AI agent. Results may contain false positives or miss Automated QA verification by Claude Code |
|
/verified by claude |
|
Scheduling tests matching the |
|
@logonoff: This PR has been marked as verified by DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
| } | ||
| } | ||
|
|
||
| func k8sFetch(ctx context.Context, client *http.Client, endpoint *url.URL, path string, originalReq *http.Request) ([]byte, error) { |
There was a problem hiding this comment.
I think we need to review the necessity of this function. We should be able to reuse an existing k8s client from somewhere instead of writing a new fetch function.
There was a problem hiding this comment.
Fair point.. claude found a k8s proxy that could be reused after questioning it a bit more
|
@logonoff: This pull request references CONSOLE-5426 which is a valid jira issue. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
/verified by claude |
|
Scheduling tests matching the |
|
@logonoff: This PR has been marked as verified by DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
After the GraphQL removal, API discovery moved from a single server-side fan-out to ~150 individual browser requests. This adds a /api/api-discovery endpoint that performs the fan-out on the bridge server and returns all API group resource lists in a single gzip-compressed response, reducing browser round-trips. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Route API discovery requests through the existing k8s reverse proxy instead of constructing a separate http.Client with duplicated TLS, transport, and impersonation logic. This eliminates ~50 lines of hand-rolled infrastructure and inherits the proxy's header handling (X-Console-Impersonate-Groups splitting, system:authenticated injection, header blacklist) automatically. Also fixes: null resourceLists serialization when all fetches fail, and redacts internal hostnames from error messages. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
64b7429 to
2036574
Compare
|
removed an extra unused dev dependency /verified by claude |
|
Scheduling tests matching the |
|
@logonoff: This PR has been marked as verified by DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
@logonoff: This pull request references CONSOLE-5426 which is a valid jira issue. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
Tests from second stage were triggered manually. Pipeline can be controlled only manually, until HEAD changes. Use command to trigger second stage. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: logonoff, TheRealJon The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/retest |
|
@logonoff: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |






















Analysis / Root cause:
The graphQL proxy adds unnecessary complexity, maintenance burden, and may even be slower than regular HTTP/2.
Solution description:
Test cases:
Browser conformance:
Summary by CodeRabbit
/api/api-discoveryendpoint to streamline resource loading and API discovery.SelfSubject*Kubernetes resources.