netbox-community / netbox

The premier source of truth powering network automation. Open source under Apache 2. Public demo: https://demo.netbox.dev
http://netboxlabs.com/oss/netbox/
Apache License 2.0
15.44k stars 2.52k forks source link

Allow Config Contexts to be deep merged #2495

Closed martink2 closed 5 years ago

martink2 commented 5 years ago

Environment

Proposed Functionality

Netbox allows Context data to be provided in a hierarchical fashion via JSON and also allows merging multiple Contexts. The current merge overrides the entire subtree if a later Context includes the same keys. For large organised Context data it would be more desirable to merge not only based on "top level" keys but also merge further down in the key hierarchy by performing a deep merge as many YAML/JSON config based automation tools like Chef or Ansible do.

Use Case

Use deep merged JSON config directly in Ansible or Chef. Example of desired Behaviour: Context1

{
    "cc": {
        "net": {
            "vrf": {
                "cc-cloud01": {
                    "global-rt-export": [
                        101
                    ]
            }
        }
    }
}

Context 2

{
    "cc": {
        "net": {
            "vpn_asn": 65126
        }
    }
}

Current Result

{
    "cc": {
        "net": {
            "vpn_asn": 65126
        }
    }
}

Desired Result

{
    "cc": {
        "net": {
            "vpn_asn": 65126,
            "vrf": {
                "cc-cloud01": {
                    "global-rt-export": [
                        101
                    ]
            }
        }
    }
}

Database Changes

None

External Dependencies

None

jeremystretch commented 5 years ago

In this example, how would a user replace the contents of net?

martink2 commented 5 years ago

In chef (im am using this example as i am most familiar with that one) the idea is one does not. Dicts are only used to structure data so values are the leafs of the config tree and you can replace any of those by just setting them to something different or null.

You could force this in the example by setting:

{
    "cc": {
        "net": null
        }
    }
}

and

{
    "cc": {
        "net": {
            "abc": 126
        }
    }
}

which would leave you with an "override" another option would be to introduce a setting on the context itself if it should be handled as "merge" or "override". The library used in the PR would make that easy to implement.

cimnine commented 5 years ago

In this example, how would a user replace the contents of net?

I'm wondering what the concrete use-case would be for this?

jeremystretch commented 5 years ago

I'm wondering what the concrete use-case would be for this?

Removing a widely applicable piece of data only for a small subset of devices, such as in a lab or other niche area or role. It's a function that NetBox supports today, so every effort should be made to preserve it.

another option would be to introduce a setting on the context itself if it should be handled as "merge" or "override".

This would probably be the preferred approach. We can introduce a boolean field on the ConfigContext model to specify the behavior, defaulting to merge.

lampwins commented 5 years ago

Just a note for when it comes time to implement this, the same boolean field will need to be added to the Device and VirtualMachine models by way of the ConfigContextModel. This is unless we want the local_context_data to only every use one method (merge or replace), which I don't think makes much sense.

sdktr commented 5 years ago

is there a specific reason that the local config context doesn't use the generic ConfigContextModel?

jeremystretch commented 5 years ago

@sdktr local config context is one-to-one: It would be much less efficient to store a separate ConfigContextModel instance for every device.