fix: update ChainType handling to use direct values instead of pointers

this update solves an issue with the FirewallConfiguration's mutating webhook that returns a nil pointer exception when the Type field is omitted

Author: riccardotornesello

apis/networking/v1beta1/firewall/chain_types.go

@@ -83,7 +83,7 @@ type Chain struct {
 	Rules RulesSet `json:"rules"`
 	// Type defines what this chain will be used for.
 	// +kubebuilder:validation:Enum="filter";"route";"nat"
-	Type *ChainType `json:"type"`
+	Type ChainType `json:"type"`
 	// Policy defines what this chain default policy will be.
 	// +kubebuilder:validation:Enum="drop";"accept"
 	Policy *ChainPolicy `json:"policy"`

apis/networking/v1beta1/firewall/zz_generated.deepcopy.go

@@ -55,8 +55,8 @@ func addChain(nftconn *nftables.Conn, chain *firewallapi.Chain, table *nftables.
 	if chain.Priority != nil {
 		setPriority(nftChain, *chain.Priority)
 	}
-	if chain.Type != nil {
-		setType(nftChain, *chain.Type)
+	if chain.Type != "" {
+		setType(nftChain, chain.Type)
 	}
 	if chain.Policy != nil {
 		setPolicy(nftChain, *chain.Policy)
@@ -198,7 +198,7 @@ func isChainOutdated(nftChain *nftables.Chain, chains []firewallapi.Chain) (outd
 // isChainModified checks if the chain has been modified.
 // It does not consider policies since they can be modified without deleting the chain.
 func isChainModified(nftChain *nftables.Chain, chain *firewallapi.Chain) bool {
-	if chain.Type != nil && *chain.Type != getType(nftChain.Type) {
+	if chain.Type != "" && chain.Type != getType(nftChain.Type) {
 		return true
 	}
 	if chain.Hook != nil && *chain.Hook != getHooknum(*nftChain.Hooknum) {
@@ -215,7 +215,7 @@ func isChainModified(nftChain *nftables.Chain, chain *firewallapi.Chain) bool {
 
 // FromChainToRulesArray converts a chain to an array of rules.
 func FromChainToRulesArray(chain *firewallapi.Chain) (rules []firewallutils.Rule) {
-	switch *chain.Type {
+	switch chain.Type {
 	case firewallapi.ChainTypeFilter:
 		rules = make([]firewallutils.Rule, len(chain.Rules.FilterRules))
 		for i := range chain.Rules.FilterRules {

pkg/firewall/chain.go

@@ -113,7 +113,7 @@ func forgeCIDRFirewallConfigurationDNATChain(cfg *networkingv1beta1.Configuratio
 	return firewall.Chain{
 		Name:     &DNATChainName,
 		Policy:   ptr.To(firewall.ChainPolicyAccept),
-		Type:     ptr.To(firewall.ChainTypeNAT),
+		Type:     firewall.ChainTypeNAT,
 		Hook:     &firewall.ChainHookPrerouting,
 		Priority: &firewall.ChainPriorityNATDest,
 		Rules: firewall.RulesSet{
@@ -127,7 +127,7 @@ func forgeCIDRFirewallConfigurationSNATChain(cfg *networkingv1beta1.Configuratio
 	return firewall.Chain{
 		Name:     &SNATChainName,
 		Policy:   ptr.To(firewall.ChainPolicyAccept),
-		Type:     ptr.To(firewall.ChainTypeNAT),
+		Type:     firewall.ChainTypeNAT,
 		Hook:     &firewall.ChainHookPostrouting,
 		Priority: &firewall.ChainPriorityNATSource,
 		Rules: firewall.RulesSet{

pkg/liqo-controller-manager/networking/external-network/remapping/cidr.go

@@ -174,15 +174,15 @@ func enforceFirewallConfigurationChains(fwcfg *networkingv1beta1.FirewallConfigu
 	chainPre := &fwcfg.Spec.Table.Chains[0]
 	chainPre.Name = &PreroutingChainName
 	chainPre.Policy = ptr.To(firewall.ChainPolicyAccept)
-	chainPre.Type = ptr.To(firewall.ChainTypeNAT)
+	chainPre.Type = firewall.ChainTypeNAT
 	chainPre.Hook = &firewall.ChainHookPrerouting
 	chainPre.Priority = ptr.To(firewall.ChainPriorityNATDest)
 	ensureFirewallConfigurationDNATRules(&chainPre.Rules, ip)
 
 	chainPost := &fwcfg.Spec.Table.Chains[1]
 	chainPost.Name = &PostroutingChainName
 	chainPost.Policy = ptr.To(firewall.ChainPolicyAccept)
-	chainPost.Type = ptr.To(firewall.ChainTypeNAT)
+	chainPost.Type = firewall.ChainTypeNAT
 	chainPost.Hook = &firewall.ChainHookPostrouting
 	chainPost.Priority = ptr.To(firewall.ChainPriorityNATSource)
 	ensureFirewallConfigurationSNATRules(&chainPost.Rules, ip)
@@ -195,15 +195,15 @@ func enforceFirewallConfigurationMasqChains(fwcfg *networkingv1beta1.FirewallCon
 	chainPre := &fwcfg.Spec.Table.Chains[0]
 	chainPre.Name = &PreroutingChainName
 	chainPre.Policy = ptr.To(firewall.ChainPolicyAccept)
-	chainPre.Type = ptr.To(firewall.ChainTypeNAT)
+	chainPre.Type = firewall.ChainTypeNAT
 	chainPre.Hook = &firewall.ChainHookPrerouting
 	chainPre.Priority = ptr.To(firewall.ChainPriorityNATDest)
 	ensureFirewallConfigurationDNATRules(&chainPre.Rules, ip)
 
 	chainPost := &fwcfg.Spec.Table.Chains[1]
 	chainPost.Name = &PostroutingChainName
 	chainPost.Policy = ptr.To(firewall.ChainPolicyAccept)
-	chainPost.Type = ptr.To(firewall.ChainTypeNAT)
+	chainPost.Type = firewall.ChainTypeNAT
 	chainPost.Hook = &firewall.ChainHookPostrouting
 	chainPost.Priority = ptr.To(firewall.ChainPriorityNATSource - 1)
 	ensureFirewallConfigurationMasqSNATRules(&chainPost.Rules, ip)

pkg/liqo-controller-manager/networking/external-network/remapping/ip.go

@@ -84,7 +84,7 @@ func forgeMutateFirewallConfiguration(fwcfg *networkingv1beta1.FirewallConfigura
 func forgeFirewallChain() *firewallapi.Chain {
 	return &firewallapi.Chain{
 		Name:     ptr.To(PrePostroutingChainName),
-		Type:     ptr.To(firewallapi.ChainTypeNAT),
+		Type:     firewallapi.ChainTypeNAT,
 		Policy:   ptr.To(firewallapi.ChainPolicyAccept),
 		Priority: ptr.To(firewallapi.ChainPriorityNATSource - 1),
 		Hook:     ptr.To(firewallapi.ChainHookPostrouting),

pkg/liqo-controller-manager/networking/internal-network/configuration-controller/firewall.go

@@ -129,7 +129,7 @@ func forgeFirewallPodUpdateFunction(internalnode *networkingv1beta1.InternalNode
 
 func setFirewallPodChain(chain *firewall.Chain) {
 	chain.Name = ptr.To(PrePostroutingChainName)
-	chain.Type = ptr.To(firewall.ChainTypeNAT)
+	chain.Type = firewall.ChainTypeNAT
 	chain.Hook = ptr.To(firewall.ChainHookPostrouting)
 	chain.Policy = ptr.To(firewall.ChainPolicyAccept)
 	chain.Priority = ptr.To(firewall.ChainPriorityNATSource - 1)

pkg/liqo-controller-manager/networking/internal-network/gw-masq-bypass/k8s.go

@@ -88,8 +88,8 @@ func (r *PodReconciler) processFirewallConfiguration(ctx context.Context, fwcfgl
 
 		chain := fwcfglist.Items[i].Spec.Table.Chains[0]
 
-		if chain.Type == nil || *chain.Type != firewall.ChainTypeNAT {
-			return fmt.Errorf("firewall configuration table chain should be of type NAT, not %s", *chain.Type)
+		if chain.Type == "" || chain.Type != firewall.ChainTypeNAT {
+			return fmt.Errorf("firewall configuration table chain should be of type NAT, not %s", chain.Type)
 		}
 
 		if err := r.processRules(ctx, &chain, getNodeFromFirewallConfigurationName(fwcfglist.Items[i].Name)); err != nil {

pkg/liqo-controller-manager/networking/internal-network/gw-masq-bypass/leftovers.go

@@ -126,7 +126,7 @@ func enforceFirewallConfigurationForwardChain(fwcfg *networkingv1beta1.FirewallC
 		fwcfg.Spec.Table.Chains = append(fwcfg.Spec.Table.Chains, firewall.Chain{})
 	}
 	fwcfg.Spec.Table.Chains[0].Name = ptr.To("mark-to-conntrack")
-	fwcfg.Spec.Table.Chains[0].Type = ptr.To(firewall.ChainTypeFilter)
+	fwcfg.Spec.Table.Chains[0].Type = firewall.ChainTypeFilter
 	fwcfg.Spec.Table.Chains[0].Policy = ptr.To(firewall.ChainPolicyAccept)
 	fwcfg.Spec.Table.Chains[0].Hook = &firewall.ChainHookForward
 	fwcfg.Spec.Table.Chains[0].Priority = &firewall.ChainPriorityFilter
@@ -171,7 +171,7 @@ func enforceFirewallConfigurationPreroutingChain(fwcfg *networkingv1beta1.Firewa
 		fwcfg.Spec.Table.Chains = append(fwcfg.Spec.Table.Chains, firewall.Chain{})
 	}
 	fwcfg.Spec.Table.Chains[1].Name = ptr.To("conntrack-mark-to-meta-mark")
-	fwcfg.Spec.Table.Chains[1].Type = ptr.To(firewall.ChainTypeFilter)
+	fwcfg.Spec.Table.Chains[1].Type = firewall.ChainTypeFilter
 	fwcfg.Spec.Table.Chains[1].Policy = ptr.To(firewall.ChainPolicyAccept)
 	fwcfg.Spec.Table.Chains[1].Hook = ptr.To(firewall.ChainHookPrerouting)
 	fwcfg.Spec.Table.Chains[1].Priority = ptr.To(firewall.ChainPriorityFilter)

pkg/liqo-controller-manager/networking/internal-network/route/internalnode_k8s.go

@@ -39,10 +39,10 @@ func checkChain(tableFamily firewallapi.TableFamily, chain *firewallapi.Chain) e
 }
 
 func checkAllowedTableFamilyChainTypeHook(tableFamily firewallapi.TableFamily, chain *firewallapi.Chain) error {
-	if !allowedTableFamilyChainTypeHook(tableFamily, *chain.Type, *chain.Hook) {
+	if !allowedTableFamilyChainTypeHook(tableFamily, chain.Type, *chain.Hook) {
 		return fmt.Errorf(`in chain %s, the combination of family %s, chain type %s and hook %s is not allowed. 
 		Please refer to https://wiki.nftables.org/wiki-nftables/index.php/Netfilter_hooks`,
-			*chain.Name, tableFamily, *chain.Type, *chain.Hook,
+			*chain.Name, tableFamily, chain.Type, *chain.Hook,
 		)
 	}
 	return nil
@@ -69,14 +69,14 @@ func totalDefinedRulesSets(rules firewallapi.RulesSet) int {
 	return total
 }
 
-func allowedChainType(chaintype *firewallapi.ChainType, rules firewallapi.RulesSet) error {
-	if rules.NatRules != nil && *chaintype != firewallapi.ChainTypeNAT {
+func allowedChainType(chaintype firewallapi.ChainType, rules firewallapi.RulesSet) error {
+	if rules.NatRules != nil && chaintype != firewallapi.ChainTypeNAT {
 		return fmt.Errorf("NAT rules must be defined only when using NAT chain")
 	}
-	if rules.FilterRules != nil && *chaintype != firewallapi.ChainTypeFilter {
+	if rules.FilterRules != nil && chaintype != firewallapi.ChainTypeFilter {
 		return fmt.Errorf("filter rules must be defined only when using Filter chain")
 	}
-	if rules.RouteRules != nil && *chaintype != firewallapi.ChainTypeRoute {
+	if rules.RouteRules != nil && chaintype != firewallapi.ChainTypeRoute {
 		return fmt.Errorf("route rules must be defined only when using Route chain")
 	}