fix(api): password was overrided by default empty value (#3905)

ovh · Feb 4, 2019 · aec2f67 · aec2f67
1 parent 77c2db1
commit aec2f67
Show file tree

Hide file tree

Showing 4 changed files with 135 additions and 36 deletions.
diff --git a/engine/api/application/application_parser.go b/engine/api/application/application_parser.go
@@ -1,7 +1,6 @@
 package application
 
 import (
-	"fmt"
 	"strings"
 	"sync"
 
@@ -21,13 +20,13 @@ func ParseAndImport(db gorp.SqlExecutor, cache cache.Store, proj *sdk.Project, e
 	//Check valid application name
 	rx := sdk.NamePatternRegex
 	if !rx.MatchString(eapp.Name) {
-		return nil, nil, sdk.WrapError(sdk.ErrInvalidApplicationPattern, "ParseAndImport>> Application name %s do not respect pattern %s", eapp.Name, sdk.NamePattern)
+		return nil, nil, sdk.WrapError(sdk.ErrInvalidApplicationPattern, "application name %s do not respect pattern %s", eapp.Name, sdk.NamePattern)
 	}
 
 	//Check if app exist
 	oldApp, errl := LoadByName(db, cache, proj.Key, eapp.Name, nil, LoadOptions.WithVariablesWithClearPassword, LoadOptions.WithKeys, LoadOptions.WithClearDeploymentStrategies)
 	if errl != nil && !sdk.ErrorIs(errl, sdk.ErrApplicationNotFound) {
-		return nil, nil, sdk.WrapError(errl, "ParseAndImport>> Unable to load application")
+		return nil, nil, sdk.WrapError(errl, "unable to load application")
 	}
 
 	//If the application exist and we don't want to force, raise an error
@@ -63,7 +62,7 @@ func ParseAndImport(db gorp.SqlExecutor, cache cache.Store, proj *sdk.Project, e
 		case sdk.SecretVariable:
 			secret, err := decryptFunc(db, proj.ID, v.Value)
 			if err != nil {
-				return app, nil, sdk.WrapError(sdk.NewError(sdk.ErrWrongRequest, err), "ParseAndImport>> Unable to decrypt secret variable")
+				return app, nil, sdk.WrapError(sdk.NewError(sdk.ErrWrongRequest, err), "unable to decrypt secret variable")
 			}
 			v.Value = secret
 		}
@@ -75,7 +74,7 @@ func ParseAndImport(db gorp.SqlExecutor, cache cache.Store, proj *sdk.Project, e
 	//Compute keys
 	for kname, kval := range eapp.Keys {
 		if !strings.HasPrefix(kname, "app-") {
-			return app, nil, sdk.WrapError(sdk.ErrInvalidKeyName, "ParseAndImport>> Unable to parse key %s", kname)
+			return app, nil, sdk.WrapError(sdk.ErrInvalidKeyName, "unable to parse key %s", kname)
 		}
 
 		var oldKey *sdk.ApplicationKey
@@ -100,7 +99,7 @@ func ParseAndImport(db gorp.SqlExecutor, cache cache.Store, proj *sdk.Project, e
 
 		kk, err := keys.Parse(db, proj.ID, kname, kval, decryptFunc)
 		if err != nil {
-			return app, nil, sdk.WrapError(sdk.NewError(sdk.ErrWrongRequest, err), "ParseAndImport>> Unable to parse key")
+			return app, nil, sdk.WrapError(sdk.NewError(sdk.ErrWrongRequest, err), "unable to parse key")
 		}
 
 		k := sdk.ApplicationKey{
@@ -126,61 +125,58 @@ func ParseAndImport(db gorp.SqlExecutor, cache cache.Store, proj *sdk.Project, e
 		app.RepositoryStrategy.ConnectionType = "https"
 	}
 	if app.RepositoryStrategy.ConnectionType == "ssh" && app.RepositoryStrategy.SSHKey == "" {
-		return app, nil, sdk.NewErrorFrom(sdk.ErrInvalidApplicationRepoStrategy, "Could not import application %s with a connection type ssh without ssh key", app.Name)
+		return app, nil, sdk.NewErrorFrom(sdk.ErrInvalidApplicationRepoStrategy, "could not import application %s with a connection type ssh without ssh key", app.Name)
 	}
 	if eapp.VCSPassword != "" {
 		clearPWD, err := decryptFunc(db, proj.ID, eapp.VCSPassword)
 		if err != nil {
-			return app, nil, sdk.WrapError(sdk.NewError(sdk.ErrWrongRequest, err), "ParseAndImport> Unable to decrypt vcs password")
+			return app, nil, sdk.WrapError(sdk.NewError(sdk.ErrWrongRequest, err), "unable to decrypt vcs password")
 		}
 		app.RepositoryStrategy.Password = clearPWD
 		if errE := EncryptVCSStrategyPassword(app); errE != nil {
-			return app, nil, sdk.WrapError(errE, "ParseAndImport> Cannot encrypt vcs password")
+			return app, nil, sdk.WrapError(errE, "cannot encrypt vcs password")
 		}
 	}
 
-	//deployment strategies
+	// deployment strategies
+	deploymentStrategies := make(map[string]sdk.IntegrationConfig)
 	for pfName, pfConfig := range eapp.DeploymentStrategies {
-		if app.DeploymentStrategies == nil {
-			app.DeploymentStrategies = make(map[string]sdk.IntegrationConfig)
-		}
-		if app.DeploymentStrategies[pfName] == nil {
-			app.DeploymentStrategies[pfName] = make(map[string]sdk.IntegrationConfigValue)
-		}
-
-		projPF, has := proj.GetIntegration(pfName)
+		// init deployment strategy from project if default exists
+		projIt, has := proj.GetIntegration(pfName)
 		if !has {
-			return app, nil, sdk.WrapError(sdk.NewError(sdk.ErrWrongRequest, fmt.Errorf("integration not found")), "ParseAndImport> Integration %s not found", pfName)
+			return app, nil, sdk.WrapError(sdk.NewErrorFrom(sdk.ErrWrongRequest, "deployment platform not found"), "deployment platform %s not found", pfName)
+		}
+		if projIt.Model.DeploymentDefaultConfig != nil {
+			deploymentStrategies[pfName] = projIt.Model.DeploymentDefaultConfig.Clone()
+		} else {
+			deploymentStrategies[pfName] = make(map[string]sdk.IntegrationConfigValue)
 		}
 
-		// Inherit from existing deployment strategy or from the project
+		// merge deployment strategy with old application deployment strategy if exists
 		if oldApp != nil {
-			oldPFConfig, has := oldApp.DeploymentStrategies[pfName]
-			if has {
-				app.DeploymentStrategies[pfName] = oldPFConfig.Clone()
+			if oldItConfig, has := oldApp.DeploymentStrategies[pfName]; has {
+				deploymentStrategies[pfName].MergeWith(oldItConfig)
 			}
-		} else {
-			app.DeploymentStrategies[pfName] = projPF.Model.DeploymentDefaultConfig.Clone()
 		}
 
-		app.DeploymentStrategies[pfName].MergeWith(projPF.Model.DeploymentDefaultConfig.Clone())
-
+		// update deployment strategy with given values from request
 		for k, v := range pfConfig {
 			if v.Value != "" {
 				if v.Type == sdk.SecretVariable {
 					clearPWD, err := decryptFunc(db, proj.ID, v.Value)
 					if err != nil {
-						return app, nil, sdk.WrapError(sdk.NewError(sdk.ErrWrongRequest, err), "ParseAndImport> Unable to decrypt deployment strategy password")
+						return app, nil, sdk.WrapError(sdk.NewError(sdk.ErrWrongRequest, err), "unable to decrypt deployment strategy password")
 					}
 					v.Value = clearPWD
 				}
 			}
-			app.DeploymentStrategies[pfName][k] = sdk.IntegrationConfigValue{
+			deploymentStrategies[pfName][k] = sdk.IntegrationConfigValue{
 				Type:  v.Type,
 				Value: v.Value,
 			}
 		}
 	}
+	app.DeploymentStrategies = deploymentStrategies
 
 	done := new(sync.WaitGroup)
 	done.Add(1)

diff --git a/engine/api/application_deployment.go b/engine/api/application_deployment.go
@@ -76,15 +76,15 @@ func (api *API) postApplicationDeploymentStrategyConfigHandler() service.Handler
 
 		oldPfConfig, has := app.DeploymentStrategies[pfName]
 		if !has {
-			oldPfConfig = pf.Model.DeploymentDefaultConfig
-		}
-		if oldPfConfig == nil {
-			oldPfConfig = sdk.IntegrationConfig{}
+			if pf.Model.DeploymentDefaultConfig != nil {
+				oldPfConfig = pf.Model.DeploymentDefaultConfig
+			} else {
+				oldPfConfig = sdk.IntegrationConfig{}
+			}
 		}
 		oldPfConfig.MergeWith(pfConfig)
-		pfConfig = oldPfConfig
 
-		if err := application.SetDeploymentStrategy(tx, proj.ID, app.ID, pf.Model.ID, pfName, pfConfig); err != nil {
+		if err := application.SetDeploymentStrategy(tx, proj.ID, app.ID, pf.Model.ID, pfName, oldPfConfig); err != nil {
 			return sdk.WrapError(err, "postApplicationDeploymentStrategyConfigHandler")
 		}
 

diff --git a/engine/api/application_import_test.go b/engine/api/application_import_test.go
@@ -1,6 +1,7 @@
 package api
 
 import (
+	"bytes"
 	"io/ioutil"
 	"net/http/httptest"
 	"strings"
@@ -716,5 +717,107 @@ func Test_postApplicationImportHandler_ExistingAppWithDeploymentStrategy(t *test
 	test.NoError(t, err)
 	assert.Equal(t, "my-secret-token-2", actualApp.DeploymentStrategies[pfname]["token"].Value)
 	assert.Equal(t, "my-url-3", actualApp.DeploymentStrategies[pfname]["url"].Value)
+}
+
+func Test_postApplicationImportHandler_DontOverrideDeploymentPasswordIfNotGiven(t *testing.T) {
+	// init test case, create a project with deployment integration then an application with deployment config
+	api, db, _, end := newTestAPI(t)
+	defer end()
+
+	u, pass := assets.InsertAdminUser(db)
+	proj := assets.InsertTestProject(t, db, api.Cache, sdk.RandomString(10), sdk.RandomString(10), u)
+	test.NotNil(t, proj)
+
+	pfname := sdk.RandomString(10)
+	pf := sdk.IntegrationModel{
+		Name:       pfname,
+		Deployment: true,
+		DeploymentDefaultConfig: sdk.IntegrationConfig{
+			"token": sdk.IntegrationConfigValue{
+				Type:  sdk.IntegrationConfigTypePassword,
+				Value: "my-secret-token",
+			},
+			"url": sdk.IntegrationConfigValue{
+				Type:  sdk.IntegrationConfigTypeString,
+				Value: "my-url",
+			},
+		},
+	}
+	test.NoError(t, integration.InsertModel(db, &pf))
+	defer func() { _ = integration.DeleteModel(db, pf.ID) }()
 
+	pp := sdk.ProjectIntegration{
+		Model:              pf,
+		Name:               pf.Name,
+		IntegrationModelID: pf.ID,
+		ProjectID:          proj.ID,
+	}
+	test.NoError(t, integration.InsertIntegration(db, &pp))
+
+	app := sdk.Application{
+		Name: "myNewApp",
+	}
+	test.NoError(t, application.Insert(db, api.Cache, proj, &app, u))
+
+	test.NoError(t, application.SetDeploymentStrategy(db, proj.ID, app.ID, pf.ID, pp.Name, sdk.IntegrationConfig{
+		"token": sdk.IntegrationConfigValue{
+			Type:  sdk.IntegrationConfigTypePassword,
+			Value: "my-secret-token-2",
+		},
+		"url": sdk.IntegrationConfigValue{
+			Type:  sdk.IntegrationConfigTypeString,
+			Value: "my-url",
+		},
+	}))
+
+	// import updated application without deployment token
+
+	appUpdated := exportentities.Application{
+		Name: "myNewApp",
+		DeploymentStrategies: map[string]map[string]exportentities.VariableValue{
+			pp.Name: {
+				"url": exportentities.VariableValue{
+					Type:  sdk.IntegrationConfigTypeString,
+					Value: "my-url-2",
+				},
+			},
+		},
+	}
+
+	uri := api.Router.GetRoute("POST", api.postApplicationImportHandler, map[string]string{
+		"permProjectKey": proj.Key,
+	})
+	test.NotEmpty(t, uri)
+	req := assets.NewAuthentifiedRequest(t, u, pass, "POST", uri+"?force=true", nil)
+
+	buf, err := yaml.Marshal(appUpdated)
+	test.NoError(t, err)
+	req.Body = ioutil.NopCloser(bytes.NewReader(buf))
+	req.Header.Set("Content-Type", "application/x-yaml")
+
+	rec := httptest.NewRecorder()
+	api.Router.Mux.ServeHTTP(rec, req)
+	assert.Equal(t, 200, rec.Code)
+
+	t.Logf(">>%s", rec.Body.String())
+
+	// check that the token is still present in the application
+
+	uri = api.Router.GetRoute("GET", api.getApplicationExportHandler, map[string]string{
+		"key":                 proj.Key,
+		"permApplicationName": app.Name,
+	})
+	test.NotEmpty(t, uri)
+	req = assets.NewAuthentifiedRequest(t, u, pass, "GET", uri, nil)
+
+	rec = httptest.NewRecorder()
+	api.Router.Mux.ServeHTTP(rec, req)
+	assert.Equal(t, 200, rec.Code)
+
+	t.Logf(">>%s", rec.Body.String())
+
+	actualApp, err := application.LoadByName(api.mustDB(), api.Cache, proj.Key, app.Name, u, application.LoadOptions.WithClearDeploymentStrategies)
+	test.NoError(t, err)
+	assert.Equal(t, "my-secret-token-2", actualApp.DeploymentStrategies[pfname]["token"].Value)
+	assert.Equal(t, "my-url-2", actualApp.DeploymentStrategies[pfname]["url"].Value)
 }
diff --git a/sdk/integration.go b/sdk/integration.go
@@ -162,7 +162,7 @@ func (pf *ProjectIntegration) HideSecrets() {
 	pf.Model.DeploymentDefaultConfig.HideSecrets()
 }
 
-// MergeWith merge two config
+// MergeWith set new values from new config and update existing values if not default.
 func (config IntegrationConfig) MergeWith(cfg IntegrationConfig) {
 	for k, v := range cfg {
 		val, has := config[k]