From 48c7e799beb39ae27ba5cf9f7993cef581159e5d Mon Sep 17 00:00:00 2001 From: Chad Smith Date: Tue, 15 Sep 2020 16:41:19 -0600 Subject: [PATCH 1/3] cli: add --system to allow validating system user-data on a machine --- cloudinit/config/schema.py | 40 +++++++++++++++++---- doc/rtd/topics/faq.rst | 6 ++-- tests/unittests/test_cli.py | 2 +- tests/unittests/test_handler/test_schema.py | 15 ++++++-- 4 files changed, 50 insertions(+), 13 deletions(-) diff --git a/cloudinit/config/schema.py b/cloudinit/config/schema.py index 8a966aeecbd..9defd70258b 100644 --- a/cloudinit/config/schema.py +++ b/cloudinit/config/schema.py @@ -2,6 +2,7 @@ """schema.py: Set of module functions for processing cloud-config schema.""" from cloudinit import importer +from cloudinit.subp import subp from cloudinit.util import find_modules, load_file import argparse @@ -39,6 +40,8 @@ SCHEMA_EXAMPLES_HEADER = '\n**Examples**::\n\n' SCHEMA_EXAMPLES_SPACER_TEMPLATE = '\n # --- Example{0} ---' +SYSTEM_USERDATA_FILE = "/var/lib/cloud/instance/user-data.txt" + class SchemaValidationError(ValueError): """Raised when validating a cloud-config file against a schema.""" @@ -173,7 +176,9 @@ def annotated_cloudconfig_file(cloudconfig, original_content, schema_errors): def validate_cloudconfig_file(config_path, schema, annotate=False): """Validate cloudconfig file adheres to a specific jsonschema. - @param config_path: Path to the yaml cloud-config file to parse. + @param config_path: Path to the yaml cloud-config file to pars, or + SYSTEM_USERDATA_FILE to indicate that the system userdata should be + queried. @param schema: Dict describing a valid jsonschema to validate against. @param annotate: Boolean set True to print original config file with error annotations on the offending lines. @@ -181,9 +186,22 @@ def validate_cloudconfig_file(config_path, schema, annotate=False): @raises SchemaValidationError containing any of schema_errors encountered. @raises RuntimeError when config_path does not exist. """ - if not os.path.exists(config_path): - raise RuntimeError('Configfile {0} does not exist'.format(config_path)) - content = load_file(config_path, decode=False) + if config_path == SYSTEM_USERDATA_FILE: + (content, err) = subp( + ["cloud-init", "query", "userdata"], decode=False + ) + if err: + raise RuntimeError( + "Could not perform `cloud-init query userdata`. %s" % err + ) + else: + if not os.path.exists(config_path): + raise RuntimeError( + 'Configfile {0} does not exist'.format( + config_path + ) + ) + content = load_file(config_path, decode=False) if not content.startswith(CLOUD_CONFIG_HEADER): errors = ( ('format-l1.c1', 'File {0} needs to begin with "{1}"'.format( @@ -425,6 +443,8 @@ def get_parser(parser=None): description='Validate cloud-config files or document schema') parser.add_argument('-c', '--config-file', help='Path of the cloud-config yaml file to validate') + parser.add_argument('--system', action='store_true', default=False, + help='Validate the system cloud-config userdata') parser.add_argument('-d', '--docs', nargs='+', help=('Print schema module docs. Choices: all or' ' space-delimited cc_names.')) @@ -435,10 +455,12 @@ def get_parser(parser=None): def handle_schema_args(name, args): """Handle provided schema args and perform the appropriate actions.""" - exclusive_args = [args.config_file, args.docs] + exclusive_args = [args.config_file, args.docs, args.system] if not any(exclusive_args) or all(exclusive_args): - error('Expected either --config-file argument or --docs') + error('Expected one of --config-file, --system or --docs arguments') full_schema = get_schema() + if args.system: + args.config_file = SYSTEM_USERDATA_FILE if args.config_file: try: validate_cloudconfig_file( @@ -449,7 +471,11 @@ def handle_schema_args(name, args): except RuntimeError as e: error(str(e)) else: - print("Valid cloud-config file {0}".format(args.config_file)) + if args.config_file == SYSTEM_USERDATA_FILE: + cfg_name = "system userdata" + else: + cfg_name = args.config_file + print("Valid cloud-config: {0}".format(cfg_name)) elif args.docs: schema_ids = [subschema['id'] for subschema in full_schema['allOf']] schema_ids += ['all'] diff --git a/doc/rtd/topics/faq.rst b/doc/rtd/topics/faq.rst index d08914b50d4..27fabf15083 100644 --- a/doc/rtd/topics/faq.rst +++ b/doc/rtd/topics/faq.rst @@ -141,12 +141,12 @@ that can validate your user data offline. .. _validate-yaml.py: https://github.com/canonical/cloud-init/blob/master/tools/validate-yaml.py -Another option is to run the following on an instance when debugging: +Another option is to run the following on an instance to debug userdata +provided to the system: .. code-block:: shell-session - $ sudo cloud-init query userdata > user-data.yaml - $ cloud-init devel schema -c user-data.yaml --annotate + $ cloud-init devel schema --system --annotate As launching instances in the cloud can cost money and take a bit longer, sometimes it is easier to launch instances locally using Multipass or LXD: diff --git a/tests/unittests/test_cli.py b/tests/unittests/test_cli.py index dcf0fe5a516..74f85959c62 100644 --- a/tests/unittests/test_cli.py +++ b/tests/unittests/test_cli.py @@ -214,7 +214,7 @@ def test_wb_devel_schema_subcommand_parser(self): self.assertEqual(1, exit_code) # Known whitebox output from schema subcommand self.assertEqual( - 'Expected either --config-file argument or --docs\n', + 'Expected one of --config-file, --system or --docs arguments\n', self.stderr.getvalue()) def test_wb_devel_schema_subcommand_doc_content(self): diff --git a/tests/unittests/test_handler/test_schema.py b/tests/unittests/test_handler/test_schema.py index 4429257182e..4a06ee59244 100644 --- a/tests/unittests/test_handler/test_schema.py +++ b/tests/unittests/test_handler/test_schema.py @@ -410,7 +410,7 @@ def test_main_missing_args(self): main() self.assertEqual(1, context_manager.exception.code) self.assertEqual( - 'Expected either --config-file argument or --docs\n', + 'Expected one of --config-file, --system or --docs arguments\n', m_stderr.getvalue()) def test_main_absent_config_file(self): @@ -443,7 +443,18 @@ def test_main_validates_config_file(self): with mock.patch('sys.stdout', new_callable=StringIO) as m_stdout: self.assertEqual(0, main(), 'Expected 0 exit code') self.assertIn( - 'Valid cloud-config file {0}'.format(myyaml), m_stdout.getvalue()) + 'Valid cloud-config: {0}'.format(myyaml), m_stdout.getvalue()) + + @mock.patch('cloudinit.config.schema.subp') + def test_main_validates_system_userdata(self, m_subp): + """When --system is provided, main validates system userdata.""" + m_subp.return_value = (b'#cloud-config\nntp:', b'') + myargs = ['mycmd', '--system'] + with mock.patch('sys.argv', myargs): + with mock.patch('sys.stdout', new_callable=StringIO) as m_stdout: + self.assertEqual(0, main(), 'Expected 0 exit code') + self.assertIn( + 'Valid cloud-config: system userdata', m_stdout.getvalue()) class CloudTestsIntegrationTest(CiTestCase): From b364ce5bc17d6ed9cda453499666f9d929273bbd Mon Sep 17 00:00:00 2001 From: Chad Smith Date: Tue, 3 Nov 2020 16:56:49 -0700 Subject: [PATCH 2/3] schema: obtain userdata_raw path using read_cfg_paths utility func Platform userdata could be configured to live in a differnt place. Use Paths object to determine that userdata_raw file path. Also add a getuid check if --system param is provided to schema because non-root users cannot access system userdata. --- cloudinit/config/schema.py | 29 +++---- tests/unittests/test_handler/test_schema.py | 94 ++++++++++++--------- 2 files changed, 68 insertions(+), 55 deletions(-) diff --git a/cloudinit/config/schema.py b/cloudinit/config/schema.py index 9defd70258b..b23bc6c100d 100644 --- a/cloudinit/config/schema.py +++ b/cloudinit/config/schema.py @@ -1,8 +1,8 @@ # This file is part of cloud-init. See LICENSE file for license information. """schema.py: Set of module functions for processing cloud-config schema.""" +from cloudinit.cmd.devel import read_cfg_paths from cloudinit import importer -from cloudinit.subp import subp from cloudinit.util import find_modules, load_file import argparse @@ -40,8 +40,6 @@ SCHEMA_EXAMPLES_HEADER = '\n**Examples**::\n\n' SCHEMA_EXAMPLES_SPACER_TEMPLATE = '\n # --- Example{0} ---' -SYSTEM_USERDATA_FILE = "/var/lib/cloud/instance/user-data.txt" - class SchemaValidationError(ValueError): """Raised when validating a cloud-config file against a schema.""" @@ -176,9 +174,8 @@ def annotated_cloudconfig_file(cloudconfig, original_content, schema_errors): def validate_cloudconfig_file(config_path, schema, annotate=False): """Validate cloudconfig file adheres to a specific jsonschema. - @param config_path: Path to the yaml cloud-config file to pars, or - SYSTEM_USERDATA_FILE to indicate that the system userdata should be - queried. + @param config_path: Path to the yaml cloud-config file to parse, or None + to default to system userdata from Paths object. @param schema: Dict describing a valid jsonschema to validate against. @param annotate: Boolean set True to print original config file with error annotations on the offending lines. @@ -186,14 +183,16 @@ def validate_cloudconfig_file(config_path, schema, annotate=False): @raises SchemaValidationError containing any of schema_errors encountered. @raises RuntimeError when config_path does not exist. """ - if config_path == SYSTEM_USERDATA_FILE: - (content, err) = subp( - ["cloud-init", "query", "userdata"], decode=False - ) - if err: + if config_path is None: + # Use system's raw userdata path + if os.getuid() != 0: raise RuntimeError( - "Could not perform `cloud-init query userdata`. %s" % err + "Unable to read system userdata as non-root user." + " Try using sudo" ) + paths = read_cfg_paths() + user_data_file = paths.get_ipath_cur("userdata_raw") + content = load_file(user_data_file, decode=False) else: if not os.path.exists(config_path): raise RuntimeError( @@ -459,9 +458,7 @@ def handle_schema_args(name, args): if not any(exclusive_args) or all(exclusive_args): error('Expected one of --config-file, --system or --docs arguments') full_schema = get_schema() - if args.system: - args.config_file = SYSTEM_USERDATA_FILE - if args.config_file: + if args.config_file or args.system: try: validate_cloudconfig_file( args.config_file, full_schema, args.annotate) @@ -471,7 +468,7 @@ def handle_schema_args(name, args): except RuntimeError as e: error(str(e)) else: - if args.config_file == SYSTEM_USERDATA_FILE: + if args.config_file is None: cfg_name = "system userdata" else: cfg_name = args.config_file diff --git a/tests/unittests/test_handler/test_schema.py b/tests/unittests/test_handler/test_schema.py index 4a06ee59244..1c084fba0ba 100644 --- a/tests/unittests/test_handler/test_schema.py +++ b/tests/unittests/test_handler/test_schema.py @@ -11,7 +11,6 @@ from copy import copy import os import pytest -from io import StringIO from pathlib import Path from textwrap import dedent from yaml import safe_load @@ -400,61 +399,78 @@ def test_annotated_cloudconfig_file_annotates_separate_line_items(self): annotated_cloudconfig_file(parsed_config, content, schema_errors)) -class MainTest(CiTestCase): +class TestMain: - def test_main_missing_args(self): + def test_main_missing_args(self, capsys): """Main exits non-zero and reports an error on missing parameters.""" with mock.patch('sys.argv', ['mycmd']): - with mock.patch('sys.stderr', new_callable=StringIO) as m_stderr: - with self.assertRaises(SystemExit) as context_manager: - main() - self.assertEqual(1, context_manager.exception.code) - self.assertEqual( - 'Expected one of --config-file, --system or --docs arguments\n', - m_stderr.getvalue()) + with pytest.raises(SystemExit) as context_manager: + main() + assert 1 == context_manager.value.code + + _out, err = capsys.readouterr() + expected = ( + 'Expected one of --config-file, --system or --docs arguments\n' + ) + assert expected == err - def test_main_absent_config_file(self): + def test_main_absent_config_file(self, capsys): """Main exits non-zero when config file is absent.""" myargs = ['mycmd', '--annotate', '--config-file', 'NOT_A_FILE'] with mock.patch('sys.argv', myargs): - with mock.patch('sys.stderr', new_callable=StringIO) as m_stderr: - with self.assertRaises(SystemExit) as context_manager: - main() - self.assertEqual(1, context_manager.exception.code) - self.assertEqual( - 'Configfile NOT_A_FILE does not exist\n', - m_stderr.getvalue()) + with pytest.raises(SystemExit) as context_manager: + main() + assert 1 == context_manager.value.code + _out, err = capsys.readouterr() + assert 'Configfile NOT_A_FILE does not exist\n' == err - def test_main_prints_docs(self): + def test_main_prints_docs(self, capsys): """When --docs parameter is provided, main generates documentation.""" myargs = ['mycmd', '--docs', 'all'] with mock.patch('sys.argv', myargs): - with mock.patch('sys.stdout', new_callable=StringIO) as m_stdout: - self.assertEqual(0, main(), 'Expected 0 exit code') - self.assertIn('\nNTP\n---\n', m_stdout.getvalue()) - self.assertIn('\nRuncmd\n------\n', m_stdout.getvalue()) + assert 0 == main(), 'Expected 0 exit code' + out, _err = capsys.readouterr() + assert '\nNTP\n---\n' in out + assert '\nRuncmd\n------\n' in out - def test_main_validates_config_file(self): + def test_main_validates_config_file(self, tmpdir, capsys): """When --config-file parameter is provided, main validates schema.""" - myyaml = self.tmp_path('my.yaml') - myargs = ['mycmd', '--config-file', myyaml] - write_file(myyaml, b'#cloud-config\nntp:') # shortest ntp schema + myyaml = tmpdir.join('my.yaml') + myargs = ['mycmd', '--config-file', myyaml.strpath] + myyaml.write(b'#cloud-config\nntp:') # shortest ntp schema with mock.patch('sys.argv', myargs): - with mock.patch('sys.stdout', new_callable=StringIO) as m_stdout: - self.assertEqual(0, main(), 'Expected 0 exit code') - self.assertIn( - 'Valid cloud-config: {0}'.format(myyaml), m_stdout.getvalue()) - - @mock.patch('cloudinit.config.schema.subp') - def test_main_validates_system_userdata(self, m_subp): + assert 0 == main(), 'Expected 0 exit code' + out, _err = capsys.readouterr() + assert 'Valid cloud-config: {0}\n'.format(myyaml) == out + + @mock.patch('cloudinit.config.schema.read_cfg_paths') + @mock.patch('cloudinit.config.schema.os.getuid', return_value=0) + def test_main_validates_system_userdata( + self, m_getuid, m_read_cfg_paths, capsys, paths + ): """When --system is provided, main validates system userdata.""" - m_subp.return_value = (b'#cloud-config\nntp:', b'') + m_read_cfg_paths.return_value = paths + ud_file = paths.get_ipath_cur("userdata_raw") + write_file(ud_file, b'#cloud-config\nntp:') myargs = ['mycmd', '--system'] with mock.patch('sys.argv', myargs): - with mock.patch('sys.stdout', new_callable=StringIO) as m_stdout: - self.assertEqual(0, main(), 'Expected 0 exit code') - self.assertIn( - 'Valid cloud-config: system userdata', m_stdout.getvalue()) + assert 0 == main(), 'Expected 0 exit code' + out, _err = capsys.readouterr() + assert 'Valid cloud-config: system userdata\n' == out + + @mock.patch('cloudinit.config.schema.os.getuid', return_value=1000) + def test_main_system_userdata_requires_root(self, m_getuid, capsys, paths): + """Non-root user can't use --system param""" + myargs = ['mycmd', '--system'] + with mock.patch('sys.argv', myargs): + with pytest.raises(SystemExit) as context_manager: + main() + assert 1 == context_manager.value.code + _out, err = capsys.readouterr() + expected = ( + 'Unable to read system userdata as non-root user. Try using sudo\n' + ) + assert expected == err class CloudTestsIntegrationTest(CiTestCase): From b3d7bcd248d4ae2ff206ad6a10ffc1e023686dad Mon Sep 17 00:00:00 2001 From: Chad Smith Date: Mon, 16 Nov 2020 12:37:02 -0700 Subject: [PATCH 3/3] schema: correct exclusive args handling docs, config-file, system --- cloudinit/config/schema.py | 4 ++-- tests/unittests/test_handler/test_schema.py | 20 ++++++++++++++++++++ 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/cloudinit/config/schema.py b/cloudinit/config/schema.py index b23bc6c100d..456bab2c090 100644 --- a/cloudinit/config/schema.py +++ b/cloudinit/config/schema.py @@ -455,7 +455,7 @@ def get_parser(parser=None): def handle_schema_args(name, args): """Handle provided schema args and perform the appropriate actions.""" exclusive_args = [args.config_file, args.docs, args.system] - if not any(exclusive_args) or all(exclusive_args): + if len([arg for arg in exclusive_args if arg]) != 1: error('Expected one of --config-file, --system or --docs arguments') full_schema = get_schema() if args.config_file or args.system: @@ -472,7 +472,7 @@ def handle_schema_args(name, args): cfg_name = "system userdata" else: cfg_name = args.config_file - print("Valid cloud-config: {0}".format(cfg_name)) + print("Valid cloud-config:", cfg_name) elif args.docs: schema_ids = [subschema['id'] for subschema in full_schema['allOf']] schema_ids += ['all'] diff --git a/tests/unittests/test_handler/test_schema.py b/tests/unittests/test_handler/test_schema.py index 1c084fba0ba..15aa77bb6c4 100644 --- a/tests/unittests/test_handler/test_schema.py +++ b/tests/unittests/test_handler/test_schema.py @@ -9,6 +9,7 @@ from cloudinit.tests.helpers import CiTestCase, mock, skipUnlessJsonSchema from copy import copy +import itertools import os import pytest from pathlib import Path @@ -401,6 +402,25 @@ def test_annotated_cloudconfig_file_annotates_separate_line_items(self): class TestMain: + exclusive_combinations = itertools.combinations( + ["--system", "--docs all", "--config-file something"], 2 + ) + + @pytest.mark.parametrize("params", exclusive_combinations) + def test_main_exclusive_args(self, params, capsys): + """Main exits non-zero and error on required exclusive args.""" + params = list(itertools.chain(*[a.split() for a in params])) + with mock.patch('sys.argv', ['mycmd'] + params): + with pytest.raises(SystemExit) as context_manager: + main() + assert 1 == context_manager.value.code + + _out, err = capsys.readouterr() + expected = ( + 'Expected one of --config-file, --system or --docs arguments\n' + ) + assert expected == err + def test_main_missing_args(self, capsys): """Main exits non-zero and reports an error on missing parameters.""" with mock.patch('sys.argv', ['mycmd']):