api: data race when calling Kustomizer.Run() from multiple goroutines

[pmalek]

Describe the bug

Data race occurs when Kustomizer.Run() is called from multiple goroutines.

The culprit seems to be in openapi.SetSchema() which manipulates the global variable: kubernetesOpenAPIVersion

Files that can reproduce the issue

	k := krusty.MakeKustomizer(krusty.MakeDefaultOptions())
	m, err := k.Run(filesys.MakeFsOnDisk(), path)

Expected output

No data race

Actual output

Data race.

Stack trace:

2022-10-07T00:06:29.6668305Z ==================
2022-10-07T00:06:29.6831615Z WARNING: DATA RACE
2022-10-07T00:06:29.7090851Z Read at 0x0000048e6d00 by goroutine 2050:
2022-10-07T00:06:29.7091666Z   sigs.k8s.io/kustomize/kyaml/openapi.SetSchema()
2022-10-07T00:06:29.7092542Z       /home/runner/go/pkg/mod/sigs.k8s.io/kustomize/kyaml@v0.13.9/openapi/openapi.go:555 +0x55
2022-10-07T00:06:29.7093591Z   sigs.k8s.io/kustomize/api/internal/target.(*KustTarget).accumulateDirectory()
2022-10-07T00:06:29.7094450Z       /home/runner/go/pkg/mod/sigs.k8s.io/kustomize/api@v0.12.1/internal/target/kusttarget.go:489 +0x419
2022-10-07T00:06:29.7095362Z   sigs.k8s.io/kustomize/api/internal/target.(*KustTarget).accumulateResources()
2022-10-07T00:06:29.7097449Z       /home/runner/go/pkg/mod/sigs.k8s.io/kustomize/api@v0.12.1/internal/target/kusttarget.go:429 +0x3d3
2022-10-07T00:06:29.7098652Z   sigs.k8s.io/kustomize/api/internal/target.(*KustTarget).accumulateTarget()
2022-10-07T00:06:29.7099520Z       /home/runner/go/pkg/mod/sigs.k8s.io/kustomize/api@v0.12.1/internal/target/kusttarget.go:188 +0xac
2022-10-07T00:06:29.7100663Z   sigs.k8s.io/kustomize/api/internal/target.(*KustTarget).AccumulateTarget()
2022-10-07T00:06:29.7104983Z       /home/runner/go/pkg/mod/sigs.k8s.io/kustomize/api@v0.12.1/internal/target/kusttarget.go:181 +0x22b
2022-10-07T00:06:29.7120296Z   sigs.k8s.io/kustomize/api/internal/target.(*KustTarget).makeCustomizedResMap()
2022-10-07T00:06:29.7121222Z       /home/runner/go/pkg/mod/sigs.k8s.io/kustomize/api@v0.12.1/internal/target/kusttarget.go:127 +0x117
2022-10-07T00:06:29.7121958Z   sigs.k8s.io/kustomize/api/internal/target.(*KustTarget).MakeCustomizedResMap()
2022-10-07T00:06:29.7125031Z       /home/runner/go/pkg/mod/sigs.k8s.io/kustomize/api@v0.12.1/internal/target/kusttarget.go:118 +0x456
2022-10-07T00:06:29.7125612Z   sigs.k8s.io/kustomize/api/krusty.(*Kustomizer).Run()
2022-10-07T00:06:29.7126358Z       /home/runner/go/pkg/mod/sigs.k8s.io/kustomize/api@v0.12.1/krusty/kustomizer.go:88 +0x449
2022-10-07T00:06:29.7127459Z   github.com/kong/kubernetes-testing-framework/pkg/utils/kubernetes/kubectl.runKustomize()
2022-10-07T00:06:29.7128575Z       /home/runner/work/kubernetes-testing-framework/kubernetes-testing-framework/pkg/utils/kubernetes/kubectl/kustomize.go:53 +0x218
2022-10-07T00:06:29.7129466Z   github.com/kong/kubernetes-testing-framework/pkg/utils/kubernetes/kubectl.GetKustomizedManifest()
2022-10-07T00:06:29.7131247Z       /home/runner/work/kubernetes-testing-framework/kubernetes-testing-framework/pkg/utils/kubernetes/kubectl/kustomize.go:43 +0x287
2022-10-07T00:06:29.7132361Z   github.com/kong/kubernetes-testing-framework/pkg/clusters/addons/metallb.getManifest()
2022-10-07T00:06:29.7133485Z       /home/runner/work/kubernetes-testing-framework/kubernetes-testing-framework/pkg/clusters/addons/metallb/metallb.go:328 +0x3ea
2022-10-07T00:06:29.7134380Z   github.com/kong/kubernetes-testing-framework/pkg/clusters/addons/metallb.metallbDeployHack()
2022-10-07T00:06:29.7135506Z       /home/runner/work/kubernetes-testing-framework/kubernetes-testing-framework/pkg/clusters/addons/metallb/metallb.go:363 +0x1f3
2022-10-07T00:06:29.7136465Z   github.com/kong/kubernetes-testing-framework/pkg/clusters/addons/metallb.deployMetallbForKindCluster()
2022-10-07T00:06:29.7137799Z       /home/runner/work/kubernetes-testing-framework/kubernetes-testing-framework/pkg/clusters/addons/metallb/metallb.go:167 +0x1fa
2022-10-07T00:06:29.7138663Z   github.com/kong/kubernetes-testing-framework/pkg/clusters/addons/metallb.(*addon).Deploy()
2022-10-07T00:06:29.7139930Z       /home/runner/work/kubernetes-testing-framework/kubernetes-testing-framework/pkg/clusters/addons/metallb/metallb.go:84 +0xed
2022-10-07T00:06:29.7140787Z   github.com/kong/kubernetes-testing-framework/pkg/clusters/types/kind.(*Cluster).DeployAddon()
2022-10-07T00:06:29.7141847Z       /home/runner/work/kubernetes-testing-framework/kubernetes-testing-framework/pkg/clusters/types/kind/cluster.go:122 +0x1dc
2022-10-07T00:06:29.7142650Z   github.com/kong/kubernetes-testing-framework/pkg/environments.(*Builder).Build.func1()
2022-10-07T00:06:29.7143664Z       /home/runner/work/kubernetes-testing-framework/kubernetes-testing-framework/pkg/environments/builder.go:154 +0x9b
2022-10-07T00:06:29.7144051Z 
2022-10-07T00:06:29.7144252Z Previous write at 0x0000048e6d00 by goroutine 2027:
2022-10-07T00:06:29.7144698Z   sigs.k8s.io/kustomize/kyaml/openapi.SetSchema()
2022-10-07T00:06:29.7145375Z       /home/runner/go/pkg/mod/sigs.k8s.io/kustomize/kyaml@v0.13.9/openapi/openapi.go:574 +0x245
2022-10-07T00:06:29.7145947Z   sigs.k8s.io/kustomize/api/internal/target.(*KustTarget).accumulateDirectory()
2022-10-07T00:06:29.7146722Z       /home/runner/go/pkg/mod/sigs.k8s.io/kustomize/api@v0.12.1/internal/target/kusttarget.go:489 +0x419
2022-10-07T00:06:29.7147538Z   sigs.k8s.io/kustomize/api/internal/target.(*KustTarget).accumulateResources()
2022-10-07T00:06:29.7148389Z       /home/runner/go/pkg/mod/sigs.k8s.io/kustomize/api@v0.12.1/internal/target/kusttarget.go:429 +0x3d3
2022-10-07T00:06:29.7148959Z   sigs.k8s.io/kustomize/api/internal/target.(*KustTarget).accumulateTarget()
2022-10-07T00:06:29.7149698Z       /home/runner/go/pkg/mod/sigs.k8s.io/kustomize/api@v0.12.1/internal/target/kusttarget.go:188 +0xac
2022-10-07T00:06:29.7150293Z   sigs.k8s.io/kustomize/api/internal/target.(*KustTarget).AccumulateTarget()
2022-10-07T00:06:29.7151025Z       /home/runner/go/pkg/mod/sigs.k8s.io/kustomize/api@v0.12.1/internal/target/kusttarget.go:181 +0x22b
2022-10-07T00:06:29.7151609Z   sigs.k8s.io/kustomize/api/internal/target.(*KustTarget).accumulateDirectory()
2022-10-07T00:06:29.7152348Z       /home/runner/go/pkg/mod/sigs.k8s.io/kustomize/api@v0.12.1/internal/target/kusttarget.go:509 +0xeb3
2022-10-07T00:06:29.7152949Z   sigs.k8s.io/kustomize/api/internal/target.(*KustTarget).accumulateResources()
2022-10-07T00:06:29.7153697Z       /home/runner/go/pkg/mod/sigs.k8s.io/kustomize/api@v0.12.1/internal/target/kusttarget.go:429 +0x3d3
2022-10-07T00:06:29.7154267Z   sigs.k8s.io/kustomize/api/internal/target.(*KustTarget).accumulateTarget()
2022-10-07T00:06:29.7154997Z       /home/runner/go/pkg/mod/sigs.k8s.io/kustomize/api@v0.12.1/internal/target/kusttarget.go:188 +0xac
2022-10-07T00:06:29.7155568Z   sigs.k8s.io/kustomize/api/internal/target.(*KustTarget).AccumulateTarget()
2022-10-07T00:06:29.7156315Z       /home/runner/go/pkg/mod/sigs.k8s.io/kustomize/api@v0.12.1/internal/target/kusttarget.go:181 +0x22b
2022-10-07T00:06:29.7156898Z   sigs.k8s.io/kustomize/api/internal/target.(*KustTarget).makeCustomizedResMap()
2022-10-07T00:06:29.7157641Z       /home/runner/go/pkg/mod/sigs.k8s.io/kustomize/api@v0.12.1/internal/target/kusttarget.go:127 +0x117
2022-10-07T00:06:29.7158215Z   sigs.k8s.io/kustomize/api/internal/target.(*KustTarget).MakeCustomizedResMap()
2022-10-07T00:06:29.7198895Z       /home/runner/go/pkg/mod/sigs.k8s.io/kustomize/api@v0.12.1/internal/target/kusttarget.go:118 +0x456
2022-10-07T00:06:29.7199528Z   sigs.k8s.io/kustomize/api/krusty.(*Kustomizer).Run()
2022-10-07T00:06:29.7200240Z       /home/runner/go/pkg/mod/sigs.k8s.io/kustomize/api@v0.12.1/krusty/kustomizer.go:88 +0x449
2022-10-07T00:06:29.7201098Z   github.com/kong/kubernetes-testing-framework/pkg/utils/kubernetes/kubectl.runKustomize()
2022-10-07T00:06:29.7202426Z       /home/runner/work/kubernetes-testing-framework/kubernetes-testing-framework/pkg/utils/kubernetes/kubectl/kustomize.go:53 +0x218
2022-10-07T00:06:29.7203378Z   github.com/kong/kubernetes-testing-framework/pkg/utils/kubernetes/kubectl.GetKustomizedManifest()
2022-10-07T00:06:29.7204536Z       /home/runner/work/kubernetes-testing-framework/kubernetes-testing-framework/pkg/utils/kubernetes/kubectl/kustomize.go:43 +0x287
2022-10-07T00:06:29.7205404Z   github.com/kong/kubernetes-testing-framework/pkg/clusters/addons/metallb.getManifest()
2022-10-07T00:06:29.7206521Z       /home/runner/work/kubernetes-testing-framework/kubernetes-testing-framework/pkg/clusters/addons/metallb/metallb.go:328 +0x3ea
2022-10-07T00:06:29.7207434Z   github.com/kong/kubernetes-testing-framework/pkg/clusters/addons/metallb.metallbDeployHack()
2022-10-07T00:06:29.7208549Z       /home/runner/work/kubernetes-testing-framework/kubernetes-testing-framework/pkg/clusters/addons/metallb/metallb.go:363 +0x1f3
2022-10-07T00:06:29.7209613Z   github.com/kong/kubernetes-testing-framework/pkg/clusters/addons/metallb.deployMetallbForKindCluster()
2022-10-07T00:06:29.7210768Z       /home/runner/work/kubernetes-testing-framework/kubernetes-testing-framework/pkg/clusters/addons/metallb/metallb.go:167 +0x1fa
2022-10-07T00:06:29.7211662Z   github.com/kong/kubernetes-testing-framework/pkg/clusters/addons/metallb.(*addon).Deploy()
2022-10-07T00:06:29.7212749Z       /home/runner/work/kubernetes-testing-framework/kubernetes-testing-framework/pkg/clusters/addons/metallb/metallb.go:84 +0xed
2022-10-07T00:06:29.7213829Z   github.com/kong/kubernetes-testing-framework/pkg/clusters/types/kind.(*Cluster).DeployAddon()
2022-10-07T00:06:29.7215033Z       /home/runner/work/kubernetes-testing-framework/kubernetes-testing-framework/pkg/clusters/types/kind/cluster.go:122 +0x1dc
2022-10-07T00:06:29.7215905Z   github.com/kong/kubernetes-testing-framework/pkg/environments.(*Builder).Build.func1()
2022-10-07T00:06:29.7217602Z       /home/runner/work/kubernetes-testing-framework/kubernetes-testing-framework/pkg/environments/builder.go:154 +0x9b
2022-10-07T00:06:29.7218319Z 
2022-10-07T00:06:29.7218658Z Goroutine 2050 (running) created at:
2022-10-07T00:06:29.7219484Z   github.com/kong/kubernetes-testing-framework/pkg/environments.(*Builder).Build()
2022-10-07T00:06:29.7220555Z       /home/runner/work/kubernetes-testing-framework/kubernetes-testing-framework/pkg/environments/builder.go:153 +0xd98
2022-10-07T00:06:29.7221424Z   github.com/kong/kubernetes-testing-framework/test/integration.TestKindClusterProxyOnly()
2022-10-07T00:06:29.7222550Z       /home/runner/work/kubernetes-testing-framework/kubernetes-testing-framework/test/integration/kind_cluster_test.go:102 +0xe99
2022-10-07T00:06:29.7223408Z   testing.tRunner()
2022-10-07T00:06:29.7224002Z       /opt/hostedtoolcache/go/1.19.1/x64/src/testing/testing.go:1446 +0x216
2022-10-07T00:06:29.7224447Z   testing.(*T).Run.func1()
2022-10-07T00:06:29.7225108Z       /opt/hostedtoolcache/go/1.19.1/x64/src/testing/testing.go:1493 +0x47
2022-10-07T00:06:29.7225366Z 
2022-10-07T00:06:29.7225532Z Goroutine 2027 (running) created at:
2022-10-07T00:06:29.7226232Z   github.com/kong/kubernetes-testing-framework/pkg/environments.(*Builder).Build()
2022-10-07T00:06:29.7227458Z       /home/runner/work/kubernetes-testing-framework/kubernetes-testing-framework/pkg/environments/builder.go:153 +0xd98
2022-10-07T00:06:29.7228352Z   github.com/kong/kubernetes-testing-framework/test/integration.TestKongEnterprisePostgres()
2022-10-07T00:06:29.7229484Z       /home/runner/work/kubernetes-testing-framework/kubernetes-testing-framework/test/integration/enterprise_test.go:46 +0xfca
2022-10-07T00:06:29.7230018Z   testing.tRunner()
2022-10-07T00:06:29.7230689Z       /opt/hostedtoolcache/go/1.19.1/x64/src/testing/testing.go:1446 +0x216
2022-10-07T00:06:29.7231132Z   testing.(*T).Run.func1()
2022-10-07T00:06:29.7231711Z       /opt/hostedtoolcache/go/1.19.1/x64/src/testing/testing.go:1493 +0x47

Kustomize version

sigs.k8s.io/kustomize/api v0.12.1

Additional context

Related issue: Kong/kubernetes-testing-framework#404

If that's the intended behaviour then at the very least the API should have a note saying that this cannot be called from multiple goroutines.

[jrsmroz]

This package

kustomize/kyaml/openapi/openapi.go

Line 28 in 2ec4b97

var kubernetesOpenAPIVersion string

uses global variables. It sets and reads kubernetesOpenAPIVersion in many places.

[KnVerey]

I agree with your assessment of why this is happening, and that ideally we wouldn't use global variables (these days we have a linter in place to flag them to help prevent problems like this from being introduced). I suspect the fix may be quite involved though, as performance is a big issue with openapi; we'd likely need the Kustomize layer to start holding the state the global variable is being used for, and any PR would need to have performance benchmarks. I'll accept the issue, but for transparency I don't think this is likely to be taken on by the Kustomize team ourselves; our bandwidth is very limited and this doesn't affect the kustomize or kubectl kustomize CLIs themselves.

/triage accepted
/help

[k8s-ci-robot]

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

I agree with your assessment of why this is happening, and that ideally we wouldn't use global variables (these days we have a linter in place to flag them to help prevent problems like this from being introduced). I suspect the fix may be quite involved though, as performance is a big issue with openapi; we'd likely need the Kustomize layer to start holding the state the global variable is being used for, and any PR would need to have performance benchmarks. I'll accept the issue, but for transparency I don't think this is likely to be taken on by the Kustomize team ourselves; our bandwidth is very limited and this doesn't affect the kustomize or kubectl kustomize CLIs themselves.

/triage accepted
/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/test-infra repository.

[pmalek]

Understood. Thanks for your response. How would you feel about introducing a lock on that variable, and making it be accessed/set from a func that controls this lock? Wouldn't that help?

[KnVerey]

Yes, you're right, that should also work and I would be happy to accept a contribution to that effect. If you're interested, please /assign yourself to this issue. 😄

[pmalek]

Submitted #4967 @KnVerey. PTAL