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

Handle IPPolicy CRD state transitions in a safer way #260

Merged
merged 2 commits into from
Jun 27, 2023

Conversation

jonstacks
Copy link
Collaborator

Contributes to #221

What

Since CIDR has to be unique within an IPPolicy: HTTP 400: IP Policy Rule with CIDR '127.0.0.1/32' already exists [ERR_NGROK_1408], we need to calculate the diff and apply the changes in a safe way.

How

Use the following diffing logic to apply creates, deletes, and updates in the following order:

  1. Create all new deny rules that don't exist in the remote with a matching CIDR.
  2. Delete any allow rules with matching CIDRs that will be changing to deny rules. Then create the deny rules
  3. Delete any deny rules with matching CIDRs that will be changing to allow rules. Then create the allow rules.
  4. Create all new allow rules that don't exist in the remote with a matching CIDR.
  5. Delete any remaining rules that are not in the spec.
  6. Update any rules that exist in the spec and remote but have only different metadata/description.

Breaking Changes

No.

@jonstacks jonstacks added the bug Something isn't working label Jun 27, 2023
@jonstacks jonstacks requested a review from a team as a code owner June 27, 2023 15:43
@jonstacks jonstacks self-assigned this Jun 27, 2023
@github-actions github-actions bot added area/controller Issues dealing with the controller area/helm-chart Issues dealing with the helm chart labels Jun 27, 2023
Copy link
Contributor

@bobzilladev bobzilladev left a comment

Choose a reason for hiding this comment

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

A couple minor comments, but overall looks to do what is the desired sequence, which does look like a series safe update transitions.

internal/controllers/ippolicy_controllers_test.go Outdated Show resolved Hide resolved
internal/controllers/ippolicy_controller.go Outdated Show resolved Hide resolved
Update tests and dry-up adding ip policy rules updates
@jonstacks jonstacks merged commit 1e031d1 into ngrok:main Jun 27, 2023
@jonstacks jonstacks deleted the fix-for-ip-policy-failures branch June 27, 2023 22:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/controller Issues dealing with the controller area/helm-chart Issues dealing with the helm chart bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants