Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dynamic Configuration #343

Merged
merged 14 commits into from
Oct 26, 2020
Merged

Dynamic Configuration #343

merged 14 commits into from
Oct 26, 2020

Conversation

cimnine
Copy link
Collaborator

@cimnine cimnine commented Oct 18, 2020

Related Issue: #318

New Behavior

All .py files in /etc/netbox/config are now considered as configuration files. All users can add individual configuration files themselves. This allows to support complex variables and also new variables for which we don't yet have an 'ENV' variant implemented.

The configuration.py file is processed first. All other files are evaluated afterwards in alphabetically. Later files overwrite the settings of previous files. If later files want to build upon the configuration of previous files, they can import them. See configuration/example.py.

The ldap configuration was moved to /etc/netbox/config/ldap. The ldap configuration works the same way as the regular configuration, i.e. there can be multiple files in /etc/netbox/config/ldap, and they all get evaluated in order, beginning with the ldap_config.py file.

To make things easy for our docker-compose users, the files in the repository folder configuration/* and configuration/ldap/* are already mounted to /etc/netbox/config (and /etc/netbox/config/ldap, respectively). So a beginner user – who is using our docker-compose setup – must not necessarily re-compile the Docker Image to adjust complex configurations.

As I was mangling with the configuration file anyway, I've aligned it with the upstream version.

Contrast to Current Behavior

Currently when users want to define custom variables in their configuration.py, they have to overwrite the default file. This will no longer be necessary. A second file can be placed in /etc/netbox/config, e.g. /etc/netbox/config/my_config.py. The values in this file will then overwrite the values of the default configuration file, configuration.py.

Discussion: Benefits and Drawbacks

LDAP users have to be aware that the location of the ldap_config.py file changed from /etc/netbox/config/ldap_config.py to /etc/netbox/config/ldap/ldap_config.py.

All users, who use our unchanged docker-compose.yml file should not need to adjust anything.

See also #318 for a full discussion that lead to this PR.

Changes to the Wiki

A new wiki page should describe this new behavior, so that LDAP-, Plugin-, Remote-Auth-, NAPALM- and all the other power-users learn about the new ability.

Proposed Release Note Entry

Dynamic Configration #343

We have added the possibility to load additional configuration files when Netbox Docker starts.

If you use our docker-compose.yml file, then just put any relevant additional configuration files into the configuration directory.
Otherwise mount them to /etc/netbox/config/ within the container.
All .py files are loaded.
They can contain arbitrary Python code.
Be aware that the files are evaluated in alphabetical order while configuration.py will always be first.
Later files overwrite the settings of earlier files.

The same works for LDAP configurations:
If you use our docker-compose.yml file, then just put any relevant additional LDAP configuration files into the configuration/ldap directory.
Otherwise mount them to /etc/netbox/config/ldap/ within the container.
All .py files are loaded.
They can contain arbitrary Python code.
Be aware that the files are evaluated in alphabetical order while configuration.py will always be first.
Later files overwrite the settings of earlier files.

Here's an example:

# In the repo: configuration/configuration.py
# -- OR --
# In the container: /etc/netbox/config/retro.py

from datetime import datetime
now = datetime.now().strftime("%d/%m/%Y %H:%M:%S")

BANNER_TOP = f'<marquee width="200px">This instance started on {now}.</marquee>'

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 cimnine changed the title Dynamic variables Dynamic Configuration Oct 18, 2020
@cimnine cimnine added the enhancement The issue describes an enhancement that we would like to implement in the future. label Oct 18, 2020
@cimnine cimnine added the discussion This issue requires further input from the community. label Oct 18, 2020
@tobiasge
Copy link
Member

I really like the approach with the overwriting of the __getattr__ method.

What might be a problem for Openshift and Kubernetes users is the placement of the configuration files. But with some changes to the loading of the config maps its possible to create the structure that is needed.
This is the new configuration needed in the deployment config of the netbox container:

      volumes:
        - name: volume-netbox-config
          configMap:
            name: netbox-config
            items:
              - key: configuration.py
                path: config/configuration.py
              - key: gunicorn_config.py
                path: gunicorn_config.py
              - key: ldap_config.py
                path: config/ldap/ldap_config.py

I tested this in Openshift 4.5.

@shuichiro-makigaki shuichiro-makigaki mentioned this pull request Oct 19, 2020
3 tasks
@cimnine cimnine added this to the 0.25.0 milestone Oct 19, 2020
@karrots
Copy link

karrots commented Oct 20, 2020

Your description says /etc/netbox/config/ldap_config.py moves to /etc/netbox/config/ldap_config.py instead of /etc/netbox/config/ldap/ldap_config.py. I like the idea and can test LDAP configs in my environment.

@cimnine
Copy link
Collaborator Author

cimnine commented Oct 20, 2020

@karrots Thanks, I've corrected this.

I've now also added a draft text for the release notes.

@cimnine cimnine marked this pull request as ready for review October 20, 2020 19:25
@cimnine cimnine requested review from tobiasge and karrots October 20, 2020 19:25
@tobiasge
Copy link
Member

The code in configuration.docker.py and ldap_config.docker.py is the same. Can this be moved into functions to be imported from those two files? I think it would be better to maintain if we had only one copy of this code.

Apart from that, it looks good.

Which is the file `docker/configuration.docker.py` in our repo.
The common code is then imported by `docker/ldap_config.docker.py`.
@cimnine
Copy link
Collaborator Author

cimnine commented Oct 26, 2020

Can this be moved into functions to be imported from those two files?

Good input! I've just pushed a commit that does that. Can you have another look?

@cimnine cimnine merged commit 64d82b5 into develop Oct 26, 2020
@cimnine cimnine deleted the DynamicVariables branch October 26, 2020 14:21
@cimnine cimnine modified the milestones: 0.25.0, 0.26.0 Oct 26, 2020
@cimnine cimnine added the hacktoberfest-accepted We think that such a PR is sufficiently good contribution, even if we don't merge it (yet). label Oct 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion This issue requires further input from the community. enhancement The issue describes an enhancement that we would like to implement in the future. hacktoberfest-accepted We think that such a PR is sufficiently good contribution, even if we don't merge it (yet).
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants