Skip to content

Commit

Permalink
Remove deprecated sapb.Authorizations.Authz "map" (#7658)
Browse files Browse the repository at this point in the history
This field was deprecated in
#7646 and the last uses of it
were removed in #7650.
  • Loading branch information
aarongable authored Aug 15, 2024
1 parent e1790a5 commit ced0117
Show file tree
Hide file tree
Showing 7 changed files with 764 additions and 879 deletions.
24 changes: 6 additions & 18 deletions grpc/pb-marshalling.go
Original file line number Diff line number Diff line change
Expand Up @@ -418,25 +418,13 @@ func PBToCertStatus(pb *corepb.CertificateStatus) core.CertificateStatus {
// PBToAuthzMap converts a protobuf map of domains mapped to protobuf authorizations to a
// golang map[string]*core.Authorization.
func PBToAuthzMap(pb *sapb.Authorizations) (map[identifier.ACMEIdentifier]*core.Authorization, error) {
m := make(map[identifier.ACMEIdentifier]*core.Authorization, len(pb.Authz))
if len(pb.Authzs) != 0 {
// Prefer the new field, if it is populated.
for _, v := range pb.Authzs {
authz, err := PBToAuthz(v)
if err != nil {
return nil, err
}
m[authz.Identifier] = &authz
}
} else {
// Fall back to the old field, if necessary.
for _, v := range pb.Authz {
authz, err := PBToAuthz(v.Authz)
if err != nil {
return nil, err
}
m[authz.Identifier] = &authz
m := make(map[identifier.ACMEIdentifier]*core.Authorization, len(pb.Authzs))
for _, v := range pb.Authzs {
authz, err := PBToAuthz(v)
if err != nil {
return nil, err
}
m[authz.Identifier] = &authz
}
return m, nil
}
5 changes: 1 addition & 4 deletions mocks/sa.go
Original file line number Diff line number Diff line change
Expand Up @@ -492,10 +492,7 @@ func (sa *StorageAuthorityReadOnly) GetValidAuthorizations2(ctx context.Context,
if err != nil {
return nil, err
}
auths.Authz = append(auths.Authz, &sapb.Authorizations_MapElement{
Domain: name,
Authz: authzPB,
})
auths.Authzs = append(auths.Authzs, authzPB)
}
return auths, nil
}
Expand Down
7 changes: 1 addition & 6 deletions ra/ra_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4149,12 +4149,7 @@ func (msa *mockSARevocationWithAuthzs) GetValidAuthorizations2(ctx context.Conte
}

for _, name := range req.DnsNames {
authzs.Authz = append(authzs.Authz, &sapb.Authorizations_MapElement{
Domain: name,
Authz: &corepb.Authorization{
DnsName: name,
},
})
authzs.Authzs = append(authzs.Authzs, &corepb.Authorization{DnsName: name})
}

return authzs, nil
Expand Down
1,545 changes: 729 additions & 816 deletions sa/proto/sa.pb.go

Large diffs are not rendered by default.

5 changes: 0 additions & 5 deletions sa/proto/sa.proto
Original file line number Diff line number Diff line change
Expand Up @@ -301,11 +301,6 @@ message GetAuthorizationsRequest {
}

message Authorizations {
message MapElement {
string domain = 1;
core.Authorization authz = 2;
}
repeated MapElement authz = 1; // Deprecated, use authzs instead.
repeated core.Authorization authzs = 2;
}

Expand Down
53 changes: 26 additions & 27 deletions sa/sa_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1779,7 +1779,7 @@ func TestGetAuthorizations2(t *testing.T) {
test.AssertNotError(t, err, "sa.GetAuthorizations2 failed")
// We should get back two authorizations since one of the three authorizations
// created above expires too soon.
test.AssertEquals(t, len(authz.Authz), 2)
test.AssertEquals(t, len(authz.Authzs), 2)

// Get authorizations for the names used above, and one name that doesn't exist
authz, err = sa.GetAuthorizations2(context.Background(), &sapb.GetAuthorizationsRequest{
Expand All @@ -1790,7 +1790,7 @@ func TestGetAuthorizations2(t *testing.T) {
// It should not fail
test.AssertNotError(t, err, "sa.GetAuthorizations2 failed")
// It should still return only two authorizations
test.AssertEquals(t, len(authz.Authz), 2)
test.AssertEquals(t, len(authz.Authzs), 2)
}

func TestCountOrders(t *testing.T) {
Expand Down Expand Up @@ -2970,12 +2970,11 @@ func TestAuthzModelMapToPB(t *testing.T) {
t.Fatal(err)
}

for _, el := range out.Authz {
model, ok := input[el.Domain]
for _, authzPB := range out.Authzs {
model, ok := input[authzPB.DnsName]
if !ok {
t.Errorf("output had element for %q, a hostname not present in input", el.Domain)
t.Errorf("output had element for %q, a hostname not present in input", authzPB.DnsName)
}
authzPB := el.Authz
test.AssertEquals(t, authzPB.Id, fmt.Sprintf("%d", model.ID))
test.AssertEquals(t, authzPB.DnsName, model.IdentifierValue)
test.AssertEquals(t, authzPB.RegistrationID, model.RegistrationID)
Expand All @@ -2984,21 +2983,21 @@ func TestAuthzModelMapToPB(t *testing.T) {
if !model.Expires.Equal(gotTime) {
t.Errorf("Times didn't match. Got %s, expected %s (%s)", gotTime, model.Expires, authzPB.Expires.AsTime())
}
if len(el.Authz.Challenges) != bits.OnesCount(uint(model.Challenges)) {
t.Errorf("wrong number of challenges for %q: got %d, expected %d", el.Domain,
len(el.Authz.Challenges), bits.OnesCount(uint(model.Challenges)))
if len(authzPB.Challenges) != bits.OnesCount(uint(model.Challenges)) {
t.Errorf("wrong number of challenges for %q: got %d, expected %d", authzPB.DnsName,
len(authzPB.Challenges), bits.OnesCount(uint(model.Challenges)))
}
switch model.Challenges {
case 1:
test.AssertEquals(t, el.Authz.Challenges[0].Type, "http-01")
test.AssertEquals(t, authzPB.Challenges[0].Type, "http-01")
case 3:
test.AssertEquals(t, el.Authz.Challenges[0].Type, "http-01")
test.AssertEquals(t, el.Authz.Challenges[1].Type, "dns-01")
test.AssertEquals(t, authzPB.Challenges[0].Type, "http-01")
test.AssertEquals(t, authzPB.Challenges[1].Type, "dns-01")
case 4:
test.AssertEquals(t, el.Authz.Challenges[0].Type, "tls-alpn-01")
test.AssertEquals(t, authzPB.Challenges[0].Type, "tls-alpn-01")
}

delete(input, el.Domain)
delete(input, authzPB.DnsName)
}

for k := range input {
Expand Down Expand Up @@ -3031,35 +3030,35 @@ func TestGetValidOrderAuthorizations2(t *testing.T) {
})
test.AssertNotError(t, err, "AddOrder failed")

authzMap, err := sa.GetValidOrderAuthorizations2(
authzPBs, err := sa.GetValidOrderAuthorizations2(
context.Background(),
&sapb.GetValidOrderAuthorizationsRequest{
Id: order.Id,
AcctID: reg.Id,
})
test.AssertNotError(t, err, "sa.GetValidOrderAuthorizations failed")
test.AssertNotNil(t, authzMap, "sa.GetValidOrderAuthorizations result was nil")
test.AssertEquals(t, len(authzMap.Authz), 2)
test.AssertNotNil(t, authzPBs, "sa.GetValidOrderAuthorizations result was nil")
test.AssertEquals(t, len(authzPBs.Authzs), 2)

namesToCheck := map[string]int64{"a.example.com": authzIDA, "b.example.com": authzIDB}
for _, a := range authzMap.Authz {
if fmt.Sprintf("%d", namesToCheck[a.Authz.DnsName]) != a.Authz.Id {
t.Fatalf("incorrect identifier %q with id %s", a.Authz.DnsName, a.Authz.Id)
for _, a := range authzPBs.Authzs {
if fmt.Sprintf("%d", namesToCheck[a.DnsName]) != a.Id {
t.Fatalf("incorrect identifier %q with id %s", a.DnsName, a.Id)
}
test.AssertEquals(t, a.Authz.Expires.AsTime(), expires)
delete(namesToCheck, a.Authz.DnsName)
test.AssertEquals(t, a.Expires.AsTime(), expires)
delete(namesToCheck, a.DnsName)
}

// Getting the order authorizations for an order that doesn't exist should return nothing
missingID := int64(0xC0FFEEEEEEE)
authzMap, err = sa.GetValidOrderAuthorizations2(
authzPBs, err = sa.GetValidOrderAuthorizations2(
context.Background(),
&sapb.GetValidOrderAuthorizationsRequest{
Id: missingID,
AcctID: reg.Id,
})
test.AssertNotError(t, err, "sa.GetValidOrderAuthorizations failed")
test.AssertEquals(t, len(authzMap.Authz), 0)
test.AssertEquals(t, len(authzPBs.Authzs), 0)
}

func TestCountInvalidAuthorizations2(t *testing.T) {
Expand Down Expand Up @@ -3111,9 +3110,9 @@ func TestGetValidAuthorizations2(t *testing.T) {
ValidUntil: timestamppb.New(now),
})
test.AssertNotError(t, err, "sa.GetValidAuthorizations2 failed")
test.AssertEquals(t, len(authzs.Authz), 1)
test.AssertEquals(t, authzs.Authz[0].Domain, ident)
test.AssertEquals(t, authzs.Authz[0].Authz.Id, fmt.Sprintf("%d", authzID))
test.AssertEquals(t, len(authzs.Authzs), 1)
test.AssertEquals(t, authzs.Authzs[0].DnsName, ident)
test.AssertEquals(t, authzs.Authzs[0].Id, fmt.Sprintf("%d", authzID))
}

func TestGetOrderExpired(t *testing.T) {
Expand Down
4 changes: 1 addition & 3 deletions sa/saro.go
Original file line number Diff line number Diff line change
Expand Up @@ -769,14 +769,12 @@ func (ssa *SQLStorageAuthorityRO) GetAuthorization2(ctx context.Context, req *sa
// protobuf authorizations map
func authzModelMapToPB(m map[string]authzModel) (*sapb.Authorizations, error) {
resp := &sapb.Authorizations{}
for k, v := range m {
for _, v := range m {
authzPB, err := modelToAuthzPB(v)
if err != nil {
return nil, err
}
resp.Authzs = append(resp.Authzs, authzPB)
// TODO(#7646): Stop populating the .Authzs field once it is no longer used.
resp.Authz = append(resp.Authz, &sapb.Authorizations_MapElement{Domain: k, Authz: authzPB})
}
return resp, nil
}
Expand Down

0 comments on commit ced0117

Please sign in to comment.