fix: avoid APIBindings stuck with our finalizers #306
Conversation
nexus49
left a comment
nexus49
left a comment
There was a problem hiding this comment.
I don't believe we can manage the AccountInfo finalizer from the AuthorizationModel Subroutine as the logic for its removal needs to consider all APIBindings in that particular workspace.
I'd suggest to add a very basic accountInfo subroutine/controller that considers all APIbindings in the workspace and only manages the finalizer
| if controllerutil.ContainsFinalizer(&toDeleteAccountInfo, authorizationModelGenerationFinalizer) { | ||
| controllerutil.RemoveFinalizer(&toDeleteAccountInfo, authorizationModelGenerationFinalizer) | ||
| if err := bindingCluster.GetClient().Update(ctx, &toDeleteAccountInfo); err != nil { | ||
| return ctrl.Result{}, errors.NewOperatorError(fmt.Errorf("updating AccountInfo to remove finalizer: %w", err), true, true) | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
I don't think this behavior is correct. The AccountInfo finalizer can then only be removed if there are no other APIBindings with the platform-mesh finalizer. Not only relative to a particular APIExport
I agree on the problem, this was an oversight because I still have a hard time wrapping my head around how things get reconciled here. I'll try to implement what you're suggesting for now, but am still questioning whether this should be a controller reconcliling |
|
Superseded by #308 |
No description provided.