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

Icinga DB feature: normalize *Command.arguments[*].{required,skip_key… #9772

Conversation

Al2Klimov
Copy link
Member

@Al2Klimov Al2Klimov commented May 24, 2023

…,repeat_key} to boolean

At the moment, the Icinga DB feature will use that value as-is and serialize it to JSON, resulting in a crash in Icinga DB down the road because it expects a boolean.

fixes #9576
closes #9771

TODO

@Al2Klimov Al2Klimov added this to the 2.14.0 milestone May 24, 2023
@cla-bot cla-bot bot added the cla/signed label May 24, 2023
@icinga-probot icinga-probot bot added area/icingadb New backend bug Something isn't working consider backporting Should be considered for inclusion in a bugfix release labels May 24, 2023
@Al2Klimov Al2Klimov requested a review from julianbrost May 24, 2023 13:50
@Al2Klimov
Copy link
Member Author

ok?

…,repeat_key} to boolean

At the moment, the Icinga DB feature will use that value as-is and
serialize it to JSON, resulting in a crash in Icinga DB down the road
because it expects a boolean.
@Al2Klimov Al2Klimov force-pushed the icinga-db-feature-should-normalize-command-arguments-required-skip_key-repeat_key-to-boolean-9576 branch from f2657b4 to ad618e9 Compare May 24, 2023 14:04
@julianbrost
Copy link
Contributor

This looks like the change I was expecting for that issue. So you can go ahead here.

(I wouldn't be opposed to a change like in #9771 if it warned instead of errored out, that could be a useful warning, but as that touches validation code, this would require looking into if/how other booleans would be affected by the change as well.)

@Al2Klimov
Copy link
Member Author

Before

2023-05-24T16:01:17.591+0200	WARN	config-sync	Aborted config sync after 713.564505ms
2023-05-24T16:01:17.591+0200	FATAL	icingadb	json: cannot unmarshal number into Go value of type bool
can't unmarshal JSON into *bool
github.com/icinga/icingadb/internal.UnmarshalJSON
	/Users/aklimov/NET/WS/icingadb/internal/internal.go:47
github.com/icinga/icingadb/pkg/types.(*Bool).UnmarshalJSON
	/Users/aklimov/NET/WS/icingadb/pkg/types/bool.go:52
encoding/json.(*decodeState).literalStore
	/usr/local/Cellar/go/1.20.4/libexec/src/encoding/json/decode.go:872
encoding/json.(*decodeState).value
	/usr/local/Cellar/go/1.20.4/libexec/src/encoding/json/decode.go:388
encoding/json.(*decodeState).object
	/usr/local/Cellar/go/1.20.4/libexec/src/encoding/json/decode.go:775
encoding/json.(*decodeState).value
	/usr/local/Cellar/go/1.20.4/libexec/src/encoding/json/decode.go:374
encoding/json.(*decodeState).unmarshal
	/usr/local/Cellar/go/1.20.4/libexec/src/encoding/json/decode.go:181
encoding/json.Unmarshal
	/usr/local/Cellar/go/1.20.4/libexec/src/encoding/json/decode.go:108
github.com/icinga/icingadb/internal.UnmarshalJSON
	/Users/aklimov/NET/WS/icingadb/internal/internal.go:47
github.com/icinga/icingadb/pkg/icingaredis.CreateEntities.func1.1
	/Users/aklimov/NET/WS/icingadb/pkg/icingaredis/utils.go:56
golang.org/x/sync/errgroup.(*Group).Go.func1
	/Users/aklimov/go/pkg/mod/golang.org/x/[email protected]/errgroup/errgroup.go:57
runtime.goexit
	/usr/local/Cellar/go/1.20.4/libexec/src/runtime/asm_amd64.s:1598
can't unmarshal JSON into *v1.CheckcommandArgument
github.com/icinga/icingadb/internal.UnmarshalJSON
	/Users/aklimov/NET/WS/icingadb/internal/internal.go:47
github.com/icinga/icingadb/pkg/icingaredis.CreateEntities.func1.1
	/Users/aklimov/NET/WS/icingadb/pkg/icingaredis/utils.go:56
golang.org/x/sync/errgroup.(*Group).Go.func1
	/Users/aklimov/go/pkg/mod/golang.org/x/[email protected]/errgroup/errgroup.go:57
runtime.goexit
	/usr/local/Cellar/go/1.20.4/libexec/src/runtime/asm_amd64.s:1598

After

2023-05-24T16:05:26.054+0200	INFO	config-sync	Finished config sync in 113.349185ms

@julianbrost julianbrost merged commit d871c5c into master May 25, 2023
@icinga-probot icinga-probot bot deleted the icinga-db-feature-should-normalize-command-arguments-required-skip_key-repeat_key-to-boolean-9576 branch May 25, 2023 09:54
@julianbrost julianbrost added backported Fix was included in a bugfix release and removed consider backporting Should be considered for inclusion in a bugfix release labels Feb 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/icingadb New backend backported Fix was included in a bugfix release bug Something isn't working cla/signed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Icinga DB feature should normalize *command.arguments[*].{required,skip_key,repeat_key}` to boolean
2 participants