Prefer secret to env var if both are available

[eemil]

Related Issue: none

New Behavior

If a secret and env var for the same setting are available, then prefer the secret.

Contrast to Current Behavior

Currently, to use a secret you need to change docker-compose.yml or env/* to make sure the corresponding environment variable doesn't exist. That is to say, secrets defined in docker-compose.override.yml may be silently ignored which is unintuitive and goes against the principle of an override file IMO.

Discussion: Benefits and Drawbacks

At the moment, everything I need to run netbox is defined in docker-compose.override.yml. Upgrades are easy, as I can just copy over this repo's directory with a new release. If I want to use secrets, than I need to make changes to docker-compose.yml, env/*, or use a custom compose file. With all of these methods, upgrades become more tedious.

Changes to the Wiki

...

Proposed Release Note Entry

Prefer secret to env var if both are available #291

If a secret and env var for the same setting are available, then prefer the secret.

Double Check

• I have read the comments and followed the PR template.
• I have explained my PR according to the information in the comments.
• My PR targets the develop branch.

[cimnine]

Thank you for bringing this to our attention.

than I need to make changes to docker-compose.yml

Can you explain the changes that are necessary.

TBH, I'm hesitant to switch this. Two reasons:

1. ENV vars are a good last resort. That's why I believe that they should have precedence over any other configuration methods.
2. It changes the current behavior and thous might unintentionally break current installations.

That's why I would like to learn more about the changes you have to make to the repository files. And maybe we can find another solution or maybe it convinces me that this PR is indeed necessary.

That being said, other members of our community shall feel free to pitch into the discussion. It's not just my opinion that should matter on this decision.

[eemil]

then I need to make changes to docker-compose.yml

Can you explain the changes that are necessary.

Basically, you need to make sure that the relevant env var is never set. By commenting it out from env/netbox.env, or removing the relevant env_file line from docker-compose.yml.

If it's possible to unset env vars in docker-compose.override.yml, that would also work. But I can't come up with a way of doing this. It's not sufficient to set e.g. SUPERUSER_PASSWORD to an empty value.

ENV vars are a good last resort. That's why I believe that they should have precedence over any other configuration methods.

Secrets are a more specific, deliberate configurable IMO. They require more configuration, and are very rarely set by default. Comparatively, env vars are often set to a default value. So if both exist, I would almost always assume that the secret is what the admin intended to be used.

It changes the current behavior and thous might unintentionally break current installations.

For existing installations using only env vars, I don't think there should be any issues.
For installations using secrets in a broken way, they might indeed break. Take the following as an example. In this situation, the person just cloned netbox-docker and created their own docker-compose.override.yml.

version: '3.4'
services:
  nginx:
    ports:
      - 8000:8080

  netbox:
    secrets:
      - secret_key

secrets:
  secret_key:
    file: ./secret_key

When bringing the environment up, netbox will silently ignore secret_key and instead use the env var from env/netbox.env. Now when the admin upgrades to a newer version of netbox-docker that gives priority to secrets, secret_key will change to value specified in ./secret_key which may cause problems (not exactly sure what the effect of this is).

So yes, some installations might break. But I think it's justified, if the installations were already in a kind-of-broken-but-still-working state. From a security standpoint, it's usually better to fail visibly than continue working in an wonky state.

[cimnine]

If it's possible to unset env vars in docker-compose.override.yml, that would also work. But I can't come up with a way of doing this. It's not sufficient to set e.g. SUPERUSER_PASSWORD to an empty value.

You should be able to just overwrite the env_file directive and have it point to a file of your own:

# docker-compose.override.yml

version: '3.4'
services:
  netbox:
    env_file: env/my.netbox.env # custom env file

[eemil]

You should be able to just overwrite the env_file directive and have it point to a file of your own:

That's what I assumed at first. But what actually happens, is the env file is added in addition to the existing one in docker-compose.yml. So the result is a combination of the two, with my.netbox.env having higher precedence in case of duplicate variables.

Perhaps we should wait for some more comments... maybe there's something obvious I'm missing.

[cimnine]

That's what I assumed at first. But what actually happens, is the env file is added in addition to the existing one in docker-compose.yml. So the result is a combination of the two, with my.netbox.env having higher precedence in case of duplicate variables.

Interesting.

[cimnine]

Sorry that it took that long. At least in the meantime I've come to like your solution and your reasoning was sound anyway. Just one tiny remark, if you don't mind.

Maybe you could also rebase your code should it not be based on the latest develop yet.

[cimnine]

Suggested change

	def read_secret(secret_name, default=''):
	def read_secret(secret_name, default=None):

Is this possible? I think it's no good to have the default value of default be an empty string.

[cimnine]

I now see that you've only copied what was there. Still, if you don't mind change it to None, that'd be great :)

[cimnine]

Merged in 121c3f8

Github did not recognize this as I rebased the branch onto the latest develop first.