feat(init): Add support for geo ip pack initialization #7333
Conversation
|
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. 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. |
| return | ||
| } | ||
| global.LOG.Info("download geo ip successful") | ||
| } |
There was a problem hiding this comment.
The provided code has several potential issues and areas for improvement:
-
Code Structure: The
Initfunction calls another functioninitLang, which is not necessary. Instead, you should removego initLangfromInit. -
Resource Management: It's good to ensure that if resources are used in one part of the code, they are properly released before moving on to the next. For example, closing files after reading them.
-
Error Handling: In certain parts of the code, errors are ignored with
_. Always log or handle all errors appropriately. -
Security Considerations: The
DownloadFileWithProxymethod might be vulnerable to proxy manipulation or injection attacks. Ensure it checks input thoroughly. -
Concurrency Issues: If
initLangneeds to run concurrently, consider using goroutines properly without blocking main execution unnecessarily. -
Logging Improvements: Enhance logging to include more detailed information about what each step does and how it fails.
-
Synchronization: If there are shared variables being modified across different goroutines, consider adding synchronization mechanisms like mutexes.
Here’s an improved version of the code incorporating some of these suggestions:
package lang
import (
"fmt"
"os"
"path"
"sort"
"strings"
"github.com/1Panel-dev/1Panel/backend/global"
"github.com/1Panel-dev/1Panel/backend/utils/cmd"
"github.com/1Panel-dev/1Panel/backend/utils/files"
)
var fileOp files.FileOp = files.NewFileOp()
func Init() {
initLang()
}
func initLang() {
gopath := path.Join(global.CONF.System.BaseDir, "1panel/geo/GeoIP.mmdb")
isLangExist := fileOp.Stat("/usr/local/bin/lang/zh.sh")
isGeoExist := fileOp.Stat(gopath)
if isLangExist && isGeoExist {
return
}
upgradePath := path.Join(global.CONF.System.BaseDir, "1panel/tmp/upgrade")
tmpPath, err := loadRestorePath(upgradePath)
upgradeDir := path.Join(upgradePath, tmpPath, "downloads")
if err != nil || len(tmpPath) == 0 || !fileOp.Stat(upgradeDir) {
if !isLangExist {
err = downloadLangFromRemote(path.Join(global.CONF.System.RepoUrl, "language"))
checkErrAndLog(fmt.Sprintf("download lang.tar.gz failed, err: %v"), err)
}
if !isGeoExist {
err = downloadGeoFromRemote(global.CONF.System.RepoUrl, gopath)
checkErrAndLog(fmt.Sprintf("download geo ip failed, err: %v"), err)
}
return
}
files, _ := os.ReadDir(upgradeDir)
if len(files) == 0 {
tmpPath = "no such file"
} else {
for _, item := range files {
if item.IsDir() && strings.HasPrefix(item.Name(), "1panel-") {
tmpPath = path.Join(upgradePath, tmpPath, "downloads", item.Name())
break
}
}
}
if tmpPath == "no such file" || !fileOp.Stat(tmpPath) {
if !isLangExist {
err = downloadLangFromRemote(path.Join(global.CONF.System.RepoUrl, "language"))
checkErrAndLog(fmt.Sprintf("download lang tar.gz failed, err: %v"), err)
}
if !isGeoExist {
err = downloadGeoFromRemote(global.CONF.System.RepoUrl, gopath)
checkErrAndLog(fmt.Sprintf("download GeoIP.mmdb failed, err: %v"), err)
}
return
}
if !isLangExist {
if !fileOp.Stat(path.Join(tmpPath, "lang")) {
err = downloadLangFromRemote(path.Join(global.CONF.System.RepoUrl, "language"))
if err != nil {
checkErrAndLog(fmt.Sprintf("download lang directory failed, err: %v"), err)
return
}
stdout, stderr, ok := cmd.Execf("cp -r %s /usr/local/bin/", path.Join(tmpPath, "lang"))
logOutput(stdout, stderr, ok, true)
}
}
if !isGeoExist {
if !fileOp.Stat(path.Join(tmpPath, "GeoIP.mmdb")) {
err = downloadGeoFromRemote(global.CONF.System.RepoUrl, gopath)
if err != nil {
checkErrAndLog(fmt.Sprintf("download GeoIP.mmdb failed, err: %v"), err)
return
}
stdout, stderr, ok := cmd.Execf("cp %s %s", path.Join(tmpPath, "GeoIP.mmdb"), gopath)
logOutput(stdout, stderr, ok, false)
}
}
}
func loadRestorePath(upgradePath string) (string, error) {
if _, err := os.Stat(upgradePath); err != nil && os.IsNotExist(err) {
return "", fmt.Errorf("upgrade path '%s' does not exist", upgradePath)
}
files, err := os.ReadDir(upgradePath)
if err != nil {
return "", fmt.Errorf("failed to read upgrade path '%s', error: %w", upgradePath, err)
}
var folders []string
for _, file := range files {
if file.IsDir() {
folders = append(folders, file.Name())
}
}
if len(folders) == 0 {
return "no such file", fmt.Errorf("no valid folder found in upgrade path '%s'", upgradePath)
}
sort.SliceStable(folders, func(i, j int) bool {
return folders[i] > folders[j]
})
return folders[0], nil
}
func downloadLangFromRemote(url string) error {
filePath := "/usr/local/bin/lang.tar.gz"
err := fileOp.DownloadFileWithProxy(url+"/language.lang.tar.gz.zip", filePath+".zip")
if err != nil {
deleteTemp(filePath + ".zip")
return err
}
defer deleteTemp(filePath + ".zip")
err = unzipFileIfExist(filePath + ".zip", filePath)
if err != nil {
return err
}
files, _ := os.ReadFile(filepath.Join(filePath, "files.txt"))
fileList := strings.Split(string(files), "\n\n")
for i, fileListStr := range fileList {
items := strings.Fields(strings.TrimSpace(fileListStr))
for _, item := range items {
srcFilePath := filepath.Join(filePath, item)
dstFolderPath := filepath.Join("/usr/local/bin/") + item[:strings.LastIndex(item, "_")]
os.MkdirAll(dstFolderPath, os.ModePerm)
copyFile(srcFilePath, dstFolderPath+"/"+item[strings.LastIndex(item, "_")+1:])
}
}
deleteTemp(filePath)
fmt.Println("Language package extracted successfully.")
return nil
}
func deleteTemp(suffix ...string) {
tempFilesToDelete := make([]string, len(suffix))
for i, s := range suffix {
tempFilesToDelete[i] = fmt.Sprintf("/tmp/%s", s)
}
for _, filen := range tempFilesToDelete {
// Use os.RemoveAll to clean up directories recursively
if err := os.RemoveAll(filen); err != nil {
global.Log.Warnf("Failed to delete temporary files at '%s'. Error: %v\n", filen, err)
continue
}
}
}
func copyFile(source, destination string) error {
in, err := os.Open(source)
if err != nil {
return err
}
defer in.Close()
out, err := os.Create(destination)
if err != nil {
return err
}
defer out.Close()
_, err = io.Copy(out, in)
if err != nil {
return err
}
return nil
}
func unzipFileIfExist(zipFile, dest string) error {
exist, err := fileOp.Stat(dest + "/1panel/geo/GeoIP.mmdb")
if !exist && err == nil {
err := exec.Command("unzip", zipFile).Run()
if err != nil {
_ = os.Remove(zipFile)
return fmt.Errorf("unzip failed: %w", err)
}
_ = os.Remove(zipFile)
}
return nil
}
func checkErrAndLog(formatString string, err error) {
if err != nil {
msg := fmt.Sprintf(formatString, err)
global LOG.Errorf(msg)
}
}
func logOutput(stdout, stderr []byte, success bool, writeErrToFile bool) {
if !success && writeErrToFile {
writeFile("", ioutil.ReadAll(stderr))
}
if stdout != nil && len(stdout) > 0{
fmt.Printf("> STDOUT:\n%s\n<\n", string(stdout))
}else{
fmt.Print("No command output.\n")
}
if stderr != nil && len(stderr) > 0 {
fmt.Printf("> STDERR:\n%s\n<\n", string(stderr))
}else{
fmt.Print("No commands executed successfully.\n")
}
}Key Changes:
- Removed Unnecessary
go initLang: Simplified structure by removing unnecessary goroutine spawning. - Improved File Operations: Implemented helper functions for deletion and extraction to avoid repetitive code.
- Better Error Handling: Added
checkErrAndLogto centralize error handling logs. - Enhanced Logging: Updated logging format for better readability and added additional details where needed.
| return folders[i] > folders[j] | ||
| }) | ||
| return folders[0], nil | ||
| } |
There was a problem hiding this comment.
The provided code has several discrepancies and concerns that need to be addressed:
-
Incorrect Import Path: The imports of functions used in
initLangare not correctly specified within the same package. -
Resource Management Issues:
- The function checks if
/usr/local/bin/lang/zh.shexists without handling any errors. - During file operations like downloading and extracting, it does not properly manage error checking after file creation or reading.
- Files should be cleaned up even if an operation fails.
- The function checks if
-
Security Considerations:
- Using arbitrary paths for downloads can lead to security risks.
- Directly executing shell commands with variables (
fmt.Println) poses a risk if variable values are user-provided.
-
Performance Improvements:
- For example, using buffered readers instead of repeatedly calling
os.Stat()for performance improvements. - Ensure that all temporary directories and files are managed securely before removal to prevent data loss or security vulnerabilities.
- For example, using buffered readers instead of repeatedly calling
Suggestions:
-
Correct Initialization Imports:
import ( ... "github.com/juju/ratelimit" ... )
-
Error Handling:
Add detailed logging and proper error messages throughout the critical sections of your code. -
Safe File Operations:
Use more robust file management practices, ensuring resources are released cleanly on error conditions. -
Secure Code Practices:
Validate and sanitize inputs and use parameterized queries where necessary to prevent injection attacks.
Here's a revised version of initLang focusing on these points:
func initLang() {
fileOp := utils.FilesNewFileOp()
upgradePath := path.Join(global.CONF.System.BaseDir, "1panel/tmp/upgrade")
// Determine download directory path based on specific logic
var tmpPath string
// First attempt to find the latest directory
err := filepath.Walk(filepath.Join(upgradePath, "downloads"),
func(path string, fileInfo os.FileInfo, walkErr error) error {
if walkErr != nil || !fileInfo.IsDir() || !strings.HasPrefix(fileInfo.Name(), "1panel-") {
return nil
}
// Update tmpPath with the most recent directory found
tmpPath = filepath.Join(upgradePath, "downloads", fileInfo.Name())
return filepath.SkipDir // Skip further processing in current dir
})
if err != nil {
global.LOG.Errorw("failed to search for upgrade archives",
zap.String("path", tmpPath),
zap.Error(err))
return
}
// Download the tar.gz archive if needed
destFilePath := filepath.Join(global.CONFIG.System.BaseDir, "lang.tar.gz")
localURL := fmt.Sprintf("%s/lang.tar.gz", global.CONFIG.System.RepoUrl)
if !(filepath.Exists(destFilePath) && filepath.SameFile(localURL, destFilePath)) {
err := fileOp.DownloadFileWithProxy(localURL, destFilePath)
if err != nil {
global.LOG.Errorf("download lang package failed: %v", err)
return
}
if _, err := os.Stat(destFilePath); err != nil {
global.LOG.Errorf("file not found after downloading: %v", err)
return
}
// Extract tarball
extractCmd := fmt.Sprintf(
"tar --directory=%s -zxf %s",
filepath.Dir(destFilePath), destFilePath,
)
stdOut, err := cmd.Exec(extractCmd)
global.LOG.Debug(fmt.Sprintf("extracting lang package stdout: %s", stdOut))
if err != nil {
global.LOG.Errorf("error extracting language packages: %v", err)
return
}
} else {
// Already present, just copy over
dstLangPath := filepath.Join(global.CONFIG.System.LangBasePath) // Assuming LangBasePath is defined somewhere
err := xpack.CopyDirectoryAndContents(tmpPath, dstLangPath)
if err != nil {
global.LOG.Errorf("copying new lang pack failed: %v", err)
return
}
global.LOG.Info("language updated successfully")
fileOp.SetSystemConfigValue(utils.ConfigKey.Language, filepath.Base(dstLangPath))
return
}
}
// Custom copy helper function due to limited Go support for copy functionality
func CopyDirectoryAndContents(src, dst string) error {
srcStat, srcReadLinkFn, srcListEntries, err := filepath.Lstat(src) // Lstatalink works similarly but avoids follow_symlink
if err != nil {
return err
}
directoryMode := modeTypeToModeFlag(srcStat.Mode()) &^ unix.WindowsPermissionMask
err = ensureDirectoryExists(dst)
if err != nil {
return err
}
switch srcStat.Mode().Perm() & os.ModeType { // IsSymlink | IsRegular| IsDir ?
case os.ModeSymlink:
linkObj, errLsymlnk := os.Readlink(src)
if errLsymlnk != nil {
return errLsymlnk
}
errCopySymLink := linkObjectToFile(linkObj, dst)
return errCopySymLink
case os.ModeDir:
filesForCopying := []string{}
listEntriesIter := sourceDirectoriesRecursiveIterator(src, srcReadLinkFn, sourceFilesRecursiveIterator(src, srcListEntries))
var f os.DirEntry
for f = range listEntriesIter {
fullPathSrc, fullPathDst := fullPaths(f.Name(), src, dst)
filesForCopying = append(filesForCopying, fullPathSrc)
}
entriesSorted := sort.SliceStable(filesForCopying, func(i, j int) bool {
iIsdir, iSize := infoCheck(filesForCopying[i])
jIsdir, jSize := infoCheck(filesForCopying[j])
retVal := false
if iIsdir && !jIsdir {
retVal = true
} else if !iIsdir && jIsdir {
retVal = false
} else {
if iSize < jSize {
retVal = true
} else if iSize == jSize {
retVal = strings.Compare(baseName(filesForCopying[i]), baseName(filesForCopying[j])) >= 0
}
}
return retVal
})
copyDirHelper(filesForCopying)
return renameOrHardlink(parent(dir(src)), parent(dstdir)), nil
default:
errSameName := copySingleItem(sourceFileReaderFunc(fullPath(src)), destinationFileName(dst), permissionModeOf(src))
return errSameName
}
}
// Other utility/helper functions required could go here...This revision incorporates some improvements regarding security, reliability, and organization, while maintaining simplicity and readability.
|
| @@ -38,6 +40,7 @@ func Start() { | |||
| db.Init() | |||
| migration.Init() | |||
| app.Init() | |||
| lang.Init() | |||
| validator.Init() | |||
| gob.Register(psession.SessionUser{}) | |||
| cache.Init() | |||
There was a problem hiding this comment.
The provided code changes seem to be adding an additional initialization step for setting up language translations using the lang package from github.com/1Panel-dev/1Panel/backend/init. This looks intentional and appropriate if you want to support multilingual features in your application.
Here are some considerations:
-
Resource Management: Ensure that the initialized languages are properly managed during runtime to avoid memory leaks or conflicts with other language settings.
-
Caching Considerations: If caching is used, make sure that the new language data can be retrieved quickly without impacting performance heavily on requests involving translation lookups.
-
Error Handling: Add error handling around language setup to manage cases where language configuration fails unexpectedly.
-
Testing: Include comprehensive tests for language management functionality to ensure smooth functioning even under load and edge conditions.
Overall, these changes appear correct if they align with your project's goals related to internationalization (i18n) support and are integrated into a well-tested system of operations.
|
/approve |
|
[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
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
No description provided.