Skip to content

ROX-29844: Add configuration of base URL in plugin requests#16097

Merged
dvail merged 3 commits intomasterfrom
dv/ROX-29844-add-configuration-of-base-url-and-e2e-requests
Jul 31, 2025
Merged

ROX-29844: Add configuration of base URL in plugin requests#16097
dvail merged 3 commits intomasterfrom
dv/ROX-29844-add-configuration-of-base-url-and-e2e-requests

Conversation

@dvail
Copy link
Copy Markdown
Contributor

@dvail dvail commented Jul 18, 2025

Description

Adds the ability to configure the following:

  1. Whether or not the local console instance injects OpenShift auth headers
  2. The endpoint of central where API requests should be proxied to
  3. An optional ACS auth token that should be added to all API requests

This change will allow us to develop the dynamic plugin frontend with or without a working ACS-side implementation of authn, and against a central instance of our choosing. See more in the provided README docs.

In addition, in order to prevent request failures for POST requests, we need to use the console API's consoleFetch implementation to ensure the correct headers are set. Since we already use axios directly throughout, a wrapper around consoleFetch was added as an axios adapter to avoid changing application code.

User-facing documentation

Testing and quality

  • the change is production ready: the change is GA, or otherwise the functionality is gated by a feature flag
  • CI results are inspected

Automated testing

  • added unit tests
  • added e2e tests
  • added regression tests
  • added compatibility tests
  • modified existing tests

How I validated my change

Load the development console. The following should be displayed with both

ACS_INJECT_OCP_AUTH_TOKEN=false ./scripts/start-ocp-console.sh
ACS_CONSOLE_DEV_TOKEN="somelongtokenstring" npm run start:ocp-plugin

and the default values (made explicit here)

ACS_INJECT_OCP_AUTH_TOKEN=true ./scripts/start-ocp-console.sh
ACS_CONSOLE_DEV_TOKEN="" npm run start:ocp-plugin
image

@dvail
Copy link
Copy Markdown
Contributor Author

dvail commented Jul 18, 2025

@openshift-ci
Copy link
Copy Markdown

openshift-ci bot commented Jul 18, 2025

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@red-hat-konflux
Copy link
Copy Markdown
Contributor

Caution

There are some errors in your PipelineRun template.

PipelineRun Error
quay-proxy no kind "ImageDigestMirrorSet" is registered for version "config.openshift.io/v1" in scheme "k8s.io/client-go/kubernetes/scheme/register.go:83"

@rhacs-bot
Copy link
Copy Markdown
Contributor

rhacs-bot commented Jul 18, 2025

Images are ready for the commit at 323f0cf.

To use with deploy scripts, first export MAIN_IMAGE_TAG=4.9.x-339-g323f0cf916.

@codecov
Copy link
Copy Markdown

codecov bot commented Jul 18, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 48.70%. Comparing base (cd5279e) to head (323f0cf).
⚠️ Report is 25 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #16097      +/-   ##
==========================================
- Coverage   48.71%   48.70%   -0.01%     
==========================================
  Files        2606     2606              
  Lines      191771   191771              
==========================================
- Hits        93418    93411       -7     
- Misses      91024    91029       +5     
- Partials     7329     7331       +2     
Flag Coverage Δ
go-unit-tests 48.70% <ø> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@dvail dvail force-pushed the dv/ROX-29845-add-start-console-dev-script branch from 0e16c8e to e6f96db Compare July 21, 2025 12:33
@dvail dvail force-pushed the dv/ROX-29844-add-configuration-of-base-url-and-e2e-requests branch from db97522 to edc05dc Compare July 21, 2025 12:33
@dvail dvail force-pushed the dv/ROX-29845-add-start-console-dev-script branch from e6f96db to 63646a8 Compare July 22, 2025 15:22
@dvail dvail force-pushed the dv/ROX-29844-add-configuration-of-base-url-and-e2e-requests branch from edc05dc to e506b60 Compare July 22, 2025 15:22
@dvail dvail force-pushed the dv/ROX-29845-add-start-console-dev-script branch from 63646a8 to 1941fcb Compare July 23, 2025 12:10
@dvail dvail force-pushed the dv/ROX-29844-add-configuration-of-base-url-and-e2e-requests branch 2 times, most recently from 2f0fb7b to 198757b Compare July 23, 2025 16:02
@dvail dvail force-pushed the dv/ROX-29845-add-start-console-dev-script branch from 1941fcb to ad66613 Compare July 23, 2025 19:12
@dvail dvail force-pushed the dv/ROX-29844-add-configuration-of-base-url-and-e2e-requests branch from 198757b to 0e49dc9 Compare July 23, 2025 19:12
@dvail dvail force-pushed the dv/ROX-29845-add-start-console-dev-script branch 2 times, most recently from dcf8a52 to 64d0116 Compare July 23, 2025 19:54
@dvail dvail force-pushed the dv/ROX-29844-add-configuration-of-base-url-and-e2e-requests branch from 0e49dc9 to b66681e Compare July 23, 2025 20:49
@dvail dvail force-pushed the dv/ROX-29845-add-start-console-dev-script branch from 64d0116 to eff3e46 Compare July 23, 2025 21:05
@dvail dvail force-pushed the dv/ROX-29844-add-configuration-of-base-url-and-e2e-requests branch 2 times, most recently from 2627b4d to 8bc4842 Compare July 24, 2025 19:14
@dvail dvail marked this pull request as ready for review July 24, 2025 19:34
@dvail dvail requested a review from a team as a code owner July 24, 2025 19:34
Copy link
Copy Markdown
Contributor

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

Hey @dvail - I've reviewed your changes - here's some feedback:

  • Avoid mutating axios.defaults.adapter globally; instead, apply the consoleFetch adapter on a dedicated axios instance to prevent unintended side effects for other modules.
  • In consoleFetchAxiosAdapter, consider parsing JSON responses automatically based on Content-Type so non-GraphQL REST endpoints return structured objects instead of raw strings.
  • In webpack resolve.extensions, it’s more conventional (and safer) to list .ts/.tsx before .js/.jsx so TypeScript sources are resolved before their JavaScript counterparts.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Avoid mutating axios.defaults.adapter globally; instead, apply the consoleFetch adapter on a dedicated axios instance to prevent unintended side effects for other modules.
- In consoleFetchAxiosAdapter, consider parsing JSON responses automatically based on Content-Type so non-GraphQL REST endpoints return structured objects instead of raw strings.
- In webpack resolve.extensions, it’s more conventional (and safer) to list .ts/.tsx before .js/.jsx so TypeScript sources are resolved before their JavaScript counterparts.

## Individual Comments

### Comment 1
<location> `ui/apps/platform/src/ConsolePlugin/consoleFetchAxiosAdapter.ts:44` </location>
<code_context>
+            };
+        })
+        .catch((error) => {
+            throw new AxiosError(error.message);
+        });
+}
</code_context>

<issue_to_address>
AxiosError instantiation may lose important error context.

Passing only error.message discards valuable information like the stack trace. Pass the original error or include additional context to preserve debugging details.
</issue_to_address>

<suggested_fix>
<<<<<<< SEARCH
        .catch((error) => {
+            throw new AxiosError(error.message);
=======
        .catch((error) => {
+            // Preserve original error context by passing the original error object and stack trace
+            const axiosError = new AxiosError(
+                error.message,
+                undefined,
+                config,
+                undefined,
+                error.response
+            );
+            axiosError.stack = error.stack;
+            // Attach the original error for further debugging if needed
+            (axiosError as any).originalError = error;
+            throw axiosError;
>>>>>>> REPLACE

</suggested_fix>

### Comment 2
<location> `ui/apps/platform/src/ConsolePlugin/consoleFetchAxiosAdapter.ts:27` </location>
<code_context>
+            // GraphQL request errors are JSON objects with an `errors` field an a HTTP status code of 200, so we
+            // need to check for that and throw an ApolloError
+            if (config.url?.startsWith('/api/graphql')) {
+                const json = JSON.parse(data);
+                if ('errors' in json) {
+                    throw new ApolloError({ graphQLErrors: json.errors });
</code_context>

<issue_to_address>
Potential unhandled exception if response is not valid JSON.

Wrap JSON.parse in a try/catch block or check the response content type before parsing to prevent unhandled exceptions.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@dvail dvail force-pushed the dv/ROX-29845-add-start-console-dev-script branch from eff3e46 to e835448 Compare July 24, 2025 20:19
@dvail dvail force-pushed the dv/ROX-29844-add-configuration-of-base-url-and-e2e-requests branch from 8bc4842 to 2917fa3 Compare July 24, 2025 20:19
Base automatically changed from dv/ROX-29845-add-start-console-dev-script to master July 25, 2025 12:27
@dvail dvail force-pushed the dv/ROX-29844-add-configuration-of-base-url-and-e2e-requests branch 2 times, most recently from 5ecd4b9 to 1c986d8 Compare July 28, 2025 18:51
@dvail dvail force-pushed the dv/ROX-29844-add-configuration-of-base-url-and-e2e-requests branch from b57a431 to 323f0cf Compare July 29, 2025 12:22
Copy link
Copy Markdown
Contributor

@sachaudh sachaudh left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks!

@dvail dvail merged commit ad003c9 into master Jul 31, 2025
152 of 181 checks passed
@dvail dvail deleted the dv/ROX-29844-add-configuration-of-base-url-and-e2e-requests branch July 31, 2025 11:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants