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.

Sign up for GitHub

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

Merged
merged 4 commits into from
Aug 12, 2024
Merged

Use consistent naming for dnsName gRPC fields #7654

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
4ed2dac
Use consistent naming for dnsName gRPC fields
aarongable Aug 9, 2024
20dfbc4
Merge branch main into rename-proto-fields
aarongable Aug 12, 2024
45de422
Review comment
aarongable Aug 12, 2024
7f90c9e
Merge branch main into rename-proto-fields
aarongable Aug 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion cmd/cert-checker/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -312,7 +312,7 @@ func (c *certChecker) checkValidations(ctx context.Context, cert core.Certificat
// Any authorization for a given name is sufficient.
nameToAuthz := make(map[string]*corepb.Authorization)
for _, m := range authzs {
nameToAuthz[m.Identifier] = m
nameToAuthz[m.DnsName] = m
}

var errors []error
Expand Down
8 changes: 4 additions & 4 deletions core/objects.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ type ValidationRecord struct {
URL string `json:"url,omitempty"`

// Shared
Hostname string `json:"hostname,omitempty"`
DnsName string `json:"hostname,omitempty"`
Copy link
Member

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).

Copy link
Contributor Author

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.

Port string `json:"port,omitempty"`
AddressesResolved []net.IP `json:"addressesResolved,omitempty"`
AddressUsed net.IP `json:"addressUsed,omitempty"`
Expand Down Expand Up @@ -217,7 +217,7 @@ func (ch Challenge) RecordsSane() bool {
for _, rec := range ch.ValidationRecord {
// TODO(#7140): Add a check for ResolverAddress == "" only after the
// core.proto change has been deployed.
if rec.URL == "" || rec.Hostname == "" || rec.Port == "" || rec.AddressUsed == nil ||
if rec.URL == "" || rec.DnsName == "" || rec.Port == "" || rec.AddressUsed == nil ||
len(rec.AddressesResolved) == 0 {
return false
}
Expand All @@ -231,7 +231,7 @@ func (ch Challenge) RecordsSane() bool {
}
// TODO(#7140): Add a check for ResolverAddress == "" only after the
// core.proto change has been deployed.
if ch.ValidationRecord[0].Hostname == "" || ch.ValidationRecord[0].Port == "" ||
if ch.ValidationRecord[0].DnsName == "" || ch.ValidationRecord[0].Port == "" ||
ch.ValidationRecord[0].AddressUsed == nil || len(ch.ValidationRecord[0].AddressesResolved) == 0 {
return false
}
Expand All @@ -241,7 +241,7 @@ func (ch Challenge) RecordsSane() bool {
}
// TODO(#7140): Add a check for ResolverAddress == "" only after the
// core.proto change has been deployed.
if ch.ValidationRecord[0].Hostname == "" {
if ch.ValidationRecord[0].DnsName == "" {
return false
}
return true
Expand Down
2 changes: 1 addition & 1 deletion core/objects_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ func TestRecordSanityCheckOnUnsupportedChallengeType(t *testing.T) {
rec := []ValidationRecord{
{
URL: "http://localhost/test",
Hostname: "localhost",
DnsName: "localhost",
Port: "80",
AddressesResolved: []net.IP{{127, 0, 0, 1}},
AddressUsed: net.IP{127, 0, 0, 1},
Expand Down
275 changes: 140 additions & 135 deletions core/proto/core.pb.go
View file
Open in desktop

Large diffs are not rendered by default.

40 changes: 22 additions & 18 deletions core/proto/core.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,19 +7,20 @@ import "google/protobuf/timestamp.proto";

message Challenge {
// Next unused field number: 13
reserved 4, 8, 11;
int64 id = 1;
// Fields specified by RFC 8555, Section 8.
string type = 2;
string url = 9;
string status = 6;
string uri = 9;
google.protobuf.Timestamp validated = 12;
ProblemDetails error = 7;
// Fields specified by individual validation methods.
string token = 3;
reserved 4; // Previously accountKey
// Additional fields for our own record keeping.
repeated ValidationRecord validationrecords = 10;
// TODO(#7514): Remove this.
string keyAuthorization = 5;
repeated ValidationRecord validationrecords = 10;
ProblemDetails error = 7;
reserved 8; // Unused and accidentally skipped during initial commit.
reserved 11; // Previously validatedNS
google.protobuf.Timestamp validated = 12;
}

message ValidationRecord {
Expand Down Expand Up @@ -90,33 +91,36 @@ message Registration {

message Authorization {
// Next unused field number: 10
reserved 5, 7, 8;
string id = 1;
string identifier = 2;
int64 registrationID = 3;
// Fields specified by RFC 8555, Section 7.1.4
string dnsName = 2;
string status = 4;
reserved 5; // Previously expiresNS
google.protobuf.Timestamp expires = 9;
repeated core.Challenge challenges = 6;
reserved 7; // previously ACMEv1 combinations
reserved 8; // previously v2
// We do not directly represent the "wildcard" field, instead inferring it
// from the identifier value.
}

message Order {
// Next unused field number: 15
reserved 3, 6, 10;
int64 id = 1;
int64 registrationID = 2;
reserved 3; // Previously expiresNS
// Fields specified by RFC 8555, Section 7.1.3
// Note that we do not respect notBefore and notAfter, and we infer the
// finalize and certificate URLs from the id and certificateSerial fields.
string status = 7;
google.protobuf.Timestamp expires = 12;
repeated string dnsNames = 8;
ProblemDetails error = 4;
repeated int64 v2Authorizations = 11;
string certificateSerial = 5;
reserved 6; // previously authorizations, deprecated in favor of v2Authorizations
string status = 7;
repeated string names = 8;
bool beganProcessing = 9;
reserved 10; // Previously createdNS
// Additional fields for our own record-keeping.
google.protobuf.Timestamp created = 13;
repeated int64 v2Authorizations = 11;
string certificateProfileName = 14;
bool beganProcessing = 9;
}

message CRLEntry {
Expand Down
10 changes: 5 additions & 5 deletions grpc/pb-marshalling.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ func ValidationRecordToPB(record core.ValidationRecord) (*corepb.ValidationRecor
return nil, err
}
return &corepb.ValidationRecord{
Hostname: record.Hostname,
Hostname: record.DnsName,
Port: record.Port,
AddressesResolved: addrs,
AddressUsed: addrUsed,
Expand Down Expand Up @@ -173,7 +173,7 @@ func PBToValidationRecord(in *corepb.ValidationRecord) (record core.ValidationRe
return
}
return core.ValidationRecord{
Hostname: in.Hostname,
DnsName: in.Hostname,
Port: in.Port,
AddressesResolved: addrs,
AddressUsed: addrUsed,
Expand Down Expand Up @@ -318,7 +318,7 @@ func AuthzToPB(authz core.Authorization) (*corepb.Authorization, error) {

return &corepb.Authorization{
Id: authz.ID,
Identifier: authz.Identifier.Value,
DnsName: authz.Identifier.Value,
RegistrationID: authz.RegistrationID,
Status: string(authz.Status),
Expires: expires,
Expand All @@ -342,7 +342,7 @@ func PBToAuthz(pb *corepb.Authorization) (core.Authorization, error) {
}
authz := core.Authorization{
ID: pb.Id,
Identifier: identifier.ACMEIdentifier{Type: identifier.DNS, Value: pb.Identifier},
Identifier: identifier.ACMEIdentifier{Type: identifier.DNS, Value: pb.DnsName},
RegistrationID: pb.RegistrationID,
Status: core.AcmeStatus(pb.Status),
Expires: expires,
Expand All @@ -366,7 +366,7 @@ func orderValid(order *corepb.Order) bool {
// `order.CertificateSerial` to be nil such that it can be used in places where
// the order has not been finalized yet.
func newOrderValid(order *corepb.Order) bool {
return !(order.RegistrationID == 0 || order.Expires == nil || len(order.Names) == 0)
return !(order.RegistrationID == 0 || order.Expires == nil || len(order.DnsNames) == 0)
}

func CertToPB(cert core.Certificate) *corepb.Certificate {
Expand Down
20 changes: 10 additions & 10 deletions grpc/pb-marshalling_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ func TestChallenge(t *testing.T) {
ip := net.ParseIP("1.1.1.1")
chall.ValidationRecord = []core.ValidationRecord{
{
Hostname: "example.com",
DnsName: "example.com",
Port: "2020",
AddressesResolved: []net.IP{ip},
AddressUsed: ip,
Expand Down Expand Up @@ -115,7 +115,7 @@ func TestChallenge(t *testing.T) {
func TestValidationRecord(t *testing.T) {
ip := net.ParseIP("1.1.1.1")
vr := core.ValidationRecord{
Hostname: "exampleA.com",
DnsName: "exampleA.com",
Port: "80",
AddressesResolved: []net.IP{ip},
AddressUsed: ip,
Expand All @@ -136,7 +136,7 @@ func TestValidationRecord(t *testing.T) {
func TestValidationResult(t *testing.T) {
ip := net.ParseIP("1.1.1.1")
vrA := core.ValidationRecord{
Hostname: "exampleA.com",
DnsName: "exampleA.com",
Port: "443",
AddressesResolved: []net.IP{ip},
AddressUsed: ip,
Expand All @@ -145,7 +145,7 @@ func TestValidationResult(t *testing.T) {
ResolverAddrs: []string{"resolver:5353"},
}
vrB := core.ValidationRecord{
Hostname: "exampleB.com",
DnsName: "exampleB.com",
Port: "443",
AddressesResolved: []net.IP{ip},
AddressUsed: ip,
Expand Down Expand Up @@ -302,7 +302,7 @@ func TestOrderValid(t *testing.T) {
Expires: timestamppb.New(expires),
CertificateSerial: "",
V2Authorizations: []int64{},
Names: []string{"example.com"},
DnsNames: []string{"example.com"},
BeganProcessing: false,
Created: timestamppb.New(created),
},
Expand All @@ -315,7 +315,7 @@ func TestOrderValid(t *testing.T) {
RegistrationID: 1,
Expires: timestamppb.New(expires),
V2Authorizations: []int64{},
Names: []string{"example.com"},
DnsNames: []string{"example.com"},
BeganProcessing: false,
Created: timestamppb.New(created),
},
Expand All @@ -333,7 +333,7 @@ func TestOrderValid(t *testing.T) {
Expires: timestamppb.New(expires),
CertificateSerial: "",
V2Authorizations: []int64{},
Names: []string{"example.com"},
DnsNames: []string{"example.com"},
BeganProcessing: false,
},
},
Expand All @@ -345,7 +345,7 @@ func TestOrderValid(t *testing.T) {
Expires: timestamppb.New(expires),
CertificateSerial: "",
V2Authorizations: []int64{},
Names: []string{"example.com"},
DnsNames: []string{"example.com"},
BeganProcessing: false,
},
},
Expand All @@ -357,7 +357,7 @@ func TestOrderValid(t *testing.T) {
Expires: nil,
CertificateSerial: "",
V2Authorizations: []int64{},
Names: []string{"example.com"},
DnsNames: []string{"example.com"},
BeganProcessing: false,
},
},
Expand All @@ -369,7 +369,7 @@ func TestOrderValid(t *testing.T) {
Expires: timestamppb.New(expires),
CertificateSerial: "",
V2Authorizations: []int64{},
Names: []string{},
DnsNames: []string{},
BeganProcessing: false,
},
},
Expand Down
6 changes: 3 additions & 3 deletions mocks/sa.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -350,7 +350,7 @@ func (sa *StorageAuthority) NewOrderAndAuthzs(_ context.Context, req *sapb.NewOr
// Fields from the input new order request.
RegistrationID: req.NewOrder.RegistrationID,
Expires: req.NewOrder.Expires,
Names: req.NewOrder.Names,
DnsNames: req.NewOrder.DnsNames,
V2Authorizations: req.NewOrder.V2Authorizations,
// Mock new fields generated by the database transaction.
Id: rand.Int64(),
Expand Down Expand Up @@ -394,7 +394,7 @@ func (sa *StorageAuthorityReadOnly) GetOrder(_ context.Context, req *sapb.OrderR
RegistrationID: 1,
Created: timestamppb.New(created),
Expires: timestamppb.New(exp),
Names: []string{"example.com"},
DnsNames: []string{"example.com"},
Status: string(core.StatusValid),
V2Authorizations: []int64{1},
CertificateSerial: "serial",
Expand Down Expand Up @@ -470,7 +470,7 @@ func (sa *StorageAuthorityReadOnly) GetValidAuthorizations2(ctx context.Context,
}
expiryCutoff := req.ValidUntil.AsTime()
auths := &sapb.Authorizations{}
for _, name := range req.Domains {
for _, name := range req.DnsNames {
exp := expiryCutoff.AddDate(100, 0, 0)
authzPB, err := bgrpc.AuthzToPB(core.Authorization{
Status: core.StatusValid,
Expand Down
Loading