SpriteLink / NIPAP

Neat IP Address Planner - NIPAP is the best open source IPAM in the known universe, challenging classical IP address management (IPAM) systems in many areas.
https://spritelink.github.io/NIPAP/
MIT License
539 stars 132 forks source link

Description or node must be speficied when adding a prefix #1083

Open emil-nasso opened 7 years ago

emil-nasso commented 7 years ago

When adding a prefix by using the add_prefix method of the xml-rpc api we get this error message: Either description or node must be specified.

This is not a requirement when adding prefixes manually through the web interface.

You can work around this by setting description to null or an empty string.

plajjan commented 7 years ago

This is per design. The idea being that it's useful to always have a description on a prefix or at least know which node it's associated with. It's the backend enforcing this and it only checks that the node or description attribute is present. Through the introduction of AVPs we open up for registering prefixes that might actually contain meaningful data without having node or description set but for someone browsing the data via web or CLI (and not API) I think it's still helpful to have node or description always be set, thus I think the check is still valid.

You are correct in that the web-ui currently "circumvents" this by settings node & description but the values are "" so it passes the check.

We can fix the check in the backend to force a non-zero length value or by letting the web interface not send the attribute when value is "".

However, it sounds like you think the check itself is a bug.

xxiii23 commented 7 years ago

Rather than enforcing an assumption, shouldn't it be up to the user (or administrator, perhaps via nipap.conf) to decide whether a comment and/or node is appropriate? sometimes the description on the containing network might be sufficient.

hasty examples: 2001:db8:beef:1::/64 "warehouse computers, interface is asset tag" 2001:db8:beef:1::12:3456 2001:db8:beef:1::12:3457 2001:db8:beef:1::12:7689 ... (with enforced description:) 100.64.0.0/24 "Call center NAT block" 100.64.0.0 "NATted address" 100.64.0.1 "NATted address" 100.64.0.2 "NATted address"

plajjan commented 7 years ago

@xxiii23 I don't see the point with your example. What is actually improved by the removal of that information?

We do not currently have the possibility of letting the administrator change things like this and I'm wary about introducing it since it would make the feature/option matrix chart absolutely explode and make testing mind boggingly difficult.

So first question is whether we should keep this enforcement at all. @xxiii23 seems to be against it. Not sure how others feel.

If we remove it, then this issue is solved. If we keep it then I guess we should, in addition to checking for the presence of these attributes, also make sure they are of non-zero length.

plajjan commented 7 years ago

Overall we do have certain assumptions and I think that's ok. If we go full-flexibility we'll end up with a turing machine that noone can maintain. Biased software is ok :)

emil-nasso commented 7 years ago

Yes. I still think this is a bug/unwanted feature. I don't think it should be up to the documentation platform to enforce how I document. There are many attributes available on a prefix that you can use to create a clear documentation. You could have order-id, customer-id, a bunch of tags and AVPs and have a very clearly documented prefix without using the node and/or description.

We interact with nipap from an external system through the api. When the prefix is not used (just reserved) it doesn't make sense to use the node-attribute. All the information we have to document what this prefix is used for and by who is stored in order_id, customer_id, tags and AVPs. I think this i preferred over just storing everything in a free-text-attribute like description.

To my understanding there is no requirement at all to use the web-interface for nipap? You could just use it as a database for storing prefix information, to be read by other systems. Free-text fields like description could be good when written/read by human but we want more structure than that.

plajjan commented 7 years ago

@emil-nasso no, it's certainly not a requirement to use the web-ui. I think the idea is that by always putting in node or description it makes it a lot easier to at least know where to start looking.

Free-text fields like description could be good when written/read by human but we want more structure than that.

It's not an choice of one or the other. You can use both. For your case, in addition to order_id, customer_id, tags or AVPs, wouldn't it be rather simple to just fill in a description as well? This would make it immediately clear to someone using the web UI or CLI what the prefix is for. I'm not saying you should cram everything into a free text field like description - just write something there to show what the prefix is used for, in addition to the other attributes. You get structured data and a helpful text for humans.

When the prefix is not used, would it not make sense to have a description saying it is reserved for customer or some such?

In searches we do text matching on description, node, order_id, customer_id, comment per default. Just filling in description with something remotely relevant allows you to easily match it in a search performed by a human that don't know all the AVPs used to store data.

From the perspective of developing NIPAP I suppose it makes things a tad simpler if we know roughly where different types of data go. It will potentially allow us to optimise SQL queries and similar.