Skip to content

Commit

Permalink
Add Strict Metaschema Validation (#1101)
Browse files Browse the repository at this point in the history
Improve schema validation.

This adds strict validation of config module definitions at testing
time, with plumbing included for future runtime validation. This
eliminates a class of bugs resulting from schemas that have definitions
that are incorrect, but get interpreted by jsonschema as
"additionalProperties" that are therefore ignored.

- Add strict meta-schema for jsonschema unit test validation
- Separate schema from module metadata structure
- Improve type annotations for various functions and data types

Cleanup:
- Remove unused jsonschema "required" elements 
- Eliminate manual memoization in schema.py:get_schema(),
        reference module.__doc__ directly
  • Loading branch information
holmanb authored Dec 6, 2021
1 parent f428ed1 commit bedac77
Show file tree
Hide file tree
Showing 25 changed files with 701 additions and 291 deletions.
8 changes: 3 additions & 5 deletions cloudinit/cmd/clean.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,9 @@

from cloudinit.stages import Init
from cloudinit.subp import (ProcessExecutionError, subp)
from cloudinit.util import (del_dir, del_file, get_config_logfiles, is_link)


def error(msg):
sys.stderr.write("ERROR: " + msg + "\n")
from cloudinit.util import (
del_dir, del_file, get_config_logfiles, is_link, error
)


def get_parser(parser=None):
Expand Down
6 changes: 1 addition & 5 deletions cloudinit/cmd/cloud_id.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import json
import sys

from cloudinit.util import error
from cloudinit.sources import (
INSTANCE_JSON_FILE, METADATA_UNKNOWN, canonical_cloud_id)

Expand Down Expand Up @@ -40,11 +41,6 @@ def get_parser(parser=None):
return parser


def error(msg):
sys.stderr.write('ERROR: %s\n' % msg)
return 1


def handle_args(name, args):
"""Handle calls to 'cloud-id' cli.
Expand Down
11 changes: 6 additions & 5 deletions cloudinit/config/cc_apk_configure.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,7 @@
from cloudinit import temp_utils
from cloudinit import templater
from cloudinit import util
from cloudinit.config.schema import (
get_schema_doc, validate_cloudconfig_schema)
from cloudinit.config.schema import get_meta_doc, validate_cloudconfig_schema
from cloudinit.settings import PER_INSTANCE

LOG = logging.getLogger(__name__)
Expand Down Expand Up @@ -56,7 +55,7 @@

frequency = PER_INSTANCE
distros = ['alpine']
schema = {
meta = {
'id': 'cc_apk_configure',
'name': 'APK Configure',
'title': 'Configure apk repositories file',
Expand Down Expand Up @@ -95,6 +94,9 @@
"""),
],
'frequency': frequency,
}

schema = {
'type': 'object',
'properties': {
'apk_repos': {
Expand Down Expand Up @@ -171,14 +173,13 @@
""")
}
},
'required': [],
'minProperties': 1, # Either preserve_repositories or alpine_repo
'additionalProperties': False,
}
}
}

__doc__ = get_schema_doc(schema)
__doc__ = get_meta_doc(meta, schema)


def handle(name, cfg, cloud, log, _args):
Expand Down
11 changes: 7 additions & 4 deletions cloudinit/config/cc_apt_configure.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,7 @@
import pathlib
from textwrap import dedent

from cloudinit.config.schema import (
get_schema_doc, validate_cloudconfig_schema)
from cloudinit.config.schema import get_meta_doc, validate_cloudconfig_schema
from cloudinit import gpg
from cloudinit import log as logging
from cloudinit import subp
Expand Down Expand Up @@ -75,7 +74,8 @@
}
}
}
schema = {

meta = {
'id': 'cc_apt_configure',
'name': 'Apt Configure',
'title': 'Configure apt for the user',
Expand Down Expand Up @@ -155,6 +155,9 @@
<key data>
------END PGP PUBLIC KEY BLOCK-------""")],
'frequency': frequency,
}

schema = {
'type': 'object',
'properties': {
'apt': {
Expand Down Expand Up @@ -398,7 +401,7 @@
}
}

__doc__ = get_schema_doc(schema)
__doc__ = get_meta_doc(meta, schema)


# place where apt stores cached repository data
Expand Down
11 changes: 6 additions & 5 deletions cloudinit/config/cc_bootcmd.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,7 @@
import os
from textwrap import dedent

from cloudinit.config.schema import (
get_schema_doc, validate_cloudconfig_schema)
from cloudinit.config.schema import get_meta_doc, validate_cloudconfig_schema
from cloudinit.settings import PER_ALWAYS
from cloudinit import temp_utils
from cloudinit import subp
Expand All @@ -29,7 +28,7 @@

distros = ['all']

schema = {
meta = {
'id': 'cc_bootcmd',
'name': 'Bootcmd',
'title': 'Run arbitrary commands early in the boot process',
Expand Down Expand Up @@ -57,6 +56,9 @@
- [ cloud-init-per, once, mymkfs, mkfs, /dev/vdb ]
""")],
'frequency': PER_ALWAYS,
}

schema = {
'type': 'object',
'properties': {
'bootcmd': {
Expand All @@ -69,12 +71,11 @@
'additionalItems': False, # Reject items of non-string non-list
'additionalProperties': False,
'minItems': 1,
'required': [],
}
}
}

__doc__ = get_schema_doc(schema) # Supplement python help()
__doc__ = get_meta_doc(meta, schema) # Supplement python help()


def handle(name, cfg, cloud, log, _args):
Expand Down
11 changes: 7 additions & 4 deletions cloudinit/config/cc_chef.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,7 @@
from textwrap import dedent

from cloudinit import subp
from cloudinit.config.schema import (
get_schema_doc, validate_cloudconfig_schema)
from cloudinit.config.schema import get_meta_doc, validate_cloudconfig_schema
from cloudinit import templater
from cloudinit import temp_utils
from cloudinit import url_helper
Expand Down Expand Up @@ -89,7 +88,8 @@

frequency = PER_ALWAYS
distros = ["all"]
schema = {

meta = {
'id': 'cc_chef',
'name': 'Chef',
'title': 'module that configures, starts and installs chef',
Expand Down Expand Up @@ -126,6 +126,9 @@
ssl_verify_mode: :verify_peer
validation_name: yourorg-validator""")],
'frequency': frequency,
}

schema = {
'type': 'object',
'properties': {
'chef': {
Expand Down Expand Up @@ -357,7 +360,7 @@
}
}

__doc__ = get_schema_doc(schema)
__doc__ = get_meta_doc(meta, schema)


def post_run_chef(chef_cfg, log):
Expand Down
9 changes: 6 additions & 3 deletions cloudinit/config/cc_install_hotplug.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
from cloudinit import util
from cloudinit import subp
from cloudinit import stages
from cloudinit.config.schema import get_schema_doc, validate_cloudconfig_schema
from cloudinit.config.schema import get_meta_doc, validate_cloudconfig_schema
from cloudinit.distros import ALL_DISTROS
from cloudinit.event import EventType, EventScope
from cloudinit.settings import PER_INSTANCE
Expand All @@ -15,7 +15,7 @@
frequency = PER_INSTANCE
distros = [ALL_DISTROS]

schema = {
meta = {
"id": "cc_install_hotplug",
"name": "Install Hotplug",
"title": "Install hotplug if supported and enabled",
Expand Down Expand Up @@ -49,6 +49,9 @@
"""),
],
"frequency": frequency,
}

schema = {
"type": "object",
"properties": {
"updates": {
Expand Down Expand Up @@ -81,7 +84,7 @@
}
}

__doc__ = get_schema_doc(schema)
__doc__ = get_meta_doc(meta, schema)


HOTPLUG_UDEV_PATH = "/etc/udev/rules.d/10-cloud-init-hook-hotplug.rules"
Expand Down
9 changes: 6 additions & 3 deletions cloudinit/config/cc_locale.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,13 @@
from textwrap import dedent

from cloudinit import util
from cloudinit.config.schema import get_schema_doc, validate_cloudconfig_schema
from cloudinit.config.schema import get_meta_doc, validate_cloudconfig_schema
from cloudinit.settings import PER_INSTANCE


frequency = PER_INSTANCE
distros = ['all']
schema = {
meta = {
'id': 'cc_locale',
'name': 'Locale',
'title': 'Set system locale',
Expand All @@ -39,6 +39,9 @@
"""),
],
'frequency': frequency,
}

schema = {
'type': 'object',
'properties': {
'locale': {
Expand All @@ -57,7 +60,7 @@
},
}

__doc__ = get_schema_doc(schema) # Supplement python help()
__doc__ = get_meta_doc(meta, schema) # Supplement python help()


def handle(name, cfg, cloud, log, args):
Expand Down
11 changes: 6 additions & 5 deletions cloudinit/config/cc_ntp.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
from cloudinit import type_utils
from cloudinit import subp
from cloudinit import util
from cloudinit.config.schema import get_schema_doc, validate_cloudconfig_schema
from cloudinit.config.schema import get_meta_doc, validate_cloudconfig_schema
from cloudinit.settings import PER_INSTANCE

LOG = logging.getLogger(__name__)
Expand Down Expand Up @@ -140,7 +140,7 @@
# configuration options before actually attempting to deploy with said
# configuration.

schema = {
meta = {
'id': 'cc_ntp',
'name': 'NTP',
'title': 'enable and configure ntp',
Expand Down Expand Up @@ -190,6 +190,9 @@
- ntp.ubuntu.com
- 192.168.23.2""")],
'frequency': PER_INSTANCE,
}

schema = {
'type': 'object',
'properties': {
'ntp': {
Expand Down Expand Up @@ -289,12 +292,10 @@
},
# Don't use REQUIRED_NTP_CONFIG_KEYS to allow for override
# of builtin client values.
'required': [],
'minProperties': 1, # If we have config, define something
'additionalProperties': False
},
},
'required': [],
'additionalProperties': False
}
}
Expand All @@ -303,7 +304,7 @@
'check_exe', 'confpath', 'packages', 'service_name'])


__doc__ = get_schema_doc(schema) # Supplement python help()
__doc__ = get_meta_doc(meta, schema) # Supplement python help()


def distro_ntp_client_configs(distro):
Expand Down
10 changes: 6 additions & 4 deletions cloudinit/config/cc_resizefs.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,7 @@
import stat
from textwrap import dedent

from cloudinit.config.schema import (
get_schema_doc, validate_cloudconfig_schema)
from cloudinit.config.schema import get_meta_doc, validate_cloudconfig_schema
from cloudinit.settings import PER_ALWAYS
from cloudinit import subp
from cloudinit import util
Expand All @@ -24,7 +23,7 @@
frequency = PER_ALWAYS
distros = ['all']

schema = {
meta = {
'id': 'cc_resizefs',
'name': 'Resizefs',
'title': 'Resize filesystem',
Expand All @@ -42,6 +41,9 @@
'examples': [
'resize_rootfs: false # disable root filesystem resize operation'],
'frequency': PER_ALWAYS,
}

schema = {
'type': 'object',
'properties': {
'resize_rootfs': {
Expand All @@ -52,7 +54,7 @@
}
}

__doc__ = get_schema_doc(schema) # Supplement python help()
__doc__ = get_meta_doc(meta, schema) # Supplement python help()


def _resize_btrfs(mount_point, devpth):
Expand Down
Loading

0 comments on commit bedac77

Please sign in to comment.