Skip to content

Configuration fixes for Docker and docker-compose#2043

Closed
sumdog wants to merge 2 commits intoiv-org:masterfrom
sumdog:docker-updates
Closed

Configuration fixes for Docker and docker-compose#2043
sumdog wants to merge 2 commits intoiv-org:masterfrom
sumdog:docker-updates

Conversation

@sumdog
Copy link

@sumdog sumdog commented Apr 28, 2021

The current docker-compose file indicates the entire configuration for invidious can be placed inside a single environment variable, but I couldn't get configuration changes to take effect. Unless I'm missing something, it seems like the program's entrypoint and crystal configuration loading does support pulling in the entire configuration from an environment variable (maybe it did at one time?)

I've adjusted the docker-compose file to define individual configuration variables and allow for custom usernames and password for the database initialization. I currently use a sed command in startup to adjust the SQL files, which is kinda hackey, and it seems like this might be addressed in #1678.

The configmap via Kubernetes seems to correctly support injecting a configuration file, but might break if this PR's startup replaces that configuration file on startup.

@unixfox
Copy link
Member

unixfox commented Apr 28, 2021

The configmap via Kubernetes seems to correctly support injecting a configuration file, but might break if this PR's startup replaces that configuration file on startup.

Maybe we should just ditch the configmap and use environment variables for the helm chart.

@sumdog
Copy link
Author

sumdog commented Apr 29, 2021

Maybe we should just ditch the configmap and use environment variables for the helm chart.

I added f9891db which will check to see if the config file exists and skip overwriting it. This should allow to keep the existing helm chart working for now.

@saltycrys
Copy link
Member

Loading config is already possibly from a YAML file, a single env var containing YAML and multiple env vars to override specific preferences. See

def self.load
# Load config from file or YAML string env var
env_config_file = "INVIDIOUS_CONFIG_FILE"
env_config_yaml = "INVIDIOUS_CONFIG"
config_file = ENV.has_key?(env_config_file) ? ENV.fetch(env_config_file) : "config/config.yml"
config_yaml = ENV.has_key?(env_config_yaml) ? ENV.fetch(env_config_yaml) : File.read(config_file)
config = Config.from_yaml(config_yaml)
# Update config from env vars (upcased and prefixed with "INVIDIOUS_")
{% for ivar in Config.instance_vars %}
{% env_id = "INVIDIOUS_#{ivar.id.upcase}" %}
if ENV.has_key?({{env_id}})
# puts %(Config.{{ivar.id}} : Loading from env var {{env_id}})
env_value = ENV.fetch({{env_id}})
success = false
# Use YAML converter if specified
{% ann = ivar.annotation(::YAML::Field) %}
{% if ann && ann[:converter] %}
puts %(Config.{{ivar.id}} : Parsing "#{env_value}" as {{ivar.type}} with {{ann[:converter]}} converter)
config.{{ivar.id}} = {{ann[:converter]}}.from_yaml(YAML::ParseContext.new, YAML::Nodes.parse(ENV.fetch({{env_id}})).nodes[0])
puts %(Config.{{ivar.id}} : Set to #{config.{{ivar.id}}})
success = true
# Use regular YAML parser otherwise
{% else %}
{% ivar_types = ivar.type.union? ? ivar.type.union_types : [ivar.type] %}
# Sort types to avoid parsing nulls and numbers as strings
{% ivar_types = ivar_types.sort_by { |ivar_type| ivar_type == Nil ? 0 : ivar_type == Int32 ? 1 : 2 } %}
{{ivar_types}}.each do |ivar_type|
if !success
begin
# puts %(Config.{{ivar.id}} : Trying to parse "#{env_value}" as #{ivar_type})
config.{{ivar.id}} = ivar_type.from_yaml(env_value)
puts %(Config.{{ivar.id}} : Set to #{config.{{ivar.id}}} (#{ivar_type}))
success = true
rescue
# nop
end
end
end
{% end %}
# Exit on fail
if !success
puts %(Config.{{ivar.id}} failed to parse #{env_value} as {{ivar.type}})
exit(1)
end
end
{% end %}
# Build database_url from db.* if it's not set directly
if config.database_url.to_s.empty?
if db = config.db
config.database_url = URI.new(
scheme: "postgres",
user: db.user,
password: db.password,
host: db.host,
port: db.port,
path: db.dbname,
)
else
puts "Config : Either database_url or db.* is required"
exit(1)
end
end
return config
end
end

The logic is:

  • Load base
    • If there is a YAML config in INVIDIOUS_CONFIG (what the docker-compose uses) load that
    • If not check INVIDIOUS_CONFIG_FILE for a config file name (defaulting to config/config.yml) and load that
  • Override
    • Check all config key env vars (INVIDIOUS_ + upcased preference key) and use the value to override

@saltycrys saltycrys closed this May 15, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 15, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants