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

apimachinery's unstructured converter panics if the destination struct contains private fields #124154

Open
hugoShaka opened this issue Apr 2, 2024 · 2 comments
Labels
kind/bug Categorizes issue or PR as related to a bug. sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. triage/accepted Indicates an issue or PR is ready to be actively worked on.

Comments

@hugoShaka
Copy link

hugoShaka commented Apr 2, 2024

What happened?

Using the DefaultUnstructuredConverter with a destination struct containing a non-exported field throws a panic.

panic: reflect: reflect.Value.Set using value obtained using unexported field

goroutine 1 [running]:
reflect.flag.mustBeAssignableSlow(0x140000337a8?)
        /usr/local/go/src/reflect/value.go:269 +0xb4
reflect.flag.mustBeAssignable(...)
        /usr/local/go/src/reflect/value.go:259
reflect.Value.Set({0x100f4c3e0?, 0x14000169800?, 0x14000011168?}, {0x100f4c3e0?, 0x101232e40?, 0x98?})
        /usr/local/go/src/reflect/value.go:2319 +0x58
k8s.io/apimachinery/pkg/runtime.structFromUnstructured({0x100f63c60?, 0x140001697d0?, 0x1012f0248?}, {0x100f7e000?, 0x14000169800?, 0x100c2d5f8?}, 0x14000033e60)
        /Users/shaka/go/pkg/mod/k8s.io/apimachinery@v0.29.3/pkg/runtime/converter.go:556 +0x490
k8s.io/apimachinery/pkg/runtime.fromUnstructured({0x100f63c60?, 0x140001697d0?, 0x1400001115c?}, {0x100f7e000?, 0x14000169800?, 0x98?}, 0x14000033e60)
        /Users/shaka/go/pkg/mod/k8s.io/apimachinery@v0.29.3/pkg/runtime/converter.go:359 +0x2c4
k8s.io/apimachinery/pkg/runtime.structFromUnstructured({0x100f63c60?, 0x140001697a0?, 0x14000033c48?}, {0x100f71840?, 0x14000169800?, 0x100cabbf4?}, 0x14000033e60)
        /Users/shaka/go/pkg/mod/k8s.io/apimachinery@v0.29.3/pkg/runtime/converter.go:550 +0x64c
k8s.io/apimachinery/pkg/runtime.fromUnstructured({0x100f63c60?, 0x140001697a0?, 0x10?}, {0x100f71840?, 0x14000169800?, 0x100c35cec?}, 0x14000033e60)
        /Users/shaka/go/pkg/mod/k8s.io/apimachinery@v0.29.3/pkg/runtime/converter.go:359 +0x2c4
k8s.io/apimachinery/pkg/runtime.(*unstructuredConverter).FromUnstructuredWithValidation(0x1011c8df0, 0x140001697a0, {0x100f44b20, 0x14000169800}, 0x1)
        /Users/shaka/go/pkg/mod/k8s.io/apimachinery@v0.29.3/pkg/runtime/converter.go:247 +0x20c
main.main()
        /Users/shaka/playground/repro-runtime-panic/main.go:30 +0x110

I might be in a specific edge case, but we use protobuf to generate gRPC structs. Those structs contain private fields holding the grpc state and cause the converter to panic, even if the unstructured object only contains the public user-facing fields.

What did you expect to happen?

I would expect the converter to ignore private fields and not panic.

How can we reproduce it (as minimally and precisely as possible)?

package main

import (
	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
	"k8s.io/apimachinery/pkg/runtime"
	"log"
)

type StructuredObject struct {
	Spec StructuredObjectSpec
}

type StructuredObjectSpec struct {
	bar []byte
	Foo string
}

func main() {
	u := &unstructured.Unstructured{
		Object: map[string]interface{}{
			"spec": map[string]interface{}{
				"foo": "value",
			},
		},
	}

	structured := StructuredObject{}
	err := runtime.DefaultUnstructuredConverter.FromUnstructuredWithValidation(
		u.Object,
		&structured,
		true, /* returnUnknownFields */
	)
	if err != nil {
		log.Fatal(err)
	}
}

Anything else we need to know?

I can send a PR if you confirm this should be fixed and that the fix is to skip the private fields.

Kubernetes version

The go module looks like

go 1.22

require k8s.io/apimachinery v0.29.3

Cloud provider

No cloud provider.

OS version

No response

Install tools

Go 1.22

Container runtime (CRI) and version (if applicable)

No response

Related plugins (CNI, CSI, ...) and versions (if applicable)

No response

@hugoShaka hugoShaka added the kind/bug Categorizes issue or PR as related to a bug. label Apr 2, 2024
@k8s-ci-robot k8s-ci-robot added needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Apr 2, 2024
@hugoShaka
Copy link
Author

/sig api-machinery

@k8s-ci-robot k8s-ci-robot added sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. and removed needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Apr 2, 2024
@alexzielenski
Copy link
Contributor

/cc @sttts
/triage accepted

@k8s-ci-robot k8s-ci-robot added triage/accepted Indicates an issue or PR is ready to be actively worked on. and removed needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Apr 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Categorizes issue or PR as related to a bug. sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. triage/accepted Indicates an issue or PR is ready to be actively worked on.
Projects
None yet
Development

No branches or pull requests

3 participants