Skip to content

Conversation

@maraino
Copy link
Contributor

@maraino maraino commented Oct 7, 2025

This commit adds support for syncing ACME accounts with majordomo.

This commit adds support for syncing ACME accounts with majordomo.
@maraino maraino force-pushed the mariano/acme-accounts branch from 7795a6e to 65dfe01 Compare October 7, 2025 01:59
@maraino maraino requested a review from hslatman October 13, 2025 20:03
@maraino maraino marked this pull request as ready for review October 15, 2025 20:24
@maraino maraino requested a review from a team as a code owner October 15, 2025 20:24
Comment on lines +289 to +295
message CreateACMEAccountRequest {
string preferred_id = 1;
bytes jwk = 2;
ACMEAccount.Status status = 3;
repeated string contact = 4;
bool terms_of_service_agreed = 5;
}
Copy link
Member

Choose a reason for hiding this comment

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

Should there be a property to link this to a specific provisioner? I remember we used to allow an ACME account to be used with any ACME provisioner in the past, but I believe we changed that at some point?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not in this case, the intention is to sync accounts between different authorities, and those will have different provisioners.

Copy link
Member

@hslatman hslatman Oct 17, 2025

Choose a reason for hiding this comment

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

So the goal is to have linked authority X sync ACME accounts from provisioner Y to majordomo, so that linked authority Z get the accounts too, and for those to be active for all provisioners? And those authorities are totally independent; not clustered?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, they can be two different environments.

Copy link
Member

Choose a reason for hiding this comment

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

I'm OK with this if it is a functional requirement, but it could result in surprising results to the end users.

@maraino maraino requested review from hslatman and smst-jeff October 16, 2025 17:14
@maraino maraino merged commit 616ff12 into main Oct 20, 2025
13 checks passed
@maraino maraino deleted the mariano/acme-accounts branch October 20, 2025 21:20
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.

4 participants