MTA-7133-LLM-Proxy-Disconnection-Network-Isolation-Procedures #381
Open
anarnold97 wants to merge 2 commits into
Open
MTA-7133-LLM-Proxy-Disconnection-Network-Isolation-Procedures #381anarnold97 wants to merge 2 commits into
anarnold97 wants to merge 2 commits into
Conversation
…Draft) Signed-off-by: A.Arnold <anarnold@redhat.com>
Contributor
📝 WalkthroughWalkthroughThis PR adds a new AsciiDoc procedure documenting emergency shutdown of the LLM proxy to isolate developer environments during security incidents, covering centralized (Tackle CR toggle) and standalone (firewall/network policy) modes, with verification and re-enable steps. It is wired into the master developer guide via an include directive. ChangesEmergency LLM Proxy Shutdown Documentation
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
anarnold97
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/topics/developer-lightspeed/proc_emergency-llm-proxy-shutdown.adoc`:
- Around line 175-182: Correct the provider name typo in the provider list so it
reads “Azure OpenAI Service” instead of “zure OpenAI Service.” Update the
relevant entry in the developer-lightspeed emergency LLM proxy shutdown document
where the OpenAI/Azure OpenAI providers are listed, keeping the rest of the
table content unchanged.
- Around line 93-104: The verification step in
proc_emergency-llm-proxy-shutdown.adoc is misleading because `oc get pods ... |
grep proxy` will not produce the stated “No resources found...” output; update
the expected result to reflect the actual shell behavior for this command, using
the existing “Verify that no proxy pods are running” step and its “Expected
output” block as the place to adjust the wording.
- Around line 243-264: The curl validation is targeting the public
`api.openai.com` endpoint instead of the actual isolation/proxy path, so it does
not validate centralized deployments correctly. Update the `curl` example in the
emergency shutdown docs to use the proxy or workstation-specific endpoint used
by the isolation flow, and make the expected output reflect that path for both
isolated and centralized modes. Keep the guidance aligned with the surrounding
`curl` validation section so readers verify the real traffic route rather than a
single provider host.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 23625628-72fe-4a48-8b6d-a0bc320e3992
📒 Files selected for processing (2)
docs/developer-lightspeed-guide/master.adocdocs/topics/developer-lightspeed/proc_emergency-llm-proxy-shutdown.adoc
Comment on lines
+93
to
+104
| . Verify that no proxy pods are running: | ||
| + | ||
| [source,terminal,subs="+quotes"] | ||
| ---- | ||
| $ *oc get pods -n __<mta_namespace>__ | grep proxy* | ||
| ---- | ||
| + | ||
| .Expected output | ||
| [source,terminal] | ||
| ---- | ||
| No resources found in mta namespace. | ||
| ---- |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Fix the verification output.
oc get pods ... | grep proxy will not print No resources found in mta namespace.; it either emits matching lines or nothing at all. As written, this verification step is misleading.
Proposed fix
- $ *oc get pods -n __<mta_namespace>__ | grep proxy*
+ $ *oc get pods -n __<mta_namespace>__ --no-headers | grep proxy*Then update the expected result to match the actual shell behavior.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/topics/developer-lightspeed/proc_emergency-llm-proxy-shutdown.adoc`
around lines 93 - 104, The verification step in
proc_emergency-llm-proxy-shutdown.adoc is misleading because `oc get pods ... |
grep proxy` will not produce the stated “No resources found...” output; update
the expected result to reflect the actual shell behavior for this command, using
the existing “Verify that no proxy pods are running” step and its “Expected
output” block as the place to adjust the wording.
Comment on lines
+175
to
+182
| |OpenAI | ||
| |`api.openai.com` | ||
| |443 | ||
|
|
||
| zure OpenAI Service | ||
| |`*.openai.azure.com` | ||
| |443 | ||
|
|
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Correct the provider name typo.
zure OpenAI Service should be Azure OpenAI Service.
Proposed fix
-|zure OpenAI Service
+|Azure OpenAI Service📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| |OpenAI | |
| |`api.openai.com` | |
| |443 | |
| zure OpenAI Service | |
| |`*.openai.azure.com` | |
| |443 | |
| |OpenAI | |
| |`api.openai.com` | |
| |443 | |
| Azure OpenAI Service | |
| |`*.openai.azure.com` | |
| |443 | |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/topics/developer-lightspeed/proc_emergency-llm-proxy-shutdown.adoc`
around lines 175 - 182, Correct the provider name typo in the provider list so
it reads “Azure OpenAI Service” instead of “zure OpenAI Service.” Update the
relevant entry in the developer-lightspeed emergency LLM proxy shutdown document
where the OpenAI/Azure OpenAI providers are listed, keeping the rest of the
table content unchanged.
Comment on lines
+243
to
+264
| .Optional: Network-level validation by using the `curl` command | ||
|
|
||
| * On the affected workstation, run the following command to confirm that the endpoint is unreachable: | ||
| + | ||
| [source,terminal,subs="+quotes"] | ||
| ---- | ||
| $ *curl -v --max-time 10 https://api.openai.com/v1/models* | ||
| ---- | ||
| + | ||
| .Expected output when isolation is active | ||
| [source,terminal,subs="+quotes"] | ||
| ---- | ||
| * connect to __<ip>__ port 443 failed: Connection refused | ||
| curl: (7) Failed to connect to api.openai.com port 443 after __<X>__ ms: Connection refused | ||
| ---- | ||
| + | ||
| Alternatively, in centralized mode where DNS resolves but the service is down, the expected output is as follows: | ||
| + | ||
| [source,terminal] | ||
| ---- | ||
| curl: (22) The requested URL returned error: 503 | ||
| ---- |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Scope the curl validation to the actual isolation path.
This check hardcodes api.openai.com, so it only validates one provider and does not verify centralized mode at all. In centralized deployments, the workstation should be reaching the proxy path, not the public LLM endpoint.
Proposed fix
-Optional: Network-level validation by using the `curl` command
+Optional: Network-level validation for standalone firewall blocks
...
-$ *curl -v --max-time 10 https://api.openai.com/v1/models*
+$ *curl -v --max-time 10 https://<configured_llm_endpoint>/*📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| .Optional: Network-level validation by using the `curl` command | |
| * On the affected workstation, run the following command to confirm that the endpoint is unreachable: | |
| + | |
| [source,terminal,subs="+quotes"] | |
| ---- | |
| $ *curl -v --max-time 10 https://api.openai.com/v1/models* | |
| ---- | |
| + | |
| .Expected output when isolation is active | |
| [source,terminal,subs="+quotes"] | |
| ---- | |
| * connect to __<ip>__ port 443 failed: Connection refused | |
| curl: (7) Failed to connect to api.openai.com port 443 after __<X>__ ms: Connection refused | |
| ---- | |
| + | |
| Alternatively, in centralized mode where DNS resolves but the service is down, the expected output is as follows: | |
| + | |
| [source,terminal] | |
| ---- | |
| curl: (22) The requested URL returned error: 503 | |
| ---- | |
| .Optional: Network-level validation for standalone firewall blocks | |
| * On the affected workstation, run the following command to confirm that the endpoint is unreachable: | |
| [source,terminal,subs="+quotes"] | |
| ---- | |
| $ *curl -v --max-time 10 https://<configured_llm_endpoint>/* | |
| .Expected output when isolation is active | |
| [source,terminal,subs="+quotes"] | |
| ---- | |
| * connect to __<ip>__ port 443 failed: Connection refused | |
| curl: (7) Failed to connect to api.openai.com port 443 after __<X>__ ms: Connection refused | |
| Alternatively, in centralized mode where DNS resolves but the service is down, the expected output is as follows: | |
| [source,terminal] | |
| ---- | |
| curl: (22) The requested URL returned error: 503 | |
| ---- |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/topics/developer-lightspeed/proc_emergency-llm-proxy-shutdown.adoc`
around lines 243 - 264, The curl validation is targeting the public
`api.openai.com` endpoint instead of the actual isolation/proxy path, so it does
not validate centralized deployments correctly. Update the `curl` example in the
emergency shutdown docs to use the proxy or workstation-specific endpoint used
by the isolation flow, and make the expected output reflect that path for both
isolated and centralized modes. Keep the guidance aligned with the surrounding
`curl` validation section so readers verify the real traffic route rather than a
single provider host.
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
1 participant
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.
JIRA
PREVIEW
7. LLM proxy interaction monitoring
7.1. LLM proxy request and data handling
7.2. Accessing LLM proxy pod logs
7.3. Planned audit logging capabilities
DESCRIPTION
Adds emergency LLM proxy shutdown and network isolation procedures to the Developer Lightspeed guide.
New file:
proc_emergency-llm-proxy-shutdown.adocCentralized mode (Operator / OpenShift deployments)
kai_llm_proxy_enabled: falsein the Tackle CRoc patchCLI methodsStandalone mode (manual desktop setup)
Validation
curl-based network-level checkAssembly change
docs/developer-lightspeed-guide/master.adoc— new module included aftercon_developer-lightspeed-logs.adoc