Better kubernetes schema

.changeset/friendly-papayas-lie.md

@@ -0,0 +1,5 @@
+---
+"@monokle/validation": patch
+---
+
+Improve Kubernetes Schema rules

packages/validation/src/__tests__/MonokleValidator.kubernetes-schema.test.ts

@@ -15,21 +15,19 @@ it('should detect deprecation error - single resource, removal', async () => {
 
   const hasErrors = response.runs.reduce((sum, r) => sum + r.results.length, 0);
 
-  expect(hasErrors).toBe(2);
+  expect(hasErrors).toBe(1);
 
   const result = response.runs[0].results[0];
   expectResult(result, 'K8S003', 'error', 'ReplicaSet');
   expect(result.message.text).toContain('uses removed');
-
-  expectResult(response.runs[0].results[1], 'K8S004', 'warning', 'ReplicaSet');
 });
 
 it('should detect deprecation error - multiple resources, removal', async () => {
   const {response} = await processResourcesInFolder('src/__tests__/resources/deprecations-2', 'v1.29');
 
   const hasErrors = response.runs.reduce((sum, r) => sum + r.results.length, 0);
 
-  expect(hasErrors).toBe(5);
+  expect(hasErrors).toBe(3);
 
   const result0 = response.runs[0].results[0];
   expectResult(result0, 'K8S003', 'error', 'FlowSchema');
@@ -39,31 +37,27 @@ it('should detect deprecation error - multiple resources, removal', async () =>
   expectResult(result1, 'K8S003', 'error', 'ValidatingWebhookConfiguration');
   expect(result1.message.text).toContain('uses removed');
 
-  expectResult(response.runs[0].results[2], 'K8S004', 'warning', 'Pod');
-  expectResult(response.runs[0].results[3], 'K8S004', 'warning', 'FlowSchema');
-  expectResult(response.runs[0].results[4], 'K8S004', 'warning', 'ValidatingWebhookConfiguration');
+  expectResult(response.runs[0].results[2], 'K8S004', 'warning', 'SomeCustomResource');
 });
 
 it('should detect deprecation error - single resource, deprecation', async () => {
   const {response} = await processResourcesInFolder('src/__tests__/resources/deprecations-3', 'v1.15');
 
   const hasErrors = response.runs.reduce((sum, r) => sum + r.results.length, 0);
 
-  expect(hasErrors).toBe(2);
+  expect(hasErrors).toBe(1);
 
   const result = response.runs[0].results[0];
   expectResult(result, 'K8S002', 'warning', 'ReplicaSet');
   expect(result.message.text).toContain('uses deprecated');
-
-  expectResult(response.runs[0].results[1], 'K8S004', 'warning', 'ReplicaSet');
 });
 
 it('should detect deprecation error - multiple resources, removal + deprecation', async () => {
   const {response} = await processResourcesInFolder('src/__tests__/resources/deprecations-4', 'v1.28');
 
   const hasErrors = response.runs.reduce((sum, r) => sum + r.results.length, 0);
 
-  expect(hasErrors).toBe(4);
+  expect(hasErrors).toBe(2);
 
   const result0 = response.runs[0].results[0];
   expectResult(result0, 'K8S002', 'warning', 'KubeSchedulerConfiguration');
@@ -72,24 +66,16 @@ it('should detect deprecation error - multiple resources, removal + deprecation'
   const result1 = response.runs[0].results[1];
   expectResult(result1, 'K8S003', 'error', 'RuntimeClass');
   expect(result1.message.text).toContain('uses removed');
-
-  expectResult(response.runs[0].results[2], 'K8S004', 'warning', 'KubeSchedulerConfiguration');
-  expectResult(response.runs[0].results[3], 'K8S004', 'warning', 'RuntimeClass');
 });
 
 it('should rise warning when no apiVersion present (K8S004)', async () => {
   const {response} = await processResourcesInFolder('src/__tests__/resources/no-apiversion');
 
   const hasErrors = response.runs.reduce((sum, r) => sum + r.results.length, 0);
-  expect(hasErrors).toBe(2);
-
-  const error1 = response.runs[0].results[1];
-  expectResult(error1, 'K8S004', 'warning', 'FlowSchema');
-  expect(error1.message.text).toContain('Missing "apiVersion"');
+  expect(hasErrors).toBe(1);
 
-  const error2 = response.runs[0].results[0];
-  expectResult(error2, 'K8S004', 'warning', 'Pod');
-  expect(error2.message.text).toContain('Missing "apiVersion"');
+  const error1 = response.runs[0].results[0];
+  expectResult(error1, 'K8S004', 'warning', 'SomeCustomResource');
 });
 
 async function processResourcesInFolder(path: string, schemaVersion?: string) {

packages/validation/src/__tests__/resources/deprecations-2/resources.yaml

@@ -5,18 +5,18 @@ metadata:
 webhooks:
   - name: webhook-server.webhook-demo.svc
     sideEffects: None
-    admissionReviewVersions: ["v1", "v1beta1"]
+    admissionReviewVersions: ['v1', 'v1beta1']
     clientConfig:
       service:
         name: webhook-server
         namespace: webhook-demo
-        path: "/validate"
-      caBundle: ""
+        path: '/validate'
+      caBundle: ''
     rules:
-      - operations: [ "CREATE" ]
-        apiGroups: [""]
-        apiVersions: ["v1"]
-        resources: ["pods"]
+      - operations: ['CREATE']
+        apiGroups: ['']
+        apiVersions: ['v1']
+        resources: ['pods']
 ---
 apiVersion: flowcontrol.apiserver.k8s.io/v1beta2
 kind: FlowSchema
@@ -28,28 +28,20 @@ spec:
     name: exempt
   rules:
     - nonResourceRules:
-      - nonResourceURLs:
-          - "/healthz"
-          - "/livez"
-          - "/readyz"
-        verbs:
-          - "*"
+        - nonResourceURLs:
+            - '/healthz'
+            - '/livez'
+            - '/readyz'
+          verbs:
+            - '*'
       subjects:
         - kind: Group
           group:
-            name: "system:unauthenticated"
+            name: 'system:unauthenticated'
 ---
-apiVersion: v1
-kind: Pod
+apiVersion: example.io/v1
+kind: SomeCustomResource
 metadata:
-  name: pod-warning
-  labels:
-    app: pod-warning
+  name: crd-example
 spec:
-  restartPolicy: OnFailure
-  securityContext:
-    runAsNonRoot: false
-  containers:
-    - name: busybox
-      image: busybox
-      command: ["sh", "-c", "echo I am running as user $(id -u)"]
+  hello: world

packages/validation/src/__tests__/resources/no-apiversion/resources.yaml

@@ -5,39 +5,25 @@ metadata:
 webhooks:
   - name: webhook-server.webhook-demo.svc
     sideEffects: None
-    admissionReviewVersions: ["v1", "v1beta1"]
+    admissionReviewVersions: ['v1', 'v1beta1']
     clientConfig:
       service:
         name: webhook-server
         namespace: webhook-demo
-        path: "/validate"
-      caBundle: ""
+        path: '/validate'
+      caBundle: ''
     rules:
-      - operations: [ "CREATE" ]
-        apiGroups: [""]
-        apiVersions: ["v1"]
-        resources: ["pods"]
+      - operations: ['CREATE']
+        apiGroups: ['']
+        apiVersions: ['v1']
+        resources: ['pods']
 ---
-apiVersion: ''
-kind: FlowSchema
+apiVersion: example.io/v1
+kind: SomeCustomResource
 metadata:
   name: health-for-strangers
 spec:
-  matchingPrecedence: 1000
-  priorityLevelConfiguration:
-    name: exempt
-  rules:
-    - nonResourceRules:
-      - nonResourceURLs:
-          - "/healthz"
-          - "/livez"
-          - "/readyz"
-        verbs:
-          - "*"
-      subjects:
-        - kind: Group
-          group:
-            name: "system:unauthenticated"
+  hello: world
 ---
 apiVersion: 'test-remove'
 kind: Pod
@@ -52,4 +38,4 @@ spec:
   containers:
     - name: busybox
       image: busybox
-      command: ["sh", "-c", "echo I am running as user $(id -u)"]
+      command: ['sh', '-c', 'echo I am running as user $(id -u)']

packages/validation/src/validators/kubernetes-schema/rules.ts

@@ -47,13 +47,13 @@ export const KUBERNETES_SCHEMA_RULES: RuleMetadata[] = [
     id: 'K8S004',
     name: 'strict-mode-violated',
     shortDescription: {
-      text: 'The resource has unsupported or invalid "apiVersion" field value.',
+      text: 'Strict mode violated.',
     },
     fullDescription: {
-      text: 'The resource is violating the strict-mode violation. The resource will not be validated against Kubernetes schema.',
+      text: 'Strict mode violated. Cannot validate schema because its either missing or there is a typo in your apiVersion/kind.',
     },
     help: {
-      text: 'Not supported or invalid "apiVersion" value has been used. You can hover the key for documentation.',
+      text: 'Check if there is a typo in the apiVersion/kind or look into registering the schema of this CRD.',
     },
     defaultConfiguration: {
       enabled: false,

packages/validation/src/validators/kubernetes-schema/validator.ts

@@ -93,20 +93,38 @@ export class KubernetesSchemaValidator extends AbstractPlugin {
       }
 
       // K8S004
-      const hasApiVersion = resource.apiVersion && resource.apiVersion.length > 0;
-      if (resourceErrors === undefined || !hasApiVersion) {
-        const errorKey = resource.apiVersion !== undefined ? 'apiVersion' : 'kind';
-        const errorText = hasApiVersion
-          ? `Invalid or unsupported "apiVersion" value for "${resource.kind}".`
-          : `Missing "apiVersion" field for "${resource.kind}".`;
-        const validationResult = this.adaptToValidationResult(resource, [errorKey], 'K8S004', errorText);
-        isDefined(validationResult) && results.push(validationResult);
+      const strictError = await this.validateStrictMode(resource, Boolean(deprecationError));
+      if (strictError) {
+        results.push(strictError);
       }
     }
 
     return results;
   }
 
+  /**
+   * Strict mode ensures that the Kubernetes schema got validated.
+   * This prevents surprises when you apply it to the Kubernetes API server.
+   *
+   * Strict mode might get violated due to typo's in apiVersion/kind or missing schemas for CRDs.
+   * Deprecated schemas are taken into account, as those are not _missing_ but _removed_ schemas.
+   */
+  private async validateStrictMode(resource: Resource, deprecated: boolean): Promise<ValidationResult | undefined> {
+    const validate = await this.getResourceValidator(resource);
+    const hasSchema = Boolean(validate);
+
+    if (hasSchema || deprecated) {
+      return;
+    }
+
+    return this.adaptToValidationResult(
+      resource,
+      ['apiVersion'],
+      'K8S004',
+      `The schema for ${resource.apiVersion}/${resource.kind} is not found and cannot be checked.`
+    );
+  }
+
   override registerCustomSchema({kind, apiVersion, schema}: CustomSchema): void | Promise<void> {
     const key = `${apiVersion}-${kind}`;
 
@@ -131,6 +149,11 @@ export class KubernetesSchemaValidator extends AbstractPlugin {
       return undefined;
     }
 
+    if (!resource.content) {
+      // This happens when the YAML has invalid syntax and the parser fails.
+      return undefined;
+    }
+
     validate(resource.content);
 
     const errors = validate.errors ?? [];