crossplane / crossplane/crossplane-tools

Behavior change in Go 1.24 causes internal/method/resolver_test.go's `TestNewResolveReferences` to fail

Open
#109 0 comments 0 reactions 0 assignees View on GitHub
bug
Dominant language
Go
Stars
37
Forks
38
PR merge metrics
No merged PRs in 30d

Description

### What happened?

If you bump the module's Go version to `1.24`, the `TestNewResolveReferences` test in internal/method/resolver_test.go starts to fail as follows:
```shell
=== RUN TestNewResolveReferences
invoke.go:205: starting GOROOT= GOPATH=/var/folders/rv/899xlzw506q0tx02w9c57x9w0000gn/T/TestNewResolveReferences200602618/modcache GO111MODULE=on GOPROXY=file:///var/folders/rv/899xlzw506q0tx02w9c57x9w0000gn/T/TestNewResolveReferences200602618/modproxy PWD=/var/folders/rv/899xlzw506q0tx02w9c57x9w0000gn/T/TestNewResolveReferences200602618/fake go list -f "{{context.GOARCH}} {{context.Compiler}}" -- unsafe
invoke.go:205: starting GOROOT= GOPATH=/var/folders/rv/899xlzw506q0tx02w9c57x9w0000gn/T/TestNewResolveReferences200602618/modcache GO111MODULE=off GOPROXY=file:///var/folders/rv/899xlzw506q0tx02w9c57x9w0000gn/T/TestNewResolveReferences200602618/modproxy PWD=/var/folders/rv/899xlzw506q0tx02w9c57x9w0000gn/T/TestNewResolveReferences200602618/fake go list -e -f {{context.ReleaseTags}} -- unsafe
invoke.go:205: 5.566833ms for GOROOT= GOPATH=/var/folders/rv/899xlzw506q0tx02w9c57x9w0000gn/T/TestNewResolveReferences200602618/modcache GO111MODULE=off GOPROXY=file:///var/folders/rv/899xlzw506q0tx02w9c57x9w0000gn/T/TestNewResolveReferences200602618/modproxy PWD=/var/folders/rv/899xlzw506q0tx02w9c57x9w0000gn/T/TestNewResolveReferences200602618/fake go list -e -f {{context.ReleaseTags}} -- unsafe
invoke.go:205: starting GOROOT= GOPATH=/var/folders/rv/899xlzw506q0tx02w9c57x9w0000gn/T/TestNewResolveReferences200602618/modcache GO111MODULE=on GOPROXY=file:///var/folders/rv/899xlzw506q0tx02w9c57x9w0000gn/T/TestNewResolveReferences200602618/modproxy PWD=/var/folders/rv/899xlzw506q0tx02w9c57x9w0000gn/T/TestNewResolveReferences200602618/fake go list -e -json=Name,ImportPath,Error,Dir,GoFiles,IgnoredGoFiles,IgnoredOtherFiles,CFiles,CgoFiles,CXXFiles,MFiles,HFiles,FFiles,SFiles,SwigFiles,SwigCXXFiles,SysoFiles,TestGoFiles,XTestGoFiles,CompiledGoFiles,DepOnly,Imports,ImportMap,TestImports,XTestImports,Module -compiled=true -test=true -export=false -deps=true -find=false -pgo=off -- /var/folders/rv/899xlzw506q0tx02w9c57x9w0000gn/T/TestNewResolveReferences200602618/fake/v1alpha1
invoke.go:205: 6.188875ms for GOROOT= GOPATH=/var/folders/rv/899xlzw506q0tx02w9c57x9w0000gn/T/TestNewResolveReferences200602618/modcache GO111MODULE=on GOPROXY=file:///var/folders/rv/899xlzw506q0tx02w9c57x9w0000gn/T/TestNewResolveReferences200602618/modproxy PWD=/var/folders/rv/899xlzw506q0tx02w9c57x9w0000gn/T/TestNewResolveReferences200602618/fake go list -f "{{context.GOARCH}} {{context.Compiler}}" -- unsafe
invoke.go:205: 12.165708ms for GOROOT= GOPATH=/var/folders/rv/899xlzw506q0tx02w9c57x9w0000gn/T/TestNewResolveReferences200602618/modcache GO111MODULE=on GOPROXY=file:///var/folders/rv/899xlzw506q0tx02w9c57x9w0000gn/T/TestNewResolveReferences200602618/modproxy PWD=/var/folders/rv/899xlzw506q0tx02w9c57x9w0000gn/T/TestNewResolveReferences200602618/fake go list -e -json=Name,ImportPath,Error,Dir,GoFiles,IgnoredGoFiles,IgnoredOtherFiles,CFiles,CgoFiles,CXXFiles,MFiles,HFiles,FFiles,SFiles,SwigFiles,SwigCXXFiles,SysoFiles,TestGoFiles,XTestGoFiles,CompiledGoFiles,DepOnly,Imports,ImportMap,TestImports,XTestImports,Module -compiled=true -test=true -export=false -deps=true -find=false -pgo=off -- /var/folders/rv/899xlzw506q0tx02w9c57x9w0000gn/T/TestNewResolveReferences200602618/fake/v1alpha1
resolver_test.go:343: NewResolveReferences(): -want, +got
(
"""
... // 182 identical lines

rsp, err = r.Resolve(ctx, reference.ResolutionRequest{
- CurrentValue: reference.FromPtrValue(mg.Spec.ForProvider.CustomConfiguration),
+ CurrentValue: mg.Spec.ForProvider.CustomConfiguration,
Extract: Configuration(),
Reference: mg.Spec.ForProvider.CustomConfigurationRef,
... // 7 identical lines
return errors.Wrap(err, "mg.Spec.ForProvider.CustomConfiguration")
}
- mg.Spec.ForProvider.CustomConfiguration = reference.ToPtrValue(rsp.ResolvedValue)
+ mg.Spec.ForProvider.CustomConfiguration = rsp.ResolvedValue
mg.Spec.ForProvider.CustomConfigurationRef = rsp.ResolvedReference

... // 19 identical lines
"""
)
--- FAIL: TestNewResolveReferences (0.15s)
```

My understanding is that there's a behavior change for Go's type checker:
If you give a try to the following program:
- With [Go 1.23](https://go.dev/play/p/zk8CSRxoVpg?v=goprev), it reports `Field type: *types.Pointer`.
- With [Go 1.24](https://go.dev/play/p/zk8CSRxoVpg), it reports: `Field type: *types.Basic`.
This observation coupled with our reference processing logic [here](https://github.com/crossplane/crossplane-tools/blob/3ad1f97263a34d93584c46c5d0ef4d40d91c14e8/internal/method/reference_processor.go#L123) explains the broken test with Go 1.24. With Go 1.23, because the unknown type is represented with a `*types.Pointer`, the test, in its current shape, succeeds because the `spec.forProvider.customConfiguration` is actually a `*Configuration`, where `Configuration` is an unknown type. With Go 1.24, we cannot deduce it's actually a pointer.

There's an easy fix that I've tried for the failing test (with Go 1.24):
```go
const (
source = `
package v1alpha1

type Configuration struct {}

type ModelParameters struct {
...
```
, i.e., adding a definition for the `Configuration` type in the source file used in the test. However, I'm hesitant to propose that fix because I'm not sure if all referenced types are always in scope of the type checker when this tool runs. In other words, if all type definitions are not always in scope when generating references, we could be making the test pass but people will observe failures. Then we would need to actually fix how we process references.

### How can we reproduce it?

Bump the module's Go version to 1.24 and run the tests in `internal/method/resolver_test.go`.

### What environment did it happen in?
Crossplane version:

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.