From 4861eb69cb0e1c2fdafb9dbbdc6cc0591daaffdc Mon Sep 17 00:00:00 2001 From: ssongliu Date: Tue, 22 Sep 2026 18:20:18 +0800 Subject: [PATCH] fix: separate firewalld batches by native option (#13896) --- agent/app/service/firewall_sync.go | 6 +- agent/app/service/firewall_utils.go | 80 +++++++++++++++++++ .../filter/providers/firewalld/adapter.go | 2 +- 3 files changed, 85 insertions(+), 3 deletions(-) diff --git a/agent/app/service/firewall_sync.go b/agent/app/service/firewall_sync.go index bfeeffa9f..34cb943bc 100644 --- a/agent/app/service/firewall_sync.go +++ b/agent/app/service/firewall_sync.go @@ -42,8 +42,10 @@ func (s *FirewallService) SyncPortWhitelist(ctx context.Context) error { if err != nil { return err } - _, err = s.syncPortWhitelist(ctx, provider, ports) - return err + if _, err := s.syncPortWhitelist(ctx, provider, ports); err != nil { + return err + } + return s.removeTransferredSystemPortRules(ctx, provider, ports) } func (s *FirewallService) PreviewRuleSync(ctx context.Context, clientIP string, request dto.FirewallRuleSyncRequest) (dto.FirewallRuleSyncPreview, error) { diff --git a/agent/app/service/firewall_utils.go b/agent/app/service/firewall_utils.go index e87dd0093..e2d04018b 100644 --- a/agent/app/service/firewall_utils.go +++ b/agent/app/service/firewall_utils.go @@ -2365,6 +2365,86 @@ func (s *FirewallService) compileRestorableFirewallRules(ctx context.Context, st return restorable, preserved, nil } +func (s *FirewallService) removeTransferredSystemPortRules(ctx context.Context, provider filter.Provider, ports []firewall.PortWhitelist) error { + if provider != filter.ProviderIptables && provider != filter.ProviderNftables { + return nil + } + firewallRuleMutationMu.Lock() + defer firewallRuleMutationMu.Unlock() + required, err := firewall.RequiredPortWhitelist(ports) + if err != nil { + return err + } + runtime, err := s.firewallAdapter(provider) + if err != nil { + return err + } + stored, err := s.rules.List(ctx) + if err != nil { + return err + } + custom := filter.NewPortWhitelistIndex(customWhitelist(ports)) + candidates := make([]model.FirewallRule, 0) + keysByUUID := make(map[string][]string) + scopes := make([]filter.Scope, 0) + for _, record := range stored { + if record.Origin != constant.FirewallRuleOriginCreated || !strings.HasPrefix(record.Owner, constant.FirewallRuleSourceSecurity+":"+constant.FirewallSystemAcceptedPortSourcePrefix) { + continue + } + restorable, preserved, err := s.compileRestorableFirewallRules(ctx, record, runtime, required) + if isFirewallPolicyIncompatible(err) { + continue + } + if err != nil { + return err + } + if len(restorable) != 0 || len(preserved) == 0 || slices.ContainsFunc(preserved, func(rule filter.DesiredRule) bool { return custom.Matches(rule.Rule) }) { + continue + } + for _, desired := range preserved { + rule := desired.Rule + rule.Scope.Chain = filter.BasicBeforeChain + key, err := filter.RuleMatchKey(rule) + if err != nil { + return err + } + keysByUUID[record.UUID] = append(keysByUUID[record.UUID], key) + scopes = append(scopes, rule.Scope) + } + candidates = append(candidates, record) + } + present := make(map[string]bool) + for _, group := range firewallScopeReadGroups(scopes) { + snapshots, err := readMutableFirewallRuleScopes(runtime, ctx, group) + if errors.Is(err, filter.ErrFamilyUnavailable) { + continue + } + if err != nil { + return err + } + for _, snapshot := range snapshots { + for _, observed := range snapshot.Rules { + if observed.ParseStatus != filter.ParseStatusSupported || observed.Rule.Action != filter.ActionAccept || (observed.Persistence != "" && observed.Persistence != filter.PersistenceStatusConverged) { + continue + } + key, err := filter.RuleMatchKey(observed.Rule) + if err != nil { + return err + } + present[key] = true + } + } + } + candidates = slices.DeleteFunc(candidates, func(record model.FirewallRule) bool { + return slices.ContainsFunc(keysByUUID[record.UUID], func(key string) bool { return !present[key] }) + }) + var failures []error + for ruleUUID, err := range s.rules.DeleteBatchWithRevision(ctx, candidates) { + failures = append(failures, fmt.Errorf("remove transferred system port rule %s: %w", ruleUUID, err)) + } + return errors.Join(failures...) +} + func firewallInventoryRuleKey(rule filter.FirewallRule) (string, error) { if rule.Scope.Provider == filter.ProviderFirewalld { key, err := filter.RuleMatchKey(rule) diff --git a/agent/utils/firewall/filter/providers/firewalld/adapter.go b/agent/utils/firewall/filter/providers/firewalld/adapter.go index d181654d6..a92def3f8 100644 --- a/agent/utils/firewall/filter/providers/firewalld/adapter.go +++ b/agent/utils/firewall/filter/providers/firewalld/adapter.go @@ -278,7 +278,7 @@ func batchCommands(plan filter.CommandBatch) (filter.RuleCommands, error) { for index, command := range rule.Commands { option := command.Args[len(command.Args)-1] rollback := rule.RollbackCommands[index].Args[len(rule.RollbackCommands[index].Args)-1] - operation := strings.SplitN(strings.TrimPrefix(option, "--"), "-", 2)[0] + operation, _, _ := strings.Cut(option, "=") if operation != previousOperation || commandBytes+max(len(option), len(rollback))+1 > 64*1024 { args := []string{"--zone=" + filter.FirewalldInputZone} if permanent {