Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Panic parsing resources not directly imported #1063

Open
mnencia opened this issue Sep 20, 2024 · 6 comments
Open

Panic parsing resources not directly imported #1063

mnencia opened this issue Sep 20, 2024 · 6 comments
Labels
help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines.

Comments

@mnencia
Copy link

mnencia commented Sep 20, 2024

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/[email protected]/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/[email protected]/command.go:1117 +0x3ff
github.com/spf13/cobra.(*Command).Execute(...)
	/Users/mnencia/.asdf/installs/golang/1.23.1/packages/pkg/mod/github.com/spf13/[email protected]/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
Copy link
Member

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: #1053
which might be then fixed via: #1061

@YanniHu1996
Copy link

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/[email protected]/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/[email protected]/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/[email protected]/pkg/crd/schema.go:108 +0xdb
sigs.k8s.io/controller-tools/pkg/crd.namedToSchema(0xc004da4480, 0xc000fd54d0)

@sbueringer
Copy link
Member

sbueringer commented Sep 24, 2024

Thx for checking!

Then we'll need someone to investigate this

/help

@k8s-ci-robot
Copy link
Contributor

@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:

Thx for checking!

Then someone has to investigate this

/help

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-sigs/prow repository.

@k8s-ci-robot k8s-ci-robot added the help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. label Sep 24, 2024
@mtardy
Copy link
Contributor

mtardy commented Sep 26, 2024

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:

if override, overridden := p.PackageOverrides[loader.NonVendorPath(pkg.PkgPath)]; overridden {

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:

func (c *schemaContext) requestSchema(pkgPath, typeName string) {
pkg := c.pkg
if pkgPath != "" {
pkg = c.pkg.Imports()[pkgPath]
}
c.schemaRequester.NeedSchemaFor(TypeIdent{
Package: pkg,
Name: typeName,
})
}

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
Copy link
Member

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 :))

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines.
Projects
None yet
Development

No branches or pull requests

5 participants