Add expandable section for image vulnerability components table

[dvail]

Description

Adds the expandable section to image single page table rows. This section contains another table with details on all image components that are affected by this CVE.

Follow ups

• Figure out why fixedIn and location fields always return empty strings (BE bug?)
• Figure out why layerIndex is always null (BE bug?)
• Reset the expanded/collapsed state on all rows when the table is sorted or paginated (depends on Add pagination and sorting to image single table #5258)

Checklist

• Investigated and inspected CI test results
• Unit test and regression tests added
• Evaluated and added CHANGELOG entry if required
• Determined and documented upgrade steps
• Documented user facing changes (create PR based on openshift/openshift-docs and merge into rhacs-docs)

If any of these don't apply, please comment below.

Testing Performed

Load an image single page and see that expandable arrows are now present.

Expand a row by clicking on the expand arrow on the left side of the table.

Collapse the row by clicking the arrow again.

Verify that multiple rows can be expanded at the same time.

Verify that a row with multiple components contains the correct number of rows in the embedded table.

[openshift-ci]

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

[dvail]

Current dependencies on/for this PR:

• master
  • PR Add status tabs and summary card shells to images single page. #5073
    • PR Add CVE severity and status icons to image page summary cards #5116
      • PR Add images table for Workload CVE Image single page #5175
        • PR Add expandable section for image vulnerability components table #5218 👈
    • PR Add URL string union hook to better integrate url params with Tab components #5111

This comment was auto-generated by Graphite.

[roxbot]

Images are ready for the commit at 9f3f51b.

To use with deploy scripts, first export MAIN_IMAGE_TAG=3.74.x-411-g9f3f51b506.

[dvail]

As straightforward as it looks, this test actually helped uncover a bug. The toggle function in useSet was relying on a closure value, which failed to update correctly when multiple toggle calls happened in a row. Using the alternate form of useState fixed the issue in the below diff:

     const [itemSet, setItemSet] = useState(initialSet);

     function toggle(key: T, isOn?: boolean) {
-        const nextSet = new Set(itemSet);
-        const shouldAdd = typeof isOn === 'undefined' ? !itemSet.has(key) : isOn;
-        if (shouldAdd) {
-            nextSet.add(key);
-        } else {
-            nextSet.delete(key);
-        }
-        setItemSet(nextSet);
+        setItemSet((prevSet) => {
+            const nextSet = new Set(prevSet);
+            const shouldAdd = typeof isOn === 'undefined' ? !itemSet.has(key) : isOn;
+            if (shouldAdd) {
+                nextSet.add(key);
+            } else {
+                nextSet.delete(key);
+            }
+            return nextSet;
+        });
     }

[pedrottimark]

Indeed, please share this at the next team meeting. Return on your investment to write tests!

[pedrottimark]

Prettier wrapping caused me to notice partial alphabetical order.

Do you mind to reorder here, and also in ImageComponentsTable above?

[dvail]

Aha, I've been pretty good about checking these and this one slipped by.

[pedrottimark]

Bravo!

[dvail]

All credit to @alwayshooin for pointing this one out.

[pedrottimark]

Maybe this is an answer to the question that I asked above about severity keys?

[dvail]

I've been using the long form LOW_VULNERABILITY_SEVERITY as the canonical severity type since it is what is declared in our cve.proto.ts file and what is returned in the BE queries. The explicit undefined checks here are for a few reasons:

1. The GraphQL API declares the return type of severity as string, even though from what I can tell in the code it will always be an enum type as declared in the proto. I'm not sure if there is a technical reason the API isn't declaring this as an enum as well, so I'm staying on the safe side assuming it is a string.
2. We have "noImplicitAny": false set in our tsconfig, so indexing into the SeverityIcons here silently treats SeverityIcon as any.
3. Even if the types were strictly declared, we could end up with an UNKNOWN_VULNERABILITY_SEVERITY here (RE: my comment on the other PR), which would be a type mismatch between the FE and BE declared types.
4. Due to all of the above, even though indexing into SeverityIcons should be exhaustive and safe given a "Severity", there is a small chance we could end up with a React component that is undefined which would cause a runtime error here.

[pedrottimark]

Thank you for explaining. What you say makes sense.

To my original question, the newest query response adds a second convention for severity keys.

String union in frontend types corresponds to enum with string values in backend types.

It seems (to me) as if enum in TypeScript corresponded to other languages and, at least for strings, string union has superseded it.

[pedrottimark]

The expandable table reminds me of your excellent solution to tree table in interview :)

A few comments, as usual.

[openshift-ci]

@dvail: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/gke-postgres-ui-e2e-tests	524b1f6	link	false	/test gke-postgres-ui-e2e-tests
ci/prow/gke-ui-e2e-tests	9f3f51b	link	false	/test gke-ui-e2e-tests

Full PR test history. Your PR dashboard.

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 kubernetes/test-infra repository. I understand the commands that are listed here.