Browse Source

treewide: rework check_site_lib.lua

In addition to significant internal differences in check_site_lib.lua (in
particular unifying error handling to a single place for the upcoming
multi-domain support), this changes the way fields are addressed in site
check scripts: rather than providing a string like 'next_node.ip6', the
path is passed as an array {'next_node', 'ip6'}.

Other changes in site check scripts:
* need_array and need_table now pass the full path to the sub fields to the
subcheck instead of the key and value
* Any check referring to a field inside a table implies that all higher
levels must be tables if they exist: a check for {'next_node', 'ip6'} adds
an implicit (optional) check for {'next_node'}, which allows to remove many
explicit checks for such tables
Matthias Schiffer 6 years ago
parent
commit
7ccdacd294

+ 1 - 1
package/gluon-authorized-keys/check_site.lua

@@ -1 +1 @@
-need_string_array(in_site('authorized_keys'))
+need_string_array(in_site({'authorized_keys'}))

+ 8 - 12
package/gluon-autoupdater/check_site.lua

@@ -1,14 +1,10 @@
-need_string(in_site('autoupdater.branch'))
+need_string(in_site({'autoupdater', 'branch'}))
 
-local function check_branch(k, _)
-   assert_uci_name(k)
+need_table({'autoupdater', 'branches'}, function(branch)
+	need_alphanumeric_key(branch)
 
-   local prefix = string.format('autoupdater.branches[%q].', k)
-
-   need_string(in_site(prefix .. 'name'))
-   need_string_array_match(prefix .. 'mirrors', '^http://')
-   need_number(in_site(prefix .. 'good_signatures'))
-   need_string_array_match(in_site(prefix .. 'pubkeys'), '^%x+$')
-end
-
-need_table('autoupdater.branches', check_branch)
+	need_string(in_site(extend(branch, {'name'})))
+	need_string_array_match(extend(branch, {'mirrors'}), '^http://')
+	need_number(in_site(extend(branch, {'good_signatures'})))
+	need_string_array_match(in_site(extend(branch, {'pubkeys'})), '^%x+$')
+end)

+ 7 - 7
package/gluon-client-bridge/check_site.lua

@@ -1,15 +1,15 @@
-need_string_match(in_domain('next_node.mac'), '^%x[02468aAcCeE]:%x%x:%x%x:%x%x:%x%x:%x%x$', false)
+need_string_match(in_domain({'next_node', 'mac'}), '^%x[02468aAcCeE]:%x%x:%x%x:%x%x:%x%x:%x%x$', false)
 
-if need_string_match(in_domain('next_node.ip4'), '^%d+.%d+.%d+.%d+$', false) then
-	need_string_match(in_domain('prefix4'), '^%d+.%d+.%d+.%d+/%d+$')
+if need_string_match(in_domain({'next_node', 'ip4'}), '^%d+.%d+.%d+.%d+$', false) then
+	need_string_match(in_domain({'prefix4'}), '^%d+.%d+.%d+.%d+/%d+$')
 end
 
-need_string_match(in_domain('next_node.ip6'), '^[%x:]+$', false)
+need_string_match(in_domain({'next_node', 'ip6'}), '^[%x:]+$', false)
 
 
 for _, config in ipairs({'wifi24', 'wifi5'}) do
-	if need_table(config .. '.ap', nil, false) then
-		need_string(in_domain(config .. '.ap.ssid'))
-		need_boolean(config .. '.ap.disabled', false)
+	if need_table({config}, nil, false) then
+		need_string(in_domain({config, 'ap', 'ssid'}))
+		need_boolean({config, 'ap', 'disabled'}, false)
 	end
 end

+ 1 - 3
package/gluon-config-mode-contact-info/check_site.lua

@@ -1,3 +1 @@
-if need_table(in_site('config_mode'), nil, false) and need_table(in_site('config_mode.owner'), nil, false) then
-  need_boolean(in_site('config_mode.owner.obligatory'), false)
-end
+need_boolean(in_site({'config_mode', 'owner', 'obligatory'}), false)

+ 1 - 3
package/gluon-config-mode-geo-location/check_site.lua

@@ -1,3 +1 @@
-if need_table(in_site('config_mode'), nil, false) and need_table(in_site('config_mode.geo_location'), nil, false) then
-  need_boolean(in_site('config_mode.geo_location.show_altitude'), false)
-end
+need_boolean(in_site({'config_mode', 'geo_location', 'show_altitude'}), false)

+ 39 - 57
package/gluon-core/check_site.lua

@@ -1,75 +1,57 @@
-need_string(in_site('site_code'))
-need_string(in_site('site_name'))
-need_string_match(in_domain('domain_seed'), '^' .. ('%x'):rep(64) .. '$')
+need_string(in_site({'site_code'}))
+need_string(in_site({'site_name'}))
+need_string_match(in_domain({'domain_seed'}), '^' .. ('%x'):rep(64) .. '$')
 
-if need_table('opkg', nil, false) then
-	need_string('opkg.lede', false)
+need_string({'opkg', 'lede'}, false)
+need_table({'opkg', 'extra'}, function(extra_repo)
+	need_alphanumeric_key(extra_repo)
+	need_string(extra_repo)
+end, false)
 
-	function check_repo(k, _)
-		-- this is not actually a uci name, but using the same naming rules here is fine
-		assert_uci_name(k)
+need_string(in_site({'hostname_prefix'}), false)
+need_string(in_site({'timezone'}))
 
-		local path = string.format('opkg.extra[%q]', k)
-		need_string(path)
-	end
-
-	need_table('opkg.extra', check_repo, false)
-end
+need_string_array({'ntp_servers'}, false)
 
-need_string(in_site('hostname_prefix'), false)
-need_string(in_site('timezone'))
-
-need_string_array('ntp_servers', false)
-
-need_string_match(in_domain('prefix6'), '^[%x:]+/64$')
+need_string_match(in_domain({'prefix6'}), '^[%x:]+/64$')
 
 
 for _, config in ipairs({'wifi24', 'wifi5'}) do
-	if need_table(config, nil, false) then
-		need_string(in_site('regdom')) -- regdom is only required when wifi24 or wifi5 is configured
+	if need_table({config}, nil, false) then
+		need_string(in_site({'regdom'})) -- regdom is only required when wifi24 or wifi5 is configured
 
-		need_number(config .. '.channel')
+		need_number({config, 'channel'})
 
 		local rates = {1000, 2000, 5500, 6000, 9000, 11000, 12000, 18000, 24000, 36000, 48000, 54000}
-		local supported_rates = need_array_of(in_site(config .. '.supported_rates'), rates, false)
-		if supported_rates then
-			need_array_of(config .. '.basic_rate', supported_rates, true)
-		else
-			need_array_of(config .. '.basic_rate', rates, false)
+		local supported_rates = need_array_of(in_site({config, 'supported_rates'}), rates, false)
+		need_array_of({config, 'basic_rate'}, supported_rates or rates, supported_rates ~= nil)
+
+		if need_table({config, 'ibss'}, nil, false) then
+			need_string(in_domain({config, 'ibss', 'ssid'}))
+			need_string_match(in_domain({config, 'ibss', 'bssid'}), '^%x[02468aAcCeE]:%x%x:%x%x:%x%x:%x%x:%x%x$')
+			need_one_of({config, 'ibss', 'mcast_rate'}, supported_rates or rates, false)
+			need_number({config, 'ibss', 'vlan'}, false)
+			need_boolean({config, 'ibss', 'disabled'}, false)
+		end
+
+		if need_table({config, 'mesh'}, nil, false) then
+			need_string(in_domain({config, 'mesh', 'id'}))
+			need_one_of({config, 'mesh', 'mcast_rate'}, supported_rates or rates, false)
+			need_boolean({config, 'mesh', 'disabled'}, false)
 		end
 	end
 end
 
-need_boolean(in_site('poe_passthrough'), false)
-if need_table('dns', nil, false) then
-	need_number('dns.cacheentries', false)
-	need_string_array_match('dns.servers', '^[%x:]+$', true)
-end
+need_boolean(in_site({'poe_passthrough'}), false)
 
-if need_table('next_node', nil, false) then
-	need_string_match(in_domain('next_node.ip6'), '^[%x:]+$', false)
-	need_string_match(in_domain('next_node.ip4'), '^%d+.%d+.%d+.%d+$', false)
+if need_table({'dns'}, nil, false) then
+	need_string_array_match({'dns', 'servers'}, '^[%x:]+$')
+	need_number({'dns', 'cacheentries'}, false)
 end
 
-for _, config in ipairs({'wifi24', 'wifi5'}) do
-  local rates = {1000, 2000, 5500, 6000, 9000, 11000, 12000, 18000, 24000, 36000, 48000, 54000}
-  rates = need_array_of(in_site(config .. '.supported_rates'), rates, false) or rates
-
-  if need_table(config .. '.ibss', nil, false) then
-    need_string(in_domain(config .. '.ibss.ssid'))
-    need_string_match(in_domain(config .. '.ibss.bssid'), '^%x[02468aAcCeE]:%x%x:%x%x:%x%x:%x%x:%x%x$')
-    need_one_of(config .. '.ibss.mcast_rate', rates, false)
-    need_number(config .. '.ibss.vlan', false)
-    need_boolean(config .. '.ibss.disabled', false)
-  end
-
-  if need_table(config .. '.mesh', nil, false) then
-    need_string(in_domain(config .. '.mesh.id'))
-    need_one_of(config .. '.mesh.mcast_rate', rates, false)
-    need_boolean(config .. '.mesh.disabled', false)
-  end
-end
+need_string_match(in_domain({'next_node', 'ip6'}), '^[%x:]+$', false)
+need_string_match(in_domain({'next_node', 'ip4'}), '^%d+.%d+.%d+.%d+$', false)
 
-need_boolean(in_site('mesh_on_wan'), false)
-need_boolean(in_site('mesh_on_lan'), false)
-need_boolean(in_site('single_as_lan'), false)
+need_boolean(in_site({'mesh_on_wan'}), false)
+need_boolean(in_site({'mesh_on_lan'}), false)
+need_boolean(in_site({'single_as_lan'}), false)

+ 2 - 2
package/gluon-ebtables-source-filter/check_site.lua

@@ -1,2 +1,2 @@
-need_string_match(in_domain('prefix4'), '^%d+.%d+.%d+.%d+/%d+$', false)
-need_string_array_match(in_domain('extra_prefixes6'), '^[%x:]+/%d+$', false)
+need_string_match(in_domain({'prefix4'}), '^%d+.%d+.%d+.%d+/%d+$', false)
+need_string_array_match(in_domain({'extra_prefixes6'}), '^[%x:]+/%d+$', false)

+ 2 - 4
package/gluon-mesh-batman-adv/check_site.lua

@@ -1,4 +1,2 @@
-if need_table('mesh', nil, false) and  need_table('mesh.batman_adv', nil, false) then
-	need_number('mesh.batman_adv.gw_sel_class', false)
-	need_one_of('mesh.batman_adv.routing_algo', {'BATMAN_IV', 'BATMAN_V'}, false)
-end
+need_number({'mesh', 'batman_adv', 'gw_sel_class'}, false)
+need_one_of({'mesh', 'batman_adv', 'routing_algo'}, {'BATMAN_IV', 'BATMAN_V'}, false)

+ 5 - 7
package/gluon-mesh-vpn-core/check_site.lua

@@ -1,8 +1,6 @@
-need_boolean(in_site('mesh_vpn.enabled'), false)
-need_number('mesh_vpn.mtu')
+need_boolean(in_site({'mesh_vpn', 'enabled'}), false)
+need_number({'mesh_vpn', 'mtu'})
 
-if need_table(in_site('mesh_vpn.bandwidth_limit'), nil, false) then
-	need_boolean(in_site('mesh_vpn.bandwidth_limit.enabled'), false)
-	need_number(in_site('mesh_vpn.bandwidth_limit.ingress'), false)
-	need_number(in_site('mesh_vpn.bandwidth_limit.egress'), false)
-end
+need_boolean(in_site({'mesh_vpn', 'bandwidth_limit', 'enabled'}), false)
+need_number(in_site({'mesh_vpn', 'bandwidth_limit', 'ingress'}), false)
+need_number(in_site({'mesh_vpn', 'bandwidth_limit', 'egress'}), false)

+ 13 - 21
package/gluon-mesh-vpn-fastd/check_site.lua

@@ -1,30 +1,22 @@
 local fastd_methods = {'salsa2012+gmac', 'salsa2012+umac', 'null+salsa2012+gmac', 'null+salsa2012+umac', 'null'}
-need_array_of('mesh_vpn.fastd.methods', fastd_methods)
-need_boolean(in_site('mesh_vpn.fastd.configurable'), false)
+need_array_of({'mesh_vpn', 'fastd', 'methods'}, fastd_methods)
+need_boolean(in_site({'mesh_vpn', 'fastd', 'configurable'}), false)
 
-need_one_of(in_site('mesh_vpn.fastd.syslog_level'), {'error', 'warn', 'info', 'verbose', 'debug', 'debug2'}, false)
+need_one_of(in_site({'mesh_vpn', 'fastd', 'syslog_level'}), {'error', 'warn', 'info', 'verbose', 'debug', 'debug2'}, false)
 
-local function check_peer(prefix)
-	return function(k, _)
-		assert_uci_name(k)
+local function check_peer(k)
+	need_alphanumeric_key(k)
 
-		local table = string.format('%s[%q].', prefix, k)
-
-		need_string_match(in_domain(table .. 'key'), '^%x+$')
-		need_string_array(in_domain(table .. 'remotes'))
-	end
+	need_string_match(in_domain(extend(k, {'key'})), '^%x+$')
+	need_string_array(in_domain(extend(k, {'remotes'})))
 end
 
-local function check_group(prefix)
-	return function(k, _)
-		assert_uci_name(k)
-
-		local table = string.format('%s[%q].', prefix, k)
+local function check_group(k)
+	need_alphanumeric_key(k)
 
-		need_number(table .. 'limit', false)
-		need_table(table .. 'peers', check_peer(table .. 'peers'), false)
-		need_table(table .. 'groups', check_group(table .. 'groups'), false)
-	end
+	need_number(extend(k, {'limit'}), false)
+	need_table(extend(k, {'peers'}), check_peer, false)
+	need_table(extend(k, {'groups'}), check_group, false)
 end
 
-need_table('mesh_vpn.fastd.groups', check_group('mesh_vpn.fastd.groups'))
+need_table({'mesh_vpn', 'fastd', 'groups'}, check_group)

+ 1 - 1
package/gluon-mesh-vpn-tunneldigger/check_site.lua

@@ -1 +1 @@
-need_string_array('mesh_vpn.tunneldigger.brokers')
+need_string_array({'mesh_vpn', 'tunneldigger', 'brokers'})

+ 1 - 1
package/gluon-node-info/check_site.lua

@@ -1 +1 @@
-need_string(in_site('roles.default'), false)
+need_string(in_site({'roles', 'default'}), false)

+ 1 - 1
package/gluon-setup-mode/check_site.lua

@@ -1 +1 @@
-need_boolean(in_site('setup_mode.skip'), false)
+need_boolean(in_site({'setup_mode', 'skip'}), false)

+ 2 - 4
package/gluon-web-admin/check_site.lua

@@ -1,4 +1,2 @@
-if need_table(in_site('config_mode'), nil, false) and need_table(in_site('config_mode.remote_login'), nil, false) then
-  need_boolean(in_site('config_mode.remote_login.show_password_form'), false)
-  need_number(in_site('config_mode.remote_login.min_password_length'), false)
-end
+need_boolean(in_site({'config_mode', 'remote_login', 'show_password_form'}), false)
+need_number(in_site({'config_mode', 'remote_login', 'min_password_length'}), false)

+ 1 - 2
package/gluon-web-mesh-vpn-fastd/check_site.lua

@@ -1,2 +1 @@
-assert(need_boolean(in_site('mesh_vpn.fastd.configurable')) == true,
-       "site.conf error: expected `mesh_vpn.fastd.configurable' to be true")
+need_value(in_site({'mesh_vpn', 'fastd', 'configurable'}), true)

+ 2 - 2
package/gluon-web-node-role/check_site.lua

@@ -1,2 +1,2 @@
-need_string(in_site('roles.default'))
-need_string_array(in_site('roles.list'))
+need_string(in_site({'roles', 'default'}))
+need_string_array(in_site({'roles', 'list'}))

+ 112 - 104
scripts/check_site_lib.lua

@@ -1,160 +1,168 @@
 function in_site(var)
-   return var
+	return var
 end
 
 function in_domain(var)
-   return var
+	return var
 end
 
 
-local function loadvar(varname)
-   local ok, val = pcall(assert(loadstring('return site.' .. varname)))
-   if ok then
-      return val
-   else
-      return nil
-   end
+local function path_to_string(path)
+	return table.concat(path, '/')
 end
 
 local function array_to_string(array)
-   local string = ''
-   for _, v in ipairs(array) do
-      if #string >= 1 then
-         string = string .. ', '
-      end
-      string = string .. v
-   end
-   return '[' .. string .. ']'
+	return '[' .. table.concat(array, ', ') .. ']'
 end
 
-local function assert_one_of(var, array, msg)
-   for _, v in ipairs(array) do
-      if v == var then
-         return true
-      end
-   end
-
-   error(msg)
+local function var_error(path, val, msg)
+	print(string.format('*** site.conf error: expected %s to %s, but it is %s', path_to_string(path), msg, tostring(val)))
+	os.exit(1)
 end
 
-local function assert_type(var, t, msg)
-   assert(type(var) == t, msg)
-end
 
+function extend(path, c)
+	local p = {unpack(path)}
 
-function assert_uci_name(var)
-   -- We don't use character classes like %w here to be independent of the locale
-   assert(var:match('^[0-9a-zA-Z_]+$'), "site.conf error: `" .. var .. "' is not a valid config section name (only alphanumeric characters and the underscore are allowed)")
+	for _, e in ipairs(c) do
+		p[#p+1] = e
+	end
+	return p
 end
 
+local function loadpath(path, base, c, ...)
+	if not c or base == nil then
+		return base
+	end
 
-function need_string(varname, required)
-   local var = loadvar(varname)
+	if type(base) ~= 'table' then
+		var_error(path, base, 'be a table')
+	end
 
-   if required == false and var == nil then
-      return nil
-   end
-
-   assert_type(var, 'string', "site.conf error: expected `" .. varname .. "' to be a string")
-   return var
+	return loadpath(extend(path, {c}), base[c], ...)
 end
 
-function need_string_match(varname, pat, required)
-   local var = need_string(varname, required)
-
-   if not var then
-      return nil
-   end
-
-   assert(var:match(pat), "site.conf error: expected `" .. varname .. "' to match pattern `" .. pat .. "'")
+local function loadvar(path)
+	return loadpath({}, site, unpack(path))
+end
 
-   return var
+local function check_type(t)
+	return function(val)
+		return type(val) == t
+	end
 end
 
-function need_number(varname, required)
-   local var = loadvar(varname)
+local function check_one_of(array)
+	return function(val)
+		for _, v in ipairs(array) do
+			if v == val then
+				return true
+			end
+		end
+		return false
+	end
+end
 
-   if required == false and var == nil then
-      return nil
-   end
+function need(path, check, required, msg)
+	local val = loadvar(path)
+	if required == false and val == nil then
+		return nil
+	end
 
-   assert_type(var, 'number', "site.conf error: expected `" .. varname .. "' to be a number")
+	if not check(val) then
+		var_error(path, val, msg)
+	end
 
-   return var
+	return val
 end
 
-function need_boolean(varname, required)
-   local var = loadvar(varname)
-
-   if required == false and var == nil then
-      return nil
-   end
+local function need_type(path, type, required, msg)
+	return need(path, check_type(type), required, msg)
+end
 
-   assert_type(var, 'boolean', "site.conf error: expected `" .. varname .. "' to be a boolean")
 
-   return var
+function need_alphanumeric_key(path)
+	local val = path[#path]
+	-- We don't use character classes like %w here to be independent of the locale
+	if not val:match('^[0-9a-zA-Z_]+$') then
+		var_error(path, val, 'have a key using only alphanumeric characters and underscores')
+	end
 end
 
-function need_array(varname, subcheck, required)
-   local var = loadvar(varname)
 
-   if required == false and var == nil then
-      return nil
-   end
+function need_string(path, required)
+	return need_type(path, 'string', required, 'be a string')
+end
 
-   assert_type(var, 'table', "site.conf error: expected `" .. varname .. "' to be an array")
+function need_string_match(path, pat, required)
+	local val = need_string(path, required)
+	if not val then
+		return nil
+	end
 
-   for _, e in ipairs(var) do
-      subcheck(e)
-   end
+	if not val:match(pat) then
+		var_error(path, val, "match pattern '" .. pat .. "'")
+	end
 
-   return var
+	return val
 end
 
-function need_table(varname, subcheck, required)
-   local var = loadvar(varname)
+function need_number(path, required)
+	return need_type(path, 'number', required, 'be a number')
+end
 
-   if required == false and var == nil then
-      return nil
-   end
+function need_boolean(path, required)
+	return need_type(path, 'boolean', required, 'be a boolean')
+end
 
-   assert_type(var, 'table', "site.conf error: expected `" .. varname .. "' to be a table")
+function need_array(path, subcheck, required)
+	local val = need_type(path, 'table', required, 'be an array')
+	if not val then
+		return nil
+	end
 
-   if subcheck then
-      for k, v in pairs(var) do
-         subcheck(k, v)
-      end
-   end
+	if subcheck then
+		for i = 1, #val do
+			subcheck(extend(path, {i}))
+		end
+	end
 
-   return var
+	return val
 end
 
-function need_one_of(varname, array, required)
-   local var = loadvar(varname)
+function need_table(path, subcheck, required)
+	local val = need_type(path, 'table', required, 'be a table')
+	if not val then
+		return nil
+	end
 
-   if required == false and var == nil then
-      return nil
-   end
+	if subcheck then
+		for k, _ in pairs(val) do
+			subcheck(extend(path, {k}))
+		end
+	end
 
-   assert_one_of(var, array, "site.conf error: expected `" .. varname .. "' to be one of given array: " .. array_to_string(array))
+	return val
+end
+
+function need_value(path, value, required)
+	return need(path, function(v)
+		return v == value
+	end, required, 'be ' .. tostring(value))
+end
 
-   return var
+function need_one_of(path, array, required)
+	return need(path, check_one_of(array), required, 'be one of the given array ' .. array_to_string(array))
 end
 
-function need_string_array(varname, required)
-   local ok, var = pcall(need_array, varname, function(e) assert_type(e, 'string') end, required)
-   assert(ok, "site.conf error: expected `" .. varname .. "' to be a string array")
-   return var
+function need_string_array(path, required)
+	return need_array(path, need_string, required)
 end
 
-function need_string_array_match(varname, pat, required)
-   local ok, var = pcall(need_array, varname, function(e) assert(e:match(pat)) end, required)
-   assert(ok, "site.conf error: expected `" .. varname .. "' to be a string array matching pattern `" .. pat .. "'")
-   return var
+function need_string_array_match(path, pat, required)
+	return need_array(path, function(e) need_string_match(e, pat) end, required)
 end
 
-function need_array_of(varname, array, required)
-   local ok, var = pcall(need_array, varname, function(e) assert_one_of(e, array) end,required)
-   assert(ok, "site.conf error: expected `" .. varname .. "' to be a subset of given array: " .. array_to_string(array))
-   return var
+function need_array_of(path, array, required)
+	return need_array(path, function(e) need_one_of(e, array) end, required)
 end