-
Notifications
You must be signed in to change notification settings - Fork 64
Allow users to omit the optional verification hashes #9
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, couple of minor nits
func TestIngnitionFileAppendNoVerification(t *testing.T) { | ||
testIgnition(t, ` | ||
data "ignition_config" "test" { | ||
append { |
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.
minor nit: odd spacing here
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.
That one was copied down from the original tests :)
I've changed them all to tabs so they should look nice.
ignition/resource_ignition_file.go
Outdated
h, err := buildHash(d.Get("source.0.verification").(string)) | ||
if err != nil { | ||
return "", err | ||
verification := d.Get("source.0.verification").(string) |
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.
Can use the helper function GetOk()
here. ie:
if v, ok := d.GetOk("source.0.verification"); ok {
h, err := buildHash(v.(string))
if err != nil {
return "", err
}
hash = &h
}
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, thanks!
As described in the configuration spec
verification
is optional for files and configuration append/replace sources.This PR does only push the
Hash
back into the ignition configuration if the verification option was set in the terraform configuration. That way the config validation does not throwmalformed hash specifier
errors if one would omit the verification option.