fix: name the interface and the reference in topology errors - #22
Conversation
A rejected inventory reported "unknown controller interface" or "unknown lower interface" without saying which interface carried the bad reference. The monitor logs the message unchanged, so the journal did not name the subject either, and a reader had to reproduce the whole inventory to find the interface at fault. Every rejection now names the interface that carries the bad reference and the reference that failed. Nine failure classes each have their own message and their own test: invalid index, empty name, duplicate index, duplicate name, unknown controller, missing lower sub-layer, unknown lower sub-layer, remote lower sub-layer, and a self reference as controller or as lower sub-layer. The final pass that rejected a relationship with two equal indices is gone. Each of the three sites that inserts a relationship now guards the equality itself, so the message can name the interface. Those three sites are the only way to create a relationship, so the guards cover the same inputs. The lower sub-layer reference is now read before the network namespace check. An interface with no reference and a namespace identifier reports the missing reference instead of the namespace. Both inputs are still rejected. Interface names come from netlink, so every message renders them with the Debug format. A name that carries a newline or a terminal escape stays quoted data on one line. Validation stays atomic. One bad reference rejects the whole inventory, and no partial topology reaches the table. Closes #18
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. Walkthrough
ChangesTopology validation
Priority: ⬇️ Low Estimated code review effort: 3 (Moderate) | ~25 minutes Change: Bug fix · Severity of issue fixed: Low Merge Risk: ⚪ Minimal · up to The validation changes retain rejection for malformed required relationships and do not show an actionable merge risk. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
✨ Simplify code
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. A rabbit checks each link in line Comment |
|
@coderabbitai review |
✅ Action performedReview finished.
|
A rejected inventory reported "unknown controller interface" or "unknown
lower interface" without saying which interface carried the bad reference.
The monitor logs the message unchanged, so the journal did not name the
subject either, and a reader had to reproduce the whole inventory to find
the interface at fault.
Every rejection now names the interface that carries the bad reference and
the reference that failed. Nine failure classes each have their own message
and their own test: invalid index, empty name, duplicate index, duplicate
name, unknown controller, missing lower sub-layer, unknown lower sub-layer,
remote lower sub-layer, and a self reference as controller or as lower
sub-layer.
The final pass that rejected a relationship with two equal indices is gone.
Each of the three sites that inserts a relationship now guards the equality
itself, so the message can name the interface. Those three sites are the
only way to create a relationship, so the guards cover the same inputs.
The lower sub-layer reference is now read before the network namespace
check. An interface with no reference and a namespace identifier reports
the missing reference instead of the namespace. Both inputs are still
rejected.
Interface names come from netlink, so every message renders them with the
Debug format. A name that carries a newline or a terminal escape stays
quoted data on one line.
Validation stays atomic. One bad reference rejects the whole inventory, and
no partial topology reaches the table.
Closes #18
Summary by CodeRabbit