kubeshop/monokle-core

Validation rule for deprecated resource apiVersions

Closed this issue · 4 comments

olensmar commented

When validating my k8s resources against a specific k8s version I want to know which resources that are no longer supported by that version - preferably via a dedicated rule that should be enabled by default

WitoDelnat commented

some additional notes on how this could be tackled

This is a total new feature that requires us to compare between 2 versions and we’d have to go over Kubernetes deprecation guide to add them manually.

I’d still keep it as part of the kubernetes-schema plugin probably as it’s nicely related. Probably as a second rule?

  • KSV001: schema-violated
  • KSV002: deprecation-violated -> This needs a rule configuration where you define to version and compare that with the original schema.

Additional discussion

Just to reiterate. What is the problem of analyzing the schema, finding storage.k8s.io/v1beta1 while your config files uses storage.k8s.io/v1alpha1…. If the API is not in the K8s schema, but the Kind is, you know is a misconfiguration.
For deprecations, the description adds DEPRECATED, but I don’t know if that is enough or we should look for additional information

Because it might be that an object is perfectly valid but we simply do not have the schema for it (definitely for CRDs). What we could do is add a strict mode that gives warnings for objects with a missing schema? Could even be a bit more strict on the core Kubernetes schemas because they should be exhaustive!?

I think there is a difference between not finding storage.k8s.io in any version, and finding a mismatched version. The first case: we don’t know if it is wrong or we just don’t have knowledge of it. In the second case, we know that we don’t know the version. If they want to understand that version, they need to install a new version of the CRD that includes it

WitoDelnat commented

It might even be handy to add a strict rule for CRDs as well. You could have a warning "This resource is not validated.".

f1ames commented

Some clarifications/notes after I started working with this:

So AFAIU there are two aspects of having deprecated resources. One is using deprecated/removed apiVersion which should be validated. And the other one (like next step of the process) is, after updating apiVersion validating the schema and showing which properties are no longer valid.

The deprecation-violated rule is about the first case - we want to validate if schema is using deprecated/removed apiVersion for any resources for given Kuberentes version.

And validating if given resource complies with schema, then KSV001: schema-violated rule (kubernetes-schema validator) is used (which we have already).

So the intention of the "deprecation" rule is to check if any resource is using outdated apiVersion only.

The idea is simple: deprecated = warning, removed = error.

Not sure if I understand correctly, but since it will be a single rule K8S002: deprecation-violated it can't be both at the same time? So for this we should have separate rules for deprecation and removals? Not sure if that was the intention here.

go over Kubernetes deprecation guide to add them manually.

Mentioned guide contains a list of removed APIs by version -- https://kubernetes.io/docs/reference/using-api/deprecation-guide/#removed-apis-by-release.

While list of deprecations can be found in CHANGELOG for each version, e.g. https://github.com/kubernetes/kubernetes/blob/master/CHANGELOG/CHANGELOG-1.25.md#deprecation

From strictly technical perspective I think we can either adjust the schemas we have (e.g. https://plugins.monokle.com/schemas/v1.26.3-standalone/deployment.json) to define allowed apiVersion in enum like:

{
  "description": "...",
  "properties": {
    "apiVersion": {
      "description": "...",
      "type": [
        "string",
        "null"
      ],
      "enum": [
        "apps/v1"
      ]
    },
    "kind": {
      "description": "...",
      "type": [
        "string",
        "null"
      ],
      "enum": [
        "Deployment"
      ]
    },
    ...
}

but this won't allow to store any additional data 🤔

Or go with custom logic for deprecation detection (and deprecation definitions). For now I went with the latter for PoC.

f1ames commented

Just to reiterate a bit after talking with @WitoDelnat. There are few scenarios here:

  1. Deprecations - deprecated apiVersion values which can be still used for given K8s version but will be removed in the future releases. For resources which have both kind and apiVersion declared.
  2. Removals - apiVersion values which were removed from the given K8s version. This means the resources using it will error and will not be created (AFAIU?). For resources which have both kind and apiVersion declared.
  3. "Strict mode" - resource has kind, but doesn't have apiVersion.
    • Rule will be triggered for any resource which has kind but doesn't have apiVersion. Not having apiVersion means the resource is not validated which is quite important to point out to users.
    • For the standard K8s resources we know what apiVersion should be used and can suggest it. For other kinds (custom) probably not.
    • Correct apiVersion values can be obtained from https://kubernetes.io/docs/reference/kubernetes-api/ (back to v1.22) manually (and maybe from other places too, faster than going through docs).
    • Could be K8S004: apiversion-required, default level - warning.
  4. Resource has kind and "invalid" apiVersion.
    • "Invalid" would mean apiVersion has value but not from the allowed list. For example, typos or using apiVersion from different resource kind that should be used. So it would mostly covers so incidental misconfigurations.
    • Could use the same list as from previous point to validated what apiVersion is valid for a given kind.
    • Could be the same rule as previous point or a separate one 🤔
  5. Resources with no kind and no apiVersion - 🤷. Probably doesn't make sense, maybe "strict mode" (see point 3) can also detect it.

I think 1 and 2 would satisfy initial intentions of this task, but 3 and 4 will also bring additional value. We can cover all at once or in 2 separate PRs if we want to have something quicker.

Also 4th overlaps to some extend with 2nd (because removed apiVersions are not allowed to).

@WitoDelnat let's discuss how we want to move forward.