Panic parsing resources not directly imported

mnencia commented 1 month ago

Minimal repro:

Create the needed directories mkdir repro1 repro2 repro3 and put the files inside

// +groupName=repro.io
package repro1

import (
    metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"

    "sigs.k8s.io/controller-tools/repro2"

        // Uncomment the following line to fix the panic
    // _ "sigs.k8s.io/controller-tools/repro3"
)

type Repro struct {
    metav1.TypeMeta   `json:",inline"`
    metav1.ObjectMeta `json:"metadata"`

    Reproducer repro2.MyReproAlias `json:"reproducer"`
}

package repro2

import "sigs.k8s.io/controller-tools/repro3"

type MyReproAlias = repro3.MyReproStruct

package repro3

type MyReproStruct struct {
    ReproString string `json:"repro"`
}

Then run controller-gen with:

go run ./cmd/controller-gen/ crd paths=./repro1

The result should be the following:

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x4d4abb2]

goroutine 1 [running]:
sigs.k8s.io/controller-tools/pkg/crd.(*Parser).NeedPackage(0xc0004040c0, 0x0)
    /Users/mnencia/prj/tmp/controller-tools/pkg/crd/parser.go:237 +0x52
sigs.k8s.io/controller-tools/pkg/crd.(*Parser).NeedSchemaFor(0xc0004040c0, {0x0, {0xc000191710, 0xd}})
    /Users/mnencia/prj/tmp/controller-tools/pkg/crd/parser.go:170 +0x5a
sigs.k8s.io/controller-tools/pkg/crd.(*schemaContext).requestSchema(0xc0014c0540?, {0xc000195d10?, 0x4e31d15?}, {0xc000191710?, 0xd?})
    /Users/mnencia/prj/tmp/controller-tools/pkg/crd/schema.go:108 +0xdb
sigs.k8s.io/controller-tools/pkg/crd.namedToSchema(0xc0014c0540, 0xc0004be1b0)
    /Users/mnencia/prj/tmp/controller-tools/pkg/crd/schema.go:280 +0x206
sigs.k8s.io/controller-tools/pkg/crd.typeToSchema(0xc0014c0540, {0x51064c8, 0xc0004be1b0})
    /Users/mnencia/prj/tmp/controller-tools/pkg/crd/schema.go:199 +0xf6
sigs.k8s.io/controller-tools/pkg/crd.structToSchema(0xc000a2a650, 0xc0004be1c8)
    /Users/mnencia/prj/tmp/controller-tools/pkg/crd/schema.go:435 +0x872
sigs.k8s.io/controller-tools/pkg/crd.typeToSchema(0xc000a2a650, {0x5106468, 0xc0004be1c8})
    /Users/mnencia/prj/tmp/controller-tools/pkg/crd/schema.go:207 +0x93
sigs.k8s.io/controller-tools/pkg/crd.infoToSchema(0xc000052650)
    /Users/mnencia/prj/tmp/controller-tools/pkg/crd/schema.go:125 +0xc5
sigs.k8s.io/controller-tools/pkg/crd.(*Parser).NeedSchemaFor(0xc0004040c0, {0xc0001baf40, {0xc0001915d0, 0x5}})
    /Users/mnencia/prj/tmp/controller-tools/pkg/crd/parser.go:193 +0x290
sigs.k8s.io/controller-tools/pkg/crd.(*Parser).NeedFlattenedSchemaFor(0xc0004040c0, {0xc0001baf40, {0xc0001915d0, 0x5}})
    /Users/mnencia/prj/tmp/controller-tools/pkg/crd/parser.go:205 +0xcd
sigs.k8s.io/controller-tools/pkg/crd.(*Parser).NeedCRDFor(0xc0004040c0, {{0xc0003a9016, 0x8}, {0xc0001915d0, 0x5}}, 0x0)
    /Users/mnencia/prj/tmp/controller-tools/pkg/crd/spec.go:93 +0x57a
sigs.k8s.io/controller-tools/pkg/crd.Generator.Generate({0x0, 0x0, 0x0, {0x0, 0x0, 0x0}, 0x0, {0x0, 0x0}, {0x0, ...}, ...}, ...)
    /Users/mnencia/prj/tmp/controller-tools/pkg/crd/gen.go:182 +0x566
sigs.k8s.io/controller-tools/pkg/genall.(*Runtime).Run(0xc000378870)
    /Users/mnencia/prj/tmp/controller-tools/pkg/genall/genall.go:272 +0x234
main.main.func1(0xc0000b2300?, {0xc0001bb900?, 0x4?, 0x4e2fe55?})
    /Users/mnencia/prj/tmp/controller-tools/cmd/controller-gen/main.go:176 +0x6a
github.com/spf13/cobra.(*Command).execute(0xc000308c08, {0xc00018a280, 0x2, 0x2})
    /Users/mnencia/.asdf/installs/golang/1.23.1/packages/pkg/mod/github.com/spf13/cobra@v1.8.1/command.go:985 +0xaaa
github.com/spf13/cobra.(*Command).ExecuteC(0xc000308c08)
    /Users/mnencia/.asdf/installs/golang/1.23.1/packages/pkg/mod/github.com/spf13/cobra@v1.8.1/command.go:1117 +0x3ff
github.com/spf13/cobra.(*Command).Execute(...)
    /Users/mnencia/.asdf/installs/golang/1.23.1/packages/pkg/mod/github.com/spf13/cobra@v1.8.1/command.go:1041
main.main()
    /Users/mnencia/prj/tmp/controller-tools/cmd/controller-gen/main.go:200 +0x2f6
exit status 2

If you uncomment the anonymous import in the repro1 directory, the generation succeeds.

sbueringer commented 1 month ago

Please check if the panic also occurs with the published controller-gen binaries from here: https://github.com/kubernetes-sigs/controller-tools/releases/tag/v0.16.3 (compiled with Go 1.22).

Trying to figure out if this is just another occurence of: https://github.com/kubernetes-sigs/controller-tools/issues/1053 which might be then fixed via: https://github.com/kubernetes-sigs/controller-tools/pull/1061

YanniHu1996 commented 1 month ago

Hi @sbueringer I have tested it, and this issue still occurs in version 0.16.3

➜  ./bin/controller-gen --version
Version: v0.16.3
➜  ./bin/controller-gen rbac:roleName=manager-role crd webhook paths="./api/...;./controllers/..." output:crd:artifacts:config=config/crd/bases         
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0xd18e792]

goroutine 1 [running]:
sigs.k8s.io/controller-tools/pkg/crd.(*Parser).NeedPackage(0xc000e8c000, 0x0)
        /Users/huyantian/go/pkg/mod/sigs.k8s.io/controller-tools@v0.16.3/pkg/crd/parser.go:237 +0x52
sigs.k8s.io/controller-tools/pkg/crd.(*Parser).NeedSchemaFor(0xc000e8c000, {0x0, {0xc0015fe660, 0x1e}})
        /Users/huyantian/go/pkg/mod/sigs.k8s.io/controller-tools@v0.16.3/pkg/crd/parser.go:170 +0x5a
sigs.k8s.io/controller-tools/pkg/crd.(*schemaContext).requestSchema(0xc004da4480?, {0xc00086eb40?, 0xd2758f5?}, {0xc0015fe660?, 0x1e?})
        /Users/huyantian/go/pkg/mod/sigs.k8s.io/controller-tools@v0.16.3/pkg/crd/schema.go:108 +0xdb
sigs.k8s.io/controller-tools/pkg/crd.namedToSchema(0xc004da4480, 0xc000fd54d0)

sbueringer commented 1 month ago

Thx for checking!

Then we'll need someone to investigate this

/help

k8s-ci-robot commented 1 month ago

@sbueringer: This request has been marked as needing help from a contributor.

Guidelines

Please ensure that the issue body includes answers to the following questions:

Why are we solving this issue?
To address this issue, are there any code changes? If there are code changes, what needs to be done in the code and what places can the assignee treat as reference points?
Does this issue have zero to low barrier of entry?
How can the assignee reach out to you for help?

For more details on the requirements of such an issue, please see here and ensure that they are met.

If this request no longer meets these requirements, the label can be removed by commenting with the /remove-help command.

In response to [this](https://github.com/kubernetes-sigs/controller-tools/issues/1063): >Thx for checking! > >Then someone has to investigate this > >/help Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes-sigs/prow](https://github.com/kubernetes-sigs/prow/issues/new?title=Prow%20issue:) repository.

mtardy commented 1 month ago

Then we'll need someone to investigate this

/help

Here is some help

Trying to figure out if this is just another occurence of: #1053 which might be then fixed via: #1061

So I took a look since you mentioned that and it seems unrelated. It's not something that was broken by upgrading the Go version, and I tried to run the reproducer on older versions and the bug seems to be there as it produces a panic.

I took a look at it since it could have been as trivial to fix as my bug, but it seems more complicated and I think I lack context and would need to spend more time on this to understand.

The panic is happening here: https://github.com/kubernetes-sigs/controller-tools/blob/f79cfef8b62a3caca5e64e0e3b871f56d45ec315/pkg/crd/parser.go#L237

Because in this case pkg is nil. NeedPackage was called by NeedSchemaFor with typ TypeIdent equal to:

Package = *loader.Package nil
Name = "MyReproStruct"

And this is nil because in requestSchema earlier in the call stack: https://github.com/kubernetes-sigs/controller-tools/blob/f79cfef8b62a3caca5e64e0e3b871f56d45ec315/pkg/crd/schema.go#L103-L112

You have pkgPath being sigs.k8s.io/controller-tools/repro3 and pkg pointing to repro1 but for some reason the package is indeed missing from c.pkg.Imports()[pkgPath]

Now I don't really know how you parse and populate this imports part, I ended up in some visitImports function but I didn't spend the time to completely how you retrieve the dependency graph. I would expect that something is failing here since you have a case of: depA import depB which imports depC and it seems you don't have depA dependent on depC in your imports map (unless you explicitly import it as mnencia showed).

So I guess maybe you can give me pointers or someone that has more knowledge over this can write a patch themselves!

sbueringer commented 1 month ago

Thx for taking a look and sharing your results. Unfortunately, I also don't know more (have been helping maintaining CT for a while now, but unfortunately I always have to research from almost scratch to figure out what's going on :))

kubernetes-sigs / controller-tools

Panic parsing resources not directly imported #1063

Guidelines