From e8b3c8c4fe69ca7bc1a15bb54b3d29261f56cbe5 Mon Sep 17 00:00:00 2001
From: yahavi <yahavi@jfrog.com>
Date: Sun, 26 Mar 2023 17:18:45 +0300
Subject: [PATCH] Config transfer - ensure target newer than source

---
 .../commands/utils/transferconfigbase.go      | 13 +++++
 .../commands/utils/transferconfigbase_test.go | 58 +++++++++++++------
 2 files changed, 54 insertions(+), 17 deletions(-)

diff --git a/artifactory/commands/utils/transferconfigbase.go b/artifactory/commands/utils/transferconfigbase.go
index 9b9fb8286..991165207 100644
--- a/artifactory/commands/utils/transferconfigbase.go
+++ b/artifactory/commands/utils/transferconfigbase.go
@@ -4,6 +4,7 @@ import (
 	"encoding/json"
 	"fmt"
 
+	"github.com/jfrog/gofrog/version"
 	"github.com/jfrog/jfrog-cli-core/v2/artifactory/utils"
 	"github.com/jfrog/jfrog-cli-core/v2/utils/config"
 	"github.com/jfrog/jfrog-cli-core/v2/utils/coreutils"
@@ -73,10 +74,22 @@ func (tcb *TransferConfigBase) ValidateMinVersionAndDifferentServers() (string,
 	if err != nil {
 		return "", err
 	}
+	targetArtifactoryVersion, err := tcb.TargetArtifactoryManager.GetVersion()
+	if err != nil {
+		return "", err
+	}
+
+	// Validate minimal Artifactory version in the source server
 	err = coreutils.ValidateMinimumVersion(coreutils.Artifactory, sourceArtifactoryVersion, minTransferConfigArtifactoryVersion)
 	if err != nil {
 		return "", err
 	}
+
+	// Validate that the target Artifactory server is at least as the source Artifactory server
+	if !version.NewVersion(targetArtifactoryVersion).AtLeast(sourceArtifactoryVersion) {
+		return "", errorutils.CheckErrorf("The source Artifactory version (%s) can't be higher than the target Artifactory version (%s).", sourceArtifactoryVersion, targetArtifactoryVersion)
+	}
+
 	// Avoid exporting and importing to the same server
 	log.Info("Verifying source and target servers are different...")
 	if tcb.SourceServerDetails.GetArtifactoryUrl() == tcb.TargetServerDetails.GetArtifactoryUrl() {
diff --git a/artifactory/commands/utils/transferconfigbase_test.go b/artifactory/commands/utils/transferconfigbase_test.go
index f9adfbf1b..1dc548b4f 100644
--- a/artifactory/commands/utils/transferconfigbase_test.go
+++ b/artifactory/commands/utils/transferconfigbase_test.go
@@ -99,31 +99,55 @@ func TestIsDefaultCredentialsLocked(t *testing.T) {
 	assert.Equal(t, 0, unlockCounter)
 }
 
+var validateMinVersionAndDifferentServersCases = []struct {
+	testName      string
+	sourceVersion string
+	targetVersion string
+	expectedError string
+}{
+	{testName: "Same version", sourceVersion: minTransferConfigArtifactoryVersion, targetVersion: minTransferConfigArtifactoryVersion, expectedError: ""},
+	{testName: "Different version", sourceVersion: "7.0.0", targetVersion: "7.0.1", expectedError: ""},
+	{testName: "Low Artifactory version", sourceVersion: "6.0.0", targetVersion: "7.0.0", expectedError: "while this operation requires version"},
+	{testName: "Source newer than target", sourceVersion: "7.0.1", targetVersion: "7.0.0", expectedError: "can't be higher than the target Artifactory version"},
+}
+
 func TestValidateMinVersionAndDifferentServers(t *testing.T) {
-	var rtVersion string
+	var sourceRtVersion, targetRtVersion string
 	// Create transfer config command
-	testServer, serverDetails, _ := commonTests.CreateRtRestsMockServer(t, func(w http.ResponseWriter, r *http.Request) {
-		content, err := json.Marshal(VersionResponse{Version: rtVersion})
+	sourceTestServer, sourceServerDetails, _ := commonTests.CreateRtRestsMockServer(t, func(w http.ResponseWriter, _ *http.Request) {
+		content, err := json.Marshal(VersionResponse{Version: sourceRtVersion})
 		assert.NoError(t, err)
 		_, err = w.Write(content)
 		assert.NoError(t, err)
 	})
-	defer testServer.Close()
-
-	// Test low Artifactory version
-	rtVersion = "6.0.0"
-	_, err := createTransferConfigBase(t, serverDetails, serverDetails).ValidateMinVersionAndDifferentServers()
-	assert.ErrorContains(t, err, "while this operation requires version")
+	defer sourceTestServer.Close()
+	targetTestServer, targetServerDetails, _ := commonTests.CreateRtRestsMockServer(t, func(w http.ResponseWriter, _ *http.Request) {
+		content, err := json.Marshal(VersionResponse{Version: targetRtVersion})
+		assert.NoError(t, err)
+		_, err = w.Write(content)
+		assert.NoError(t, err)
+	})
+	defer targetTestServer.Close()
 
-	// Test same source and target Artifactory servers
-	rtVersion = minTransferConfigArtifactoryVersion
-	_, err = createTransferConfigBase(t, serverDetails, serverDetails).ValidateMinVersionAndDifferentServers()
-	assert.ErrorContains(t, err, "The source and target Artifactory servers are identical, but should be different.")
+	for _, testCase := range validateMinVersionAndDifferentServersCases {
+		t.Run(testCase.testName, func(t *testing.T) {
+			sourceRtVersion = testCase.sourceVersion
+			targetRtVersion = testCase.targetVersion
+			actualSourceVersion, err := createTransferConfigBase(t, sourceServerDetails, targetServerDetails).ValidateMinVersionAndDifferentServers()
+			if testCase.expectedError == "" {
+				assert.NoError(t, err)
+				assert.Equal(t, testCase.sourceVersion, actualSourceVersion)
+			} else {
+				assert.ErrorContains(t, err, testCase.expectedError)
+			}
+		})
+	}
 
-	// Positive test
-	actualVersion, err := createTransferConfigBase(t, serverDetails, &config.ServerDetails{ArtifactoryUrl: "some-different-url"}).ValidateMinVersionAndDifferentServers()
-	assert.NoError(t, err)
-	assert.Equal(t, rtVersion, actualVersion)
+	t.Run("Same source and target servers", func(t *testing.T) {
+		sourceRtVersion = minTransferConfigArtifactoryVersion
+		_, err := createTransferConfigBase(t, sourceServerDetails, sourceServerDetails).ValidateMinVersionAndDifferentServers()
+		assert.ErrorContains(t, err, "The source and target Artifactory servers are identical, but should be different.")
+	})
 }
 
 func TestGetSelectedRepositories(t *testing.T) {