Concrete implementation for third-party repo add

sudo swupd 3rd-party add <repo_name> <upstream_url>

With some clean up from Otavio

Signed-off-by: Karthik Prabhu Vinod <karthik.prabhu.vinod@intel.com>

Makefile.am

@@ -34,6 +34,7 @@ AM_CPPFLAGS = $(AM_CFLAGS) $(libarchive_CFLAGS)
 swupd_SOURCES = \
 	src/3rd_party_add.c \
 	src/3rd_party.c \
+	src/3rd_party_config.c \
 	src/3rd_party_internal.h \
 	src/3rd_party_list.c \
 	src/3rd_party_remove.c \
@@ -68,6 +69,8 @@ swupd_SOURCES = \
 	src/lib/archives.h \
 	src/lib/config_parser.c \
 	src/lib/config_parser.h \
+	src/lib/config_writer.c \
+	src/lib/config_writer.h \
 	src/lib/formatter_json.c \
 	src/lib/formatter_json.h \
 	src/lib/hashmap.c \

configure.ac

@@ -83,12 +83,13 @@ AS_IF(
      THIRDPARTY="yes"],
   [THIRDPARTY="no"]
 )
+AM_CONDITIONAL([ENABLE_THIRD_PARTY_TESTS], [test "x$enable_third_party" = "xyes"])
 
 AH_TEMPLATE([THIRDPARTY_REPO_PREFIX], [prefix path for third party repos])
 
 AS_IF(
   [test "$enable_third_party" = "yes"],
-  [AC_DEFINE([THIRDPARTY_REPO_PREFIX], ["/opt/user-repo/"])]
+  [AC_DEFINE([THIRDPARTY_REPO_PREFIX], ["/opt/3rd_party/"])]
 )
 
 AC_ARG_ENABLE(

src/3rd_party_add.c

@@ -19,7 +19,6 @@
 
 #define _GNU_SOURCE
 #include "3rd_party_internal.h"
-#include "config.h"
 
 #ifdef THIRDPARTY
 
@@ -48,21 +47,22 @@ static bool parse_options(int argc, char **argv)
 		return false;
 	}
 
-	if (argc == 1) {
-		error("The positional args: repo-name and URL are missing\n\n");
+	int positional_args = argc - ind;
+	/* Ensure that repo add expects only two args: repo-name, repo-url */
+	switch (positional_args) {
+	case 0:
+		error("The positional args: repo-name and repo-url are missing\n\n");
 		return false;
-	} else if (argc == 2) {
-		error("The positional args: repo-URL is missing\n\n");
+	case 1:
+		error("The positional args: repo-url is missing\n\n");
 		return false;
-	}
-
-	/* Ensure that repo add only expects only two args: repo-name, repo-url */
-	if ((argc - ind) != EXACT_ARG_COUNT) {
+	case EXACT_ARG_COUNT:
+		return true;
+	default:
 		error("Unexpected arguments\n\n");
 		return false;
 	}
-
-	return true;
+	return false;
 }
 
 enum swupd_code third_party_add_main(int argc, char **argv)
@@ -75,18 +75,23 @@ enum swupd_code third_party_add_main(int argc, char **argv)
 		print_help();
 		return SWUPD_INVALID_OPTION;
 	}
-	progress_init_steps("third-party", step_in_third_party_add);
+	progress_init_steps("third-party-add", step_in_third_party_add);
 
-	ret = swupd_init(SWUPD_NO_ROOT);
+	ret = swupd_init(SWUPD_ALL);
 	if (ret != SWUPD_OK) {
 		goto finish;
 	}
 
-	/* TODO implement */
-
-	swupd_deinit();
+	/* The last two in reverse are the repo-name, repo-url */
+	if (add_repo_config(argv[argc - 2], argv[argc - 1]) == 0) {
+		info("Repository %s added successfully\n", argv[argc - 2]);
+	} else {
+		ret = SWUPD_COULDNT_WRITE_FILE;
+		error("Failed to add repo: %s to config\n", argv[argc - 2]);
+	}
 
 finish:
+	swupd_deinit();
 	progress_finish_steps(ret);
 	return ret;
 }

src/3rd_party_config.c

@@ -0,0 +1,223 @@
+/*
+ *   Software Updater - client side
+ *
+ *      Copyright © 2019 Intel Corporation.
+ *
+ *   This program is free software: you can redistribute it and/or modify
+ *   it under the terms of the GNU General Public License as published by
+ *   the Free Software Foundation, version 2 or later of the License.
+ *
+ *   This program is distributed in the hope that it will be useful,
+ *   but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *   GNU General Public License for more details.
+ *
+ *   You should have received a copy of the GNU General Public License
+ *   along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+#include "3rd_party_internal.h"
+#include "config.h"
+#include "lib/config_parser.h"
+#include "lib/config_writer.h"
+#include "lib/list.h"
+#include "lib/sys.h"
+#include "swupd.h"
+
+#include <errno.h>
+#include <fcntl.h>
+#include <stdlib.h>
+#include <unistd.h>
+
+#ifdef THIRDPARTY
+
+/** @brief Internal Memory representation of a repo.  */
+struct repo {
+	char *name;
+	char *url;
+};
+
+/** @brief List of repos. */
+struct list *repos;
+
+/**
+ * @brief This function is called by the repo ini parse.
+ *
+ * @param section A string containing section of the file
+ * @param key A string containing key
+ * @param value A string containing value
+ *
+ * @returns True for every valid line which could be extracted, false otherwise
+ */
+bool parse_key_values(char *section, char *key, char *value, void UNUSED_PARAM *data)
+{
+	struct repo *repo;
+	char *lkey;
+
+	lkey = str_tolower(key);
+	if (strcmp(lkey, "url") == 0) {
+		repo = calloc(1, sizeof(struct repo));
+		ON_NULL_ABORT(repo);
+		repo->name = strdup_or_die(section);
+		repo->url = strdup_or_die(value);
+		repos = list_append_data(repos, repo);
+		repos = list_head(repos);
+	}
+	free_string(&lkey);
+
+	return true;
+}
+
+static char *get_repo_config_path(void)
+{
+	char *repo_config_file_path;
+	string_or_die(&repo_config_file_path, "%s/%s/%s", globals.state_dir, "3rd_party", "repo.ini");
+	return repo_config_file_path;
+}
+
+static bool repo_config_read(void)
+{
+	bool parse_ret = false;
+	char *repo_config_file_path = get_repo_config_path();
+
+	parse_ret = config_parse(repo_config_file_path, parse_key_values, NULL);
+	free_string(&repo_config_file_path);
+	return parse_ret;
+}
+
+/* TODO move to 3rd-party helpers */
+static void repo_free(struct repo *repo)
+{
+	if (!repo) {
+		return;
+	}
+
+	free_string(&repo->name);
+	free_string(&repo->url);
+	free(repo);
+}
+
+/* TODO move to 3rd-party helpers */
+static void repo_free_data(void *data)
+{
+	struct repo *repo = (struct repo *)data;
+	repo_free(repo);
+}
+
+static int third_party_config_create_dir(void)
+{
+	int ret = 0;
+	char *repo_config_file_path = get_repo_config_path();
+	char *repo_config_dir = sys_dirname(repo_config_file_path);
+
+	ret = mkdir_p(repo_config_dir);
+	if (ret) {
+		error("Failed to create repository config directory\n");
+	}
+
+	free_string(&repo_config_dir);
+	free_string(&repo_config_file_path);
+	return ret;
+}
+
+static int third_party_config_create_file(void)
+{
+	char *repo_config_file_path = get_repo_config_path();
+	int ret = 0;
+	int fd = open(repo_config_file_path, O_CREAT | O_EXCL, S_IRUSR | S_IWUSR);
+	if (fd < 0 && errno != EEXIST) {
+		error("Failed to create repo config file, error: %d", errno);
+		ret = -1;
+	}
+
+	close(fd);
+	free_string(&repo_config_file_path);
+	return ret;
+}
+
+int repo_config_init(void)
+{
+	int ret = 0;
+	ret = third_party_config_create_dir();
+
+	if (ret) {
+		goto exit;
+	}
+
+	ret = third_party_config_create_file();
+
+	if (ret) {
+		goto exit;
+	}
+
+	if (!repo_config_read()) {
+		error("Failed to read & parse repo config file");
+		ret = -1;
+	}
+
+exit:
+	return ret;
+}
+
+void repo_config_deinit(void)
+{
+	list_free_list_and_data(repos, repo_free_data);
+}
+
+/**
+ * @brief This function performs the repo config operation part for repo add
+ * It checks if a repo with repo_name already exists and if not creates an
+ * entry in the repo config file.
+ *
+ * @param repo_name A string containing repo_name
+ * @param repo_url A string containing repo_url
+ *
+ * @returns 0 on success ie: a repo with the name is successfully added to repo config
+ * otherwise a -1 on any failure
+ */
+int add_repo_config(char *repo_name, char *repo_url)
+{
+	struct list *repo;
+	int ret = 0;
+	FILE *fp = NULL;
+	char *repo_config_file_path;
+
+	if (repo_config_init()) {
+		ret = -1;
+		goto exit;
+	}
+
+	repo = list_head(repos);
+	while (repo) {
+		struct repo *repo_iter = repo->data;
+		if (!strncmp(repo_iter->name, repo_name, strlen(repo_name))) {
+			error("The repo: %s already exists\n", repo_name);
+			ret = -1;
+			goto exit;
+		}
+		repo = repo->next;
+	}
+
+	// If we are here, we are cleared to write to file
+	repo_config_file_path = get_repo_config_path();
+	fp = fopen(repo_config_file_path, "a");
+
+	ret = config_write_section(fp, repo_name);
+	if (ret) {
+		goto exit;
+	}
+
+	// write url
+	ret = config_write_config(fp, "url", repo_url);
+
+exit:
+	if (fp) {
+		fclose(fp);
+	}
+	free_string(&repo_config_file_path);
+	repo_config_deinit();
+	return ret;
+}
+
+#endif

src/lib/config_writer.c

@@ -0,0 +1,69 @@
+/*
+ *   Software Updater - client side
+ *
+ *      Copyright © 2012-2019 Intel Corporation.
+ *
+ *   This program is free software: you can redistribute it and/or modify
+ *   it under the terms of the GNU General Public License as published by
+ *   the Free Software Foundation, version 2 or later of the License.
+ *
+ *   This program is distributed in the hope that it will be useful,
+ *   but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *   GNU General Public License for more details.
+ *
+ *   You should have received a copy of the GNU General Public License
+ *   along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+#define _GNU_SOURCE
+#include <stdio.h>
+#include <unistd.h>
+
+#include "log.h"
+#include "strings.h"
+
+int config_write_section(FILE *file, const char *section)
+{
+	int ret = 0;
+	char *repo_string;
+
+	if (!file) {
+		return -1;
+	}
+
+	string_or_die(&repo_string, "\n[%s]\n\n", section);
+	ret = fputs(repo_string, file);
+
+	if (ret < 0 || ret == EOF) {
+		error("config section write failed\n");
+	} else if (ret > 0) {
+		ret = 0;
+	}
+
+	free_string(&repo_string);
+	return ret;
+}
+
+int config_write_config(FILE *file, const char *key, const char *value)
+{
+	int ret = 0;
+	char *repo_string;
+
+	if (!file) {
+		return -1;
+	}
+
+	string_or_die(&repo_string, "%s=%s\n", key, value);
+	ret = fputs(repo_string, file);
+
+	if (ret < 0 || ret == EOF) {
+		error("config(key,value) write failed\n");
+	} else if (ret > 0) {
+		ret = 0;
+	}
+
+	free_string(&repo_string);
+	return ret;
+}

src/lib/config_writer.h

@@ -0,0 +1,39 @@
+#ifndef __CONFIG_WRITER__
+#define __CONFIG_WRITER__
+
+/**
+ * @file
+ * @brief Configuration fie writing
+ */
+
+#include <stdbool.h>
+#include <stdio.h>
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+/**
+ * @brief Writes a section to ini file using string as an argument.
+ *
+ * @param FILE file pointer *FILE to the config file
+ * @param section string to written to config file
+ * @returns 0 if no errors or negative on any error
+ */
+int config_write_section(FILE *file, const char *section);
+
+/**
+ * @brief Writes a config(key,value) line to ini file using key,value strings as an
+ * argument
+ *
+ * @param FILE file pointer *FILE to the config file
+ * @param key string
+ * @param value string
+ * @returns 0 if no errors or negative on any error
+ */
+int config_write_config(FILE *file, const char *key, const char *value);
+
+#ifdef __cplusplus
+}
+#endif
+#endif

test/functional/3rd-party-repo-add/3rd-party-repo-add-negative.bats

@@ -0,0 +1,68 @@
+#!/usr/bin/env bats
+
+# Author: Karthik Prabhu Vinod
+# Email: karthik.prabhu.vinod@intel.com
+
+load "../testlib"
+
+@test "TRA003: Negative test, repo add usage" {
+
+	run sudo sh -c "$SWUPD 3rd-party add $SWUPD_OPTS"
+	assert_status_is "$SWUPD_INVALID_OPTION"
+	expected_output=$(cat <<-EOM
+			Error: The positional args: repo-name and repo-url are missing
+	EOM
+	)
+	assert_in_output "$expected_output"
+
+	run sudo sh -c "$SWUPD 3rd-party add test-repo1 $SWUPD_OPTS"
+	assert_status_is "$SWUPD_INVALID_OPTION"
+	expected_output=$(cat <<-EOM
+			Error: The positional args: repo-url is missing
+	EOM
+	)
+	assert_in_output "$expected_output"
+
+	run sudo sh -c "$SWUPD 3rd-party add test-repo1 http://abc.com junk_positional $SWUPD_OPTS"
+	assert_status_is "$SWUPD_INVALID_OPTION"
+	expected_output=$(cat <<-EOM
+			Error: Unexpected arguments
+	EOM
+	)
+	assert_in_output "$expected_output"
+
+}
+
+@test "TRA004: Negative test, Add an already added repo" {
+
+	run sudo sh -c "$SWUPD 3rd-party add test-repo1 https://www.xyz.com $SWUPD_OPTS"
+	assert_status_is "$SWUPD_OK"
+	expected_output=$(cat <<-EOM
+			Repository test-repo1 added successfully
+	EOM
+	)
+	assert_is_output "$expected_output"
+
+	run sudo sh -c "$SWUPD 3rd-party add test-repo1 https://abc.com $SWUPD_OPTS"
+	assert_status_is "$SWUPD_COULDNT_WRITE_FILE"
+	expected_output=$(cat <<-EOM
+		Error: The repo: test-repo1 already exists
+		Error: Failed to add repo: test-repo1 to config
+	EOM
+	)
+	assert_is_output "$expected_output"
+
+}
+
+@test "TRA005: Negative test, Invalid URL" {
+
+	run sudo sh -c "$SWUPD 3rd-party add test-repo1 www.xyz.com $SWUPD_OPTS"
+	assert_status_is "$SWUPD_COULDNT_WRITE_FILE"
+	expected_output=$(cat <<-EOM
+			Error: Invalid argument: repo-url
+
+	EOM
+	)
+	assert_is_output "$expected_output"
+
+}

test/functional/3rd-party-repo-add/3rd-party-repo-add.bats

@@ -0,0 +1,98 @@
+#!/usr/bin/env bats
+
+# Author: Karthik Prabhu Vinod
+# Email: karthik.prabhu.vinod@intel.com
+
+load "../testlib"
+
+@test "TRA001: Add a single repo" {
+
+	run sudo sh -c "$SWUPD 3rd-party add test-repo1 http://xyz.com $SWUPD_OPTS"
+	assert_status_is "$SWUPD_OK"
+	expected_output=$(cat <<-EOM
+			Repository test-repo1 added successfully
+	EOM
+	)
+	assert_is_output "$expected_output"
+
+	run sudo sh -c "cat $STATEDIR/3rd_party/repo.ini"
+	expected_output=$(cat <<-EOM
+			[test-repo1]
+			url=http://xyz.com
+	EOM
+	)
+	assert_is_output "$expected_output"
+
+}
+
+@test "TRA002: Add multiple repos in repo config" {
+
+	# Add 5 repos in a row one by one
+	run sudo sh -c "$SWUPD 3rd-party add test-repo1 https://xyz.com $SWUPD_OPTS"
+	assert_status_is "$SWUPD_OK"
+	expected_output=$(cat <<-EOM
+			Repository test-repo1 added successfully
+	EOM
+	)
+	assert_is_output "$expected_output"
+
+	run sudo sh -c "$SWUPD 3rd-party add test-repo2 file://abc.com $SWUPD_OPTS"
+	assert_status_is "$SWUPD_OK"
+	expected_output=$(cat <<-EOM
+			Repository test-repo2 added successfully
+	EOM
+	)
+	assert_is_output "$expected_output"
+
+	run sudo sh -c "$SWUPD 3rd-party add test-repo3 https://efg.com $SWUPD_OPTS"
+	assert_status_is "$SWUPD_OK"
+	expected_output=$(cat <<-EOM
+			Repository test-repo3 added successfully
+	EOM
+	)
+	assert_is_output "$expected_output"
+
+	run sudo sh -c "$SWUPD 3rd-party add test-repo4 http://hij.com $SWUPD_OPTS"
+	assert_status_is "$SWUPD_OK"
+	expected_output=$(cat <<-EOM
+			Repository test-repo4 added successfully
+	EOM
+	)
+	assert_is_output "$expected_output"
+
+	run sudo sh -c "$SWUPD 3rd-party add test-repo5 https://klm.com $SWUPD_OPTS"
+	assert_status_is "$SWUPD_OK"
+	expected_output=$(cat <<-EOM
+			Repository test-repo5 added successfully
+	EOM
+	)
+	assert_is_output "$expected_output"
+
+	run sudo sh -c "cat $STATEDIR/3rd_party/repo.ini"
+	assert_status_is "$SWUPD_OK"
+	expected_output=$(cat <<-EOM
+
+			[test-repo1]
+
+			url=https://xyz.com
+
+			[test-repo2]
+
+			url=file://abc.com
+
+			[test-repo3]
+
+			url=https://efg.com
+
+			[test-repo4]
+
+			url=http://hij.com
+
+			[test-repo5]
+
+			url=https://klm.com
+	EOM
+	)
+	assert_is_output --identical "$expected_output"
+
+}

test/functional/README.md

@@ -138,6 +138,7 @@ or more of the [assertions](#assertions) provided by the test library.
 | Clean cached files                             | clean         | CLN      |
 | Configure mirror URL for swupd content         | mirror        | MIR      |
 | Usability                                      | N/A           | USA      |
+| 3rd-party repo add                             | 3rd-party-repo-add | TRA |
 
 ### Test Principles
 

test/functional/testlib.bash

@@ -3179,6 +3179,7 @@ get_next_available_id() { # swupd_function
 	id=$((id+1))
 	test_dir=$(basename "$(realpath "$test_dir")")
 	case "$test_dir" in
+		3rd-party-repo-add) group=TRA;;
 		autoupdate) group=AUT;;
 		bundleadd) group=ADD;;
 		bundleremove) group=REM;;