kubeconform icon indicating copy to clipboard operation
kubeconform copied to clipboard

Cache schemas to disk

Open Nitive opened this issue 4 years ago • 11 comments

Hello!

Please add caching resource schemas to disk. It's typical for such an instrument to be run between different projects and downloading the same schemas every time affects performance. It also become very slow on bad internet connection

Nitive avatar Dec 29 '20 16:12 Nitive

Hi Nitive! You could actually run kubeconform without an internet connection. You would need to copy the right folder from https://github.com/instrumenta/kubernetes-json-schema to a local folder, and then run kubeconform like this:

./bin/kubeconform -schema-location '/path/to/your/local/copy/{{ .NormalizedKubernetesVersion }}-standalone{{ .StrictSuffix }}/{{ .ResourceKind }}{{ .KindSuffix }}.json' folder

If you stick to a single kubernetes version it shouldnt be big enough to be a concern!

Note: I might add an integration test + documentation for this use case.

yannh avatar Dec 29 '20 16:12 yannh

Yeah I can download schemas and use local copy but it's not very convenient.

It would be okey if downloading schemas was one-time thing but it isn't.

  1. New resources appear when we update kuberentes version
  2. Custom resources come and go

I was hoping to upload schemas to some server and let kubeconform download and cache it. When a new version of schemas is released, I would change schemas url for kubeconform to redownload it

  kubeconform -strict -schema-location https://kubernetesjsonschema.dev -schema-location \
-   https://kubernetesjsonschema.my-company.com/v1
+   https://kubernetesjsonschema.my-company.com/v2

Also kubeconform can do caching more efficiently because it can download only schemas resources that actually used, not everything that kubernetes support

Nitive avatar Dec 30 '20 09:12 Nitive

Agree - just pointing out that "everything kubernetes support" is only about 50MB for the whole folder from kubernetes-json-schema (per k8s version) - less if you strip out the files you dont need (for example non -strict files if you use -strict, etc). It's a bit of work to maintain, that's true.

There are a couple ways to implement a cache. There actually already is an in-memory cache - but it caches the parsed schemas, not the schemas. I could persist that to disk, but that would mean it would be a binary cache. Or I could add a second layer of cache to the HTTP registry driver :thinking:

This will take a little bit of time to implement, I need to think about it a bit more :)

yannh avatar Dec 30 '20 10:12 yannh

@Nitive I made a PoC here https://github.com/yannh/kubeconform/pull/24/files - you specify a folder to cache schemas with -cache, the filename is the md5 checksum of the URL, and contains the schema in clear text. The existing cache also caches things like 404 and would probably never detect new files.

The cache I implemented here never expires, it assumes the files at a given URL never change. Would this work for you? Maybe as a future iteration I could implement cache-control header...

EDIT: Refactored this a little bit so that the in-memory and on-disk cache use the same interface. If you don't mind building this from source feel free to give the branch a shot. I'll merge this in some time when I have given this another round of thoughts :)

yannh avatar Jan 01 '21 13:01 yannh

Merged, I made a new minor release v0.4.2, feel free to try it out!

yannh avatar Jan 02 '21 11:01 yannh

Thank you very much! Validation works much faster with disk cache!

I tried it out and have some proposals for improvements

  1. It would be nice to write cache to default location based on OS (Linux: ~/.cache, macOS: ~/Library/Caches, Windows: %LOCALAPPDATA%). There is XDG spec and Go module which should make implementation easier. This will allow users to put kubeconform in Makefile and use it on every OS without having to tweak it individually.
  2. It's possible that later there will be another cache (for example validation results) so it would be good to change schema cache location {cache-directory}{cache-directory}/schemas
  3. There is an error when provided cache directory doesn't exists. It probably would be better UX to just create such directory
  4. We use $ref for metadata field in schemas for custom resources
"metadata": {
  "$ref": "https://kubernetesjsonschema.dev/v1.14.0/_definitions.json#/definitions/io.k8s.apimachinery.pkg.apis.meta.v1.ObjectMeta",
  "description": "Standard object metadata."
},

Refs' schemas don't seem to be cached. I have written a test to show the problem and I'll create a PR soon (updated: #26)


The existing cache also caches things like 404 and would probably never detect new files

This could be a problem, especially if 5xx errors are cached. I took a look at HTTP status codes and seems like safely cached can be only 200, 201, 202, 203, 204, 206, 207, 208, 226.

The cache I implemented here never expires, it assumes the files at a given URL never change. Would this work for you? Maybe as a future iteration I could implement cache-control header...

Thank you, it suits me very well. I really like how cache is implemented in Deno JS runtime: everything is cached forever but there is an easy way to clear the cache --reload flag and also deno clean command (currently proposal). Kubernetes by default caches docker images the same way (imagePullPolicy: IfNotPresent). Implementing Cache-Control seems like overkill.

I'm ready to contribute those features if you like, starting with the easiest ones to get familiar with the code.

Nitive avatar Jan 07 '21 12:01 Nitive

Looks like 404 responses do not get cached, I've added test for that in #28

Nitive avatar Jan 08 '21 12:01 Nitive

It's on purpose :grimacing: for full offline capabilities I d really rather work on a lean way to get the required schemas...

yannh avatar Jan 08 '21 13:01 yannh

1 & 3: I thought about this, and I m not sure. Creating the folder might mean that it would create files in the wrong place if it is misconfigured. Forcing it to write to a folder that exists ensures that you are indeed writing to the correct folder. On the location itself - all these would not exist in the context of a Docker container, so we would be building more complex logic here. If you are looking for something cross platform, you could probably conditionally set a CACHE_FOLDER variable, (warning, not tested):

PLATFORM := $(shell uname)
ifeq ($(PLATFORM),Linux)
  export KUBECONFORM_CACHE='~/.cache/kubeconform' 
else ifeq ($(PLATFORM),Darwin)
  export KUBECONFORM_CACHE='~/Library/Caches/kubeconform'
else
  export KUBECONFORM_CACHE='./cache'
endif
mkdir -p ${KUBECONFORM_CACHE}
kubeconform -cache ${KUBECONFORM_CACHE} ....

4: Are you sure about the caching of refs not happening? I just merged the "offline" tests, do you think you could make a failing test?

2: I think we can change that when we do, I'd rather keep it as simple as possible for now?

yannh avatar Jan 09 '21 12:01 yannh

Are you sure about the caching of refs not happening? I just merged the "offline" tests, do you think you could make a failing test?

#30

Nitive avatar Jan 09 '21 13:01 Nitive

I thought about this, and I m not sure. Creating the folder might mean that it would create files in the wrong place if it is misconfigured. Forcing it to write to a folder that exists ensures that you are indeed writing to the correct folder.

I think it would be best for users do not configure anything for cache and let kubeconform handle it. This is how most tools work. For example kubectl keeps its cache in ~/.kube/cache and user don't have to create this directory. Helm uses XDG Spec and also creates directory by itself.

If you are looking for something cross platform, you could probably conditionally set a CACHE_FOLDER variable

Maybe I'll do that but I would try to avoid adding complexity and duplication to Makefiles. It's better to have simple configuration (-cache kubeconform-cache + .gitignore) in every project and worse performance than better performance and complex configuration

I think we can change that when we do, I'd rather keep it as simple as possible for now?

No problem. The only thing that bothers me is that when we do it, people will have to redownload cache but I don't think it's very important to prevent that

Nitive avatar Jan 09 '21 13:01 Nitive

Closing as implemented so far - I would consider PRs improving on the current state, but I am unlikely to spend more time on it myself since I personally do not need it.

yannh avatar Oct 16 '22 13:10 yannh