Merge pull request #536 from kubeshop/razvantopliceanu/feat/improve-k…

…8s-validation-rules feat: handle messageExpression
kubeshop · Oct 3, 2023 · 16b7d70 · 16b7d70
2 parents 5d7d36b + 9f4e7f5
commit 16b7d70
Show file tree

Hide file tree

Showing 7 changed files with 118 additions and 11 deletions.
diff --git a/.changeset/slimy-steaks-walk.md b/.changeset/slimy-steaks-walk.md
@@ -0,0 +1,5 @@
+---
+"@monokle/validation": patch
+---
+
+Handle messageExpression
diff --git a/packages/validation/src/__tests__/MonokleValidator.vap.test.ts b/packages/validation/src/__tests__/MonokleValidator.vap.test.ts
@@ -18,7 +18,7 @@ import {
   PARAMS_VALIDATING_ADMISSION_POLICY,
   PARAMS_VALIDATING_ADMISSION_POLICY_BINDING,
 } from './resources/admissionPolicy/ParamsValidatorResources.js';
-import {CRD, CRD_2, CUSTOM_RESOURCE} from './resources/admissionPolicy/CRDValidatorResources.js';
+import {CRD, CRD_MESSAGE_EXPRESSION, CUSTOM_RESOURCE} from './resources/admissionPolicy/CRDValidatorResources.js';
 
 it('test basic admission policy', async () => {
   const parser = new ResourceParser();
@@ -73,6 +73,19 @@ it('test crd admission policy', async () => {
   expect(hasErrors).toBe(1);
 });
 
+it('test crd with message expression admission policy', async () => {
+  const parser = new ResourceParser();
+
+  const validator = createTestValidator(parser);
+
+  const response = await validator.validate({
+    resources: [CRD_MESSAGE_EXPRESSION, CUSTOM_RESOURCE],
+  });
+
+  const hasErrors = response.runs.reduce((sum, r) => sum + r.results.length, 0);
+  expect(hasErrors).toBe(1);
+});
+
 function createTestValidator(parser: ResourceParser, config?: ValidationConfig) {
   return new MonokleValidator(
     {

diff --git a/packages/validation/src/__tests__/resources/admissionPolicy/BasicValidatorResources.ts b/packages/validation/src/__tests__/resources/admissionPolicy/BasicValidatorResources.ts
@@ -28,6 +28,7 @@ export const VALIDATING_ADMISSION_POLICY: Resource = {
       validations: [
         {
           expression: 'object.spec.replicas > 3',
+          messageExpression: '"spec replicas should be greater than 3"',
         },
       ],
     },

diff --git a/packages/validation/src/__tests__/resources/admissionPolicy/CRDValidatorResources.ts b/packages/validation/src/__tests__/resources/admissionPolicy/CRDValidatorResources.ts
@@ -143,3 +143,62 @@ export const CUSTOM_RESOURCE: Resource = {
   id: '13cf92c8d3181f-0',
   name: 'my-new-cron-object',
 };
+
+export const CRD_MESSAGE_EXPRESSION: Resource = {
+  fileId: 'd4fc87d28c6bd',
+  filePath: 'crd.yaml',
+  fileOffset: 0,
+  text: 'apiVersion: apiextensions.k8s.io/v1\nkind: CustomResourceDefinition\nmetadata:\n  name: mycustomresources.example.com\nspec:\n  group: example.com\n  names:\n    kind: MyCustomResource\n    plural: mycustomresources\n    singular: mycustomresource\n    shortNames:\n      - mcr\n  scope: Namespaced\n  versions:\n    - name: v1\n      schema:\n        openAPIV3Schema:\n          type: object\n          properties:\n            spec:\n              type: object\n              x-kubernetes-validation:\n                - rule: "self.replicas <= self.maxReplicas"\n                  message: "replicas must be less than or equal to maxReplicas"\n              properties:\n                replicas:\n                  type: integer\n                maxReplicas:\n                  type: integer\n              required:\n                - replicas\n                - maxReplicas\n',
+  apiVersion: 'apiextensions.k8s.io/v1',
+  kind: 'CustomResourceDefinition',
+  content: {
+    apiVersion: 'apiextensions.k8s.io/v1',
+    kind: 'CustomResourceDefinition',
+    metadata: {
+      name: 'mycustomresources.example.com',
+    },
+    spec: {
+      group: 'example.com',
+      names: {
+        kind: 'MyCustomResource',
+        plural: 'mycustomresources',
+        singular: 'mycustomresource',
+        shortNames: ['mcr'],
+      },
+      scope: 'Namespaced',
+      versions: [
+        {
+          name: 'v1',
+          schema: {
+            openAPIV3Schema: {
+              type: 'object',
+              properties: {
+                spec: {
+                  type: 'object',
+                  'x-kubernetes-validation': [
+                    {
+                      rule: 'self.replicas <= self.maxReplicas',
+                      message: 'replicas must be less than or equal to maxReplicas',
+                      messageExpression: '"replicas exceeded max limit of " + string(self.maxReplicas)',
+                    },
+                  ],
+                  properties: {
+                    replicas: {
+                      type: 'integer',
+                    },
+                    maxReplicas: {
+                      type: 'integer',
+                    },
+                  },
+                  required: ['replicas', 'maxReplicas'],
+                },
+              },
+            },
+          },
+        },
+      ],
+    },
+  },
+  id: 'd4fc87d28c6bd-0',
+  name: 'mycustomresources.example.com',
+};
diff --git a/packages/validation/src/validators/admission-policy/types.ts b/packages/validation/src/validators/admission-policy/types.ts
@@ -3,7 +3,8 @@ import {RuleLevel} from '../../commonExports.js';
 
 export type Expression = {
   expression: string;
-  message: string;
+  message?: string;
+  messageExpression?: string;
 };
 
 export type PolicyExpressionsAndFilteredResources = Record<

diff --git a/packages/validation/src/validators/admission-policy/utils.ts b/packages/validation/src/validators/admission-policy/utils.ts
@@ -33,7 +33,11 @@ export function getCRDExpressions(schema: CustomSchema, crd: Resource): Record<s
       expressions[key] = [];
 
       for (const rule of values['x-kubernetes-validation']) {
-        expressions[key].push({expression: rule.rule, message: rule.message ?? ''});
+        expressions[key].push({
+          expression: rule.rule,
+          message: rule.message,
+          messageExpression: rule.messageExpression,
+        });
       }
     }
   }

diff --git a/packages/validation/src/validators/admission-policy/validator.ts b/packages/validation/src/validators/admission-policy/validator.ts
@@ -112,34 +112,53 @@ export class AdmissionPolicyValidator extends AbstractPlugin {
     const {
       type,
       resource,
-      expression: {message, expression},
+      expression: {message, messageExpression, expression},
       level,
       params,
       property,
     } = args;
 
     let output: any;
+    let ruleMessage: string | undefined = message;
+    let messageExpressionOutput: string | undefined;
 
     if (type === 'validating-admission-policy') {
-      output = (globalThis as any).eval(expression, YAML.stringify({object: resource.content, params})).output;
+      const expressionParams = YAML.stringify({object: resource.content, params});
+      output = this.evaluateCelExpression(expression, expressionParams);
+
+      if (messageExpression) {
+        messageExpressionOutput = this.evaluateCelExpression(messageExpression, expressionParams);
+      }
     } else if (type === 'crd' && property) {
-      output = (globalThis as any).eval(
-        expression,
-        YAML.stringify({self: property === '<root>' ? resource.content : resource.content[property]})
-      ).output;
+      const expressionParams = YAML.stringify({
+        self: property === '<root>' ? resource.content : resource.content[property],
+      });
+      output = this.evaluateCelExpression(expression, expressionParams);
+
+      if (messageExpression) {
+        messageExpressionOutput = this.evaluateCelExpression(messageExpression, expressionParams);
+      }
     }
 
     if (output === 'true' || output.includes('ERROR:') || !output) {
       return [];
     }
 
+    if (
+      messageExpressionOutput &&
+      !messageExpressionOutput.includes('failed to evaluate') &&
+      !messageExpressionOutput.includes('ERROR:')
+    ) {
+      ruleMessage = messageExpressionOutput;
+    }
+
     if (output.includes('failed to evaluate')) {
       return [
         this.adaptToValidationResult(
           resource,
           ['kind'],
           'VAP002',
-          message ?? 'Admission policy failed to evaluate expression',
+          ruleMessage ?? 'Admission policy failed to evaluate expression',
           level
         ),
       ].filter(isDefined);
@@ -150,7 +169,7 @@ export class AdmissionPolicyValidator extends AbstractPlugin {
         resource,
         ['kind'],
         'VAP001',
-        message ?? 'Admission policy conditions violated',
+        ruleMessage ?? 'Admission policy conditions violated',
         level
       ),
     ].filter(isDefined);
@@ -289,6 +308,7 @@ export class AdmissionPolicyValidator extends AbstractPlugin {
         expressions: validations.map((v: any) => ({
           expression: v.expression,
           message: v.message,
+          messageExpression: v.messageExpression,
         })),
         level,
         params,
@@ -321,4 +341,8 @@ export class AdmissionPolicyValidator extends AbstractPlugin {
       level,
     });
   }
+
+  private evaluateCelExpression(expression: string, params: string) {
+    return (globalThis as any).eval(expression, params).output;
+  }
 }