search

docker / buildx

Docker CLI plugin for extended build capabilities with BuildKit
Apache License 2.0
3.33k stars 448 forks source link

bake: default variable to null if not defined #2530

Open crazy-max opened 2 weeks ago

crazy-max commented 2 weeks ago

fixes #2529

With null val supported for variables in https://github.com/docker/buildx/pull/1449, we should default to this type instead of empty string. Might have been an oversight.

tonistiigi commented 2 weeks ago

Although I agree that this is a better default, it is a breaking change and if there are users atm who don't define default and rely on it being empty string, this change would break their files either by giving a parse error or sending a different value to buildkit. So I'm not sure if this is worth the risk as currently users can define default = null themselves for same behavior. If we add smth like type in the future maybe we can decide a better default for the non-string variables?

crazy-max commented 2 weeks ago

Although I agree that this is a better default, it is a breaking change and if there are users atm who don't define default and rely on it being empty string, this change would break their files either by giving a parse error or sending a different value to buildkit. So I'm not sure if this is worth the risk as currently users can define default = null themselves for same behavior. If we add smth like type in the future maybe we can decide a better default for the non-string variables?

Agreed if we add some validation flavor to variables similar to what terraform does: https://github.com/docker/buildx/pull/491#issue-773693934. I move this one to bake-ga milestone.