OBSINTA-1002: consolidate COO 1.4.0 test case documentation#793
Conversation
openshift-ci
bot
added
the
do-not-merge/work-in-progress
|
Skipping CI for Draft Pull Request. |
openshift-ci
bot
removed
the
do-not-merge/work-in-progress
WalkthroughAdds three Kubernetes/OpenShift manifests for HTPasswd auth and a limited-permissions test user, and updates incident detection test docs with added CSV prerequisites, new UI test cases (trimming, multi-severity boundaries, padding, data-delay), large-alerts API behavior, and permission-denied handling scenarios. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
docs/incident_detection/tests/3.api_calls_data_loading_flows.md (1)
110-112: Track this TODO with an owner or issue link.
TODO COO 1.4is easy to lose during release prep; adding an explicit tracking reference will keep this actionable.If useful, I can draft a ready-to-paste checklist item format with owner/date/issue fields.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/incident_detection/tests/3.api_calls_data_loading_flows.md` around lines 110 - 112, Replace the ambiguous "TODO COO 1.4" under the "### 3.5 Data Integrity" section with a tracked item that includes an explicit owner and issue reference: change the checklist line "- [ ] Incident grouping by `group_id` works correctly" to include fields like "Owner: <github-username>", "Issue: <link/number>", and "Due: <date>" (or a direct issue link) so the task is traceable; ensure the `TODO COO 1.4` token is removed or replaced with the issue/owner metadata to prevent it being lost during release prep.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/incident_detection/resources/htpasswd-secret.yaml`:
- Around line 4-19: Remove the checked-in default credentials by deleting the
base64 value under the Secret named "htpass-secret" (metadata.name) and
replacing the static data key "htpasswd" with a templated placeholder or
removing the data section entirely so each environment must generate the secret;
update the manifest to either use stringData with a placeholder (e.g.,
<REPLACE_WITH_BASE64_OR_GENERATE>) or include comments directing operators to
create the secret per-environment (e.g., generate htpasswd and kubectl create
secret) rather than committing the hashed credential in the file, ensuring the
Secret still targets the same namespace "openshift-config".
In `@docs/incident_detection/resources/oauth-htpasswd.yaml`:
- Around line 10-17: Add a prominent warning at the top of the OAuth manifest
stating that applying this spec.identityProviders manifest will replace all
existing OAuth identity providers and can lock out clusters with existing
authentication; update the docs/incident_detection/resources/oauth-htpasswd.yaml
file to include the warning before the spec (e.g., reference the affected block
like the spec.identityProviders entry and the htpasswd provider named
my_htpasswd_provider with htpasswd.fileData.name set to htpass-secret) so
readers clearly know not to apply this to production clusters.
In `@docs/incident_detection/tests/2.ui_display_flows.md`:
- Around line 150-168: Update the two test checklist items that currently
reference AlertC to instead use the multi-severity Incident D: change the
reference under "End Times Should Be 5-Minute Rounded" and the reference under
"Start Times Match Alert Tooltip" so they instruct testers to use Incident D
(multi-severity) rather than AlertC, and add a brief note in those items that
Incident D is the multi-severity test case to ensure the boundary-time checks
are actionable.
---
Nitpick comments:
In `@docs/incident_detection/tests/3.api_calls_data_loading_flows.md`:
- Around line 110-112: Replace the ambiguous "TODO COO 1.4" under the "### 3.5
Data Integrity" section with a tracked item that includes an explicit owner and
issue reference: change the checklist line "- [ ] Incident grouping by
`group_id` works correctly" to include fields like "Owner: <github-username>",
"Issue: <link/number>", and "Due: <date>" (or a direct issue link) so the task
is traceable; ensure the `TODO COO 1.4` token is removed or replaced with the
issue/owner metadata to prevent it being lost during release prep.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
⛔ Files ignored due to path filters (7)
docs/incident_detection/simulate_scenarios/100-alerts-14-days.csvis excluded by!**/*.csvdocs/incident_detection/simulate_scenarios/1000-alerts-15-min.csvis excluded by!**/*.csvdocs/incident_detection/simulate_scenarios/complete-test-data.csvis excluded by!**/*.csvdocs/incident_detection/simulate_scenarios/data-loading-silences.csvis excluded by!**/*.csvdocs/incident_detection/simulate_scenarios/filtering-test-data.csvis excluded by!**/*.csvdocs/incident_detection/simulate_scenarios/long-incident-15-days.csvis excluded by!**/*.csvdocs/incident_detection/simulate_scenarios/mixed-severity-escalation.csvis excluded by!**/*.csv
📒 Files selected for processing (7)
docs/incident_detection/resources/htpasswd-secret.yamldocs/incident_detection/resources/limited-permissions-user.yamldocs/incident_detection/resources/oauth-htpasswd.yamldocs/incident_detection/tests/0.overview.mddocs/incident_detection/tests/1.filtering_flows.mddocs/incident_detection/tests/2.ui_display_flows.mddocs/incident_detection/tests/3.api_calls_data_loading_flows.md
| # htpasswd -c -B -b users.htpasswd testuser password123 | ||
| # | ||
| # Then base64 encode it: | ||
| # base64 -w0 users.htpasswd | ||
| # | ||
| # Replace the data.htpasswd value below with your encoded content | ||
|
|
||
| apiVersion: v1 | ||
| kind: Secret | ||
| metadata: | ||
| name: htpass-secret | ||
| namespace: openshift-config | ||
| type: Opaque | ||
| data: | ||
| # base64 encoded: testuser:$2y$05$fBn5ChTgiV0A/6HEfoNKleU3CLVIWuV2816XVIsmmhwAz.fBpDObe | ||
| htpasswd: dGVzdHVzZXI6JDJ5JDA1JGZCbjVDaFRnaVYwQS82SEVmb05LbGVVM0NMVklXdVYyODE2WFZJc21taHdBei5mQnBET2JlCg== |
There was a problem hiding this comment.
Remove checked-in default credentials from the manifest.
Line 4 and Line 19 effectively publish reusable credentials. Please keep this file templated and require per-environment secret generation.
Proposed change
-# htpasswd -c -B -b users.htpasswd testuser password123
+# htpasswd -c -B -b users.htpasswd <username> <strong-random-password>
@@
- # base64 encoded: testuser:$2y$05$fBn5ChTgiV0A/6HEfoNKleU3CLVIWuV2816XVIsmmhwAz.fBpDObe
- htpasswd: dGVzdHVzZXI6JDJ5JDA1JGZCbjVDaFRnaVYwQS82SEVmb05LbGVVM0NMVklXdVYyODE2WFZJc21taHdBei5mQnBET2JlCg==
+ # base64 encoded content of users.htpasswd
+ htpasswd: <REPLACE_WITH_BASE64_HTPASSWD_CONTENT>📝 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.
| # htpasswd -c -B -b users.htpasswd testuser password123 | |
| # | |
| # Then base64 encode it: | |
| # base64 -w0 users.htpasswd | |
| # | |
| # Replace the data.htpasswd value below with your encoded content | |
| apiVersion: v1 | |
| kind: Secret | |
| metadata: | |
| name: htpass-secret | |
| namespace: openshift-config | |
| type: Opaque | |
| data: | |
| # base64 encoded: testuser:$2y$05$fBn5ChTgiV0A/6HEfoNKleU3CLVIWuV2816XVIsmmhwAz.fBpDObe | |
| htpasswd: dGVzdHVzZXI6JDJ5JDA1JGZCbjVDaFRnaVYwQS82SEVmb05LbGVVM0NMVklXdVYyODE2WFZJc21taHdBei5mQnBET2JlCg== | |
| # htpasswd -c -B -b users.htpasswd <username> <strong-random-password> | |
| # | |
| # Then base64 encode it: | |
| # base64 -w0 users.htpasswd | |
| # | |
| # Replace the data.htpasswd value below with your encoded content | |
| apiVersion: v1 | |
| kind: Secret | |
| metadata: | |
| name: htpass-secret | |
| namespace: openshift-config | |
| type: Opaque | |
| data: | |
| # base64 encoded content of users.htpasswd | |
| htpasswd: <REPLACE_WITH_BASE64_HTPASSWD_CONTENT> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/incident_detection/resources/htpasswd-secret.yaml` around lines 4 - 19,
Remove the checked-in default credentials by deleting the base64 value under the
Secret named "htpass-secret" (metadata.name) and replacing the static data key
"htpasswd" with a templated placeholder or removing the data section entirely so
each environment must generate the secret; update the manifest to either use
stringData with a placeholder (e.g., <REPLACE_WITH_BASE64_OR_GENERATE>) or
include comments directing operators to create the secret per-environment (e.g.,
generate htpasswd and kubectl create secret) rather than committing the hashed
credential in the file, ensuring the Secret still targets the same namespace
"openshift-config".
There was a problem hiding this comment.
Those are dummy invalid values that serve as example for local testing.
There was a problem hiding this comment.
@DavidRajnoha, understood — since these are intentionally invalid placeholder values in a documentation-only test resource, the concern doesn't apply here. Thanks for the clarification!
(´• ω •`) 🐇
📝 Learnings were identified and not saved (knowledge base disabled). Enable
jgbernalp
changed the title
openshift-ci-robot
added
the
jira/valid-reference
|
@DavidRajnoha: This pull request references OBSINTA-1002 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.22.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
Add test cases for OU-632, OU-1039, OU-1123, OU-1205, OU-1213, and OU-1221. Include simulation scenario CSVs and RBAC resources referenced by the test documentation. Add CSV file references to prerequisites sections across all test docs. Co-Authored-By: Claude Opus 4.6 <[email protected]>
DavidRajnoha
force-pushed
the
docs/coo-1.4-test-cases
branch
from
314beb9 to
a837775
Compare
|
@DavidRajnoha: This pull request references OBSINTA-1002 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.22.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
docs/incident_detection/resources/htpasswd-secret.yaml (1)
18-19:⚠️ Potential issue | 🟠 MajorReplace concrete secret payload with a non-reusable placeholder.
Keeping a real-looking hash/base64 value in a committed Secret manifest makes accidental insecure apply more likely. Prefer an explicit placeholder so each environment must generate its own value.
Suggested doc fix
- # base64 encoded: testuser:$2y$05$fBn5ChTgiV0A/6HEfoNKleU3CLVIWuV2816XVIsmmhwAz.fBpDObe - htpasswd: dGVzdHVzZXI6JDJ5JDA1JGZCbjVDaFRnaVYwQS82SEVmb05LbGVVM0NMVklXdVYyODE2WFZJc21taHdBei5mQnBET2JlCg== + # base64 encoded content of users.htpasswd (generate per environment) + htpasswd: <REPLACE_WITH_BASE64_HTPASSWD_CONTENT>As per coding guidelines, "Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/incident_detection/resources/htpasswd-secret.yaml` around lines 18 - 19, The htpasswd secret contains a concrete base64-encoded payload and comment (the htpasswd key and its commented plaintext) that must be replaced with a non-reusable placeholder; update the htpasswd entry to a clear placeholder string (e.g., "<BASE64_HTPASSWD_PAYLOAD>") and replace the example comment with a short instruction to generate a new value per environment (e.g., "Generate with: htpasswd -nB <user> | base64 -w0") so consumers must create their own secret rather than using the committed value.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/incident_detection/resources/htpasswd-secret.yaml`:
- Line 4: The example command in the comment that uses "htpasswd -c -B -b
users.htpasswd testuser password123" exposes the password on the command line;
update the documentation comment to show a secure alternative such as using the
interactive prompt form "htpasswd -c -B users.htpasswd testuser" (omit -b and
the plain password) or describe piping a password from stdin/secure vault tools,
and mention that -b should not be used to pass plaintext passwords in the
htpasswd example.
---
Duplicate comments:
In `@docs/incident_detection/resources/htpasswd-secret.yaml`:
- Around line 18-19: The htpasswd secret contains a concrete base64-encoded
payload and comment (the htpasswd key and its commented plaintext) that must be
replaced with a non-reusable placeholder; update the htpasswd entry to a clear
placeholder string (e.g., "<BASE64_HTPASSWD_PAYLOAD>") and replace the example
comment with a short instruction to generate a new value per environment (e.g.,
"Generate with: htpasswd -nB <user> | base64 -w0") so consumers must create
their own secret rather than using the committed value.
🪄 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: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 56a167d4-7bef-4cd4-96c4-958b2613a883
⛔ Files ignored due to path filters (7)
docs/incident_detection/simulate_scenarios/100-alerts-14-days.csvis excluded by!**/*.csvdocs/incident_detection/simulate_scenarios/1000-alerts-15-min.csvis excluded by!**/*.csvdocs/incident_detection/simulate_scenarios/complete-test-data.csvis excluded by!**/*.csvdocs/incident_detection/simulate_scenarios/data-loading-silences.csvis excluded by!**/*.csvdocs/incident_detection/simulate_scenarios/filtering-test-data.csvis excluded by!**/*.csvdocs/incident_detection/simulate_scenarios/long-incident-15-days.csvis excluded by!**/*.csvdocs/incident_detection/simulate_scenarios/mixed-severity-escalation.csvis excluded by!**/*.csv
📒 Files selected for processing (7)
docs/incident_detection/resources/htpasswd-secret.yamldocs/incident_detection/resources/limited-permissions-user.yamldocs/incident_detection/resources/oauth-htpasswd.yamldocs/incident_detection/tests/0.overview.mddocs/incident_detection/tests/1.filtering_flows.mddocs/incident_detection/tests/2.ui_display_flows.mddocs/incident_detection/tests/3.api_calls_data_loading_flows.md
✅ Files skipped from review due to trivial changes (4)
- docs/incident_detection/tests/0.overview.md
- docs/incident_detection/tests/1.filtering_flows.md
- docs/incident_detection/resources/limited-permissions-user.yaml
- docs/incident_detection/resources/oauth-htpasswd.yaml
🚧 Files skipped from review as they are similar to previous changes (2)
- docs/incident_detection/tests/3.api_calls_data_loading_flows.md
- docs/incident_detection/tests/2.ui_display_flows.md
| # HTPasswd Secret for creating a test user | ||
| # | ||
| # Before applying, generate the htpasswd file: | ||
| # htpasswd -c -B -b users.htpasswd testuser password123 |
There was a problem hiding this comment.
Avoid passing passwords on the command line.
Using -b ... password123 exposes credentials via shell history and process args. Use interactive prompt mode instead.
Suggested doc fix
-# htpasswd -c -B -b users.htpasswd testuser password123
+# htpasswd -c -B users.htpasswd testuser
+# # enter password interactively when promptedAs per coding guidelines, "Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/incident_detection/resources/htpasswd-secret.yaml` at line 4, The
example command in the comment that uses "htpasswd -c -B -b users.htpasswd
testuser password123" exposes the password on the command line; update the
documentation comment to show a secure alternative such as using the interactive
prompt form "htpasswd -c -B users.htpasswd testuser" (omit -b and the plain
password) or describe piping a password from stdin/secure vault tools, and
mention that -b should not be used to pass plaintext passwords in the htpasswd
example.
|
/lgtm |
|
/label qe-approved |
openshift-ci
bot
added
the
qe-approved
|
@DavidRajnoha: This pull request references OBSINTA-1002 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.22.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: DavidRajnoha, etmurasaki The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
bot
added
the
approved
|
@DavidRajnoha: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
3 participants
Add test cases for OU-632, OU-1039, OU-1123, OU-1205, OU-1213, and OU-1221. Include simulation scenario CSVs and RBAC resources referenced by the test documentation. Add CSV file references to prerequisites sections across all test docs.