deploy: ApplyParams: rewrite with tempfile (opendatahub-io#1178)

* deploy: ApplyParams: factor out params.env parsing

Mostly refactoring, just factor out parsing code, but it closes the
file after parsing.

Signed-off-by: Yauheni Kaliuta <[email protected]>

* deploy: ApplyParams: rewrite with tempfile

Use traditional UNIX pattern of writing to a temporary file and
renaming it to the original one instead of modifying inplace.
Rename operation is atomic.

It also fixes previous code for handling unclosed files on the
filesystem (it used defer to close them on exit of the function).

Addionally add comment in case of value replaced with environment.

Signed-off-by: Yauheni Kaliuta <[email protected]>

---------

Signed-off-by: Yauheni Kaliuta <[email protected]>
(cherry picked from commit e8a09fa)

pkg/deploy/envParams.go

@@ -8,6 +8,51 @@ import (
 	"strings"
 )
 
+func parseParams(fileName string) (map[string]string, error) {
+	paramsEnv, err := os.Open(fileName)
+	if err != nil {
+		return nil, err
+	}
+	defer paramsEnv.Close()
+
+	paramsEnvMap := make(map[string]string)
+	scanner := bufio.NewScanner(paramsEnv)
+	for scanner.Scan() {
+		line := scanner.Text()
+		parts := strings.SplitN(line, "=", 2)
+		if len(parts) == 2 {
+			paramsEnvMap[parts[0]] = parts[1]
+		}
+	}
+	if err := scanner.Err(); err != nil {
+		return nil, err
+	}
+
+	return paramsEnvMap, nil
+}
+
+func writeParamsToTmp(params map[string]string, tmpDir string) (string, error) {
+	tmp, err := os.CreateTemp(tmpDir, "params.env-")
+	if err != nil {
+		return "", err
+	}
+	defer tmp.Close()
+
+	// Write the new map to temporary file
+	writer := bufio.NewWriter(tmp)
+	for key, value := range params {
+		if _, err := fmt.Fprintf(writer, "%s=%s\n", key, value); err != nil {
+			return "", err
+		}
+	}
+	if err := writer.Flush(); err != nil {
+		fmt.Printf("Failed to write to file: %v", err)
+		return "", err
+	}
+
+	return tmp.Name(), nil
+}
+
 /*
 overwrite values in components' manifests params.env file
 This is useful for air gapped cluster
@@ -20,7 +65,8 @@ extraParamsMaps is used to set extra parameters which are not carried from ENV v
 func ApplyParams(componentPath string, imageParamsMap map[string]string, extraParamsMaps ...map[string]string) error {
 	paramsFile := filepath.Join(componentPath, "params.env")
 	// Require params.env at the root folder
-	paramsEnv, err := os.Open(paramsFile)
+
+	paramsEnvMap, err := parseParams(paramsFile)
 	if err != nil {
 		if os.IsNotExist(err) {
 			// params.env doesn't exist, do not apply any changes
@@ -29,21 +75,6 @@ func ApplyParams(componentPath string, imageParamsMap map[string]string, extraPa
 		return err
 	}
 
-	defer paramsEnv.Close()
-
-	paramsEnvMap := make(map[string]string)
-	scanner := bufio.NewScanner(paramsEnv)
-	for scanner.Scan() {
-		line := scanner.Text()
-		parts := strings.SplitN(line, "=", 2)
-		if len(parts) == 2 {
-			paramsEnvMap[parts[0]] = parts[1]
-		}
-	}
-	if err := scanner.Err(); err != nil {
-		return err
-	}
-
 	// 1. Update images with env variables
 	// e.g "odh-kuberay-operator-controller-image": "RELATED_IMAGE_ODH_KUBERAY_OPERATOR_CONTROLLER_IMAGE",
 	for i := range paramsEnvMap {
@@ -60,41 +91,14 @@ func ApplyParams(componentPath string, imageParamsMap map[string]string, extraPa
 		}
 	}
 
-	// Move the existing file to a backup file and create empty file
-	paramsBackupFile := paramsFile + ".bak"
-	if err := os.Rename(paramsFile, paramsBackupFile); err != nil {
-		return err
-	}
-
-	file, err := os.Create(paramsFile)
+	tmp, err := writeParamsToTmp(paramsEnvMap, componentPath)
 	if err != nil {
-		// If create fails, try to restore the backup file
-		_ = os.Rename(paramsBackupFile, paramsFile)
-		return err
-	}
-	defer file.Close()
-
-	// Now, write the new map back to params.env
-	writer := bufio.NewWriter(file)
-	for key, value := range paramsEnvMap {
-		if _, fErr := fmt.Fprintf(writer, "%s=%s\n", key, value); fErr != nil {
-			return fErr
-		}
-	}
-	if err := writer.Flush(); err != nil {
-		if removeErr := os.Remove(paramsFile); removeErr != nil {
-			fmt.Printf("Failed to remove file: %v", removeErr)
-		}
-		if renameErr := os.Rename(paramsBackupFile, paramsFile); renameErr != nil {
-			fmt.Printf("Failed to restore file from backup: %v", renameErr)
-		}
-		fmt.Printf("Failed to write to file: %v", err)
 		return err
 	}
 
-	// cleanup backup file params.env.bak
-	if err := os.Remove(paramsBackupFile); err != nil {
-		fmt.Printf("Failed to remove backup file: %v", err)
+	if err = os.Rename(tmp, paramsFile); err != nil {
+		fmt.Printf("Failed rename %s to %s\n", tmp, paramsFile)
+		_ = os.Remove(tmp)
 		return err
 	}