diff --git a/lib/kamal/configuration/registry.rb b/lib/kamal/configuration/registry.rb index 763cf976..e56c954b 100644 --- a/lib/kamal/configuration/registry.rb +++ b/lib/kamal/configuration/registry.rb @@ -24,7 +24,16 @@ def password private def lookup(key) if registry_config[key].is_a?(Array) - secrets[registry_config[key].first] + secret = secrets[registry_config[key].first] + # Although the key is present, its value may be empty due to environment + # variable substitution. I.e. if it refers to an empty or unset env var. + unless secret.present? + raise Kamal::ConfigurationError, "The required secret " \ + "'registry.#{key}' does not have a value. Did you forget to set " \ + "its value with an environment variable or a secret helper?" + end + + secret else registry_config[key] end diff --git a/lib/kamal/secrets.rb b/lib/kamal/secrets.rb index 7a382f98..09c73451 100644 --- a/lib/kamal/secrets.rb +++ b/lib/kamal/secrets.rb @@ -10,9 +10,13 @@ def initialize(destination: nil) def [](key) # Fetching secrets may ask the user for input, so ensure only one thread does that - @mutex.synchronize do + value = @mutex.synchronize do secrets.fetch(key) end + + blank_key_warning(key) unless value.present? + + value rescue KeyError if secrets_files.present? raise Kamal::ConfigurationError, "Secret '#{key}' not found in #{secrets_files.join(", ")}" @@ -30,6 +34,25 @@ def secrets_files end private + def blank_key_warning(key) + warn "Warning: Kamal secret #{key} is blank." + secrets_files.each do |secrets_file| + next unless File.exist?(secrets_file) + + File.foreach(secrets_file).with_index do |line, line_num| + next unless line.match?(/^\s*#{key}=/) + + warn "Tip: see #{secrets_file}:#{line_num + 1}: #{line.strip}" + if line.match?(/^\s*#{key}=\$\w+/) + warn "Tip: the environment variable #{key} is #{ENV[key].nil? ? + "not set" : "blank"}. Did you forget to set it?" + elsif (matches = line.match(/^\s*#{key}=\$\(([^)]+)\)/)).present? + warn "Tip: the shell command \`#{matches[1]}\` returned an empty value." + end + end + end + end + def secrets @secrets ||= secrets_files.inject({}) do |secrets, secrets_file| secrets.merge!(::Dotenv.parse(secrets_file)) @@ -39,4 +62,12 @@ def secrets def secrets_filenames [ ".kamal/secrets-common", ".kamal/secrets#{(".#{@destination}" if @destination)}" ] end + + # Suppress warnings if launched from bin/test + alias_method :kernel_warn, :warn + def warn(message) + return if ENV["RAILS_ENV"] == "test" + + kernel_warn message + end end diff --git a/test/commands/registry_test.rb b/test/commands/registry_test.rb index cf2734b7..250e3b15 100755 --- a/test/commands/registry_test.rb +++ b/test/commands/registry_test.rb @@ -49,6 +49,18 @@ class CommandsRegistryTest < ActiveSupport::TestCase end end + test "registry login with blank ENV password" do + with_test_secrets("secrets" => "KAMAL_REGISTRY_PASSWORD=") do + @config[:registry]["password"] = [ "KAMAL_REGISTRY_PASSWORD" ] + + assert_raises Kamal::ConfigurationError do + assert_equal \ + "docker login hub.docker.com -u \"dhh\" -p \"\"", + registry.login.join(" ") + end + end + end + test "registry logout" do assert_equal \ "docker logout hub.docker.com",