Repository: istio/istio
File: tools/istio-iptables/pkg/dependencies/implementation.go (Go)

Code before:
// A: Because we are running in our own container and do not have access to the host default binary.
// We are using our local binaries to update host rules, and we must pick the right match.
//
// Basic selection logic is as follows:
// 1. Check if we have `iptables-legacy` binary in our $PATH and if it has any existing rules in the netns
// 2. If so, use `legacy` binary immediately
// 3. Otherwise, check if we have `iptables-nft` binary in our $PATH and if so, use `nft` binary set
// 4. Otherwise, see if we have `iptables` binary set, and use that.
func (r *RealDependencies) DetectIptablesVersion(ipV6 bool) (IptablesVersion, error) {
	// Begin detecting
	//
	// iptables variants all have ipv6 variants, so decide which set we're looking for
	var nftBin, legacyBin, plainBin string
	if ipV6 {
		nftBin = ip6tablesNftBin
		legacyBin = ip6tablesLegacyBin
		plainBin = ip6tablesBin
	} else {
		nftBin = iptablesNftBin
		legacyBin = iptablesLegacyBin
		plainBin = iptablesBin
	}

	// the user has specifically chosen an iptables
	// version so use that binary or fail
	if r.ForceIptablesBinary != "" {
		switch r.ForceIptablesBinary {
		case legacy:
			legVer, err := shouldUseBinaryForContext(legacyBin)
			if err != nil {
				log.Errorf("did not find iptables binary, error was %v: %+v", err, legVer)
				return IptablesVersion{}, err
			}
			return legVer, nil
		case nft:
			nftVer, err := shouldUseBinaryForContext(nftBin)
			if err != nil {
				log.Errorf("did not find iptables binary, error was %v: %+v", err, nftVer)
				return IptablesVersion{}, err
			}
			return nftVer, nil
		}
	}

	// 1. What binaries we have
	// 2. What binary we should use, based on existing rules defined in our current context.
	//
	// does the legacy binary set exist, and are legacy rules present?
	legVer, err := shouldUseBinaryForContext(legacyBin)
	if err == nil && legVer.ExistingRules {
		// if so, immediately use it

Review comment (bug):
If ForceIptablesBinary is set to an invalid value, the switch doesn’t have a default case and silently falls back to auto-detection. This breaks the expected “break glass” behavior. When a user explicitly forces a value, invalid input should cause a clear failure.

Code after:
// A: Because we are running in our own container and do not have access to the host default binary.
// We are using our local binaries to update host rules, and we must pick the right match.
//
// Basic selection logic is as follows:
// 1. Check if we have `iptables-legacy` binary in our $PATH and if it has any existing rules in the netns
// 2. If so, use `legacy` binary immediately
// 3. Otherwise, check if we have `iptables-nft` binary in our $PATH and if so, use `nft` binary set
// 4. Otherwise, see if we have `iptables` binary set, and use that.
func (r *RealDependencies) DetectIptablesVersion(ipV6 bool) (IptablesVersion, error) {
	// Begin detecting
	//
	// iptables variants all have ipv6 variants, so decide which set we're looking for
	var nftBin, legacyBin, plainBin string
	if ipV6 {
		nftBin = ip6tablesNftBin
		legacyBin = ip6tablesLegacyBin
		plainBin = ip6tablesBin
	} else {
		nftBin = iptablesNftBin
		legacyBin = iptablesLegacyBin
		plainBin = iptablesBin
	}

	// the user has specifically chosen an iptables
	// version so use that binary or fail
	if r.ForceIptablesBinary != "" {
		switch r.ForceIptablesBinary {
		case legacy:
			legVer, err := shouldUseBinaryForContext(legacyBin)
			if err != nil {
				log.Errorf("did not find iptables binary, error was %v: %+v", err, legVer)
				return IptablesVersion{}, err
			}
			return legVer, nil
		case nft:
			nftVer, err := shouldUseBinaryForContext(nftBin)
			if err != nil {
				log.Errorf("did not find iptables binary, error was %v: %+v", err, nftVer)
				return IptablesVersion{}, err
			}
			return nftVer, nil
		default:
			log.Errorf("iptables binary unsupported: %s, supported values are 'legacy' or 'nft'", r.ForceIptablesBinary)
			return IptablesVersion{}, fmt.Errorf("iptables binary %q unsupported", r.ForceIptablesBinary)
		}
	}

	// 1. What binaries we have
	// 2. What binary we should use, based on existing rules defined in our current context.
	//
	// does the legacy binary set exist, and are legacy rules present?