Geo: Remove checksum_mismatch and verification_checksum_mismatched from SSF

checksum_mismatch and verification_checksum_mismatched were added to the SSF to follow project/wiki repo verification behavior. These fields on ProjectRegistry are only exposed by Prometheus, and by /api/v4/geo_nodes/current/failures. These project/wiki fields are not exposed in UI, and are not used for other logic. In SSF currently, these fields are not used at all.

Assertion: Users don't use /api/v4/geo_nodes/current/failures.

If so, then:

Proposal

  • Very easy first iteration, weight 0.5: Remove the code managing these fields from SSF, and remove mention of the fields from the framework doc.
  • Later, no urgency, weight 1: Remove the columns.

Benefits:

  • don't need to update /api/v4/geo_nodes/current/failures for SSF or add a similar one for SSF
  • don't need to add these fields to all current and future registry tables
  • don't need to maintain the related code, e.g. overrides in VerifiableRegistry

Drawbacks:

  • Project/wiki repos will lose functionality if we remove the endpoint (we don't have to remove the endpoint upon migrating project/wiki repos to SSF, but then that will be a special case to maintain)
  • It will be more effort to add back later after the codebase has moved on. Perhaps we do want to expose checksum_mismatch in the UI?
Click here to expand/collapse related Slack conversation

https://gitlab.slack.com/archives/C7U95P909/p1611689281025200

@valery @douglas (and anyone else who has an opinion!) I preserved checksum_mismatch and verification_checksum_mismatched in the current state of verification for package files, but it looks like the only purpose of these fields is to be able to expose and filter by them in /api/v4/geo_nodes/current/failures https://gitlab.com/gitlab-org/gitlab/blob/master/ee/lib/api/geo_nodes.rb#L120-125. I see some value in that but also it's a lot of moving parts to maintain considering it is not in the UI and isn't used for other logic. It isn't exposed at all (except in prometheus) by the SSF yet. I'm inclined to not add that endpoint to SSF, and in that case we could also remove these fields from SSF registry tables and remove the code managing them. WDYT? (edited) 

GitLabGitLab
ee/lib/api/geo_nodes.rb · master · GitLab.org / GitLab
GitLab is an open source end-to-end software development platform with built-in version control, issue tracking, code review, CI/CD, and more. Self-host GitLab on your own servers, in a...
6 replies
New

douglas  4 minutes ago
IIRC, the idea behind these columns is to be able to distinguish between the real failures during the verification process and the checksum mismatches.

douglas  3 minutes ago
Before removing them, it would be good to remember why we add them in the first place.
:+1:
1


mkozono  3 minutes ago
agreed, and I think it does accomplish that, but I am skeptical that many people are using this endpoint to discover that information

mkozono  2 minutes ago
I will open an issue

douglas  1 minute ago
Yeap, if it is not being used, I'm 100% in favor of removing it.

mkozono  1 minute ago
:+1:
Edited by Michael Kozono