Icinga DB: serialize icinga:config:checkcommand:argument#value and #set_if as expected #8725

Al2Klimov · 2021-04-19T14:23:17Z

I.e. keep the serializations as simple as possible:

null          => null
true          => true
42.0          => 42
"foobar"      => foobar
["foobar"]    => ["foobar"]
{"foo"="bar"} => {"foo":"bar"}
{{42}}        => Object of type 'Function'

Al2Klimov · 2021-04-19T14:25:33Z

Config

object IcingaDB "icingadb" {
	host = "127.0.0.1"
	port = 6379
}

object CheckCommand "nothing" {
	command = [ "echo" ]
	arguments = {
		"nothing" = { }
	}
}

object CheckCommand "null" {
	command = [ "echo" ]
	arguments = {
		"null" = { set_if = null; value = set_if }
	}
	vars.x = null
}

object CheckCommand "true" {
	command = [ "echo" ]
	arguments = {
		"true" = { set_if = true; value = set_if }
	}
	vars.x = true
}

object CheckCommand "1" {
	command = [ "echo" ]
	arguments = {
		"1" = { set_if = 1; value = set_if }
	}
	vars.x = 1
}

object CheckCommand "s" {
	command = [ "echo" ]
	arguments = {
		"s" = { set_if = "s"; value = set_if }
	}
	vars.x = "s"
}

object CheckCommand "[]" {
	command = [ "echo" ]
	arguments = {
	}
	vars.x = []
}

object CheckCommand "{}" {
	command = [ "echo" ]
	arguments = {
	}
	vars.x = {}
}

object CheckCommand "f" {
	command = [ "echo" ]
	arguments = {
		"f" = { set_if = {{ null }}; value = set_if }
	}
	vars.x = {{ null }}
}

Before

$ redis-cli hgetall icinga:config:checkcommand:argument
 1) "e4ada320596512c4efcad3190399eb5679249995"
 2) "{\"argument_key\":\"f\",\"command_id\":\"1a424a736ad37a4c6481832baf8af16bb9686407\",\"environment_id\":\"da39a3ee5e6b4b0d3255bfef95601890afd80709\",\"set_if\":null,\"value\":\"null\"}"
 3) "642b3f6ae99f2b2f00fd802fd933e7865e95876d"
 4) "{\"argument_key\":\"s\",\"command_id\":\"ed9670f52ceb14d997dd9628adb232df4ef311bf\",\"environment_id\":\"da39a3ee5e6b4b0d3255bfef95601890afd80709\",\"set_if\":\"s\",\"value\":\"\\\"s\\\"\"}"
 5) "569ce001e0adfbb4247e9b166e4339b1354c21ff"
 6) "{\"argument_key\":\"null\",\"command_id\":\"6231268d8c8296fa6c3155e26fd84aa81a89c882\",\"environment_id\":\"da39a3ee5e6b4b0d3255bfef95601890afd80709\",\"set_if\":null,\"value\":\"null\"}"
 7) "1cf1ea6bfcd8240bf3f1aa807c531f91ffb6e736"
 8) "{\"argument_key\":\"true\",\"command_id\":\"ded67d48b4138447bc67fe7d2099aa4d1e2bb1a3\",\"environment_id\":\"da39a3ee5e6b4b0d3255bfef95601890afd80709\",\"set_if\":true,\"value\":\"true\"}"
 9) "1098437a22a87b8d2a4f6461ea9ce77a91d41a10"
10) "{\"argument_key\":\"1\",\"command_id\":\"6307defdb2969b32cfa859da58df40a93bea8a2f\",\"environment_id\":\"da39a3ee5e6b4b0d3255bfef95601890afd80709\",\"set_if\":1,\"value\":\"1\"}"
11) "4625fc63fc50e6b10698e7a2c4016da9ef0e3803"
12) "{\"argument_key\":\"nothing\",\"command_id\":\"2e79d44ecfd742e864e1e0fde55d3fbd0dc62b3f\",\"environment_id\":\"da39a3ee5e6b4b0d3255bfef95601890afd80709\"}"
$ redis-cli hgetall icinga:config:customvar
 1) "83c28696319eff62e7944107ba468f8e69d66b1f"
 2) "{\"environment_id\":\"da39a3ee5e6b4b0d3255bfef95601890afd80709\",\"name\":\"x\",\"name_checksum\":\"11f6ad8ec52a2984abaafd7c3b516503785c2072\",\"value\":\"[]\"}"
 3) "e04e078eb0bb1aa784ac54caa4d485e19f8d545c"
 4) "{\"environment_id\":\"da39a3ee5e6b4b0d3255bfef95601890afd80709\",\"name\":\"x\",\"name_checksum\":\"11f6ad8ec52a2984abaafd7c3b516503785c2072\",\"value\":\"{}\"}"
 5) "4adcbef3e62dc345760c11f7ad1357dafbc243c5"
 6) "{\"environment_id\":\"da39a3ee5e6b4b0d3255bfef95601890afd80709\",\"name\":\"x\",\"name_checksum\":\"11f6ad8ec52a2984abaafd7c3b516503785c2072\",\"value\":\"null\"}"
 7) "6545a6d591ba0b2f62147173c9cbd36c09e8e4bb"
 8) "{\"environment_id\":\"da39a3ee5e6b4b0d3255bfef95601890afd80709\",\"name\":\"x\",\"name_checksum\":\"11f6ad8ec52a2984abaafd7c3b516503785c2072\",\"value\":\"\\\"s\\\"\"}"
 9) "5a9387e879ab6418759b13676a4c14bf697b974b"
10) "{\"environment_id\":\"da39a3ee5e6b4b0d3255bfef95601890afd80709\",\"name\":\"x\",\"name_checksum\":\"11f6ad8ec52a2984abaafd7c3b516503785c2072\",\"value\":\"1\"}"
11) "716b12a7e3e780f9b1a8c56c5b52d978089259e7"
12) "{\"environment_id\":\"da39a3ee5e6b4b0d3255bfef95601890afd80709\",\"name\":\"x\",\"name_checksum\":\"11f6ad8ec52a2984abaafd7c3b516503785c2072\",\"value\":\"true\"}"
$

After

$ redis-cli hgetall icinga:config:checkcommand:argument
 1) "e4ada320596512c4efcad3190399eb5679249995"
 2) "{\"argument_key\":\"f\",\"command_id\":\"1a424a736ad37a4c6481832baf8af16bb9686407\",\"environment_id\":\"da39a3ee5e6b4b0d3255bfef95601890afd80709\",\"set_if\":\"Object of type 'Function'\",\"value\":\"Object of type 'Function'\"}"
 3) "642b3f6ae99f2b2f00fd802fd933e7865e95876d"
 4) "{\"argument_key\":\"s\",\"command_id\":\"ed9670f52ceb14d997dd9628adb232df4ef311bf\",\"environment_id\":\"da39a3ee5e6b4b0d3255bfef95601890afd80709\",\"set_if\":\"s\",\"value\":\"s\"}"
 5) "569ce001e0adfbb4247e9b166e4339b1354c21ff"
 6) "{\"argument_key\":\"null\",\"command_id\":\"6231268d8c8296fa6c3155e26fd84aa81a89c882\",\"environment_id\":\"da39a3ee5e6b4b0d3255bfef95601890afd80709\",\"set_if\":\"null\",\"value\":\"null\"}"
 7) "1cf1ea6bfcd8240bf3f1aa807c531f91ffb6e736"
 8) "{\"argument_key\":\"true\",\"command_id\":\"ded67d48b4138447bc67fe7d2099aa4d1e2bb1a3\",\"environment_id\":\"da39a3ee5e6b4b0d3255bfef95601890afd80709\",\"set_if\":\"true\",\"value\":\"true\"}"
 9) "1098437a22a87b8d2a4f6461ea9ce77a91d41a10"
10) "{\"argument_key\":\"1\",\"command_id\":\"6307defdb2969b32cfa859da58df40a93bea8a2f\",\"environment_id\":\"da39a3ee5e6b4b0d3255bfef95601890afd80709\",\"set_if\":\"1\",\"value\":\"1\"}"
11) "4625fc63fc50e6b10698e7a2c4016da9ef0e3803"
12) "{\"argument_key\":\"nothing\",\"command_id\":\"2e79d44ecfd742e864e1e0fde55d3fbd0dc62b3f\",\"environment_id\":\"da39a3ee5e6b4b0d3255bfef95601890afd80709\"}"
$ redis-cli hgetall icinga:config:customvar
 1) "e04e078eb0bb1aa784ac54caa4d485e19f8d545c"
 2) "{\"environment_id\":\"da39a3ee5e6b4b0d3255bfef95601890afd80709\",\"name\":\"x\",\"name_checksum\":\"11f6ad8ec52a2984abaafd7c3b516503785c2072\",\"value\":\"{}\"}"
 3) "83c28696319eff62e7944107ba468f8e69d66b1f"
 4) "{\"environment_id\":\"da39a3ee5e6b4b0d3255bfef95601890afd80709\",\"name\":\"x\",\"name_checksum\":\"11f6ad8ec52a2984abaafd7c3b516503785c2072\",\"value\":\"[]\"}"
 5) "4adcbef3e62dc345760c11f7ad1357dafbc243c5"
 6) "{\"environment_id\":\"da39a3ee5e6b4b0d3255bfef95601890afd80709\",\"name\":\"x\",\"name_checksum\":\"11f6ad8ec52a2984abaafd7c3b516503785c2072\",\"value\":\"\\\"Object of type 'Function'\\\"\"}"
 7) "6545a6d591ba0b2f62147173c9cbd36c09e8e4bb"
 8) "{\"environment_id\":\"da39a3ee5e6b4b0d3255bfef95601890afd80709\",\"name\":\"x\",\"name_checksum\":\"11f6ad8ec52a2984abaafd7c3b516503785c2072\",\"value\":\"\\\"s\\\"\"}"
 9) "5a9387e879ab6418759b13676a4c14bf697b974b"
10) "{\"environment_id\":\"da39a3ee5e6b4b0d3255bfef95601890afd80709\",\"name\":\"x\",\"name_checksum\":\"11f6ad8ec52a2984abaafd7c3b516503785c2072\",\"value\":\"1\"}"
11) "716b12a7e3e780f9b1a8c56c5b52d978089259e7"
12) "{\"environment_id\":\"da39a3ee5e6b4b0d3255bfef95601890afd80709\",\"name\":\"x\",\"name_checksum\":\"11f6ad8ec52a2984abaafd7c3b516503785c2072\",\"value\":\"true\"}"
$

lib/icingadb/icingadb-objects.cpp

julianbrost

This PR still changes the behavior of JsonEncode, even though this change should no longer have an effect on the two attributes mentioned in the PR title. Do we still want this change?

julianbrost · 2021-04-30T12:43:35Z

lib/base/json.cpp

-			stateMachine.Null();
-
-			break;
+				// obj is most likely a function => "Object of type 'Function'"


This is not about likeliness but about what you put in. I'd just say "other object types are encoded as their string representation, for example "Object of type 'Function'"

According to Eric it is.

lib/icingadb/icingadb-objects.cpp

…et_if as expected I.e. keep the serializations as simple as possible: null => null true => true 42.0 => 42 "foobar" => foobar {{42}} => Object of type 'Function' (["foobar"] and {"foo"="bar"} can't occur there.)

lippserd · 2021-06-22T11:43:56Z

Can you please explain exactly what changes with this PR? A real JSON representation for this would be helpful.

Al2Klimov · 2021-06-24T16:07:58Z

In Icinga 2 command args' value and set_if may be various values (examples listed left). However in Icinga DB they’re strings and must represent the origin values somehow (examples listed right).

Al2Klimov added bug Something isn't working area/icingadb New backend labels Apr 19, 2021

icinga-probot bot added this to the 2.13.0 milestone Apr 19, 2021

Al2Klimov marked this pull request as ready for review April 19, 2021 14:26

Al2Klimov requested a review from lippserd April 19, 2021 14:26

Al2Klimov force-pushed the bugfix/icingadb-serialize-leaves branch 2 times, most recently from f0cf3c1 to 737e535 Compare April 20, 2021 10:14

julianbrost self-requested a review April 28, 2021 14:36

julianbrost reviewed Apr 28, 2021

View reviewed changes

lib/icingadb/icingadb-objects.cpp Outdated Show resolved Hide resolved

Al2Klimov requested a review from julianbrost April 28, 2021 15:13

julianbrost reviewed Apr 29, 2021

View reviewed changes

lib/icingadb/icingadb-objects.cpp Show resolved Hide resolved

Al2Klimov requested review from julianbrost and removed request for julianbrost and lippserd April 29, 2021 08:59

Al2Klimov self-assigned this Apr 29, 2021

Al2Klimov force-pushed the bugfix/icingadb-serialize-leaves branch from 737e535 to 81204c5 Compare April 29, 2021 09:17

Al2Klimov removed their assignment Apr 29, 2021

Al2Klimov requested review from lippserd and removed request for lippserd April 29, 2021 09:17

Al2Klimov self-assigned this Apr 30, 2021

Al2Klimov force-pushed the bugfix/icingadb-serialize-leaves branch from 81204c5 to fdd4726 Compare April 30, 2021 09:53

Al2Klimov removed their assignment Apr 30, 2021

Al2Klimov requested a review from lippserd April 30, 2021 09:53

julianbrost requested changes Apr 30, 2021

View reviewed changes

Al2Klimov force-pushed the bugfix/icingadb-serialize-leaves branch from fdd4726 to f5abec2 Compare April 30, 2021 13:48

julianbrost self-requested a review June 21, 2021 15:17

lippserd assigned Al2Klimov Jun 22, 2021

lippserd added the needs feedback We'll only proceed once we hear from you again label Jun 22, 2021

julianbrost approved these changes Jun 29, 2021

View reviewed changes

lippserd approved these changes Jun 29, 2021

View reviewed changes

Al2Klimov merged commit 00af435 into master Jun 29, 2021

icinga-probot bot deleted the bugfix/icingadb-serialize-leaves branch June 29, 2021 15:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Icinga DB: serialize icinga:config:checkcommand:argument#value and #set_if as expected #8725

Icinga DB: serialize icinga:config:checkcommand:argument#value and #set_if as expected #8725

Al2Klimov commented Apr 19, 2021 •

edited

Al2Klimov commented Apr 19, 2021

julianbrost left a comment

julianbrost Apr 30, 2021

Al2Klimov Apr 30, 2021

lippserd commented Jun 22, 2021

Al2Klimov commented Jun 24, 2021

Icinga DB: serialize icinga:config:checkcommand:argument#value and #set_if as expected #8725

Icinga DB: serialize icinga:config:checkcommand:argument#value and #set_if as expected #8725

Conversation

Al2Klimov commented Apr 19, 2021 • edited

Al2Klimov commented Apr 19, 2021

Config

Before

After

julianbrost left a comment

Choose a reason for hiding this comment

julianbrost Apr 30, 2021

Choose a reason for hiding this comment

Al2Klimov Apr 30, 2021

Choose a reason for hiding this comment

lippserd commented Jun 22, 2021

Al2Klimov commented Jun 24, 2021

Al2Klimov commented Apr 19, 2021 •

edited