From 6f769f306db3837471a1413de6175e3c5a831803 Mon Sep 17 00:00:00 2001 From: Kapil Bansal Date: Wed, 24 Nov 2021 18:38:16 +0530 Subject: [PATCH 1/5] [refactor] Use stored dir for storing configuration --- openwisp-config/files/sbin/openwisp-update-config.lua | 4 ++-- openwisp-config/tests/test_update_config.lua | 9 +++++---- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/openwisp-config/files/sbin/openwisp-update-config.lua b/openwisp-config/files/sbin/openwisp-update-config.lua index 2df5e34..e7ba722 100755 --- a/openwisp-config/files/sbin/openwisp-update-config.lua +++ b/openwisp-config/files/sbin/openwisp-update-config.lua @@ -32,7 +32,7 @@ local test_root_dir = working_dir .. '/update-test' local remote_dir = openwisp_dir .. '/remote' local remote_config_dir = remote_dir .. '/etc/config' local stored_dir = openwisp_dir .. '/stored' -local stored_config_dir = openwisp_dir .. '/etc/config' +local stored_config_dir = stored_dir .. '/etc/config' local added_file = openwisp_dir .. '/added.list' local modified_file = openwisp_dir .. '/modified.list' local get_standard = function() return uci.cursor(standard_config_dir) end @@ -84,8 +84,8 @@ if lfs.attributes(remote_config_dir, 'mode') == 'directory' then if section_stored == nil then utils.remove_uci_options(standard, file, section) -- section is in the backup configuration -> restore - -- delete all options first else + -- delete all options first for option, value in pairs(section) do if not utils.starts_with_dot(option) then standard:delete(file, section['.name'], option) diff --git a/openwisp-config/tests/test_update_config.lua b/openwisp-config/tests/test_update_config.lua index 051a6bf..6265f8f 100644 --- a/openwisp-config/tests/test_update_config.lua +++ b/openwisp-config/tests/test_update_config.lua @@ -7,6 +7,7 @@ local update_config = assert(loadfile("../files/sbin/openwisp-update-config.lua" local write_dir = './update-test/' local config_dir = write_dir .. 'etc/config/' local openwisp_dir = './openwisp/' +local stored_dir = openwisp_dir .. '/stored/' local remote_config_dir = openwisp_dir .. 'remote/etc/config' local function string_count(base, pattern) @@ -38,8 +39,8 @@ TestUpdateConfig = { os.execute('echo restore-me > '..openwisp_dir..'/stored/etc/restore-me') os.execute('echo /etc/restore-me > '..openwisp_dir..'/modified.list') -- this file is stored in the backup - os.execute('mkdir -p ' .. openwisp_dir ..'etc/config/') - os.execute("cp ./update/stored_wireless " ..openwisp_dir.. '/etc/config/wireless') + os.execute('mkdir -p ' .. stored_dir ..'etc/config/') + os.execute("cp ./update/stored_wireless " ..stored_dir.. '/etc/config/wireless') end, tearDown = function() os.execute('rm -rf ' .. write_dir) @@ -123,12 +124,12 @@ function TestUpdateConfig.test_update() luaunit.assertEquals(restoreFile:read('*all'), 'restore-me\n') luaunit.assertNil(io.open(openwisp_dir..'/stored/etc/restore-me')) -- ensure network configuration file is backed up - local storedNetworkFile = io.open(openwisp_dir .. '/etc/config/network') + local storedNetworkFile = io.open(stored_dir .. '/etc/config/network') luaunit.assertNotNil(storedNetworkFile) local initialNetworkFile = io.open('update/network') luaunit.assertEquals(storedNetworkFile:read('*all'), initialNetworkFile:read('*all')) -- ensure system configuration file is backed up - local storedSystemFile = io.open(openwisp_dir .. '/etc/config/system') + local storedSystemFile = io.open(stored_dir .. '/etc/config/system') luaunit.assertNotNil(storedSystemFile) local initialSystemFile = io.open('update/system') luaunit.assertEquals(storedSystemFile:read('*all'), initialSystemFile:read('*all')) From 09e8c249f37882ac7c38b04bb6b6d6dda90b6450 Mon Sep 17 00:00:00 2001 From: Kapil Bansal Date: Thu, 25 Nov 2021 03:01:33 +0530 Subject: [PATCH 2/5] [fix] Fix configuration files not getting removed #153 Closes #153 --- .../files/sbin/openwisp-update-config.lua | 2 +- openwisp-config/tests/test_update_config.lua | 13 ++++--------- 2 files changed, 5 insertions(+), 10 deletions(-) diff --git a/openwisp-config/files/sbin/openwisp-update-config.lua b/openwisp-config/files/sbin/openwisp-update-config.lua index e7ba722..d5deaa4 100755 --- a/openwisp-config/files/sbin/openwisp-update-config.lua +++ b/openwisp-config/files/sbin/openwisp-update-config.lua @@ -148,7 +148,7 @@ if lfs.attributes(remote_config_dir, 'mode') == 'directory' then -- ensure we are acting on a file if lfs.attributes(remote_path, 'mode') == 'file' then -- if there's no backup of the file yet, create one - if not utils.file_exists(stored_path) then + if (not utils.file_exists(stored_path) and not utils.file_exists(remote_path)) then os.execute('cp '..standard_path..' '..stored_path) end -- MERGE mode diff --git a/openwisp-config/tests/test_update_config.lua b/openwisp-config/tests/test_update_config.lua index 6265f8f..b0af434 100644 --- a/openwisp-config/tests/test_update_config.lua +++ b/openwisp-config/tests/test_update_config.lua @@ -123,16 +123,11 @@ function TestUpdateConfig.test_update() luaunit.assertNotNil(restoreFile) luaunit.assertEquals(restoreFile:read('*all'), 'restore-me\n') luaunit.assertNil(io.open(openwisp_dir..'/stored/etc/restore-me')) - -- ensure network configuration file is backed up - local storedNetworkFile = io.open(stored_dir .. '/etc/config/network') - luaunit.assertNotNil(storedNetworkFile) - local initialNetworkFile = io.open('update/network') - luaunit.assertEquals(storedNetworkFile:read('*all'), initialNetworkFile:read('*all')) - -- ensure system configuration file is backed up + -- ensure network and configuration file is not backed up as it is overwritten by remote + local storedNetworkFile = io.open(openwisp_dir .. '/etc/config/network') + luaunit.assertNil(storedNetworkFile) local storedSystemFile = io.open(stored_dir .. '/etc/config/system') - luaunit.assertNotNil(storedSystemFile) - local initialSystemFile = io.open('update/system') - luaunit.assertEquals(storedSystemFile:read('*all'), initialSystemFile:read('*all')) + luaunit.assertNil(storedSystemFile) end function TestUpdateConfig.test_update_conf_arg() From a57f06f4a5ccfcfe741a75292b1a5111d6356bca Mon Sep 17 00:00:00 2001 From: Kapil Bansal Date: Tue, 30 Nov 2021 02:24:15 +0530 Subject: [PATCH 3/5] [change] Remove options and sections coming from remote configuration --- .../files/sbin/openwisp-update-config.lua | 31 +++++++++++++++---- openwisp-config/tests/test_update_config.lua | 2 +- 2 files changed, 26 insertions(+), 7 deletions(-) diff --git a/openwisp-config/files/sbin/openwisp-update-config.lua b/openwisp-config/files/sbin/openwisp-update-config.lua index d5deaa4..19c0477 100755 --- a/openwisp-config/files/sbin/openwisp-update-config.lua +++ b/openwisp-config/files/sbin/openwisp-update-config.lua @@ -112,11 +112,6 @@ if lfs.attributes(remote_config_dir, 'mode') == 'directory' then end end end - -- remove entire section if empty - local result = standard:get_all(file, section['.name']) - if result and utils.is_uci_empty(result) then - standard:delete(file, section['.name']) - end end end standard:commit(file) @@ -148,8 +143,32 @@ if lfs.attributes(remote_config_dir, 'mode') == 'directory' then -- ensure we are acting on a file if lfs.attributes(remote_path, 'mode') == 'file' then -- if there's no backup of the file yet, create one - if (not utils.file_exists(stored_path) and not utils.file_exists(remote_path)) then + if (not utils.file_exists(stored_path)) then os.execute('cp '..standard_path..' '..stored_path) + if (utils.file_exists(remote_path)) then + for key, section in pairs(stored:get_all(file)) do + -- check if section is in remote configuration + local section_check = check:get(file, section['.name']) + if section_check ~= nil then + -- check if options is in remote configuration + for option, value in pairs(section) do + if not utils.starts_with_dot(option) then + local option_check = check:get(file, section['.name'], option) + if option_check ~= nil then + -- if option is in remote configuration, remove it + stored:delete(file, section['.name'], option) + end + end + end + end + end + stored:commit(file) + -- remove uci file if empty + local uci_file = stored:get_all(file) + if uci_file and utils.is_table_empty(uci_file) then + os.remove(stored_path) + end + end end -- MERGE mode if MERGE then diff --git a/openwisp-config/tests/test_update_config.lua b/openwisp-config/tests/test_update_config.lua index b0af434..44d1e35 100644 --- a/openwisp-config/tests/test_update_config.lua +++ b/openwisp-config/tests/test_update_config.lua @@ -126,7 +126,7 @@ function TestUpdateConfig.test_update() -- ensure network and configuration file is not backed up as it is overwritten by remote local storedNetworkFile = io.open(openwisp_dir .. '/etc/config/network') luaunit.assertNil(storedNetworkFile) - local storedSystemFile = io.open(stored_dir .. '/etc/config/system') + local storedSystemFile = io.open(openwisp_dir .. '/etc/config/system') luaunit.assertNil(storedSystemFile) end From d6f2b0fdd546c632459d663056cc320a0f73f8f3 Mon Sep 17 00:00:00 2001 From: Kapil Bansal Date: Tue, 30 Nov 2021 03:48:50 +0530 Subject: [PATCH 4/5] [tests] Added tests --- .../files/sbin/openwisp-update-config.lua | 18 +++++++-- openwisp-config/tests/good-config.tar.gz | Bin 483 -> 519 bytes openwisp-config/tests/test_update_config.lua | 38 +++++++++++++----- openwisp-config/tests/update/network | 5 +++ 4 files changed, 47 insertions(+), 14 deletions(-) diff --git a/openwisp-config/files/sbin/openwisp-update-config.lua b/openwisp-config/files/sbin/openwisp-update-config.lua index 19c0477..4f9e0cf 100755 --- a/openwisp-config/files/sbin/openwisp-update-config.lua +++ b/openwisp-config/files/sbin/openwisp-update-config.lua @@ -112,6 +112,11 @@ if lfs.attributes(remote_config_dir, 'mode') == 'directory' then end end end + -- remove entire section if empty + local result = standard:get_all(file, section['.name']) + if result and utils.is_uci_empty(result) then + standard:delete(file, section['.name']) + end end end standard:commit(file) @@ -143,23 +148,28 @@ if lfs.attributes(remote_config_dir, 'mode') == 'directory' then -- ensure we are acting on a file if lfs.attributes(remote_path, 'mode') == 'file' then -- if there's no backup of the file yet, create one - if (not utils.file_exists(stored_path)) then + if not utils.file_exists(stored_path) then os.execute('cp '..standard_path..' '..stored_path) - if (utils.file_exists(remote_path)) then + if utils.file_exists(remote_path) then for key, section in pairs(stored:get_all(file)) do -- check if section is in remote configuration local section_check = check:get(file, section['.name']) - if section_check ~= nil then + if section_check then -- check if options is in remote configuration for option, value in pairs(section) do if not utils.starts_with_dot(option) then local option_check = check:get(file, section['.name'], option) - if option_check ~= nil then + if option_check then -- if option is in remote configuration, remove it stored:delete(file, section['.name'], option) end end end + -- remove entire section if empty + local result = stored:get_all(file, section['.name']) + if result and utils.is_uci_empty(result) then + stored:delete(file, section['.name']) + end end end stored:commit(file) diff --git a/openwisp-config/tests/good-config.tar.gz b/openwisp-config/tests/good-config.tar.gz index dc31ada4781ce26f2873f98dd72c6ccbce11601e..b0ebe92d84c9a19fe1e65053c99592f8058f68cc 100644 GIT binary patch literal 519 zcmV+i0{HzOiwFP!000001MQc=j+-zLhPgIR;Txy69&AG&VfViGiei&k8*BtlY2Lo$ zz!s`i8dX||Ry$uLtPB&|;nzPNOsKvlE5nhZP;rDp@#XkJ0+LG!nF5t39C!{z!qnOu zi=6sUc8n#@WowWB$h&p_onlab^UL%jn}reM(^&u5#laH{|D2DHP^W5f37C~=NV*x{s)C2^8W@z?aBKem+1e>yV~y8 zYXkqM{}VF$KjS&4{%;Ew`a2T_-#x$m8khPnuh)N69rXtVnZ))F^FYXg`oB#WwP)4^ z)752VSXydrbh`WG+hDz8wsvLn66SNf?R@Yob>5kDw|hO!u!mR4_^5-eFnq)p4u&&! zrteM9Qs7_WM_#|BQaYv9VB!D%+y~RFu*NO_Czk#nY-f&G zb>DoATkn4vFaGp@s<`++mJ9O#225TZY;8Yl^V7zoyDPQzVlD#O_T`VG(Rdio!OEXp z=*}#Cd|I6I2Mpk4kr&P_PT7>zH=e;Feh7`%c<13M<8-zY1VIo4K@bE%5ClQ2#%~%@ Jv<(0#006`H3Qzz5 literal 483 zcmV<90UZ7xiwFP!000001MQbVZ`&XgfPHO#1vgI9z+l)<*zUbg)Q|=>0n3nO`S*L+ zuHvfdqDi%@F7G>l^88{PzkE;Na{IMf7zSwFl$q;>&F5z-E}4|PZg`_A#+l-}BF)kd zRyd|?dP1rv)A{{h@~&O~q&TR58^hK=F84S1m--9Iq5d^f4C}uJv-+oV%I>hh8l}KW z$~Wr|A1W?H!+FCbFZCCqmK9+OJj*L?y#8bD$yj?r$C+;KVwZgkWEWEo=HLiDcGhI~ zbIhk30!%qAyut|I9cQ}PO!o}#L?{d|lCw*h&@w{eNB?+a?tgIk6#J+5U*oF(Ob`Au ztu(6#|D{&w{~AoL`jB0}H7$s4tabL%0sMCKe+aWnyz66*;CBoTP5!*h{6G2L?OjUm zzQ*9cdfWfH_&@Fc8a?=5+6DT*0+atIzx7|N`{hdq=#BMJTvWF{nTNfzgz`(UjmMDt zGoe53XH8)T0T^a^`P{)R52pPFt$h~X Date: Thu, 2 Dec 2021 21:07:41 +0530 Subject: [PATCH 5/5] [refactor] Remove unnecessary if block --- .../files/sbin/openwisp-update-config.lua | 46 +++++++++---------- 1 file changed, 22 insertions(+), 24 deletions(-) diff --git a/openwisp-config/files/sbin/openwisp-update-config.lua b/openwisp-config/files/sbin/openwisp-update-config.lua index 4f9e0cf..43bc587 100755 --- a/openwisp-config/files/sbin/openwisp-update-config.lua +++ b/openwisp-config/files/sbin/openwisp-update-config.lua @@ -150,34 +150,32 @@ if lfs.attributes(remote_config_dir, 'mode') == 'directory' then -- if there's no backup of the file yet, create one if not utils.file_exists(stored_path) then os.execute('cp '..standard_path..' '..stored_path) - if utils.file_exists(remote_path) then - for key, section in pairs(stored:get_all(file)) do - -- check if section is in remote configuration - local section_check = check:get(file, section['.name']) - if section_check then - -- check if options is in remote configuration - for option, value in pairs(section) do - if not utils.starts_with_dot(option) then - local option_check = check:get(file, section['.name'], option) - if option_check then - -- if option is in remote configuration, remove it - stored:delete(file, section['.name'], option) - end + for key, section in pairs(stored:get_all(file)) do + -- check if section is in remote configuration + local section_check = check:get(file, section['.name']) + if section_check then + -- check if options is in remote configuration + for option, value in pairs(section) do + if not utils.starts_with_dot(option) then + local option_check = check:get(file, section['.name'], option) + if option_check then + -- if option is in remote configuration, remove it + stored:delete(file, section['.name'], option) end end - -- remove entire section if empty - local result = stored:get_all(file, section['.name']) - if result and utils.is_uci_empty(result) then - stored:delete(file, section['.name']) - end + end + -- remove entire section if empty + local result = stored:get_all(file, section['.name']) + if result and utils.is_uci_empty(result) then + stored:delete(file, section['.name']) end end - stored:commit(file) - -- remove uci file if empty - local uci_file = stored:get_all(file) - if uci_file and utils.is_table_empty(uci_file) then - os.remove(stored_path) - end + end + stored:commit(file) + -- remove uci file if empty + local uci_file = stored:get_all(file) + if uci_file and utils.is_table_empty(uci_file) then + os.remove(stored_path) end end -- MERGE mode