fix(api): default env vars should always be merged for docker model (#…

…5293)
ovh · Jul 3, 2020 · 1c64809 · 1c64809
1 parent d177c8f
commit 1c64809
Show file tree

Hide file tree

Showing 3 changed files with 36 additions and 9 deletions.
diff --git a/engine/api/workermodel/dao.go b/engine/api/workermodel/dao.go
@@ -199,6 +199,8 @@ func Insert(ctx context.Context, db gorp.SqlExecutor, model *sdk.Model) error {
 	dbmodel.UserLastModified = time.Now()
 	dbmodel.NeedRegistration = true
 
+	mergeModelEnvsWithDefaultEnvs(&dbmodel)
+
 	needSaveRegistryPassword, dockerRegistryPassword, err := replaceDockerRegistryPassword(db, &dbmodel)
 	if err != nil {
 		return err
@@ -232,6 +234,8 @@ func UpdateDB(ctx context.Context, db gorp.SqlExecutor, model *sdk.Model) error
 		return err
 	}
 
+	mergeModelEnvsWithDefaultEnvs(&dbmodel)
+
 	needSaveRegistryPassword, dockerRegistryPassword, err := replaceDockerRegistryPassword(db, &dbmodel)
 	if err != nil {
 		return err

diff --git a/engine/api/workermodel/dao_test.go b/engine/api/workermodel/dao_test.go
@@ -135,6 +135,29 @@ func TestInsert(t *testing.T) {
 	assert.EqualValues(t, *src, *res)
 }
 
+func TestMergeModelEnvsWithDefaultEnvs(t *testing.T) {
+	db, _ := test.SetupPG(t, bootstrap.InitiliazeDB)
+
+	g := assets.InsertGroup(t, db)
+
+	m := sdk.Model{
+		Name: sdk.RandomString(10),
+		Type: sdk.Docker,
+		ModelDocker: sdk.ModelDocker{
+			Image: "foo/bar:3.4",
+		},
+		GroupID: g.ID,
+	}
+	require.NoError(t, workermodel.Insert(context.TODO(), db, &m))
+	require.Len(t, m.ModelDocker.Envs, 6, "all default vars should be added by insert")
+
+	m.ModelDocker.Envs = map[string]string{
+		"myvar": "myvalue",
+	}
+	require.NoError(t, workermodel.UpdateDB(context.TODO(), db, &m))
+	require.Len(t, m.ModelDocker.Envs, 7, "all default vars should be merged to given vars by update")
+}
+
 func TestLoadByNameAndGroupID(t *testing.T) {
 	db, _ := test.SetupPG(t, bootstrap.InitiliazeDB)
 

diff --git a/engine/api/workermodel/utils.go b/engine/api/workermodel/utils.go
@@ -16,17 +16,19 @@ var defaultEnvs = map[string]string{
 	"CDS_GRAYLOG_EXTRA_VALUE": "{{.GraylogExtraValue}}",
 }
 
-func MergeModelEnvsWithDefaultEnvs(envs map[string]string) map[string]string {
-	if envs == nil {
-		return defaultEnvs
+func mergeModelEnvsWithDefaultEnvs(m *workerModel) {
+	if m.Type != sdk.Docker {
+		return
+	}
+
+	if m.ModelDocker.Envs == nil {
+		m.ModelDocker.Envs = make(map[string]string)
 	}
 	for envName := range defaultEnvs {
-		if _, ok := envs[envName]; !ok {
-			envs[envName] = defaultEnvs[envName]
+		if _, ok := m.ModelDocker.Envs[envName]; !ok {
+			m.ModelDocker.Envs[envName] = defaultEnvs[envName]
 		}
 	}
-
-	return envs
 }
 
 const registryPasswordSecretName = "secrets.registry_password"
@@ -48,8 +50,6 @@ func replaceDockerRegistryPassword(db gorp.SqlExecutor, dbmodel *workerModel) (b
 		return false, "", nil
 	}
 
-	dbmodel.ModelDocker.Envs = MergeModelEnvsWithDefaultEnvs(dbmodel.ModelDocker.Envs)
-
 	// Password not changed
 	if dbmodel.ModelDocker.Password == "{{."+registryPasswordSecretName+"}}" {
 		return false, "", nil