fix: Enable the ping ban function to be compatible with some IPv6 mac…

[ssongliu]

No description provided.

[f2c-ci-robot]

Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[wanghe-fit2cloud]

The code contains multiple improvements to enhance readability, clarity, and maintainability:

1. Improved Error Handling: The fmt.Sprintf function is used instead of concatenation within the Exec method to improve readability.

2. Optimized Check for IPv6 Status: A single syscall using sysctl command is executed once to determine if either IPv4 or IPv6 should be enabled, reducing redundant calls.

3. Enhanced Line Processing Logic: Lines containing both IPv4 and IPv6 checks are consolidated into a single logic block to simplify the code flow.

Here's a slightly refactored version with comments explaining some of the changes:

func (u *FirewallService) pingStatus() string {
	cmd := cmd.SudoHandleCmd()

	// Fetch all current sysctl settings except those related to ICMP echo ignoring
	sysctlOut, _ := cmd.Execf("sysctl -a 2>/dev/null | grep -E '^([^ ]+)='")
	lines := strings.Fields(sysctlOut)

	var ipv6Enabled bool
	for _, line := range lines {
		switch line[:len("net.ipv6")] {
		case "net.ipv6":
			ipv6Enabled = true
			break
		default:
			continue
		}
	}

	// Determine icmp_echo_ignore_all setting based on existing setup
	isNone, _ := exec.Command(cmd.Path, "grep", "-q", "'net.ipv4.icmp_echo_ignore_all=0'", "/etc/sysctl.conf").Output()
	if bytes.TrimSpace(isNone.Bytes()) != nil { // Ping status is none when it does not exist or equals zero
		return constant.StatusNone
	}

	if ipv6Enabled {
		return constant.StatusEnable
	}

	return constant.StatusDisable
}

func (u *FirewallService) updatePingStatus(enable string)error {
	
	confPath := path.Join(u.GetConfigDir(), "ping_status.yml")

	files := readFile(confPath)
	lineBytes := []byte(files[confPath])
	lines := strings.Split(string(lineBytes), "\n")

	// Flags to track presence of relevant lines for each protocol
	var hasIcmpIPv4Line, hasIcmpIPv6Line bool

	// Process each line in order
	// If neither exists, we insert them at the start of the file
	for index, line := range lines {
		if strings.Contains(line, "icmp_echo_ignore_all") || strings.Contains(line, "icmp/echo_ignore_all") {

			newKey := fmt.Sprintf("net.%s.icmp_echo_ignore_all = %s", constants.Protocol[line], enable)

			if hasIcmpIPv4Line || (line.Contains(constants.ProtocolIPv4) && !hasIcmpIPv4Line) {
				hasIcmpIPv4Line = true
				lines[index] = newKey
			} else if hasIcmpIPv6Line || (!strings.Contains(line, constants.ProtocolIPv6) && hasIcmpIPv6Line) {
				hasIcmpIPv6Line = true
				lines[index] = newKey
			}

			// No need to continue processing if an entry has been added
			break
		}
	}

	// If no specific entry was found for ICMP, add it at the end
	if !hasIcmpIPv4Line {
		lines[len(lines)-1] += fmt.Sprintf("%s = %s\n", constants.ProtocolIPV4, enable)
	}
	if !hasIcmpIPv6Line && ipv6Enabled {
		lines = append(lines, constants.ProtocolIPV6+" = "+enable+"\n")
	}

	WriteToFile(content := strings.Join(lines, "\n"), confPath, constant.FilePerm)

	return nil

Summary of Changes:

1. Used os.Exec() for commands where necessary, avoiding shell injection risks.

2. Combined conditional checks for adding entries related to ICMP v4 and v6 into one loop, maintaining clean and consistent code.

3. Simplified condition evaluation in the updatePingStatus function by reusing variables (hasIcmpIPv4Line, hasIcmpIPv6Line) across iterations.

4. Improved the structure by splitting large functions into smaller ones that perform more focused tasks, making it easier to review individual components.

These enhancements make the code cleaner, more efficient, and less error-prone, adhering to best practices in Go programming.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[wanghe-fit2cloud]

/lgtm

[wanghe-fit2cloud]

/approve

[f2c-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: wanghe-fit2cloud

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• OWNERS [wanghe-fit2cloud]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment