Add in audit review recurrence presets. #32843
Conversation
mdwn
force-pushed
the
mike.wilson/access-list-recurrence-presets
branch
2 times, most recently
from
7f8547e
to
25aa389
Compare
mdwn
force-pushed
the
mike.wilson/access-list-recurrence-presets
branch
2 times, most recently
from
4498f1c
to
ce1873c
Compare
public-teleport-github-review-bot
bot
removed request for
espadolini and
ibeckermayer
Access Lists now have audit review recurrence presets. These allow users to specify review frequencies of 1, 3, 6, or 12 months, and specify the 1st, 15th, or last days of the target month. Presets have been used for their simplicity over other various recurrence definition mechanisms, as these presets are much clearer than many of the other options.
mdwn
force-pushed
the
mike.wilson/access-list-recurrence-presets
branch
from
ce1873c
to
d4d55fb
Compare
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
| @@ -80,11 +79,40 @@ message AccessListOwner { | |||
|
|
|||
| // AccessListAudit describes the audit configuration for an access list. | |||
| message AccessListAudit { | |||
| // frequency is a duration that describes how often an access list must be audited. | |||
| google.protobuf.Duration frequency = 1; | |||
There was a problem hiding this comment.
This change has broken the tests in teleport.e as there are tests that still refer to these fields. I'm sort of surprised and not surprised that the teleport.e tests do not run on CI, but that aside, I would have though that this sort of change of removing a field from a protobuf message goes against the major philosophy of protobuf being strong support for forward and backward compatibility. Typically fields hang around forever, but get deprecated instead of being removed. I don't know if this is the Teleport stance though - I'm coming at this from outside experience with protobuf/grpc. Perhaps because the accesslist stuff is all rather new we're ok with this but I wonder if the package name should have been teleport.accesslist.v0 (or v1alpha1) until stabilised.
There was a problem hiding this comment.
You're not wrong. I was originally intending on keeping the frequency around and having it convert, but I got some pushback on that end. I have (and have had) a fix for this: https://github.com/gravitational/teleport.e/pull/2324, but it's still waiting on reviews.
Access Lists now have audit review recurrence presets. These allow users to specify review frequencies of 1, 3, 6, or 12 months, and specify the 1st, 15th, or last days of the target month. Presets have been used for their simplicity over other various recurrence definition mechanisms, as these presets are much clearer than many of the other options.