konveyor / analyzer-lsp

Add-on that is focused on providing analysis based on the Language Server Protocol.
Apache License 2.0
12 stars 43 forks source link

[RFE] Improving yq provider's k8sResourceMatched capability #466

Open pranavgaikwad opened 8 months ago

pranavgaikwad commented 8 months ago

Currently, a rule to match a certain k8s resource using yq provider looks like this:

- message: Deprecated/removed Kubernetes API version 'extensions/v1beta1' is used for 'Deployment'. Consider using 'apps/v1'.
  ruleID: k8s-deprecated-api-001
  when:
    yaml.k8sResourceMatched:
        apiVersion: "extensions/v1beta1"
        kind: "Deployment"
        deprecatedIn: "v1.9.0"
        removedIn: "v1.16.0"
        replacementAPI: "apps/v1"

The parameters deprecatedIn, removedIn and replacementAPI make the provider's ability limited to only work for one use case of finding deprecations / removals. Additionally, the provider compares these values against the latest k8s version to decide whether or not to produce a violation. This implicit decision is hidden from the user.

First, we should be able to use k8sResourceMatched capability for more than just finding deprecations, removals and second, user should have full control over which versions of Kubernetes the rules are evaluated against (the provider shouldn't make an implicit choice).

I propose that we remove deprecatedIn, removedIn and replacementAPI parameters and only keep the definition of a k8s resource as input to k8sResourceMatched. Provider's responsibility becomes only to match the fields. It will be exactly similar to how Ansible's k8s module (thanks to @fabianvf) takes input like below:

  when:
    yaml.k8sResourceMatched:
        apiVersion: "extensions/v1beta1"
        kind: "Deployment"

To achieve the use case of finding deprecations / removals, users should make use of labels on the rules and use the labels to dynamically select rules based on what Kubernetes version they are running against. So the previous rule could become:

- message: Deprecated/removed Kubernetes API version 'extensions/v1beta1' is used for 'Deployment'. Consider using 'apps/v1'.
  ruleID: k8s-deprecated-api-001
  labels:
  - konveyor.io/target=kubernetes1.9+
  - deprecatedIn=1.9.0
  - removedIn=1.16.0
  when:
    yaml.k8sResourceMatched:
        apiVersion: "extensions/v1beta1"
        kind: "Deployment"

At the time of running analysis, based on which Kubernetes version the user is running against, the users can selectively choose this rule using --label-selector. For instance, if my target platform is kubernetes 1.21, I will run:

konveyor-analyzer --rules /my/ruleset/ --label-selector konveyor.io/target=kubernetes1.21

I do understand that there are limitations to specifying exact version ranges in labels right now. But I think we should be improving the label system in the engine instead of having each provider take such things as parameters in the interest of making rules easier to write and more flexible.

shawn-hurley commented 8 months ago

Can we use the labels in the rule message to tell a user what is happening?

If we could, this could significantly reduce the burden of creating these rules I think.

I also think we need the ability to run all the rules less then a certain number then for labels.

For instance I want to run all the rules that have been removed prior to k8s 1.25

pranavgaikwad commented 8 months ago

@shawn-hurley we already have some level of support for that (we did that to handle windup version ranges). We support + and - operators in labels. For instance, to match anything >= 1.25, you will have a label on the rule 1.25+

But this is limiting in that it does not help to specify a closed range which I think will be useful for this kind of use case.

shawn-hurley commented 8 months ago

hmm, we should a doc for that :)

but that does remove one of the concerns!