feat(RHINENG-26171): add iop mode - #2102
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 guide (collapsed on small PRs)Reviewer's GuideAdds an IOP development mode for Advisor, including a new IOP proxy start script, configuration updates, and documentation for running via the frontend-development-proxy, while updating frontend-components-config and wiring in custom IOP routes. 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 found 9 security issues, 1 other issue, 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)
Fixed security issues:
- glob (link)
General comments:
- The new
start:proxy:iopscript relies on shell-specific syntax like$(pwd)and${IOP_URL}, which may not work on non-POSIX environments (e.g., Windows); consider using a cross-platform approach (e.g.,cross-envor a small Node script) to construct these values. - Since
start:proxy:iopdepends onIOP_URLbeing set, you might want to add a simple guard (e.g., in a wrapper script) that fails fast with a clear error message whenIOP_URLis missing, to avoid confusing runtime failures.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new `start:proxy:iop` script relies on shell-specific syntax like `$(pwd)` and `${IOP_URL}`, which may not work on non-POSIX environments (e.g., Windows); consider using a cross-platform approach (e.g., `cross-env` or a small Node script) to construct these values.
- Since `start:proxy:iop` depends on `IOP_URL` being set, you might want to add a simple guard (e.g., in a wrapper script) that fails fast with a clear error message when `IOP_URL` is missing, to avoid confusing runtime failures.
## Individual Comments
### Comment 1
<location path="package.json" line_range="22" />
<code_context>
"server:ctr": "node src/server/generateServerKey.js",
"start": "fec dev",
"start:proxy": "PROXY=true fec dev",
+ "start:proxy:iop": "PROXY=true IOP=true HCC_ENV=iop HCC_ENV_URL=${IOP_URL} FEC_IOP_CUSTOM_ROUTES_PATH=$(pwd)/custom_routes.json fec dev-proxy --iop",
"static": "fec static",
"test": "jest --passWithNoTests",
</code_context>
<issue_to_address>
**suggestion:** Consider making the `start:proxy:iop` script more robust to shell differences and paths with spaces.
This command relies on POSIX syntax (`$(pwd)`, `${IOP_URL}`) and leaves `$(pwd)/custom_routes.json` unquoted, which can fail on Windows and in paths containing spaces. To make this more robust, either quote the path (e.g. `"FEC_IOP_CUSTOM_ROUTES_PATH=\"$(pwd)/custom_routes.json\""`) or move the path/ENV construction into a small Node script using `process.cwd()` so it’s platform‑agnostic.
Suggested implementation:
```
"start:proxy": "PROXY=true fec dev",
"start:proxy:iop": "node scripts/startProxyIop.js",
"static": "fec static",
```
Create a new file `scripts/startProxyIop.js` (or adjust the path/name to match your repo conventions) with contents similar to:
```js
// scripts/startProxyIop.js
const { spawn } = require('child_process');
const path = require('path');
const env = {
...process.env,
PROXY: 'true',
IOP: 'true',
HCC_ENV: 'iop',
HCC_ENV_URL: process.env.IOP_URL,
FEC_IOP_CUSTOM_ROUTES_PATH: path.join(process.cwd(), 'custom_routes.json'),
};
const child = spawn('fec', ['dev-proxy', '--iop'], {
stdio: 'inherit',
env,
shell: process.platform === 'win32', // helps on Windows
});
child.on('exit', (code) => {
process.exit(code ?? 0);
});
child.on('error', (err) => {
console.error('Failed to start dev proxy:', err);
process.exit(1);
});
```
This moves all POSIX-specific syntax into Node, uses `process.cwd()` for the path, and sets environment variables in a cross-platform way. If your project uses ES modules, adjust to `import` syntax and `type: "module"` accordingly.
</issue_to_address>
### Comment 2
<location path="package-lock.json" line_range="7323-7331" />
<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 3
<location path="package-lock.json" line_range="7353-7360" />
<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 4
<location path="package-lock.json" line_range="7366-7383" />
<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 5
<location path="package-lock.json" line_range="7384-7401" />
<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 6
<location path="package-lock.json" line_range="7402-7420" />
<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 7
<location path="package-lock.json" line_range="7421-7438" />
<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 8
<location path="package-lock.json" line_range="7439-7454" />
<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 9
<location path="package-lock.json" line_range="7455-7466" />
<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 10
<location path="package-lock.json" line_range="7472-7482" />
<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.
Contributor
Author
|
/retest |
adonispuente
merged commit Jul 28, 2026
121e983
into
RedHatInsights:foreman-3.16
1 of 4 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:
Run npm i
export IOP_URL={current IOP instance, DM if you need a url} npm run start:proxy:iop
verify everything works as expected after navigating to iop.foo.redhat.com and logging in
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 and documentation for running Advisor in IOP proxy mode.
New Features:
Enhancements:
Build:
Documentation: