-
Notifications
You must be signed in to change notification settings - Fork 114
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
Multiple domain components in issuer #301
base: main
Are you sure you want to change the base?
Conversation
5243530
to
7192b84
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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?
7192b84
to
b327923
Compare
b327923
to
062af97
Compare
e12523c
to
0ecfc1b
Compare
There can be multiple of the same entry, e.g. DC (domain component) entries in an issuer.
2e18fbb
to
8e0466f
Compare
8e0466f
to
dd62f8c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, this looks good to me!
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.