-
Notifications
You must be signed in to change notification settings - Fork 464
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
fix: omit null defaultValue in bpmetadata #2048
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM except for a minor nit.
cli/bpmetadata/tfconfig.go
Outdated
if modVar.Default != nil { | ||
vl, err := structpb.NewValue(modVar.Default) | ||
if err == nil { | ||
v.DefaultValue = vl | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: can we make this a bit more left aligned?
if modVar.Default != nil { | |
vl, err := structpb.NewValue(modVar.Default) | |
if err == nil { | |
v.DefaultValue = vl | |
} | |
if modVar.Default == nil { | |
return v | |
} | |
vl, err := structpb.NewValue(modVar.Default) | |
if err == nil { | |
v.DefaultValue = vl | |
} | |
return v |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please update the version with a patch: https://github.com/rzy-goog/cloud-foundation-toolkit/blob/master/cli/Makefile#L4
Related to b/316180349
Currently, cft blueprint metadata -d appears to set the defaultValue to null if there's no default set in the Terraform file.
This breaks current terraform metadata parsing in g3
This change will omit defaultvalue from being added when null.