From b2c8f1ba8534ef2c1b200c3a46186e5cbbed040b Mon Sep 17 00:00:00 2001 From: Hugo Hervieux Date: Wed, 17 Jan 2024 10:12:25 -0500 Subject: [PATCH 1/3] add test for AL next audit date zero time --- .../accesslist/convert/v1/accesslist_test.go | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/api/types/accesslist/convert/v1/accesslist_test.go b/api/types/accesslist/convert/v1/accesslist_test.go index 9d6729a629bc4..30fcc61db1b92 100644 --- a/api/types/accesslist/convert/v1/accesslist_test.go +++ b/api/types/accesslist/convert/v1/accesslist_test.go @@ -242,3 +242,27 @@ func newAccessList(t *testing.T, name string) *accesslist.AccessList { require.NoError(t, err) return accessList } + +func TestNextAuditDateZeroTime(t *testing.T) { + // When a proto message without expiration is converted to an AL + // we expect next audit date to be mapped to golang's zero time. Then + // AccessList.CheckAndSetDefaults will set a time in the future based on + // the recurrence rules. + accessList := ToProto(newAccessList(t, "access-list")) + accessList.Spec.Audit.NextAuditDate = nil + converted, err := FromProto(accessList) + + require.NoError(t, err) + require.False( + t, + converted.Spec.Audit.NextAuditDate.Unix() == 0, + "next audit date should not be epoch", + ) + + converted.Spec.Audit.NextAuditDate = time.Time{} + // When an Access List without next audit date is converted to protobuf + // it should be nil. + convertedTwice := ToProto(converted) + + require.Nil(t, convertedTwice.Spec.Audit.NextAuditDate) +} From 25c2a25196975d6d01127fb2c9459cc49720366d Mon Sep 17 00:00:00 2001 From: Hugo Hervieux Date: Wed, 17 Jan 2024 10:12:43 -0500 Subject: [PATCH 2/3] fix AL next audit date time conversion --- api/types/accesslist/convert/v1/accesslist.go | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/api/types/accesslist/convert/v1/accesslist.go b/api/types/accesslist/convert/v1/accesslist.go index 5a043ad9cd626..87bb4aafbe296 100644 --- a/api/types/accesslist/convert/v1/accesslist.go +++ b/api/types/accesslist/convert/v1/accesslist.go @@ -20,6 +20,7 @@ import ( "github.com/gravitational/trace" "google.golang.org/protobuf/types/known/durationpb" "google.golang.org/protobuf/types/known/timestamppb" + "time" accesslistv1 "github.com/gravitational/teleport/api/gen/proto/go/teleport/accesslist/v1" "github.com/gravitational/teleport/api/types/accesslist" @@ -83,12 +84,20 @@ func FromProto(msg *accesslistv1.AccessList, opts ...AccessListOption) (*accessl } } + // We map the zero protobuf time (nil) to the zero go time. + // NewAccessList will handle this properly and set a time in the future + // based on the recurrence rules. + var nextAuditDate time.Time + if msg.Spec.Audit.NextAuditDate != nil { + nextAuditDate = msg.Spec.Audit.NextAuditDate.AsTime() + } + accessList, err := accesslist.NewAccessList(headerv1.FromMetadataProto(msg.Header.Metadata), accesslist.Spec{ Title: msg.Spec.Title, Description: msg.Spec.Description, Owners: owners, Audit: accesslist.Audit{ - NextAuditDate: msg.Spec.Audit.NextAuditDate.AsTime(), + NextAuditDate: nextAuditDate, Recurrence: recurrence, Notifications: notifications, }, @@ -149,6 +158,12 @@ func ToProto(accessList *accesslist.AccessList) *accesslistv1.AccessList { ownerGrants.Traits = traitv1.ToProto(accessList.Spec.OwnerGrants.Traits) } + // We map the zero go time to the zero protobuf time (nil). + var nextAuditDate *timestamppb.Timestamp + if !accessList.Spec.Audit.NextAuditDate.IsZero() { + nextAuditDate = timestamppb.New(accessList.Spec.Audit.NextAuditDate) + } + return &accesslistv1.AccessList{ Header: headerv1.ToResourceHeaderProto(accessList.ResourceHeader), Spec: &accesslistv1.AccessListSpec{ @@ -158,7 +173,7 @@ func ToProto(accessList *accesslist.AccessList) *accesslistv1.AccessList { Membership: string(accessList.Spec.Membership), Owners: owners, Audit: &accesslistv1.AccessListAudit{ - NextAuditDate: timestamppb.New(accessList.Spec.Audit.NextAuditDate), + NextAuditDate: nextAuditDate, Recurrence: &accesslistv1.Recurrence{ Frequency: accesslistv1.ReviewFrequency(accessList.Spec.Audit.Recurrence.Frequency), DayOfMonth: accesslistv1.ReviewDayOfMonth(accessList.Spec.Audit.Recurrence.DayOfMonth), From a1ee4d23724100438396663c6e133df8e30eace5 Mon Sep 17 00:00:00 2001 From: Hugo Hervieux Date: Wed, 17 Jan 2024 14:03:48 -0500 Subject: [PATCH 3/3] lint --- api/types/accesslist/convert/v1/accesslist.go | 3 ++- api/types/accesslist/convert/v1/accesslist_test.go | 4 ++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/api/types/accesslist/convert/v1/accesslist.go b/api/types/accesslist/convert/v1/accesslist.go index 87bb4aafbe296..82d29e1dad6b0 100644 --- a/api/types/accesslist/convert/v1/accesslist.go +++ b/api/types/accesslist/convert/v1/accesslist.go @@ -17,10 +17,11 @@ limitations under the License. package v1 import ( + "time" + "github.com/gravitational/trace" "google.golang.org/protobuf/types/known/durationpb" "google.golang.org/protobuf/types/known/timestamppb" - "time" accesslistv1 "github.com/gravitational/teleport/api/gen/proto/go/teleport/accesslist/v1" "github.com/gravitational/teleport/api/types/accesslist" diff --git a/api/types/accesslist/convert/v1/accesslist_test.go b/api/types/accesslist/convert/v1/accesslist_test.go index 30fcc61db1b92..d885811948976 100644 --- a/api/types/accesslist/convert/v1/accesslist_test.go +++ b/api/types/accesslist/convert/v1/accesslist_test.go @@ -253,9 +253,9 @@ func TestNextAuditDateZeroTime(t *testing.T) { converted, err := FromProto(accessList) require.NoError(t, err) - require.False( + require.NotZero( t, - converted.Spec.Audit.NextAuditDate.Unix() == 0, + converted.Spec.Audit.NextAuditDate.Unix(), "next audit date should not be epoch", )