diff --git a/pkg/controller/backup_controller_test.go b/pkg/controller/backup_controller_test.go index 438af4c09..fbab58ce9 100644 --- a/pkg/controller/backup_controller_test.go +++ b/pkg/controller/backup_controller_test.go @@ -156,6 +156,7 @@ func TestProcessBackup(t *testing.T) { sharedInformers = informers.NewSharedInformerFactory(client, 0) logger = arktest.NewLogger() pluginManager = &MockManager{} + clockTime, _ = time.Parse("Mon Jan 2 15:04:05 2006", "Mon Jan 2 15:04:05 2006") ) c := NewBackupController( @@ -169,7 +170,8 @@ func TestProcessBackup(t *testing.T) { pluginManager, NewBackupTracker(), ).(*backupController) - c.clock = clock.NewFakeClock(time.Now()) + + c.clock = clock.NewFakeClock(clockTime) var expiration time.Time @@ -248,40 +250,43 @@ func TestProcessBackup(t *testing.T) { actions := client.Actions() require.Equal(t, 2, len(actions)) - // validate Patch call 1 (setting version, expiration, and phase) - patchAction, ok := actions[0].(core.PatchAction) - require.True(t, ok, "action is not a PatchAction") - - patch := make(map[string]interface{}) - require.NoError(t, json.Unmarshal(patchAction.GetPatch(), &patch), "cannot unmarshal patch") - - // should have status - assert.Equal(t, 1, len(patch), "patch has wrong number of keys") - - expectedStatusKeys := 2 - if test.backup.Spec.TTL.Duration > 0 { - assert.True(t, collections.HasKeyAndVal(patch, "status.expiration", expiration.UTC().Format(time.RFC3339)), "patch's status.expiration does not match") - expectedStatusKeys = 3 + // structs and func for decoding patch content + type StatusPatch struct { + Expiration time.Time `json:"expiration"` + Version int `json:"version"` + Phase v1.BackupPhase `json:"phase"` } - assert.True(t, collections.HasKeyAndVal(patch, "status.version", float64(1))) - assert.True(t, collections.HasKeyAndVal(patch, "status.phase", string(v1.BackupPhaseInProgress)), "patch's status.phase does not match") + type Patch struct { + Status StatusPatch `json:"status"` + } - res, _ := collections.GetMap(patch, "status") - assert.Equal(t, expectedStatusKeys, len(res), "patch's status has the wrong number of keys") + decode := func(decoder *json.Decoder) (interface{}, error) { + actual := new(Patch) + err := decoder.Decode(actual) + + return *actual, err + } + + // validate Patch call 1 (setting version, expiration, and phase) + expected := Patch{ + Status: StatusPatch{ + Version: 1, + Phase: v1.BackupPhaseInProgress, + Expiration: expiration, + }, + } + + arktest.ValidatePatch(t, actions[0], expected, decode) // validate Patch call 2 (setting phase) - patchAction, ok = actions[1].(core.PatchAction) - require.True(t, ok, "action is not a PatchAction") + expected = Patch{ + Status: StatusPatch{ + Phase: v1.BackupPhaseCompleted, + }, + } - patch = make(map[string]interface{}) - require.NoError(t, json.Unmarshal(patchAction.GetPatch(), &patch), "cannot unmarshal patch") - - assert.Equal(t, 1, len(patch), "patch has wrong number of keys") - - res, _ = collections.GetMap(patch, "status") - assert.Equal(t, 1, len(res), "patch's status has the wrong number of keys") - assert.True(t, collections.HasKeyAndVal(patch, "status.phase", string(v1.BackupPhaseCompleted)), "patch's status.phase does not match") + arktest.ValidatePatch(t, actions[1], expected, decode) }) } } diff --git a/pkg/controller/download_request_controller_test.go b/pkg/controller/download_request_controller_test.go index 737d0baff..f1165b479 100644 --- a/pkg/controller/download_request_controller_test.go +++ b/pkg/controller/download_request_controller_test.go @@ -25,12 +25,11 @@ import ( "github.com/stretchr/testify/require" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - core "k8s.io/client-go/testing" + "k8s.io/apimachinery/pkg/util/clock" "github.com/heptio/ark/pkg/apis/ark/v1" "github.com/heptio/ark/pkg/generated/clientset/versioned/fake" informers "github.com/heptio/ark/pkg/generated/informers/externalversions" - "github.com/heptio/ark/pkg/util/collections" arktest "github.com/heptio/ark/pkg/util/test" ) @@ -109,6 +108,7 @@ func TestProcessDownloadRequest(t *testing.T) { restoresInformer = sharedInformers.Ark().V1().Restores() backupService = &arktest.BackupService{} logger = arktest.NewLogger() + clockTime, _ = time.Parse("Mon Jan 2 15:04:05 2006", "Mon Jan 2 15:04:05 2006") ) defer backupService.AssertExpectations(t) @@ -121,6 +121,8 @@ func TestProcessDownloadRequest(t *testing.T) { logger, ).(*downloadRequestController) + c.clock = clock.NewFakeClock(clockTime) + var downloadRequest *v1.DownloadRequest if tc.expectedPhase == v1.DownloadRequestPhaseProcessed { @@ -168,26 +170,33 @@ func TestProcessDownloadRequest(t *testing.T) { // otherwise, we should get exactly 1 patch require.Equal(t, 1, len(actions)) - patchAction, ok := actions[0].(core.PatchAction) - require.True(t, ok, "action is not a PatchAction") - patch := make(map[string]interface{}) - require.NoError(t, json.Unmarshal(patchAction.GetPatch(), &patch), "cannot unmarshal patch") + type PatchStatus struct { + DownloadURL string `json:"downloadURL"` + Phase v1.DownloadRequestPhase `json:"phase"` + Expiration time.Time `json:"expiration"` + } - // check the URL - assert.True(t, collections.HasKeyAndVal(patch, "status.downloadURL", tc.expectedURL), "patch's status.downloadURL does not match") + type Patch struct { + Status PatchStatus `json:"status"` + } - // check the Phase - assert.True(t, collections.HasKeyAndVal(patch, "status.phase", string(tc.expectedPhase)), "patch's status.phase does not match") + decode := func(decoder *json.Decoder) (interface{}, error) { + actual := new(Patch) + err := decoder.Decode(actual) - // check that Expiration exists - // TODO pass a fake clock to the controller and verify - // the expiration value - assert.True(t, collections.Exists(patch, "status.expiration"), "patch's status.expiration does not exist") + return *actual, err + } - // we expect 3 total updates. - res, _ := collections.GetMap(patch, "status") - assert.Equal(t, 3, len(res), "patch's status has the wrong number of keys") + expected := Patch{ + Status: PatchStatus{ + DownloadURL: tc.expectedURL, + Phase: tc.expectedPhase, + Expiration: clockTime.Add(signedURLTTL), + }, + } + + arktest.ValidatePatch(t, actions[0], expected, decode) }) } } diff --git a/pkg/controller/restore_controller_test.go b/pkg/controller/restore_controller_test.go index 97c253c56..781feea48 100644 --- a/pkg/controller/restore_controller_test.go +++ b/pkg/controller/restore_controller_test.go @@ -181,7 +181,7 @@ func TestProcessRestore(t *testing.T) { { name: "restore with non-existent backup name fails", - restore: arktest.NewTestRestore("foo", "bar", api.RestorePhaseNew).WithBackup("backup-1").WithIncludedNamespace("ns-1").Restore, + restore: NewRestore("foo", "bar", "backup-1", "ns-1", "*", api.RestorePhaseNew).Restore, expectedErr: false, expectedPhase: string(api.RestorePhaseFailedValidation), expectedValidationErrors: []string{"Error retrieving backup: no backup here"}, @@ -371,35 +371,35 @@ func TestProcessRestore(t *testing.T) { return } + // structs and func for decoding patch content + type StatusPatch struct { + Phase api.RestorePhase `json:"phase"` + ValidationErrors []string `json:"validationErrors"` + Errors int `json:"errors"` + } + + type Patch struct { + Status StatusPatch `json:"status"` + } + + decode := func(decoder *json.Decoder) (interface{}, error) { + actual := new(Patch) + err := decoder.Decode(actual) + + return *actual, err + } + // validate Patch call 1 (setting phase, validation errs) require.True(t, len(actions) > 0, "len(actions) is too small") - patchAction, ok := actions[0].(core.PatchAction) - require.True(t, ok, "action is not a PatchAction") - - patch := make(map[string]interface{}) - require.NoError(t, json.Unmarshal(patchAction.GetPatch(), &patch), "cannot unmarshal patch") - - expectedStatusKeys := 1 - - assert.True(t, collections.HasKeyAndVal(patch, "status.phase", test.expectedPhase), "patch's status.phase does not match") - - if len(test.expectedValidationErrors) > 0 { - errs, err := collections.GetSlice(patch, "status.validationErrors") - require.NoError(t, err, "error getting patch's status.validationErrors") - - var errStrings []string - for _, err := range errs { - errStrings = append(errStrings, err.(string)) - } - - assert.Equal(t, test.expectedValidationErrors, errStrings, "patch's status.validationErrors does not match") - - expectedStatusKeys++ + expected := Patch{ + Status: StatusPatch{ + Phase: api.RestorePhase(test.expectedPhase), + ValidationErrors: test.expectedValidationErrors, + }, } - res, _ := collections.GetMap(patch, "status") - assert.Equal(t, expectedStatusKeys, len(res), "patch's status has the wrong number of keys") + arktest.ValidatePatch(t, actions[0], expected, decode) // if we don't expect a restore, validate it wasn't called and exit the test if test.expectedRestorerCall == nil { @@ -410,24 +410,15 @@ func TestProcessRestore(t *testing.T) { assert.Equal(t, 1, len(restorer.Calls)) // validate Patch call 2 (setting phase) - patchAction, ok = actions[1].(core.PatchAction) - require.True(t, ok, "action is not a PatchAction") - require.NoError(t, json.Unmarshal(patchAction.GetPatch(), &patch), "cannot unmarshal patch") - - assert.Equal(t, 1, len(patch), "patch has wrong number of keys") - - res, _ = collections.GetMap(patch, "status") - expectedStatusKeys = 1 - - assert.True(t, collections.HasKeyAndVal(patch, "status.phase", string(api.RestorePhaseCompleted)), "patch's status.phase does not match") - - if test.expectedRestoreErrors != 0 { - assert.True(t, collections.HasKeyAndVal(patch, "status.errors", float64(test.expectedRestoreErrors)), "patch's status.errors does not match") - expectedStatusKeys++ + expected = Patch{ + Status: StatusPatch{ + Phase: api.RestorePhaseCompleted, + Errors: test.expectedRestoreErrors, + }, } - assert.Equal(t, expectedStatusKeys, len(res), "patch's status has wrong number of keys") + arktest.ValidatePatch(t, actions[1], expected, decode) // explicitly capturing the argument passed to Restore myself because // I want to validate the called arg as of the time of calling, but @@ -449,7 +440,7 @@ func NewRestore(ns, name, backup, includeNS, includeResource string, phase api.R } for _, n := range nonRestorableResources { - restore.WithExcludedResource(n) + restore = restore.WithExcludedResource(n) } return restore diff --git a/pkg/controller/schedule_controller_test.go b/pkg/controller/schedule_controller_test.go index cdd90f398..07917661d 100644 --- a/pkg/controller/schedule_controller_test.go +++ b/pkg/controller/schedule_controller_test.go @@ -18,7 +18,6 @@ package controller import ( "encoding/json" - "fmt" "testing" "time" @@ -41,15 +40,15 @@ import ( func TestProcessSchedule(t *testing.T) { tests := []struct { - name string - scheduleKey string - schedule *api.Schedule - fakeClockTime string - expectedErr bool - expectedPhase string - expectedValidationError string - expectedBackupCreate *api.Backup - expectedLastBackup string + name string + scheduleKey string + schedule *api.Schedule + fakeClockTime string + expectedErr bool + expectedPhase string + expectedValidationErrors []string + expectedBackupCreate *api.Backup + expectedLastBackup string }{ { name: "invalid key returns error", @@ -67,25 +66,25 @@ func TestProcessSchedule(t *testing.T) { expectedErr: false, }, { - name: "schedule with phase New gets validated and failed if invalid", - schedule: arktest.NewTestSchedule("ns", "name").WithPhase(api.SchedulePhaseNew).Schedule, - expectedErr: false, - expectedPhase: string(api.SchedulePhaseFailedValidation), - expectedValidationError: "Schedule must be a non-empty valid Cron expression", + name: "schedule with phase New gets validated and failed if invalid", + schedule: arktest.NewTestSchedule("ns", "name").WithPhase(api.SchedulePhaseNew).Schedule, + expectedErr: false, + expectedPhase: string(api.SchedulePhaseFailedValidation), + expectedValidationErrors: []string{"Schedule must be a non-empty valid Cron expression"}, }, { - name: "schedule with phase gets validated and failed if invalid", - schedule: arktest.NewTestSchedule("ns", "name").Schedule, - expectedErr: false, - expectedPhase: string(api.SchedulePhaseFailedValidation), - expectedValidationError: "Schedule must be a non-empty valid Cron expression", + name: "schedule with phase gets validated and failed if invalid", + schedule: arktest.NewTestSchedule("ns", "name").Schedule, + expectedErr: false, + expectedPhase: string(api.SchedulePhaseFailedValidation), + expectedValidationErrors: []string{"Schedule must be a non-empty valid Cron expression"}, }, { - name: "schedule with phase Enabled gets re-validated and failed if invalid", - schedule: arktest.NewTestSchedule("ns", "name").WithPhase(api.SchedulePhaseEnabled).Schedule, - expectedErr: false, - expectedPhase: string(api.SchedulePhaseFailedValidation), - expectedValidationError: "Schedule must be a non-empty valid Cron expression", + name: "schedule with phase Enabled gets re-validated and failed if invalid", + schedule: arktest.NewTestSchedule("ns", "name").WithPhase(api.SchedulePhaseEnabled).Schedule, + expectedErr: false, + expectedPhase: string(api.SchedulePhaseFailedValidation), + expectedValidationErrors: []string{"Schedule must be a non-empty valid Cron expression"}, }, { name: "schedule with phase New gets validated and triggers a backup", @@ -191,34 +190,34 @@ func TestProcessSchedule(t *testing.T) { actions := client.Actions() index := 0 + type PatchStatus struct { + ValidationErrors []string `json:"validationErrors"` + Phase api.SchedulePhase `json:"phase"` + LastBackup time.Time `json:"lastBackup"` + } + + type Patch struct { + Status PatchStatus `json:"status"` + } + + decode := func(decoder *json.Decoder) (interface{}, error) { + actual := new(Patch) + err := decoder.Decode(actual) + + return *actual, err + } + if test.expectedPhase != "" { require.True(t, len(actions) > index, "len(actions) is too small") - patchAction, ok := actions[index].(core.PatchAction) - require.True(t, ok, "action is not a PatchAction") - - patch := make(map[string]interface{}) - require.NoError(t, json.Unmarshal(patchAction.GetPatch(), &patch), "cannot unmarshal patch") - - assert.Equal(t, 1, len(patch), "patch has wrong number of keys") - - expectedStatusKeys := 1 - - assert.True(t, collections.HasKeyAndVal(patch, "status.phase", test.expectedPhase), "patch's status.phase does not match") - - if test.expectedValidationError != "" { - errs, err := collections.GetSlice(patch, "status.validationErrors") - require.NoError(t, err, "error getting patch's status.validationErrors") - - require.Equal(t, 1, len(errs)) - - assert.Equal(t, test.expectedValidationError, errs[0].(string), "patch's status.validationErrors does not match") - - expectedStatusKeys++ + expected := Patch{ + Status: PatchStatus{ + ValidationErrors: test.expectedValidationErrors, + Phase: api.SchedulePhase(test.expectedPhase), + }, } - res, _ := collections.GetMap(patch, "status") - assert.Equal(t, expectedStatusKeys, len(res), "patch's status has the wrong number of keys") + arktest.ValidatePatch(t, actions[index], expected, decode) index++ } @@ -239,27 +238,13 @@ func TestProcessSchedule(t *testing.T) { if test.expectedLastBackup != "" { require.True(t, len(actions) > index, "len(actions) is too small") - patchAction, ok := actions[index].(core.PatchAction) - require.True(t, ok, "action is not a PatchAction") + expected := Patch{ + Status: PatchStatus{ + LastBackup: parseTime(test.expectedLastBackup), + }, + } - patch := make(map[string]interface{}) - require.NoError(t, json.Unmarshal(patchAction.GetPatch(), &patch), "cannot unmarshal patch") - - assert.Equal(t, 1, len(patch), "patch has wrong number of keys") - - lastBackup, _ := collections.GetValue(patch, "status.lastBackup") - fmt.Println(lastBackup) - - assert.True( - t, - collections.HasKeyAndVal(patch, "status.lastBackup", parseTime(test.expectedLastBackup).UTC().Format(time.RFC3339)), - "patch's status.lastBackup does not match", - ) - - res, _ := collections.GetMap(patch, "status") - assert.Equal(t, 1, len(res), "patch's status has the wrong number of keys") - - index++ + arktest.ValidatePatch(t, actions[index], expected, decode) } }) } diff --git a/pkg/util/collections/map_utils.go b/pkg/util/collections/map_utils.go index 8bfea989c..5dfdaa9b4 100644 --- a/pkg/util/collections/map_utils.go +++ b/pkg/util/collections/map_utils.go @@ -122,14 +122,3 @@ func Exists(root map[string]interface{}, path string) bool { _, err := GetValue(root, path) return err == nil } - -// HasKeyAndVal returns true if root[path] exists and the value -// contained is equal to val, or false otherwise. -func HasKeyAndVal(root map[string]interface{}, path string, val interface{}) bool { - valObj, err := GetValue(root, path) - if err != nil { - return false - } - - return valObj == val -} diff --git a/pkg/util/test/comparisons.go b/pkg/util/test/comparisons.go index f5844f571..3c485119b 100644 --- a/pkg/util/test/comparisons.go +++ b/pkg/util/test/comparisons.go @@ -17,10 +17,13 @@ limitations under the License. package test import ( + "bytes" + "encoding/json" "reflect" "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" core "k8s.io/client-go/testing" ) @@ -59,3 +62,20 @@ func CompareActions(t *testing.T, expected, actual []core.Action) { } } } + +// ValidatePatch tests the validity of an action. It checks +// that the action is a PatchAction, that the patch decodes from JSON +// with the provided decode func and has no extraneous fields, and that +// the decoded patch matches the expected. +func ValidatePatch(t *testing.T, action core.Action, expected interface{}, decodeFunc func(*json.Decoder) (interface{}, error)) { + patchAction, ok := action.(core.PatchAction) + require.True(t, ok, "action is not a PatchAction") + + decoder := json.NewDecoder(bytes.NewReader(patchAction.GetPatch())) + decoder.DisallowUnknownFields() + + actual, err := decodeFunc(decoder) + require.NoError(t, err) + + assert.Equal(t, expected, actual) +}