Config Structural Validation Schema Part 1: Commons, HNC, CAPI, Harbor, Storage

[Zash]

Warning

This is a public repository, ensure not to disclose:

• personal data beyond what is necessary for interacting with this pull request, nor
• business confidential information, such as customer names.

What kind of PR is this?

Required: Mark one of the following that is applicable:

• kind/feature
• kind/improvement
• kind/deprecation
• kind/documentation
• kind/clean-up
• kind/bug
• kind/other

Optional: Mark one or more of the following that are applicable:

Important

Breaking changes should be marked kind/admin-change or kind/dev-change depending on type
Critical security fixes should be marked with kind/security

• kind/admin-change
• kind/dev-change
• kind/security
• kind/adr

What does this PR do / why do we need this PR?

This adds structural validation for the following top-level config sections:

• user
• hnc
• clusterApi
• harbor
• clusterApi
• storageClasses
• objectStorage
• rookCeph

For more context including the required scripts and see #1862
Part of issue #1427

Information to reviewers

ck8s validate should now perform validation of the above sections.

While the CI is failing:
Please help determine whether it is the schema that is lacking or whether it detected a config mistake.

Suggestions and improvements very welcome!

Also see the README for brief information about the schema format

Checklist

• Proper commit message prefix on all commits
• Change checks:
  • The change is transparent
  • The change is disruptive
  • The change requires no migration steps
  • The change requires migration steps
  • The change upgrades CRDs
• Metrics checks:
  • The metrics are still exposed and present in Grafana after the change
  • The metrics names didn't change (Grafana dashboards and Prometheus alerts are not affected)
  • The metrics names did change (Grafana dashboards and Prometheus alerts were fixed)
• Logs checks:
  • The logs do not show any errors after the change
• Pod Security Policy checks:
  • Any changed pod is covered by Pod Security Admission
  • Any changed pod is covered by Gatekeeper Pod Security Policies
  • The change does not cause any pods to be blocked by Pod Security Admission or Policies
• Network Policy checks:
  • Any changed pod is covered by Network Policies
  • The change does not cause any dropped packets in the NetworkPolicy Dashboard
• Audit checks:
  • The change does not cause any unnecessary Kubernetes audit events
  • The change requires changes to Kubernetes audit policy
• Falco checks:
  • The change does not cause any alerts to be generated by Falco
• Bug checks:
  • The bug fix is covered by regression tests
• Config checks:
  • The schema was updated

[Xartos]

I'm not familiar with this schema format. Do you have any good link to where I can read about it? Or maybe just the name of it so I can search for it

[Zash]

See https://github.com/elastisys/compliantkubernetes-apps/blob/main/config/schemas/README.md and remind me tomorrow to add a link to that in the reviewer info.

It's also closely related to the OpenAPI format used in CRDs.

[Xartos]

Some minor things but overall it looks good 👍

[aarnq]

You should be able to source from the upstream OpenAPI spec, though it don't know how well it matches.
The bulk of common definitions are in api_openapi.json and basically everything are refs.

[Zash]

diff --git a/config/schemas/config.yaml b/config/schemas/config.yaml
index b10d2a9f..9c3fe3f9 100644
--- a/config/schemas/config.yaml
+++ b/config/schemas/config.yaml
@@ -8,7 +8,6 @@ description: |
   file will contain different settings.
 $defs:
   kubernetesAffinity:
-    $comment: Real k8s resources might have existing schemas that could be reused as-is
     properties:
       nodeAffinity:
         type: object
@@ -19,11 +18,7 @@ $defs:
     additionalProperties: false
     examples:
       - nodeAffinity:
-          requiredDuringSchedulingIgnoredDuringExecution:
-            nodeSelectorTerms:
-              - matchExpressions:
-                  - key: node-role.kubernetes.io/control-plane
-                    operator: Exists
+          $ref: https://github.com/kubernetes/kubernetes/raw/master/api/openapi-spec/v3/api__v1_openapi.json#/components/schemas/io.k8s.api.core.v1.NodeAffinity
     title: Kubernetes Affinity
     type: object
   component:

🤔

[Zash]

Heh, accidentally put the $ref under examples. Now fixed.

I'm a bit uneasy about referencing remote files tho, and not sure if licensing allows copying subsets.

[aarnq]

Should be fine to copy the subset with proper attribution given that the upstream spec is Apache-2.0, so if you add a comment in the spec where you include it and a notice in the readme then it should be OK.

[Zash]

Those affinity settings contain further $references which point to things with even more references, turning this into a recursive chase that may need further though or some scripting.

[Zash]

Hooray for loops!

     [ck8s] Failed schema validation:
     wc-config.yaml: hnc.manager.nodeSelector: nodeSelectorTerms is required
     wc-config.yaml: velero.nodeSelector: nodeSelectorTerms is required

What to do with these? This could be due to importing definitions from latest k8s git. I haven't investigated yet.

[Zash]

Oh no, NodeSelector and nodeSelector are different things.

[Zash]

I've made it so that .$defs.kubernetesThing are kubernetes properties where we wrote the schema, while those imported from the kubernetes API spec files have names like .$defs["io.k8s.api.core.v1.Affinity"].

[robinAwallace]

I think it LGTM 🙂 Found one question mark.

[Zash]

Incoming rebase onto latest main...

[Zash]

I've prepared a squashed commit, aiming to push and merge on Monday.