-
Notifications
You must be signed in to change notification settings - Fork 159
ROX-29840: access ocp console #16436
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ROX-29840: access ocp console #16436
Conversation
Skipping CI for Draft Pull Request. |
Caution There are some errors in your PipelineRun template.
|
Images are ready for the commit at bc117fb. To use with deploy scripts, first |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## dv/ROX-29840-base-config-for-cypress-e2e-test-ocp #16436 +/- ##
=====================================================================================
- Coverage 49.04% 48.91% -0.14%
=====================================================================================
Files 2625 2616 -9
Lines 193919 193258 -661
=====================================================================================
- Hits 95114 94537 -577
+ Misses 91351 91290 -61
+ Partials 7454 7431 -23
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for adding this!
Overall LGTM, although I think a lot of what I added in my parent branch was testing code that we may not want to merge.
I can approve once this branch is targeting master
and the UI team can give input regarding the blockHosts
option.
// TODO: can we allow the ocp console but block other internet access? do we need to? | ||
// blockHosts: ['*.*'], // Browser options |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did find this https://issues.redhat.com/browse/ROX-1865 and this https://github.com/stackrox/rox/pull/2304 from when this option was first added.
My initial thought is that it is probably fine to remove this? It only blocks external requests in CI, so I don't see why it would be a problem if the UI doesn't rely on external requests.
@alwayshooin do you have any context on this somewhat ancient change or opinions on whether or not we can remove this?
@@ -28,8 +29,8 @@ fi | |||
artifacts_dir="${TEST_RESULTS_OUTPUT_DIR:-cypress/test-results}/ocp-artifacts" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change from my branch isn't quite right either - it looks like the video artifacts are not available in the job results. This is something I can fix separately though. I was just looking for video evidence of the test working beyond the text output.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I ran it locally with cypress (and it confirmed it was logging in) but I didn't know how/where to find the video artifacts on a prow run.
Co-authored-by: David Vail <[email protected]>
Can we merge this into your branch and then you can adjust it from there? |
If you'd rather do it in one PR with this new branch, I had tested it on a draft PR to master that you can add/change and use: #16415 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we merge this into your branch and then you can adjust it from there?
Sounds good to me, thank you!
f612602
into
dv/ROX-29840-base-config-for-cypress-e2e-test-ocp
Co-authored-by: David Vail <[email protected]>
Co-authored-by: David Vail <[email protected]>
The smoke test passes on OCP4.19 with these changes (https://gcsweb-ci.apps.ci.l2s4.p1.openshiftapps.com/gcs/test-platform-results/pr-logs/pull/stackrox_stackrox/16415/pull-ci-stackrox-stackrox-master-ocp-4-19-ui-e2e-tests/1957866898653712384/artifacts/ui-e2e-tests/stackrox-stackrox-e2e-test/build-log.txt):
The smoke test fails on ocp4.18 because of some text/intro changes (https://prow.ci.openshift.org/view/gs/test-platform-results/pr-logs/pull/stackrox_stackrox/16415/pull-ci-stackrox-stackrox-master-ocp-4-18-ui-e2e-tests/1957866898611769344):
Testing of this branch on PR: #16415
This uses the automation-flavors fix to prepare the dotenv file for use is here: https://github.com/stackrox/automation-flavors/pull/324