Multiple domain components in issuer #301
Conversation
da-kami
force-pushed
the
multiple-domain-components-in-issuer
branch
from
5243530 to
7192b84
Compare
There was a problem hiding this comment.
Hi there, thanks for the issue/PR.
I haven't sat down to form a strong opinion on the core subject, but the specific spot in 5280 you cite and the linked cert-manager issue don't strike me as strong motivation. I think I should page in more context and revisit.
With respect to the diff: it's fairly invasive for an issue that at first glance feels uncommon/niche. Certainly the semver break would be important to avoid unless critical.
WDYT about getting CI to pass and reworking to be semver compatible as a first step?
| } | ||
| /// Iterate over the entries | ||
| pub fn iter(&self) -> DistinguishedNameIterator<'_> { |
There was a problem hiding this comment.
removing this pub API item will break semver. I think you should try to orient your diff to avoid that, or to make a stronger case for why it's necessary.
There was a problem hiding this comment.
tbh I don't understand why the previous implementation would require the DistinguishedNameIterator as own iterator implementation.
I'm implementing Iterator for DistinguishedName - so iter() is actually still available even though the explicit struct function was removed.
I would argue that the interface is actually not breaking - iter() is still available after the change.
@cpu Is there a better way to achieve this so that semver accepts the change as non-breaking?
There was a problem hiding this comment.
@da-kami implementing Iterator for DistinguishedName does not make any method called iter() available -- Iterator::iter() does not exist. If you want to make it possible to represent multiple entries with the same DnType, I'd suggest changing the representation to hold a Vec<(DnType, DnValue)> and then build on top of that.
There can be multiple of the same entry, e.g. DC (domain component) entries in an issuer. Using a HashMap is too restrictive.
da-kami
force-pushed
the
multiple-domain-components-in-issuer
branch
from
7192b84 to
b327923
Compare
See: #300
I did not invest a big amount of time into changing the interface; please let me know if this PR is welcome otherwise I'm happy to close it. It would be great if #300 can be overcome at some point.