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.

Sign up for GitHub

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

Precompute IsNamespaceScoped to avoid expensive schema reads #4152

Merged
merged 2 commits into from
Sep 9, 2021
Merged

Precompute IsNamespaceScoped to avoid expensive schema reads #4152

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c727d5e
Precompute IsNamespaceScoped to avoid expensive schema reads
howardjohn Aug 29, 2021
0eb4c61
Add documentation
howardjohn Sep 3, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions kyaml/openapi/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,12 @@ The above command will update the [OpenAPI schema] and the [Kustomization schema
create a directory kubernetesapi/v1212 and store the resulting
swagger.json and swagger.go files there.

#### Precomputations

To avoid expensive schema lookups, some functions have precomputed results based on the schema. Unit tests
ensure these are kept in sync with the schema; if these tests fail you will need to follow the suggested diff
to update the precomputed results.

### Run all tests

At the top of the repository, run the tests.
Expand Down
98 changes: 97 additions & 1 deletion kyaml/openapi/openapi.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,95 @@ type openapiData struct {
schemaInit bool
}

// 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
// from the failure if it changes.
var precomputedIsNamespaceScoped = map[yaml.TypeMeta]bool{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how was this computed? did you have a script that parses the openapi document?

Copy link
Contributor

@natasha41575 natasha41575 Aug 30, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: if you had a script to do this, add the contents of the script to to kyaml/openapi/scripts/makeOpenApiInfoDotGo.sh, so that makeOpenApiInfoDotGo.sh adds the var precomputedIsNamespaceScoped to kyaml/openapi/kubernetesapi/openapiinfo.go, automatically. That way, we can recompute this variable every time we update the openapi data.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test output shows the exact map in go syntax, so I just copy and pasted from the failure. If it changes, the test will output the diff that needs to be updated

Copy link
Contributor

@natasha41575 natasha41575 Aug 30, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ack, thanks for the explanation.

The test output shows the exact map in go syntax, so I just copy and pasted from the failure. If it changes, the test will output the diff that needs to be updated

Can you please add this as a comment above the test, and also instructions for how to update the var in https://github.com/kubernetes-sigs/kustomize/blob/master/kyaml/openapi/README.md, perhaps a new subsection under Update the built-in schema to a new version?

{APIVersion: "admissionregistration.k8s.io/v1", Kind: "MutatingWebhookConfiguration"}: false,
{APIVersion: "admissionregistration.k8s.io/v1", Kind: "ValidatingWebhookConfiguration"}: false,
{APIVersion: "admissionregistration.k8s.io/v1beta1", Kind: "MutatingWebhookConfiguration"}: false,
{APIVersion: "admissionregistration.k8s.io/v1beta1", Kind: "ValidatingWebhookConfiguration"}: false,
{APIVersion: "apiextensions.k8s.io/v1", Kind: "CustomResourceDefinition"}: false,
{APIVersion: "apiextensions.k8s.io/v1beta1", Kind: "CustomResourceDefinition"}: false,
{APIVersion: "apiregistration.k8s.io/v1", Kind: "APIService"}: false,
{APIVersion: "apiregistration.k8s.io/v1beta1", Kind: "APIService"}: false,
{APIVersion: "apps/v1", Kind: "ControllerRevision"}: true,
{APIVersion: "apps/v1", Kind: "DaemonSet"}: true,
{APIVersion: "apps/v1", Kind: "Deployment"}: true,
{APIVersion: "apps/v1", Kind: "ReplicaSet"}: true,
{APIVersion: "apps/v1", Kind: "StatefulSet"}: true,
{APIVersion: "autoscaling/v1", Kind: "HorizontalPodAutoscaler"}: true,
{APIVersion: "autoscaling/v1", Kind: "Scale"}: true,
{APIVersion: "autoscaling/v2beta1", Kind: "HorizontalPodAutoscaler"}: true,
{APIVersion: "autoscaling/v2beta2", Kind: "HorizontalPodAutoscaler"}: true,
{APIVersion: "batch/v1", Kind: "CronJob"}: true,
{APIVersion: "batch/v1", Kind: "Job"}: true,
{APIVersion: "batch/v1beta1", Kind: "CronJob"}: true,
{APIVersion: "certificates.k8s.io/v1", Kind: "CertificateSigningRequest"}: false,
{APIVersion: "certificates.k8s.io/v1beta1", Kind: "CertificateSigningRequest"}: false,
{APIVersion: "coordination.k8s.io/v1", Kind: "Lease"}: true,
{APIVersion: "coordination.k8s.io/v1beta1", Kind: "Lease"}: true,
{APIVersion: "discovery.k8s.io/v1", Kind: "EndpointSlice"}: true,
{APIVersion: "discovery.k8s.io/v1beta1", Kind: "EndpointSlice"}: true,
{APIVersion: "events.k8s.io/v1", Kind: "Event"}: true,
{APIVersion: "events.k8s.io/v1beta1", Kind: "Event"}: true,
{APIVersion: "extensions/v1beta1", Kind: "Ingress"}: true,
{APIVersion: "flowcontrol.apiserver.k8s.io/v1beta1", Kind: "FlowSchema"}: false,
{APIVersion: "flowcontrol.apiserver.k8s.io/v1beta1", Kind: "PriorityLevelConfiguration"}: false,
{APIVersion: "networking.k8s.io/v1", Kind: "Ingress"}: true,
{APIVersion: "networking.k8s.io/v1", Kind: "IngressClass"}: false,
{APIVersion: "networking.k8s.io/v1", Kind: "NetworkPolicy"}: true,
{APIVersion: "networking.k8s.io/v1beta1", Kind: "Ingress"}: true,
{APIVersion: "networking.k8s.io/v1beta1", Kind: "IngressClass"}: false,
{APIVersion: "node.k8s.io/v1", Kind: "RuntimeClass"}: false,
{APIVersion: "node.k8s.io/v1beta1", Kind: "RuntimeClass"}: false,
{APIVersion: "policy/v1", Kind: "PodDisruptionBudget"}: true,
{APIVersion: "policy/v1beta1", Kind: "PodDisruptionBudget"}: true,
{APIVersion: "policy/v1beta1", Kind: "PodSecurityPolicy"}: false,
{APIVersion: "rbac.authorization.k8s.io/v1", Kind: "ClusterRole"}: false,
{APIVersion: "rbac.authorization.k8s.io/v1", Kind: "ClusterRoleBinding"}: false,
{APIVersion: "rbac.authorization.k8s.io/v1", Kind: "Role"}: true,
{APIVersion: "rbac.authorization.k8s.io/v1", Kind: "RoleBinding"}: true,
{APIVersion: "rbac.authorization.k8s.io/v1beta1", Kind: "ClusterRole"}: false,
{APIVersion: "rbac.authorization.k8s.io/v1beta1", Kind: "ClusterRoleBinding"}: false,
{APIVersion: "rbac.authorization.k8s.io/v1beta1", Kind: "Role"}: true,
{APIVersion: "rbac.authorization.k8s.io/v1beta1", Kind: "RoleBinding"}: true,
{APIVersion: "scheduling.k8s.io/v1", Kind: "PriorityClass"}: false,
{APIVersion: "scheduling.k8s.io/v1beta1", Kind: "PriorityClass"}: false,
{APIVersion: "storage.k8s.io/v1", Kind: "CSIDriver"}: false,
{APIVersion: "storage.k8s.io/v1", Kind: "CSINode"}: false,
{APIVersion: "storage.k8s.io/v1", Kind: "StorageClass"}: false,
{APIVersion: "storage.k8s.io/v1", Kind: "VolumeAttachment"}: false,
{APIVersion: "storage.k8s.io/v1beta1", Kind: "CSIDriver"}: false,
{APIVersion: "storage.k8s.io/v1beta1", Kind: "CSINode"}: false,
{APIVersion: "storage.k8s.io/v1beta1", Kind: "CSIStorageCapacity"}: true,
{APIVersion: "storage.k8s.io/v1beta1", Kind: "StorageClass"}: false,
{APIVersion: "storage.k8s.io/v1beta1", Kind: "VolumeAttachment"}: false,
{APIVersion: "v1", Kind: "ComponentStatus"}: false,
{APIVersion: "v1", Kind: "ConfigMap"}: true,
{APIVersion: "v1", Kind: "Endpoints"}: true,
{APIVersion: "v1", Kind: "Event"}: true,
{APIVersion: "v1", Kind: "LimitRange"}: true,
{APIVersion: "v1", Kind: "Namespace"}: false,
{APIVersion: "v1", Kind: "Node"}: false,
{APIVersion: "v1", Kind: "NodeProxyOptions"}: false,
{APIVersion: "v1", Kind: "PersistentVolume"}: false,
{APIVersion: "v1", Kind: "PersistentVolumeClaim"}: true,
{APIVersion: "v1", Kind: "Pod"}: true,
{APIVersion: "v1", Kind: "PodAttachOptions"}: true,
{APIVersion: "v1", Kind: "PodExecOptions"}: true,
{APIVersion: "v1", Kind: "PodPortForwardOptions"}: true,
{APIVersion: "v1", Kind: "PodProxyOptions"}: true,
{APIVersion: "v1", Kind: "PodTemplate"}: true,
{APIVersion: "v1", Kind: "ReplicationController"}: true,
{APIVersion: "v1", Kind: "ResourceQuota"}: true,
{APIVersion: "v1", Kind: "Secret"}: true,
{APIVersion: "v1", Kind: "Service"}: true,
{APIVersion: "v1", Kind: "ServiceAccount"}: true,
{APIVersion: "v1", Kind: "ServiceProxyOptions"}: true,
}

// ResourceSchema wraps the OpenAPI Schema.
type ResourceSchema struct {
// Schema is the OpenAPI schema for a Resource or field
Expand Down Expand Up @@ -265,10 +354,17 @@ func GetSchema(s string, schema *spec.Schema) (*ResourceSchema, error) {
// cluster-scoped by looking at the information in the openapi schema.
// The second return value tells whether the provided type could be found
// in the openapi schema. If the value is false here, the scope of the
// resource is not known. If the type if found, the first return value will
// resource is not known. If the type is found, the first return value will
// be true if the resource is namespace-scoped, and false if the type is
// cluster-scoped.
func IsNamespaceScoped(typeMeta yaml.TypeMeta) (bool, bool) {
if res, f := precomputedIsNamespaceScoped[typeMeta]; f {
return res, true
}
return isNamespaceScopedFromSchema(typeMeta)
}

func isNamespaceScopedFromSchema(typeMeta yaml.TypeMeta) (bool, bool) {
initSchema()
isNamespaceScoped, found := globalSchema.namespaceabilityByResourceType[typeMeta]
return isNamespaceScoped, found
Expand Down
9 changes: 9 additions & 0 deletions kyaml/openapi/openapi_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"io/ioutil"
"testing"

"github.com/google/go-cmp/cmp"
"github.com/stretchr/testify/assert"
"sigs.k8s.io/kustomize/kyaml/yaml"
)
Expand Down Expand Up @@ -271,6 +272,14 @@ func TestIsNamespaceScoped_builtin(t *testing.T) {
}
}

// TestIsNamespaceScopedPrecompute checks that the precomputed result meets the actual result
func TestIsNamespaceScopedPrecompute(t *testing.T) {
initSchema()
if diff := cmp.Diff(globalSchema.namespaceabilityByResourceType, precomputedIsNamespaceScoped); diff != "" {
t.Fatalf(diff)
}
}

func TestIsNamespaceScoped_custom(t *testing.T) {
SuppressBuiltInSchemaUse()
err := AddSchema([]byte(`
Expand Down