-
Notifications
You must be signed in to change notification settings - Fork 9
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix duplicate report_id generation #4681
Conversation
Terraform plan for meta No changes. Your infrastructure matches the configuration.
✅ Plan applied in Deploy to Development and Management Environment #947 |
Terraform plan for dev Plan: 1 to add, 3 to change, 1 to destroy.Terraform used the selected providers to generate the following execution
plan. Resource actions are indicated with the following symbols:
!~ update in-place
-/+ destroy and then create replacement
Terraform will perform the following actions:
# module.dev.cloudfoundry_app.postgrest will be updated in-place
!~ resource "cloudfoundry_app" "postgrest" {
!~ docker_image = "ghcr.io/gsa-tts/fac/postgrest@sha256:7d69652cbaa659b714766c5f5343308e7f0993cb4fb5581520c8745ca0275927" -> "ghcr.io/gsa-tts/fac/postgrest@sha256:fe10dfad7c13ee2a0828641ef7ab4544461d08681980112d147b8565c4377c78"
id = "5de6f04c-52e0-400f-961a-16243b1c7c9a"
name = "postgrest"
# (17 unchanged attributes hidden)
# (2 unchanged blocks hidden)
}
# module.dev.module.clamav.cloudfoundry_app.clamav_api will be updated in-place
!~ resource "cloudfoundry_app" "clamav_api" {
!~ docker_image = "ghcr.io/gsa-tts/fac/clamav@sha256:09e86b5cae004732505f7d3828e7121c54fd88572d72d929fec0f50bb5597d6e" -> "ghcr.io/gsa-tts/fac/clamav@sha256:49a1b0ba138e45f00d89001d8dd63733a4658bc43237e2ac378028bb37619dac"
id = "779bbc51-f78a-4186-90eb-5acb68d7d746"
name = "fac-av-dev"
# (17 unchanged attributes hidden)
# (1 unchanged block hidden)
}
# module.dev.module.cors.null_resource.cors_header must be replaced
-/+ resource "null_resource" "cors_header" {
!~ id = "*******************" -> (known after apply)
!~ triggers = { # forces replacement
!~ "always_run" = "2025-01-30T20:42:20Z" -> (known after apply)
}
}
# module.dev.module.file_scanner_clamav.cloudfoundry_app.clamav_api will be updated in-place
!~ resource "cloudfoundry_app" "clamav_api" {
!~ docker_image = "ghcr.io/gsa-tts/fac/clamav@sha256:09e86b5cae004732505f7d3828e7121c54fd88572d72d929fec0f50bb5597d6e" -> "ghcr.io/gsa-tts/fac/clamav@sha256:49a1b0ba138e45f00d89001d8dd63733a4658bc43237e2ac378028bb37619dac"
id = "65c83416-4126-4785-99c2-5e1adb810422"
name = "fac-av-dev-fs"
# (17 unchanged attributes hidden)
# (1 unchanged block hidden)
}
Plan: 1 to add, 3 to change, 1 to destroy. ❌ Plan not applied in Deploy to Development and Management Environment #947 (Plan has changed) |
This pull request is not up to date with main. Please merge main into this brach or rebase this branch onto main. This PR should not be approved until all status checks pass. If you see this message, please rerun all status checks before merging. |
Minimum allowed line rate is |
Addresses #4638.
This PR is expected to fix the following error that appears for POST
/report_submission/accessandsubmission
:Our routine for generating
report_id
takes the audit date and the total count ofSingleAuditChecklist
+ 1. The total count is appearing to return a number on creation of a new SAC that is already tied to an existing report_id in the database. My suspicion is that our current logic allows for the race condition.In theory, two SACs (with the same audit year and month) could be created at the same time and attempt to pull the same "count" when generating the report_id. See this document for reference.
How to test
Step 1 - Prepare race condition check
Inside
backend/report_submission/views.py
, insert the following method:At the start of the
transaction.atomic()
block in the post method ofAccessAndSubmissionFormView
, replace:with this block:
Step 2 - Test the race condition
localhost:8000
andlocalhost:8001
. Have them up as separate tabs.localhost:8000
and thenlocalhost:8001
in quick succession. Both buttons should be grayed out as soon as they are clicked. This prevents double-clicks.PR Checklist: Submitter
main
into your branch shortly before creating the PR. (You should also be mergingmain
into your branch regularly during development.)git status | grep migrations
. If there are any results, you probably need to add them to the branch for the PR. Your PR should have only one new migration file for each of the component apps, except in rare circumstances; you may need to delete some and re-runpython manage.py makemigrations
to reduce the number to one. (Also, unless in exceptional circumstances, your PR should not delete any migration files.)PR Checklist: Reviewer
make docker-clean; make docker-first-run && docker compose up
; then rundocker compose exec web /bin/bash -c "python manage.py test"
The larger the PR, the stricter we should be about these points.
Pre Merge Checklist: Merger
-/+ resource "null_resource" "cors_header"
should be destroying and recreating its self and~ resource "cloudfoundry_app" "clamav_api"
might be updating itssha256
for thefac-file-scanner
andfac-av-${ENV}
by default.main
.