chore(config): update base configuration set up - #131
Conversation
0b1e79b to
ae518be
Compare
charlesmulder
left a comment
There was a problem hiding this comment.
Review Summary
Integration test validates implementation works end-to-end with frontend-components PR #2308. Core functionality solid, but need fixes before merge.
Issues to Address
1. Hardcoded IP not portable (package.json:7)
"IOP": "IOP=true HCC_ENV=iop HCC_ENV_URL=https://ip-10-0-167-196.rhos-01.prod.psi.rdu2.redhat.com ..."Internal IP won't work for all developers. Use env var with fallback:
"IOP": "IOP=true HCC_ENV=iop HCC_ENV_URL=${HCC_IOP_URL:-https://console.stage.redhat.com} ..."2. Missing newline at EOF (entrypoint.sh:87)
POSIX requires newline at end of file. Add newline after final line.
3. Security warning too weak (README.md:76-79)
Current text understates risk of tls_insecure_skip_verify. Replace with:
**⚠️ SECURITY: IOP MODE IS DEVELOPMENT ONLY**
In IOP mode specifically:
- The fallback proxy uses TLS transport with `tls_insecure_skip_verify` which **disables all certificate validation**
- **DO NOT use in production or with untrusted networks** - vulnerable to MITM attacks
- Only use on trusted local development networks
- Generated local route `reverse_proxy` blocks omit `header_up Host {http.reverse_proxy.upstream.hostport}`.4. Unnecessary flag (entrypoint.sh:87)
/usr/bin/caddy run --config "$CADDY_CONFIG_PATH" --adapter caddyfileCaddy auto-detects format from filename. Remove --adapter caddyfile.
What Works
✅ Non-breaking - default behavior unchanged
✅ Clear docs - README explains IOP mode
✅ Simple toggle - IOP=true explicit
✅ Docker Compose - easier local dev
✅ Route separation - IOP routes isolated
✅ Integration verified - works with frontend-components PR #2308
charlesmulder
left a comment
There was a problem hiding this comment.
Additional Issue: Unnecessary package.json
5. package.json in non-JavaScript project
Repository is Go/Caddy/Bash - no JavaScript code. package.json only used as task runner for docker compose commands:
"scripts": {
"dev-proxy": "docker compose up --build",
"dev-proxy:down": "docker compose down",
"IOP": "IOP=true HCC_ENV=iop ..."
}Problem: Adds npm dependency + package.json/package-lock.json just to alias shell commands. Confusing for Go project.
Better alternatives:
Option 1 - Makefile (standard for Go projects):
.PHONY: dev-proxy dev-proxy-down iop
dev-proxy:
docker compose up --build
dev-proxy-down:
docker compose down
iop:
IOP=true HCC_ENV=iop HCC_ENV_URL=${HCC_IOP_URL:-https://console.stage.redhat.com} docker compose up --buildOption 2 - Just document commands in README:
# Normal mode
docker compose up --build
# IOP mode
IOP=true HCC_ENV=iop HCC_ENV_URL=${HCC_IOP_URL:-https://console.stage.redhat.com} docker compose up --build
# Stop
docker compose downRecommend removing package.json/package-lock.json and using Makefile or direct commands.
charlesmulder
left a comment
There was a problem hiding this comment.
Security Concern: TLS Verification Skip
6. tls_insecure_skip_verify in fallback handler
Caddyfile.iop disables TLS certificate validation for all fallback traffic (requests not matching custom routes):
handle {
reverse_proxy ${HCC_ENV_URL} {
transport http {
tls_insecure_skip_verify # ← Disables all cert validation
}
}
}Risk:
- All non-custom-route traffic (auth, APIs, etc) to
https://ip-10-0-167-196.rhos-01.prod.psi.rdu2.redhat.comvulnerable to MITM - No certificate validation = attacker can intercept/modify traffic
- Applies to sensitive data (auth tokens, user data)
Question: Why is TLS skip needed for IOP environment?
Better alternatives:
If internal env has self-signed certs → Add CA to container:
# In Dockerfile
COPY internal-ca.crt /usr/local/share/ca-certificates/
RUN update-ca-certificatesRemove tls_insecure_skip_verify entirely.
If internal env doesn't need TLS → Use HTTP:
HCC_ENV_URL=http://ip-10-0-167-196.rhos-01.prod.psi.rdu2.redhat.comRemove transport http block.
If skip truly required → Document restriction:
- Add to README: IOP mode MUST only run on isolated internal networks
- Document threat model (assumes network-level isolation)
- Consider env var toggle:
IOP_SKIP_TLS_VERIFY(explicit opt-in)
Please clarify why TLS verification skip is necessary and consider alternatives.
charlesmulder
left a comment
There was a problem hiding this comment.
Recommended Solution for TLS Skip Issue
mkcert approach (already used in insights-chrome)
The insights-chrome project already solves this problem using mkcert for locally-trusted certificates.
Same approach works for proxy IOP mode:
1. User one-time setup:
# Install mkcert
# See: https://github.com/FiloSottile/mkcert#installation
# Install local CA
mkcert -install2. Add CA to proxy container:
# In Dockerfile - add before final stage
COPY mkcert-ca.crt /usr/local/share/ca-certificates/
RUN update-ca-certificates3. Remove tls_insecure_skip_verify from Caddyfile.iop:
handle {
reverse_proxy ${HCC_ENV_URL} {
header_up Accept-Encoding "gzip;q=0,deflate,sdch"
header_up -Origin
- transport http {
- tls_insecure_skip_verify
- }
}
}4. Document in README:
### IOP Mode TLS Setup
IOP environment may use self-signed certificates. To enable proper TLS verification:
1. Install mkcert (one-time): https://github.com/FiloSottile/mkcert#installation
2. Install local CA: \`mkcert -install\`
3. Copy CA to repo: \`cp ~/.local/share/mkcert/rootCA.pem ./mkcert-ca.crt\`
4. Rebuild image: \`podman build -t frontend-development-proxy .\`
Container will trust mkcert CA and validate IOP certificates properly.Benefits:
- Proper TLS validation (no security warnings)
- Consistent with insights-chrome approach
- Works with self-signed internal certs
- No MITM vulnerability
See insights-chrome README lines 98-127 for reference implementation.
3ab486f to
25e7cbf
Compare
|
@charlesmulder I believe ive addressed everything now, thank you so much for the comprehensive review. a bunch of good points |
|
@charlesmulderThe issue is that removing tls_insecure_skip_verify breaks IOP connection because the IOP instances we use like |
|
Is there any possibility of breaking PR test pipelines? They consume this image and use it for handling the proxying to stage environment. |
Rewrites Location headers in IOP mode to keep users on the local proxy URL. When IOP instances send redirects (e.g., for login), they include absolute Location headers pointing to the IOP instance. This change rewrites them to point to the local proxy, ensuring local assets continue loading. Example: Before: Location: https://ip-10-0-167-79.../users/login After: Location: https://iop.foo.redhat.com:1337/users/login Tested with IOP instance running Foreman 3.16.
|
@catastrophe-brandon im unsure how to verify/test that specifically. Though I would assume if the PR checks pass on the this PR itself that we should be good |
|
/retest |
3 similar comments
|
/retest |
|
/retest |
|
/retest |
- Remove hardcoded IP from package.json IOP script (use $HCC_IOP_URL variable) - Remove unnecessary --adapter flag from entrypoint.sh (auto-detected by Caddy) - Add newline at EOF in entrypoint.sh (POSIX requirement) - Add strong security warning for tls_insecure_skip_verify in IOP mode Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
a2b5c08 to
dc2e0dd
Compare
| RUN chmod +x /usr/local/bin/entrypoint.sh | ||
|
|
||
| # Run tests during build — fails the build (and CI) if tests break | ||
| # Tests use SCRIPT_DIR (parent of tests/) to locate entrypoint.sh, |
There was a problem hiding this comment.
This will fail every single time when running things locally, even on master branch.
| } | ||
|
|
||
| # --- Test 1: PROXY_LOGGING=false sets LOG_OUTPUT=discard --- | ||
| result=$(PROXY_LOGGING=false bash -c ' |
There was a problem hiding this comment.
- The old test tried to source and run entrypoint.sh in a subprocess which was fragile
- The new version just tests the logging logic directly (simpler and more reliable)
|
@vkrizan the new changes now only have 1 single caddy file gated by an iop flag |
In an effort to make onboarding easier for anyone trying to run things locally, I wanted to update the repo to include the custom routes and associated file to just simple be able to run
podman compose up proxy-iopSummary
Adds IOP (Insights on Premises) mode support for local development against Satellite/Foreman instances.
Key Changes
instance URLs)
Testing Locally
Build the proxy image:
podman build -t ghcr.io/redhatinsights/frontend-development-proxy:latest .
Test with vulnerability-ui:
cd vulnerability-ui
IOP_URL="https://ip-10-0-167-79.rhos-01.prod.psi.rdu2.redhat.com" npm run start:proxy:iop:local
Access at: https://iop.foo.redhat.com:1337/insights/vulnerability
Testing
Tested against IOP instance running Foreman 3.16. Verified:
Related PRs
-proxy: chore(config): update base configuration set up #131