search

balena-os / balena-supervisor

Balena Supervisor: balena's agent on devices.
https://balena.io
Other
150 stars 63 forks source link

Allow dots in environment variable names #1104

Closed pdcastro closed 4 years ago

pdcastro commented 5 years ago

As originally reported in the CLI repo: https://github.com/balena-io/balena-cli/issues/1446

The problem is caused by the dots in the env var name, but the loraserver application is designed this way and also running this with official docker-compose works as expected.

version: "2.1"

services:
  gatewaybridge:
    image: arribada/lora-gateway-bridge
    ports:
      - 1700:1700/udp
    environment:
      - INTEGRATION.MQTT.AUTH.GENERIC.SERVER=tcp://mosquitto:1883
  mosquitto:
    image: eclipse-mosquitto
DEBUG=1 balena push 192.168.0.113

returns an error:

[Debug]   Sending request to http://192.168.0.113:48484/v2/local/target-state
BadRequestDeviceAPIError: Invalid apps
    at promisifiedRequest.Bluebird.fromCallback.then (/snapshot/versioned-source/build/utils/device/api.js:135:27)
    at tryCatcher (/snapshot/versioned-source/node_modules/bluebird/js/release/util.js:16:23)
    at Promise.module.exports.Promise._settlePromiseFromHandler (/snapshot/versioned-source/node_modules/bluebird/js/release/promise.js:517:31)
    at Promise.module.exports.Promise._settlePromise (/snapshot/versioned-source/node_modules/bluebird/js/release/promise.js:574:18)
    at Promise.module.exports.Promise._settlePromise0 (/snapshot/versioned-source/node_modules/bluebird/js/release/promise.js:619:10)
    at Promise.module.exports.Promise._settlePromises (/snapshot/versioned-source/node_modules/bluebird/js/release/promise.js:699:18)
    at Promise.module.exports.Promise._fulfill (/snapshot/versioned-source/node_modules/bluebird/js/release/promise.js:643:18)
    at Request.module.exports [as _callback] (/snapshot/versioned-source/node_modules/bluebird/js/release/nodeback.js:45:21)
    at Request.init.self.callback (/snapshot/versioned-source/node_modules/request/request.js:185:22)
    at Request.emit (events.js:189:13)
    at Request.EventEmitter.emit (domain.js:441:20)
    at Request.<anonymous> (/snapshot/versioned-source/node_modules/request/request.js:1161:10)
    at Request.emit (events.js:189:13)
    at Request.EventEmitter.emit (domain.js:441:20)
    at IncomingMessage.<anonymous> (/snapshot/versioned-source/node_modules/request/request.js:1083:12)
    at Object.onceWrapper (events.js:277:13)
    at IncomingMessage.emit (events.js:194:15)
    at IncomingMessage.EventEmitter.emit (domain.js:441:20)
    at endReadableNT (_stream_readable.js:1125:12)
    at process._tickCallback (internal/process/next_tick.js:63:19)

the same works running it locally with 

docker-compose up
pdcastro commented 5 years ago

The following code seems relevant:

const ENV_VAR_KEY_REGEX = /^[a-zA-Z_][a-zA-Z0-9_]*$/;
...
    if (!ENV_VAR_KEY_REGEX.test(key)) {
        log.debug(
            `Invalid env var key passed to validation.isValidEnv\nKey: ${inspect(key,}`,
pdcastro commented 5 years ago

Also (not sure if the change would be in balena-supervisor or balena-cli), it would be good to expose the more detailed error message (as above: Invalid env var key passed to validation.isValidEnv) to the CLI. Currently, what users get is BadRequestDeviceAPIError: Invalid apps.

pdcastro commented 5 years ago

Loosely related supervisor issue: #748 Better handle spaces and other special characters in environment variables

krasi-georgiev commented 5 years ago

I have hit another problem related to special characters. This time with the env value.

again in local mode this returns an error

balena push --env POSTGRESQL_DSN=postgres://loraserver_ns:59XTH2995@postgresql/loraserver_ns?sslmode=disable 192.168.10.103

tried escaping, quotes etc, but nothing worked. I have now added a workaround(modified the golang app a bit), but I am thinking the proper soltuion should be that adding quotes should be enough to not cause problems with any special characters.

relevant docker compose entry

loraserver:
    image: arribada/loraserver
    environment:
      - REDIS_URL=redis://redis:6379
      - NETWORK__SERVER_BAND_NAME=EU_863_870
      - NETWORK__SERVER_GATEWAY_BACKEND_MQTT_SERVER=tcp://mosquitto:1883
      - JOIN__SERVER_DEFAULT_SERVER=http://appserver:8003
      - GEOLOCATION__SERVER_SERVER=geoserver:8005
krasi-georgiev commented 5 years ago

https://github.com/brocaar/lora-app-server/issues/369 Had a reply upstream that using dots in the env names should be removed so the only issue now is that the env value can't contain : which is needed in the POSTGRESQL_DSN env variable

krasi-georgiev commented 5 years ago

loraserver doesn't need dots in the env vars now so only : should be added.

CameronDiver commented 4 years ago

@krasi-georgiev the value of the environment variable can contain :, that isn't an issue, this issue is related to the variable names. If you're having problems with the value of environment variables, could you make an issue please?

krasi-georgiev commented 4 years ago

are you saying that something like this works for you?

balena push --env POSTGRESQL_DSN=postgres://loraserver_ns:59XTH2995@postgresql/loraserver_ns?sslmode=disable 192.168.10.103
krasi-georgiev commented 4 years ago

the original issues was for dots in the env names, but dots in env vars is not valid in the Unix systems anyway so the fact the compose supports it was probably just an overlook.

CameronDiver commented 4 years ago

@krasi-georgiev I did a little investigation, and this is actually an issue with the CLI parsing command line arguments, the supervisor is more than happy to accept this.

I've created an issue you can track here: https://github.com/balena-io/balena-cli/issues/1705 and a workaround would be to provide these env vars in your docker-compose.yml.

I'm going to close this issue, because as you say, dots in env names aren't allowed in Unix systems.