Arnavion / k8s-openapi

Rust definitions of the resource types in the Kubernetes client API
Apache License 2.0
373 stars 42 forks source link

Secrets can contain null as the value for a data field #114

Closed nicklan closed 1 year ago

nicklan commented 2 years ago

Here is an example secret that can be returned:

{
  "metadata": {
    "name": "a-cert",
    "namespace": "ns",
    "selfLink": "/api/v1/namespaces/ns/secrets/a-cert",
    "uid": "c18dd343-b8ba-11e9-94ec-83289476e83dc0",
    "resourceVersion": "1082189042",
    "creationTimestamp": "2023-08-07T02:26:26Z"
  },
  "data": {
    "tls.crt": null,
    "tls.key": "[snip-base64-string]"
  },
  "type": "kubernetes.io/tls"
}

This will fail to deserialize with: invalid type: null, expected a base64-encoded string at line X column Y

I think ByteString should probably support deserializing from null and just return an empty byte string. This at least seems to be how kubectl does it:

kubectl -n ns describe secret a-cert
Name:         a-cert
Namespace:    ns
Labels:       <none>
Type:  kubernetes.io/tls

Data
====
tls.crt:  0 bytes
tls.key:  1704 bytes
Arnavion commented 2 years ago

Does the server allow creating a Secret with a null value? If yes, then rather than changing ByteString::deserialize to tolerate null the fix would be to change Secret::data's type to BTreeMap<String, Option<ByteString>>. ConfigMap::binaryData also has the same JSON schema { "additionalProperties": { "type": "string", "format": "byte" } }, so it needs to be tested too.

nicklan commented 2 years ago

Well, it certainly allowed it at some point since this secret exists in one of our clusters :). The best I could find for a schema quickly was https://github.com/instrumenta/kubernetes-json-schema/blob/master/v1.12.5/secret.json, which seems to indicate that it can indeed be null, but I imagine there's a more authoritative document somewhere.

I agree that making the ByteString an Option<ByteString> would work, but it's a more breaking change, so I thought perhaps just having it deserialize to empty would be less disruptive.

Arnavion commented 2 years ago

The schema is https://github.com/Arnavion/k8s-openapi/blob/3024c3f25bc79825effab9e01c4a740793315928/k8s-openapi-codegen/src/supported_version.rs#L53-L60 , but my point about testing was not to look at the schema (which I already did, hence my note about ConfigMap::binaryData being the same) but to test against a cluster.

nicklan commented 2 years ago

Got it. I'll test if I can create secrets and configmaps with null and report back

nicklan commented 2 years ago

Creating a secret with null.

$ cat /tmp/null_secret.json
{
    "apiVersion": "v1",
    "data": {
        "nullkey": null,
        "key": "dXNlcg=="
    },
    "kind": "Secret",
    "metadata": {
        "name": "test-null",
        "namespace": "default"
    },
    "type": "Opaque"
}

$ kubectl create -f /tmp/null_secret.json
error: error validating "/tmp/null_secret.json": error validating data: unknown object type "nil" in Secret.data.nullkey; if you choose to ignore these errors, turn validation off with --validate=false
$ kubectl --validate=false create -f /tmp/null_secret.json
secret/test-null created
$ kubectl -n default get secret test-null -o yaml
apiVersion: v1
data:
  key: dXNlcg==
  nullkey: null
kind: Secret
metadata:
  creationTimestamp: "2022-04-19T20:07:04Z"
  name: test-null
  namespace: default
  resourceVersion: "3349160838"
  selfLink: /api/v1/namespaces/default/secrets/test-null
  uid: 56a98079-2375-4ea4-aefe-6ddfc7877a8b
type: Opaque

And for a configmap:

$ cat /tmp/null_configmap.json
{
    "apiVersion": "v1",
    "kind": "ConfigMap",
    "metadata": {
        "name": "nullconfigmap",
        "namespace": "default"
    },
    "binaryData": {
        "nulldata": null
    }
}
$ kubectl create -f /tmp/null_configmap.json
error: error validating "/tmp/null_configmap.json": error validating data: unknown object type "nil" in ConfigMap.binaryData.nulldata; if you choose to ignore these errors, turn validation off with --validate=false
$ kubectl --validate=false create -f /tmp/null_configmap.json
configmap/nullconfigmap created
$ kubectl -n default get configmap nullconfigmap -o yaml
apiVersion: v1
binaryData:
  nulldata: null
kind: ConfigMap
metadata:
  creationTimestamp: "2022-04-19T20:13:00Z"
  name: nullconfigmap
  namespace: default
  resourceVersion: "3349184254"
  selfLink: /api/v1/namespaces/default/configmaps/nullconfigmap
  uid: de3e06d9-c4a7-4aec-85ce-d3b410230fa5

So you have to not validate, but it will create both kinds. Both cause this issue.

Arnavion commented 2 years ago

Thanks. As one last test, can you check if ConfigMap::data has the same behavior? ie does it need to be fixed only for { "additionalProperties": { "type": "string", "format": "byte" } } fields or also for { "additionalProperties": { "type": "string" } } fields (of which there are a lot more).

nicklan commented 2 years ago

Looks like it'll create it, but actually turn that into the empty string:

$cat /tmp/null_configmap.json
{
    "apiVersion": "v1",
    "kind": "ConfigMap",
    "metadata": {
        "name": "nullconfigmap",
        "namespace": "default"
    },
    "data": {
        "nulldata": null
    },
    "binaryData": {
        "nullbindata": null
    }
}
$ kubectl create -f /tmp/null_configmap.json
error: error validating "/tmp/null_configmap.json": error validating data: [unknown object type "nil" in ConfigMap.binaryData.nullbindata, unknown object type "nil" in ConfigMap.data.nulldata]; if you choose to ignore these errors, turn validation off with --validate=false
$ kubectl --validate=false create -f /tmp/null_configmap.json
configmap/nullconfigmap created
$ kubectl -n default get configmap nullconfigmap -o yaml
apiVersion: v1
binaryData:
  nullbindata: null
data:
  nulldata: ""
kind: ConfigMap
metadata:
  creationTimestamp: "2022-04-19T20:35:29Z"
  name: nullconfigmap
  namespace: default
  resourceVersion: "3349269858"
  selfLink: /api/v1/namespaces/default/configmaps/nullconfigmap
  uid: 256caab4-8923-4050-a2c6-30cf62a0436c