Skip to content

EDM-4253: Add UI Vulnerability coverage #30

Open
amalykhi wants to merge 2 commits into
flightctl:mainfrom
amalykhi:EDM-4253
Open

EDM-4253: Add UI Vulnerability coverage #30
amalykhi wants to merge 2 commits into
flightctl:mainfrom
amalykhi:EDM-4253

Conversation

@amalykhi

Copy link
Copy Markdown

No description provided.

Add comprehensive E2E tests for vulnerability reporting across Device,
Fleet, and Overview pages in the FlightCtl UI.

New files:
- cypress/e2e/vulnerability.cy.js: Complete E2E test suite
- cypress/views/securityPage.js: Reusable page object with selectors

Updated:
- cypress/views/devicesPage.js: Added viewDeviceSecurity() helper
- cypress/views/fleetsPage.js: Added viewFleetSecurity() helper

Test coverage:
- Initial state verification (0 vulnerabilities)
- Fleet creation with vulnerable CentOS image
- Device attachment and CVE appearance
- CVE details panel, filtering, and search
- Verification across Device, Fleet, and Overview pages
- Device detachment and CVE cleanup

Test image: quay.io/centos-bootc/centos-bootc:stream9
Expected CVEs: 7 total (1 Critical, 1 High, 4 Medium, 1 Low)

Prerequisites:
- FlightCtl with flightctl_trustify_enabled: true
- Trustify with SBOM data for test image
- At least one enrolled device

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
*/
const waitForDeviceUpToDate = (deviceName, timeout = 300000) => {
cy.log(`Waiting for device ${deviceName} to reach UpToDate status...`)
cy.waitUntil(

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is waitUntil? where is it coming from ?

@talhil-rh talhil-rh left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review — Critical & Medium Issues

Critical

FLIGHTCTL_API env var undefined
Cypress.env('FLIGHTCTL_API') at line 23 of vulnerability.cy.js returns undefined because cypress.config.js declares no env defaults and run-e2e-test.sh never passes it to Cypress (no --env flag, no CYPRESS_ prefix). Every cy.request() hits undefinedapi/v1/devices. All API-driven tests fail.

Rollback assumption is wrong
"Detach Device and Verify CVEs Disappear" (line 315) assumes removing the fleet label causes the device to revert its image. FlightCtl doesn't work that way — a detached device stays unmanaged with the same image. The poll (imageDigest !== KNOWN_VULNERABLE_IMAGE.digest) will never become true and times out after 5 minutes.

Hardcoded image digest is a time bomb
The fleet spec uses the floating tag centos-bootc:stream9 but the test asserts a pinned digest. When the image is rebuilt (it will be), the digest check in waitForDeviceDigest will time out at 5 minutes. Either pin the image by digest in the fleet spec, or query the expected digest dynamically.

Medium

PATCH requests may need explicit Content-Type
cy.request('PATCH', ...) at lines 174 and 303 send JSON but don't set Content-Type. FlightCtl (Kubernetes-style API) may require application/merge-patch+json. If rejected, those tests fail silently or with a 415.

Hardcoded CVE IDs and counts will drift
KNOWN_VULNERABLE_IMAGE in securityPage.js pins 7 specific CVEs. Even with a pinned digest, Trustify's vulnerability DB evolves — new CVEs can be attributed, existing ones reclassified. Consider range checks or dynamic API queries.

8+ arbitrary cy.wait() calls
Hardcoded waits of 1s, 2s, and 15s throughout (lines 109, 121, 159, 200, 254, 289, 339). The 15s waits for "vulnerability sync" are fragile — too short and tests flake, too long and tests waste time. Replace with element-based assertions or API polling.

closeCveDetails() uses should('not.exist') — likely wrong
PatternFly v6 drawers (.pf-v6-c-drawer__panel) stay in the DOM when closed, hidden via CSS/aria-hidden. should('not.exist') will probably fail. Should be should('not.be.visible').

Double fleet deletion is redundant
Both after() (line 42) and describe('Cleanup') (line 365) delete the fleet. Won't actually cause a failure (the after() hook swallows the 404), but it's confusing and one should be removed.

* Navigate to device details and scroll to Security overview section
* @param {string} deviceName - Device name or reference
*/
viewDeviceSecurity: (deviceName) => {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where is it being used?

* Navigate to fleet details and scroll to Security overview section
* @param {string} fleetName - Fleet name
*/
viewFleetSecurity: (fleetName) => {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where is it being used?

* @param {string} fleetName - Fleet name
*/
viewFleetSecurity: (fleetName) => {
cy.visit(`/fleets/${fleetName}`)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this function is needed - better to navigate through UI and not cy.visit

* @param {string} deviceName - Device name or reference
*/
viewDeviceSecurity: (deviceName) => {
cy.visit(`/devices/${deviceName}`)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this function is needed - better to navigate through UI and not cy.visit

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants