Skip to content

Add value equality to Proxy::Run#1818

Open
teal-bauer wants to merge 1 commit intobasecamp:mainfrom
teal-bauer:fix/proxy-run-value-equality
Open

Add value equality to Proxy::Run#1818
teal-bauer wants to merge 1 commit intobasecamp:mainfrom
teal-bauer:fix/proxy-run-value-equality

Conversation

@teal-bauer
Copy link
Copy Markdown

When multiple roles share a host, the global proxy.run config gets merged into each role's proxy config via deep_merge. This creates separate Run objects with identical run_config hashes.

ensure_no_conflicting_proxy_runs uses Array#uniq to detect conflicts, but Run doesn't implement ==/eql?/hash, so uniq compares by object identity. The check always fails when two or more roles with proxy config exist on the same host, even if the merged run configs are identical.

Changes

  • Add ==, eql?, and hash to Proxy::Run based on run_config
  • Add tests for value equality and the multi-role-same-host scenario

Reproduction

servers:
  web:
    hosts:
      - 1.1.1.1
  worker:
    hosts:
      - 1.1.1.1
    proxy:
      hosts:
        - worker.example.com
      app_port: 8080

proxy:
  hosts:
    - example.com
  run:
    log_max_size: ""
    options:
      log-driver: journald
$ kamal config
ERROR (Kamal::ConfigurationError): Conflicting proxy run configurations for host 1.1.1.1

When multiple roles share a host and the global proxy run config gets
merged into role-specific proxy configs, ensure_no_conflicting_proxy_runs
raises ConfigurationError even though the configs are identical.

The Run class didn't implement ==/eql?/hash, so Array#uniq compared by
object identity. Two Run objects created from the same merged config
were always treated as different.
Copilot AI review requested due to automatic review settings April 7, 2026 15:36
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes a critical issue where the ensure_no_conflicting_proxy_runs validation incorrectly reports conflicts when multiple roles share the same host and inherit proxy run configuration via deep_merge. The problem was that Proxy::Run objects lacked proper value equality methods, causing Array#uniq to compare by object identity rather than configuration content.

Changes:

  • Implemented ==, eql?, and hash methods in Proxy::Run class based on run_config hash comparison
  • Added comprehensive test suite covering identical/different configurations and the multi-role-same-host scenario

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
lib/kamal/configuration/proxy/run.rb Added value equality implementation (==, eql?, hash) to enable proper comparison of Run objects by configuration content
test/configuration/proxy/run_test.rb Added three test cases: value equality with identical configs, inequality with different configs, and integration test for the multi-role scenario

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants