Skip to content

Conversation

@LarsxGitHub
Copy link

@LarsxGitHub LarsxGitHub commented Dec 17, 2024

While profiles have been added into the rotation via #200, the Get and Probe RPCs have remained profile-less. If a client is able to add multiple profiles, it should also be able to get and probe these profiles individually.

@robshakir robshakir self-requested a review December 17, 2024 18:13
//
// Note that the authz profile is considered independent from a SSL profile
// ID (as referenced by gnsi.Certz).
string authz_profile_id = 3;
Copy link
Member

Choose a reason for hiding this comment

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

I discussed with Marcus too. I think this makes sense to have here so that you can know exactly what authz policy you're probing.

However, can we add a clarification that says that you can only have one rotate operation in progress at once, to avoid the complexity of the state machine having to handle "what happens if you start multiple operations and then probe another one".

Copy link
Member

Choose a reason for hiding this comment

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

I made a similar change for Certz here: #201

Copy link
Author

Choose a reason for hiding this comment

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

Maybe I'm missing something, but the authz_profile_id in RotateAuthzRequest already has an extra docstring paragraph specifying "It is not permitted to rotate multiple profiles' policies at the same time ..." (lines 156--160).

My understanding from the current probe docstring is that it is supposed to query whatever the current engine state (for that profile) is. Even within a rotation, there is only one such engine state (either the initial one or the one after upload request), so I'm not quite sure where the state machine complexity is supposed to come from?

I think my point is: In contrast to the Rotate RPC, Probe and Get read to me as an "atomic", isolated operation rather than a state transition, so I don't see how it may complicate the state machine.

Copy link
Contributor

Choose a reason for hiding this comment

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

+1

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