crdpuller: missing defaults in OpenAPI spec for map-keys break compatibility

[sttts]

Against Kube kubectl create deployment --image=gcr.io/kuar-demo/kuard-amd64:blue --port=8080 kuard works, against kcp it does not (server-side validation error):

$ kubectl create deployment --image=gcr.io/kuar-demo/kuard-amd64:blue --port=8080 kuard

error: failed to create deployment: Deployment.apps “kuard” is invalid: spec.template.spec.containers.ports.protocol: Required value
$ kubectl create deployment --image=gcr.io/kuar-demo/kuard-amd64:blue --port=8080 kuard --dry-run=client -o yaml
apiVersion: apps/v1
kind: Deployment
metadata:
  creationTimestamp: null
  labels:
    app: kuard
  name: kuard
spec:
  replicas: 1
  selector:
    matchLabels:
      app: kuard
  strategy: {}
  template:
    metadata:
      creationTimestamp: null
      labels:
        app: kuard
    spec:
      containers:
      - image: gcr.io/kuar-demo/kuard-amd64:blue
        name: kuard-amd64
        ports:
        - containerPort: 8080 # protocol missing
        resources: {}
status: {}
$ kubectl create -f kuard.yaml
<same error but from client-side validation>

The root cause of this is that kube's ContainerPort has an optional field protocol (ref) with

1. a default value ("TCP") that is not published via OpenAPI
2. protocol is a map-key.

Map-keys by kube API definition must be either required or they need a default (ref).

In the crdpuller we add map-keys as required fields (ref).

We can question this behaviour if there is no default provided in OpenAPI because it leads to incompatible APIs, being stricter than the original one from the pcluster.

On the other hand, without a known default value, we can also not make the field non-required because that breaks CRD creation of the imported resources on the kcp side because "required or defaulted" is validated by apiextensions-apiserver.

The only way out it to get default values into the kcp CRDs, whereever from:

1. from OpenAPI from the pcluster. OpenAPI v3 will add defaults to be exported (still under an alpha feature gate) in 1.23, but likely beta in 1.24
2. from some OpenAPI schema patch for know cases in kcp. I.e. for a apps/v1 Deployment imported from a pcluster we would know to fix-up the spec.

TODOs:

1. [ ] Check all k8s.io/api types with x-kubernetes-map-keys for being optional. They must have a +default marker attached. If not, we should open a PR.
2. [ ] Add OpenAPI v3 support to the crdpuller.

[ncdc]

Would it also be possible to create a CRD where we can provide kcp with hints for these types of issues? e.g. a "FixDeploymentSchema" CR that says "default spec.template.spec.containers[*].ports.protocol to TCP"?

[varshaprasad96]

@sttts could you please elaborate on what is to be done to add OpenAPI v3 support to crdpuller. I can work on it. Thanks!

[ncdc]

Clearing milestone to re-triage

[lionelvillard]

I'd like to work on this.

/assign

[kcp-ci-bot]

Issues go stale after 90d of inactivity. After a furter 30 days, they will turn rotten. Mark the issue as fresh with /remove-lifecycle stale.

If this issue is safe to close now please do so with /close.

/lifecycle stale

[kcp-ci-bot]

Stale issues rot after 30d of inactivity. Mark the issue as fresh with /remove-lifecycle rotten. Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

/lifecycle rotten

[kcp-ci-bot]

Rotten issues close after 30d of inactivity. Reopen the issue with /reopen. Mark the issue as fresh with /remove-lifecycle rotten.

/close

[kcp-ci-bot]

@kcp-ci-bot: Closing this issue.

In response to this:

Rotten issues close after 30d of inactivity. Reopen the issue with /reopen. Mark the issue as fresh with /remove-lifecycle rotten.

/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.