Skip to content

lints: implement json.Unmarshaler for LintStatus.#297

Merged
zakird merged 1 commit intozmap:masterfrom
cpu:cpu-unmarshal-lintstatus
Jul 30, 2019
Merged

lints: implement json.Unmarshaler for LintStatus.#297
zakird merged 1 commit intozmap:masterfrom
cpu:cpu-unmarshal-lintstatus

Conversation

@cpu
Copy link
Copy Markdown
Member

@cpu cpu commented Jul 30, 2019

The lints.LintStatus type implements json.Marshaler to marshal to a human readable string (e.g. "error" instead of 6). Without providing a compatible json.Unmarshaler implementation downstream users that marshal a lints.LintStatus to JSON will encounter an error when they try to unmarshal it due to a mismatch of types (int vs string).

Notably this is causing me problems because when I added pre-issuance linting to CFSSL I made the configuration value on the cfssl.config.SigningProfile type a zlint.lints.LintStatus after a suggestion by the maintainers. That seemed fine at the time but I later discovered in Boulder we expect to be able to round-trip our CFSSL config to/from JSON without error.

Without the changes in this branch the de-serialization of the CFSSL config fails with an error like:

Failed to create CA: {"code":5200,"message":"failed to unmarshal configuration: json: cannot unmarshal string into Go struct field SigningProfile.lint_error_level of type lints.LintStatus"}

The `lints.LintStatus` type implements `json.Marshaler` to marshal to
a human readable string (e.g. `"error"` instead of `6`). Without
providing a compatible `json.Unmarshaler` implementation downstream
users that marshal a `lints.LintStatus` to JSON will encounter an error
when they try to unmarshal it due to a mixmatch of types (`int` vs
`string`).
@zakird zakird merged commit 9971d62 into zmap:master Jul 30, 2019
@cpu cpu deleted the cpu-unmarshal-lintstatus branch July 30, 2019 22:08
@cpu
Copy link
Copy Markdown
Member Author

cpu commented Jul 30, 2019

@zakird Thanks for the quick 🔍

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants