feat: add IOP mode support with custom routes - #2036
Merged
Merged
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Reviewer's GuideAdds IOP mode support for the Advisor frontend by patching the federated modules config, wiring new IOP-specific entry points, and introducing an IOP proxy start script and custom routes configuration, along with a required frontend-components-config version bump. Flow diagram for start:proxy:iop development startupflowchart LR
Dev[Developer]
NpmScriptStartProxyIop[npm run start:proxy:iop]
FecDevProxy[fec dev-proxy --iop]
EnvVars[IOP=true, HCC_ENV=iop,<br>HCC_ENV_URL, FEC_IOP_CUSTOM_ROUTES_PATH]
AdvisorApp[Advisor frontend in IOP mode]
Dev --> NpmScriptStartProxyIop
NpmScriptStartProxyIop --> EnvVars
EnvVars --> FecDevProxy
FecDevProxy --> AdvisorApp
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The
start:proxy:iopscript relies on shell-specific syntax like$(pwd)and$IOP_URL, which may not work on all environments (e.g. Windows/npm on non-bash shells); consider using a cross-platform approach or deferring this logic to a small Node script. - The
custom_routes.jsoncurrently hardcodes port 8004; consider reading the target port from an environment variable or shared config so the routes automatically stay in sync with the--staticPortyou pass to the dev server.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `start:proxy:iop` script relies on shell-specific syntax like `$(pwd)` and `$IOP_URL`, which may not work on all environments (e.g. Windows/npm on non-bash shells); consider using a cross-platform approach or deferring this logic to a small Node script.
- The `custom_routes.json` currently hardcodes port 8004; consider reading the target port from an environment variable or shared config so the routes automatically stay in sync with the `--staticPort` you pass to the dev server.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
This was referenced Jun 24, 2026
Merged
adonispuente
marked this pull request as draft
June 25, 2026 20:57
adonispuente
marked this pull request as ready for review
July 27, 2026 19:38
There was a problem hiding this comment.
Hey - I've found 9 security issues, 2 other issues, and left some high level feedback:
Security issues:
- FSL-1.1-MIT: Open-source license could not be identified (link)
- FSL-1.1-MIT: Open-source license could not be identified (link)
- FSL-1.1-MIT: Open-source license could not be identified (link)
- FSL-1.1-MIT: Open-source license could not be identified (link)
- FSL-1.1-MIT: Open-source license could not be identified (link)
- FSL-1.1-MIT: Open-source license could not be identified (link)
- FSL-1.1-MIT: Open-source license could not be identified (link)
- FSL-1.1-MIT: Open-source license could not be identified (link)
- FSL-1.1-MIT: Open-source license could not be identified (link)
General comments:
- The global
require('./config/patchFederationForIop');infec.config.jswill modify the federated-modules behaviour for all environments; consider gating this patch behindprocess.env.IOP === 'true'so standard Chrome-hosted builds keep the default sharing behavior. - In
patchFederationForIop.js, it would be safer to guard against missing or unexpected shapes ofincludes.chromeProvidedbefore deleting keys (e.g., check thatincludes.chromeProvidedexists and is an object) to avoid hard-to-debug runtime errors if the upstream utilities package changes.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The global `require('./config/patchFederationForIop');` in `fec.config.js` will modify the federated-modules behaviour for all environments; consider gating this patch behind `process.env.IOP === 'true'` so standard Chrome-hosted builds keep the default sharing behavior.
- In `patchFederationForIop.js`, it would be safer to guard against missing or unexpected shapes of `includes.chromeProvided` before deleting keys (e.g., check that `includes.chromeProvided` exists and is an object) to avoid hard-to-debug runtime errors if the upstream utilities package changes.
## Individual Comments
### Comment 1
<location path="fec.config.js" line_range="4" />
<code_context>
const { resolve } = require('path');
const { sentryWebpackPlugin } = require('@sentry/webpack-plugin');
+require('./config/patchFederationForIop');
+
module.exports = {
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Conditionally apply the federation patch only in IOP environments.
Since `patchFederationForIop` is intended only for Foreman/IOP hosts, consider guarding the `require('./config/patchFederationForIop')` with `if (process.env.IOP === 'true')` so that non-IOP environments keep their existing federation behavior.
```suggestion
if (process.env.IOP === 'true') {
require('./config/patchFederationForIop');
}
```
</issue_to_address>
### Comment 2
<location path="config/patchFederationForIop.js" line_range="17-21" />
<code_context>
+const federatedModulesUtil = require(federatedModulesPath);
+const originalCreateIncludes = federatedModulesUtil.createIncludes;
+
+federatedModulesUtil.createIncludes = () => {
+ const includes = originalCreateIncludes();
+ delete includes.chromeProvided['react/jsx-runtime'];
+ delete includes.chromeProvided['react-intl'];
+ return includes;
+};
</code_context>
<issue_to_address>
**issue (bug_risk):** Preserve the original `createIncludes` signature and guard against missing `chromeProvided`.
Two robustness points:
1. This override drops any arguments to `createIncludes`, so future upstream parameters would be ignored. Forward them instead:
```js
federatedModulesUtil.createIncludes = (...args) => {
const includes = originalCreateIncludes(...args);
if (includes.chromeProvided) {
delete includes.chromeProvided['react/jsx-runtime'];
delete includes.chromeProvided['react-intl'];
}
return includes;
};
```
2. Guard `includes.chromeProvided` to avoid a possible `TypeError` if that field is ever missing or the structure changes.
</issue_to_address>
### Comment 3
<location path="package-lock.json" line_range="7675-7683" />
<code_context>
</code_context>
<issue_to_address>
**security (license/@sentry/cli):** FSL-1.1-MIT: Open-source license could not be identified
The obligations of the `FSL-1.1-MIT` license for this code could not be determined automatically. Unknown licenses may carry obligations or restrictions and should be reviewed manually to ensure compliance
*Source: trivy*
</issue_to_address>
### Comment 4
<location path="package-lock.json" line_range="7705-7712" />
<code_context>
</code_context>
<issue_to_address>
**security (license/@sentry/cli-darwin):** FSL-1.1-MIT: Open-source license could not be identified
The obligations of the `FSL-1.1-MIT` license for this code could not be determined automatically. Unknown licenses may carry obligations or restrictions and should be reviewed manually to ensure compliance
*Source: trivy*
</issue_to_address>
### Comment 5
<location path="package-lock.json" line_range="7718-7735" />
<code_context>
</code_context>
<issue_to_address>
**security (license/@sentry/cli-linux-arm):** FSL-1.1-MIT: Open-source license could not be identified
The obligations of the `FSL-1.1-MIT` license for this code could not be determined automatically. Unknown licenses may carry obligations or restrictions and should be reviewed manually to ensure compliance
*Source: trivy*
</issue_to_address>
### Comment 6
<location path="package-lock.json" line_range="7736-7753" />
<code_context>
</code_context>
<issue_to_address>
**security (license/@sentry/cli-linux-arm64):** FSL-1.1-MIT: Open-source license could not be identified
The obligations of the `FSL-1.1-MIT` license for this code could not be determined automatically. Unknown licenses may carry obligations or restrictions and should be reviewed manually to ensure compliance
*Source: trivy*
</issue_to_address>
### Comment 7
<location path="package-lock.json" line_range="7754-7772" />
<code_context>
</code_context>
<issue_to_address>
**security (license/@sentry/cli-linux-i686):** FSL-1.1-MIT: Open-source license could not be identified
The obligations of the `FSL-1.1-MIT` license for this code could not be determined automatically. Unknown licenses may carry obligations or restrictions and should be reviewed manually to ensure compliance
*Source: trivy*
</issue_to_address>
### Comment 8
<location path="package-lock.json" line_range="7773-7790" />
<code_context>
</code_context>
<issue_to_address>
**security (license/@sentry/cli-linux-x64):** FSL-1.1-MIT: Open-source license could not be identified
The obligations of the `FSL-1.1-MIT` license for this code could not be determined automatically. Unknown licenses may carry obligations or restrictions and should be reviewed manually to ensure compliance
*Source: trivy*
</issue_to_address>
### Comment 9
<location path="package-lock.json" line_range="7791-7806" />
<code_context>
</code_context>
<issue_to_address>
**security (license/@sentry/cli-win32-arm64):** FSL-1.1-MIT: Open-source license could not be identified
The obligations of the `FSL-1.1-MIT` license for this code could not be determined automatically. Unknown licenses may carry obligations or restrictions and should be reviewed manually to ensure compliance
*Source: trivy*
</issue_to_address>
### Comment 10
<location path="package-lock.json" line_range="7807-7818" />
<code_context>
</code_context>
<issue_to_address>
**security (license/@sentry/cli-win32-i686):** FSL-1.1-MIT: Open-source license could not be identified
The obligations of the `FSL-1.1-MIT` license for this code could not be determined automatically. Unknown licenses may carry obligations or restrictions and should be reviewed manually to ensure compliance
*Source: trivy*
</issue_to_address>
### Comment 11
<location path="package-lock.json" line_range="7824-7834" />
<code_context>
</code_context>
<issue_to_address>
**security (license/@sentry/cli-win32-x64):** FSL-1.1-MIT: Open-source license could not be identified
The obligations of the `FSL-1.1-MIT` license for this code could not be determined automatically. Unknown licenses may carry obligations or restrictions and should be reviewed manually to ensure compliance
*Source: trivy*
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+17
to
+21
| federatedModulesUtil.createIncludes = () => { | ||
| const includes = originalCreateIncludes(); | ||
| delete includes.chromeProvided['react/jsx-runtime']; | ||
| delete includes.chromeProvided['react-intl']; | ||
| return includes; |
There was a problem hiding this comment.
issue (bug_risk): Preserve the original createIncludes signature and guard against missing chromeProvided.
Two robustness points:
- This override drops any arguments to
createIncludes, so future upstream parameters would be ignored. Forward them instead:
federatedModulesUtil.createIncludes = (...args) => {
const includes = originalCreateIncludes(...args);
if (includes.chromeProvided) {
delete includes.chromeProvided['react/jsx-runtime'];
delete includes.chromeProvided['react-intl'];
}
return includes;
};- Guard
includes.chromeProvidedto avoid a possibleTypeErrorif that field is ever missing or the structure changes.
Contributor
|
After updating frontend development proxy and adding symlink it worked :) https://github.com/RedHatInsights/frontend-development-proxy#iop-mode |
Contributor
Author
|
/retest |
adonispuente
merged commit Jul 28, 2026
54cfc6c
into
RedHatInsights:foreman-3.18
4 of 5 checks passed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
To test this PR, all you need to do:
Theres currently an issue where when trying to kill the applicaiton, it doesnt always kill webpack instances. If it hangs up, try
ps aux | grep webpack | grep -v grep
and then kill -9 whatever port is running
Theres a currently a PR in FEC to fix this: RedHatInsights/frontend-components#2397
Summary by Sourcery
Add support for running the Advisor frontend in IOP mode with custom routes and compatibility tweaks for Foreman/IOP-hosted module federation.
New Features:
Enhancements:
Build: