Skip to content

Add int-type Authz ID proto fields alongside any string-type proto fields#8754

Open
ezekiel wants to merge 15 commits into
mainfrom
ezekiel/unify-authzid-type-stage1
Open

Add int-type Authz ID proto fields alongside any string-type proto fields#8754
ezekiel wants to merge 15 commits into
mainfrom
ezekiel/unify-authzid-type-stage1

Conversation

@ezekiel

@ezekiel ezekiel commented May 19, 2026

Copy link
Copy Markdown
Member

This is stage one of normalizing Authz ID to be consistently int64-type (#8722).

This is stage one of normalizing Authz ID to be consistently int64-type.
@ezekiel ezekiel requested a review from a team as a code owner May 19, 2026 15:22
@ezekiel ezekiel requested a review from jsha May 19, 2026 15:22
Comment thread va/caa.go Outdated
Comment thread core/objects.go Outdated
Comment thread grpc/pb-marshalling.go Outdated
Comment thread grpc/pb-marshalling.go Outdated
Comment thread grpc/pb-marshalling.go Outdated
ezekiel and others added 2 commits May 29, 2026 13:07
Co-authored-by: Aaron Gable <aaron@letsencrypt.org>
AuthorizationID and AuthorizationIDs in sa.proto where unreferenced and
unused. Apparent remnants from before Authz 2.
@ezekiel

ezekiel commented Jun 1, 2026

Copy link
Copy Markdown
Member Author

Still adding real fixes for load-generator.

Comment thread grpc/pb-marshalling.go
Comment thread grpc/pb-marshalling_test.go
Comment thread va/caa.go
Comment thread va/caa.go Outdated
Comment thread va/va_test.go
Comment thread ra/ra_test.go Outdated
Comment thread ra/ra_test.go
Comment thread ra/ra.go Outdated
Comment thread ra/ra.go Outdated
@ezekiel ezekiel requested a review from aarongable June 10, 2026 16:47

@jsha jsha left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Two comments:

You asked this morning about possible split points for future PRs like this. One natural split point I noticed: the load-generator changes. That could be a nice tidy separate PR refactoring the load-generator not to depend on core.Authorization. Not needed for this one but as an FYI.

Also, I noticed the if id != 0 { ... } else if idstr != "" { ... } else { ... } pattern in four places. That's enough to justify a shared helper:

// integerID returns `id` if it is non-zero, otherwise `idStr` parsed as an integer.
// Returns either if both args have their zero value.
func integerID(id int64, idStr string) (int64, error) {

@ezekiel

ezekiel commented Jun 11, 2026

Copy link
Copy Markdown
Member Author

Very helpful, thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants