Skip to content

Add support for unordered XML comparison in compliance checks#1022

Open
jmpettit wants to merge 4 commits intodevelopfrom
xml-sorting
Open

Add support for unordered XML comparison in compliance checks#1022
jmpettit wants to merge 4 commits intodevelopfrom
xml-sorting

Conversation

@jmpettit
Copy link
Copy Markdown
Contributor

Closes: #1020

What's Changed

Adds unordered support for XML

To Do

  • Explanation of Change(s)
  • Added change log fragment(s) (for more information see the documentation)
  • Attached Screenshots, Payload Example
  • Unit, Integration Tests
  • Documentation Updates (when adding/changing features)
  • Outline Remaining Work, Constraints from Design

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Oct 31, 2025

Coverage report

Click to see where and how coverage changed

FileStatementsMissingCoverageCoverage
(new stmts)
Lines missing
  nautobot_golden_config
  models.py 164-165
Project Total  

This report was generated by python-coverage-comment-action

Comment thread nautobot_golden_config/models.py Outdated
compliance = not missing and not extra
compliance_int = int(compliance)
ordered = obj.ordered
ordered = obj.rule.config_ordered
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is accurate. I think that ordered here showed if it was in fact ordered. Meaning "I may or may not care it's ordered, but this config is fact ordered"

I could be wrong on that, but at least that is how it works on the cli side. It allowed me to short circuit certain conditions.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i see, i think we could replicate this but would need to diff twice if they didn't have order checked in the rule

      ordered_missing = main.diff_texts(obj.actual, obj.intended, diff_options=diff_options)
      ordered_extra = main.diff_texts(obj.intended, obj.actual, diff_options=diff_options)
      ordered_compliant = not ordered_missing and not ordered_extra

      if obj.rule.config_ordered:
          missing = ordered_missing
          extra = ordered_extra
      else:
          actual_xml = _sort_xml(obj.actual)
          intended_xml = _sort_xml(obj.intended)
          missing = main.diff_texts(actual_xml, intended_xml, diff_options=diff_options)
          extra = main.diff_texts(intended_xml, actual_xml, diff_options=diff_options)

      compliance = not missing and not extra
      compliance_int = int(compliance)
      ordered = ordered_compliant

@itdependsnetworks
Copy link
Copy Markdown
Contributor

overall, i think this is good, @jeffkala thoughts?

Copy link
Copy Markdown
Contributor

@jeffkala jeffkala left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems good to me. Have you fully tested it in a lab type env?

@jmpettit
Copy link
Copy Markdown
Contributor Author

jmpettit commented Nov 4, 2025

seems good to me. Have you fully tested it in a lab type env?

I haven't, the tests run the actual compliance jobs tho - is there something i'm missing that the tests don't cover?

@itdependsnetworks
Copy link
Copy Markdown
Contributor

seems good to me. Have you fully tested it in a lab type env?

I haven't, the tests run the actual compliance jobs tho - is there something i'm missing that the tests don't cover?

is there something i'm missing that the tests don't cover? ... a lot :)

Nearly every time this has not been tested it has introduced some kind of bug so we are a bit skittish.

@jmpettit
Copy link
Copy Markdown
Contributor Author

jmpettit commented Nov 4, 2025

hmm ok, i just checked my dev environment and i don't have anything setup - if one of you guys can easily test this i'd appreciate it, otherwise i'll get to it when i get to it

@matt852
Copy link
Copy Markdown
Contributor

matt852 commented Nov 24, 2025

PR #1013 may help with spinning up a test environment quickly for this and other new features

@gsnider2195 gsnider2195 mentioned this pull request Dec 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Allow Unordered Config Compliance Check for XML

4 participants