Skip to content

Commit

Permalink
parse: introduce parser flags
Browse files Browse the repository at this point in the history
Parser flags are intended to be used to change the parser's behavior.
This patch introduces a single flag used to instruct the parser that
parsing errors should be ignored and it should continue to parse
netdefs.

The main goal with this flag is to make Netplan more resilient.
Currently, any mistake made in the YAML files will prevent Netplan to
generate configuration. So, rebooting the system while one of the YAML
files are broken will result in the system having no network conectivity.

The IGNORE_ERRORS flag is enabled by default when the generate command
is called from systemd as a generator.
  • Loading branch information
daniloegea committed Feb 20, 2024
1 parent 19917d7 commit e4eecd0
Show file tree
Hide file tree
Showing 12 changed files with 322 additions and 10 deletions.
9 changes: 9 additions & 0 deletions include/parse.h
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,15 @@ netplan_parser_reset(NetplanParser *npp);
NETPLAN_PUBLIC void
netplan_parser_clear(NetplanParser **npp);

/**
* @brief Set @ref NetplanParser flags.
* @details Parser flags are used to change the default behavior of the parser.
* @param[in] npp The @ref NetplanParser to set the flags.
* @param[in] flags The value of the flags. The possible values are defined in @ref NETPLAN_PARSER_FLAGS
*/
NETPLAN_PUBLIC void
netplan_parser_set_flags(NetplanParser* npp, int flags);

/**
* @brief Parse a given YAML file and create or update the list of @ref NetplanNetDefinition inside @p npp.
* @param[in] npp The @ref NetplanParser object that should contain the parsed data
Expand Down
7 changes: 7 additions & 0 deletions include/types.h
Original file line number Diff line number Diff line change
Expand Up @@ -164,3 +164,10 @@ enum NETPLAN_EMITTER_ERRORS {
enum NETPLAN_FORMAT_ERRORS {
NETPLAN_ERROR_FORMAT_INVALID_YAML,
};

/**
* @brief Flags used to change the parser behavior.
*/
enum NETPLAN_PARSER_FLAGS {
NETPLAN_PARSER_IGNORE_ERRORS = 1, ///< Ignore parsing errors such as bad YAML files and definitions.
};
1 change: 1 addition & 0 deletions python-cffi/netplan/_build_cffi.py
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@
// Parser
NetplanParser* netplan_parser_new();
void netplan_parser_clear(NetplanParser **npp);
void netplan_parser_set_flags(NetplanParser *npp, int flags);
gboolean netplan_parser_load_yaml(NetplanParser* npp, const char* filename, NetplanError** error);
gboolean netplan_parser_load_yaml_from_fd(NetplanParser* npp, int input_fd, NetplanError** error);
gboolean netplan_parser_load_yaml_hierarchy(NetplanParser* npp, const char* rootdir, NetplanError** error);
Expand Down
8 changes: 8 additions & 0 deletions python-cffi/netplan/parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,17 @@
# You should have received a copy of the GNU General Public License
# along with this program. If not, see <http://www.gnu.org/licenses/>.

from enum import IntEnum
from typing import Union, IO

from ._netplan_cffi import ffi, lib
from ._utils import _checked_lib_call


class NETPLAN_PARSER_FLAGS(IntEnum):
IGNORE_ERRORS = 1


class Parser():
def __init__(self):
self._ptr = lib.netplan_parser_new()
Expand Down Expand Up @@ -46,3 +51,6 @@ def load_nullable_fields(self, input_file: IO):
def _load_nullable_overrides(self, input_file: IO, constraint: str):
return _checked_lib_call(lib.netplan_parser_load_nullable_overrides,
self._ptr, input_file.fileno(), constraint.encode('utf-8'))

def set_flags(self, flags: int):
lib.netplan_parser_set_flags(self._ptr, flags)
5 changes: 5 additions & 0 deletions src/generate.c
Original file line number Diff line number Diff line change
Expand Up @@ -44,10 +44,12 @@ static gchar** files;
static gboolean any_networkd = FALSE;
static gboolean any_nm = FALSE;
static gchar* mapping_iface;
static gboolean ignore_errors = FALSE;

static GOptionEntry options[] = {
{"root-dir", 'r', 0, G_OPTION_ARG_FILENAME, &rootdir, "Search for and generate configuration files in this root directory instead of /", NULL},
{G_OPTION_REMAINING, 0, 0, G_OPTION_ARG_FILENAME_ARRAY, &files, "Read configuration from this/these file(s) instead of /etc/netplan/*.yaml", "[config file ..]"},
{"ignore-errors", 'i', 0, G_OPTION_ARG_NONE, &ignore_errors, "Ignores files and/or interfaces that fail parsing.", NULL},
{"mapping", 0, 0, G_OPTION_ARG_STRING, &mapping_iface, "Only show the device to backend mapping for the specified interface.", NULL},
{NULL}
};
Expand Down Expand Up @@ -243,6 +245,9 @@ int main(int argc, char** argv)
}

npp = netplan_parser_new();
if (ignore_errors || called_as_generator)
netplan_parser_set_flags(npp, NETPLAN_PARSER_IGNORE_ERRORS);

/* Read all input files */
if (files && !called_as_generator) {
for (gchar** f = files; f && *f; ++f) {
Expand Down
63 changes: 55 additions & 8 deletions src/parse.c
Original file line number Diff line number Diff line change
Expand Up @@ -3134,6 +3134,23 @@ node_is_nulled_out(yaml_document_t* doc, yaml_node_t* node, const char* key_pref
return TRUE;
}

/*
* Remove a given netdef from the parser state
*/
STATIC void
netplan_parser_drop_netdef(NetplanParser* npp, NetplanNetDefinition *netdef)
{
if(npp->parsed_defs) {
g_hash_table_steal(npp->parsed_defs, netdef->id);
}
if(npp->ordered) {
npp->ordered = g_list_remove(npp->ordered, netdef);
}

reset_netdef(netdef, NETPLAN_DEF_TYPE_NONE, NETPLAN_BACKEND_NONE);
g_free(netdef);
}

/**
* Callback for a net device type entry like "ethernets:" in "network:"
* @data: netdef_type (as pointer)
Expand All @@ -3145,6 +3162,7 @@ handle_network_type(NetplanParser* npp, yaml_node_t* node, const char* key_prefi
yaml_node_t* key, *value;
const mapping_entry_handler* handlers;
g_autofree char* full_key = NULL;
gboolean new_netdef = FALSE;

key = yaml_document_get_node(&npp->doc, entry->key);
if (!assert_valid_id(npp, key, error))
Expand Down Expand Up @@ -3180,12 +3198,6 @@ handle_network_type(NetplanParser* npp, yaml_node_t* node, const char* key_prefi

assert_type(npp, value, YAML_MAPPING_NODE);

/* At this point we've seen a new starting definition, if it has been
* already mentioned in another netdef, removing it from our "missing"
* list. */
if(g_hash_table_remove(npp->missing_id, scalar(key)))
npp->missing_ids_found++;

npp->current.netdef = npp->parsed_defs ? g_hash_table_lookup(npp->parsed_defs, scalar(key)) : NULL;
if (npp->current.netdef) {
/* already exists, overriding/amending previous definition */
Expand All @@ -3198,6 +3210,7 @@ handle_network_type(NetplanParser* npp, yaml_node_t* node, const char* key_prefi
}
} else {
npp->current.netdef = netplan_netdef_new(npp, scalar(key), GPOINTER_TO_UINT(data), npp->current.backend);
new_netdef = TRUE;
}
if (npp->current.filepath) {
if (npp->current.netdef->filepath)
Expand Down Expand Up @@ -3245,8 +3258,29 @@ handle_network_type(NetplanParser* npp, yaml_node_t* node, const char* key_prefi
npp->current.netdef->vxlan = vxlan;
}

if (!process_mapping(npp, value, full_key, handlers, NULL, error))
return FALSE;
if (!process_mapping(npp, value, full_key, handlers, NULL, error)) {
if (npp->flags & NETPLAN_PARSER_IGNORE_ERRORS) {
gchar* warning_msg = NULL;
if (error && *error)
warning_msg = (*error)->message;
g_warning("Skipping interface due to parsing errors. %s: %s", scalar(key), warning_msg);
if (new_netdef) {
netplan_parser_drop_netdef(npp, npp->current.netdef);
npp->current.netdef = NULL;
}
g_clear_error(error);
continue;
} else {
return FALSE;
}
}

/* At this point we've seen a new starting definition, if it has been
* already mentioned in another netdef, removing it from our "missing"
* list. */
if(g_hash_table_remove(npp->missing_id, scalar(key)))
npp->missing_ids_found++;


/* Postprocessing */
/* Implicit VXLAN settings, which can be deduced from parsed data. */
Expand Down Expand Up @@ -3454,6 +3488,11 @@ _netplan_parser_load_single_file(NetplanParser* npp, const char *opt_filepath, y
yaml_document_delete(doc);
g_hash_table_destroy(npp->ids_in_file);
npp->ids_in_file = NULL;

if (npp->flags & NETPLAN_PARSER_IGNORE_ERRORS) {
g_clear_error(error);
return TRUE;
}
return ret;
}

Expand Down Expand Up @@ -3653,6 +3692,8 @@ netplan_parser_reset(NetplanParser* npp)
g_hash_table_destroy(npp->global_renderer);
npp->global_renderer = NULL;
}

npp->flags = 0;
}

void
Expand All @@ -3664,6 +3705,12 @@ netplan_parser_clear(NetplanParser** npp_p)
g_free(npp);
}

void
netplan_parser_set_flags(NetplanParser* npp, const int flags)
{
npp->flags = flags;
}

/* Check if this is a Netdef-ID or global keyword which can be nullified.
* Overrides (depending on YAML hierarchy) can only happen on global values
* (like "renderer") or on the individual netdef level.
Expand Down
3 changes: 3 additions & 0 deletions src/types-internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -263,6 +263,9 @@ struct netplan_parser {
GHashTable* null_fields;
GHashTable* null_overrides;
GHashTable* global_renderer;

/* Flags used to change the parser's behavior */
int flags;
};

struct netplan_state_iterator {
Expand Down
12 changes: 11 additions & 1 deletion src/util.c
Original file line number Diff line number Diff line change
Expand Up @@ -656,11 +656,21 @@ netplan_parser_load_yaml_hierarchy(NetplanParser* npp, const char* rootdir, GErr

config_keys = g_list_sort(g_hash_table_get_keys(configs), (GCompareFunc) strcmp);

for (GList* i = config_keys; i != NULL; i = i->next)
for (GList* i = config_keys; i != NULL; i = i->next) {
if (!netplan_parser_load_yaml(npp, g_hash_table_lookup(configs, i->data), error)) {
if (npp->flags & NETPLAN_PARSER_IGNORE_ERRORS) {
gchar* warning_msg = NULL;
if (error && *error)
warning_msg = (*error)->message;

g_warning("Skipping YAML file due to parsing errors. %s", warning_msg);
g_clear_error(error);
continue;
}
globfree(&gl);
return FALSE;
}
}
globfree(&gl);
return TRUE;
}
Expand Down
92 changes: 92 additions & 0 deletions tests/ctests/test_netplan_parser.c
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,15 @@
#include <stdarg.h>
#include <stddef.h>
#include <setjmp.h>
#include <sys/stat.h>

#include <cmocka.h>
#include <yaml.h>

#include "glib.h"
#include "netplan.h"
#include "parse.h"
#include "types.h"
#include "util.h"
#include "util-internal.h"

Expand Down Expand Up @@ -255,6 +258,94 @@ test_nm_device_backend_is_nm_by_default(__unused void** state)
netplan_state_clear(&np_state);
}

void
test_parser_flags(__unused void** state)
{
const char* yaml1 =
"network:\n"
" version: 2\n"
" ethernets:\n"
" eth0: {}\n";

const char* yaml2 = ":";

const char* yaml3 =
"network:\n"
" version: 2\n"
" ethernets:\n"
" eth1:\n"
" dhcp4: yesplease\n"
" eth2: {}\n";

const char* yaml4 =
"network:\n"
" version: 2\n"
" ethernets:\n"
" eth3: {}\n";

GError *error = NULL;
char template[] = "/tmp/netplan-XXXXXX";
char* tempdir = mkdtemp(template);
g_autofree gchar* etcpath = g_strdup_printf("%s/etc", tempdir);
g_autofree gchar* fullpath = g_strdup_printf("%s/etc/netplan/", tempdir);
g_autofree gchar* file1 = g_strdup_printf("%s/etc/netplan/file1.yaml", tempdir);
g_autofree gchar* file2 = g_strdup_printf("%s/etc/netplan/file2.yaml", tempdir);
g_autofree gchar* file3 = g_strdup_printf("%s/etc/netplan/file3.yaml", tempdir);
g_autofree gchar* file4 = g_strdup_printf("%s/etc/netplan/file4.yaml", tempdir);
g_mkdir_with_parents(fullpath, 0770);
mode_t old_umask = umask(0077);
FILE* f1 = fopen(file1, "w");
FILE* f2 = fopen(file2, "w");
FILE* f3 = fopen(file3, "w");
FILE* f4 = fopen(file4, "w");

fwrite(yaml1, strlen(yaml1), 1, f1);
fclose(f1);
fwrite(yaml2, strlen(yaml2), 1, f2);
fclose(f2);
fwrite(yaml3, strlen(yaml3), 1, f3);
fclose(f3);
fwrite(yaml4, strlen(yaml4), 1, f4);
fclose(f4);

umask(old_umask);

NetplanParser* npp = netplan_parser_new();
netplan_parser_set_flags(npp, NETPLAN_PARSER_IGNORE_ERRORS);
gboolean ret = netplan_parser_load_yaml_hierarchy(npp, tempdir, &error);

// Despite the errors in the YAML it should report success
assert_true(ret);
assert_null(error);

NetplanState* np_state = netplan_state_new();
netplan_state_import_parser_results(np_state, npp, &error);
NetplanStateIterator iter;
NetplanNetDefinition* netdef = NULL;
netplan_state_iterator_init(np_state, &iter);

// Check if all the good netdefs are present in the state
netdef = netplan_state_iterator_next(&iter);
assert_string_equal(netdef->id, "eth0");

netdef = netplan_state_iterator_next(&iter);
assert_string_equal(netdef->id, "eth2");

netdef = netplan_state_iterator_next(&iter);
assert_string_equal(netdef->id, "eth3");

netplan_parser_clear(&npp);
netplan_state_clear(&np_state);

unlink(file1);
unlink(file2);
unlink(file3);
unlink(file4);
rmdir(fullpath);
rmdir(etcpath);
rmdir(tempdir);
}

int
setup(__unused void** state)
{
Expand Down Expand Up @@ -284,6 +375,7 @@ main()
cmocka_unit_test(test_netplan_parser_process_document_proper_error),
cmocka_unit_test(test_netplan_parser_process_document_missing_interface_error),
cmocka_unit_test(test_nm_device_backend_is_nm_by_default),
cmocka_unit_test(test_parser_flags),
};

return cmocka_run_group_tests(tests, setup, tear_down);
Expand Down
13 changes: 12 additions & 1 deletion tests/generator/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -317,7 +317,8 @@ def validate_generated_yaml(self, yaml_input):
print(line, flush=True)
self.fail('Re-generated YAML file does not match (adopt netplan.c YAML generator?)')

def generate(self, yaml, expect_fail=False, extra_args=[], confs=None, skip_generated_yaml_validation=False):
def generate(self, yaml, expect_fail=False, extra_args=[], confs=None, skip_generated_yaml_validation=False,
ignore_errors=False):
'''Call generate with given YAML string as configuration
Return stderr output.
Expand All @@ -342,6 +343,9 @@ def generate(self, yaml, expect_fail=False, extra_args=[], confs=None, skip_gene
print('Test is about to run:\n%s' % ' '.join(argv))
subprocess.call(['bash', '-i'], cwd=self.workdir.name)

if ignore_errors:
argv += ['--ignore-errors']

p = subprocess.Popen(argv, stdout=subprocess.PIPE,
stderr=subprocess.PIPE, text=True)
(out, err) = p.communicate()
Expand All @@ -364,6 +368,13 @@ def eth_name(self):
"""
return 'eth' + ''.join(random.sample(string.ascii_letters + string.digits, k=4))

def file_exists(self, filename, backend='systemd') -> bool:
if backend == 'systemd':
path = os.path.join(self.workdir.name, 'run', 'systemd', 'network', filename)
else:
path = os.path.join(self.workdir.name, 'run', 'NetworkManager', 'system-connections', filename)
return os.path.exists(path)

def assert_networkd(self, file_contents_map):
networkd_dir = os.path.join(self.workdir.name, 'run', 'systemd', 'network')
if not file_contents_map:
Expand Down
Loading

0 comments on commit e4eecd0

Please sign in to comment.