[v18] AccessList: Restore (deprecated) "dynamic" type for compatibility

api/types/accesslist/accesslist.go

@@ -180,6 +180,11 @@ type Spec struct {
 type Type string
 
 const (
+	// TODO(kopiczko) v21: Remove DeprecatedDynamic. The only version setting this type is 17.5.4.
+
+	// DeprecatedDynamic is deprecated and should not be used. Use [Default] instead. It has
+	// the same semantic meaning.
+	DeprecatedDynamic Type = "dynamic"
 	// Default Access Lists are the default type supposed to be managed with the web UI. They
 	// require periodic audit reviews.
 	Default Type = ""
@@ -191,23 +196,19 @@ const (
 	SCIM Type = "scim"
 )
 
-func NewTypeFromString(s string) (Type, error) {
-	switch s {
-	case string(Default):
-		return Default, nil
-	case string(Static):
-		return Static, nil
-	case string(SCIM):
-		return SCIM, nil
+func validateType(t Type) error {
+	switch t {
+	case DeprecatedDynamic, Default, Static, SCIM:
+		return nil
 	default:
-		return "", trace.BadParameter("unknown access_list type %q", s)
+		return trace.BadParameter("unknown access_list type %q", t)
 	}
 }
 
 // IsReviewable returns true if the AccessList type supports the audit reviews in the web UI.
 func (t Type) IsReviewable() bool {
 	switch t {
-	case Default:
+	case DeprecatedDynamic, Default:
 		return true
 	default:
 		return false
@@ -339,7 +340,12 @@ func (a *AccessList) CheckAndSetDefaults() error {
 		return trace.Wrap(err)
 	}
 
-	if _, err := NewTypeFromString(string(a.Spec.Type)); err != nil {
+	// Restore the type if the cluster was ever running in version 17.5.4.
+	if a.Spec.Type == DeprecatedDynamic {
+		a.Spec.Type = Default
+	}
+
+	if err := validateType(a.Spec.Type); err != nil {
 		return trace.Wrap(err)
 	}
 

api/types/accesslist/accesslist_test.go

@@ -18,6 +18,7 @@ package accesslist
 
 import (
 	"encoding/json"
+	"fmt"
 	"testing"
 	"time"
 
@@ -280,40 +281,44 @@ func TestAccessListDefaults(t *testing.T) {
 		require.ErrorContains(t, err, `unknown access_list type "test_unknown_type"`)
 	})
 }
-
 func TestSelectNextReviewDate(t *testing.T) {
 	t.Parallel()
 
 	tests := []struct {
 		name              string
+		accessListTypes   []Type
 		frequency         ReviewFrequency
 		dayOfMonth        ReviewDayOfMonth
 		currentReviewDate time.Time
 		expected          time.Time
 	}{
 		{
 			name:              "one month, first day",
+			accessListTypes:   []Type{Default, DeprecatedDynamic},
 			frequency:         OneMonth,
 			dayOfMonth:        FirstDayOfMonth,
 			currentReviewDate: time.Date(2023, 1, 1, 0, 0, 0, 0, time.UTC),
 			expected:          time.Date(2023, 2, 1, 0, 0, 0, 0, time.UTC),
 		},
 		{
 			name:              "one month, fifteenth day",
+			accessListTypes:   []Type{Default, DeprecatedDynamic},
 			frequency:         OneMonth,
 			dayOfMonth:        FifteenthDayOfMonth,
 			currentReviewDate: time.Date(2023, 1, 1, 0, 0, 0, 0, time.UTC),
 			expected:          time.Date(2023, 2, 15, 0, 0, 0, 0, time.UTC),
 		},
 		{
 			name:              "one month, last day",
+			accessListTypes:   []Type{Default, DeprecatedDynamic},
 			frequency:         OneMonth,
 			dayOfMonth:        LastDayOfMonth,
 			currentReviewDate: time.Date(2023, 1, 1, 0, 0, 0, 0, time.UTC),
 			expected:          time.Date(2023, 2, 28, 0, 0, 0, 0, time.UTC),
 		},
 		{
 			name:              "six months, last day",
+			accessListTypes:   []Type{Default, DeprecatedDynamic},
 			frequency:         SixMonths,
 			dayOfMonth:        LastDayOfMonth,
 			currentReviewDate: time.Date(2023, 1, 1, 0, 0, 0, 0, time.UTC),
@@ -324,14 +329,18 @@ func TestSelectNextReviewDate(t *testing.T) {
 	for _, test := range tests {
 		test := test
 		t.Run(test.name, func(t *testing.T) {
-			t.Parallel()
-			accessList := AccessList{}
-			accessList.Spec.Audit.NextAuditDate = test.currentReviewDate
-			accessList.Spec.Audit.Recurrence = Recurrence{
-				Frequency:  test.frequency,
-				DayOfMonth: test.dayOfMonth,
+			for _, typ := range test.accessListTypes {
+				t.Run(fmt.Sprintf("type=%q", typ), func(t *testing.T) {
+					accessList := AccessList{}
+					accessList.Spec.Type = typ
+					accessList.Spec.Audit.NextAuditDate = test.currentReviewDate
+					accessList.Spec.Audit.Recurrence = Recurrence{
+						Frequency:  test.frequency,
+						DayOfMonth: test.dayOfMonth,
+					}
+					require.Equal(t, test.expected, accessList.SelectNextReviewDate())
+				})
 			}
-			require.Equal(t, test.expected, accessList.SelectNextReviewDate())
 		})
 	}
 }

api/types/accesslist/convert/v1/accesslist_test.go

@@ -89,26 +89,57 @@ func TestWithOwnersIneligibleStatusField(t *testing.T) {
 }
 
 func TestRoundtrip(t *testing.T) {
-	t.Run("with custom subkind", func(t *testing.T) {
-		accessList := newAccessList(t, "access-list")
-		accessList.ResourceHeader.SetSubKind("access-list-subkind")
+	t.Parallel()
 
-		converted, err := FromProto(ToProto(accessList))
-		require.NoError(t, err)
+	type testCase struct {
+		name           string
+		modificationFn func(*accesslist.AccessList)
+	}
 
-		require.Empty(t, cmp.Diff(accessList, converted))
-	})
+	for _, tc := range []testCase{
+		{
+			name:           "no-modifications",
+			modificationFn: func(accessList *accesslist.AccessList) {},
+		},
+		{
+			name: "with-subkind",
+			modificationFn: func(accessList *accesslist.AccessList) {
+				accessList.ResourceHeader.SetSubKind("access-list-subkind")
+			},
+		},
+		{
+			name: "deprecated-dynamic-type",
+			modificationFn: func(accessList *accesslist.AccessList) {
+				accessList.Spec.Type = accesslist.DeprecatedDynamic
+			},
+		},
+		{
+			name: "default-type",
+			modificationFn: func(accessList *accesslist.AccessList) {
+				accessList.Spec.Type = accesslist.Default
+			},
+		},
+		{
+			name: "static-type",
+			modificationFn: func(accessList *accesslist.AccessList) {
+				accessList.Spec.Type = accesslist.Static
+			},
+		},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			accessList := newAccessList(t, "access-list")
+			tc.modificationFn(accessList)
 
-	t.Run("with non-default type", func(t *testing.T) {
-		accessList := newAccessList(t, "access-list")
-		accessList.Spec.Type = accesslist.Static
+			converted, err := FromProto(ToProto(accessList))
+			require.NoError(t, err)
 
-		converted, err := FromProto(ToProto(accessList))
-		require.NoError(t, err)
+			if accessList.Spec.Type == accesslist.DeprecatedDynamic {
+				accessList.Spec.Type = accesslist.Default
+			}
 
-		require.Empty(t, cmp.Diff(accessList, converted))
-		require.Equal(t, accesslist.Static, converted.Spec.Type)
-	})
+			require.Empty(t, cmp.Diff(accessList, converted))
+		})
+	}
 }
 
 func Test_FromProto_withBadType(t *testing.T) {

integrations/access/accesslist/app_test.go

@@ -148,62 +148,65 @@ func TestAccessListReminders_Single(t *testing.T) {
 		require.NoError(t, app.Err())
 	})
 
-	accessList, err := accesslist.NewAccessList(header.Metadata{
-		Name: "test-access-list",
-	}, accesslist.Spec{
-		Title:  "test access list",
-		Owners: []accesslist.Owner{{Name: "owner1"}, {Name: "not-found"}},
-		Grants: accesslist.Grants{
-			Roles: []string{"role"},
-		},
-		Audit: accesslist.Audit{
-			NextAuditDate: clock.Now().Add(28 * 24 * time.Hour), // Four weeks out from today
-			Notifications: accesslist.Notifications{
-				Start: oneDay * 14, // Start alerting at two weeks before audit date
+	for _, typ := range []accesslist.Type{accesslist.Default, accesslist.DeprecatedDynamic} {
+		accessList, err := accesslist.NewAccessList(header.Metadata{
+			Name: "test-access-list",
+		}, accesslist.Spec{
+			Type:   typ,
+			Title:  "test access list",
+			Owners: []accesslist.Owner{{Name: "owner1"}, {Name: "not-found"}},
+			Grants: accesslist.Grants{
+				Roles: []string{"role"},
 			},
-		},
-	})
-	require.NoError(t, err)
+			Audit: accesslist.Audit{
+				NextAuditDate: clock.Now().Add(28 * 24 * time.Hour), // Four weeks out from today
+				Notifications: accesslist.Notifications{
+					Start: oneDay * 14, // Start alerting at two weeks before audit date
+				},
+			},
+		})
+		require.NoError(t, err)
 
-	accessLists := []*accesslist.AccessList{accessList}
+		accessLists := []*accesslist.AccessList{accessList}
 
-	// No notifications for today
-	advanceAndLookForRecipients(t, bot, as, clock, 0, accessLists)
+		// No notifications for today
+		advanceAndLookForRecipients(t, bot, as, clock, 0, accessLists)
 
-	// Advance by one week, expect no notifications.
-	advanceAndLookForRecipients(t, bot, as, clock, oneDay*7, accessLists)
+		// Advance by one week, expect no notifications.
+		advanceAndLookForRecipients(t, bot, as, clock, oneDay*7, accessLists)
 
-	// Advance by one week, expect a notification. "not-found" will be missing as a recipient.
-	advanceAndLookForRecipients(t, bot, as, clock, oneDay*7, accessLists, "owner1")
+		// Advance by one week, expect a notification. "not-found" will be missing as a recipient.
+		advanceAndLookForRecipients(t, bot, as, clock, oneDay*7, accessLists, "owner1")
 
-	// Add a new owner.
-	accessList.Spec.Owners = append(accessList.Spec.Owners, accesslist.Owner{Name: "owner2"})
+		// Add a new owner.
+		accessList.Spec.Owners = append(accessList.Spec.Owners, accesslist.Owner{Name: "owner2"})
 
-	// Advance by one day, expect a notification only to the new owner.
-	advanceAndLookForRecipients(t, bot, as, clock, oneDay, accessLists, "owner2")
+		// Advance by one day, expect a notification only to the new owner.
+		advanceAndLookForRecipients(t, bot, as, clock, oneDay, accessLists, "owner2")
 
-	// Advance by one day, expect no notifications.
-	advanceAndLookForRecipients(t, bot, as, clock, oneDay, accessLists)
+		// Advance by one day, expect no notifications.
+		advanceAndLookForRecipients(t, bot, as, clock, oneDay, accessLists)
 
-	// Advance by five more days, to the next week, expect two notifications
-	advanceAndLookForRecipients(t, bot, as, clock, oneDay*5, accessLists, "owner1", "owner2")
+		// Advance by five more days, to the next week, expect two notifications
+		advanceAndLookForRecipients(t, bot, as, clock, oneDay*5, accessLists, "owner1", "owner2")
 
-	// Advance by one day, expect no notifications
-	advanceAndLookForRecipients(t, bot, as, clock, oneDay, accessLists)
+		// Advance by one day, expect no notifications
+		advanceAndLookForRecipients(t, bot, as, clock, oneDay, accessLists)
 
-	// Advance by one day, expect no notifications
-	advanceAndLookForRecipients(t, bot, as, clock, oneDay, accessLists)
+		// Advance by one day, expect no notifications
+		advanceAndLookForRecipients(t, bot, as, clock, oneDay, accessLists)
 
-	// Advance by five more days, to the next week, expect two notifications
-	advanceAndLookForRecipients(t, bot, as, clock, oneDay*5, accessLists, "owner1", "owner2")
+		// Advance by five more days, to the next week, expect two notifications
+		advanceAndLookForRecipients(t, bot, as, clock, oneDay*5, accessLists, "owner1", "owner2")
 
-	// Advance 60 days a day at a time, expect two notifications each time.
-	for i := 0; i < 60; i++ {
-		// Make sure we only get a notification once per day by iterating through each 6 hours at a time.
-		for j := 0; j < 3; j++ {
-			advanceAndLookForRecipients(t, bot, as, clock, 6*time.Hour, accessLists)
+		// Advance 60 days a day at a time, expect two notifications each time.
+		for range 60 {
+			// Make sure we only get a notification once per day by iterating through each 6 hours at a time.
+			for range 3 {
+				advanceAndLookForRecipients(t, bot, as, clock, 6*time.Hour, accessLists)
+			}
+			advanceAndLookForRecipients(t, bot, as, clock, 6*time.Hour, accessLists, "owner1", "owner2")
 		}
-		advanceAndLookForRecipients(t, bot, as, clock, 6*time.Hour, accessLists, "owner1", "owner2")
 	}
 }
 

lib/services/local/access_list_test.go

@@ -722,7 +722,7 @@ func TestAccessListReviewCRUD(t *testing.T) {
 
 	// Create a couple access lists.
 	accessList1 := newAccessList(t, "accessList1", clock)
-	accessList2 := newAccessList(t, "accessList2", clock)
+	accessList2 := newAccessList(t, "accessList2", clock, withType(accesslist.DeprecatedDynamic))
 
 	accessList1OrigDate := accessList1.Spec.Audit.NextAuditDate
 	accessList2OrigDate := accessList2.Spec.Audit.NextAuditDate
@@ -1017,7 +1017,19 @@ func TestAccessListRequiresEqual(t *testing.T) {
 	}
 }
 
-func newAccessList(t *testing.T, name string, clock clockwork.Clock) *accesslist.AccessList {
+type newAccessListOptions struct {
+	typ accesslist.Type
+}
+
+type newAccessListOpt func(*newAccessListOptions)
+
+func withType(typ accesslist.Type) newAccessListOpt {
+	return func(o *newAccessListOptions) {
+		o.typ = typ
+	}
+}
+
+func newAccessList(t *testing.T, name string, clock clockwork.Clock, opts ...newAccessListOpt) *accesslist.AccessList {
 	t.Helper()
 
 	accessList, err := accesslist.NewAccessList(