Clarify and enforce bundle FQID equality semantics (#6671)

nadove-ucsc commented 3 weeks ago

Connected issues: #6671

Checklist

Author

[x] PR is a draft
[x] Target branch is develop
[x] Name of PR branch matches issues/<GitHub handle of author>/<issue#>-<slug>
[x] On ZenHub, PR is connected to all issues it (partially) resolves
[x] PR description links to connected issues
[x] PR title matches¹ that of a connected issue _{or comment in PR explains why they're different}
[x] PR title references all connected issues
[x] For each connected issue, there is at least one commit whose title references that issue

¹ when the issue title describes a problem, the corresponding PR title is Fix: followed by the issue title

Author (partiality)

[x] Added p tag to titles of partial commits
[x] This PR is labeled partial _{or completely resolves all connected issues}
[x] This PR partially resolves each of the connected issues _{or does not have the partial label}

Author (chains)

[x] This PR is blocked by previous PR in the chain _{or is not chained to another PR}
[x] The blocking PR is labeled base _{or this PR is not chained to another PR}
[x] This PR is labeled chained _{or is not chained to another PR}

Author (reindex, API changes)

[x] Added r tag to commit title _{or the changes introduced by this PR will not require reindexing of any deployment}
[x] This PR is labeled reindex:dev _{or the changes introduced by it will not require reindexing of dev}
[x] This PR is labeled reindex:anvildev _{or the changes introduced by it will not require reindexing of anvildev}
[x] This PR is labeled reindex:anvilprod _{or the changes introduced by it will not require reindexing of anvilprod}
[x] This PR is labeled reindex:prod _{or the changes introduced by it will not require reindexing of prod}
[x] This PR is labeled reindex:partial and its description documents the specific reindexing procedure for dev, anvildev, anvilprod and prod _{or requires a full reindex or carries none of the labels reindex:dev, reindex:anvildev, reindex:anvilprod and reindex:prod}
[x] This PR and its connected issues are labeled API _{or this PR does not modify a REST API}
[x] Added a (A) tag to commit title for backwards (in)compatible changes _{or this PR does not modify a REST API}
[x] Updated REST API version number in app.py _{or this PR does not modify a REST API}

Author (upgrading deployments)

[x] Ran make docker_images.json and committed the resulting changes _{or this PR does not modify azul_docker_images, or any other variables referenced in the definition of that variable}
[x] Documented upgrading of deployments in UPGRADING.rst _{or this PR does not require upgrading deployments}
[x] Added u tag to commit title _{or this PR does not require upgrading deployments}
[x] This PR is labeled upgrade _{or does not require upgrading deployments}
[x] This PR is labeled deploy:shared _{or does not modify docker_images.json, and does not require deploying the shared component for any other reason}
[x] This PR is labeled deploy:gitlab _{or does not require deploying the gitlab component}
[x] This PR is labeled deploy:runner _{or does not require deploying the runner image}

Author (hotfixes)

[x] Added F tag to main commit title _{or this PR does not include permanent fix for a temporary hotfix}
[x] Reverted the temporary hotfixes for any connected issues _{or the none of the stable branches (anvilprod and prod) have temporary hotfixes for any of the issues connected to this PR}

Author (before every review)

[x] Rebased PR branch on develop, squashed old fixups
[x] Ran make requirements_update _{or this PR does not modify requirements*.txt, common.mk, Makefile and Dockerfile}
[x] Added R tag to commit title _{or this PR does not modify requirements*.txt}
[x] This PR is labeled reqs _{or does not modify requirements*.txt}
[x] make integration_test passes in personal deployment _{or this PR does not modify functionality that could affect the IT outcome}

Peer reviewer (after approval)

[x] PR is not a draft
[x] Ticket is in Review requested column
[x] PR is awaiting requested review from system administrator
[x] PR is assigned to only the system administrator

System administrator (after approval)

[x] Actually approved the PR
[x] Labeled connected issues as demo or no demo
[x] Commented on connected issues about demo expectations _{or all connected issues are labeled no demo}
[x] Decided if PR can be labeled no sandbox
[x] A comment to this PR details the completed security design review
[x] PR title is appropriate as title of merge commit
[x] N reviews label is accurate
[x] Moved connected issues to Approved column
[x] PR is assigned to only the operator

Operator (before pushing merge the commit)

[x] Checked reindex:… labels and r commit title tag
[x] Checked that demo expectations are clear _{or all connected issues are labeled no demo}
[x] Squashed PR branch and rebased onto develop
[x] Sanity-checked history
[x] Pushed PR branch to GitHub
[x] Ran _select dev.shared && CI_COMMIT_REF_NAME=develop make -C terraform/shared apply_keep_unused _{or this PR is not labeled deploy:shared}
[x] Ran _select dev.gitlab && CI_COMMIT_REF_NAME=develop make -C terraform/gitlab apply _{or this PR is not labeled deploy:gitlab}
[x] Ran _select anvildev.shared && CI_COMMIT_REF_NAME=develop make -C terraform/shared apply_keep_unused _{or this PR is not labeled deploy:shared}
[x] Ran _select anvildev.gitlab && CI_COMMIT_REF_NAME=develop make -C terraform/gitlab apply _{or this PR is not labeled deploy:gitlab}
[x] Checked the items in the next section _{or this PR is labeled deploy:gitlab}
[x] PR is assigned to only the system administrator _{or this PR is not labeled deploy:gitlab}

System administrator

[x] Background migrations for dev.gitlab are complete _{or this PR is not labeled deploy:gitlab}
[x] Background migrations for anvildev.gitlab are complete _{or this PR is not labeled deploy:gitlab}
[x] PR is assigned to only the operator

Operator (before pushing merge the commit)

[x] Ran _select dev.gitlab && make -C terraform/gitlab/runner _{or this PR is not labeled deploy:runner}
[x] Ran _select anvildev.gitlab && make -C terraform/gitlab/runner _{or this PR is not labeled deploy:runner}
[x] Added sandbox label _{or PR is labeled no sandbox}
[x] Pushed PR branch to GitLab dev _{or PR is labeled no sandbox}
[x] Pushed PR branch to GitLab anvildev _{or PR is labeled no sandbox}
[x] Build passes in sandbox deployment _{or PR is labeled no sandbox}
[x] Build passes in anvilbox deployment _{or PR is labeled no sandbox}
[x] Reviewed build logs for anomalies in sandbox deployment _{or PR is labeled no sandbox}
[x] Reviewed build logs for anomalies in anvilbox deployment _{or PR is labeled no sandbox}
[x] Deleted unreferenced indices in sandbox _{or this PR does not remove catalogs or otherwise causes unreferenced indices in dev}
[x] Deleted unreferenced indices in anvilbox _{or this PR does not remove catalogs or otherwise causes unreferenced indices in anvildev}
[x] Started reindex in sandbox _{or this PR is not labeled reindex:dev}
[x] Started reindex in anvilbox _{or this PR is not labeled reindex:anvildev}
[x] Checked for failures in sandbox _{or this PR is not labeled reindex:dev}
[x] Checked for failures in anvilbox _{or this PR is not labeled reindex:anvildev}
[x] The title of the merge commit starts with the title of this PR
[x] Added PR # reference to merge commit title
[x] Collected commit title tags in merge commit title _{but only included p if the PR is also labeled partial}
[x] Moved connected issues to Merged lower column in ZenHub
[x] Moved blocked issues to Triage _{or no issues are blocked on the connected issues}
[x] Pushed merge commit to GitHub

Operator (chain shortening)

[x] Changed the target branch of the blocked PR to develop _{or this PR is not labeled base}
[x] Removed the chained label from the blocked PR _{or this PR is not labeled base}
[x] Removed the blocking relationship from the blocked PR _{or this PR is not labeled base}
[x] Removed the base label from this PR _{or this PR is not labeled base}

Operator (after pushing the merge commit)

[x] Pushed merge commit to GitLab dev
[x] Pushed merge commit to GitLab anvildev
[x] Build passes on GitLab dev
[x] Reviewed build logs for anomalies on GitLab dev
[x] Build passes on GitLab anvildev
[x] Reviewed build logs for anomalies on GitLab anvildev
[x] Ran _select dev.shared && make -C terraform/shared apply _{or this PR is not labeled deploy:shared}
[x] Ran _select anvildev.shared && make -C terraform/shared apply _{or this PR is not labeled deploy:shared}
[x] Deleted PR branch from GitHub
[x] Deleted PR branch from GitLab dev
[x] Deleted PR branch from GitLab anvildev

Operator (reindex)

[x] Deindexed all unreferenced catalogs in dev _{or this PR is neither labeled reindex:partial nor reindex:dev}
[x] Deindexed all unreferenced catalogs in anvildev _{or this PR is neither labeled reindex:partial nor reindex:anvildev}
[x] Deindexed specific sources in dev _{or this PR is neither labeled reindex:partial nor reindex:dev}
[x] Deindexed specific sources in anvildev _{or this PR is neither labeled reindex:partial nor reindex:anvildev}
[x] Indexed specific sources in dev _{or this PR is neither labeled reindex:partial nor reindex:dev}
[x] Indexed specific sources in anvildev _{or this PR is neither labeled reindex:partial nor reindex:anvildev}
[x] Started reindex in dev _{or this PR does not require reindexing dev}
[x] Started reindex in anvildev _{or this PR does not require reindexing anvildev}
[x] Checked for, triaged and possibly requeued messages in both fail queues in dev _{or this PR does not require reindexing dev}
[x] Checked for, triaged and possibly requeued messages in both fail queues in anvildev _{or this PR does not require reindexing anvildev}
[x] Emptied fail queues in dev _{or this PR does not require reindexing dev}
[x] Emptied fail queues in anvildev _{or this PR does not require reindexing anvildev}

Operator

[x] Propagated the deploy:shared, deploy:gitlab, deploy:runner, API, reindex:partial, reindex:anvilprod and reindex:prod labels to the next promotion PRs _{or this PR carries none of these labels}
[x] Propagated any specific instructions related to the deploy:shared, deploy:gitlab, deploy:runner, API, reindex:partial, reindex:anvilprod and reindex:prod labels, from the description of this PR to that of the next promotion PRs _{or this PR carries none of these labels}
[x] PR is assigned to no one

Shorthand for review comments

L line is too long
W line wrapping is wrong
Q bad quotes
F other formatting problem

codecov[bot] commented 3 weeks ago

Codecov Report

Attention: Patch coverage is 93.10345% with 2 lines in your changes missing coverage. Please review.

Project coverage is 85.40%. Comparing base (14a2633) to head (7cb40d2). Report is 3 commits behind head on develop.

Files with missing lines	Patch %	Lines
test/integration_test.py	0.00%	2 Missing :warning:

Additional details and impacted files

```diff @@ Coverage Diff @@ ## develop #6672 +/- ## =========================================== + Coverage 85.35% 85.40% +0.05% =========================================== Files 155 155 Lines 20779 20783 +4 =========================================== + Hits 17735 17749 +14 + Misses 3044 3034 -10 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

coveralls commented 3 weeks ago

coverage: 85.419% (+0.05%) from 85.368% when pulling 7cb40d2ac822833ff32fbc61081dddfd709fa237 on issues/nadove-ucsc/6671-clarify-enforce-bundle-fqid-equality into 14a2633880114005ab5c849ee78f329a3f8e8bc5 on develop.

achave11-ucsc commented 3 weeks ago

Skipping peer review, as per @hannes-ucsc's request.

hannes-ucsc commented 2 weeks ago

Security design review

[x] Security design review completed; this PR does not …
- [x] … affect authentication; for example:
- OAuth 2.0 with the application (API or Swagger UI)
- Authentication of developers with Google Cloud APIs
- Authentication of developers with AWS APIs
- Authentication with a GitLab instance in the system
- Password and 2FA authentication with GitHub
- API access token authentication with GitHub
- Authentication with Terra
- [x] … affect the permissions of internal users like access to
- Cloud resources on AWS and GCP
- GitLab repositories, projects and groups, administration
- an EC2 instance via SSH
- GitHub issues, pull requests, commits, commit statuses, wikis, repositories, organizations
- [x] … affect the permissions of external users like access to
- TDR snapshots
- [x] … affect permissions of service or bot accounts
- Cloud resources on AWS and GCP
- [x] … affect audit logging in the system, like
- adding, removing or changing a log message that represents an auditable event
- changing the routing of log messages through the system
- [x] … affect monitoring of the system
- [x] … introduce a new software dependency like
- Python packages on PYPI
- Command-line utilities
- Docker images
- Terraform providers
- [x] … add an interface that exposes sensitive or confidential data at the security boundary
- [x] … affect the encryption of data at rest
- [x] … require persistence of sensitive or confidential data that might require encryption at rest
- [x] … require unencrypted transmission of data within the security boundary
- [x] … affect the network security layer; for example by
- modifying, adding or removing firewall rules
- modifying, adding or removing security groups
- changing or adding a port a service, proxy or load balancer listens on
[x] Documentation on any unchecked boxes is provided in comments below

DataBiosphere / azul