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

KeyValuePairs::extend implements a nonsense merge #887

Open
nightkr opened this issue Oct 11, 2024 · 0 comments · May be fixed by #889 or #888
Open

KeyValuePairs::extend implements a nonsense merge #887

nightkr opened this issue Oct 11, 2024 · 0 comments · May be fixed by #889 or #888
Assignees
Labels

Comments

@nightkr
Copy link
Member

nightkr commented Oct 11, 2024

Affected version

sble-operator 0.76.0

Current and expected behavior

If you try to merge the following KVP sets:

a: b
b: a
---
a: a
b: b

and then build a k8s_openapi label map from it you get

a: b
b: b

Which is nonsense, I'd expect one of them to take precedence (and according to convention/ObjectMetaBuilder::with_label docs) it should be the latter ({a: a, b: b}).

Possible solution

Ultimately, this comes down to that we try to store reified KeyValuePairs in a BTreeSet. This isn't what sets exist for.

As a "quick fix" we could change the backing store for KeyValuePairs to BTreeMap<Key, V> (though its Deref<BTreeSet> impl makes that problematic too, on top of a bunch of lifetime issues). Ultimately I'd like to rip the band-aid and just kill KeyValuePair.

Additional context

No response

Environment

No response

Would you like to work on fixing this bug?

yes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment