Schema processing changes (SC-676)

[TheRealFalcon]

Proposed Commit Message

Schema processing changes

* Use proper logging
* Add parsing for patternProperties
* Add label to annotate patternProperties
* Log warning if schema parsing fails during metaschema processing
* Some schema test fixes

Additional Context

Test Steps

Checklist:

• My code follows the process laid out in the documentation
• I have updated or added any unit tests accordingly
• I have updated or added any documentation accordingly

[TheRealFalcon]

My use of "darker" has resulted in a few formatting-only changes. If it's annoying enough I can change them back.

[TheRealFalcon]

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.

[holmanb]

This looks good to me.

I have a couple of minor nits and comments, but nothing worth holding up this PR over. It might be worthwhile to add a test to test_schema.py demonstrating how a a label in the schema overrides the property key. I would be fine with it getting merged as-is, however.

[holmanb]

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

[holmanb]

Nit: This change happens in the get_jsonschema_validator(), but the purpose for this change is for generating meaningful doc labels. It might be helpful to explain "why it's helpful when using a patternProperties regex"

perhaps:

    # "patternProperties" regex, otherwise the property label in the generated docs will be a regular expression.

[blackboxsw]

Good idea with the label.
Only thing I think is really worth fixing

non-blocking comments:

1. early skip from for loop if properties or patternProperties are empty
2. If you can find someway in metaschema to define that our objects need either "properties" or "patternProperties" go4it. Otherwise no concerns there as we do raise errors during processing of docs

[blackboxsw]

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(

[TheRealFalcon]

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

[blackboxsw]

This needs label I think right?

Suggested change

	prefix=new_prefix, prop_name=prop_key
	prefix=new_prefix, prop_name=label

[blackboxsw]

My use of "darker" has resulted in a few formatting-only changes. If it's annoying enough I can change them back.

I think this is fine here as I don't expect any downstreams to have patched the schema tool. It's worth us announcing the use of darker on the mailing list to align future development and make coding style standard. I think it is totally worth us doing this work to darken cloud-init code, but I want to generally do that in a separate non-functional set of PRs. This way downstreams won't have to sift through white-space related content versus functional changes in multiple PRs.

[blackboxsw]

Thanks for the unit test on the last commit. LGTM.