cc_locale: introduce schema

[OddBloke]

No description provided.

[blackboxsw]

I think you are missing an 'additionalProperties': False
which makes sure we don't allow anything besides locale/locale_configfile in the config dict.

[OddBloke]

I don't think so in this case, because that rejects any top-level keys other than "locale" and "locale_configfile", meaning if I add it then this file:

#cloud-config
locale: ar
locale_configfile: "some string"
runcmd: ["foo"]

gives this error:

Cloud config schema errors: : Additional properties are not allowed ('runcmd' was unexpected)

(I think you're thinking of a case like cc_ubuntu_drivers.py where the value of a config key is an object; in those cases, the schema for that config key can define the full set of keys that the object should take.)

[blackboxsw]

Ahh right you are correct. Since these are top-level properties and not nested objects, we don't provide the strict schema definition preventing additional keys. We may want tighten that in the GLOBAL_SCHEMA definition once we have full coverage, but this is not something for individual config module schema definitions to provide.

[blackboxsw]

There are a few cloud config modules that actuall read args (which I think is only possibly sourced from /etc/cloud/cloud.cfg(.d/*)?)

In those modules case, I think we would need to cfg.update({'locale': args[0]}) before calling validate_cloudconfig_schema.
We also might need to think about how we would have our schema validation reject such content (as some image could have provided an invalid value for that locale)

[blackboxsw]

I also think we probably need docs on this 'secret' way to pass config options to each module in /etc/cloud/cloud.cfg
https://github.com/canonical/cloud-init/blob/master/cloudinit/stages.py#L752-L772

[raharper]

I don't think we need to validate "Args" it's not in the config passed to the module; that's a runtime decision to use that instead ... and the "secret" way is incredible awkward since you 1) have to know the list of modules defined in cloud-init by default since we have no way of appending/removing items from the config module list 2) it's not documented as you say.

[OddBloke]

We may not need to validate args, but we do need to be sure that our validation isn't going to always fail, breaking this use case. (Given that we don't require keys to be present, I think it should be fine, but I'll spend a bit of time convincing myself of this.)

[OddBloke]

Other places that take args do not do anything specific, so I think we're fine to leave this as-is.

[blackboxsw]

Agreed, since providing args is an undocumented secret, and each module that uses args makes that decision in an unique way based on the number of positional args it expects.

The two paths where folks can trigger the 'args' parsing are:

1. hijacking/overriding the list of cloud_final_modules, cloud_config_modules, cloud_init_modules with an /etc/cloud/cloud.cfg.d/* file and providing a list for the cfg module:

• [cc_ssh_import_id, ALWAYS, ubuntu, chad.smith]

2. calling the CLI cloud-init single cc_ssh_import_id --freq always ubuntu chad.smith

Since these are not documented uses of the single module, and since each module behaves uniquely related to the positional args, I think it's safe for us to avoid schema coverage for that use.

[blackboxsw]

LGTM. Thanks for this Dan!