Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Fixed the fault in determining the firewall port forwarding version #7141

Merged
merged 1 commit into from
Nov 21, 2024
+4 −29
Merged

fix: Fixed the fault in determining the firewall port forwarding version #7141

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 0 additions & 22 deletions backend/app/service/firewall.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import (

"github.com/1Panel-dev/1Panel/backend/app/dto"
"github.com/1Panel-dev/1Panel/backend/app/model"
"github.com/1Panel-dev/1Panel/backend/buserr"
"github.com/1Panel-dev/1Panel/backend/constant"
"github.com/1Panel-dev/1Panel/backend/global"
"github.com/1Panel-dev/1Panel/backend/utils/cmd"
Expand Down Expand Up @@ -87,11 +86,6 @@ func (u *FirewallService) SearchWithPage(req dto.RuleSearch) (int64, interface{}
case "port":
rules, err = client.ListPort()
case "forward":
isSupport, errSup := checkIsSupport()
if !isSupport {
return 0, nil, errSup
}

rules, err = client.ListForward()
case "address":
rules, err = client.ListAddress()
Expand Down Expand Up @@ -312,11 +306,6 @@ func (u *FirewallService) OperatePortRule(req dto.PortRuleOperate, reload bool)
}

func (u *FirewallService) OperateForwardRule(req dto.ForwardRuleOperate) error {
isSupport, errSup := checkIsSupport()
if !isSupport {
return errSup
}

client, err := firewall.NewFirewallClient()
if err != nil {
return err
Expand Down Expand Up @@ -737,14 +726,3 @@ func checkPortUsed(ports, proto string, apps []portOfApp) string {
}
return ""
}

func checkIsSupport() (bool, error) {
std, err := cmd.Exec("iptables --version")
if err != nil {
return false, fmt.Errorf("handle iptables --version failed, stdout: %s, err: %v", std, err)
}
if strings.Contains(std, "nf_tables") {
return false, buserr.New(constant.ErrNFTables)
}
return true, nil
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

该代码没有明显的不规范问题和潜在的问题。优化建议：考虑到安全性和效率，可以考虑将防火墙客户端方法封装为一个单独的函数，并在主逻辑中进行调用来减少重复部分。

import (
  // ...
@@ -742,12 +735,5 @@
      const: constant,
      cmd: &cmd.Client{},
)

func (u FirewallService) CheckIsSupport() ([]string, error) {
	stdOut, err := cmd.Exec("iptables --version")
	if err != nil {
		return []string{}, fmt.Errorf("handle iptables --version failed, stdout: %s, err: %v", stdOut, err)
	}
	if strings.Contains(stdOut, "nf_tables") {
		return []string{"Error: NF tables support only", ErrNFTabsError}, nil
	}

	return nil, nil

1 change: 0 additions & 1 deletion backend/constant/errs.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,6 @@ var (
var (
ErrFirewallNone = "ErrFirewallNone"
ErrFirewallBoth = "ErrFirewallBoth"
ErrNFTables = "ErrNFTables"
)

// cronjob
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个代码没有明显的错误或缺失的内容，请确保代码中的变量和结构是完整的，并且符合语法规则。

然而，在当前的环境中，我无法提供特定于2021年到最近日期的所有细节关于该环境的技术状况，但是通常来说，对于维护稳定性和准确性，定期更新项目源码至最新版本是非常重要的，以保持与最新的语言特性和支持的变化相适应。

此外，如果有任何问题出现或发生，你可能需要更详细的上下文信息才能确定具体是什么问题。

Expand Down
1 change: 0 additions & 1 deletion backend/i18n/lang/en.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,6 @@ ErrUserFindErr: "Failed to find user {{ .name }} {{ .err }}"
#ssh
ErrFirewallNone: "No firewalld or ufw service detected on the system. Please check and try again!"
ErrFirewallBoth: "Both firewalld and ufw services are detected on the system. To avoid conflicts, please uninstall one and try again!"
ErrNFTables: "Port forwarding functionality relies on the iptables service and is currently not compatible with nftables operations!"

#cronjob
ErrBashExecute: "Script execution error, please check the specific information in the task output text area."
Expand Down
1 change: 0 additions & 1 deletion backend/i18n/lang/zh-Hant.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,6 @@ ErrUserFindErr: "用戶 {{ .name }} 查找失敗 {{ .err }}"
#ssh
ErrFirewallNone: "未檢測到系統 firewalld 或 ufw 服務，請檢查後重試！"
ErrFirewallBoth: "檢測到系統同時存在 firewalld 或 ufw 服務，為避免衝突，請卸載後重試！"
ErrNFTables: "端口轉發功能依賴於 iptables 服務，暫不兼容 nftables 操作！"

#cronjob
ErrBashExecute: "腳本執行錯誤，請在任務輸出文本域中查看具體信息。"
Expand Down
1 change: 0 additions & 1 deletion backend/i18n/lang/zh.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,6 @@ ErrUserFindErr: "用户 {{ .name }} 查找失败 {{ .err }}"
#ssh
ErrFirewallNone: "未检测到系统 firewalld 或 ufw 服务，请检查后重试！"
ErrFirewallBoth: "检测到系统同时存在 firewalld 或 ufw 服务，为避免冲突，请卸载后重试！"
ErrNFTables: "端口转发功能依赖于 iptables 服务，暂不兼容 nftables 操作！"

#cronjob
ErrBashExecute: "脚本执行错误，请在任务输出文本域中查看具体信息。"
Expand Down
7 changes: 4 additions & 3 deletions backend/utils/firewall/client/iptables.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,12 @@ package client

import (
"fmt"
"regexp"
"strings"

"github.com/1Panel-dev/1Panel/backend/app/model"
"github.com/1Panel-dev/1Panel/backend/global"
"github.com/1Panel-dev/1Panel/backend/utils/cmd"
"regexp"
"strings"
)

const NatChain = "1PANEL"
Expand Down Expand Up @@ -51,7 +52,7 @@ func (iptables *Iptables) Check() error {
}

func (iptables *Iptables) NatNewChain() error {
return iptables.runf("-N %s", NatChain)
return iptables.runf("-t nat -N %s", NatChain)
}

func (iptables *Iptables) NatAppendChain() error {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这段代码存在一些问题和建议：

  1. const 符号在不同包之间的使用是不一致的。
  2. 原始字符串处理方法应该更加明确，比如替换或截断等。
  3. 在错误回退函数中，“%s”没有被正确解释。

另外，由于我当前无法查看和运行具体代码（因为我是语言模型，需要输入文本以生成响应）。然而我可以提供以下一些建议：

  • 将 const 定义放在相应文件内，并与相应的模块保持同步；

    

    • 



    

  • const bool IsWindows;
    • 




- 对于原始字符串的格式化，在某些情况下可能更直观地理解它作为元字符。例如： 

```C
var ipAddStr = strings.Replace(ipAddress, "*", "", -1)



    

  • 针对“%s”的解释，这似乎用于拼接命令行参数到指令中的一部分。但是缺少实际使用场景和变量定义。如果你尝试在特定环境下应用此功能，请务必确保这些环境条件已经被考虑到了。
    • 

  • 运动性好的程序员通常会避免一次性执行多个命令，尤其是在开发环境中。
    • 

  • 变量声明最好集中在一个地方并在整个循环中引用，这样可以提高可读性和一致性。
    • 


    

  
  



    


        

      
  

    
  
    
    

  

  




        

      
    








    


  








  


    


  		



  

    
          
            
          
          Expand Down
    
    
  



                
              


          


    

  



  



    

  
    
      Loading