fix: improve import + migrate variables components to OnPush strategy (…

…#4374)

engine/api/application.go

@@ -375,7 +375,7 @@ func cloneApplication(db gorp.SqlExecutor, store cache.Store, proj *sdk.Project,
 		if v.Type == sdk.KeyVariable {
 			errVar = application.AddKeyPairToApplication(db, store, newApp, v.Name, u)
 		} else {
-			errVar = application.InsertVariable(db, store, newApp, v, u)
+			errVar = application.InsertVariable(db, store, newApp, &v, u)
 		}
 		if errVar != nil {
 			return errVar

engine/api/application/application_importer.go

@@ -117,7 +117,7 @@ func importVariables(db gorp.SqlExecutor, store cache.Store, proj *sdk.Project,
 			errCreate = AddKeyPairToApplication(db, store, app, newVar.Name, u)
 			break
 		default:
-			errCreate = InsertVariable(db, store, app, newVar, u)
+			errCreate = InsertVariable(db, store, app, &newVar, u)
 			break
 		}
 		if errCreate != nil {

engine/api/application/application_variable.go

@@ -10,7 +10,6 @@ import (
 	"github.com/go-gorp/gorp"
 
 	"github.com/ovh/cds/engine/api/cache"
-	"github.com/ovh/cds/engine/api/event"
 	"github.com/ovh/cds/engine/api/keys"
 	"github.com/ovh/cds/engine/api/secret"
 	"github.com/ovh/cds/sdk"
@@ -204,7 +203,7 @@ func GetAllVariableByID(db gorp.SqlExecutor, applicationID int64, fargs ...FuncA
 }
 
 // InsertVariable Insert a new variable in the given application
-func InsertVariable(db gorp.SqlExecutor, store cache.Store, app *sdk.Application, variable sdk.Variable, u *sdk.User) error {
+func InsertVariable(db gorp.SqlExecutor, store cache.Store, app *sdk.Application, variable *sdk.Variable, u *sdk.User) error {
 	//Check variable name
 	rx := sdk.NamePatternRegex
 	if !rx.MatchString(variable.Name) {
@@ -233,16 +232,14 @@ func InsertVariable(db gorp.SqlExecutor, store cache.Store, app *sdk.Application
 		ApplicationID: app.ID,
 		Type:          sdk.AuditAdd,
 		Author:        u.Username,
-		VariableAfter: &variable,
+		VariableAfter: variable,
 		VariableID:    variable.ID,
 		Versionned:    time.Now(),
 	}
 
 	if err := inserAudit(db, ava); err != nil {
 		return sdk.WrapError(err, "Cannot insert audit for variable %d", variable.ID)
 	}
-	event.PublishAddVariableApplication(app.ProjectKey, *app, variable, u)
-
 	return nil
 }
 
@@ -288,8 +285,6 @@ func UpdateVariable(db gorp.SqlExecutor, store cache.Store, app *sdk.Application
 	if err := inserAudit(db, ava); err != nil {
 		return sdk.WrapError(err, "Cannot insert audit for variable %s", variable.Name)
 	}
-	event.PublishUpdateVariableApplication(app.ProjectKey, *app, *variable, *variableBefore, u)
-
 	return nil
 }
 
@@ -322,8 +317,6 @@ func DeleteVariable(db gorp.SqlExecutor, store cache.Store, app *sdk.Application
 	if err := inserAudit(db, ava); err != nil {
 		return sdk.WrapError(err, "Cannot insert audit for variable %s", variable.Name)
 	}
-	event.PublishDeleteVariableApplication(app.ProjectKey, *app, *variable, u)
-
 	return nil
 }
 
@@ -351,7 +344,7 @@ func AddKeyPairToApplication(db gorp.SqlExecutor, store cache.Store, app *sdk.Ap
 		Value: k.Private,
 	}
 
-	if err := InsertVariable(db, store, app, v, u); err != nil {
+	if err := InsertVariable(db, store, app, &v, u); err != nil {
 		return err
 	}
 
@@ -361,7 +354,7 @@ func AddKeyPairToApplication(db gorp.SqlExecutor, store cache.Store, app *sdk.Ap
 		Value: k.Public,
 	}
 
-	return InsertVariable(db, store, app, p, u)
+	return InsertVariable(db, store, app, &p, u)
 }
 
 // insertAudit  insert an application variable audit

engine/api/application_export_test.go

@@ -33,15 +33,15 @@ func Test_getApplicationExportHandler(t *testing.T) {
 		Type:  sdk.StringVariable,
 	}
 
-	test.NoError(t, application.InsertVariable(db, api.Cache, app, v1, u))
+	test.NoError(t, application.InsertVariable(db, api.Cache, app, &v1, u))
 
 	v2 := sdk.Variable{
 		Name:  "var2",
 		Value: "value 2",
 		Type:  sdk.SecretVariable,
 	}
 
-	test.NoError(t, application.InsertVariable(db, api.Cache, app, v2, u))
+	test.NoError(t, application.InsertVariable(db, api.Cache, app, &v2, u))
 
 	//Insert ssh and gpg keys
 	k := &sdk.ApplicationKey{

engine/api/application_import_test.go

@@ -121,7 +121,7 @@ func Test_postApplicationImportHandler_NewAppFromYAMLWithKeysAndSecrets(t *testi
 		t.Fatal(err)
 	}
 
-	test.NoError(t, application.InsertVariable(api.mustDB(), api.Cache, app, sdk.Variable{
+	test.NoError(t, application.InsertVariable(api.mustDB(), api.Cache, app, &sdk.Variable{
 		Name:  "myPassword",
 		Type:  sdk.SecretVariable,
 		Value: "MySecretValue",
@@ -232,7 +232,7 @@ func Test_postApplicationImportHandler_NewAppFromYAMLWithKeysAndSecretsAndReImpo
 		t.Fatal(err)
 	}
 
-	test.NoError(t, application.InsertVariable(api.mustDB(), api.Cache, app, sdk.Variable{
+	test.NoError(t, application.InsertVariable(api.mustDB(), api.Cache, app, &sdk.Variable{
 		Name:  "myPassword",
 		Type:  sdk.SecretVariable,
 		Value: "MySecretValue",
@@ -413,7 +413,7 @@ func Test_postApplicationImportHandler_NewAppFromYAMLWithKeysAndSecretsAndReImpo
 	k2.KeyID = kssh.KeyID
 	test.NoError(t, application.InsertKey(api.mustDB(), k2))
 
-	test.NoError(t, application.InsertVariable(api.mustDB(), api.Cache, app, sdk.Variable{
+	test.NoError(t, application.InsertVariable(api.mustDB(), api.Cache, app, &sdk.Variable{
 		Name:  "myPassword",
 		Type:  sdk.SecretVariable,
 		Value: "MySecretValue",

engine/api/application_variable.go

@@ -126,14 +126,9 @@ func (api *API) deleteVariableFromApplicationHandler() service.Handler {
 			return sdk.WrapError(err, "Cannot commit transaction")
 		}
 
-		app.Variable, err = application.GetAllVariableByID(api.mustDB(), app.ID)
-		if err != nil {
-			return sdk.WrapError(err, "Cannot load variables")
-		}
-
 		event.PublishDeleteVariableApplication(key, *app, *varToDelete, deprecatedGetUser(ctx))
 
-		return service.WriteJSON(w, app, http.StatusOK)
+		return service.WriteJSON(w, nil, http.StatusOK)
 	}
 }
 
@@ -179,14 +174,13 @@ func (api *API) updateVariableInApplicationHandler() service.Handler {
 			return sdk.WrapError(err, "Cannot commit transaction")
 		}
 
-		app.Variable, err = application.GetAllVariableByID(api.mustDB(), app.ID)
-		if err != nil {
-			return sdk.WrapError(err, "Cannot load variables")
-		}
-
 		event.PublishUpdateVariableApplication(key, *app, newVar, *variableBefore, deprecatedGetUser(ctx))
 
-		return service.WriteJSON(w, app, http.StatusOK)
+		if sdk.NeedPlaceholder(newVar.Type) {
+			newVar.Value = sdk.PasswordPlaceholder
+		}
+
+		return service.WriteJSON(w, newVar, http.StatusOK)
 	}
 }
 
@@ -225,7 +219,7 @@ func (api *API) addVariableInApplicationHandler() service.Handler {
 			err = application.AddKeyPairToApplication(tx, api.Cache, app, newVar.Name, deprecatedGetUser(ctx))
 			break
 		default:
-			err = application.InsertVariable(tx, api.Cache, app, newVar, deprecatedGetUser(ctx))
+			err = application.InsertVariable(tx, api.Cache, app, &newVar, deprecatedGetUser(ctx))
 			break
 		}
 		if err != nil {
@@ -236,13 +230,12 @@ func (api *API) addVariableInApplicationHandler() service.Handler {
 			return sdk.WrapError(err, "Cannot commit transaction")
 		}
 
-		app.Variable, err = application.GetAllVariableByID(api.mustDB(), app.ID)
-		if err != nil {
-			return sdk.WrapError(err, "Cannot get variables")
-		}
-
 		event.PublishAddVariableApplication(key, *app, newVar, deprecatedGetUser(ctx))
 
-		return service.WriteJSON(w, app, http.StatusOK)
+		if sdk.NeedPlaceholder(newVar.Type) {
+			newVar.Value = sdk.PasswordPlaceholder
+		}
+
+		return service.WriteJSON(w, newVar, http.StatusOK)
 	}
 }

engine/api/application_variable_test.go

@@ -38,7 +38,7 @@ func Test_getVariableAuditInApplicationHandler(t *testing.T) {
 		Type:  "string",
 		Value: "bar",
 	}
-	if err := application.InsertVariable(api.mustDB(), api.Cache, app, v, u); err != nil {
+	if err := application.InsertVariable(api.mustDB(), api.Cache, app, &v, u); err != nil {
 		t.Fatal(err)
 	}
 

engine/api/environment_variable.go

@@ -8,7 +8,6 @@ import (
 
 	"github.com/ovh/cds/engine/api/environment"
 	"github.com/ovh/cds/engine/api/event"
-	"github.com/ovh/cds/engine/api/project"
 	"github.com/ovh/cds/engine/service"
 	"github.com/ovh/cds/sdk"
 )
@@ -77,11 +76,6 @@ func (api *API) deleteVariableFromEnvironmentHandler() service.Handler {
 		envName := vars["environmentName"]
 		varName := vars["name"]
 
-		p, errProj := project.Load(api.mustDB(), api.Cache, key, deprecatedGetUser(ctx), project.LoadOptions.Default)
-		if errProj != nil {
-			return sdk.WrapError(errProj, "deleteVariableFromEnvironmentHandler: Cannot load project %s", key)
-		}
-
 		env, errEnv := environment.LoadEnvironmentByName(api.mustDB(), key, envName)
 		if errEnv != nil {
 			return sdk.WrapError(errEnv, "deleteVariableFromEnvironmentHandler: Cannot load environment %s", envName)
@@ -109,16 +103,9 @@ func (api *API) deleteVariableFromEnvironmentHandler() service.Handler {
 		if err := tx.Commit(); err != nil {
 			return sdk.WrapError(err, "deleteVariableFromEnvironmentHandler: Cannot commit transaction")
 		}
-
-		var errEnvs error
-		p.Environments, errEnvs = environment.LoadEnvironments(api.mustDB(), key, true, deprecatedGetUser(ctx))
-		if errEnvs != nil {
-			return sdk.WrapError(errEnvs, "deleteVariableFromEnvironmentHandler: Cannot load environments")
-		}
-
 		event.PublishEnvironmentVariableDelete(key, *env, *varToDelete, deprecatedGetUser(ctx))
 
-		return service.WriteJSON(w, p, http.StatusOK)
+		return service.WriteJSON(w, nil, http.StatusOK)
 	}
 }
 
@@ -129,11 +116,6 @@ func (api *API) updateVariableInEnvironmentHandler() service.Handler {
 		envName := vars["environmentName"]
 		varName := vars["name"]
 
-		p, errProj := project.Load(api.mustDB(), api.Cache, key, deprecatedGetUser(ctx), project.LoadOptions.Default)
-		if errProj != nil {
-			return sdk.WrapError(errProj, "updateVariableInEnvironment: Cannot load %s", key)
-		}
-
 		var newVar sdk.Variable
 		if err := service.UnmarshalBody(r, &newVar); err != nil {
 			return sdk.ErrWrongRequest
@@ -169,15 +151,13 @@ func (api *API) updateVariableInEnvironmentHandler() service.Handler {
 			return sdk.WrapError(err, "updateVariableInEnvironmentHandler: Cannot commit transaction")
 		}
 
-		var errEnvs error
-		p.Environments, errEnvs = environment.LoadEnvironments(api.mustDB(), key, true, deprecatedGetUser(ctx))
-		if errEnvs != nil {
-			return sdk.WrapError(errEnvs, "updateVariableInEnvironmentHandler: Cannot load environments")
-		}
-
 		event.PublishEnvironmentVariableUpdate(key, *env, newVar, varBefore, deprecatedGetUser(ctx))
 
-		return service.WriteJSON(w, p, http.StatusOK)
+		if sdk.NeedPlaceholder(newVar.Type) {
+			newVar.Value = sdk.PasswordPlaceholder
+		}
+
+		return service.WriteJSON(w, newVar, http.StatusOK)
 	}
 }
 
@@ -188,11 +168,6 @@ func (api *API) addVariableInEnvironmentHandler() service.Handler {
 		envName := vars["environmentName"]
 		varName := vars["name"]
 
-		p, errProj := project.Load(api.mustDB(), api.Cache, key, deprecatedGetUser(ctx), project.LoadOptions.Default)
-		if errProj != nil {
-			return sdk.WrapError(errProj, "addVariableInEnvironmentHandler: Cannot load project %s", key)
-		}
-
 		var newVar sdk.Variable
 		if err := service.UnmarshalBody(r, &newVar); err != nil {
 			return sdk.ErrWrongRequest
@@ -231,14 +206,12 @@ func (api *API) addVariableInEnvironmentHandler() service.Handler {
 			return sdk.WrapError(err, "addVariableInEnvironmentHandler: cannot commit tx")
 		}
 
-		var errEnvs error
-		p.Environments, errEnvs = environment.LoadEnvironments(api.mustDB(), key, true, deprecatedGetUser(ctx))
-		if errEnvs != nil {
-			return sdk.WrapError(errEnvs, "addVariableInEnvironmentHandler: Cannot load environments")
-		}
-
 		event.PublishEnvironmentVariableAdd(key, *env, newVar, deprecatedGetUser(ctx))
 
-		return service.WriteJSON(w, p, http.StatusOK)
+		if sdk.NeedPlaceholder(newVar.Type) {
+			newVar.Value = sdk.PasswordPlaceholder
+		}
+
+		return service.WriteJSON(w, newVar, http.StatusOK)
 	}
 }

engine/api/environment_variable_test.go

@@ -65,10 +65,9 @@ func TestAddVariableInEnvironmentHandler(t *testing.T) {
 
 	assert.Equal(t, 200, w.Code)
 	res, _ := ioutil.ReadAll(w.Body)
-	projectResult := &sdk.Project{}
-	json.Unmarshal(res, &projectResult)
-	assert.Equal(t, len(projectResult.Environments), 1)
-	assert.Equal(t, len(projectResult.Environments[0].Variable), 1)
+	v := &sdk.Variable{}
+	assert.NoError(t, json.Unmarshal(res, &v))
+	assert.NotEqual(t, v.ID, 0)
 
 	envDb, err := environment.LoadEnvironmentByName(api.mustDB(), proj.Key, "Prod")
 	if err != nil {
@@ -136,10 +135,9 @@ func TestUpdateVariableInEnvironmentHandler(t *testing.T) {
 
 	assert.Equal(t, 200, w.Code)
 	res, _ := ioutil.ReadAll(w.Body)
-	projectResult := &sdk.Project{}
-	json.Unmarshal(res, &projectResult)
-	assert.Equal(t, len(projectResult.Environments), 1)
-	assert.Equal(t, len(projectResult.Environments[0].Variable), 1)
+	vUpdated := sdk.Variable{}
+	assert.NoError(t, json.Unmarshal(res, &vUpdated))
+	assert.Equal(t, vUpdated.Value, "new bar")
 
 	envDb, err := environment.LoadEnvironmentByName(api.mustDB(), proj.Key, "Prod")
 	if err != nil {
@@ -200,11 +198,6 @@ func TestDeleteVariableFromEnvironmentHandler(t *testing.T) {
 	router.Mux.ServeHTTP(w, req)
 
 	assert.Equal(t, 200, w.Code)
-	res, _ := ioutil.ReadAll(w.Body)
-	projectResult := &sdk.Project{}
-	json.Unmarshal(res, &projectResult)
-	assert.Equal(t, len(projectResult.Environments), 1)
-	assert.Equal(t, len(projectResult.Environments[0].Variable), 0)
 
 	envDb, err := environment.LoadEnvironmentByName(api.mustDB(), proj.Key, "Prod")
 	if err != nil {

engine/api/project_variable.go

@@ -143,6 +143,10 @@ func (api *API) updateVariableInProjectHandler() service.Handler {
 
 		event.PublishUpdateProjectVariable(p, newVar, *previousVar, deprecatedGetUser(ctx))
 
+		if sdk.NeedPlaceholder(newVar.Type) {
+			newVar.Value = sdk.PasswordPlaceholder
+		}
+
 		return service.WriteJSON(w, newVar, http.StatusOK)
 	}
 }
@@ -203,13 +207,11 @@ func (api *API) addVariableInProjectHandler() service.Handler {
 		// Send Add variable event
 		event.PublishAddProjectVariable(p, newVar, deprecatedGetUser(ctx))
 
-		p.Variable, err = project.GetAllVariableInProject(api.mustDB(), p.ID)
-		if err != nil {
-			return sdk.WrapError(err, "AddVariableInProject: Cannot get variables")
-
+		if sdk.NeedPlaceholder(newVar.Type) {
+			newVar.Value = sdk.PasswordPlaceholder
 		}
 
-		return service.WriteJSON(w, p, http.StatusOK)
+		return service.WriteJSON(w, newVar, http.StatusOK)
 	}
 }
 

engine/api/warning/warning_project_variable_test.go

@@ -243,7 +243,7 @@ func TestMissingProjectVariableApplication(t *testing.T) {
 		Type:  "string",
 		Value: "foo{{.cds.proj." + v.Name + "}}bar",
 	}
-	test.NoError(t, application.InsertVariable(db, cache, &app, appV, u))
+	test.NoError(t, application.InsertVariable(db, cache, &app, &appV, u))
 
 	// Create Delete var
 	ePayloadDelete := sdk.EventProjectVariableDelete{