yaml-language-server

yaml-language-server copied to clipboard

What does this PR do?

This PR allows to set custom Kubernetes schema versions and Kubernetes Custom Resource Definitions (CRDs).

How Does it Work?

This PR extends https://github.com/redhat-developer/yaml-language-server/pull/824 by introducing a new yamlSettings.kubernetesSchemaURLs setting and its corresponding notification, e.g:

{
    "yaml.kubernetesSchemaURLs": [ 
        "https://raw.githubusercontent.com/tricktron/CRDs-catalog/f-openshift-v4.11/openshift.io/v4.11/all.json",
        "https://raw.githubusercontent.com/yannh/kubernetes-json-schema/master/v1.26.1-standalone-strict/all.json" 
    ]
}

What issues does this PR fix or reference?

• Fixes https://github.com/redhat-developer/yaml-language-server/issues/778
• Fixes https://github.com/redhat-developer/yaml-language-server/issues/211
• Fixes https://github.com/redhat-developer/yaml-language-server/issues/307

Is it tested? How?

• Yes I added three tests.
• Manually building the vscode extension and this yaml language server and then using/testing it.

Why not allow one to specify, within the settings JSON under the kubernetes property, either a simple string value (currently a glob) or a map specifying the version(s) of schemas to be used?

The default configuration would be as it is today, and would simply use the latest released version of the schema. But, with the proposal above, the configuration could look like (note the syntax is using formal JSON syntax to be compatible with coc.nvim, but this could equally apply to VS Code, too, by not using quotes around the property names):

{
    "yaml.settings": {
        "kubernetes": {
            "versions": [
                "1.22.3",
                "1.22.4",
                "1.22.5"
            ],
            "files": "*.ya?ml"
        }
    }
}

NOTE
I was struggling to name the property containing the glob. I'm not married to the name, just had to put something there ;)

I am not thrilled with the idea that a segment in the URL changes the meaning of the setting. It is prone to users unintentionally defining Kubernetes schema. I would rather have an intentional setting to manage Kubernetes schemas. Perhaps a combination of what is proposed on #824 and what @fourpastmidnight is proposing.

OTOH, I am a bit torn about Kubernetes support. Kubernetes is the only built-in schema that we have on yaml-language-server and this is due to the reasons how I had bootstrapped the project initially. Once we implemented a generic YAML LSP, the built-in Kubernetes support became an outlier.

I thought it was strange that this lsp server explicitly supported Kubernetes while providing a generic mechanism for other YAML schemas. I wasn't sure if there was any appetite for removing this support, or plans for adding explicit support for other "popular"types.

I also think it would be better to remove explicit Kubernetes support as well in light of no plans for explicitly supporting other schemas in lieu of the generic mechanism. I only made the suggestion above based on the current implementation.

@gorkem

I am not thrilled with the idea that a segment in the URL changes the meaning of the setting. It is prone to users unintentionally defining Kubernetes schema. I would rather have an intentional setting to manage Kubernetes schemas.

I thought about this as well. I went with this approach because it is a non-breaking API change, e.g. the clients do not have to change anything. I was unable to come up with an intentional non-breaking change.

Maybe we can open a new pull request for the intentional kubernetes change in a second step?

If we document the magic kubernetes regex in the readme for now, e.g. in a separate Kubernetes support section, then I think this is good enough because allowing to specify the exact Kubernetes version is a great enhancement.

Coverage: 83.286% (+0.05%) from 83.241% when pulling 596b92634a8a4a203cdf14f4d960ef2083c821ef on tricktron:f-custom-kubernetes-schema into 762209ccdfca713d203ead757698a47ad3cabf50 on redhat-developer:main.

@gorkem @fourpastmidnight Any updates?

@gorkem @fourpastmidnight @msivasubramaniaan I completely reworked this PR on the basis of https://github.com/redhat-developer/yaml-language-server/pull/824. What do you think?

Hi, what is the status of this pr? I would like to have this feature. We are running Kubernetes in Version 1.27. There happened some changes since the hard coded version 👀

Hi, what is the status of this pr? I would like to have this feature. We are running Kubernetes in Version 1.27. There happened some changes since the hard coded version 👀

I'll rebase and fix the code scanning results so that it is ready for merge again.

Ok. I rebased it. @gorkem @fourpastmidnight @msivasubramaniaan Could you run the build again and maybe give some feedback😃.

Any update @gorkem @fourpastmidnight @msivasubramaniaan?

any updates?

@msivasubramaniaan I have seen that you recently merged https://github.com/redhat-developer/vscode-yaml/pull/912 which depends on this pull request. Could you merge this as well?

@msivasubramaniaan I rebased again because I suspect that the merge of main through the GUI somehow triggered the duplicate items issue in the upload sarif stage.

If the same duplicate items issue happens again, then it might be worth to look at this similar issue: https://github.com/microsoft/sarif-js-sdk/issues/86.

Any updates?