From b98b95890a1523ca86f85ee763ded80a83b92d06 Mon Sep 17 00:00:00 2001 From: petruki <31597636+petruki@users.noreply.github.com> Date: Fri, 13 Sep 2024 17:39:10 -0700 Subject: [PATCH 1/2] Fixed account update and other minor issues with syncing --- .env.test | 2 +- src/controller/account.go | 12 ++ src/controller/account_test.go | 17 ++- src/controller/controller_test.go | 8 +- src/core/comparator.go | 12 ++ src/core/comparator_test.go | 229 ++++++++++++++++-------------- src/core/handler.go | 42 +++--- src/core/handler_test.go | 37 ++++- src/repository/account.go | 46 +++--- 9 files changed, 252 insertions(+), 153 deletions(-) diff --git a/.env.test b/.env.test index 21b3e70..feb5e1a 100644 --- a/.env.test +++ b/.env.test @@ -1,5 +1,5 @@ PORT=8000 -LOG_LEVEL=INFO +LOG_LEVEL=DEBUG MONGO_URI=mongodb://localhost:27017 MONGO_DB=switcher-gitops-test GIT_TOKEN_PRIVATE_KEY=SecretSecretSecretSecretSecretSe diff --git a/src/controller/account.go b/src/controller/account.go index 00b9864..0c5ae3e 100644 --- a/src/controller/account.go +++ b/src/controller/account.go @@ -5,6 +5,7 @@ import ( "net/http" "github.com/gorilla/mux" + "github.com/switcherapi/switcher-gitops/src/config" "github.com/switcherapi/switcher-gitops/src/core" "github.com/switcherapi/switcher-gitops/src/model" "github.com/switcherapi/switcher-gitops/src/repository" @@ -48,6 +49,12 @@ func (controller *AccountController) CreateAccountHandler(w http.ResponseWriter, return } + // Encrypt token before saving + if accountRequest.Token != "" { + println("Encrypting token", accountRequest.Token) + accountRequest.Token = utils.Encrypt(accountRequest.Token, config.GetEnv("GIT_TOKEN_PRIVATE_KEY")) + } + accountCreated, err := controller.AccountRepository.Create(&accountRequest) if err != nil { utils.Log(utils.LogLevelError, "Error creating account: %s", err.Error()) @@ -83,6 +90,11 @@ func (controller *AccountController) UpdateAccountHandler(w http.ResponseWriter, return } + // Encrypt token before saving + if accountRequest.Token != "" { + accountRequest.Token = utils.Encrypt(accountRequest.Token, config.GetEnv("GIT_TOKEN_PRIVATE_KEY")) + } + accountUpdated, err := controller.AccountRepository.Update(&accountRequest) if err != nil { utils.Log(utils.LogLevelError, "Error updating account: %s", err.Error()) diff --git a/src/controller/account_test.go b/src/controller/account_test.go index 46260c9..65e6167 100644 --- a/src/controller/account_test.go +++ b/src/controller/account_test.go @@ -98,8 +98,10 @@ func TestUpdateAccountHandler(t *testing.T) { accountController.CreateAccountHandler(givenAccountRequest(accountV1)) // Test - payload, _ := json.Marshal(accountV2) - req, _ := http.NewRequest(http.MethodPut, accountController.RouteAccountPath+"/"+accountV2.Domain.ID, bytes.NewBuffer(payload)) + accountV2Copy := accountV2 + accountV2Copy.Domain.Message = "Updated successfully" + payload, _ := json.Marshal(accountV2Copy) + req, _ := http.NewRequest(http.MethodPut, accountController.RouteAccountPath+"/"+accountV2Copy.Domain.ID, bytes.NewBuffer(payload)) response := executeRequest(req) // Assert @@ -108,11 +110,11 @@ func TestUpdateAccountHandler(t *testing.T) { assert.Equal(t, http.StatusOK, response.Code) assert.Nil(t, err) - assert.Equal(t, accountV1.Repository, accountResponse.Repository) assert.NotEmpty(t, accountResponse.Token) - assert.NotEqual(t, accountV1.Branch, accountResponse.Branch) - assert.NotEqual(t, accountV1.Settings.Window, accountResponse.Settings.Window) - assert.NotEqual(t, accountV1.Settings.Active, accountResponse.Settings.Active) + assert.Equal(t, accountV1.Repository, accountResponse.Repository) + assert.Equal(t, model.StatusSynced, accountResponse.Domain.Status) + assert.Equal(t, "Updated successfully", accountResponse.Domain.Message) + assert.Equal(t, "5m", accountResponse.Settings.Window) }) t.Run("Should update account token only", func(t *testing.T) { @@ -137,6 +139,9 @@ func TestUpdateAccountHandler(t *testing.T) { encryptedToken := utils.Encrypt(accountV1.Token, config.GetEnv("GIT_TOKEN_PRIVATE_KEY")) assert.NotEqual(t, encryptedToken, accountResponse.Token) + + decryptedToken, _ := utils.Decrypt(accountResponse.Token, config.GetEnv("GIT_TOKEN_PRIVATE_KEY")) + assert.Equal(t, "new-token", decryptedToken) }) t.Run("Should not update an account - invalid request", func(t *testing.T) { diff --git a/src/controller/controller_test.go b/src/controller/controller_test.go index 383e2d1..cb4aad8 100644 --- a/src/controller/controller_test.go +++ b/src/controller/controller_test.go @@ -68,6 +68,7 @@ var accountV1 = model.Account{ Name: "Switcher GitOps", Version: 123, LastCommit: "123", + LastDate: "2021-01-01", Status: model.StatusSynced, Message: "Synced successfully", }, @@ -82,11 +83,8 @@ var accountV2 = model.Account{ Repository: "switcherapi/switcher-gitops", Branch: "main", Domain: model.DomainDetails{ - ID: "123-controller-test", - Name: "Switcher GitOps", - Version: 123, - LastCommit: "123", - Status: model.StatusSynced, + ID: "123-controller-test", + Name: "Switcher GitOps", }, Settings: model.Settings{ Active: false, diff --git a/src/core/comparator.go b/src/core/comparator.go index 0c8257c..a5e54bf 100644 --- a/src/core/comparator.go +++ b/src/core/comparator.go @@ -14,6 +14,7 @@ type IComparatorService interface { CheckSnapshotDiff(left model.Snapshot, right model.Snapshot, diffType DiffType) model.DiffResult MergeResults(diffResults []model.DiffResult) model.DiffResult NewSnapshotFromJson(jsonData []byte) model.Snapshot + RemoveDeleted(diffResult model.DiffResult) model.DiffResult } type ComparatorService struct{} @@ -55,6 +56,17 @@ func (c *ComparatorService) MergeResults(diffResults []model.DiffResult) model.D return result } +func (c *ComparatorService) RemoveDeleted(diffResult model.DiffResult) model.DiffResult { + diff := model.DiffResult{Changes: []model.DiffDetails{}} + for _, change := range diffResult.Changes { + if change.Action != string(DELETED) { + diff.Changes = append(diff.Changes, change) + } + } + + return diff +} + func checkGroupDiff(left model.Snapshot, right model.Snapshot, diffType DiffType, diffResult model.DiffResult) model.DiffResult { for _, leftGroup := range left.Domain.Group { if !slices.Contains(model.GroupNames(right.Domain.Group), leftGroup.Name) { diff --git a/src/core/comparator_test.go b/src/core/comparator_test.go index caaab69..1471d8b 100644 --- a/src/core/comparator_test.go +++ b/src/core/comparator_test.go @@ -15,15 +15,15 @@ func TestCheckGroupSnapshot(t *testing.T) { t.Run("Should return changes in group", func(t *testing.T) { // Given - jsonLeft := utils.ReadJsonFromFile(DEFAULT_JSON) - jsonRight := utils.ReadJsonFromFile("../../resources/fixtures/comparator/changed_group.json") - snapshotLeft := c.NewSnapshotFromJson([]byte(jsonLeft)) - snapshotRight := c.NewSnapshotFromJson([]byte(jsonRight)) + jsonApi := utils.ReadJsonFromFile(DEFAULT_JSON) + jsonRepo := utils.ReadJsonFromFile("../../resources/fixtures/comparator/changed_group.json") + fromApi := c.NewSnapshotFromJson([]byte(jsonApi)) + fromRepo := c.NewSnapshotFromJson([]byte(jsonRepo)) // Test Check/Merge changes - diffChanged := c.CheckSnapshotDiff(snapshotLeft, snapshotRight, CHANGED) - diffNew := c.CheckSnapshotDiff(snapshotRight, snapshotLeft, NEW) - diffDeleted := c.CheckSnapshotDiff(snapshotLeft, snapshotRight, DELETED) + diffChanged := c.CheckSnapshotDiff(fromApi, fromRepo, CHANGED) + diffNew := c.CheckSnapshotDiff(fromRepo, fromApi, NEW) + diffDeleted := c.CheckSnapshotDiff(fromApi, fromRepo, DELETED) actual := c.MergeResults([]model.DiffResult{diffChanged, diffNew, diffDeleted}) assert.NotNil(t, actual) @@ -45,15 +45,15 @@ func TestCheckGroupSnapshot(t *testing.T) { t.Run("Should return new group", func(t *testing.T) { // Given - jsonLeft := utils.ReadJsonFromFile(DEFAULT_JSON) - jsonRight := utils.ReadJsonFromFile("../../resources/fixtures/comparator/new_group.json") - snapshotLeft := c.NewSnapshotFromJson([]byte(jsonLeft)) - snapshotRight := c.NewSnapshotFromJson([]byte(jsonRight)) + jsonApi := utils.ReadJsonFromFile(DEFAULT_JSON) + jsonRepo := utils.ReadJsonFromFile("../../resources/fixtures/comparator/new_group.json") + fromApi := c.NewSnapshotFromJson([]byte(jsonApi)) + fromRepo := c.NewSnapshotFromJson([]byte(jsonRepo)) // Test Check/Merge changes - diffChanged := c.CheckSnapshotDiff(snapshotLeft, snapshotRight, CHANGED) - diffNew := c.CheckSnapshotDiff(snapshotRight, snapshotLeft, NEW) - diffDeleted := c.CheckSnapshotDiff(snapshotLeft, snapshotRight, DELETED) + diffChanged := c.CheckSnapshotDiff(fromApi, fromRepo, CHANGED) + diffNew := c.CheckSnapshotDiff(fromRepo, fromApi, NEW) + diffDeleted := c.CheckSnapshotDiff(fromApi, fromRepo, DELETED) actual := c.MergeResults([]model.DiffResult{diffChanged, diffNew, diffDeleted}) assert.NotNil(t, actual) @@ -83,15 +83,15 @@ func TestCheckGroupSnapshot(t *testing.T) { t.Run("Should return deleted group", func(t *testing.T) { // Given - jsonLeft := utils.ReadJsonFromFile(DEFAULT_JSON) - jsonRight := utils.ReadJsonFromFile("../../resources/fixtures/comparator/deleted_group.json") - snapshotLeft := c.NewSnapshotFromJson([]byte(jsonLeft)) - snapshotRight := c.NewSnapshotFromJson([]byte(jsonRight)) + jsonApi := utils.ReadJsonFromFile(DEFAULT_JSON) + jsonRepo := utils.ReadJsonFromFile("../../resources/fixtures/comparator/deleted_group.json") + fromApi := c.NewSnapshotFromJson([]byte(jsonApi)) + fromRepo := c.NewSnapshotFromJson([]byte(jsonRepo)) // Test Check/Merge changes - diffChanged := c.CheckSnapshotDiff(snapshotLeft, snapshotRight, CHANGED) - diffNew := c.CheckSnapshotDiff(snapshotRight, snapshotLeft, NEW) - diffDeleted := c.CheckSnapshotDiff(snapshotLeft, snapshotRight, DELETED) + diffChanged := c.CheckSnapshotDiff(fromApi, fromRepo, CHANGED) + diffNew := c.CheckSnapshotDiff(fromRepo, fromApi, NEW) + diffDeleted := c.CheckSnapshotDiff(fromApi, fromRepo, DELETED) actual := c.MergeResults([]model.DiffResult{diffChanged, diffNew, diffDeleted}) assert.NotNil(t, actual) @@ -108,17 +108,36 @@ func TestCheckGroupSnapshot(t *testing.T) { ]}`, utils.ToJsonFromObject(actual)) }) + t.Run("Should not return deleted group when removing from diff", func(t *testing.T) { + // Given + jsonApi := utils.ReadJsonFromFile(DEFAULT_JSON) + jsonRepo := utils.ReadJsonFromFile("../../resources/fixtures/comparator/deleted_group.json") + fromApi := c.NewSnapshotFromJson([]byte(jsonApi)) + fromRepo := c.NewSnapshotFromJson([]byte(jsonRepo)) + + diffChanged := c.CheckSnapshotDiff(fromApi, fromRepo, CHANGED) + diffNew := c.CheckSnapshotDiff(fromRepo, fromApi, NEW) + diffDeleted := c.CheckSnapshotDiff(fromApi, fromRepo, DELETED) + actual := c.MergeResults([]model.DiffResult{diffChanged, diffNew, diffDeleted}) + + // Test Remove deleted + actual = c.RemoveDeleted(actual) + + assert.NotNil(t, actual) + assert.JSONEq(t, `{"changes": []}`, utils.ToJsonFromObject(actual)) + }) + t.Run("Should return new group from empty group", func(t *testing.T) { // Given - jsonLeft := utils.ReadJsonFromFile("../../resources/fixtures/comparator/default_empty.json") - jsonRight := utils.ReadJsonFromFile(DEFAULT_JSON) - snapshotLeft := c.NewSnapshotFromJson([]byte(jsonLeft)) - snapshotRight := c.NewSnapshotFromJson([]byte(jsonRight)) + jsonApi := utils.ReadJsonFromFile("../../resources/fixtures/comparator/default_empty.json") + jsonRepo := utils.ReadJsonFromFile(DEFAULT_JSON) + fromApi := c.NewSnapshotFromJson([]byte(jsonApi)) + fromRepo := c.NewSnapshotFromJson([]byte(jsonRepo)) // Test Check/Merge changes - diffChanged := c.CheckSnapshotDiff(snapshotLeft, snapshotRight, CHANGED) - diffNew := c.CheckSnapshotDiff(snapshotRight, snapshotLeft, NEW) - diffDeleted := c.CheckSnapshotDiff(snapshotLeft, snapshotRight, DELETED) + diffChanged := c.CheckSnapshotDiff(fromApi, fromRepo, CHANGED) + diffNew := c.CheckSnapshotDiff(fromRepo, fromApi, NEW) + diffDeleted := c.CheckSnapshotDiff(fromApi, fromRepo, DELETED) actual := c.MergeResults([]model.DiffResult{diffChanged, diffNew, diffDeleted}) assert.NotNil(t, actual) @@ -128,15 +147,15 @@ func TestCheckGroupSnapshot(t *testing.T) { t.Run("Should return new group from empty config", func(t *testing.T) { // Given - jsonLeft := utils.ReadJsonFromFile("../../resources/fixtures/comparator/default_empty_config.json") - jsonRight := utils.ReadJsonFromFile(DEFAULT_JSON) - snapshotLeft := c.NewSnapshotFromJson([]byte(jsonLeft)) - snapshotRight := c.NewSnapshotFromJson([]byte(jsonRight)) + jsonApi := utils.ReadJsonFromFile("../../resources/fixtures/comparator/default_empty_config.json") + jsonRepo := utils.ReadJsonFromFile(DEFAULT_JSON) + fromApi := c.NewSnapshotFromJson([]byte(jsonApi)) + fromRepo := c.NewSnapshotFromJson([]byte(jsonRepo)) // Test Check/Merge changes - diffChanged := c.CheckSnapshotDiff(snapshotLeft, snapshotRight, CHANGED) - diffNew := c.CheckSnapshotDiff(snapshotRight, snapshotLeft, NEW) - diffDeleted := c.CheckSnapshotDiff(snapshotLeft, snapshotRight, DELETED) + diffChanged := c.CheckSnapshotDiff(fromApi, fromRepo, CHANGED) + diffNew := c.CheckSnapshotDiff(fromRepo, fromApi, NEW) + diffDeleted := c.CheckSnapshotDiff(fromApi, fromRepo, DELETED) actual := c.MergeResults([]model.DiffResult{diffChanged, diffNew, diffDeleted}) assert.NotNil(t, actual) @@ -150,15 +169,15 @@ func TestCheckConfigSnapshot(t *testing.T) { t.Run("Should return changes in config", func(t *testing.T) { // Given - jsonLeft := utils.ReadJsonFromFile(DEFAULT_JSON) - jsonRight := utils.ReadJsonFromFile("../../resources/fixtures/comparator/changed_config.json") - snapshotLeft := c.NewSnapshotFromJson([]byte(jsonLeft)) - snapshotRight := c.NewSnapshotFromJson([]byte(jsonRight)) + jsonApi := utils.ReadJsonFromFile(DEFAULT_JSON) + jsonRepo := utils.ReadJsonFromFile("../../resources/fixtures/comparator/changed_config.json") + fromApi := c.NewSnapshotFromJson([]byte(jsonApi)) + fromRepo := c.NewSnapshotFromJson([]byte(jsonRepo)) // Test Check/Merge changes - diffChanged := c.CheckSnapshotDiff(snapshotLeft, snapshotRight, CHANGED) - diffNew := c.CheckSnapshotDiff(snapshotRight, snapshotLeft, NEW) - diffDeleted := c.CheckSnapshotDiff(snapshotLeft, snapshotRight, DELETED) + diffChanged := c.CheckSnapshotDiff(fromApi, fromRepo, CHANGED) + diffNew := c.CheckSnapshotDiff(fromRepo, fromApi, NEW) + diffDeleted := c.CheckSnapshotDiff(fromApi, fromRepo, DELETED) actual := c.MergeResults([]model.DiffResult{diffChanged, diffNew, diffDeleted}) assert.NotNil(t, actual) @@ -181,15 +200,15 @@ func TestCheckConfigSnapshot(t *testing.T) { t.Run("Should return new config", func(t *testing.T) { // Given - jsonLeft := utils.ReadJsonFromFile(DEFAULT_JSON) - jsonRight := utils.ReadJsonFromFile("../../resources/fixtures/comparator/new_config.json") - snapshotLeft := c.NewSnapshotFromJson([]byte(jsonLeft)) - snapshotRight := c.NewSnapshotFromJson([]byte(jsonRight)) + jsonApi := utils.ReadJsonFromFile(DEFAULT_JSON) + jsonRepo := utils.ReadJsonFromFile("../../resources/fixtures/comparator/new_config.json") + fromApi := c.NewSnapshotFromJson([]byte(jsonApi)) + fromRepo := c.NewSnapshotFromJson([]byte(jsonRepo)) // Test Check/Merge changes - diffChanged := c.CheckSnapshotDiff(snapshotLeft, snapshotRight, CHANGED) - diffNew := c.CheckSnapshotDiff(snapshotRight, snapshotLeft, NEW) - diffDeleted := c.CheckSnapshotDiff(snapshotLeft, snapshotRight, DELETED) + diffChanged := c.CheckSnapshotDiff(fromApi, fromRepo, CHANGED) + diffNew := c.CheckSnapshotDiff(fromRepo, fromApi, NEW) + diffDeleted := c.CheckSnapshotDiff(fromApi, fromRepo, DELETED) actual := c.MergeResults([]model.DiffResult{diffChanged, diffNew, diffDeleted}) assert.NotNil(t, actual) @@ -214,15 +233,15 @@ func TestCheckConfigSnapshot(t *testing.T) { t.Run("Should return deleted config", func(t *testing.T) { // Given - jsonLeft := utils.ReadJsonFromFile(DEFAULT_JSON) - jsonRight := utils.ReadJsonFromFile("../../resources/fixtures/comparator/deleted_config.json") - snapshotLeft := c.NewSnapshotFromJson([]byte(jsonLeft)) - snapshotRight := c.NewSnapshotFromJson([]byte(jsonRight)) + jsonApi := utils.ReadJsonFromFile(DEFAULT_JSON) + jsonRepo := utils.ReadJsonFromFile("../../resources/fixtures/comparator/deleted_config.json") + fromApi := c.NewSnapshotFromJson([]byte(jsonApi)) + fromRepo := c.NewSnapshotFromJson([]byte(jsonRepo)) // Test Check/Merge changes - diffChanged := c.CheckSnapshotDiff(snapshotLeft, snapshotRight, CHANGED) - diffNew := c.CheckSnapshotDiff(snapshotRight, snapshotLeft, NEW) - diffDeleted := c.CheckSnapshotDiff(snapshotLeft, snapshotRight, DELETED) + diffChanged := c.CheckSnapshotDiff(fromApi, fromRepo, CHANGED) + diffNew := c.CheckSnapshotDiff(fromRepo, fromApi, NEW) + diffDeleted := c.CheckSnapshotDiff(fromApi, fromRepo, DELETED) actual := c.MergeResults([]model.DiffResult{diffChanged, diffNew, diffDeleted}) assert.NotNil(t, actual) @@ -246,15 +265,15 @@ func TestCheckStrategySnapshot(t *testing.T) { t.Run("Should return changes in strategy", func(t *testing.T) { // Given - jsonLeft := utils.ReadJsonFromFile(DEFAULT_JSON) - jsonRight := utils.ReadJsonFromFile("../../resources/fixtures/comparator/changed_strategy.json") - snapshotLeft := c.NewSnapshotFromJson([]byte(jsonLeft)) - snapshotRight := c.NewSnapshotFromJson([]byte(jsonRight)) + jsonApi := utils.ReadJsonFromFile(DEFAULT_JSON) + jsonRepo := utils.ReadJsonFromFile("../../resources/fixtures/comparator/changed_strategy.json") + fromApi := c.NewSnapshotFromJson([]byte(jsonApi)) + fromRepo := c.NewSnapshotFromJson([]byte(jsonRepo)) // Test Check/Merge changes - diffChanged := c.CheckSnapshotDiff(snapshotLeft, snapshotRight, CHANGED) - diffNew := c.CheckSnapshotDiff(snapshotRight, snapshotLeft, NEW) - diffDeleted := c.CheckSnapshotDiff(snapshotLeft, snapshotRight, DELETED) + diffChanged := c.CheckSnapshotDiff(fromApi, fromRepo, CHANGED) + diffNew := c.CheckSnapshotDiff(fromRepo, fromApi, NEW) + diffDeleted := c.CheckSnapshotDiff(fromApi, fromRepo, DELETED) actual := c.MergeResults([]model.DiffResult{diffChanged, diffNew, diffDeleted}) assert.NotNil(t, actual) @@ -277,15 +296,15 @@ func TestCheckStrategySnapshot(t *testing.T) { t.Run("Should return new strategy", func(t *testing.T) { // Given - jsonLeft := utils.ReadJsonFromFile(DEFAULT_JSON) - jsonRight := utils.ReadJsonFromFile("../../resources/fixtures/comparator/new_strategy.json") - snapshotLeft := c.NewSnapshotFromJson([]byte(jsonLeft)) - snapshotRight := c.NewSnapshotFromJson([]byte(jsonRight)) + jsonApi := utils.ReadJsonFromFile(DEFAULT_JSON) + jsonRepo := utils.ReadJsonFromFile("../../resources/fixtures/comparator/new_strategy.json") + fromApi := c.NewSnapshotFromJson([]byte(jsonApi)) + fromRepo := c.NewSnapshotFromJson([]byte(jsonRepo)) // Test Check/Merge changes - diffChanged := c.CheckSnapshotDiff(snapshotLeft, snapshotRight, CHANGED) - diffNew := c.CheckSnapshotDiff(snapshotRight, snapshotLeft, NEW) - diffDeleted := c.CheckSnapshotDiff(snapshotLeft, snapshotRight, DELETED) + diffChanged := c.CheckSnapshotDiff(fromApi, fromRepo, CHANGED) + diffNew := c.CheckSnapshotDiff(fromRepo, fromApi, NEW) + diffDeleted := c.CheckSnapshotDiff(fromApi, fromRepo, DELETED) actual := c.MergeResults([]model.DiffResult{diffChanged, diffNew, diffDeleted}) assert.NotNil(t, actual) @@ -312,15 +331,15 @@ func TestCheckStrategySnapshot(t *testing.T) { t.Run("Should return deleted strategy", func(t *testing.T) { // Given - jsonLeft := utils.ReadJsonFromFile(DEFAULT_JSON) - jsonRight := utils.ReadJsonFromFile("../../resources/fixtures/comparator/deleted_strategy.json") - snapshotLeft := c.NewSnapshotFromJson([]byte(jsonLeft)) - snapshotRight := c.NewSnapshotFromJson([]byte(jsonRight)) + jsonApi := utils.ReadJsonFromFile(DEFAULT_JSON) + jsonRepo := utils.ReadJsonFromFile("../../resources/fixtures/comparator/deleted_strategy.json") + fromApi := c.NewSnapshotFromJson([]byte(jsonApi)) + fromRepo := c.NewSnapshotFromJson([]byte(jsonRepo)) // Test Check/Merge changes - diffChanged := c.CheckSnapshotDiff(snapshotLeft, snapshotRight, CHANGED) - diffNew := c.CheckSnapshotDiff(snapshotRight, snapshotLeft, NEW) - diffDeleted := c.CheckSnapshotDiff(snapshotLeft, snapshotRight, DELETED) + diffChanged := c.CheckSnapshotDiff(fromApi, fromRepo, CHANGED) + diffNew := c.CheckSnapshotDiff(fromRepo, fromApi, NEW) + diffDeleted := c.CheckSnapshotDiff(fromApi, fromRepo, DELETED) actual := c.MergeResults([]model.DiffResult{diffChanged, diffNew, diffDeleted}) assert.NotNil(t, actual) @@ -341,15 +360,15 @@ func TestCheckStrategySnapshot(t *testing.T) { t.Run("Should return new strategy value", func(t *testing.T) { // Given - jsonLeft := utils.ReadJsonFromFile(DEFAULT_JSON) - jsonRight := utils.ReadJsonFromFile("../../resources/fixtures/comparator/new_strategy_value.json") - snapshotLeft := c.NewSnapshotFromJson([]byte(jsonLeft)) - snapshotRight := c.NewSnapshotFromJson([]byte(jsonRight)) + jsonApi := utils.ReadJsonFromFile(DEFAULT_JSON) + jsonRepo := utils.ReadJsonFromFile("../../resources/fixtures/comparator/new_strategy_value.json") + fromApi := c.NewSnapshotFromJson([]byte(jsonApi)) + fromRepo := c.NewSnapshotFromJson([]byte(jsonRepo)) // Test Check/Merge changes - diffChanged := c.CheckSnapshotDiff(snapshotLeft, snapshotRight, CHANGED) - diffNew := c.CheckSnapshotDiff(snapshotRight, snapshotLeft, NEW) - diffDeleted := c.CheckSnapshotDiff(snapshotLeft, snapshotRight, DELETED) + diffChanged := c.CheckSnapshotDiff(fromApi, fromRepo, CHANGED) + diffNew := c.CheckSnapshotDiff(fromRepo, fromApi, NEW) + diffDeleted := c.CheckSnapshotDiff(fromApi, fromRepo, DELETED) actual := c.MergeResults([]model.DiffResult{diffChanged, diffNew, diffDeleted}) assert.NotNil(t, actual) @@ -372,15 +391,15 @@ func TestCheckStrategySnapshot(t *testing.T) { t.Run("Should return deleted strategy value", func(t *testing.T) { // Given - jsonLeft := utils.ReadJsonFromFile(DEFAULT_JSON) - jsonRight := utils.ReadJsonFromFile("../../resources/fixtures/comparator/deleted_strategy_value.json") - snapshotLeft := c.NewSnapshotFromJson([]byte(jsonLeft)) - snapshotRight := c.NewSnapshotFromJson([]byte(jsonRight)) + jsonApi := utils.ReadJsonFromFile(DEFAULT_JSON) + jsonRepo := utils.ReadJsonFromFile("../../resources/fixtures/comparator/deleted_strategy_value.json") + fromApi := c.NewSnapshotFromJson([]byte(jsonApi)) + fromRepo := c.NewSnapshotFromJson([]byte(jsonRepo)) // Test Check/Merge changes - diffChanged := c.CheckSnapshotDiff(snapshotLeft, snapshotRight, CHANGED) - diffNew := c.CheckSnapshotDiff(snapshotRight, snapshotLeft, NEW) - diffDeleted := c.CheckSnapshotDiff(snapshotLeft, snapshotRight, DELETED) + diffChanged := c.CheckSnapshotDiff(fromApi, fromRepo, CHANGED) + diffNew := c.CheckSnapshotDiff(fromRepo, fromApi, NEW) + diffDeleted := c.CheckSnapshotDiff(fromApi, fromRepo, DELETED) actual := c.MergeResults([]model.DiffResult{diffChanged, diffNew, diffDeleted}) assert.NotNil(t, actual) @@ -407,15 +426,15 @@ func TestCheckComponentSnapshot(t *testing.T) { t.Run("Should return new component", func(t *testing.T) { // Given - jsonLeft := utils.ReadJsonFromFile(DEFAULT_JSON) - jsonRight := utils.ReadJsonFromFile("../../resources/fixtures/comparator/new_component.json") - snapshotLeft := c.NewSnapshotFromJson([]byte(jsonLeft)) - snapshotRight := c.NewSnapshotFromJson([]byte(jsonRight)) + jsonApi := utils.ReadJsonFromFile(DEFAULT_JSON) + jsonRepo := utils.ReadJsonFromFile("../../resources/fixtures/comparator/new_component.json") + fromApi := c.NewSnapshotFromJson([]byte(jsonApi)) + fromRepo := c.NewSnapshotFromJson([]byte(jsonRepo)) // Test Check/Merge changes - diffChanged := c.CheckSnapshotDiff(snapshotLeft, snapshotRight, CHANGED) - diffNew := c.CheckSnapshotDiff(snapshotRight, snapshotLeft, NEW) - diffDeleted := c.CheckSnapshotDiff(snapshotLeft, snapshotRight, DELETED) + diffChanged := c.CheckSnapshotDiff(fromApi, fromRepo, CHANGED) + diffNew := c.CheckSnapshotDiff(fromRepo, fromApi, NEW) + diffDeleted := c.CheckSnapshotDiff(fromApi, fromRepo, DELETED) actual := c.MergeResults([]model.DiffResult{diffChanged, diffNew, diffDeleted}) assert.NotNil(t, actual) @@ -437,15 +456,15 @@ func TestCheckComponentSnapshot(t *testing.T) { t.Run("Should return deleted component", func(t *testing.T) { // Given - jsonLeft := utils.ReadJsonFromFile(DEFAULT_JSON) - jsonRight := utils.ReadJsonFromFile("../../resources/fixtures/comparator/deleted_component.json") - snapshotLeft := c.NewSnapshotFromJson([]byte(jsonLeft)) - snapshotRight := c.NewSnapshotFromJson([]byte(jsonRight)) + jsonApi := utils.ReadJsonFromFile(DEFAULT_JSON) + jsonRepo := utils.ReadJsonFromFile("../../resources/fixtures/comparator/deleted_component.json") + fromApi := c.NewSnapshotFromJson([]byte(jsonApi)) + fromRepo := c.NewSnapshotFromJson([]byte(jsonRepo)) // Test Check/Merge changes - diffChanged := c.CheckSnapshotDiff(snapshotLeft, snapshotRight, CHANGED) - diffNew := c.CheckSnapshotDiff(snapshotRight, snapshotLeft, NEW) - diffDeleted := c.CheckSnapshotDiff(snapshotLeft, snapshotRight, DELETED) + diffChanged := c.CheckSnapshotDiff(fromApi, fromRepo, CHANGED) + diffNew := c.CheckSnapshotDiff(fromRepo, fromApi, NEW) + diffDeleted := c.CheckSnapshotDiff(fromApi, fromRepo, DELETED) actual := c.MergeResults([]model.DiffResult{diffChanged, diffNew, diffDeleted}) assert.NotNil(t, actual) diff --git a/src/core/handler.go b/src/core/handler.go index f173dac..21f58a4 100644 --- a/src/core/handler.go +++ b/src/core/handler.go @@ -70,7 +70,7 @@ func (c *CoreHandler) StartAccountHandler(accountId string, gitService IGitServi // Wait for account to be active if !account.Settings.Active { - utils.Log(utils.LogLevelInfo, "[%s] Account is not active, waiting for activation", accountId) + utils.Log(utils.LogLevelInfo, "[%s - %s] Account is not active, waiting for activation", accountId, account.Domain.Name) c.updateDomainStatus(*account, model.StatusPending, "Account was deactivated") time.Sleep(1 * time.Minute) continue @@ -83,7 +83,7 @@ func (c *CoreHandler) StartAccountHandler(accountId string, gitService IGitServi repositoryData, err := gitService.GetRepositoryData(account.Environment) if err != nil { - utils.Log(utils.LogLevelError, "[%s] Failed to fetch repository data - %s", accountId, err.Error()) + utils.Log(utils.LogLevelError, "[%s - %s] Failed to fetch repository data - %s", accountId, account.Domain.Name, err.Error()) c.updateDomainStatus(*account, model.StatusError, "Failed to fetch repository data - "+err.Error()) time.Sleep(1 * time.Minute) continue @@ -93,14 +93,14 @@ func (c *CoreHandler) StartAccountHandler(accountId string, gitService IGitServi snapshotVersionPayload, err := c.ApiService.FetchSnapshotVersion(account.Domain.ID, account.Environment) if err != nil { - utils.Log(utils.LogLevelError, "[%s] Failed to fetch snapshot version - %s", accountId, err.Error()) + utils.Log(utils.LogLevelError, "[%s - %s] Failed to fetch snapshot version - %s", accountId, account.Domain.Name, err.Error()) c.updateDomainStatus(*account, model.StatusError, "Failed to fetch snapshot version - "+err.Error()) time.Sleep(1 * time.Minute) continue } // Check if repository is out of sync - if c.isRepositoryOutSync(*account, repositoryData.CommitHash, snapshotVersionPayload) { + if c.isOutSync(*account, repositoryData.CommitHash, snapshotVersionPayload) { c.syncUp(*account, repositoryData, gitService) } @@ -114,8 +114,6 @@ func (c *CoreHandler) syncUp(account model.Account, repositoryData *model.Reposi utils.Log(utils.LogLevelInfo, "[%s - %s] Syncing up", account.ID.Hex(), account.Domain.Name) // Update account status: Out of sync - account.Domain.LastCommit = repositoryData.CommitHash - account.Domain.LastDate = repositoryData.CommitDate c.updateDomainStatus(account, model.StatusOutSync, model.MessageSyncingUp) // Check for changes @@ -133,7 +131,7 @@ func (c *CoreHandler) syncUp(account model.Account, repositoryData *model.Reposi changeSource := "" if snapshotApi.Domain.Version > account.Domain.Version { changeSource = "Repository" - if len(diff.Changes) > 0 { + if c.isRepositoryOutSync(account, repositoryData, diff) { account, err = c.applyChangesToRepository(account, snapshotApi, gitService) } else { utils.Log(utils.LogLevelInfo, "[%s - %s] Repository is up to date", account.ID.Hex(), account.Domain.Name) @@ -142,7 +140,7 @@ func (c *CoreHandler) syncUp(account model.Account, repositoryData *model.Reposi } } else if len(diff.Changes) > 0 { changeSource = "API" - account = c.applyChangesToAPI(account, repositoryData) + account = c.applyChangesToAPI(account, repositoryData, diff) } if err != nil { @@ -174,18 +172,19 @@ func (c *CoreHandler) checkForChanges(account model.Account, content string) (mo // Compare Snapshots and get diff diffNew := c.ComparatorService.CheckSnapshotDiff(fromRepo, fromApi, NEW) diffChanged := c.ComparatorService.CheckSnapshotDiff(fromApi, fromRepo, CHANGED) - - var diffDeleted model.DiffResult - if account.Settings.ForcePrune { - diffDeleted = c.ComparatorService.CheckSnapshotDiff(fromApi, fromRepo, DELETED) - } + diffDeleted := c.ComparatorService.CheckSnapshotDiff(fromApi, fromRepo, DELETED) return c.ComparatorService.MergeResults([]model.DiffResult{diffNew, diffChanged, diffDeleted}), snapshotApi.Snapshot, nil } -func (c *CoreHandler) applyChangesToAPI(account model.Account, repositoryData *model.RepositoryData) model.Account { +func (c *CoreHandler) applyChangesToAPI(account model.Account, repositoryData *model.RepositoryData, diff model.DiffResult) model.Account { utils.Log(utils.LogLevelInfo, "[%s - %s] Pushing changes to API", account.ID.Hex(), account.Domain.Name) + // Removed deleted if force prune is disabled + if !account.Settings.ForcePrune { + c.ComparatorService.RemoveDeleted(diff) + } + // Push changes to API // Update domain @@ -204,14 +203,18 @@ func (c *CoreHandler) applyChangesToRepository(account model.Account, snapshot m lastCommit, err := gitService.PushChanges(account.Environment, utils.ToJsonFromObject(snapshotContent)) + if err != nil { + return account, err + } + // Update domain account.Domain.Version = snapshot.Domain.Version account.Domain.LastCommit = lastCommit - return account, err + return account, nil } -func (c *CoreHandler) isRepositoryOutSync(account model.Account, lastCommit string, snapshotVersionPayload string) bool { +func (c *CoreHandler) isOutSync(account model.Account, lastCommit string, snapshotVersionPayload string) bool { snapshotVersion := c.ApiService.NewDataFromJson([]byte(snapshotVersionPayload)).Snapshot.Domain.Version utils.Log(utils.LogLevelDebug, "[%s - %s] Checking account - Last commit: %s - Domain Version: %d - Snapshot Version: %d", @@ -222,8 +225,13 @@ func (c *CoreHandler) isRepositoryOutSync(account model.Account, lastCommit stri account.Domain.Version != snapshotVersion // API out of sync } +func (c *CoreHandler) isRepositoryOutSync(account model.Account, repositoryData *model.RepositoryData, diff model.DiffResult) bool { + return account.Domain.Version == 0 || // First/Force-push sync + len(repositoryData.Content) <= 1 || // File is empty + len(diff.Changes) > 0 // Changes detected +} + func (c *CoreHandler) updateDomainStatus(account model.Account, status string, message string) { - account.Token = "" account.Domain.Status = status account.Domain.Message = message account.Domain.LastDate = time.Now().Format(time.ANSIC) diff --git a/src/core/handler_test.go b/src/core/handler_test.go index 5e61d94..69107fd 100644 --- a/src/core/handler_test.go +++ b/src/core/handler_test.go @@ -211,6 +211,7 @@ func TestStartAccountHandler(t *testing.T) { account := givenAccount() account.Domain.ID = "123-up-to-date-not-synced" + account.Domain.Version = -1 // Different from the API version accountCreated, _ := coreHandler.AccountRepository.Create(&account) // Test @@ -264,6 +265,40 @@ func TestStartAccountHandler(t *testing.T) { tearDown() }) + t.Run("Should sync and not prune when repository is out of sync", func(t *testing.T) { + // Given + fakeGitService := NewFakeGitService() + fakeGitService.content = `{ + "domain": { + "group": [] + } + }` + fakeApiService := NewFakeApiService() + coreHandler = NewCoreHandler(coreHandler.AccountRepository, fakeApiService, NewComparatorService()) + + account := givenAccount() + account.Domain.ID = "123-out-sync-not-prune" + account.Domain.Version = 1 + account.Settings.ForcePrune = false + accountCreated, _ := coreHandler.AccountRepository.Create(&account) + + // Test + go coreHandler.StartAccountHandler(accountCreated.ID.Hex(), fakeGitService) + + // Wait for goroutine to process + time.Sleep(1 * time.Second) + + // Assert + accountFromDb, _ := coreHandler.AccountRepository.FetchByDomainId(accountCreated.Domain.ID) + assert.Equal(t, model.StatusSynced, accountFromDb.Domain.Status) + assert.Contains(t, accountFromDb.Domain.Message, model.MessageSynced) + assert.Equal(t, "123", accountFromDb.Domain.LastCommit) + assert.Equal(t, 2, accountFromDb.Domain.Version) + assert.NotEqual(t, "", accountFromDb.Domain.LastDate) + + tearDown() + }) + t.Run("Should sync successfully when API has a newer version", func(t *testing.T) { // Given fakeGitService := NewFakeGitService() @@ -315,7 +350,7 @@ func TestStartAccountHandler(t *testing.T) { accountFromDb, _ := coreHandler.AccountRepository.FetchByDomainId(accountCreated.Domain.ID) assert.Equal(t, model.StatusError, accountFromDb.Domain.Status) assert.Contains(t, accountFromDb.Domain.Message, "Failed to check for changes") - assert.Equal(t, "123", accountFromDb.Domain.LastCommit) + assert.Equal(t, "", accountFromDb.Domain.LastCommit) assert.NotEqual(t, "", accountFromDb.Domain.LastDate) tearDown() diff --git a/src/repository/account.go b/src/repository/account.go index 1c623d4..3138485 100644 --- a/src/repository/account.go +++ b/src/repository/account.go @@ -5,7 +5,6 @@ import ( "errors" "time" - "github.com/switcherapi/switcher-gitops/src/config" "github.com/switcherapi/switcher-gitops/src/model" "github.com/switcherapi/switcher-gitops/src/utils" "go.mongodb.org/mongo-driver/bson" @@ -40,9 +39,6 @@ func (repo *AccountRepositoryMongo) Create(account *model.Account) (*model.Accou collection, ctx, cancel := getDbContext(repo) defer cancel() - // Encrypt token before saving - account.Token = utils.Encrypt(account.Token, config.GetEnv("GIT_TOKEN_PRIVATE_KEY")) - result, err := collection.InsertOne(ctx, account) if err != nil { return nil, err @@ -104,11 +100,6 @@ func (repo *AccountRepositoryMongo) Update(account *model.Account) (*model.Accou collection, ctx, cancel := getDbContext(repo) defer cancel() - // Encrypt token before saving - if account.Token != "" { - account.Token = utils.Encrypt(account.Token, config.GetEnv("GIT_TOKEN_PRIVATE_KEY")) - } - filter := primitive.M{domainIdFilter: account.Domain.ID} update := getUpdateFields(account) @@ -173,19 +164,38 @@ func registerAccountRepositoryValidators(db *mongo.Database) { } } -func getUpdateFields(account *model.Account) primitive.M { - accountMap := utils.ToMapFromObject(account) +func getUpdateFields(account *model.Account) bson.M { + setMap := bson.M{} - update := primitive.M{ - "$set": accountMap, - } + setMap["repository"] = account.Repository + setMap["branch"] = account.Branch + setMap["environment"] = account.Environment + setMap["domain.name"] = account.Domain.Name + setMap["settings.active"] = account.Settings.Active + setMap["settings.window"] = account.Settings.Window + setMap["settings.forcePrune"] = account.Settings.ForcePrune + + setIfNotEmpty(setMap, "token", account.Token) + setIfNotEmpty(setMap, "domain.version", account.Domain.Version) + setIfNotEmpty(setMap, "domain.lastCommit", account.Domain.LastCommit) + setIfNotEmpty(setMap, "domain.lastDate", account.Domain.LastDate) + setIfNotEmpty(setMap, "domain.status", account.Domain.Status) + setIfNotEmpty(setMap, "domain.message", account.Domain.Message) + + update := bson.M{"$set": setMap} - deleteEmpty(accountMap, "token") return update } -func deleteEmpty(setMap map[string]interface{}, key string) { - if setMap[key] == "" { - delete(setMap, key) +func setIfNotEmpty(setMap bson.M, key string, value interface{}) { + switch v := value.(type) { + case string: + if v != "" { + setMap[key] = v + } + case int: + if v != 0 { + setMap[key] = v + } } } From a3e98be3ab8d68a1c4d07772ca7164534062f23c Mon Sep 17 00:00:00 2001 From: petruki <31597636+petruki@users.noreply.github.com> Date: Fri, 13 Sep 2024 17:58:27 -0700 Subject: [PATCH 2/2] Added missing test for comparator --- src/core/comparator_test.go | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/src/core/comparator_test.go b/src/core/comparator_test.go index 1471d8b..905b4df 100644 --- a/src/core/comparator_test.go +++ b/src/core/comparator_test.go @@ -198,6 +198,38 @@ func TestCheckConfigSnapshot(t *testing.T) { ]}`, utils.ToJsonFromObject(actual)) }) + t.Run("Should return changes in config after calling RemoveDeleted", func(t *testing.T) { + // Given + jsonApi := utils.ReadJsonFromFile(DEFAULT_JSON) + jsonRepo := utils.ReadJsonFromFile("../../resources/fixtures/comparator/changed_config.json") + fromApi := c.NewSnapshotFromJson([]byte(jsonApi)) + fromRepo := c.NewSnapshotFromJson([]byte(jsonRepo)) + + // Test Check/Merge changes + diffChanged := c.CheckSnapshotDiff(fromApi, fromRepo, CHANGED) + diffNew := c.CheckSnapshotDiff(fromRepo, fromApi, NEW) + diffDeleted := c.CheckSnapshotDiff(fromApi, fromRepo, DELETED) + actual := c.MergeResults([]model.DiffResult{diffChanged, diffNew, diffDeleted}) + actual = c.RemoveDeleted(actual) + + assert.NotNil(t, actual) + assert.JSONEq(t, `{ + "changes": [ + { + "action": "CHANGED", + "diff": "CONFIG", + "path": [ + "Release 1", + "MY_SWITCHER_2" + ], + "content": { + "activated": true, + "description": "New description" + } + } + ]}`, utils.ToJsonFromObject(actual)) + }) + t.Run("Should return new config", func(t *testing.T) { // Given jsonApi := utils.ReadJsonFromFile(DEFAULT_JSON)