kafka-ops / julie

A solution to help you build automation and gitops in your Apache Kafka deployments. The Kafka gitops!
MIT License
421 stars 114 forks source link

ACLs are not removed from cluster with FileBackend #125

Closed akselh closed 3 years ago

akselh commented 4 years ago

I just found out that KTB is not deleting obsolete ACLs. I did a little bit of investigation and it seems that the problem was introduced in https://github.com/kafka-ops/kafka-topology-builder/pull/77. Not immediately sure how the problem should be fixed though...

1

These changes (was ClusterState, now in BackendController) seems to be one of the problems: https://github.com/kafka-ops/kafka-topology-builder/blob/1fcda5b84844d76daac21773f0060961047ed6e2/src/main/java/com/purbon/kafka/topology/BackendController.java#L55

The semantics of method createOrOpen for FileBackend is actually createOrOverwrite. :-) So no bindings is loaded from .cluster-state and all ACLs from topology are marked for creation (and never any for deletion).

FileWriter have to be initialized with append=true to not overwrite/delete the file contents. But changing to append=true have other undesired effects like not removing deleted ACLs from the file.

2

The bindings sent to buildUpdateBindingsActions should be the ones from .cluster-state not the bindings from the actions: https://github.com/kafka-ops/kafka-topology-builder/blob/1fcda5b84844d76daac21773f0060961047ed6e2/src/main/java/com/purbon/kafka/topology/AccessControlManager.java#L68

3

The handling of bindings in ExecutionPlan also looks a bit weird: https://github.com/kafka-ops/kafka-topology-builder/blob/1fcda5b84844d76daac21773f0060961047ed6e2/src/main/java/com/purbon/kafka/topology/ExecutionPlan.java#L70 Should maybe only add to bindings if !(action instanceof ClearBindings)?

akselh commented 4 years ago

This issue also makes running with --dryRun remove all contents from .cluster-state