haproxytech / haproxy-consul-connect

HaProxy Connector for Consul Connect. Enables Service Mesh with Consul and HaProxy using TLS and Consul Discovery
Apache License 2.0
95 stars 20 forks source link

Feat/version compare #56

Closed amelhusic closed 4 years ago

gdubicki commented 4 years ago

While we are here shouldn't we rather start checking for version X.*, not >= X.0 ? (<- updated to be more clear)

Not only theoretically major semver version uptick may cause issues but as issue #6 shows it also happens in practice.

amelhusic commented 4 years ago

Hi @gdubicki. Not sure what do you mean by X.*? HAProxy semver versions are in either major.minor or major.minor.patch format. Dataplaneapi follows major.minor.patch format.

Here we check minimum required versions.

gdubicki commented 4 years ago

I mean that I think that we should test if the major version is equal to expected, not equal or greater than.

amelhusic commented 4 years ago

Got it. @ShimmerGlass what do you think?

ShimmerGlass commented 4 years ago

I agree with @gdubicki, it's probably better to require a specific major version

amelhusic commented 4 years ago

It makes sense to me too. Updated.