terraform-provider-dns icon indicating copy to clipboard operation
terraform-provider-dns copied to clipboard

Bump Expected Minimum Go Version to 1.18

Open bendbennett opened this issue 3 years ago • 1 comments

Terraform CLI and Provider Versions

TF: v1.2.7 Provider: v3.2.3

Use Cases or Problem Statement

Following the Go support policy and given the ecosystem availability of the latest Go minor version, it's time to upgrade. This will ensure that this project can use recent improvements to the Go runtime, standard library functionality, and continue to receive security updates

Proposal

  • Run the following commands to upgrade the Go module files and automatically fix outdated Go code:
go mod edit -go=1.18
go mod tidy
go fix
  • Ensure any GitHub Actions workflows (.github/workflows/*.yml) use 1.19 in place of any 1.18 and 1.18 in place of any 1.17 or earlier
  • Ensure the README or any Contributing documentation notes the Go 1.18 expected minimum
  • (Not applicable to all projects) Ensure the .go-version is at least 1.18 or later

How much impact is this issue causing?

Low

Additional Information

References

  • https://github.com/hashicorp/terraform-plugin-sdk/#go-compatibility
  • https://formulae.brew.sh/formula/go
  • https://github.com/microsoft/winget-pkgs/pull/68523
  • https://community.chocolatey.org/packages/golang
  • https://packages.ubuntu.com/kinetic/golang-1.19

Code of Conduct

  • [X] I agree to follow this project's Code of Conduct

bendbennett avatar Aug 11 '22 09:08 bendbennett

As discovered in #223, there is one particular change in the Go 1.17 standard library net package which affects this provider (and we'll separately need to double check terraform-plugin-sdk/v2/helper/validation functions) whereas the ParseIP() and ParseCIDR() functions no longer work with leading zeroes:

The ParseIP and ParseCIDR functions now reject IPv4 addresses which contain decimal components with leading zeros. These components were always interpreted as decimal, but some operating systems treat them as octal. This mismatch could hypothetically lead to security issues if a Go application was used to validate IP addresses which were then used in their original form with non-Go applications which interpreted components as octal. Generally, it is advisable to always re-encode values after validation, which avoids this class of parser misalignment issues.

Terraform configurations either in the Terraform configuration language (as shown via terraform console below), or via outside scripting, such as using the seq CLI command with the -w flag, may currently be reliant on the existing leading zero behavior.

> [for i in range(1, 11): format("%03d", i)]
[
  "001",
  "002",
  "003",
  "004",
  "005",
  "006",
  "007",
  "008",
  "009",
  "010",
]

Luckily the only logic in this provider currently is in the internal hashIPString() function, called as a *schema.Schema type Set function, used for normalizing set elements for syntactical differences in pre-Terraform 0.12 versions, generally in cases where it was a set block with computed attributes. Terraform 0.12 and later disallow indexing into sets and this isn't a set block in the schema with computed attributes, so the code implementation is very likely moot nowadays, but to ensure 1:1 compatibility for safety we can update that function to automatically handle stripping leading zeroes just to match the pre-Go 1.17 logic (certainly not the most efficient code, but not really a big concern here -- Go Playground link):

func hashIPString(v interface{}) int {
	addr := v.(string)
	ip := net.ParseIP(addr)
	if ip != nil {
		return hashcode.String(ip.String())
	}
	// Preserve pre-Go 1.17 handling of leading zeroes
	if strings.Contains(addr, ".") {
		classes := strings.Split(addr, ".")
		if len(classes) != 4 {
			return hashcode.String(addr)
		}
		for classIndex, class := range classes {
			if len(class) <= 1 {
				continue
			}
			classes[classIndex] = strings.TrimLeft(class, "0")
			if classes[classIndex] == "" {
				classes[classIndex] = "0"
			}
		}
		return hashcode.String(strings.Join(classes, "."))
	}
	return hashcode.String(addr)
}

Maybe it is a little over cautious, but it might be good to prevent any potential behavior differences.

bflad avatar Aug 25 '22 16:08 bflad

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

github-actions[bot] avatar May 22 '24 23:05 github-actions[bot]