Sideeffects of new `${ENV:<MY_VAR>}` based thing configurations

[holger_hees]

Hello,

First of all, I really like this feature; it makes my own deployment so much easier. That said, I might have gone a bit overboard with moving config values ​​into environment variables On the upside, it serves as a very comprehensive test of the impact.

In principle, it works, and minor issues can be bypassed using workarounds. However, below is a list of the difficulties I encountered.

1. As soon as I use environment variables, the WebUI no longer displays the final value, but only the placeholder. One could certainly debate what should be displayed here, but in my opinion, the actual value should be shown rather than the placeholder.

However, this is more of a “nice-to-have.”

2. Channel and service configs should work similarly to Thing configs. For example, JDBC and MySQL service configurations also contain passwords. Or, with the MQTT binding, one might want to use channel topics as environment variables. However, this is also more of a “nice-to-have” and is mentioned here only for the sake of completeness.

3. The Shelly binding. Here, I’ve moved both the IP address and the password into environment variables.

It works fine so far, but as soon as I open the Shelly Manager (‘/shelly/manager’), I get a 500 error with the following exception.

My guess is that the Shelly Manager receives the raw IP string—meaning the one with the unresolved placeholder and therefore runs into some kind of parsing issue.

If I replace the IP placeholder with reals values, the shelly manager works again.

4. The Home Connect Direct binding. Here, I have moved, both the IP address and the Device UID (Home Application ID), to env vars.

However, as can be seen in the Home Connect Manager (‘/home connect direct/appliances’), the variables are displayed here in their “unresolved” state as well. This, too, is a “nice-to-have” feature.

However, the problem is that the binding is no longer correctly assigning the profile. It keeps prompting me to upload one, which, of course, I have already done. My guess is that he can’t find the right profile based on device UID anymore. Because it still contains the placeholder.

This is not a huge problem for me, because it affects only “special channel” from the profile. Basic channels are not affected, which means that the homeconnect binding itself works fro me. If I replace the placeholder for the device UID (Home Appliance ID) with the real value, it works.

---

My question now is: is there a more central point where all of this can be handled? As I understand it, the bindings themselves should never need to be aware of the existence of these placeholders; any requests for these configuration values ​​should always be resolved beforehand, including for the display in the Web UI.

The second solution would be to fix all these special cases individually within the bindings, though this would be very labor-intensive and prone to error.

[Nadahar (Nadar)]

@holger_hees

My question now is: is there a more central point where all of this can be handled? As I understand it, the bindings themselves should never need to be aware of the existence of these placeholders; any requests for these configuration values ​​should always be resolved beforehand, including for the display in the Web UI.

The second solution would be to fix all these special cases individually within the bindings, though this would be very labor-intensive and prone to error.

I haven’t followed this, and don’t know how it’s done. But, this seems like a bit of a conundrum. The configurations are part of OSGi, so if you save the actual value there instead of the placeholder, the value will become part of the configuration permanently. Updating the environment value won’t be reflected. The only way to keep this “live” is to maintain the placeholder as the configured value.

When it comes to how bindings deal with this, there is a lot of variation. If the substitution has been made a part of the “get configuration as” routine, that won’t suffice. I’m pretty sure that e.g. the Shelly binding processes the raw configuration parameters in various places, it use them as a parameter map, not as a ShellyConfiguration instance. I expect it to be quite a lot of that, and frankly, that the only way you can deal with dynamic configuration updates.

The “standard” working of a Thing handler is to dispose of and recreate the handler every time a configuration changes. This is fine for simple bindings, but isn’t always a good solution in more complex situation. Bindings can override this “simple” dispose/reinit system and actually handle updates dynamically. My guess is that all these situations will fail to handle the substitution, and as far as I can tell, there’s not really a way to do this, since the source of the configuration parameters is OSGi itself.

[Nadahar (Nadar)]

Also, when thinking about it, the placeholder must be kept in the configuration to have any “value” in hiding the actual value from LLMs etc. It must be substituted “as late as possible” so that what you get from the REST API etc. is the placeholder value.

But, if the substitution is done in the “get configuration as” routine, any code can call that, so it’s not really protected. Any code that want to find the values of the substituted values can do so.

Another thing is that this will clash with the UI validation for the context, so you won’t be able to use the placeholders from the UI for things like IP addresses, URLs etc.

[florian-h05 (Florian H.)]

@holger_hees

1. As soon as I use environment variables, the WebUI no longer displays the final value, but only the placeholder. One could certainly debate what should be displayed here, but in my opinion, the actual value should be shown rather than the placeholder.

It was deliberately chosen to not display the actual value – the idea behind this feature was to also allow our demo.openhab.org server to access APIs that require credentials and keep those credentials private – so they must not be available through REST.

@holger_hees

2. Channel and service configs should work similarly to Thing configs. For example, JDBC and MySQL service configurations also contain passwords. Or, with the MQTT binding, one might want to use channel topics as environment variables. However, this is also more of a “nice-to-have” and is mentioned here only for the sake of completeness.

@holger_hees

3. The Shelly binding. Here, I’ve moved both the IP address and the password into environment variables.

Those are difficult to handle automatically. Things retrieve their config through a method defined in the BaseThingHandler, so it is super easy for openHAB Core to automatically resolve vars there. Channel and service configs are “manually pulled” by the add-on’s through no dedicated way, so we cannot hook into this to resolve variables and at the same time avoid variables to make it into JSONDB backups, REST, etc.

@holger_hees

My question now is: is there a more central point where all of this can be handled?

See my statement above – I had put a lot of thoughts into this when developing it and decided to only support it for Thing config for above reasons.

[Nadahar (Nadar)]

@florian-h05

Things retrieve their config through a method defined in the BaseThingHandler

That’s the “flaw” in the logic. Those methods can be and is overridden, and even BaseThingHandler itself is optional. It’s just a “template” that does a lot of standard things for you in the most common situations, you can’t rely on that path always being used.

[florian-h05 (Florian H.)]

@Nadahar

That’s the “flaw” in the logic. Those methods can be and is overridden, and even BaseThingHandler itself is optional. It’s just a “template” that does a lot of standard things for you in the most common situations, you can’t rely on that path always being used.

If that path isn’t used it “is not my problem” from a core perspective that a ThingHandler then is unable to use ENV vars in its configuration … for stuff where we have no standard path, it’s a different situation. But if there is a standard path and it isn’t used, it is no core issue but a ThingHandler issue.
In that case, the ThingHandler is responsible for manually calling the method to resolve variables.

[Nadahar (Nadar)]

@florian-h05

If that path isn’t used it “is not my problem” from a core perspective that a ThingHandler then is unable to use ENV vars in its configuration …
In that case, the ThingHandler is responsible for manually calling the method to resolve variables.

I disagree - that has never been the premise. I’m pretty sure that you’ll find a lot of binding that don’t do this, I have even written a pretty simple binding that doesn’t do this. It’s just something that is provided “for your convenience”, not something you’re in any way mandated to use.

The “contract” is the usual, ThingHandler define what you must fulfill “to be a ThingHandler”. BaseThingHandler is an entirely optional aid to get you there quicker. The whole idea of “not my problem” goes against the very principles of OO.

[florian-h05 (Florian H.)]

Can you please provide an example how you load the config?

[Nadahar (Nadar)]

bundles/org.openhab.binding.milllan/src/main/java/org/openhab/binding/milllan/internal/AbstractMillThingHandler.java

[florian-h05 (Florian H.)]

@Nadahar

The “contract” is the usual, ThingHandler define what you must fulfill “to be a ThingHandler”. BaseThingHandler is an entirely optional aid to get you there quicker. The whole idea of “not my problem” goes against the very principles of OO.

If you do not extend BaseThingHandler, you have to handle configuration changes yourself (or if you override handleConfigurationUpdated) including notifying the Thing manager, so all of the config change handling already is “your problem” then … resolving variables in configuration only becomes an additional step then.

We can check if we can move the config variable resolution up in the chain, possible to the ThingRegistryImpl that calls ThingHandler::handleConfigurationUpdated.

[Nadahar (Nadar)]

@florian-h05

If you do not extend BaseThingHandler, you have to handle configuration changes yourself (or if you override handleConfigurationUpdated) including notifying the Thing manager, so all of the config change handling already is “your problem” then … resolving variables in configuration only becomes an additional step then.

You can view it however you like, but the point is that this was always just convenience functionality, you can’t all of a sudden claim that “those that don’t use it are to blame” for breaking this. Since the substitution was never a thing, it isn’t handled, and in the case of my binding for example, it can’t be handled, because the code is written to work with OH 4.1 and up, so I can’t use anything that only exists in more recent versions, like some new method to substitute parameters.

[florian-h05 (Florian H.)]

I implemented it the best way I found back then, I don’t know what to add here.
If a better place to invoke the variable resolution is found, let’s change it – as long as the resolved values stay out of persisted config or the REST API, I’m fine with it.

[Nadahar (Nadar)]

I haven’t looked at the details, there might be some way to make it work more universally (like substituting the values before handleConfigurationUpdated() is called), but ultimately, I’m guessing that some bindings must be modified for this to work in any case.

[Nadahar (Nadar)]

Changing the UI context validation shouldn’t be forgotten either… or using this will only be available for file configurations.

[holger_hees]

@florian-h05

I implemented it the best way I found back then, I don’t know what to add here.
If a better place to invoke the variable resolution is found, let’s change it – as long as the resolved values stay out of persisted config or the REST API, I’m fine with it.

I agree, that it is not easy to find a solution which works for all.

My purpose with this topic was just to document my findings somewhere and start a discussion/brainstorming about it.

The current solution already works for me. With some workarounds/limitations. But it is usable and a huge help to simplify things.

[Nadahar (Nadar)]

I think doing the substitution in ThingRegistryImpl here, could make it work quite a lot broader:

    @Override
    public void updateConfiguration(ThingUID thingUID, Map<String, Object> configurationParameters) {
        Thing thing = get(thingUID);
        if (thing != null) {
            ThingHandler thingHandler = thing.getHandler();
            if (thingHandler != null) {
                thingHandler.handleConfigurationUpdate(configurationParameters);
            } else {
                throw new IllegalStateException("Thing with UID " + thingUID + " has no handler attached.");
            }
        } else {
            throw new IllegalArgumentException("Thing with UID " + thingUID + " does not exist.");
        }
    }

[florian-h05 (Florian H.)]

So I did some investigation, and the ThingHandler is the highest layer we can resolve variables. Above would be the Thing, but resolving it there would make variables accessible over REST or persisted in config.

So ThingHandler::handleConfigurationUpdate is a good place to resolve vars, when a ThingHandler calls Thing::getConfiguration() it still has to resolve vars itself …

[Nadahar (Nadar)]

@florian-h05

So ThingHandler::handleConfiguration is a good place to resolve vars, when a ThingHandler calls Thing::getConfiguration() it still has to resolve vars itself …

Are you saying that we can’t do it in ThinkRegistryImpl.updateConfiguration()? That shouldn’t impact what’s available in the REST API, and it has the benefit of working also when handleConfigurationUpdate() is overridden.

[florian-h05 (Florian H.)]

No, I meant to say that your proposal seems reasonable and I think might be the only place we can do it. I just meant to say that it might not cover 100% of cases, but significantly more than right now.

[Nadahar (Nadar)]

I’m just not sure yet if everything goes through that method. I’m trying to debug it. Is the initial configuration set that way for example? I’m not sure.