Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WiP] Assertions based on strategic merge patch (partial list comparisons) #388

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

porridge
Copy link
Member

Why we need this

IMHO one of the most pressing issue in kuttl is the problem with assertions on lists (arrays, slices, whatever). To match a list:

  • all elements must be listed
  • they must be exactly in the same order

For example with status.conditions, depending on the operator which writes the list, this might be impossible to achieve (e.g. if ordering is random or some conditions are present or absent inconsistently.

See #76 for details.

What this PR does

During a recent team hackathon I took a stab at finding out how easy it would be to use the strategic merge patch machinery in order to relax the requirements mentioned above. We basically treat the assertion as a strategic merge patch, apply it to the object at hand, and the assertion passes if and only if the resulting object does not differ from the original.

Credit for the idea goes to @misberner

To be completely explicit, this is just a prototype, but can serve as an inspiration to someone who wanted to take this further.

@dwerder
Copy link

dwerder commented Oct 20, 2022

That would be really helpful. Whats needed to get this merged?

@porridge
Copy link
Member Author

I think more testing and cleaning up the code would be necessary.

Since this has plenty of potential to trip people up in some corner cases I didn't think about, we should probably consider some more gentle rollout strategy than just wholesale replacing the old comparison mechanism with the new one. Perhaps a toggle in a TestAssert? 🤔

@chipzoller
Copy link

We (Kyverno) would be very interested in having this. The Policy Report format we use outputs an array/object within the CR and that ordering is not guaranteed. We will want to be able to test that a single object that looks like X definition occurs somewhere in that list.

@anderssonw
Copy link

We would also love for this to work. We have loads of partial list comparisons in our tests, which for some reason only sometimes fails, probably due to some race condition we are not interested in testing for the tests where we get fails.

@haarchri
Copy link

haarchri commented Jan 7, 2023

@porridge can we help with something ? we very interested out of crossplane project in this feature ;)

@porridge
Copy link
Member Author

porridge commented Jan 9, 2023

@porridge can we help with something ?

I think these comments still apply @haarchri

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.

5 participants