-
-
Notifications
You must be signed in to change notification settings - Fork 612
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
Use consistent naming for dnsName gRPC fields #7654
Conversation
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.
Overall I love this change. I think it also clarifies what a huge task it'll be to support multiple identifier types.
@@ -125,7 +125,7 @@ type ValidationRecord struct { | |||
URL string `json:"url,omitempty"` | |||
|
|||
// Shared | |||
Hostname string `json:"hostname,omitempty"` | |||
DnsName string `json:"hostname,omitempty"` |
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.
Do we want to adjust the JSON field of hostname
here? If so, there could be some downstream workflows that will need changes as well (e.g. log checkers).
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.
Yeah, it was my thinking that it's okay to leave the json name of this field as-is for now, since we're going to have to replace this field entirely in the near future anyway. No need for the downstream tools to have to go through two changes, when the main point of this change is to just get Boulder using the same language everywhere to make future changes easier.
Find all gRPC fields which represent DNS Names -- sometimes called "identifier", "hostname", "domain", "identifierValue", or other things -- and unify their naming. This naming makes it very clear that these values are strings which may be included in the SAN extension of a certificate with type dnsName.
As we move towards issuing IP Address certificates, all of these fields will need to be replaced by fields which carry both an identifier type and value, not just a single name. This unified naming makes it very clear which messages and methods need to be updated to support non-dnsName identifiers.
Part of #7647