From 2ce1808ecb579cec6073fb980ffe30f42cc37c3d Mon Sep 17 00:00:00 2001 From: daengdaengLee Date: Sat, 21 Sep 2024 17:18:14 +0900 Subject: [PATCH 1/5] fix: check err before use schedule and duration Signed-off-by: daengdaengLee --- pkg/apis/application/v1alpha1/types.go | 34 ++++++++++++++++++++++---- 1 file changed, 29 insertions(+), 5 deletions(-) diff --git a/pkg/apis/application/v1alpha1/types.go b/pkg/apis/application/v1alpha1/types.go index a0abc3d5573c0..f8f6a09f8a695 100644 --- a/pkg/apis/application/v1alpha1/types.go +++ b/pkg/apis/application/v1alpha1/types.go @@ -2399,8 +2399,16 @@ func (s *SyncWindows) active(currentTime time.Time) *SyncWindows { var active SyncWindows specParser := cron.NewParser(cron.Minute | cron.Hour | cron.Dom | cron.Month | cron.Dow) for _, w := range *s { - schedule, _ := specParser.Parse(w.Schedule) - duration, _ := time.ParseDuration(w.Duration) + schedule, sErr := specParser.Parse(w.Schedule) + if sErr != nil { + log.Warnf("invalid sync window schedule '%v': %s", w.Schedule, sErr) + continue + } + duration, dErr := time.ParseDuration(w.Duration) + if dErr != nil { + log.Warnf("invalid sync window duration '%v': %s", w.Duration, dErr) + continue + } // Offset the nextWindow time to consider the timeZone of the sync window timeZoneOffsetDuration := w.scheduleOffsetByTimeZone() @@ -2434,12 +2442,20 @@ func (s *SyncWindows) inactiveAllows(currentTime time.Time) *SyncWindows { for _, w := range *s { if w.Kind == "allow" { schedule, sErr := specParser.Parse(w.Schedule) + if sErr != nil { + log.Warnf("invalid sync window schedule '%v': %s", w.Schedule, sErr) + continue + } duration, dErr := time.ParseDuration(w.Duration) + if dErr != nil { + log.Warnf("invalid sync window duration '%v': %s", w.Duration, dErr) + continue + } // Offset the nextWindow time to consider the timeZone of the sync window timeZoneOffsetDuration := w.scheduleOffsetByTimeZone() nextWindow := schedule.Next(currentTime.Add(timeZoneOffsetDuration - duration)) - if !nextWindow.Before(currentTime.Add(timeZoneOffsetDuration)) && sErr == nil && dErr == nil { + if !nextWindow.Before(currentTime.Add(timeZoneOffsetDuration)) { inactive = append(inactive, w) } } @@ -2646,8 +2662,16 @@ func (w SyncWindow) active(currentTime time.Time) bool { currentTime = currentTime.UTC() specParser := cron.NewParser(cron.Minute | cron.Hour | cron.Dom | cron.Month | cron.Dow) - schedule, _ := specParser.Parse(w.Schedule) - duration, _ := time.ParseDuration(w.Duration) + schedule, sErr := specParser.Parse(w.Schedule) + if sErr != nil { + log.Warnf("invalid sync window schedule '%v': %s", w.Schedule, sErr) + return false + } + duration, dErr := time.ParseDuration(w.Duration) + if dErr != nil { + log.Warnf("invalid sync window duration '%v': %s", w.Duration, dErr) + return false + } // Offset the nextWindow time to consider the timeZone of the sync window timeZoneOffsetDuration := w.scheduleOffsetByTimeZone() From 3e2440952fd8929cca8d77f0d59c0dc42f982d71 Mon Sep 17 00:00:00 2001 From: daengdaengLee Date: Sun, 22 Sep 2024 17:21:19 +0900 Subject: [PATCH 2/5] test: add tests for invalid schedule and duration Signed-off-by: daengdaengLee --- pkg/apis/application/v1alpha1/types_test.go | 58 +++++++++++++++++++++ 1 file changed, 58 insertions(+) diff --git a/pkg/apis/application/v1alpha1/types_test.go b/pkg/apis/application/v1alpha1/types_test.go index d614711b811db..3413666f4e9e1 100644 --- a/pkg/apis/application/v1alpha1/types_test.go +++ b/pkg/apis/application/v1alpha1/types_test.go @@ -1926,6 +1926,24 @@ func TestSyncWindows_Active(t *testing.T) { matchingIndex: 0, expectedLength: 1, }, + { + name: "MatchNone-InvalidSchedule", + syncWindow: SyncWindows{ + syncWindow("allow", "* 10 * * 7", "3h", ""), + syncWindow("allow", "* 11 * * 7", "3h", ""), + }, + currentTime: timeWithHour(12, time.UTC), + expectedLength: 0, + }, + { + name: "MatchNone-InvalidDuration", + syncWindow: SyncWindows{ + syncWindow("allow", "* 10 * * *", "3a", ""), + syncWindow("allow", "* 11 * * *", "3a", ""), + }, + currentTime: timeWithHour(12, time.UTC), + expectedLength: 0, + }, } for _, tt := range tests { @@ -2099,6 +2117,22 @@ func TestSyncWindows_InactiveAllows(t *testing.T) { matchingIndex: 0, expectedLength: 1, }, + { + name: "MatchNone-InvalidSchedule", + syncWindow: SyncWindows{ + syncWindow("allow", "* 10 * * 7", "2h", ""), + }, + currentTime: timeWithHour(17, time.UTC), + expectedLength: 0, + }, + { + name: "MatchNone-InvalidDuration", + syncWindow: SyncWindows{ + syncWindow("allow", "* 10 * * *", "2a", ""), + }, + currentTime: timeWithHour(17, time.UTC), + expectedLength: 0, + }, } for _, tt := range tests { @@ -2653,6 +2687,30 @@ func TestSyncWindow_Active(t *testing.T) { currentTime: timeWithHour(13-4, utcM4Zone), expectedResult: false, }, + { + name: "Allow-inactive-InvalidSchedule", + syncWindow: syncWindow("allow", "* 10 * * 7", "2h"), + currentTime: timeWithHour(11, time.UTC), + expectedResult: false, + }, + { + name: "Deny-inactive-InvalidSchedule", + syncWindow: syncWindow("deny", "* 10 * * 7", "2h"), + currentTime: timeWithHour(11, time.UTC), + expectedResult: false, + }, + { + name: "Allow-inactive-InvalidDuration", + syncWindow: syncWindow("allow", "* 10 * * *", "2a"), + currentTime: timeWithHour(11, time.UTC), + expectedResult: false, + }, + { + name: "Deny-inactive-InvalidDuration", + syncWindow: syncWindow("deny", "* 10 * * *", "2a"), + currentTime: timeWithHour(11, time.UTC), + expectedResult: false, + }, } for _, tt := range tests { From b40a80892928bb0d7ee24a6eb85cae4e892c68d7 Mon Sep 17 00:00:00 2001 From: daengdaengLee Date: Wed, 9 Oct 2024 15:58:07 +0900 Subject: [PATCH 3/5] feat: change to return error when sync window is invalid Signed-off-by: daengdaengLee --- cmd/argocd/commands/app.go | 11 +- cmd/argocd/commands/projectwindows.go | 3 +- controller/appcontroller.go | 3 +- controller/sync.go | 25 +++- pkg/apis/application/v1alpha1/types.go | 66 ++++----- pkg/apis/application/v1alpha1/types_test.go | 141 +++++++++++++++----- server/application/application.go | 17 ++- server/project/project.go | 5 +- 8 files changed, 189 insertions(+), 82 deletions(-) diff --git a/cmd/argocd/commands/app.go b/cmd/argocd/commands/app.go index 8c32ce34399e4..4c262a67fddca 100644 --- a/cmd/argocd/commands/app.go +++ b/cmd/argocd/commands/app.go @@ -585,8 +585,8 @@ func printAppSummaryTable(app *argoappv1.Application, appURL string, windows *ar var status string var allow, deny, inactiveAllows bool if windows.HasWindows() { - active := windows.Active() - if active.HasWindows() { + active, err := windows.Active() + if err == nil && active.HasWindows() { for _, w := range *active { if w.Kind == "deny" { deny = true @@ -595,13 +595,14 @@ func printAppSummaryTable(app *argoappv1.Application, appURL string, windows *ar } } } - if windows.InactiveAllows().HasWindows() { + inactiveAllowWindows, err := windows.InactiveAllows() + if err == nil && inactiveAllowWindows.HasWindows() { inactiveAllows = true } - s := windows.CanSync(true) if deny || !deny && !allow && inactiveAllows { - if s { + s, err := windows.CanSync(true) + if err == nil && s { status = "Manual Allowed" } else { status = "Sync Denied" diff --git a/cmd/argocd/commands/projectwindows.go b/cmd/argocd/commands/projectwindows.go index d824222306419..b04615e22fd41 100644 --- a/cmd/argocd/commands/projectwindows.go +++ b/cmd/argocd/commands/projectwindows.go @@ -352,9 +352,10 @@ func printSyncWindows(proj *v1alpha1.AppProject) { fmt.Fprintf(w, fmtStr, headers...) if proj.Spec.SyncWindows.HasWindows() { for i, window := range proj.Spec.SyncWindows { + isActive, _ := window.Active() vals := []interface{}{ strconv.Itoa(i), - formatBoolOutput(window.Active()), + formatBoolOutput(isActive), window.Kind, window.Schedule, window.Duration, diff --git a/controller/appcontroller.go b/controller/appcontroller.go index 6db7d2dc3ed23..e7ef632bc478a 100644 --- a/controller/appcontroller.go +++ b/controller/appcontroller.go @@ -1696,7 +1696,8 @@ func (ctrl *ApplicationController) processAppRefreshQueueItem() (processNext boo app.Status.Summary = tree.GetSummary(app) } - if project.Spec.SyncWindows.Matches(app).CanSync(false) { + canSync, _ := project.Spec.SyncWindows.Matches(app).CanSync(false) + if canSync { syncErrCond, opMS := ctrl.autoSync(app, compareResult.syncStatus, compareResult.resources, compareResult.revisionUpdated) setOpMs = opMS if syncErrCond != nil { diff --git a/controller/sync.go b/controller/sync.go index 522155977093e..5f9ce3985d2ab 100644 --- a/controller/sync.go +++ b/controller/sync.go @@ -179,12 +179,18 @@ func (m *appStateManager) SyncAppState(app *v1alpha1.Application, state *v1alpha state.Phase = common.OperationError state.Message = fmt.Sprintf("Failed to load application project: %v", err) return - } else if syncWindowPreventsSync(app, proj) { - // If the operation is currently running, simply let the user know the sync is blocked by a current sync window - if state.Phase == common.OperationRunning { - state.Message = "Sync operation blocked by sync window" + } else { + isBlocked, err := syncWindowPreventsSync(app, proj) + if isBlocked { + // If the operation is currently running, simply let the user know the sync is blocked by a current sync window + if state.Phase == common.OperationRunning { + state.Message = "Sync operation blocked by sync window" + if err != nil { + state.Message = fmt.Sprintf("%s: %v", state.Message, err) + } + } + return } - return } if !isMultiSourceRevision { @@ -579,13 +585,18 @@ func delayBetweenSyncWaves(phase common.SyncPhase, wave int, finalWave bool) err return nil } -func syncWindowPreventsSync(app *v1alpha1.Application, proj *v1alpha1.AppProject) bool { +func syncWindowPreventsSync(app *v1alpha1.Application, proj *v1alpha1.AppProject) (bool, error) { window := proj.Spec.SyncWindows.Matches(app) isManual := false if app.Status.OperationState != nil { isManual = !app.Status.OperationState.Operation.InitiatedBy.Automated } - return !window.CanSync(isManual) + canSync, err := window.CanSync(isManual) + if err != nil { + // prevents sync because sync window has an error + return true, err + } + return !canSync, nil } // deriveServiceAccountToImpersonate determines the service account to be used for impersonation for the sync operation. diff --git a/pkg/apis/application/v1alpha1/types.go b/pkg/apis/application/v1alpha1/types.go index f8f6a09f8a695..3963f0afdf1a0 100644 --- a/pkg/apis/application/v1alpha1/types.go +++ b/pkg/apis/application/v1alpha1/types.go @@ -2386,11 +2386,11 @@ func (s *SyncWindows) HasWindows() bool { } // Active returns a list of sync windows that are currently active -func (s *SyncWindows) Active() *SyncWindows { +func (s *SyncWindows) Active() (*SyncWindows, error) { return s.active(time.Now()) } -func (s *SyncWindows) active(currentTime time.Time) *SyncWindows { +func (s *SyncWindows) active(currentTime time.Time) (*SyncWindows, error) { // If SyncWindows.Active() is called outside of a UTC locale, it should be // first converted to UTC before we scan through the SyncWindows. currentTime = currentTime.In(time.UTC) @@ -2401,13 +2401,11 @@ func (s *SyncWindows) active(currentTime time.Time) *SyncWindows { for _, w := range *s { schedule, sErr := specParser.Parse(w.Schedule) if sErr != nil { - log.Warnf("invalid sync window schedule '%v': %s", w.Schedule, sErr) - continue + return nil, fmt.Errorf("cannot parse schedule '%s': %w", w.Schedule, sErr) } duration, dErr := time.ParseDuration(w.Duration) if dErr != nil { - log.Warnf("invalid sync window duration '%v': %s", w.Duration, dErr) - continue + return nil, fmt.Errorf("cannot parse duration '%s': %w", w.Duration, dErr) } // Offset the nextWindow time to consider the timeZone of the sync window @@ -2418,20 +2416,20 @@ func (s *SyncWindows) active(currentTime time.Time) *SyncWindows { } } if len(active) > 0 { - return &active + return &active, nil } } - return nil + return nil, nil } // InactiveAllows will iterate over the SyncWindows and return all inactive allow windows // for the current time. If the current time is in an inactive allow window, syncs will // be denied. -func (s *SyncWindows) InactiveAllows() *SyncWindows { +func (s *SyncWindows) InactiveAllows() (*SyncWindows, error) { return s.inactiveAllows(time.Now()) } -func (s *SyncWindows) inactiveAllows(currentTime time.Time) *SyncWindows { +func (s *SyncWindows) inactiveAllows(currentTime time.Time) (*SyncWindows, error) { // If SyncWindows.InactiveAllows() is called outside of a UTC locale, it should be // first converted to UTC before we scan through the SyncWindows. currentTime = currentTime.In(time.UTC) @@ -2443,13 +2441,11 @@ func (s *SyncWindows) inactiveAllows(currentTime time.Time) *SyncWindows { if w.Kind == "allow" { schedule, sErr := specParser.Parse(w.Schedule) if sErr != nil { - log.Warnf("invalid sync window schedule '%v': %s", w.Schedule, sErr) - continue + return nil, fmt.Errorf("cannot parse schedule '%s': %w", w.Schedule, sErr) } duration, dErr := time.ParseDuration(w.Duration) if dErr != nil { - log.Warnf("invalid sync window duration '%v': %s", w.Duration, dErr) - continue + return nil, fmt.Errorf("cannot parse duration '%s': %w", w.Duration, dErr) } // Offset the nextWindow time to consider the timeZone of the sync window timeZoneOffsetDuration := w.scheduleOffsetByTimeZone() @@ -2461,10 +2457,10 @@ func (s *SyncWindows) inactiveAllows(currentTime time.Time) *SyncWindows { } } if len(inactive) > 0 { - return &inactive + return &inactive, nil } } - return nil + return nil, nil } func (w *SyncWindow) scheduleOffsetByTimeZone() time.Duration { @@ -2568,36 +2564,42 @@ func (w *SyncWindows) Matches(app *Application) *SyncWindows { } // CanSync returns true if a sync window currently allows a sync. isManual indicates whether the sync has been triggered manually. -func (w *SyncWindows) CanSync(isManual bool) bool { +func (w *SyncWindows) CanSync(isManual bool) (bool, error) { if !w.HasWindows() { - return true + return true, nil } - active := w.Active() + active, err := w.Active() + if err != nil { + return false, fmt.Errorf("invalid sync windows: %w", err) + } hasActiveDeny, manualEnabled := active.hasDeny() if hasActiveDeny { if isManual && manualEnabled { - return true + return true, nil } else { - return false + return false, nil } } if active.hasAllow() { - return true + return true, nil } - inactiveAllows := w.InactiveAllows() + inactiveAllows, err := w.InactiveAllows() + if err != nil { + return false, fmt.Errorf("invalid sync windows: %w", err) + } if inactiveAllows.HasWindows() { if isManual && inactiveAllows.manualEnabled() { - return true + return true, nil } else { - return false + return false, nil } } - return true + return true, nil } // hasDeny will iterate over the SyncWindows and return if a deny window is found and if @@ -2652,11 +2654,11 @@ func (w *SyncWindows) manualEnabled() bool { } // Active returns true if the sync window is currently active -func (w SyncWindow) Active() bool { +func (w SyncWindow) Active() (bool, error) { return w.active(time.Now()) } -func (w SyncWindow) active(currentTime time.Time) bool { +func (w SyncWindow) active(currentTime time.Time) (bool, error) { // If SyncWindow.Active() is called outside of a UTC locale, it should be // first converted to UTC before search currentTime = currentTime.UTC() @@ -2664,20 +2666,18 @@ func (w SyncWindow) active(currentTime time.Time) bool { specParser := cron.NewParser(cron.Minute | cron.Hour | cron.Dom | cron.Month | cron.Dow) schedule, sErr := specParser.Parse(w.Schedule) if sErr != nil { - log.Warnf("invalid sync window schedule '%v': %s", w.Schedule, sErr) - return false + return false, fmt.Errorf("cannot parse schedule '%s': %w", w.Schedule, sErr) } duration, dErr := time.ParseDuration(w.Duration) if dErr != nil { - log.Warnf("invalid sync window duration '%v': %s", w.Duration, dErr) - return false + return false, fmt.Errorf("cannot parse duration '%s': %w", w.Duration, dErr) } // Offset the nextWindow time to consider the timeZone of the sync window timeZoneOffsetDuration := w.scheduleOffsetByTimeZone() nextWindow := schedule.Next(currentTime.Add(timeZoneOffsetDuration - duration)) - return nextWindow.Before(currentTime.Add(timeZoneOffsetDuration)) + return nextWindow.Before(currentTime.Add(timeZoneOffsetDuration)), nil } // Update updates a sync window's settings with the given parameter diff --git a/pkg/apis/application/v1alpha1/types_test.go b/pkg/apis/application/v1alpha1/types_test.go index 3413666f4e9e1..a0c0a0fb22d31 100644 --- a/pkg/apis/application/v1alpha1/types_test.go +++ b/pkg/apis/application/v1alpha1/types_test.go @@ -1792,7 +1792,9 @@ func TestSyncWindows_HasWindows(t *testing.T) { func TestSyncWindows_Active(t *testing.T) { t.Run("WithTestProject", func(t *testing.T) { proj := newTestProjectWithSyncWindows() - assert.Len(t, *proj.Spec.SyncWindows.Active(), 1) + activeWindows, err := proj.Spec.SyncWindows.Active() + assert.Nil(t, err) + assert.Len(t, *activeWindows, 1) }) syncWindow := func(kind string, schedule string, duration string, timeZone string) *SyncWindow { @@ -1819,6 +1821,7 @@ func TestSyncWindows_Active(t *testing.T) { currentTime time.Time matchingIndex int expectedLength int + isErr bool }{ { name: "MatchFirst", @@ -1934,6 +1937,7 @@ func TestSyncWindows_Active(t *testing.T) { }, currentTime: timeWithHour(12, time.UTC), expectedLength: 0, + isErr: true, }, { name: "MatchNone-InvalidDuration", @@ -1943,12 +1947,18 @@ func TestSyncWindows_Active(t *testing.T) { }, currentTime: timeWithHour(12, time.UTC), expectedLength: 0, + isErr: true, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - result := tt.syncWindow.active(tt.currentTime) + result, err := tt.syncWindow.active(tt.currentTime) + if tt.isErr { + assert.NotNil(t, err) + } else { + assert.Nil(t, err) + } if result == nil { result = &SyncWindows{} } @@ -1965,7 +1975,9 @@ func TestSyncWindows_InactiveAllows(t *testing.T) { t.Run("WithTestProject", func(t *testing.T) { proj := newTestProjectWithSyncWindows() proj.Spec.SyncWindows[0].Schedule = "0 0 1 1 1" - assert.Len(t, *proj.Spec.SyncWindows.InactiveAllows(), 1) + inactiveAllowWindows, err := proj.Spec.SyncWindows.InactiveAllows() + assert.Nil(t, err) + assert.Len(t, *inactiveAllowWindows, 1) }) syncWindow := func(kind string, schedule string, duration string, timeZone string) *SyncWindow { @@ -1992,6 +2004,7 @@ func TestSyncWindows_InactiveAllows(t *testing.T) { currentTime time.Time matchingIndex int expectedLength int + isErr bool }{ { name: "MatchFirst", @@ -2124,6 +2137,7 @@ func TestSyncWindows_InactiveAllows(t *testing.T) { }, currentTime: timeWithHour(17, time.UTC), expectedLength: 0, + isErr: true, }, { name: "MatchNone-InvalidDuration", @@ -2132,12 +2146,18 @@ func TestSyncWindows_InactiveAllows(t *testing.T) { }, currentTime: timeWithHour(17, time.UTC), expectedLength: 0, + isErr: true, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - result := tt.syncWindow.inactiveAllows(tt.currentTime) + result, err := tt.syncWindow.inactiveAllows(tt.currentTime) + if tt.isErr { + assert.NotNil(t, err) + } else { + assert.Nil(t, err) + } if result == nil { result = &SyncWindows{} } @@ -2248,9 +2268,10 @@ func TestSyncWindows_CanSync(t *testing.T) { proj := newProjectBuilder().withInactiveDenyWindow(true).build() // when - canSync := proj.Spec.SyncWindows.CanSync(true) + canSync, err := proj.Spec.SyncWindows.CanSync(true) // then + assert.Nil(t, err) assert.True(t, canSync) }) t.Run("will allow manual sync if inactive-deny-window set with manual false", func(t *testing.T) { @@ -2259,9 +2280,10 @@ func TestSyncWindows_CanSync(t *testing.T) { proj := newProjectBuilder().withInactiveDenyWindow(false).build() // when - canSync := proj.Spec.SyncWindows.CanSync(true) + canSync, err := proj.Spec.SyncWindows.CanSync(true) // then + assert.Nil(t, err) assert.True(t, canSync) }) t.Run("will deny manual sync if one inactive-allow-windows set with manual false", func(t *testing.T) { @@ -2273,9 +2295,10 @@ func TestSyncWindows_CanSync(t *testing.T) { build() // when - canSync := proj.Spec.SyncWindows.CanSync(true) + canSync, err := proj.Spec.SyncWindows.CanSync(true) // then + assert.Nil(t, err) assert.False(t, canSync) }) t.Run("will allow manual sync if on active-allow-window set with manual true", func(t *testing.T) { @@ -2286,9 +2309,10 @@ func TestSyncWindows_CanSync(t *testing.T) { build() // when - canSync := proj.Spec.SyncWindows.CanSync(true) + canSync, err := proj.Spec.SyncWindows.CanSync(true) // then + assert.Nil(t, err) assert.True(t, canSync) }) t.Run("will allow manual sync if on active-allow-window set with manual false", func(t *testing.T) { @@ -2299,9 +2323,10 @@ func TestSyncWindows_CanSync(t *testing.T) { build() // when - canSync := proj.Spec.SyncWindows.CanSync(true) + canSync, err := proj.Spec.SyncWindows.CanSync(true) // then + assert.Nil(t, err) assert.True(t, canSync) }) t.Run("will allow auto sync if on active-allow-window", func(t *testing.T) { @@ -2312,9 +2337,10 @@ func TestSyncWindows_CanSync(t *testing.T) { build() // when - canSync := proj.Spec.SyncWindows.CanSync(false) + canSync, err := proj.Spec.SyncWindows.CanSync(false) // then + assert.Nil(t, err) assert.True(t, canSync) }) t.Run("will allow manual sync active-allow and inactive-deny", func(t *testing.T) { @@ -2326,9 +2352,10 @@ func TestSyncWindows_CanSync(t *testing.T) { build() // when - canSync := proj.Spec.SyncWindows.CanSync(true) + canSync, err := proj.Spec.SyncWindows.CanSync(true) // then + assert.Nil(t, err) assert.True(t, canSync) }) t.Run("will allow auto sync active-allow and inactive-deny", func(t *testing.T) { @@ -2340,9 +2367,10 @@ func TestSyncWindows_CanSync(t *testing.T) { build() // when - canSync := proj.Spec.SyncWindows.CanSync(false) + canSync, err := proj.Spec.SyncWindows.CanSync(false) // then + assert.Nil(t, err) assert.True(t, canSync) }) t.Run("will deny manual sync inactive-allow", func(t *testing.T) { @@ -2353,9 +2381,10 @@ func TestSyncWindows_CanSync(t *testing.T) { build() // when - canSync := proj.Spec.SyncWindows.CanSync(true) + canSync, err := proj.Spec.SyncWindows.CanSync(true) // then + assert.Nil(t, err) assert.False(t, canSync) }) t.Run("will deny auto sync inactive-allow", func(t *testing.T) { @@ -2366,9 +2395,10 @@ func TestSyncWindows_CanSync(t *testing.T) { build() // when - canSync := proj.Spec.SyncWindows.CanSync(false) + canSync, err := proj.Spec.SyncWindows.CanSync(false) // then + assert.Nil(t, err) assert.False(t, canSync) }) t.Run("will allow manual sync inactive-allow with ManualSync enabled", func(t *testing.T) { @@ -2379,9 +2409,10 @@ func TestSyncWindows_CanSync(t *testing.T) { build() // when - canSync := proj.Spec.SyncWindows.CanSync(true) + canSync, err := proj.Spec.SyncWindows.CanSync(true) // then + assert.Nil(t, err) assert.True(t, canSync) }) t.Run("will deny auto sync inactive-allow with ManualSync enabled", func(t *testing.T) { @@ -2392,9 +2423,10 @@ func TestSyncWindows_CanSync(t *testing.T) { build() // when - canSync := proj.Spec.SyncWindows.CanSync(false) + canSync, err := proj.Spec.SyncWindows.CanSync(false) // then + assert.Nil(t, err) assert.False(t, canSync) }) t.Run("will deny manual sync with inactive-allow and inactive-deny", func(t *testing.T) { @@ -2406,9 +2438,10 @@ func TestSyncWindows_CanSync(t *testing.T) { build() // when - canSync := proj.Spec.SyncWindows.CanSync(true) + canSync, err := proj.Spec.SyncWindows.CanSync(true) // then + assert.Nil(t, err) assert.False(t, canSync) }) t.Run("will deny auto sync with inactive-allow and inactive-deny", func(t *testing.T) { @@ -2420,9 +2453,10 @@ func TestSyncWindows_CanSync(t *testing.T) { build() // when - canSync := proj.Spec.SyncWindows.CanSync(false) + canSync, err := proj.Spec.SyncWindows.CanSync(false) // then + assert.Nil(t, err) assert.False(t, canSync) }) t.Run("will allow auto sync with active-allow and inactive-allow", func(t *testing.T) { @@ -2434,9 +2468,10 @@ func TestSyncWindows_CanSync(t *testing.T) { build() // when - canSync := proj.Spec.SyncWindows.CanSync(false) + canSync, err := proj.Spec.SyncWindows.CanSync(false) // then + assert.Nil(t, err) assert.True(t, canSync) }) t.Run("will deny manual sync with active-deny", func(t *testing.T) { @@ -2447,9 +2482,10 @@ func TestSyncWindows_CanSync(t *testing.T) { build() // when - canSync := proj.Spec.SyncWindows.CanSync(true) + canSync, err := proj.Spec.SyncWindows.CanSync(true) // then + assert.Nil(t, err) assert.False(t, canSync) }) t.Run("will deny auto sync with active-deny", func(t *testing.T) { @@ -2460,9 +2496,10 @@ func TestSyncWindows_CanSync(t *testing.T) { build() // when - canSync := proj.Spec.SyncWindows.CanSync(false) + canSync, err := proj.Spec.SyncWindows.CanSync(false) // then + assert.Nil(t, err) assert.False(t, canSync) }) t.Run("will allow manual sync with active-deny with ManualSync enabled", func(t *testing.T) { @@ -2473,9 +2510,10 @@ func TestSyncWindows_CanSync(t *testing.T) { build() // when - canSync := proj.Spec.SyncWindows.CanSync(true) + canSync, err := proj.Spec.SyncWindows.CanSync(true) // then + assert.Nil(t, err) assert.True(t, canSync) }) t.Run("will deny auto sync with active-deny with ManualSync enabled", func(t *testing.T) { @@ -2486,9 +2524,10 @@ func TestSyncWindows_CanSync(t *testing.T) { build() // when - canSync := proj.Spec.SyncWindows.CanSync(false) + canSync, err := proj.Spec.SyncWindows.CanSync(false) // then + assert.Nil(t, err) assert.False(t, canSync) }) t.Run("will deny manual sync with many active-deny having one with ManualSync disabled", func(t *testing.T) { @@ -2502,9 +2541,10 @@ func TestSyncWindows_CanSync(t *testing.T) { build() // when - canSync := proj.Spec.SyncWindows.CanSync(true) + canSync, err := proj.Spec.SyncWindows.CanSync(true) // then + assert.Nil(t, err) assert.False(t, canSync) }) t.Run("will deny auto sync with many active-deny having one with ManualSync disabled", func(t *testing.T) { @@ -2518,9 +2558,10 @@ func TestSyncWindows_CanSync(t *testing.T) { build() // when - canSync := proj.Spec.SyncWindows.CanSync(false) + canSync, err := proj.Spec.SyncWindows.CanSync(false) // then + assert.Nil(t, err) assert.False(t, canSync) }) t.Run("will deny manual sync with active-deny and active-allow windows with ManualSync disabled", func(t *testing.T) { @@ -2532,9 +2573,10 @@ func TestSyncWindows_CanSync(t *testing.T) { build() // when - canSync := proj.Spec.SyncWindows.CanSync(true) + canSync, err := proj.Spec.SyncWindows.CanSync(true) // then + assert.Nil(t, err) assert.False(t, canSync) }) t.Run("will allow manual sync with active-deny and active-allow windows with ManualSync enabled", func(t *testing.T) { @@ -2546,9 +2588,10 @@ func TestSyncWindows_CanSync(t *testing.T) { build() // when - canSync := proj.Spec.SyncWindows.CanSync(true) + canSync, err := proj.Spec.SyncWindows.CanSync(true) // then + assert.Nil(t, err) assert.True(t, canSync) }) t.Run("will deny auto sync with active-deny and active-allow windows with ManualSync enabled", func(t *testing.T) { @@ -2560,9 +2603,24 @@ func TestSyncWindows_CanSync(t *testing.T) { build() // when - canSync := proj.Spec.SyncWindows.CanSync(false) + canSync, err := proj.Spec.SyncWindows.CanSync(false) + + // then + assert.Nil(t, err) + assert.False(t, canSync) + }) + t.Run("will deny and return error with invalid windows", func(t *testing.T) { + // given + t.Parallel() + proj := newProjectBuilder(). + withInvalidWindows(). + build() + + // when + canSync, err := proj.Spec.SyncWindows.CanSync(false) // then + assert.NotNil(t, err) assert.False(t, canSync) }) } @@ -2612,8 +2670,9 @@ func TestSyncWindows_hasAllow(t *testing.T) { func TestSyncWindow_Active(t *testing.T) { window := &SyncWindow{Schedule: "* * * * *", Duration: "1h"} t.Run("ActiveWindow", func(t *testing.T) { - window.Active() - assert.True(t, window.Active()) + isActive, err := window.Active() + assert.Nil(t, err) + assert.True(t, isActive) }) syncWindow := func(kind string, schedule string, duration string) SyncWindow { @@ -2638,6 +2697,7 @@ func TestSyncWindow_Active(t *testing.T) { syncWindow SyncWindow currentTime time.Time expectedResult bool + isErr bool }{ { name: "Allow-active", @@ -2692,30 +2752,39 @@ func TestSyncWindow_Active(t *testing.T) { syncWindow: syncWindow("allow", "* 10 * * 7", "2h"), currentTime: timeWithHour(11, time.UTC), expectedResult: false, + isErr: true, }, { name: "Deny-inactive-InvalidSchedule", syncWindow: syncWindow("deny", "* 10 * * 7", "2h"), currentTime: timeWithHour(11, time.UTC), expectedResult: false, + isErr: true, }, { name: "Allow-inactive-InvalidDuration", syncWindow: syncWindow("allow", "* 10 * * *", "2a"), currentTime: timeWithHour(11, time.UTC), expectedResult: false, + isErr: true, }, { name: "Deny-inactive-InvalidDuration", syncWindow: syncWindow("deny", "* 10 * * *", "2a"), currentTime: timeWithHour(11, time.UTC), expectedResult: false, + isErr: true, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - result := tt.syncWindow.active(tt.currentTime) + result, err := tt.syncWindow.active(tt.currentTime) + if tt.isErr { + assert.NotNil(t, err) + } else { + assert.Nil(t, err) + } assert.Equal(t, tt.expectedResult, result) }) } @@ -2827,6 +2896,16 @@ func (b *projectBuilder) withInactiveDenyWindow(allowManual bool) *projectBuilde return b } +func (b *projectBuilder) withInvalidWindows() *projectBuilder { + b.proj.Spec.SyncWindows = append(b.proj.Spec.SyncWindows, + newSyncWindow("allow", "* 10 * * 7", false), + newSyncWindow("deny", "* 10 * * 7", false), + newSyncWindow("allow", "* 10 * * 7", true), + newSyncWindow("deny", "* 10 * * 7", true), + ) + return b +} + func inactiveCronSchedule() string { hourPlus10, _, _ := time.Now().Add(10 * time.Hour).Clock() return fmt.Sprintf("0 %d * * *", hourPlus10) diff --git a/server/application/application.go b/server/application/application.go index dbb6a85bd2d50..055bdca54f2b9 100644 --- a/server/application/application.go +++ b/server/application/application.go @@ -1898,7 +1898,11 @@ func (s *Server) Sync(ctx context.Context, syncReq *application.ApplicationSyncR s.inferResourcesStatusHealth(a) - if !proj.Spec.SyncWindows.Matches(a).CanSync(true) { + canSync, err := proj.Spec.SyncWindows.Matches(a).CanSync(true) + if err != nil { + return a, status.Errorf(codes.PermissionDenied, "cannot sync: invalid sync window: %v", err) + } + if !canSync { return a, status.Errorf(codes.PermissionDenied, "cannot sync: blocked by sync window") } @@ -2615,10 +2619,17 @@ func (s *Server) GetApplicationSyncWindows(ctx context.Context, q *application.A } windows := proj.Spec.SyncWindows.Matches(a) - sync := windows.CanSync(true) + sync, err := windows.CanSync(true) + if err != nil { + return nil, fmt.Errorf("invalid sync windows: %w", err) + } + activeWindows, err := windows.Active() + if err != nil { + return nil, fmt.Errorf("invalid sync windows: %w", err) + } res := &application.ApplicationSyncWindowsResponse{ - ActiveWindows: convertSyncWindows(windows.Active()), + ActiveWindows: convertSyncWindows(activeWindows), AssignedWindows: convertSyncWindows(windows), CanSync: &sync, } diff --git a/server/project/project.go b/server/project/project.go index 02d393564ddf0..62487b268a705 100644 --- a/server/project/project.go +++ b/server/project/project.go @@ -525,7 +525,10 @@ func (s *Server) GetSyncWindowsState(ctx context.Context, q *project.SyncWindows res := &project.SyncWindowsResponse{} - windows := proj.Spec.SyncWindows.Active() + windows, err := proj.Spec.SyncWindows.Active() + if err != nil { + return nil, err + } if windows.HasWindows() { res.Windows = *windows } else { From db0d18d06563080ec6012eae7d073789796e9c60 Mon Sep 17 00:00:00 2001 From: daengdaengLee Date: Wed, 9 Oct 2024 16:06:51 +0900 Subject: [PATCH 4/5] fix: use assert.Error or assert.NoError Signed-off-by: daengdaengLee --- pkg/apis/application/v1alpha1/types_test.go | 68 ++++++++++----------- 1 file changed, 34 insertions(+), 34 deletions(-) diff --git a/pkg/apis/application/v1alpha1/types_test.go b/pkg/apis/application/v1alpha1/types_test.go index a0c0a0fb22d31..482793ce88a92 100644 --- a/pkg/apis/application/v1alpha1/types_test.go +++ b/pkg/apis/application/v1alpha1/types_test.go @@ -1793,7 +1793,7 @@ func TestSyncWindows_Active(t *testing.T) { t.Run("WithTestProject", func(t *testing.T) { proj := newTestProjectWithSyncWindows() activeWindows, err := proj.Spec.SyncWindows.Active() - assert.Nil(t, err) + assert.NoError(t, err) assert.Len(t, *activeWindows, 1) }) @@ -1955,9 +1955,9 @@ func TestSyncWindows_Active(t *testing.T) { t.Run(tt.name, func(t *testing.T) { result, err := tt.syncWindow.active(tt.currentTime) if tt.isErr { - assert.NotNil(t, err) + assert.Error(t, err) } else { - assert.Nil(t, err) + assert.NoError(t, err) } if result == nil { result = &SyncWindows{} @@ -1976,7 +1976,7 @@ func TestSyncWindows_InactiveAllows(t *testing.T) { proj := newTestProjectWithSyncWindows() proj.Spec.SyncWindows[0].Schedule = "0 0 1 1 1" inactiveAllowWindows, err := proj.Spec.SyncWindows.InactiveAllows() - assert.Nil(t, err) + assert.NoError(t, err) assert.Len(t, *inactiveAllowWindows, 1) }) @@ -2154,9 +2154,9 @@ func TestSyncWindows_InactiveAllows(t *testing.T) { t.Run(tt.name, func(t *testing.T) { result, err := tt.syncWindow.inactiveAllows(tt.currentTime) if tt.isErr { - assert.NotNil(t, err) + assert.Error(t, err) } else { - assert.Nil(t, err) + assert.NoError(t, err) } if result == nil { result = &SyncWindows{} @@ -2271,7 +2271,7 @@ func TestSyncWindows_CanSync(t *testing.T) { canSync, err := proj.Spec.SyncWindows.CanSync(true) // then - assert.Nil(t, err) + assert.NoError(t, err) assert.True(t, canSync) }) t.Run("will allow manual sync if inactive-deny-window set with manual false", func(t *testing.T) { @@ -2283,7 +2283,7 @@ func TestSyncWindows_CanSync(t *testing.T) { canSync, err := proj.Spec.SyncWindows.CanSync(true) // then - assert.Nil(t, err) + assert.NoError(t, err) assert.True(t, canSync) }) t.Run("will deny manual sync if one inactive-allow-windows set with manual false", func(t *testing.T) { @@ -2298,7 +2298,7 @@ func TestSyncWindows_CanSync(t *testing.T) { canSync, err := proj.Spec.SyncWindows.CanSync(true) // then - assert.Nil(t, err) + assert.NoError(t, err) assert.False(t, canSync) }) t.Run("will allow manual sync if on active-allow-window set with manual true", func(t *testing.T) { @@ -2312,7 +2312,7 @@ func TestSyncWindows_CanSync(t *testing.T) { canSync, err := proj.Spec.SyncWindows.CanSync(true) // then - assert.Nil(t, err) + assert.NoError(t, err) assert.True(t, canSync) }) t.Run("will allow manual sync if on active-allow-window set with manual false", func(t *testing.T) { @@ -2326,7 +2326,7 @@ func TestSyncWindows_CanSync(t *testing.T) { canSync, err := proj.Spec.SyncWindows.CanSync(true) // then - assert.Nil(t, err) + assert.NoError(t, err) assert.True(t, canSync) }) t.Run("will allow auto sync if on active-allow-window", func(t *testing.T) { @@ -2340,7 +2340,7 @@ func TestSyncWindows_CanSync(t *testing.T) { canSync, err := proj.Spec.SyncWindows.CanSync(false) // then - assert.Nil(t, err) + assert.NoError(t, err) assert.True(t, canSync) }) t.Run("will allow manual sync active-allow and inactive-deny", func(t *testing.T) { @@ -2355,7 +2355,7 @@ func TestSyncWindows_CanSync(t *testing.T) { canSync, err := proj.Spec.SyncWindows.CanSync(true) // then - assert.Nil(t, err) + assert.NoError(t, err) assert.True(t, canSync) }) t.Run("will allow auto sync active-allow and inactive-deny", func(t *testing.T) { @@ -2370,7 +2370,7 @@ func TestSyncWindows_CanSync(t *testing.T) { canSync, err := proj.Spec.SyncWindows.CanSync(false) // then - assert.Nil(t, err) + assert.NoError(t, err) assert.True(t, canSync) }) t.Run("will deny manual sync inactive-allow", func(t *testing.T) { @@ -2384,7 +2384,7 @@ func TestSyncWindows_CanSync(t *testing.T) { canSync, err := proj.Spec.SyncWindows.CanSync(true) // then - assert.Nil(t, err) + assert.NoError(t, err) assert.False(t, canSync) }) t.Run("will deny auto sync inactive-allow", func(t *testing.T) { @@ -2398,7 +2398,7 @@ func TestSyncWindows_CanSync(t *testing.T) { canSync, err := proj.Spec.SyncWindows.CanSync(false) // then - assert.Nil(t, err) + assert.NoError(t, err) assert.False(t, canSync) }) t.Run("will allow manual sync inactive-allow with ManualSync enabled", func(t *testing.T) { @@ -2412,7 +2412,7 @@ func TestSyncWindows_CanSync(t *testing.T) { canSync, err := proj.Spec.SyncWindows.CanSync(true) // then - assert.Nil(t, err) + assert.NoError(t, err) assert.True(t, canSync) }) t.Run("will deny auto sync inactive-allow with ManualSync enabled", func(t *testing.T) { @@ -2426,7 +2426,7 @@ func TestSyncWindows_CanSync(t *testing.T) { canSync, err := proj.Spec.SyncWindows.CanSync(false) // then - assert.Nil(t, err) + assert.NoError(t, err) assert.False(t, canSync) }) t.Run("will deny manual sync with inactive-allow and inactive-deny", func(t *testing.T) { @@ -2441,7 +2441,7 @@ func TestSyncWindows_CanSync(t *testing.T) { canSync, err := proj.Spec.SyncWindows.CanSync(true) // then - assert.Nil(t, err) + assert.NoError(t, err) assert.False(t, canSync) }) t.Run("will deny auto sync with inactive-allow and inactive-deny", func(t *testing.T) { @@ -2456,7 +2456,7 @@ func TestSyncWindows_CanSync(t *testing.T) { canSync, err := proj.Spec.SyncWindows.CanSync(false) // then - assert.Nil(t, err) + assert.NoError(t, err) assert.False(t, canSync) }) t.Run("will allow auto sync with active-allow and inactive-allow", func(t *testing.T) { @@ -2471,7 +2471,7 @@ func TestSyncWindows_CanSync(t *testing.T) { canSync, err := proj.Spec.SyncWindows.CanSync(false) // then - assert.Nil(t, err) + assert.NoError(t, err) assert.True(t, canSync) }) t.Run("will deny manual sync with active-deny", func(t *testing.T) { @@ -2485,7 +2485,7 @@ func TestSyncWindows_CanSync(t *testing.T) { canSync, err := proj.Spec.SyncWindows.CanSync(true) // then - assert.Nil(t, err) + assert.NoError(t, err) assert.False(t, canSync) }) t.Run("will deny auto sync with active-deny", func(t *testing.T) { @@ -2499,7 +2499,7 @@ func TestSyncWindows_CanSync(t *testing.T) { canSync, err := proj.Spec.SyncWindows.CanSync(false) // then - assert.Nil(t, err) + assert.NoError(t, err) assert.False(t, canSync) }) t.Run("will allow manual sync with active-deny with ManualSync enabled", func(t *testing.T) { @@ -2513,7 +2513,7 @@ func TestSyncWindows_CanSync(t *testing.T) { canSync, err := proj.Spec.SyncWindows.CanSync(true) // then - assert.Nil(t, err) + assert.NoError(t, err) assert.True(t, canSync) }) t.Run("will deny auto sync with active-deny with ManualSync enabled", func(t *testing.T) { @@ -2527,7 +2527,7 @@ func TestSyncWindows_CanSync(t *testing.T) { canSync, err := proj.Spec.SyncWindows.CanSync(false) // then - assert.Nil(t, err) + assert.NoError(t, err) assert.False(t, canSync) }) t.Run("will deny manual sync with many active-deny having one with ManualSync disabled", func(t *testing.T) { @@ -2544,7 +2544,7 @@ func TestSyncWindows_CanSync(t *testing.T) { canSync, err := proj.Spec.SyncWindows.CanSync(true) // then - assert.Nil(t, err) + assert.NoError(t, err) assert.False(t, canSync) }) t.Run("will deny auto sync with many active-deny having one with ManualSync disabled", func(t *testing.T) { @@ -2561,7 +2561,7 @@ func TestSyncWindows_CanSync(t *testing.T) { canSync, err := proj.Spec.SyncWindows.CanSync(false) // then - assert.Nil(t, err) + assert.NoError(t, err) assert.False(t, canSync) }) t.Run("will deny manual sync with active-deny and active-allow windows with ManualSync disabled", func(t *testing.T) { @@ -2576,7 +2576,7 @@ func TestSyncWindows_CanSync(t *testing.T) { canSync, err := proj.Spec.SyncWindows.CanSync(true) // then - assert.Nil(t, err) + assert.NoError(t, err) assert.False(t, canSync) }) t.Run("will allow manual sync with active-deny and active-allow windows with ManualSync enabled", func(t *testing.T) { @@ -2591,7 +2591,7 @@ func TestSyncWindows_CanSync(t *testing.T) { canSync, err := proj.Spec.SyncWindows.CanSync(true) // then - assert.Nil(t, err) + assert.NoError(t, err) assert.True(t, canSync) }) t.Run("will deny auto sync with active-deny and active-allow windows with ManualSync enabled", func(t *testing.T) { @@ -2606,7 +2606,7 @@ func TestSyncWindows_CanSync(t *testing.T) { canSync, err := proj.Spec.SyncWindows.CanSync(false) // then - assert.Nil(t, err) + assert.NoError(t, err) assert.False(t, canSync) }) t.Run("will deny and return error with invalid windows", func(t *testing.T) { @@ -2620,7 +2620,7 @@ func TestSyncWindows_CanSync(t *testing.T) { canSync, err := proj.Spec.SyncWindows.CanSync(false) // then - assert.NotNil(t, err) + assert.Error(t, err) assert.False(t, canSync) }) } @@ -2671,7 +2671,7 @@ func TestSyncWindow_Active(t *testing.T) { window := &SyncWindow{Schedule: "* * * * *", Duration: "1h"} t.Run("ActiveWindow", func(t *testing.T) { isActive, err := window.Active() - assert.Nil(t, err) + assert.NoError(t, err) assert.True(t, isActive) }) @@ -2781,9 +2781,9 @@ func TestSyncWindow_Active(t *testing.T) { t.Run(tt.name, func(t *testing.T) { result, err := tt.syncWindow.active(tt.currentTime) if tt.isErr { - assert.NotNil(t, err) + assert.Error(t, err) } else { - assert.Nil(t, err) + assert.NoError(t, err) } assert.Equal(t, tt.expectedResult, result) }) From bf49c83d8202e2e3c91d5b269ec2882b7c3a31ab Mon Sep 17 00:00:00 2001 From: daengdaengLee Date: Wed, 9 Oct 2024 16:19:14 +0900 Subject: [PATCH 5/5] fix: use require instead of assert Signed-off-by: daengdaengLee --- pkg/apis/application/v1alpha1/types_test.go | 68 ++++++++++----------- 1 file changed, 34 insertions(+), 34 deletions(-) diff --git a/pkg/apis/application/v1alpha1/types_test.go b/pkg/apis/application/v1alpha1/types_test.go index 482793ce88a92..1df2f5c74baea 100644 --- a/pkg/apis/application/v1alpha1/types_test.go +++ b/pkg/apis/application/v1alpha1/types_test.go @@ -1793,7 +1793,7 @@ func TestSyncWindows_Active(t *testing.T) { t.Run("WithTestProject", func(t *testing.T) { proj := newTestProjectWithSyncWindows() activeWindows, err := proj.Spec.SyncWindows.Active() - assert.NoError(t, err) + require.NoError(t, err) assert.Len(t, *activeWindows, 1) }) @@ -1955,9 +1955,9 @@ func TestSyncWindows_Active(t *testing.T) { t.Run(tt.name, func(t *testing.T) { result, err := tt.syncWindow.active(tt.currentTime) if tt.isErr { - assert.Error(t, err) + require.Error(t, err) } else { - assert.NoError(t, err) + require.NoError(t, err) } if result == nil { result = &SyncWindows{} @@ -1976,7 +1976,7 @@ func TestSyncWindows_InactiveAllows(t *testing.T) { proj := newTestProjectWithSyncWindows() proj.Spec.SyncWindows[0].Schedule = "0 0 1 1 1" inactiveAllowWindows, err := proj.Spec.SyncWindows.InactiveAllows() - assert.NoError(t, err) + require.NoError(t, err) assert.Len(t, *inactiveAllowWindows, 1) }) @@ -2154,9 +2154,9 @@ func TestSyncWindows_InactiveAllows(t *testing.T) { t.Run(tt.name, func(t *testing.T) { result, err := tt.syncWindow.inactiveAllows(tt.currentTime) if tt.isErr { - assert.Error(t, err) + require.Error(t, err) } else { - assert.NoError(t, err) + require.NoError(t, err) } if result == nil { result = &SyncWindows{} @@ -2271,7 +2271,7 @@ func TestSyncWindows_CanSync(t *testing.T) { canSync, err := proj.Spec.SyncWindows.CanSync(true) // then - assert.NoError(t, err) + require.NoError(t, err) assert.True(t, canSync) }) t.Run("will allow manual sync if inactive-deny-window set with manual false", func(t *testing.T) { @@ -2283,7 +2283,7 @@ func TestSyncWindows_CanSync(t *testing.T) { canSync, err := proj.Spec.SyncWindows.CanSync(true) // then - assert.NoError(t, err) + require.NoError(t, err) assert.True(t, canSync) }) t.Run("will deny manual sync if one inactive-allow-windows set with manual false", func(t *testing.T) { @@ -2298,7 +2298,7 @@ func TestSyncWindows_CanSync(t *testing.T) { canSync, err := proj.Spec.SyncWindows.CanSync(true) // then - assert.NoError(t, err) + require.NoError(t, err) assert.False(t, canSync) }) t.Run("will allow manual sync if on active-allow-window set with manual true", func(t *testing.T) { @@ -2312,7 +2312,7 @@ func TestSyncWindows_CanSync(t *testing.T) { canSync, err := proj.Spec.SyncWindows.CanSync(true) // then - assert.NoError(t, err) + require.NoError(t, err) assert.True(t, canSync) }) t.Run("will allow manual sync if on active-allow-window set with manual false", func(t *testing.T) { @@ -2326,7 +2326,7 @@ func TestSyncWindows_CanSync(t *testing.T) { canSync, err := proj.Spec.SyncWindows.CanSync(true) // then - assert.NoError(t, err) + require.NoError(t, err) assert.True(t, canSync) }) t.Run("will allow auto sync if on active-allow-window", func(t *testing.T) { @@ -2340,7 +2340,7 @@ func TestSyncWindows_CanSync(t *testing.T) { canSync, err := proj.Spec.SyncWindows.CanSync(false) // then - assert.NoError(t, err) + require.NoError(t, err) assert.True(t, canSync) }) t.Run("will allow manual sync active-allow and inactive-deny", func(t *testing.T) { @@ -2355,7 +2355,7 @@ func TestSyncWindows_CanSync(t *testing.T) { canSync, err := proj.Spec.SyncWindows.CanSync(true) // then - assert.NoError(t, err) + require.NoError(t, err) assert.True(t, canSync) }) t.Run("will allow auto sync active-allow and inactive-deny", func(t *testing.T) { @@ -2370,7 +2370,7 @@ func TestSyncWindows_CanSync(t *testing.T) { canSync, err := proj.Spec.SyncWindows.CanSync(false) // then - assert.NoError(t, err) + require.NoError(t, err) assert.True(t, canSync) }) t.Run("will deny manual sync inactive-allow", func(t *testing.T) { @@ -2384,7 +2384,7 @@ func TestSyncWindows_CanSync(t *testing.T) { canSync, err := proj.Spec.SyncWindows.CanSync(true) // then - assert.NoError(t, err) + require.NoError(t, err) assert.False(t, canSync) }) t.Run("will deny auto sync inactive-allow", func(t *testing.T) { @@ -2398,7 +2398,7 @@ func TestSyncWindows_CanSync(t *testing.T) { canSync, err := proj.Spec.SyncWindows.CanSync(false) // then - assert.NoError(t, err) + require.NoError(t, err) assert.False(t, canSync) }) t.Run("will allow manual sync inactive-allow with ManualSync enabled", func(t *testing.T) { @@ -2412,7 +2412,7 @@ func TestSyncWindows_CanSync(t *testing.T) { canSync, err := proj.Spec.SyncWindows.CanSync(true) // then - assert.NoError(t, err) + require.NoError(t, err) assert.True(t, canSync) }) t.Run("will deny auto sync inactive-allow with ManualSync enabled", func(t *testing.T) { @@ -2426,7 +2426,7 @@ func TestSyncWindows_CanSync(t *testing.T) { canSync, err := proj.Spec.SyncWindows.CanSync(false) // then - assert.NoError(t, err) + require.NoError(t, err) assert.False(t, canSync) }) t.Run("will deny manual sync with inactive-allow and inactive-deny", func(t *testing.T) { @@ -2441,7 +2441,7 @@ func TestSyncWindows_CanSync(t *testing.T) { canSync, err := proj.Spec.SyncWindows.CanSync(true) // then - assert.NoError(t, err) + require.NoError(t, err) assert.False(t, canSync) }) t.Run("will deny auto sync with inactive-allow and inactive-deny", func(t *testing.T) { @@ -2456,7 +2456,7 @@ func TestSyncWindows_CanSync(t *testing.T) { canSync, err := proj.Spec.SyncWindows.CanSync(false) // then - assert.NoError(t, err) + require.NoError(t, err) assert.False(t, canSync) }) t.Run("will allow auto sync with active-allow and inactive-allow", func(t *testing.T) { @@ -2471,7 +2471,7 @@ func TestSyncWindows_CanSync(t *testing.T) { canSync, err := proj.Spec.SyncWindows.CanSync(false) // then - assert.NoError(t, err) + require.NoError(t, err) assert.True(t, canSync) }) t.Run("will deny manual sync with active-deny", func(t *testing.T) { @@ -2485,7 +2485,7 @@ func TestSyncWindows_CanSync(t *testing.T) { canSync, err := proj.Spec.SyncWindows.CanSync(true) // then - assert.NoError(t, err) + require.NoError(t, err) assert.False(t, canSync) }) t.Run("will deny auto sync with active-deny", func(t *testing.T) { @@ -2499,7 +2499,7 @@ func TestSyncWindows_CanSync(t *testing.T) { canSync, err := proj.Spec.SyncWindows.CanSync(false) // then - assert.NoError(t, err) + require.NoError(t, err) assert.False(t, canSync) }) t.Run("will allow manual sync with active-deny with ManualSync enabled", func(t *testing.T) { @@ -2513,7 +2513,7 @@ func TestSyncWindows_CanSync(t *testing.T) { canSync, err := proj.Spec.SyncWindows.CanSync(true) // then - assert.NoError(t, err) + require.NoError(t, err) assert.True(t, canSync) }) t.Run("will deny auto sync with active-deny with ManualSync enabled", func(t *testing.T) { @@ -2527,7 +2527,7 @@ func TestSyncWindows_CanSync(t *testing.T) { canSync, err := proj.Spec.SyncWindows.CanSync(false) // then - assert.NoError(t, err) + require.NoError(t, err) assert.False(t, canSync) }) t.Run("will deny manual sync with many active-deny having one with ManualSync disabled", func(t *testing.T) { @@ -2544,7 +2544,7 @@ func TestSyncWindows_CanSync(t *testing.T) { canSync, err := proj.Spec.SyncWindows.CanSync(true) // then - assert.NoError(t, err) + require.NoError(t, err) assert.False(t, canSync) }) t.Run("will deny auto sync with many active-deny having one with ManualSync disabled", func(t *testing.T) { @@ -2561,7 +2561,7 @@ func TestSyncWindows_CanSync(t *testing.T) { canSync, err := proj.Spec.SyncWindows.CanSync(false) // then - assert.NoError(t, err) + require.NoError(t, err) assert.False(t, canSync) }) t.Run("will deny manual sync with active-deny and active-allow windows with ManualSync disabled", func(t *testing.T) { @@ -2576,7 +2576,7 @@ func TestSyncWindows_CanSync(t *testing.T) { canSync, err := proj.Spec.SyncWindows.CanSync(true) // then - assert.NoError(t, err) + require.NoError(t, err) assert.False(t, canSync) }) t.Run("will allow manual sync with active-deny and active-allow windows with ManualSync enabled", func(t *testing.T) { @@ -2591,7 +2591,7 @@ func TestSyncWindows_CanSync(t *testing.T) { canSync, err := proj.Spec.SyncWindows.CanSync(true) // then - assert.NoError(t, err) + require.NoError(t, err) assert.True(t, canSync) }) t.Run("will deny auto sync with active-deny and active-allow windows with ManualSync enabled", func(t *testing.T) { @@ -2606,7 +2606,7 @@ func TestSyncWindows_CanSync(t *testing.T) { canSync, err := proj.Spec.SyncWindows.CanSync(false) // then - assert.NoError(t, err) + require.NoError(t, err) assert.False(t, canSync) }) t.Run("will deny and return error with invalid windows", func(t *testing.T) { @@ -2620,7 +2620,7 @@ func TestSyncWindows_CanSync(t *testing.T) { canSync, err := proj.Spec.SyncWindows.CanSync(false) // then - assert.Error(t, err) + require.Error(t, err) assert.False(t, canSync) }) } @@ -2671,7 +2671,7 @@ func TestSyncWindow_Active(t *testing.T) { window := &SyncWindow{Schedule: "* * * * *", Duration: "1h"} t.Run("ActiveWindow", func(t *testing.T) { isActive, err := window.Active() - assert.NoError(t, err) + require.NoError(t, err) assert.True(t, isActive) }) @@ -2781,9 +2781,9 @@ func TestSyncWindow_Active(t *testing.T) { t.Run(tt.name, func(t *testing.T) { result, err := tt.syncWindow.active(tt.currentTime) if tt.isErr { - assert.Error(t, err) + require.Error(t, err) } else { - assert.NoError(t, err) + require.NoError(t, err) } assert.Equal(t, tt.expectedResult, result) })