Skip to content

Commit

Permalink
fix: add lock for kubernetesOpenAPIVersion
Browse files Browse the repository at this point in the history
  • Loading branch information
pmalek committed Jan 10, 2023
1 parent 4456221 commit d035931
Showing 1 changed file with 27 additions and 4 deletions.
31 changes: 27 additions & 4 deletions kyaml/openapi/openapi.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"path/filepath"
"reflect"
"strings"
"sync"

openapi_v2 "github.com/google/gnostic/openapiv2"
"google.golang.org/protobuf/proto"
Expand All @@ -24,9 +25,6 @@ import (
// globalSchema contains global state information about the openapi
var globalSchema openapiData

// kubernetesOpenAPIVersion specifies which builtin kubernetes schema to use
var kubernetesOpenAPIVersion string

// customSchemaFile stores the custom OpenApi schema if it is provided
var customSchema []byte

Expand Down Expand Up @@ -59,6 +57,20 @@ const (
Proto format = "proto"
)

// kubernetesOpenAPIVersionLock lock for kubernetesOpenAPIVersion.
//
// NOTE: This lock helps with preventing panics that might occur due to the data
// race that concurrent access on this variable might cause but it doesn't
// fully fix the issue described in https://github.com/kubernetes-sigs/kustomize/issues/4824.
// For instance concurrently running goroutines where each of them calls SetSchema()
// and/or GetSchemaVersion might end up received nil errors (success) whereas the
// seconds one would overwrite the global variable that has been written by the
// first one.
var kubernetesOpenAPIVersionLock sync.RWMutex

// kubernetesOpenAPIVersion specifies which builtin kubernetes schema to use.
var kubernetesOpenAPIVersion string

// precomputedIsNamespaceScoped precomputes IsNamespaceScoped for known types. This avoids Schema creation,
// which is expensive
// The test output from TestIsNamespaceScopedPrecompute shows the expected map in go syntax,and can be copy and pasted
Expand Down Expand Up @@ -279,8 +291,11 @@ func AddSchema(s []byte) error {
// ResetOpenAPI resets the openapi data to empty
func ResetOpenAPI() {
globalSchema = openapiData{}
kubernetesOpenAPIVersion = ""
customSchema = nil

kubernetesOpenAPIVersionLock.Lock()
defer kubernetesOpenAPIVersionLock.Unlock()
kubernetesOpenAPIVersion = ""
}

// AddDefinitions adds the definitions to the global schema.
Expand Down Expand Up @@ -551,6 +566,9 @@ const (

// SetSchema sets the kubernetes OpenAPI schema version to use
func SetSchema(openAPIField map[string]string, schema []byte, reset bool) error {
kubernetesOpenAPIVersionLock.Lock()
defer kubernetesOpenAPIVersionLock.Unlock()

// this should only be set once
schemaIsSet := (kubernetesOpenAPIVersion != "") || customSchema != nil
if schemaIsSet && !reset {
Expand Down Expand Up @@ -588,6 +606,9 @@ func SetSchema(openAPIField map[string]string, schema []byte, reset bool) error

// GetSchemaVersion returns what kubernetes OpenAPI version is being used
func GetSchemaVersion() string {
kubernetesOpenAPIVersionLock.RLock()
defer kubernetesOpenAPIVersionLock.RUnlock()

switch {
case kubernetesOpenAPIVersion == "" && customSchema == nil:
return kubernetesOpenAPIDefaultVersion
Expand All @@ -612,6 +633,8 @@ func initSchema() {
panic("invalid schema file")
}
} else {
kubernetesOpenAPIVersionLock.RLock()
defer kubernetesOpenAPIVersionLock.RUnlock()
if kubernetesOpenAPIVersion == "" {
parseBuiltinSchema(kubernetesOpenAPIDefaultVersion)
} else {
Expand Down

0 comments on commit d035931

Please sign in to comment.