search

hall / kubenix

Kubernetes management with Nix
https://kubenix.org/
MIT License
279 stars 27 forks source link

Generated kubernetes module requires `protocol` in `Container.ports` which is not required by spec #52

Open xhalo32 opened 8 months ago

xhalo32 commented 8 months ago

Line https://github.com/hall/kubenix/blob/e4d036576436b9983216584a89388af3da995043/modules/generated/v1.27.nix#L5190 requires that protocol key be present in all helm generated Pod specifications. For example the following kubenix resource definition

resources.pods.hello.spec = {
  containers = [
    {
      name = "hello";
      image = "hello";
      ports = [
        {containerPort = 1234;}
      ];
    }
  ];
};

fails with the following error

       error: attribute 'protocol' missing

       at /nix/store/jznzhivlpsahv83fsvkp866bda50nd23-source/modules/generated/v1.27.nix:82:28:

           81|               (key:
           82|                 if isAttrs value.${key}
             |                            ^
           83|                 then toString value.${key}.content

It should not fail when protocol is missing from the ports.

There are certainly other similar issues with e.g. EphemeralContainer which need to be addressed as well.

Workaround

Currently I have just deleted the "protocol" from https://github.com/hall/kubenix/blob/e4d036576436b9983216584a89388af3da995043/modules/generated/v1.27.nix#L5190 and everything is working smoothly.

Backstory

I ran into this issue while trying to deploy prometheus as a Helm chart. After a bit of debugging I noticed that the prometheus chart doesn't specify the port protocol: https://github.com/prometheus-community/helm-charts/blob/d628ebad62f119ef2985319a5f7a1dd5bee1863b/charts/prometheus/templates/deploy.yaml#L157

xhalo32 commented 8 months ago

I have made hacky preliminary changes to the k8s generator in my fork (which I guarantee are incorrect in some way) which has fixed my issue.

My change does not reintroduce https://github.com/hall/kubenix/issues/13

pizzapim commented 5 months ago

To me this looks like an upstream Kubernetes bug. This conversation mentions some current problems with merge strategies with respect to some fields including ports: https://github.com/kubernetes-sigs/kustomize/issues/3111

the current consensus is that merging should look for a x-kubernetes-list-map-keys field and use the list to merge if the list exists, then fall back to the singular field x-kubernetes-patch-merge-key and use that. The libraries must respect the intent of the schema.

I think their solution still does not account for the issue at hand though. So in my opinion a hacky way to fix it is desirable.

Here is a broader conversation about port related issues: https://github.com/kubernetes/kubernetes/issues/105610