home-assistant/supervisor

Addons with `schema: false` are broken in 2021.02.5

yozik04 opened this issue Β· 30 comments

Describe the issue

Home Assistant addons that does not specify configuration schema seems to be broken. All provided settings are filtered out.

https://developers.home-assistant.io/docs/add-ons/configuration/ documentation tells:

Key Type Required Description
schema dict no Schema for options value of the add-on. It can be false to disable schema validation and use custom options.

If addon uses schema: false it will not be able to read any configuration from options.json
Example config here: https://github.com/ParadoxAlarmInterface/hassio-repository/blob/master/paradox_alarm_interface/config.json

Supervisor just strips all configuration options out.

I think bug was introduced with this change: https://github.com/home-assistant/supervisor/pull/2510/files

Steps to reproduce

  1. Update HA OS from 5.10 to 5.11
  2. Some plugins stop accepting configuration: ParadoxAlarmInterface/pai#199

Enviroment details

  • Operating System:: RPI4 HA OS
  • Supervisor version:: 2021.02.5
  • Home Assistant version: 2021.2.1

Supervisor logs

Supervisor logs
21-02-08 20:36:01 WARNING (MainThread) [supervisor.addons.options] Unknown options LOGGING_FILE
21-02-08 20:36:01 WARNING (MainThread) [supervisor.addons.options] Unknown options LOGGING_LEVEL_CONSOLE
21-02-08 20:36:01 WARNING (MainThread) [supervisor.addons.options] Unknown options LOGGING_LEVEL_FILE
21-02-08 20:36:01 WARNING (MainThread) [supervisor.addons.options] Unknown options LOGGING_DUMP_PACKETS
21-02-08 20:36:01 WARNING (MainThread) [supervisor.addons.options] Unknown options LOGGING_DUMP_MESSAGES
21-02-08 20:36:01 WARNING (MainThread) [supervisor.addons.options] Unknown options LOGGING_DUMP_STATUS
21-02-08 20:36:01 WARNING (MainThread) [supervisor.addons.options] Unknown options LOGGING_DUMP_EVENTS
21-02-08 20:36:01 WARNING (MainThread) [supervisor.addons.options] Unknown options CONNECTION_TYPE
21-02-08 20:36:01 WARNING (MainThread) [supervisor.addons.options] Unknown options SERIAL_PORT
21-02-08 20:36:01 WARNING (MainThread) [supervisor.addons.options] Unknown options SERIAL_BAUD
21-02-08 20:36:01 WARNING (MainThread) [supervisor.addons.options] Unknown options IP_CONNECTION_HOST
21-02-08 20:36:01 WARNING (MainThread) [supervisor.addons.options] Unknown options IP_CONNECTION_PORT
21-02-08 20:36:01 WARNING (MainThread) [supervisor.addons.options] Unknown options IP_CONNECTION_PASSWORD
21-02-08 20:36:01 WARNING (MainThread) [supervisor.addons.options] Unknown options IP_CONNECTION_SITEID
21-02-08 20:36:01 WARNING (MainThread) [supervisor.addons.options] Unknown options IP_CONNECTION_EMAIL
21-02-08 20:36:01 WARNING (MainThread) [supervisor.addons.options] Unknown options IP_CONNECTION_PANEL_SERIAL
21-02-08 20:36:01 WARNING (MainThread) [supervisor.addons.options] Unknown options IP_CONNECTION_BARE
21-02-08 20:36:01 WARNING (MainThread) [supervisor.addons.options] Unknown options LIMITS
21-02-08 20:36:01 WARNING (MainThread) [supervisor.addons.options] Unknown options SYNC_TIME
21-02-08 20:36:01 WARNING (MainThread) [supervisor.addons.options] Unknown options PASSWORD
21-02-08 20:36:01 WARNING (MainThread) [supervisor.addons.options] Unknown options MQTT_ENABLE
21-02-08 20:36:01 WARNING (MainThread) [supervisor.addons.options] Unknown options MQTT_HOST
21-02-08 20:36:01 WARNING (MainThread) [supervisor.addons.options] Unknown options MQTT_PORT
21-02-08 20:36:01 WARNING (MainThread) [supervisor.addons.options] Unknown options MQTT_USERNAME
21-02-08 20:36:01 WARNING (MainThread) [supervisor.addons.options] Unknown options MQTT_PASSWORD
21-02-08 20:36:01 WARNING (MainThread) [supervisor.addons.options] Unknown options MQTT_RETAIN
21-02-08 20:36:01 WARNING (MainThread) [supervisor.addons.options] Unknown options MQTT_BIND_ADDRESS
21-02-08 20:36:01 WARNING (MainThread) [supervisor.addons.options] Unknown options MQTT_BIND_PORT
21-02-08 20:36:01 WARNING (MainThread) [supervisor.addons.options] Unknown options MQTT_HOMEASSISTANT_AUTODISCOVERY_ENABLE
21-02-08 20:36:01 WARNING (MainThread) [supervisor.addons.options] Unknown options MQTT_PUBLISH_STATUS
21-02-08 20:36:03 WARNING (MainThread) [supervisor.addons.options] Unknown options port

System Information

System Information
arch: aarch64
channel: stable
docker: 19.03.13
features:
- reboot
- shutdown
- services
- network
- hostname
- hassos
hassos: "5.11"
homeassistant: 2021.2.2
hostname: homeassistant
logging: info
machine: raspberrypi4-64
operating_system: Home Assistant OS 5.11
state: running
supervisor: 2021.02.6
supported: true
supported_arch:
- aarch64
- armv7
- armhf
timezone: Europe/Brussels

That was stale documentation: home-assistant/developers.home-assistant#800

So we fix the bug and not created one :)

That is evil. I doubt this is a good decision.
I have more than 100 options that people can define and for most people just a 10 is sufficient. Now I will have to show them all 100 and get a lot of reports that something is not working because they have changed a setting without reading documentation.

You don't have to show them all? You can still make optional settings in a schema (I do that all the time).

Any logical reason why extra options are bad?

Extra options are not bad? You can mark them optional.

I would lobby for bringing the old behavior back as well - this change has broken the room-assistant add-on for a lot of users without warning (or at least I didn't read anything about this before my users approached me on GitHub with the issue). Bringing the complex option tree under schema validation is quite difficult. Last time I tried I failed to depict the YAML tree using the schema correctly, which is the whole reason I disabled it.

I would be interested to hear more about the reasoning for this change.

EDIT:

Only nested arrays and dictionaries are supported with a deep of two size

This alone makes it impossible for me to write a schema for the room-assistant config. Only way to make it work is by making the application config YAML an inline document, which is a breaking change and isn't great from a UX point of view imo.

EDIT 2:

According to a conversation on Discord this limitation doesn’t really exist and was more meant as a recommendation. That would make some parts of my previous arguments void, but it still doesn’t allow for some flexibility features such as polytypes. Writing a schema is also still a time consuming task for projects with bigger configs.

I really doubt if I will be able to create a valid json schema that would cover all the options I have in my addon. JSON schema format is so limited.

JSON schema format is so limited

If there is schema validation missing, please raise that separately. Thanks πŸ‘

Forced schema validation has broken PAI add-on. I cannot seem to toll back to a previous version of Supervisor either using the HA CLI command. What can I do to roll back or should I apply an old snapshot?

@pvizeli Even if you see your approach as the correct behavior, what was done was a breaking change without a prior notification.
IMHO, you should mark "schema: false" as deprecated, specify some kind of deadline for removal and allow devs of the addons that use it to align, then when your deadline arrived you can remove it.
It's really a bad approach to introduce such drastic changes in bevahior without prior notification.

I can say I'm rather new to HA (using it for about two years) but the number of times the Supervisor updates is breaking things is pretty uneasy.
And on-top of that you can't downgrade the Supervisor so you get back to a release which was working for you (until the broken thing gets fixed) because there's the auto-upgrade feature (which I understand the reason for, sure).

Maybe a "postpone auto-updates for x days" CLI option when downgrading the Supervisor would be helpful. Even Windows 10 with their aggressive Updates allows for posponing them :). Think about it.

@frenck and @pvizeli So with this "fix" you have broken 3 addons. Can you please at least tell us why this "fix" was required and what it actually solves?
You just forced addon developers to write and maintain two separate config validation solutions in a different repos. Keeping them in sync is extra useless effort.
Why supervisor deals with validation at all. It is not its responsibility. Only addon knows what setting combination is valid and what is not. Ok, let it do it for addons with simple configuration if a dev wants it. But why it must enforce schema validation for all? I do not understand.

@frenck How can I specify in schema so a sub dict could have any keys and values?

{
"a": {<anykey=anyvalue_or_sub_dict>,...}
}

And please reopen this issue if you do not want people to create another dozen of dupes. Our debate is in progress. Closing the issue before a solution is found is a bad manner.

@yozik04 A schema can have dicts and subvalues, that has been available since the early beginnings.
For example https://github.com/hassio-addons/addon-ftp/blob/main/ftp/config.json#L34

But a schema is explicit, "any" is not part of that.

Why supervisor deals with validation at all. It is not its responsibility.

Yes, it is, it always has been.

@frenck And can we have that "any" part? Can you or @pvizeli implement it? Some "json" or what ever type?

IMHO, that is a bad idea in general. I (personally) would not support it.

@frenck Is it possible to make a configuration sub tree optional?

{
"a": {"b":"c"},
"optional": {"a": "b"}
}

That is the same question you have asked above twice already in different words. If someone decides it should be part of the add-on configuration πŸ€·β€β™‚οΈ As for my personal opinion, I would not support that.

No I mean this way:

{
    "a": {
        "b": ["int"],
        "optional": ["int"]
    },
}

Some hardware that we support does not have "optional" sub configuration.

Options can be marked with a ? to make them optional.

So only leaf nodes can be optional?

The optionality is not limited to roots or leaves. It applies to a node by itself.

{
    "a": {
        "b": ["int"],
        "optional?": ["int"]
    },
}

So optionality in keys also supported? See optional?.

{
    "a": {
        "b": ["int"],
        "optional": ["int?"]
    },
}

that optional, is already optional?

As one doesn't have to provide values?

optional: []

@frenck "email?" and "device?" types does not want to be optional. Validation fails if we do not provide a value.

all provided validation types should be able to support the optional flag. If you conclude that is not the case, please raise a separate issue for that with a reproduction scenario, so we can take a look at the reason for that.

@frenck How does one specify an optional string value with default value "null" ?
This one, for example, does not work, it makes LOGGING_FILE mandatory:

"options": {
    "LOGGING_FILE": null
},
"schema": {
    "LOGGING_FILE": "str?"
}

@clau-bucur Lets not stack issues. Please raise a separate issue if you have a specific case. Thanks πŸ‘

I'd say was a simple question related to the issue at hand. But ok. Thanks.

@frenck Thank you for your answers. It was helpful. But you really like to skip questions or answer partially... That makes a negative effect.

I'm sorry, it helps if an issue is created for a single thing. If you need a chat or support, please join our Discord server, as we have developer channels on there.