Repository navigation
Prevent deleting a silo's last certificate - #11510
Open
david-crespo wants to merge 1 commit into
Open
david-crespo wants to merge 1 commit into
david-crespo wants to merge 1 commit into
Conversation
Collaborator
|
as an FYI, you may also be interested in #11238, which overlaps here |
david-crespo
force-pushed
the
keep-last-silo-cert
branch
from
October 15, 2026 22:36
a048cfb to
82c64ef
Compare
Contributor
Author
|
Saw it — am I right that they're complementary and don't conflict? |
Collaborator
I believe that's the case! If anything I might be able to make the guarantees in the diagnosis engine a little stronger with this PR |
david-crespo
force-pushed
the
keep-last-silo-cert
branch
from
October 22, 2026 16:47
82c64ef to
f4b9b65
Compare
Contributor
Author
|
@claude review |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #11272
Deleting a silo's only certificate leaves Nexus unable to serve TLS for the silo's domain, so nobody in the silo can reach the API to upload a replacement. #11470 mitigates this by making it so fleet admins can fix this from a different silo, but it can still be a problem if it happens to the fleet admins' own silo.
This PR makes
certificate_deleteerror out unless another certificate exists for the same silo. The check is anEXISTSsubquery in the sameUPDATEthat setstime_deleted, avoiding a TOCTOU race that would let two concurrent deletes on the last two certs both succeed. (#11109 does the same thing for unassigning a service's last IP pool.)The error is a 400. 409 could make sense, but 400 is the convention for refusing to delete; it's what we do for non-empty projects and silos, VPCs with subnets, IP pools with ranges, attached floating IPs and disks, etc.