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

Prevent validation on custom structs for nil values #147

Closed
simitt opened this issue Feb 3, 2020 · 3 comments · Fixed by #148
Closed

Prevent validation on custom structs for nil values #147

simitt opened this issue Feb 3, 2020 · 3 comments · Fixed by #148
Assignees

Comments

@simitt
Copy link

simitt commented Feb 3, 2020

According to https://github.com/elastic/go-ucfg/blob/master/validator.go#L213, the validation tag nonzero should only return an error if the configuration value is present but zero.
For custom types this is not always the case at the moment.

Eg. have following custom type and configuration raises an error even if the hosts field is not present:

type urls []*url.URL

type MyConfig struct {
  Hosts urls `config:"hosts" validate:"nonzero"`
}
@simitt simitt added the bug label Feb 3, 2020
@simitt simitt changed the title Prevent validation on custom structs for nil values Prevent nonzero validation on custom structs when value is not set Feb 3, 2020
@andresrc andresrc changed the title Prevent nonzero validation on custom structs when value is not set Prevent validation on custom structs for nil values Feb 3, 2020
simitt added a commit to simitt/apm-server that referenced this issue Feb 3, 2020
Validation `nonzero` is broken with latest beats update. Temporarily
implement validation instead of using validation tags until
elastic/go-ucfg#147 is fixed.
@urso
Copy link

urso commented Feb 3, 2020

Would it make sense to use required for your use-case?

@simitt
Copy link
Author

simitt commented Feb 3, 2020

Not really, the value is not required, but if given it is not allowed to be zero, basically exactly what I would expect from nonzero.
We have a workaround by doing a manual validation for now, but this seems to have been introduced with one of the latest changes, as we used the tag on the custom type already.

@blakerouse
Copy link
Contributor

@simitt You are correct, the issue is with the latest changes the nonzero is being called when the array is nil. Working on a fix so nonzero doesn't validate when the value is nil.

simitt added a commit to elastic/apm-server that referenced this issue Feb 4, 2020
Validation `nonzero` is broken with latest beats update. Temporarily
implement validation instead of using validation tags until
elastic/go-ucfg#147 is fixed.
simitt added a commit to simitt/apm-server that referenced this issue Mar 24, 2020
Validation `nonzero` is broken with latest beats update. Temporarily
implement validation instead of using validation tags until
elastic/go-ucfg#147 is fixed.

backports commit f604f27
simitt added a commit to elastic/apm-server that referenced this issue Mar 24, 2020
* Remove non-existing config option

* Tmp validation fix for self instrumentation cfg

Validation `nonzero` is broken with latest beats update. Temporarily
implement validation instead of using validation tags until
elastic/go-ucfg#147 is fixed.

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

Successfully merging a pull request may close this issue.

4 participants