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.36k stars 2.49k forks source link

Support for a peer-review workflow #11942

Open rmanyari opened 1 year ago

rmanyari commented 1 year ago

NetBox version

v3.4.5

Feature type

Data model extension

Proposed functionality

Extend the staging mechanism to support a peer-review workflow.

The idea here is to provide users with a way of proposing changes, both programmatically and through the UI, and have another team member with more elevated permissions review, approve and apply them.

To support this, we'd need changes across different components and I'd suggest to implement them parts. Phases 1 through 6 are all backend work that can be done incrementally in different MRs. Phases 7 through 9 are all UI work that I believe can be released independently.

Phase 1 - Scaffolding for notifications
Phase 2 - Introduce the ReviewRequest model.
Phase 3 - Enhance Branch to identify stale changes.
Phase 4 - Extend the permissions model to support the creation of review requests by non power users.
Phase 5 - Extend the ObjectChange model to support review/reviewer information.
Phase 6 - E2E test suite to go through the peer-review workflow programmatically
Phase 7 - Update the UI to support notifications.
Phase 8 - Update the UI to support review requests.
Phase 9 - Update the change log in the UI.

Use case

Netbox users with create/edit/delete permissions can independently change the state of an instance.

While convenient from a velocity point of view, the current model doesn't support organizations with strict change management processes that require all production changes to be peer-reviewed. This feature request aim to add that functionality to Netbox.

Database changes

New Notifications model (minimum fields required):
New ReviewRequest model (minimum fields required):
Update ObjectChange model (add optional field):

External dependencies

None that I can think of.

jsenecal commented 1 year ago

Very well written and thought of FR. Much appreciated.

The only concern we have is that this is in fact two features in one request : Notifications and Review functionalities.

We could decouple them, or handle them together as the notifications system wouldn't serve much use in other models anyways. (Report/Script completion notifications maybe?)

Marking this as under review for the time being.

rmanyari commented 1 year ago

As discussed in Slack, I'm more than happy to take a shot at implementing this. I'll work on a PoC.

jsenecal commented 1 year ago

Great! Thanks for contributing 🥇

baldgeek commented 1 year ago

Can I ask what under review means for this moving forward? My company was very excited about this, and it appeared he was making progress before that PR was closed. It has also garnered the most likes out of the "under review" category (my company is only one of those).

Thanks.

jeremystretch commented 1 year ago

@baldgeek you can find descriptions of all our issue labels here. The under review status means:

Further discussion is needed to determine this issue's scope and/or implementation

It will be some time before we can dig into this FR as we're currently focusing on the upcoming v3.5 release. If you'd like to help us reduce the backlog of open issues in the meantime, I would greatly appreciate volunteers for any of the open issues marked needs owner. These can be tackled immediately, and completing them will help to free up maintainer time to consider feature proposals like this one.

rmanyari commented 1 year ago

For context @baldgeek - It was my mistake to open a PR here. What I meant to do was work on a PoC on my side to discuss the approach with some of the maintainers and take it from there. I'm glad to see that there's support for these type of features from the community, and once the maintainers prioritize it and flesh out a design, I'm happy to keep contributing as needed.

tobikris commented 1 year ago

Thank you for working on this feature, I am looking forward to it! I have a use case where it is not clear to me if this would already be included in your feature: Several changes on different objects related to the same "changeset" (e.g. a new device with its IP addresses and cables to other devices). I would like to create a review request for the full change instead of several requests per object. Is this your understanding as well?

tom0010 commented 4 months ago

I don't see any updates on here, is this something that could be thought about for 4.0 release?