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 #7150

Merged
merged 1 commit into from
Nov 22, 2024
Merged

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

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
4 changes: 4 additions & 0 deletions backend/utils/firewall/client/firewalld.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (

"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 @@ -118,6 +119,9 @@ func (f *Firewall) ListPort() ([]FireInfo, error) {
}

func (f *Firewall) ListForward() ([]FireInfo, error) {
if err := f.EnableForward(); err != nil {
global.LOG.Errorf("init port forward failed, err: %v", err)
}
stdout, err := cmd.Exec("firewall-cmd --zone=public --list-forward-ports")
if err != nil {
return nil, err
Copy link

Choose a reason for hiding this comment

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

该段代码的中文描述为:

这是一个在Go语言中创建的应用程序，并使用了一个包来管理防火墙服务。其中包含一个名为"Firewall"的对象，它包含了两个不同的方法：ListPort和ListForward。

以下是这个代码的部分问题：

  1. 这个应用是单例还是多线程？如果没有指定，则默认应该是一个单例。
  2. 在ListPort()函数中没有返回值，在调用时会抛出panic错误；需要重写此函数使其返回结果。
  3. EnableForward(),这似乎是在启动或初始化端口转发的过程中的语句，但实际并没有实现，应考虑添加这段功能；
    同样的，应在所有地方提供正确的API签名（尤其是error参数）以保持一致性和健壮性。

总之，整体而言，由于缺少必要的注释以及一些关键部分的处理，这个应用程序存在很多待改进之处。

Expand Down
2 changes: 1 addition & 1 deletion backend/utils/firewall/client/iptables.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ func (iptables *Iptables) Check() error {
}

func (iptables *Iptables) NatNewChain() error {
return iptables.runf("-t nat -N %s", NatChain)
return iptables.runf("-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.

在检查代码时，没有发现明显错误或潜在问题。不过根据你的要求，我们可以将代码简化，并提供一些简洁化的实现方式：

func (iptables *Iptables) Check() error {
}
  
// NAT 新链定义和删除

对于nat新链添加与删除的操作，请使用 iptables runf 并直接调用其内部子函数进行操作。

这样可以避免不必要的重复声明以及代码冗余性。

Copy link

Choose a reason for hiding this comment

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

在iptables脚本中的某些地方有可改进的地方，例如：

  1. 多余的信息被删除。对于check()函数，多余的空行已移除。
  2. NAT链名是不必要的字符串常量（NatChain），可以在运行时获取该变量值。

可以将这些调整加入到源码中，并根据上述反馈进行修改或优化代码。

Expand Down
7 changes: 7 additions & 0 deletions backend/utils/firewall/client/ufw.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (

"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 @@ -108,6 +109,12 @@ func (f *Ufw) ListForward() ([]FireInfo, error) {
if err != nil {
return nil, err
}
panelChian, _ := cmd.Execf("%s iptables -t nat -L -n | grep 'Chain 1PANEL'", iptables.CmdStr)
if len(strings.ReplaceAll(panelChian, "\n", "")) == 0 {
if err := f.EnableForward(); err != nil {
global.LOG.Errorf("init port forward failed, err: %v", err)
}
}
rules, err := iptables.NatList()
if err != nil {
return nil, err
Copy link

Choose a reason for hiding this comment

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

这段代码包含了一些潜在的问题，但没有直接涉及到具体的技术细节或操作层面的错误。问题可能集中在：

  1. global.log 中定义了错误日志输出格式，但在调用该函数时并未正确地使用此环境变量。

  2. iptables.CmdStr 被引用为 cmd.Execf() 的参数，这并不合理，因为 CmdStr 类似于字符串常量而不应该作为命令执行器，应该考虑其他方式来处理这种场景。

  3. 缺乏适当的输入异常处理和对 EnableForward 函数的调用逻辑优化等。

对于这个问题来说，我给出一个简化版，并添加一些通用提示给你的开发者团队：

  • 避免在生产环境中暴露全局变量和脚本命令。
  • 使用更合适的结构化编程方法，比如利用 Go 命令行工具的 API 来读取配置文件或其他外部资源（例如 /etc/udev/rules.d/99-persistent-net.rules）以减少重复并提高可维护性。
  • 对于多模块应用，确保各个模块之间有良好的接口交互与状态通信机制，避免死循环等问题。

Copy link

Choose a reason for hiding this comment

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

目前没有明显的区别或其他异常情况。该部分代码看起来已经非常正确，并且与当前日期（2024年11月21日）不符。如果需要对某些特定方面进行优化或改进，请提供更具体的信息以便我能给出有效的反馈。

以下是针对代码中的一般化需求可以调整的部分：

func NewUFW() (*UFW, error) {
  // ...
}

func (ufw *UFW) EnableIPv6Forwarding() error {
//...

请注意，我建议你根据实际业务场景和你的项目风格来进行适当的修改和扩展。希望这些建议对你有所帮助！

Expand Down
Loading