From 277fc0284eb5e71bd7c81a466bfc4a74a528d989 Mon Sep 17 00:00:00 2001 From: Otavio Pontes Date: Tue, 23 Jul 2019 14:06:51 -0700 Subject: [PATCH] mirror: Handle global parameters properly on mirror set We have 2 global parameters to change content and version url that were confusing when using mirror. For example user's didn't know what to expect from: $ swupd mirror --content-url https://new_url or $ swupd mirror --set https://new_url1 --content-url https://new_url2 Fix #941 Signed-off-by: Otavio Pontes --- src/mirror.c | 125 +++++++----- test/functional/mirror/mirror-allow-http.bats | 2 +- .../mirror/mirror-createdir-negative.bats | 12 +- test/functional/mirror/mirror-createdir.bats | 6 +- test/functional/mirror/mirror-json.bats | 2 +- .../mirror/mirror-set-unset-invalid.bats | 105 ++++++++++ test/functional/mirror/mirror-set-unset.bats | 182 ++++++++++++++++++ 7 files changed, 373 insertions(+), 61 deletions(-) create mode 100755 test/functional/mirror/mirror-set-unset-invalid.bats create mode 100755 test/functional/mirror/mirror-set-unset.bats diff --git a/src/mirror.c b/src/mirror.c index c8511ef9..5fa19c97 100644 --- a/src/mirror.c +++ b/src/mirror.c @@ -30,8 +30,9 @@ #include "swupd.h" -static char *set = NULL; +static bool set = false; static bool unset = false; +const char *set_url = NULL; static void print_help(void) { @@ -43,17 +44,17 @@ static void print_help(void) global_print_help(); print("Options:\n"); - print(" -s, --set set mirror url\n"); + print(" -s, --set [URL] set mirror url\n"); print(" -U, --unset unset mirror url\n"); print("\n"); } static const struct option prog_opts[] = { - { "set", required_argument, 0, 's' }, + { "set", no_argument, 0, 's' }, { "unset", no_argument, 0, 'U' }, }; -static bool parse_opt(int opt, char *optarg) +static bool parse_opt(int opt, UNUSED_PARAM char *optarg) { switch (opt) { case 's': @@ -61,10 +62,10 @@ static bool parse_opt(int opt, char *optarg) error("cannot set and unset at the same time\n"); return false; } - set = optarg; + set = true; return true; case 'U': - if (set != NULL) { + if (set) { error("cannot set and unset at the same time\n"); return false; } @@ -91,30 +92,38 @@ static bool parse_options(int argc, char **argv) return false; } - if (argc > ind) { - error("unexpected arguments\n\n"); + if (!set && (globals.content_url || globals.version_url)) { + error("Version or content url can only be used with --set\n"); return false; } - return true; + if (argc <= ind) { + // Check if URLs are set + if (set && !globals.content_url && !globals.version_url) { + error("Option '--set' requires an argument\n"); + return false; + } + return true; + } else if (set && argc == ind + 1) { + set_url = argv[ind]; + return true; + } + + error("Unexpected arguments\n\n"); + return false; } static int unset_mirror_url() { char *content_path; char *version_path; - int ret = 0; + int ret1 = 0, ret2 = 0; content_path = mk_full_filename(globals.path_prefix, MIRROR_CONTENT_URL_PATH); version_path = mk_full_filename(globals.path_prefix, MIRROR_VERSION_URL_PATH); - if ((ret = swupd_rm(content_path))) { - goto out; - } - if ((ret = swupd_rm(version_path))) { - goto out; - } + ret1 = swupd_rm(content_path); + ret2 = swupd_rm(version_path); -out: /* we need to also unset the mirror urls from the cache and set it to * the central version */ free_string(&globals.version_url); @@ -123,10 +132,20 @@ out: set_default_content_url(); free_string(&content_path); free_string(&version_path); - return ret; + + // No errors + if ((ret1 == -ENOENT && !ret2) || + (ret2 == -ENOENT && !ret1)) { + return 0; + } + + if (ret1) { + return ret1; + } + return ret2; } -static enum swupd_code write_to_path(char *content, char *path) +static enum swupd_code write_to_path(const char *content, const char *path) { char *dir, *tmp = NULL; struct stat dirstat; @@ -178,11 +197,10 @@ out: return ret; } -static enum swupd_code set_mirror_url(char *url) +static enum swupd_code set_new_url(const char *url, const char *url_path) { - char *content_path; - char *version_path; - int ret = SWUPD_OK; + int ret; + char *path; /* enforce the use of https */ if (!is_url_allowed(url)) { @@ -191,34 +209,48 @@ static enum swupd_code set_mirror_url(char *url) /* concatenate path_prefix and configuration paths if necessary * if path_prefix is NULL the second argument will be returned */ - content_path = mk_full_filename(globals.path_prefix, MIRROR_CONTENT_URL_PATH); - version_path = mk_full_filename(globals.path_prefix, MIRROR_VERSION_URL_PATH); - - /* write url to path_prefix/MIRROR_CONTENT_URL_PATH */ - ret = write_to_path(url, content_path); - if (ret != SWUPD_OK) { - goto out; - } + path = mk_full_filename(globals.path_prefix, url_path); /* write url to path_prefix/MIRROR_VERSION_URL_PATH */ - ret = write_to_path(url, version_path); - if (ret != SWUPD_OK) { - goto out; + ret = write_to_path(url, path); + free(path); + + return ret; +} + +static enum swupd_code set_mirror_url(const char *url_new) +{ + int ret; + const char *content_url_new = globals.content_url; + const char *version_url_new = globals.version_url; + + if (url_new) { + content_url_new = url_new; + version_url_new = url_new; } - /* if mirror is http, warn user about needing to set - * allow_insecure_http=true in order for auto-update to continue working */ - if (is_url_insecure(url)) { + if (version_url_new) { + ret = set_new_url(version_url_new, MIRROR_VERSION_URL_PATH); + if (ret != SWUPD_OK) { + return ret; + } + } + + if (content_url_new) { + ret = set_new_url(content_url_new, MIRROR_CONTENT_URL_PATH); + if (ret != SWUPD_OK) { + return ret; + } + } + + if (is_url_insecure(content_url_new) || is_url_insecure(version_url_new)) { warn("The mirror was set up using HTTP. In order for autoupdate " "to continue working you will need to set allow_insecure_http=true " "in the swupd configuration file. Alternatively you can set the " "mirror using HTTPS (recommended)\n\n"); } -out: - free_string(&content_path); - free_string(&version_path); - return ret; + return SWUPD_OK; } static bool mirror_is_set(void) @@ -321,13 +353,14 @@ enum swupd_code mirror_main(int argc, char **argv) } progress_init_steps("mirror", steps_in_mirror); - if (set != NULL) { + if (set) { check_root(); - ret = set_mirror_url(set); + ret = set_mirror_url(set_url); if (ret != SWUPD_OK) { - warn("Unable to set mirror url\n"); + error("Unable to set mirror url\n"); + goto finish; } else { - print("Set upstream mirror to %s\n", set); + print("Mirror url set\n"); } } else if (unset) { check_root(); @@ -337,7 +370,7 @@ enum swupd_code mirror_main(int argc, char **argv) ret = SWUPD_OK; } else if (ret != 0) { ret = SWUPD_COULDNT_REMOVE_FILE; - warn("Unable to remove mirror configuration\n"); + error("Unable to remove mirror configuration\n"); } else { /* ret == 0 */ print("Mirror url configuration removed\n"); } diff --git a/test/functional/mirror/mirror-allow-http.bats b/test/functional/mirror/mirror-allow-http.bats index 0d75969d..f5fedb3e 100755 --- a/test/functional/mirror/mirror-allow-http.bats +++ b/test/functional/mirror/mirror-allow-http.bats @@ -53,7 +53,7 @@ global_teardown() { Warning: This is an insecure connection The --allow-insecure-http flag was used, be aware that this poses a threat to the system Warning: The mirror was set up using HTTP. In order for autoupdate to continue working you will need to set allow_insecure_http=true in the swupd configuration file. Alternatively you can set the mirror using HTTPS (recommended) - Set upstream mirror to http://example.com/swupd-file + Mirror url set Distribution: Swupd Test Distro Installed version: 10 Version URL: http://example.com/swupd-file diff --git a/test/functional/mirror/mirror-createdir-negative.bats b/test/functional/mirror/mirror-createdir-negative.bats index 039f810f..b7681e77 100755 --- a/test/functional/mirror/mirror-createdir-negative.bats +++ b/test/functional/mirror/mirror-createdir-negative.bats @@ -37,11 +37,7 @@ global_teardown() { assert_status_is_not 0 expected_output=$(cat <<-EOM .*/etc/swupd: not a directory - Warning: Unable to set mirror url - Distribution: Swupd Test Distro - Installed version: 10 - Version URL: file://.*/web-dir - Content URL: file://.*/web-dir + Error: Unable to set mirror url EOM ) assert_regex_is_output "$expected_output" @@ -59,11 +55,7 @@ global_teardown() { assert_status_is_not 0 expected_output=$(cat <<-EOM .*/etc/swupd: not a directory - Warning: Unable to set mirror url - Distribution: Swupd Test Distro - Installed version: 10 - Version URL: file://.*/web-dir - Content URL: file://.*/web-dir + Error: Unable to set mirror url EOM ) assert_regex_is_output "$expected_output" diff --git a/test/functional/mirror/mirror-createdir.bats b/test/functional/mirror/mirror-createdir.bats index ba9453c4..e97e2449 100755 --- a/test/functional/mirror/mirror-createdir.bats +++ b/test/functional/mirror/mirror-createdir.bats @@ -33,7 +33,7 @@ global_teardown() { assert_status_is 0 expected_output=$(cat <<-EOM - Set upstream mirror to https://example.com/swupd-file + Mirror url set Distribution: Swupd Test Distro Installed version: 10 Version URL: https://example.com/swupd-file @@ -54,7 +54,7 @@ global_teardown() { assert_status_is 0 expected_output=$(cat <<-EOM - Set upstream mirror to https://example.com/swupd-file + Mirror url set Distribution: Swupd Test Distro Installed version: 10 Version URL: https://example.com/swupd-file @@ -76,7 +76,7 @@ global_teardown() { assert_status_is 0 expected_output=$(cat <<-EOM - Set upstream mirror to https://example.com/swupd-file + Mirror url set Distribution: Swupd Test Distro Installed version: 10 Version URL: https://example.com/swupd-file diff --git a/test/functional/mirror/mirror-json.bats b/test/functional/mirror/mirror-json.bats index 8b851135..e54ea4d1 100755 --- a/test/functional/mirror/mirror-json.bats +++ b/test/functional/mirror/mirror-json.bats @@ -26,7 +26,7 @@ test_setup() { { "type" : "warning", "msg" : "This is an insecure connection " }, { "type" : "info", "msg" : "The --allow-insecure-http flag was used, be aware that this poses a threat to the system " }, { "type" : "warning", "msg" : "The mirror was set up using HTTP. In order for autoupdate to continue working you will need to set allow_insecure_http=true in the swupd configuration file. Alternatively you can set the mirror using HTTPS (recommended) " }, - { "type" : "info", "msg" : "Set upstream mirror to http://example.com/swupd-file " }, + { "type" : "info", "msg" : "Mirror url set " }, { "type" : "info", "msg" : "Distribution: Swupd Test Distro " }, { "type" : "info", "msg" : "Installed version: 10 " }, { "type" : "info", "msg" : "Version URL: http://example.com/swupd-file " }, diff --git a/test/functional/mirror/mirror-set-unset-invalid.bats b/test/functional/mirror/mirror-set-unset-invalid.bats new file mode 100755 index 00000000..949266ca --- /dev/null +++ b/test/functional/mirror/mirror-set-unset-invalid.bats @@ -0,0 +1,105 @@ +#!/usr/bin/env bats + +# Author: Otavio Pontes +# Email: otavio.pontes@intel.com + +load "../testlib" + +global_setup() { + + create_test_environment "$TEST_NAME" + +} + +test_setup() { + + # do nothing + return + +} + +test_teardown() { + + sudo rm -rf "$TARGETDIR"/etc/swupd + +} + +global_teardown() { + + destroy_test_environment "$TEST_NAME" + +} + +@test "MIR016: Set invalid parameters" { + + run sudo sh -c "$SWUPD mirror --set $SWUPD_OPTS" + assert_status_is "$SWUPD_INVALID_OPTION" + expected_output=$(cat <<-EOM + Error: Option '--set' requires an argument + EOM + ) + assert_in_output "$expected_output" + + run sudo sh -c "$SWUPD mirror --set url1 url2 $SWUPD_OPTS" + assert_status_is "$SWUPD_INVALID_OPTION" + expected_output=$(cat <<-EOM + Error: Unexpected arguments + EOM + ) + assert_in_output "$expected_output" +} + +@test "MIR017: Unset invalid parameters" { + + run sudo sh -c "$SWUPD mirror --unset url $SWUPD_OPTS" + assert_status_is "$SWUPD_INVALID_OPTION" + expected_output=$(cat <<-EOM + Error: Unexpected arguments + EOM + ) + assert_in_output "$expected_output" + + run sudo sh -c "$SWUPD mirror --unset -c url $SWUPD_OPTS" + assert_status_is "$SWUPD_INVALID_OPTION" + expected_output=$(cat <<-EOM + Error: Version or content url can only be used with --set + EOM + ) + assert_in_output "$expected_output" + + run sudo sh -c "$SWUPD mirror --unset -v url $SWUPD_OPTS" + assert_status_is "$SWUPD_INVALID_OPTION" + expected_output=$(cat <<-EOM + Error: Version or content url can only be used with --set + EOM + ) + assert_in_output "$expected_output" + + run sudo sh -c "$SWUPD mirror --unset -u url $SWUPD_OPTS" + assert_status_is "$SWUPD_INVALID_OPTION" + expected_output=$(cat <<-EOM + Error: Version or content url can only be used with --set + EOM + ) + assert_in_output "$expected_output" +} + +@test "MIR018: Set/Unset invalid parameter" { + + run sudo sh -c "$SWUPD mirror --set url --unset $SWUPD_OPTS" + assert_status_is "$SWUPD_INVALID_OPTION" + expected_output=$(cat <<-EOM + Error: cannot set and unset at the same time + EOM + ) + assert_in_output "$expected_output" + + run sudo sh -c "$SWUPD mirror --set url1 url2 $SWUPD_OPTS" + assert_status_is "$SWUPD_INVALID_OPTION" + expected_output=$(cat <<-EOM + Error: Unexpected arguments + EOM + ) + assert_in_output "$expected_output" +} + diff --git a/test/functional/mirror/mirror-set-unset.bats b/test/functional/mirror/mirror-set-unset.bats new file mode 100755 index 00000000..a4b13b2d --- /dev/null +++ b/test/functional/mirror/mirror-set-unset.bats @@ -0,0 +1,182 @@ +#!/usr/bin/env bats + +# Author: Otavio Pontes +# Email: otavio.pontes@intel.com + +load "../testlib" + +global_setup() { + + create_test_environment "$TEST_NAME" + +} + +test_setup() { + + # do nothing + return + +} + +test_teardown() { + + sudo rm -rf "$TARGETDIR"/etc/swupd + +} + +global_teardown() { + + destroy_test_environment "$TEST_NAME" + +} + +@test "MIR011: Set/Unset a full mirror" { + + run sudo sh -c "$SWUPD mirror --set https://example.com/swupd-file $SWUPD_OPTS" + assert_status_is 0 + expected_output=$(cat <<-EOM + Mirror url set + Distribution: Swupd Test Distro + Installed version: 10 + Version URL: https://example.com/swupd-file + Content URL: https://example.com/swupd-file + EOM + ) + assert_is_output "$expected_output" + assert_equal "https://example.com/swupd-file" "$(<"$TARGETDIR"/etc/swupd/mirror_contenturl)" + assert_equal "https://example.com/swupd-file" "$(<"$TARGETDIR"/etc/swupd/mirror_versionurl)" + + run sudo sh -c "$SWUPD mirror --unset $SWUPD_OPTS" + assert_status_is 0 + + expected_output=$(cat <<-EOM + Mirror url configuration removed + Distribution: Swupd Test Distro + Installed version: 10 + Version URL: file://$TEST_DIRNAME/web-dir + Content URL: file://$TEST_DIRNAME/web-dir + EOM + ) + assert_is_output "$expected_output" + + assert_file_not_exists "$TARGETDIR"/etc/swupd/mirror_contenturl + assert_file_not_exists "$TARGETDIR"/etc/swupd/mirror_versionurl +} + +@test "MIR012: Unsetting a mirror not set" { + + run sudo sh -c "$SWUPD mirror --unset $SWUPD_OPTS" + assert_status_is 0 + + expected_output=$(cat <<-EOM + No mirror url configuration to remove + Distribution: Swupd Test Distro + Installed version: 10 + Version URL: file://$TEST_DIRNAME/web-dir + Content URL: file://$TEST_DIRNAME/web-dir + EOM + ) + assert_is_output "$expected_output" + + assert_file_not_exists "$TARGETDIR"/etc/swupd/mirror_contenturl + assert_file_not_exists "$TARGETDIR"/etc/swupd/mirror_versionurl +} + +@test "MIR013: Set/Unset a full mirror with globals" { + + run sudo sh -c "$SWUPD mirror --set -u https://example.com/swupd-file $SWUPD_OPTS" + assert_status_is 0 + expected_output=$(cat <<-EOM + Mirror url set + Distribution: Swupd Test Distro + Installed version: 10 + Version URL: https://example.com/swupd-file + Content URL: https://example.com/swupd-file + EOM + ) + assert_is_output "$expected_output" + assert_equal "https://example.com/swupd-file" "$(<"$TARGETDIR"/etc/swupd/mirror_contenturl)" + assert_equal "https://example.com/swupd-file" "$(<"$TARGETDIR"/etc/swupd/mirror_versionurl)" + + run sudo sh -c "$SWUPD mirror --unset $SWUPD_OPTS" + assert_status_is 0 + + expected_output=$(cat <<-EOM + Mirror url configuration removed + Distribution: Swupd Test Distro + Installed version: 10 + Version URL: file://$TEST_DIRNAME/web-dir + Content URL: file://$TEST_DIRNAME/web-dir + EOM + ) + assert_is_output "$expected_output" + + assert_file_not_exists "$TARGETDIR"/etc/swupd/mirror_contenturl + assert_file_not_exists "$TARGETDIR"/etc/swupd/mirror_versionurl +} + +@test "MIR014: Set/Unset a content mirror with globals" { + + run sudo sh -c "$SWUPD mirror --set -c https://example.com/swupd-file $SWUPD_OPTS" + assert_status_is 0 + expected_output=$(cat <<-EOM + Mirror url set + Distribution: Swupd Test Distro + Installed version: 10 + Version URL: file://$TEST_DIRNAME/web-dir + Content URL: https://example.com/swupd-file + EOM + ) + assert_is_output "$expected_output" + assert_equal "https://example.com/swupd-file" "$(<"$TARGETDIR"/etc/swupd/mirror_contenturl)" + assert_file_not_exists "$TARGETDIR"/etc/swupd/mirror_versionurl + + run sudo sh -c "$SWUPD mirror --unset $SWUPD_OPTS" + assert_status_is 0 + + expected_output=$(cat <<-EOM + Mirror url configuration removed + Distribution: Swupd Test Distro + Installed version: 10 + Version URL: file://$TEST_DIRNAME/web-dir + Content URL: file://$TEST_DIRNAME/web-dir + EOM + ) + assert_is_output "$expected_output" + + assert_file_not_exists "$TARGETDIR"/etc/swupd/mirror_contenturl + assert_file_not_exists "$TARGETDIR"/etc/swupd/mirror_versionurl +} + +@test "MIR015: Set/Unset a version mirror with globals" { + + run sudo sh -c "$SWUPD mirror --set -v https://example.com/swupd-file $SWUPD_OPTS" + assert_status_is 0 + expected_output=$(cat <<-EOM + Mirror url set + Distribution: Swupd Test Distro + Installed version: 10 + Version URL: https://example.com/swupd-file + Content URL: file://$TEST_DIRNAME/web-dir + EOM + ) + assert_is_output "$expected_output" + assert_file_not_exists "$TARGETDIR"/etc/swupd/mirror_contenturl + assert_equal "https://example.com/swupd-file" "$(<"$TARGETDIR"/etc/swupd/mirror_versionurl)" + + run sudo sh -c "$SWUPD mirror --unset $SWUPD_OPTS" + assert_status_is 0 + + expected_output=$(cat <<-EOM + Mirror url configuration removed + Distribution: Swupd Test Distro + Installed version: 10 + Version URL: file://$TEST_DIRNAME/web-dir + Content URL: file://$TEST_DIRNAME/web-dir + EOM + ) + assert_is_output "$expected_output" + + assert_file_not_exists "$TARGETDIR"/etc/swupd/mirror_contenturl + assert_file_not_exists "$TARGETDIR"/etc/swupd/mirror_versionurl +}