destination inheritance might be confusing

[adrienpoly]

I noticed that destination-specific configurations inherit values from the main (deploy.yml) configuration. Initially, I thought this was a bug, but upon checking the documentation, it appears this is intended behavior:

"In this case, the configuration will also be read from config/deploy.staging.yml and merged with the base configuration."

This behavior surprised me, as it was not intuitive from a user's standpoint, leading to confusion.

---

Use Case

I needed different builder configurations for production and staging environments because I'm migrating architectures.

Main configuration (deploy.yml):

builder:
  arch:
    - arm64
  cache:
    type: gha
    options: mode=max
    image: rubyvideo-build-cache
  secrets:
    - RAILS_MASTER_KEY
+ remote: ssh://[email protected]

Staging configuration (deploy.staging.yml):

builder:
  arch:
    - amd64
  cache:
    type: gha
    options: mode=max
    image: rubyvideo-staging-build-cache
  secrets:
    - RAILS_MASTER_KEY

However, the remote configuration from deploy.yml unexpectedly leaked into the staging environment. When running kamal config -d staging, the output included the inherited value:

builder:
  arch:
    - amd64
  cache:
    type: gha
    options: mode=max
    image: rubyvideo-staging-build-cache
  secrets:
    - RAILS_MASTER_KEY
+ remote: ssh://[email protected]

This inheritance caused deployment to fail in my staging environment.

---

Suggested Improvement

If inheritance is intended behavior, I think it would greatly enhance usability to visually indicate inherited configuration values in the output. For example:

# kamal config -d staging

builder:
  arch:
    - amd64
  cache:
    type: gha
    options: mode=max
    image: rubyvideo-staging-build-cache
  secrets:
    - RAILS_MASTER_KEY
- remote: ssh://[email protected]
+ remote: ssh://[email protected] [Inherited]

This would help users understand which configurations are directly specified and which are inherited, preventing confusion and unexpected errors.

[djmb]

Thanks for the report @adrienpoly. The configuration inheritance can be confusing alright.

Our plan for this is that we'll have a default destination instead. So you can put the production config into deploy.production.yml, and then kamal deploy will default to using the production destination.

It's a fairly invasive change though so probably requires waiting for the next major version.

[igor-alexandrov]

@djmb I also wanted to suggest the default destination. In a multienvironmental setup, it is easier to avoid specifying -d production every time. If it will be implemented as an optional setting, it won't be so invasive change.

If you didn't start this work yet, I can do this.

[igor-alexandrov]

@adrienpoly the reason it happened for you is that currently configuration files are deep merged when loaded (https://github.com/basecamp/kamal/blob/main/lib/kamal/configuration.rb#L32).

{a: {1 => 1,2 => 2}, b: {3 => 3}}.deep_merge!({b: {4=>4}, c: {5=>5}})
# => {:a=>{1=>1, 2=>2}, :b=>{3=>3, 4=>4}, :c=>{5=>5}}

[igor-alexandrov]

@djmb another idea that may be useful is to add destinations validation.

➜  safariportal git:(master) ./bin/kamal app version -d env-not-exist
  ERROR (RuntimeError): Configuration file not found in /Users/igor/workspace/onetribe/config/deploy.env-not-exist.yml

Currently, if you try to run Kamal with the env, that does not exist, you will get a runtime error. Together with a default environment feature current behavior can be improved. I suggest adding an optional destinations configuration key.

destinations:
  - production # default destination without an explicit configuration
  - staging

# no default environment
destinations:
  - staging
  - integration

# explicit default environment
destinations:
  production:
    default: false
  staging:
    default: true

By default, Kamal will consider a production destination to be default if it exists, but this behavior can be overridden. With the list of the destinations defined ./bin/kamal app version -d env-not-exist will not raise a RuntimeError, but will exit with a validation message.

[djmb]

@igor-alexandrov - we were thinking to maybe just have one configuration value - default_destination which defaults to production and we could get rid of the require_destination configuration value, instead you could set default_destination to nil.

Worth some more thought though - there are cases where users would like to have "dynamic" destinations where they don't want to have to create a deploy.dest.yml file for each one but instead use ERB to insert the right host.

In any case I think we'll need to wait for v3. A default destination means that kamal deploy and kamal deploy -d production should do the same thing, but right now we'd create different container name prefixes (app- vs app-production-) so we need to find a way to untangle that.

[igor-alexandrov]

@djmb I am excited to hear about the v3!

Yes, if we will add the default_destination, then it should have a default value of "production" and in this case, everything will break because of the container prefixes problem, you described.

My suggestion to define destinations explicitly is softer, it will also replace the require_destination option, but won't break anything in no-destination setup. If you don't have a require_destination in your config – nothing will break for you, if you have it – it will be ignored, and you will have to define a new destinations key.

Dynamic destinations is an interesting example. I'm slowly working on a GitHub bot that allows to create dedicated environments for each Pull Request (inspired by Heroku's environment per branch feature). Currently, I use ERB to create a deployment config file from the template and run Kamal with a destination option after. I don't see any problems with continuing doing this with both yours and mine suggestions.

[djmb]

Don't get too excited, there's no specific plans for v3 yet! More this is a change that would need to wait for it whenever it happens.

Defining the destinations explicitly is an interesting idea - I'll have a think about it. If we did go for that then setting the default as part of it seems like the way to go. We could have something like:

destinations:
  - production: default
  - staging
  - integration

[igor-alexandrov]

Absolutely, the default destination should definitely be a part of the destinations config. And, it seems, it won't break anything in current configurations, just the require_destination option will stop working.

Having destinations explicitly defined opens the door for a future features. The example below can become a valid config too, which will simplify things a bit for “dynamic” destinations.

destinations:
  - production: default
  - staging
  - integration:
    servers:
      web:
        hosts:
          - <%= ENV["IP_ADDRESS"] %>

[djmb]

#809 has some more context here.