[Feature request] With dedup mode enabled, we should hash the key with template file content and the consul-template version

[chinmaygupta28]

https://github.com/hashicorp/consul-template/blob/870905de57f085588c3b718b779d8550aefc5dcf/manager/dedup.go#L436-L440

We were recently hitting this log in our service mesh where parsing of the template file was failing because there were different versions of consul-template running in services. Our consul cluster is shared among all the services with each having their own consul-template as sidecar, so during an upgrade process for one service some lower version (v0.24.0) was setting the data for the template_file_hash key while a newer version version (v0.25.0) in another service was trying to parse it but was getting returned in this if block.

Here the hash was same because this is an org wide common settings file for each microservice. We tried with the newer version as well, the case remains with v0.27.1 as well.

From a very brief understanding of the project, I can judge the rationale behind warning the user about mismatch of version as we could change the way we encode the data for a key in newer versions (let me know if there is any other reason), but do you think we should generate hash key with the "template_file content + consul-template version_number" so that this state is never reached at all, is there any downside to this approach?

[eikenb]

Hey @chinmaygupta28, thanks for the idea!

The main downside of that approach would be that it leaves data in consul as versions changed. Right now it re-uses the same KV pair over and over where with this change it would use new KV entries over time as versions update. There are ways to configure consul to expire these but that doesn't happen by default. It also might be possible to remove it with logic around the leader exiting, but I'm not 100% on that either.

I think it is probably doable, particularly if the "remove when last member/leader exits" works. If not then maybe add an option to the deduplication config section that turns it on (to make it explicit and have a place to document the issue).

That said... this probably won't make it into my queue for the foreseeable future because priorities, but I'll happily look at a PR for it.

[chinmaygupta28]

Hey @eikenb

I will take a stab at this approach in the coming week. A dirty workaround that is working for us right now is initialising the de-duplication config with consul-template version prefix as well, but yes that doesn't make up for the data left in the memory that you have mentioned.