Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bake: default variable to null if not defined #2530

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

crazy-max
Copy link
Member

@crazy-max crazy-max commented Jun 17, 2024

fixes #2529

With null val supported for variables in #1449, we should default to this type instead of empty string. Might have been an oversight.

Signed-off-by: CrazyMax <1951866+crazy-max@users.noreply.github.com>
@crazy-max crazy-max added this to the v0.16.0 milestone Jun 17, 2024
@crazy-max crazy-max marked this pull request as ready for review June 17, 2024 09:18
@tonistiigi
Copy link
Member

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
Copy link
Member Author

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: #491 (comment). I move this one to bake-ga milestone.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bake: empty block should equal null, not empty string
3 participants