orthanc-server / orthanc-builder

Repo used to build osimis/orthanc docker images, windows installers and OSX packages
GNU Affero General Public License v3.0
29 stars 12 forks source link

Env vars substitution problem #9

Closed andgineer closed 1 year ago

andgineer commented 1 year ago

In ENTRYPOINT we load config files and substitute env vars. Unfortunately we first load the configs as JSON and only after that we substitute ${}

That means we cannot use env vars for non-string values

For example "IndexConnectionsCount" : ${ORTHANC_DB_INDEX_CONNECTIONS_COUNT} because this is not valid JSON. And if we enclose the value to quotes that will be an invalid int value in the config.

Maybe we should consider using a more straightforward substitution approach like just envsubst for the files

amazy commented 1 year ago

Seems like we could indeed call envsubst from python before parsing the file

for filePath in configFiles:
  logInfo("reading configuration from " + filePath)
  with open(filePath, "r") as f:
    content = f.read()

------------ here -------------

    try:
      cleanedContent = removeCppCommentsFromJson(content)
      configFromFile = json.loads(cleanedContent)
    except:
      logError("Unable to parse Json file '{f}'; check syntax".format(f = filePath))

    configurator.mergeConfigFromFile(configFromFile, filePath)

The substitution that happens afterwards is somehow different since it uses Env var names to add values at the right place in the config file.

andgineer commented 1 year ago

that would be nice thing to have or may be to keep backward compatibility we can do that only on json load exception

amazy commented 1 year ago

it is safe to do it in anycase. Fixed in https://github.com/orthanc-server/orthanc-builder/commit/6e6c105c4c18ebab78aeebc09e5cfd7aca4c91c5