ROX-29844: Add configuration of base URL in plugin requests#16097
ROX-29844: Add configuration of base URL in plugin requests#16097
Conversation
|
This change is part of the following stack: Change managed by git-spice. |
|
Skipping CI for Draft Pull Request. |
|
Caution There are some errors in your PipelineRun template.
|
|
Images are ready for the commit at 323f0cf. To use with deploy scripts, first |
Codecov Report✅ All modified and coverable lines are covered by tests. 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
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
0e16c8e to
e6f96db
Compare
db97522 to
edc05dc
Compare
e6f96db to
63646a8
Compare
edc05dc to
e506b60
Compare
63646a8 to
1941fcb
Compare
2f0fb7b to
198757b
Compare
1941fcb to
ad66613
Compare
198757b to
0e49dc9
Compare
dcf8a52 to
64d0116
Compare
0e49dc9 to
b66681e
Compare
64d0116 to
eff3e46
Compare
2627b4d to
8bc4842
Compare
ui/apps/platform/src/ConsolePlugin/SecurityVulnerabilitiesPage/Index.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
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>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
eff3e46 to
e835448
Compare
8bc4842 to
2917fa3
Compare
5ecd4b9 to
1c986d8
Compare
b57a431 to
323f0cf
Compare
Description
Adds the ability to configure the following:
centralwhere API requests should be proxied toThis 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
consoleFetchimplementation to ensure the correct headers are set. Since we already use axios directly throughout, a wrapper aroundconsoleFetchwas added as an axios adapter to avoid changing application code.User-facing documentation
Testing and quality
Automated testing
How I validated my change
Load the development console. The following should be displayed with both
and the default values (made explicit here)