search

konpyutaika / nifikop

The NiFiKop NiFi Kubernetes operator makes it easy to run Apache NiFi on Kubernetes. Apache NiFI is a free, open-source solution that support powerful and scalable directed graphs of data routing, transformation, and system mediation logic.
https://konpyutaika.github.io/nifikop/
Apache License 2.0
122 stars 40 forks source link

Added bootstrap-gcp.conf, bootstrap-aws.conf, bootstrap-azure.conf and bootstrap-hashicorp-vault.conf files management #361

Open juldrixx opened 5 months ago

juldrixx commented 5 months ago
Q A
Bug fix? no
New feature? yes
API breaks? no
Deprecations? no
Related tickets Partial https://github.com/konpyutaika/nifikop/issues/360
License Apache 2.0

What's in this PR?

Added the management of bootstrap-gcp.conf, bootstrap-aws.conf, bootstrap-azure.conf and bootstrap-hashicorp-vault.conf files.

Why?

NiFiKop already manages all the other configuration files, so it should manages them.

Checklist

mh013370 commented 5 months ago

I believe these would all need exposed in the nifi-cluster helm chart as we do with the logback configuration. Except in these cases, i don't think we need to provide an override in the chart :)

juldrixx commented 5 months ago

And should I renamed to BoostrapXConf instead of BoostrapXProperties?

mh013370 commented 5 months ago

And should I renamed to BoostrapXConf instead of BoostrapXProperties?

I feel like the properties ought to be named consistently, even though I think we've named them incorrectly. The actual files are .conf, but in the CRDs and helm charts we named them properties. What do you think?

juldrixx commented 5 months ago

And should I renamed to BoostrapXConf instead of BoostrapXProperties?

I feel like the properties ought to be named consistently, even though I think we've named them incorrectly. The actual files are .conf, but in the CRDs and helm charts we named them properties. What do you think?

In https://github.com/konpyutaika/nifikop/pull/363, I remplace some of BoostrapProperties to BoostrapConf, was it a good idea?

juldrixx commented 5 months ago

And I had an idea, instead of adding 4 new fields with the same configuration, could it be interesting to add 1 field that is an array where you provide the 3 fields + the filename? So that in the futur, if new files are added the user could override them without NiFiKop update. At least, until we need it and made an official field.

mh013370 commented 5 months ago

And should I renamed to BoostrapXConf instead of BoostrapXProperties?

I feel like the properties ought to be named consistently, even though I think we've named them incorrectly. The actual files are .conf, but in the CRDs and helm charts we named them properties. What do you think?

In #363, I remplace some of BoostrapProperties to BoostrapConf, was it a good idea?

I think that is fine because you didn't change the CRDs, so there weren't any breaking changes. It was all internal to the helm chart and documentation. We should double check though. We should avoid introducing any CRD breaking changes IMO

mh013370 commented 5 months ago

And I had an idea, instead of adding 4 new fields with the same configuration, could it be interesting to add 1 field that is an array where you provide the 3 fields + the filename? So that in the futur, if new files are added the user could override them without NiFiKop update. At least, until we need it and made an official field.

Not a bad idea - perhaps this could be part of the v2alpha1/NifiCluster spec? We could introduce ConfigurationOverrides that all get mapped into NiFi's conf/ directory. Yeah?

One potential downside is that with it being so open/flexible, it might not be clear what you can override with it. So with the explicit options, it's clear what things you can override in NiFi