PMM-12913 refactor /v1/backups/locations

percona · Apr 8, 2024 · b9b5192 · b9b5192
1 parent 908b241
commit b9b5192
Show file tree

Hide file tree

Showing 37 changed files with 3,659 additions and 3,650 deletions.
diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
@@ -93,7 +93,8 @@ jobs:
               exit $code
             else
               echo
-              # suppress passing to reviewdog because of https://github.com/reviewdog/reviewdog/issues/1696
+              # Suppress passing to reviewdog because of https://github.com/reviewdog/reviewdog/issues/1696
+              # TODO: re-enable before merging to v3
               # echo "$out" | bin/reviewdog -f=buf -reporter=github-pr-review -fail-on-error=true
             fi
           else

diff --git a/.github/workflows/managed.yml b/.github/workflows/managed.yml
@@ -65,9 +65,9 @@ jobs:
         run: |
           # Note: launching the container with --wait fails for an unknown reason.
           # A temporary workaround is to run it manually. To be reverted once the issue is resolved.
-          # make env-compose-up # the container workdir is /root/go/src/github.com/percona/pmm
-          docker compose --profile pmm up -d
-          sleep 100s
+          make env-compose-up # the container workdir is /root/go/src/github.com/percona/pmm
+          # docker compose --profile pmm up -d
+          # sleep 100s
           docker logs pmm-server
 
       - name: Restore Go build cache

diff --git a/api-tests/backup/backup_test.go b/api-tests/backup/backup_test.go
@@ -159,10 +159,8 @@ func TestScheduleBackup(t *testing.T) {
 			assert.Equal(t, "backup_folder", bkp.Folder)
 
 			_, err = client.RemoveScheduledBackup(&backup.RemoveScheduledBackupParams{
-				Body: backup.RemoveScheduledBackupBody{
-					ScheduledBackupID: backupRes.Payload.ScheduledBackupID,
-				},
-				Context: pmmapitests.Context,
+				ScheduledBackupID: backupRes.Payload.ScheduledBackupID,
+				Context:           pmmapitests.Context,
 			})
 			require.NoError(t, err)
 
@@ -357,10 +355,8 @@ func TestScheduleBackup(t *testing.T) {
 func removeScheduledBackup(t *testing.T, id string) {
 	t.Helper()
 	_, err := backupClient.Default.BackupService.RemoveScheduledBackup(&backup.RemoveScheduledBackupParams{
-		Body: backup.RemoveScheduledBackupBody{
-			ScheduledBackupID: id,
-		},
-		Context: pmmapitests.Context,
+		ScheduledBackupID: id,
+		Context:           pmmapitests.Context,
 	})
 	require.NoError(t, err)
 }
diff --git a/api-tests/backup/locations_test.go b/api-tests/backup/locations_test.go
@@ -19,6 +19,7 @@ import (
 	"os"
 	"testing"
 
+	"github.com/AlekSi/pointer"
 	"github.com/brianvoe/gofakeit/v6"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
@@ -230,11 +231,11 @@ func TestChangeLocation(t *testing.T) {
 	t.Parallel()
 	client := backupClient.Default.LocationsService
 
-	checkChange := func(t *testing.T, req locations.ChangeLocationBody, locations []*locations.ListLocationsOKBodyLocationsItems0) {
+	checkChange := func(t *testing.T, req locations.ChangeLocationBody, locationID string, locations []*locations.ListLocationsOKBodyLocationsItems0) {
 		t.Helper()
 		var found bool
 		for _, loc := range locations {
-			if loc.LocationID == req.LocationID {
+			if loc.LocationID == locationID {
 				assert.Equal(t, req.Name, loc.Name)
 				if req.Description != "" {
 					assert.Equal(t, req.Description, loc.Description)
@@ -282,22 +283,22 @@ func TestChangeLocation(t *testing.T) {
 		defer deleteLocation(t, client, resp.Payload.LocationID)
 
 		updateBody := locations.ChangeLocationBody{
-			LocationID: resp.Payload.LocationID,
-			Name:       gofakeit.Name(),
+			Name: gofakeit.Name(),
 			FilesystemConfig: &locations.ChangeLocationParamsBodyFilesystemConfig{
 				Path: "/tmp/nested",
 			},
 		}
 		_, err = client.ChangeLocation(&locations.ChangeLocationParams{
-			Body:    updateBody,
-			Context: pmmapitests.Context,
+			LocationID: resp.Payload.LocationID,
+			Body:       updateBody,
+			Context:    pmmapitests.Context,
 		})
 		require.NoError(t, err)
 
 		listResp, err := client.ListLocations(&locations.ListLocationsParams{Context: pmmapitests.Context})
 		require.NoError(t, err)
 
-		checkChange(t, updateBody, listResp.Payload.Locations)
+		checkChange(t, updateBody, resp.Payload.LocationID, listResp.Payload.Locations)
 	})
 
 	t.Run("update only name", func(t *testing.T) {
@@ -318,12 +319,12 @@ func TestChangeLocation(t *testing.T) {
 		defer deleteLocation(t, client, resp.Payload.LocationID)
 
 		updateBody := locations.ChangeLocationBody{
-			LocationID: resp.Payload.LocationID,
-			Name:       gofakeit.Name(),
+			Name: gofakeit.Name(),
 		}
 		_, err = client.ChangeLocation(&locations.ChangeLocationParams{
-			Body:    updateBody,
-			Context: pmmapitests.Context,
+			LocationID: resp.Payload.LocationID,
+			Body:       updateBody,
+			Context:    pmmapitests.Context,
 		})
 		require.NoError(t, err)
 
@@ -366,8 +367,7 @@ func TestChangeLocation(t *testing.T) {
 		defer deleteLocation(t, client, resp.Payload.LocationID)
 
 		updateBody := locations.ChangeLocationBody{
-			LocationID: resp.Payload.LocationID,
-			Name:       gofakeit.Name(),
+			Name: gofakeit.Name(),
 			S3Config: &locations.ChangeLocationParamsBodyS3Config{
 				Endpoint:   "https://s3.us-west-2.amazonaws.com",
 				AccessKey:  accessKey,
@@ -376,15 +376,16 @@ func TestChangeLocation(t *testing.T) {
 			},
 		}
 		_, err = client.ChangeLocation(&locations.ChangeLocationParams{
-			Body:    updateBody,
-			Context: pmmapitests.Context,
+			LocationID: resp.Payload.LocationID,
+			Body:       updateBody,
+			Context:    pmmapitests.Context,
 		})
 		require.NoError(t, err)
 
 		listResp, err := client.ListLocations(&locations.ListLocationsParams{Context: pmmapitests.Context})
 		require.NoError(t, err)
 
-		checkChange(t, updateBody, listResp.Payload.Locations)
+		checkChange(t, updateBody, resp.Payload.LocationID, listResp.Payload.Locations)
 	})
 
 	t.Run("change to existing name - error", func(t *testing.T) {
@@ -419,15 +420,15 @@ func TestChangeLocation(t *testing.T) {
 		defer deleteLocation(t, client, resp2.Payload.LocationID)
 
 		updateBody := locations.ChangeLocationBody{
-			LocationID: resp2.Payload.LocationID,
-			Name:       addReqBody1.Name,
+			Name: addReqBody1.Name,
 			FilesystemConfig: &locations.ChangeLocationParamsBodyFilesystemConfig{
 				Path: "/tmp",
 			},
 		}
 		_, err = client.ChangeLocation(&locations.ChangeLocationParams{
-			Body:    updateBody,
-			Context: pmmapitests.Context,
+			LocationID: resp2.Payload.LocationID,
+			Body:       updateBody,
+			Context:    pmmapitests.Context,
 		})
 
 		pmmapitests.AssertAPIErrorf(t, err, 409, codes.AlreadyExists, `Location with name "%s" already exists.`, updateBody.Name)
@@ -450,11 +451,9 @@ func TestRemoveLocation(t *testing.T) {
 	require.NoError(t, err)
 
 	_, err = client.RemoveLocation(&locations.RemoveLocationParams{
-		Body: locations.RemoveLocationBody{
-			LocationID: resp.Payload.LocationID,
-			Force:      false,
-		},
-		Context: pmmapitests.Context,
+		LocationID: resp.Payload.LocationID,
+		Force:      pointer.ToBool(false),
+		Context:    pmmapitests.Context,
 	})
 
 	require.NoError(t, err)
@@ -569,11 +568,9 @@ func TestLocationConfigValidation(t *testing.T) {
 func deleteLocation(t *testing.T, client locations.ClientService, id string) {
 	t.Helper()
 	_, err := client.RemoveLocation(&locations.RemoveLocationParams{
-		Body: locations.RemoveLocationBody{
-			LocationID: id,
-			Force:      false,
-		},
-		Context: pmmapitests.Context,
+		LocationID: id,
+		Force:      pointer.ToBool(false),
+		Context:    pmmapitests.Context,
 	})
 	assert.NoError(t, err)
 }
diff --git a/api/MIGRATION_TO_V3.md b/api/MIGRATION_TO_V3.md
@@ -106,11 +106,11 @@ POST /v1/backup/Backups/Schedule                    POST /v1/backups:schedule
 POST /v1/backup/Backups/Start                       POST /v1/backups:start                           ✅
 
 **LocationsService**                                **LocationsService**
-POST /v1/backup/Locations/Add                       POST /v1/backups/locations
-POST /v1/backup/Locations/Change                    PUT /v1/backups/locations/{id}                   Extract the location_id from the body to {id}
-POST /v1/backup/Locations/List                      GET /v1/backups/locations
-POST /v1/backup/Locations/Remove                    DELETE /v1/backups/locations/{id}                ?force=true
-POST /v1/backup/Locations/TestConfig                POST /v1/backups/locations:testConfig
+POST /v1/backup/Locations/Add                       POST /v1/backups/locations                       ✅
+POST /v1/backup/Locations/Change                    PUT /v1/backups/locations/{location_id}          ✅
+POST /v1/backup/Locations/List                      GET /v1/backups/locations                        ✅
+POST /v1/backup/Locations/Remove                    DELETE /v1/backups/locations/{location_id}       ✅ ?force=true
+POST /v1/backup/Locations/TestConfig                POST /v1/backups/locations:testConfig            ✅
 
 **RestoreHistoryService**                           **RestoreService**
 POST /v1/backup/RestoreHistory/List                 GET /v1/backups/restores