From fedd3d178d580ffcfa110d86a97bf3fe0657f7fc Mon Sep 17 00:00:00 2001
From: Jake Goldsborough <ducks@discourse.org>
Date: Wed, 13 May 2026 10:45:10 -0700
Subject: [PATCH] DEV: Warn when top-level config keys are unrecognized

Previously, any top-level key not consumed by pups or the
discourse_docker launcher was silently ignored. This made typos and
misplaced keys invisible. A hook stanza at the wrong indent level,
or 'befor_code' with a typo, would parse cleanly and produce nothing.

Log a warning at boot time for any top-level key that's outside the
known set. Behavior is unchanged: unknown keys are still ignored, just
no longer silently.

Keys explicitly passed in the 'ignored' argument don't trigger the
warning, since the caller has signaled intent.
---
 lib/pups/config.rb  | 41 +++++++++++++++++++++++++++++++++++++++++
 test/config_test.rb | 44 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 85 insertions(+)

diff --git a/lib/pups/config.rb b/lib/pups/config.rb
index 9bfcdfd..42fdf91 100644
--- a/lib/pups/config.rb
+++ b/lib/pups/config.rb
@@ -4,6 +4,29 @@ module Pups
   class Config
     attr_reader :config, :params
 
+    # Top-level keys recognized by pups itself or by callers that consume the
+    # same config (notably the discourse_docker launcher). Keys outside this
+    # set are silently ignored, which has historically led to bugs where a
+    # typo or misplaced key produced no warning and no effect.
+    KNOWN_TOP_LEVEL_KEYS = %w[
+      base_image
+      boot_command
+      docker_args
+      env
+      env_template
+      expose
+      hooks
+      labels
+      links
+      no_boot_command
+      params
+      run
+      run_image
+      templates
+      update_pups
+      volumes
+    ].freeze
+
     def initialize(
       config,
       ignored = nil,
@@ -16,6 +39,8 @@ def initialize(
       # remove any ignored config elements prior to any more processing
       ignored&.each { |e| @config.delete(e) }
 
+      warn_about_unknown_keys
+
       filter_tags(include_tags: tags, exclude_tags: skip_tags)
 
       # set some defaults to prevent checks in various functions
@@ -121,6 +146,22 @@ def self.combine_template_and_process_env(config, env)
       config["env"].each { |k, v| env[k] = v.to_s }
     end
 
+    # Log a warning for any top-level keys that pups (and the surrounding
+    # discourse_docker launcher) won't read. These are silently ignored at
+    # runtime, so a warning is the only signal a typo or misplaced key
+    # produces nothing.
+    def warn_about_unknown_keys
+      return unless @config.is_a?(Hash)
+
+      unknown = @config.keys - KNOWN_TOP_LEVEL_KEYS
+      unknown.each do |key|
+        Pups.log.warn(
+          "Unknown top-level config key '#{key}' will be ignored. " \
+            "Known keys: #{KNOWN_TOP_LEVEL_KEYS.join(", ")}."
+        )
+      end
+    end
+
     # Filter run commands by tag: by default, keep all commands that contain tags.
     # If skip_tags argument is true, keep all commands that DO NOT contain tags.
     def filter_tags(include_tags: nil, exclude_tags: nil)
diff --git a/test/config_test.rb b/test/config_test.rb
index b3c34c9..3abe251 100644
--- a/test/config_test.rb
+++ b/test/config_test.rb
@@ -293,5 +293,49 @@ def test_extra_params
       assert_equal("2", config.params["one"])
       assert_equal("2", config.params["two"])
     end
+
+    def test_warns_on_unknown_top_level_keys
+      io = StringIO.new
+      original_logger = Pups.log
+      Pups.log = Logger.new(io)
+
+      Config.new({ "env" => {}, "weird_random_key" => "foo" })
+
+      assert_match(/Unknown top-level config key 'weird_random_key'/, io.string)
+    ensure
+      Pups.log = original_logger
+    end
+
+    def test_does_not_warn_on_known_top_level_keys
+      io = StringIO.new
+      original_logger = Pups.log
+      Pups.log = Logger.new(io)
+
+      Config.new(
+        {
+          "base_image" => "x",
+          "templates" => ["y"],
+          "env" => {},
+          "hooks" => {},
+          "run" => [],
+        }
+      )
+
+      refute_match(/Unknown top-level config key/, io.string)
+    ensure
+      Pups.log = original_logger
+    end
+
+    def test_does_not_warn_on_explicitly_ignored_keys
+      io = StringIO.new
+      original_logger = Pups.log
+      Pups.log = Logger.new(io)
+
+      Config.new({ "env" => {}, "weird_random_key" => "foo" }, ["weird_random_key"])
+
+      refute_match(/Unknown top-level config key/, io.string)
+    ensure
+      Pups.log = original_logger
+    end
   end
 end