From 3ab10848c8c140b257c66d4a8ab61ba6677595fe Mon Sep 17 00:00:00 2001 From: ssongliu Date: Mon, 31 Aug 2026 17:15:56 +0800 Subject: [PATCH] fix: restore firewall-dependent rules after reset (#13674) --- agent/app/dto/firewall.go | 3 +- agent/app/service/firewall.go | 90 +++++++++++++++++-- agent/utils/firewall/lifecycle/lifecycle.go | 7 ++ agent/utils/firewall/lifecycle/operator.go | 50 ++++++++--- .../firewall/lifecycle/providers/firewalld.go | 14 +-- frontend/src/api/interface/firewall.ts | 1 + frontend/src/api/modules/firewall.ts | 2 +- .../src/views/host/firewall/rule/index.vue | 28 +++++- .../src/views/host/firewall/status/index.vue | 2 +- 9 files changed, 167 insertions(+), 30 deletions(-) diff --git a/agent/app/dto/firewall.go b/agent/app/dto/firewall.go index 38b8ee10d..747d7f87a 100644 --- a/agent/app/dto/firewall.go +++ b/agent/app/dto/firewall.go @@ -103,7 +103,8 @@ type FirewallRuleResetResponse struct { } type FirewallRuleReset struct { - Provider filter.Provider `json:"provider,omitempty" validate:"omitempty,oneof=firewalld ufw iptables nftables"` + Provider filter.Provider `json:"provider,omitempty" validate:"omitempty,oneof=firewalld ufw iptables nftables"` + WithDockerRestart bool `json:"withDockerRestart"` } type FirewallRuleCheckResult struct { diff --git a/agent/app/service/firewall.go b/agent/app/service/firewall.go index c62e0e413..553acecbc 100644 --- a/agent/app/service/firewall.go +++ b/agent/app/service/firewall.go @@ -17,6 +17,7 @@ import ( "github.com/1Panel-dev/1Panel/agent/constant" "github.com/1Panel-dev/1Panel/agent/global" "github.com/1Panel-dev/1Panel/agent/i18n" + "github.com/1Panel-dev/1Panel/agent/utils/controller" "github.com/1Panel-dev/1Panel/agent/utils/firewall" "github.com/1Panel-dev/1Panel/agent/utils/firewall/filter" filterruntime "github.com/1Panel-dev/1Panel/agent/utils/firewall/filter/runtime" @@ -37,7 +38,10 @@ type FirewallService struct { iptablesHelper *iptables_helper.Manager cleanupBackend func(string) error cleanupInactiveBackend func(string) error - resetBackend func(string) error + resetBackend func(string, bool) error + dockerActive func() (bool, error) + restoreForwarding func(context.Context) error + restoreDockerGuard func(context.Context) error baseClient func() (lifecycle.Client, error) } @@ -82,7 +86,14 @@ func newFirewallService() *FirewallService { cleanupBackend: cleanupSystemBackend, cleanupInactiveBackend: cleanupInactiveSystemBackend, resetBackend: resetServiceFirewallBackend, - baseClient: selectedSystemFirewallClient, + dockerActive: func() (bool, error) { + return controller.CheckActive("docker") + }, + restoreForwarding: func(ctx context.Context) error { + return newForwardingService().Restore(ctx) + }, + restoreDockerGuard: ReconcileDockerPortGuard, + baseClient: selectedSystemFirewallClient, } } @@ -286,8 +297,38 @@ func (s *FirewallService) Reset(ctx context.Context, request dto.FirewallRuleRes if reset == nil { reset = resetServiceFirewallBackend } - if err := reset(string(provider)); err != nil { - return dto.FirewallRuleResetResponse{}, err + restartDocker := false + if provider == filter.ProviderFirewalld && request.WithDockerRestart { + dockerActive := s.dockerActive + if dockerActive == nil { + dockerActive = func() (bool, error) { return controller.CheckActive("docker") } + } + active, err := dockerActive() + if err != nil { + return dto.FirewallRuleResetResponse{}, fmt.Errorf("check Docker status before resetting firewalld: %w", err) + } + restartDocker = active + } + resetErr := reset(string(provider), restartDocker) + if resetErr != nil { + var dockerRestartErr *lifecycle.DockerRestartError + if provider != filter.ProviderFirewalld || !errors.As(resetErr, &dockerRestartErr) { + return dto.FirewallRuleResetResponse{}, resetErr + } + } + if provider == filter.ProviderFirewalld { + restoreForwarding := s.restoreForwarding + if restoreForwarding == nil { + restoreForwarding = func(ctx context.Context) error { return newForwardingService().Restore(ctx) } + } + restoreDockerGuard := s.restoreDockerGuard + if restoreDockerGuard == nil { + restoreDockerGuard = ReconcileDockerPortGuard + } + restoreErr := restoreFirewalldDependents(ctx, restartDocker, restoreForwarding, restoreDockerGuard) + if err := errors.Join(resetErr, restoreErr); err != nil { + return dto.FirewallRuleResetResponse{}, err + } } return dto.FirewallRuleResetResponse{Removed: len(stored), Disabled: true}, nil } @@ -296,23 +337,56 @@ func isDirectFirewallProvider(provider filter.Provider) bool { return provider == filter.ProviderIptables || provider == filter.ProviderNftables } -func resetServiceFirewallBackend(provider string) error { +func resetServiceFirewallBackend(provider string, withDockerRestart bool) error { client, err := lifecycle.NewClientFor(provider) if err != nil { return err } + return resetServiceFirewallClient(client, withDockerRestart, func( + client lifecycle.Client, + restartDocker bool, + prepareStop func() error, + ) error { + return lifecycle.NewOperator(client).StopWithPrepare(restartDocker, prepareStop) + }) +} + +func resetServiceFirewallClient( + client lifecycle.Client, + withDockerRestart bool, + stop func(lifecycle.Client, bool, func() error) error, +) error { resetter, ok := client.(lifecycle.Resetter) if !ok { - return fmt.Errorf("firewall provider %s does not support reset", provider) + return fmt.Errorf("firewall provider %s does not support reset", client.Name()) } - if provider == constant.FirewallProviderFirewalld { - if err := lifecycle.NewOperator(client).Operate(lifecycle.OperationStop, false, nil); err != nil { + if resetBeforeStop, ok := client.(lifecycle.PreStopResetter); ok { + if err := stop(client, withDockerRestart, resetBeforeStop.ResetBeforeStop); err != nil { return err } + return nil } return resetter.Reset() } +func restoreFirewalldDependents( + ctx context.Context, + restartDocker bool, + restoreForwarding func(context.Context) error, + restoreDockerGuard func(context.Context) error, +) error { + var errs []error + if err := restoreForwarding(ctx); err != nil { + errs = append(errs, fmt.Errorf("restore port forwarding after resetting firewalld: %w", err)) + } + if restartDocker { + if err := restoreDockerGuard(ctx); err != nil { + errs = append(errs, fmt.Errorf("restore Docker port guard after resetting firewalld: %w", err)) + } + } + return errors.Join(errs...) +} + func (s *FirewallService) deleteFirewallRuleRecords( ctx context.Context, stored []model.FirewallRule, diff --git a/agent/utils/firewall/lifecycle/lifecycle.go b/agent/utils/firewall/lifecycle/lifecycle.go index 43926d5ab..be2980438 100644 --- a/agent/utils/firewall/lifecycle/lifecycle.go +++ b/agent/utils/firewall/lifecycle/lifecycle.go @@ -102,6 +102,13 @@ type Resetter interface { Reset() error } +// PreStopResetter prepares a service-backed firewall reset without stopping +// the service. The lifecycle operator can then stop the service and perform +// dependent recovery, such as rebuilding Docker firewall rules, in order. +type PreStopResetter interface { + ResetBeforeStop() error +} + func NewClient() (Client, error) { runtime, err := DetectRuntime() if err != nil { diff --git a/agent/utils/firewall/lifecycle/operator.go b/agent/utils/firewall/lifecycle/operator.go index c75b61459..efa817a80 100644 --- a/agent/utils/firewall/lifecycle/operator.go +++ b/agent/utils/firewall/lifecycle/operator.go @@ -22,17 +22,25 @@ type Operator struct { client Client } +// DockerRestartError reports that the requested firewall operation completed, +// but rebuilding Docker's firewall rules failed. +type DockerRestartError struct { + Err error +} + +func (e *DockerRestartError) Error() string { + return fmt.Sprintf("failed to restart Docker: %v", e.Err) +} + +func (e *DockerRestartError) Unwrap() error { + return e.Err +} + func NewOperator(client Client) *Operator { return &Operator{client: client} } func (o *Operator) Operate(operation Operation, withDockerRestart bool, prepareStart func(Client) error) error { - if o.client.Name() == ProviderFirewalld && operation == OperationStop { - if err := rememberFail2BanBeforeFirewallStop(); err != nil { - return err - } - } - switch operation { case OperationStart: if err := o.client.Start(); err != nil { @@ -44,9 +52,7 @@ func (o *Operator) Operate(operation Operation, withDockerRestart bool, prepareS } } case OperationStop: - if err := o.client.Stop(); err != nil { - return err - } + return o.StopWithPrepare(withDockerRestart, nil) case OperationRestart: if err := o.client.Restart(); err != nil { return err @@ -62,7 +68,7 @@ func (o *Operator) Operate(operation Operation, withDockerRestart bool, prepareS if withDockerRestart { if err := controller.HandleRestart("docker"); err != nil { - return fmt.Errorf("failed to restart Docker: %v", err) + return &DockerRestartError{Err: err} } } if o.client.Name() == ProviderFirewalld && operation == OperationStart { @@ -71,6 +77,30 @@ func (o *Operator) Operate(operation Operation, withDockerRestart bool, prepareS return nil } +// StopWithPrepare records dependent service state, runs preparation, stops the +// firewall, and optionally restarts Docker in that order. +func (o *Operator) StopWithPrepare(withDockerRestart bool, prepareStop func() error) error { + if o.client.Name() == ProviderFirewalld { + if err := rememberFail2BanBeforeFirewallStop(); err != nil { + return err + } + } + if prepareStop != nil { + if err := prepareStop(); err != nil { + return err + } + } + if err := o.client.Stop(); err != nil { + return err + } + if withDockerRestart { + if err := controller.HandleRestart("docker"); err != nil { + return &DockerRestartError{Err: err} + } + } + return nil +} + func rememberFail2BanBeforeFirewallStop() error { exists, err := controller.CheckExist("fail2ban.service") if err != nil { diff --git a/agent/utils/firewall/lifecycle/providers/firewalld.go b/agent/utils/firewall/lifecycle/providers/firewalld.go index 63d45c3ce..ce6722986 100644 --- a/agent/utils/firewall/lifecycle/providers/firewalld.go +++ b/agent/utils/firewall/lifecycle/providers/firewalld.go @@ -76,16 +76,16 @@ func (f *Firewalld) Stop() error { } func (f *Firewalld) Reset() error { - running, err := f.Status() - if err != nil { - return fmt.Errorf("reset firewalld to defaults failed: %w", err) + if err := f.ResetBeforeStop(); err != nil { + return err } - if running { - if err := f.Stop(); err != nil { - return fmt.Errorf("reset firewalld to defaults failed: %w", err) - } + if err := f.Stop(); err != nil { + return fmt.Errorf("stop firewalld after reset failed: %w", err) } + return nil +} +func (f *Firewalld) ResetBeforeStop() error { backupDir := fmt.Sprintf("%s.1panel-backup-%d", firewalldConfigDir, time.Now().UnixNano()) rollback, err := replaceFirewalldConfig( firewalldConfigDir, diff --git a/frontend/src/api/interface/firewall.ts b/frontend/src/api/interface/firewall.ts index 832820297..040434f00 100644 --- a/frontend/src/api/interface/firewall.ts +++ b/frontend/src/api/interface/firewall.ts @@ -201,6 +201,7 @@ export namespace Firewall { export interface ResetRequest { provider?: Provider; + withDockerRestart?: boolean; } export type CheckDecision = 'ready' | 'confirmation_required' | 'blocked' | 'no_change'; diff --git a/frontend/src/api/modules/firewall.ts b/frontend/src/api/modules/firewall.ts index d2bdaf12f..f30478cfb 100644 --- a/frontend/src/api/modules/firewall.ts +++ b/frontend/src/api/modules/firewall.ts @@ -13,7 +13,7 @@ export const searchForwardRule = (request: Firewall.ForwardRuleSearch) => http.post>('/hosts/firewall/forward/search', request, TimeoutEnum.T_40S); export const operateFire = (operation: string, withDockerRestart: boolean) => - http.post('/hosts/firewall/operate', { operation, withDockerRestart }, TimeoutEnum.T_60S); + http.post('/hosts/firewall/operate', { operation, withDockerRestart }, TimeoutEnum.T_10M); export const operateForwardRule = (request: { rules: Firewall.RuleForward[]; forceDelete?: boolean }) => http.post('/hosts/firewall/forward/operate', request, TimeoutEnum.T_40S); diff --git a/frontend/src/views/host/firewall/rule/index.vue b/frontend/src/views/host/firewall/rule/index.vue index c5fbb639e..ee2c2b131 100644 --- a/frontend/src/views/host/firewall/rule/index.vue +++ b/frontend/src/views/host/firewall/rule/index.vue @@ -368,7 +368,12 @@ - + + @@ -398,6 +403,8 @@ import FireRouter from '@/views/host/firewall/index.vue'; import FireStatus from '@/views/host/firewall/status/index.vue'; import ProcessDetail from '@/views/host/process/process/detail/index.vue'; import ConfirmDialog from '@/components/confirm-dialog/index.vue'; +import DockerRestart from '@/components/docker-proxy/docker-restart.vue'; +import { loadDockerStatus } from '@/api/modules/container'; import { computed, onMounted, reactive, ref } from 'vue'; import { ElMessageBox } from 'element-plus'; import { Expand, Filter, Lock, WarningFilled } from '@element-plus/icons-vue'; @@ -461,6 +468,8 @@ const ruleImportRef = ref>(); const ruleSyncRef = ref>(); const processDetailRef = ref>(); const resetConfirmRef = ref>(); +const dockerRestartRef = ref>(); +const withDockerRestart = ref(false); const loading = ref(false); const resetting = ref(false); const syncOpening = ref(false); @@ -1111,6 +1120,7 @@ const removeRules = async (selected: RuleRow[]) => { const removeSelectedRules = () => removeRules(selects.value.filter((row) => isDeletableManagedRule(row))); const resetRules = () => { + withDockerRestart.value = false; const message = i18n.global.t( isDirectBackend.value ? 'firewall.resetDirectRulesHelper' : 'firewall.resetWhitelistRulesHelper', [provider.value], @@ -1122,11 +1132,25 @@ const resetRules = () => { }); }; +const prepareResetRules = async () => { + if (provider.value === 'firewalld') { + const status = await loadDockerStatus(); + if (status.data.isActive) { + dockerRestartRef.value?.acceptParams({ title: i18n.global.t('firewall.dockerRestart') }); + return; + } + } + await submitResetRules(); +}; + const submitResetRules = async () => { resetting.value = true; loading.value = true; try { - await resetFirewallRules({ provider: provider.value as Firewall.Provider }); + await resetFirewallRules({ + provider: provider.value as Firewall.Provider, + withDockerRestart: provider.value === 'firewalld' && withDockerRestart.value, + }); MsgSuccess(i18n.global.t('commons.msg.operationSuccess')); } finally { resetting.value = false; diff --git a/frontend/src/views/host/firewall/status/index.vue b/frontend/src/views/host/firewall/status/index.vue index 28162ee50..fb9537cf5 100644 --- a/frontend/src/views/host/firewall/status/index.vue +++ b/frontend/src/views/host/firewall/status/index.vue @@ -333,7 +333,7 @@ const loadBaseInfo = async (search: boolean) => { const loadDocker = async () => { const res = await loadDockerStatus(); - dockerStatus.value = res.data.isExist; + dockerStatus.value = res.data.isActive; }; const onInit = async () => {