Raise exception on blank registry credentials, warn on blank secrets

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

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
+
+    missing_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,24 @@ def secrets_files
   end
 
   private
+    def missing_key_warning(key)
+      warn "Warning: Kamal secret #{key} has no value, did you forget to set it?"
+      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"}."
+          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 +61,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

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",