kafka-ops / julie

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

Make allowDelete optional #156

Closed Fobhep closed 3 years ago

Fobhep commented 3 years ago

As I user I would like to have the option to block allowDelete via a config file setting. In addition it might be an idea to allow/forbid deletion via an environment variable

akselh commented 3 years ago

Is it deletion of topics you think is the main problem? I would like to have a split into allowDeleteTopics and allowDeleteAcls, ref #96

Fobhep commented 3 years ago

I support #96 as I think it is a good idea as well. However more generally speaking I really mean that for ANY deletion it should be possibly to block it.

purbon commented 3 years ago

I agree with you two, what is a nice feature of the lightbend config library is that you can override configuration properties from the environment.

I think we can implement this by having detailed config variables:

and others, I am thinking to keep the CLI options as primary option for now.

The question is what to do with the CLI options in 2.0, remove them ?

akselh commented 3 years ago

Yes indeed, introducing lightbend was a very nice thing!

Regarding this issue I agree that ability to override the two detailed configs with env variables should be enough.

However if you want to make your Kafka cluster really secure (which I think everyone should :-) ) I think the most important measures is how you set up the initial permissions at provisioning of the Kafka cluster. In case KTB should not do topic deletes the GitOps process should not have the topic DELETE ACL operation. Relying on this alone is however not the best, since KTB will fail badly if a topic is removed in the topology.

Unfortunately I do not think Kafka ACLs provide av way to enable create ACLs and not allow delete ACLs. since these operations are both given by the cluster ALTER acl operation.

akselh commented 3 years ago

Not sure if I understand your comment regarding 2.0 CLI options correct @purbon. :-)

But I think the CLI options should be kept, env variables should only be for overrides. The --allowDelete option should however be removed in 2.0 I think, will only be confusing once the two more detailed is introduced.

Fobhep commented 3 years ago

Agree to on that. Introducing env overwrites for 2 config values and removing the CLI flag sounds legit to me

purbon commented 3 years ago

Hi @akselh,

However if you want to make your Kafka cluster really secure (which I think everyone should :-) ) I think the most important measures is how you set up the initial permissions at provisioning of the Kafka cluster. In case KTB should not do topic deletes the GitOps process should not have the topic DELETE ACL operation. Relying on this alone is however not the best, since KTB will fail badly if a topic is removed in the topology.

I am confused, from the tool perspective the option to enable/disable deletion selectively I agree is powerful. However I am confused with:

Relying on this alone is however not the best, since KTB will fail badly if a topic is removed in the topology.

can you elaborate a bit more?

purbon commented 3 years ago

Then we have a plan.

Thanks you @Fobhep and @akselh for your comments.

purbon commented 3 years ago

Just as a thought

KTB will fail badly if a topic is removed in the topology.

if the topic is removed, no ACLs are created.

I wonder, where do you see this fail badly? can you help me understand?

akselh commented 3 years ago

Just as a thought

KTB will fail badly if a topic is removed in the topology.

if the topic is removed, no ACLs are created.

I wonder, where do you see this fail badly? can you help me understand?

If the Kafka user for KTB does not have the delete topic ACL permission then KTB will crash with an exception on applying topology changes where a topic is removed. Then some of the changes in the topology will have been applied to the cluster (i.e. new topics and sync topic) and some will not (changed bindings). However, adding the topic to the topology again and doing a re-run will fix the issue.

akselh commented 3 years ago

So not a big problem with KTB really, but with --allowTopicDelete option this will be handled properly.

purbon commented 3 years ago

related to #96 , thanks @akselh for bringing this up.