edgexfoundry / go-mod-bootstrap

go-mod-bootstrap
Apache License 2.0
16 stars 55 forks source link

Issue overriding AppCustom settings with enviroment variables #314

Closed siggiskulason closed 2 years ago

siggiskulason commented 2 years ago

There is an issue with overriding the AppCustom settings with environment variables if an existing value is already in Consul.

Example: configuration.toml contains the setting

[AppCustom]
# List of IPv4 subnets to perform LLRP discovery process on, in CIDR format (X.X.X.X/Y)
# separated by commas ex: "192.168.1.0/24,10.0.0.0/24"
DiscoverySubnets = ""

We then run the service (I have tested this both running natively and using the snap)

# this tokenfile was copied from edgexfoundry
export SECRETSTORE_TOKENFILE="/home/sidar/tmp/secrets-token.json"

export DEVICE_PROFILESDIR="/home/sidar/go/src/github.com/siggiskulason/device-rfid-llrp-go/cmd/res/profiles"
export APPCUSTOM_PROVISIONWATCHERDIR="/home/sidar/go/src/github.com/siggiskulason/device-rfid-llrp-go/cmd/res/provision_watchers"
export APPCUSTOM_DISCOVERYSUBNETS="192.168.20.0/24"
./cmd/device-rfid-llrp --confdir="/home/sidar/go/src/github.com/siggiskulason/device-rfid-llrp-go/cmd/res" --cp=consul://localhost:8500 --registry

Run the above - that will generate the key/value pairs in Consul, using the values from the config file, including an empy DiscoverySubnets string

What then happens when we run is that as expected we get a message about "Empty CIDR provided, unable to scan for LLRP readers."

Now stop the device service, set the environment variable and start it again

export APPCUSTOM_DISCOVERYSUBNETS="192.168.20.0/24"
./cmd/device-rfid-llrp --confdir="/home/sidar/go/src/github.com/siggiskulason/device-rfid-llrp-go/cmd/res" --cp=consul://localhost:8500 --registry

The problem is that when it starts up, we get it starting up with the overriden value as expected:

level=INFO ts=2022-02-14T23:27:49.984529529Z app=device-rfid-llrp source=variables.go:352 msg="Variables override of 'AppCustom.DiscoverySubnets' by environment variable: APPCUSTOM_DISCOVERYSUBNETS=192.168.20.0/24"

but then,updateWritableConfig(rawWritableConfig in driver.go gets called to update the value back to the setting in Consul

level=INFO ts=2022-02-14T23:27:49.986922829Z app=device-rfid-llrp source=config.go:322 msg="Updated custom configuration 'AppCustom' has been received from the Configuration Provider"
level=INFO ts=2022-02-14T23:27:49.986968116Z app=device-rfid-llrp source=driver.go:202 msg="Discover configuration has changed! Discovery will be triggered momentarily."

and we have the values

oldSubnets =  192.168.20.0/24 
new subnet = ""

so the value gets replaced by the setting in Consul and discovery fails, as it has an empty subnet value

level=WARN ts=2022-02-14T23:27:49.992889594Z app=device-rfid-llrp source=discover.go:92 msg="Empty CIDR provided, unable to scan for LLRP readers."

What this issue means in practice, is that it's not possible to override the DiscoverySubnet value in Consul. Therefore the "snap set" method of setting these values will not work, as it works by setting environment overrides.

lenny-goodell commented 2 years ago

root caused the issue to ListenForCustomConfigChanges in go-mod-bootstrap needs to ignore the first change notification same as regular config does here: https://github.com/edgexfoundry/go-mod-bootstrap/blob/main/bootstrap/config/config.go#L466-L472

The first change notification is bogus and causes issues with values that have been override with Env.