source: allow to specify source ID in autoinstall config

[ogayot]

This PR brings support for the following data in autoinstall configuration:

source:
  id: ubuntu-server-minimal

One of the particularity of the source controller is that it does not have access to the source catalog by the time the autoinstall data is loaded. Therefore, we have to store the data first and do the magic later in apply_autoinstall_config.

The search_drivers key used to be required as part of the source section. This made sense (I wonder, really?) when search_drivers was the only key supported (what's the point of declaring the section if we're not setting any value?) but it should be made optional now since the user may want to specify the source ID without bothering with drivers stuff.

moved to https://github.com//pull/1377

Also, I realized that the top level autoinstall sections accepted the `null` value although such a configuration should have been rejected according to the JSON schema:

source: null      # invalid - source should be of type object
keyboard: null    # invalid - keyboard should be of type object
snaps: null       # invalid - snaps should be of type array
...

This happened because the JSON validation was skipped when a section was set to null. The goal was to allow the controllers to set autoinstall_default to None (which is a sensible thing to do).

However, instead of skipping validation when the section resolves to None, the better thing to do is to skip the validation when autoinstall_default is effectively used.

If we want to accept null for a given section, we can by adding "null" to the list of accepted types:

- "type": "object"
+ "type": ["object", "null"]

the proxy section is an example.

[dbungert]

Thank you, both for addressing the immediate need for source selection and the general schema validation improvements. All of this is great.

I think we should make the needed schema changes to allow foo: null for all toplevel keys except version. There is a small risk that someone has used that to explicitly say "give me the defaults for this section" rather than relying on obtaining that by omission, and proxy serves as a sort of precedent for that.

If there are other cases like version where that doesn't make sense, let's discuss.

[ogayot]

Thank you, both for addressing the immediate need for source selection and the general schema validation improvements. All of this is great.

I think we should make the needed schema changes to allow foo: null for all toplevel keys except version. There is a small risk that someone has used that to explicitly say "give me the defaults for this section" rather than relying on obtaining that by omission, and proxy serves as a sort of precedent for that.

If there are other cases like version where that doesn't make sense, let's discuss.

Yes, I see your point but I don't think this is sane to let the users think they can pass section: null and expect to get the default values because in many cases, this is not what they get:

For the following controllers, we set a custom value that gets used when said section is absent:

subiquity/server/controllers/cmdlist.py:    autoinstall_default = []
subiquity/server/controllers/debconf.py:    autoinstall_default = ""
subiquity/server/controllers/drivers.py:    autoinstall_default = {"install": False}
subiquity/server/controllers/locale.py:    autoinstall_default = 'en_US.UTF-8'
subiquity/server/controllers/package.py:    autoinstall_default = []
subiquity/server/controllers/snaplist.py:    autoinstall_default = []
subiquity/server/controllers/source.py:    autoinstall_default = {"search_drivers": True}
subiquity/server/controllers/timezone.py:    autoinstall_default = ''
subiquity/server/controllers/updates.py:    autoinstall_default = 'security'
subiquity/server/controllers/userdata.py:    autoinstall_default = {}

but if the section is set to null, we won't load the default autoinstall value, we will try to load None instead.
For instance with locale, this is what we get:

    def load_autoinstall_data(self, data):
        log.debug("data is %s", data)
        os.environ["LANG"] = data

with no locale section:

2022-08-03 16:50:55,038 DEBUG subiquity.server.controllers.locale:36 data is en_US.UTF-8

with locale: null:

2022-08-03 16:52:10,087 DEBUG subiquity.server.controllers.locale:36 data is None

(and also it fails later on):

2022-08-03 16:52:10,088 ERROR root:37 finish: subiquity/Locale/load_autoinstall_data: FAIL: str expected, not NoneType
2022-08-03 16:52:10,089 ERROR subiquity.server.server:422 top level error
Traceback (most recent call last):
  File "/home/olivier/dev/canonical/subiquity/subiquity/server/server.py", line 653, in start
    self.load_autoinstall_config(only_early=False)
  File "/home/olivier/dev/canonical/subiquity/subiquity/server/server.py", line 492, in load_autoinstall_config
    controller.setup_autoinstall()
  File "/home/olivier/dev/canonical/subiquity/subiquity/server/controller.py", line 69, in setup_autoinstall
    self.load_autoinstall_data(ai_data)
  File "/home/olivier/dev/canonical/subiquity/subiquity/server/controllers/locale.py", line 37, in load_autoinstall_data
    os.environ["LANG"] = data
  File "/usr/lib/python3.10/os.py", line 684, in __setitem__
    value = self.encodevalue(value)
  File "/usr/lib/python3.10/os.py", line 756, in encode
    raise TypeError("str expected, not %s" % type(value).__name__)
TypeError: str expected, not NoneType

I would prefer rejecting null values where they weren't supposed to be.
But if we decide otherwise and want to accept null everywhere, we need to make these controllers smarter for sure.

[ogayot]

I created #1377 to address the issue with the top-level sections potentially being null.
What's left in this PR is only related to selecting the source during autoinstall.

[dbungert]

LGTM.
A runtests update to check for this value would be nice.

[ogayot]

LGTM. A runtests update to check for this value would be nice.

Good point, I added one test.