Spike: Rearchitect Security Configuration frontend

Why are we doing this work

The existing Security Configuration page is currently implemented twice: once for GitLab Ultimate users, and again for non-Ultimate users.

The decision to have a separate, static implementation of the page for non-Ultimate users was taken so as to minimise the effort/time to make something available for non-Ultimate - i.e., it's an MVC.

Having two implementations increases the maintenance burden in the long term, so this issue is about investigating a different frontend architecture that would accommodate sharing code/implementations between the two versions.

What's the goal of the spike?

To determine whether the proposed architectures will make it easier to:

Relevant links

Non-functional requirements

  • [-] Documentation:
  • [-] Feature flag:
  • [-] Performance:
  • Testing: See #322462 (closed)

Proposals

1. Using scanner-specific rows

Since this is a spike, the plan isn't concrete. But in general, the approach to investigate will move user-facing strings to the frontend, and to move away from the feature abstraction and use concrete scanner-specific rows. The old implementation is column-centric, leading to various code smells; this rearchitecturing will be row/scanner-centric.

2. Using scanner-specific cells

This is closer to what the CE implementation does already, though for both the Status and Manage cells. This was the approach taken in !55623 (closed).

Next steps

  • Open follow-up issue(s) to implement new architecture (unless the effort can just be done and finished off as part of this issue)

Auto-Summary 🤖

Discoto Usage

Points

Discussion points are declared by headings, list items, and single lines that start with the text (case-insensitive) point:. For example, the following are all valid points:

  • #### POINT: This is a point
  • * point: This is a point
  • + Point: This is a point
  • - pOINT: This is a point
  • point: This is a **point**

Note that any markdown used in the point text will also be propagated into the topic summaries.

Outcomes

Outcomes define the decisions or resolutions of a discussion. Once outcomes are defined, sub-topics and points are collapsed underneath the outcomes.

Outcomes are declared in a similar manner as points:

  • #### OUTCOME: This is an outcome
  • * outcome: This is an outcome
  • + Outcome: This is an outcome
  • - oUTCOME: This is an outcome
  • outcome: This is an outcome

Note that multiple outcomes may be declared for each topic.

Topics

Topics can be stand-alone and contained within an issuable (epic, issue, MR), or can be inline.

Inline topics are defined by creating a new thread (discussion) where the first line of the first comment is a heading that starts with (case-insensitive) topic:. For example, the following are all valid topics:

  • # Topic: Inline discussion topic 1
  • ## TOPIC: **{+A Green, bolded topic+}**
  • ### tOpIc: Another topic

Quick Actions

Action Description
/discuss sub-topic TITLE Create an issue for a sub-topic. Does not work in epics
/discuss link ISSUABLE-LINK Link an issuable as a child of this discussion

Discussion-Size Indicators

The relative size of the discussion occurring within a topic and its sub-topics is indicated via braille dots.

More dots means that more points or sub-topics exist within a given topic.

Examples:

  • TOPIC ⣿⣿⡆ A large discussion occurred here
  • TOPIC ⣇ A smaller discussion occurred here

Last updated by this job

  • TOPIC ⡆ ⢠ Defining UI strings in the frontend https://gitlab.com/gitlab-org/gitlab/-/issues/322624#note_521279530
         OUTCOME: Do this! https://gitlab.com/gitlab-org/gitlab/-/issues/322624#note_521403030
    • Would make the frontend the SSOT for the name, description and help link paths for each of the scanners, and also the status messages. https://gitlab.com/gitlab-org/gitlab/-/issues/322624#note_521299683
    • We currently have two SOTs for these: in the backend for the GitLab Ultimate version, and in the frontend for the CE version. There are _already_ some discrepancies between these (e.g., DAST Scans description; different ordering of scanners) https://gitlab.com/gitlab-org/gitlab/-/issues/322624#note_521299683
  • TOPIC ⡆ ⣸ Using scanner-specific rows https://gitlab.com/gitlab-org/gitlab/-/issues/322624#note_521280283
         OUTCOME: Don't do this https://gitlab.com/gitlab-org/gitlab/-/issues/322624#note_521402925
    • Requires using plain HTML table markup https://gitlab.com/gitlab-org/gitlab/-/issues/322624#note_521303460
    • PRO Co-locates all the scanner-specific behaviour into one component, making it easier to understand/modify https://gitlab.com/gitlab-org/gitlab/-/issues/322624#note_521303460
    • CON Requires a total rewrite of the entire table https://gitlab.com/gitlab-org/gitlab/-/issues/322624#note_521303460
    • CON We lose some benefits of `GlTable`, e.g., stacking behaviour (`stacked="md"`) for display on smaller screens, and also some a11y https://gitlab.com/gitlab-org/gitlab/-/issues/322624#note_521303460
    • CON Cannot so easily add/remove columns https://gitlab.com/gitlab-org/gitlab/-/issues/322624#note_521303460
  • TOPIC ⡆ ⣾ Using GraphQL as data source for scanner status https://gitlab.com/gitlab-org/gitlab/-/issues/322624#note_521291915
         OUTCOME: Do this! (Though blocked from doing this for CE by https://gitlab.com/gitlab-org/gitlab/-/issues/322627) https://gitlab.com/gitlab-org/gitlab/-/issues/322624#note_521402769
    • `Project.securityScanners` already exists in our GraphQL API, which is used for the Vulnerability Report page, as it indicates which scanners are enabled https://gitlab.com/gitlab-org/gitlab/-/issues/322624#note_521318692
    • Planned to make this API available in CE: https://gitlab.com/gitlab-org/gitlab/-/issues/322627 https://gitlab.com/gitlab-org/gitlab/-/issues/322624#note_521318692
    • CON Does not include License Compliance status https://gitlab.com/gitlab-org/gitlab/-/issues/322624#note_521318692
    • CON Does not include information about _which_ pipeline the scan last ran on (though does have a `pipelineRun` field) https://gitlab.com/gitlab-org/gitlab/-/issues/322624#note_521318692
    • PRO Would make for a consistent way to get scanner status across CE/EE https://gitlab.com/gitlab-org/gitlab/-/issues/322624#note_521318692
    • Includes `available` field, although that appears not to work as expected. In theory, this _could_ be used to determine whether or not to display the upgrade CTA in the Manage column. https://gitlab.com/gitlab-org/gitlab/-/issues/322624#note_521318692
    • Does not (_should_ not) provide all information necessary to drive the UI - e.g., scanner-specific configuration page paths (a Rails concern rather than an API one) https://gitlab.com/gitlab-org/gitlab/-/issues/322624#note_521318692
  • TOPIC ⡆ ⢸ Using scanner-specific cells https://gitlab.com/gitlab-org/gitlab/-/issues/322624#note_521315486
         OUTCOME: Do this! https://gitlab.com/gitlab-org/gitlab/-/issues/322624#note_521401973
    • Basically the strategy already used in the CE implementation https://gitlab.com/gitlab-org/gitlab/-/issues/322624#note_521364940
    • PRO Makes for simpler, self-contained single-responsibility components (e.g., a button to fire a GraphQL mutation) https://gitlab.com/gitlab-org/gitlab/-/issues/322624#note_521364940
    • CON Spreads the behaviour of how a particular scanner row renders across more components, potentially making it harder to understand https://gitlab.com/gitlab-org/gitlab/-/issues/322624#note_521364940
    • PRO This is much closer to the current implementation, so is less work to adopt https://gitlab.com/gitlab-org/gitlab/-/issues/322624#note_521364940
  • TOPIC ⡆ ⢀ Sharing code between CE/EE https://gitlab.com/gitlab-org/gitlab/-/issues/322624#note_521391691
         OUTCOME: We can improve this incrementally. Doing this optimally requires a lot of work, though (e.g., https://gitlab.com/gitlab-org/gitlab/-/issues/322627, https://gitlab.com/gitlab-org/gitlab/-/issues/248105, https://gitlab.com/groups/gitlab-org/-/epics/4894) https://gitlab.com/gitlab-org/gitlab/-/issues/322624#note_521405592
    • It _should_ be easier to share code between CE/EE following any of the proposals here, but it's not entirely clear what the best strategy is yet. Doing better than this is outside the scope of this issue. In other words: we'll iterate on this as and when we need to! https://gitlab.com/gitlab-org/gitlab/-/issues/322624#note_521393516
  • TOPIC ⡆ ⢠ Moving scanners to lower tiers https://gitlab.com/gitlab-org/gitlab/-/issues/322624#note_521393909
         OUTCOME: Figuring out how to handle this better is out of scope of this issue, and not necessary to solve right now. https://gitlab.com/gitlab-org/gitlab/-/issues/322624#note_521403528
    • We currently only have two different behaviours: GitLab Ultimate and non-Ultimate. We rely on this dichotomy and conflate it with CE/EE, which is not a long term or "correct" solution. https://gitlab.com/gitlab-org/gitlab/-/issues/322624#note_521401323
    • To handle this better, the backend will need to provide more granular information for each scanner, e.g., whether it's available for the project; whether it can be enabled via MR, or whether it has a configuration page/UI, or some other thing specific to the scanner. https://gitlab.com/gitlab-org/gitlab/-/issues/248105 and https://gitlab.com/groups/gitlab-org/-/epics/4894 are relevant for this. https://gitlab.com/gitlab-org/gitlab/-/issues/322624#note_521401323
Edited by 🤖 GitLab Bot 🤖