Skip to content

Schema processing changes (SC-676) #1144

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
blackboxsw merged 4 commits into from
Dec 14, 2021
Merged

Schema processing changes (SC-676) #1144

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b7f5c36
Schema processing changes
TheRealFalcon Dec 9, 2021
042a494
Add comment describing the new label
TheRealFalcon Dec 9, 2021
96e9361
Review comments
TheRealFalcon Dec 9, 2021
69973ce
Update label (and test) for arrays
TheRealFalcon Dec 13, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
97 changes: 65 additions & 32 deletions cloudinit/config/schema.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import yaml

error = partial(error, sys_exit=True)
LOG = logging.getLogger(__name__)

_YAML_MAP = {True: 'true', False: 'false', None: 'null'}
CLOUD_CONFIG_HEADER = b'#cloud-config'
Expand Down Expand Up @@ -91,7 +92,16 @@ def get_jsonschema_validator():
# This allows #cloud-config to provide valid yaml "content: !!binary | ..."

strict_metaschema = deepcopy(Draft4Validator.META_SCHEMA)
strict_metaschema['additionalProperties'] = False
strict_metaschema["additionalProperties"] = False

# This additional label allows us to specify a different name
# than the property key when generating docs.
# This is especially useful when using a "patternProperties" regex,
# otherwise the property label in the generated docs will be a
# regular expression.
# http://json-schema.org/understanding-json-schema/reference/object.html#pattern-properties
strict_metaschema["properties"]["label"] = {"type": "string"}
Copy link
Copy Markdown
Collaborator

@blackboxsw blackboxsw Dec 13, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Initially, I was wondering if we could also prescribe that either "properties" or "patternProperties" would satisfy our meta schema definition, but there is an interaction which breaks any config modules (cc_bootcmd) which defines a "oneOf" composition.

diff --git a/cloudinit/config/schema.py b/cloudinit/config/schema.py
index 597107497..bfdc9c9f0 100644
--- a/cloudinit/config/schema.py
+++ b/cloudinit/config/schema.py
@@ -101,6 +101,9 @@ def get_jsonschema_validator():
     # regular expression.
     # http://json-schema.org/understanding-json-schema/reference/object.html#pattern-properties
     strict_metaschema["properties"]["label"] = {"type": "string"}
+    strict_metaschema["anyOf"] = [
+        {"required": ["properties"]}, {"required": ["patternProperties"]}
+    ]
 
     if hasattr(Draft4Validator, 'TYPE_CHECKER'):  # jsonschema 3.0+
         type_checker = Draft4Validator.TYPE_CHECKER.redefine(

Copy link
Copy Markdown
Contributor Author

@TheRealFalcon TheRealFalcon Dec 13, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nothing trivial comes to mind. I'm ok skipping it.


if hasattr(Draft4Validator, 'TYPE_CHECKER'): # jsonschema 3.0+
type_checker = Draft4Validator.TYPE_CHECKER.redefine(
'string', is_schema_byte_string)
Expand Down Expand Up @@ -140,7 +150,7 @@ def validate_cloudconfig_metaschema(validator, schema: dict, throw=True):
('.'.join([str(p) for p in err.path]), err.message),
)
) from err
logging.warning(
LOG.warning(
"Meta-schema validation failed, attempting to validate config "
"anyway: %s", err)

Expand Down Expand Up @@ -168,7 +178,7 @@ def validate_cloudconfig_schema(
validate_cloudconfig_metaschema(
cloudinitValidator, schema, throw=False)
except ImportError:
logging.debug("Ignoring schema validation. jsonschema is not present")
LOG.debug("Ignoring schema validation. jsonschema is not present")
return

validator = cloudinitValidator(schema, format_checker=FormatChecker())
Expand All @@ -180,8 +190,8 @@ def validate_cloudconfig_schema(
if strict:
raise SchemaValidationError(errors)
else:
messages = ['{0}: {1}'.format(k, msg) for k, msg in errors]
logging.warning('Invalid config:\n%s', '\n'.join(messages))
messages = ["{0}: {1}".format(k, msg) for k, msg in errors]
LOG.warning("Invalid config:\n%s", "\n".join(messages))


def annotated_cloudconfig_file(cloudconfig, original_content, schema_errors):
Expand Down Expand Up @@ -410,34 +420,53 @@ def _get_property_doc(schema: dict, prefix=" ") -> str:
"""Return restructured text describing the supported schema properties."""
new_prefix = prefix + ' '
properties = []
for prop_key, prop_config in schema.get('properties', {}).items():
# Define prop_name and description for SCHEMA_PROPERTY_TMPL
description = prop_config.get('description', '')

# Define prop_name and description for SCHEMA_PROPERTY_TMPL
properties.append(
SCHEMA_PROPERTY_TMPL.format(
prefix=prefix,
prop_name=prop_key,
description=_parse_description(description, prefix),
prop_type=_get_property_type(prop_config),
property_keys = [
schema.get("properties", {}),
schema.get("patternProperties", {}),
]

for props in property_keys:
for prop_key, prop_config in props.items():
# Define prop_name and description for SCHEMA_PROPERTY_TMPL
Comment thread
blackboxsw marked this conversation as resolved.
description = prop_config.get("description", "")

# Define prop_name and description for SCHEMA_PROPERTY_TMPL
label = prop_config.get("label", prop_key)
Copy link
Copy Markdown
Member

@holmanb holmanb Dec 9, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Glad to see this change is non-breaking (falls back to prop_key when label isn't defined).

properties.append(
SCHEMA_PROPERTY_TMPL.format(
prefix=prefix,
prop_name=label,
description=_parse_description(description, prefix),
prop_type=_get_property_type(prop_config),
)
)
)
items = prop_config.get("items")
if items:
if isinstance(items, list):
for item in items:
items = prop_config.get("items")
if items:
if isinstance(items, list):
for item in items:
properties.append(
_get_property_doc(item, prefix=new_prefix)
)
elif isinstance(items, dict) and (
items.get("properties") or items.get("patternProperties")
):
properties.append(
_get_property_doc(item, prefix=new_prefix))
elif isinstance(items, dict) and items.get('properties'):
properties.append(SCHEMA_LIST_ITEM_TMPL.format(
prefix=new_prefix, prop_name=prop_key))
new_prefix += ' '
properties.append(_get_property_doc(items, prefix=new_prefix))
if 'properties' in prop_config:
properties.append(
_get_property_doc(prop_config, prefix=new_prefix))
return '\n\n'.join(properties)
SCHEMA_LIST_ITEM_TMPL.format(
prefix=new_prefix, prop_name=label
)
)
new_prefix += " "
properties.append(
_get_property_doc(items, prefix=new_prefix)
)
if (
"properties" in prop_config
or "patternProperties" in prop_config
):
properties.append(
_get_property_doc(prop_config, prefix=new_prefix)
)
return "\n\n".join(properties)


def _get_examples(meta: MetaSchema) -> str:
Expand Down Expand Up @@ -494,7 +523,11 @@ def get_meta_doc(meta: MetaSchema, schema: dict) -> str:

# cast away type annotation
meta_copy = dict(deepcopy(meta))
meta_copy["property_doc"] = _get_property_doc(schema)
try:
meta_copy["property_doc"] = _get_property_doc(schema)
except AttributeError:
LOG.warning("Unable to render property_doc due to invalid schema")
meta_copy["property_doc"] = ""
Comment thread
blackboxsw marked this conversation as resolved.
meta_copy["examples"] = _get_examples(meta)
meta_copy["distros"] = ", ".join(meta["distros"])
# Need an underbar of the same length as the name
Expand Down
55 changes: 48 additions & 7 deletions tests/unittests/config/test_schema.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ def get_module_variable(var_name) -> dict:
schemas = {}

files = list(
Path(cloud_init_project_dir("../../cloudinit/config/")).glob("cc_*.py")
Path(cloud_init_project_dir("cloudinit/config/")).glob("cc_*.py")
)

modules = [mod.stem for mod in files]
Expand Down Expand Up @@ -215,12 +215,13 @@ class TestCloudConfigExamples:
@pytest.mark.parametrize("schema_id, example", params)
@skipUnlessJsonSchema()
def test_validateconfig_schema_of_example(self, schema_id, example):
""" For a given example in a config module we test if it is valid
"""For a given example in a config module we test if it is valid
according to the unified schema of all config modules
"""
config_load = safe_load(example)
validate_cloudconfig_schema(
config_load, self.schema, strict=True)
config_load, self.schema[schema_id], strict=True
)


class ValidateCloudConfigFileTest(CiTestCase):
Expand Down Expand Up @@ -462,6 +463,44 @@ def test_get_meta_doc_raises_key_errors(self):
get_meta_doc(invalid_meta, schema)
self.assertIn(key, str(context_mgr.exception))

def test_label_overrides_property_name(self):
"""get_meta_doc overrides property name with label."""
schema = {
"properties": {
"prop1": {
"type": "string",
"label": "label1",
},
"prop_no_label": {
"type": "string",
},
"prop_array": {
"label": 'array_label',
"type": "array",
"items": {
"type": "object",
"properties": {
"some_prop": {"type": "number"},
},
},
},
},
"patternProperties": {
"^.*$": {
"type": "string",
"label": "label2",
}
}
}
meta_doc = get_meta_doc(self.meta, schema)
assert "**label1:** (string)" in meta_doc
assert "**label2:** (string" in meta_doc
assert "**prop_no_label:** (string)" in meta_doc
assert "Each item in **array_label** list" in meta_doc

assert "prop1" not in meta_doc
assert ".*" not in meta_doc


class AnnotatedCloudconfigFileTest(CiTestCase):
maxDiff = None
Expand Down Expand Up @@ -626,9 +665,11 @@ def _get_meta_doc_examples():
examples_dir = Path(cloud_init_project_dir('doc/examples'))
assert examples_dir.is_dir()

all_text_files = (f for f in examples_dir.glob('cloud-config*.txt')
if not f.name.startswith('cloud-config-archive'))
return all_text_files
return (
Copy link
Copy Markdown
Contributor Author

@TheRealFalcon TheRealFalcon Dec 9, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Without this, pytest will name the parametrized tests "example_path1", "example_path2", etc. With this, we see the actual path of the schema being tested.

str(f)
for f in examples_dir.glob("cloud-config*.txt")
if not f.name.startswith("cloud-config-archive")
)


class TestSchemaDocExamples:
Expand All @@ -637,7 +678,7 @@ class TestSchemaDocExamples:
@pytest.mark.parametrize("example_path", _get_meta_doc_examples())
@skipUnlessJsonSchema()
def test_schema_doc_examples(self, example_path):
validate_cloudconfig_file(str(example_path), self.schema)
validate_cloudconfig_file(example_path, self.schema)


class TestStrictMetaschema:
Expand Down