Skip to content
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

Insert a default allowlist during schema migrations #5700

Merged
merged 4 commits into from
May 6, 2024

Conversation

bnaecker
Copy link
Collaborator

@bnaecker bnaecker commented May 5, 2024

On new racks, an allowlist for user-facing services is populated during RSS. Nexus waits for the allowlist to exist before starting its external server, to ensure that the rules exist from the first request. On existing racks, however, RSS has already run and the allowlist is never inserted, causing Nexus to wait forever.

This commit adds a one-time insertion of the default allowlist (allowing any external traffic) as a data-only DB migration. An existing allowlist is not modified to preserve existing allowlists during further migrations.

On new racks, an allowlist for user-facing services is populated during
RSS. Nexus waits for the allowlist to exist before starting its external
server, to ensure that the rules exist from the first request. On
existing racks, however, RSS has already run and the allowlist is never
inserted, causing Nexus to wait forever.

This commit adds a one-time insertion of the default allowlist (allowing
any external traffic) as a data-only DB migration. An existing allowlist
is not modified to preserve existing allowlists during further
migrations.
@bnaecker bnaecker force-pushed the insert-default-allowlist-on-migration branch from 14af9b5 to 90ca30b Compare May 5, 2024 21:26
Copy link
Contributor

@rcgoodfellow rcgoodfellow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works in the testbed. Thanks!

@bnaecker bnaecker enabled auto-merge (squash) May 5, 2024 21:57
Copy link
Contributor

@andrewjstone andrewjstone left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense to me. Thanks for all the heroic work getting this right @bnaecker

@bnaecker bnaecker merged commit 47c8c15 into main May 6, 2024
20 checks passed
@bnaecker bnaecker deleted the insert-default-allowlist-on-migration branch May 6, 2024 03:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants