BGP unnumbered peers: make database representation consistent and more strict#10155
Open
jgallagher wants to merge 21 commits intojohn/stronger-unnumbered-types-2from
Open
BGP unnumbered peers: make database representation consistent and more strict#10155jgallagher wants to merge 21 commits intojohn/stronger-unnumbered-types-2from
jgallagher wants to merge 21 commits intojohn/stronger-unnumbered-types-2from
Conversation
…ersion module where we changed over
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.
Staged on top of #10122, which strengthened the types used for BGP peers in the external API.
This PR is pretty big, but the actual prod code changes are more like +600/-300 (there are a bunch of new tests and a bunch of SQL migration Stuff pushing the total size up). I'd recommend starting with the changes to
dbinit.sql, which should be in line with the plan @internet-diglett and I discussed. Summarizing #9832 (comment):NULLaddressCHECKconstraints that prevent storing the addresses0.0.0.0or::to ensure we don't have any leftover sentinel valuesidprimary key (a random UUID)The database migrations are similar in spirit to #9976: while we're not correcting mismatches between diesel and CRDB here, we are introducing migrations that could fail if real databases have data we don't expect (e.g., an address of
0.0.0.0in the table where we expect unnumbered peers to be represented as NULL). The tactic I went with in the migration is:NULL.NULLthen0.0.0.0) and delete the rest. This shouldn't delete anything unless someone has some already-invalid-at-runtime data.There's a data migration test that should cover a sample of each kind of possibly-invalid input and confirm the above behavior.
One other thing that snuck in here is a bugfix for #10151; that's in 70ae97e.