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

fix: ComputeGatewayDiffs when missing target HTTPRoute #139

Merged
merged 4 commits into from
Jan 26, 2023

Conversation

guicassolato
Copy link
Contributor

@guicassolato guicassolato commented Jan 13, 2023

Related to #138

@guicassolato guicassolato requested a review from a team as a code owner January 13, 2023 13:52
@guicassolato guicassolato force-pushed the fix/net-resource-not-found branch 2 times, most recently from 1ec9115 to ba5477e Compare January 13, 2023 14:47
@guicassolato guicassolato marked this pull request as draft January 13, 2023 15:13
@guicassolato
Copy link
Contributor Author

Flagging this as WIP so we can fix as well other points where reconciliation fails due to missing the targeted HTTPRoute.

@guicassolato guicassolato changed the title fix: RLP cleanup when the target HTTPRoute is missing fix: ComputeGatewayDiffs when missing target HTTPRoute Jan 13, 2023
@guicassolato guicassolato force-pushed the fix/net-resource-not-found branch from ba5477e to 4dcd7de Compare January 13, 2023 15:35
@guicassolato
Copy link
Contributor Author

Let's make this about ComputeGatewayDiffs only (so it unblocks the fix for #126), and let #138 and related issues cover the rest of the work needed to fix the reconciliation of RLP upon deletion of target HTTPRoutes.

@guicassolato guicassolato marked this pull request as ready for review January 13, 2023 15:42
@guicassolato guicassolato requested a review from eguzki January 13, 2023 15:49
…t is missing (#142)

* fix: reconciliation of implementation resources when the network object is missing

* RateLimitPolicy
  - EnvoyFilter
  - WasmPlugin

* AuthPolicy
  - IstioAuthorizationPolicy

* fix: envoyfilter and wasmplugin cleanup

* fix: race condition between resource cleanup and back ref annotation removal

* refactor: gateway and httproute to policy event mappers

+ set gateway event mapper to the authpolicyreconciler

* refactor: dry: read k8s object annotations

* debug log added to the authpolicy reconciler: print the policy as json

* fix: race condition between handling network object not found and policy marked for deletion

+ partial rollback of the previous race condition fix: cleanup and back ref annotation

* fix: gateways should refer no deleted policies

* gitignore tmp

* reconcile istio authorizationpolicies and authorino authconfigs only when specs or annotations differ between existing and desired

* refactor: rename target network object variables and refs for clarity
Copy link
Member

@didierofrivia didierofrivia left a comment

Choose a reason for hiding this comment

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

👍🏼

- `ctx` unused in `istioAuthorizationPolicyRules` function
- eliminating type assertions in switch case by assigning the result of the assertion to a variable
- using copy() instead of a loop to copy slice
- avoid copying a sync.Mutex when using reflect.DeepEqual on istio.io/api/security/v1beta1.AuthorizationPolicy.Spec
@guicassolato guicassolato requested review from didierofrivia and eguzki and removed request for eguzki January 25, 2023 20:31
@guicassolato guicassolato merged commit fc44989 into main Jan 26, 2023
@guicassolato guicassolato deleted the fix/net-resource-not-found branch January 26, 2023 11:08
mikenairn pushed a commit to mikenairn/kuadrant-operator that referenced this pull request Mar 23, 2023
* fix examples/toystore/ratelimitpolicy.yaml

* run lint check from makefile
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