[bitnami/metallb] Adding the MetalLB Helm Chart.

[Cellebyte]

Description of the change
This adds the metallb Chart to the Bitnami Repository.

Benefits
Kubernetes Native LoadBalancer.

Applicable issues

• partially fixes {feature request} Onboard [bitnami/metallb] and [bitnami/contour] helm charts. #2030

Additional information

Checklist

• Chart version bumped in Chart.yaml according to semver.
• Variables are documented in the README.md
• Title of the PR starts with chart name (e.g. [bitnami/chart])
• If the chart contains a values-production.yaml apart from values.yaml, ensure that you implement the changes in both files

⚠️ Keep in mind that if you want to make changes to the kubeapps chart, please implement them in the kubeapps repository. This is only a synchronized mirror.

[carrodher]

Thanks! Great work, we are going to review it and continue working on the images, I just added some comments

[carrodher]

Pinging other colleagues that would be happy to take a look @juan131 @javsalgar @miguelaeh

[Cellebyte]

@carrodher added you changes to the PR.

[miguelaeh]

Thank you very much for this PR!!!
We really appreciate this kind of contribution.
I left some comments, please take a lok when you have the chance.

[Cellebyte]

@miguelaeh I added some stuff regarding your comments

[Cellebyte]

@miguelaeh I tested the deployment of the edited templates and it seems to be working fine.
For that runAsUser: 0 I had an Issue open on the container repository itself. It was working as expected after I granted the container the root user. Before that it was not working as expected. Because spoofing ARP is not allowed by a normal user on the system.

[miguelaeh]

Thank you very much for the changes @Cellebyte ,
I saw you were testing it using this:

        securityContext:
          allowPrivilegeEscalation: false
          readOnlyRootFilesystem: true
          capabilities:
            drop:
            - ALL
            add:
            - NET_ADMIN
            - NET_RAW
            - SYS_ADMIN

Cheking the docs for allowPrivilegeEscalation the say:

This flag will prevent setuid binaries from changing the effective user ID, and prevent files from enabling extra capabilities (e.g. it will prevent the use of the ping tool). This behavior is required to effectively enforce MustRunAsNonRoot.

So I understand that should be set to true in order to enable the capabilities, so I think in that configuration there is a contradiction, because we are setting capabilities but setting allowPrivilegeEscalation to false, that will block the use of capabilities.
I am not totally sure about this, could you test it running as non-root and setting allowPrivilegeEscalation to true?

I just want to be sure before left the containers running as root.

[Cellebyte]

I just want to be sure before left the containers running as root.

Will test that.

with privilegeEscalation: true and runAsUser: 1001 the following appears in the log.

{"caller":"announcer.go:99","error":"creating ARP responder for \"vethcda9a590\": operation not permitted","interface":"vethcda9a590","msg":"failed to create ARP responder","op":"createARPResponder","ts":"2020-04-22T10:00:49.34006298Z"}
{"caller":"announcer.go:99","error":"creating ARP responder for \"vethcda9a590\": operation not permitted","interface":"vethcda9a590","msg":"failed to create ARP responder","op":"createARPResponder","ts":"2020-04-22T10:00:59.341347513Z"}
{"caller":"announcer.go:99","error":"creating ARP responder for \"vethcda9a590\": operation not permitted","interface":"vethcda9a590","msg":"failed to create ARP responder","op":"createARPResponder","ts":"2020-04-22T10:01:09.342642912Z"}
{"caller":"announcer.go:99","error":"creating ARP responder for \"vethcda9a590\": operation not permitted","interface":"vethcda9a590","msg":"failed to create ARP responder","op":"createARPResponder","ts":"2020-04-22T10:01:19.3438333Z"}
{"caller":"announcer.go:99","error":"creating ARP responder for \"vethcda9a590\": operation not permitted","interface":"vethcda9a590","msg":"failed to create ARP responder","op":"createARPResponder","ts":"2020-04-22T10:01:29.344959529Z"}
{"caller":"announcer.go:99","error":"creating ARP responder for \"vethcda9a590\": operation not permitted","interface":"vethcda9a590","msg":"failed to create ARP responder","op":"createARPResponder","ts":"2020-04-22T10:01:39.346006552Z"}

with privilegeEscalation: false and runAsUser: 0 it shows:

{"caller":"announcer.go:103","event":"createARPResponder","interface":"vethcda9a590","msg":"created ARP responder for interface","ts":"2020-04-22T10:02:40.114569248Z"}
{"caller":"announcer.go:103","event":"createARPResponder","interface":"veth675a8f9c","msg":"created ARP responder for interface","ts":"2020-04-22T10:02:40.114945704Z"}
{"caller":"announcer.go:103","event":"createARPResponder","interface":"veth5211c8f0","msg":"created ARP responder for interface","ts":"2020-04-22T10:02:40.115213938Z"}
{"caller":"announcer.go:103","event":"createARPResponder","interface":"veth0a65f3d0","msg":"created ARP responder for interface","ts":"2020-04-22T10:02:40.115443937Z"}
{"caller":"announcer.go:103","event":"createARPResponder","interface":"eth0","msg":"created ARP responder for interface","ts":"2020-04-22T10:02:40.115643897Z"}

nope @miguelaeh it is not working with privilegeEscalation: true and runAsUser: 1001

[Cellebyte]

@miguelaeh MetalLB is an infrastructure Helm Chart so it will not be deployed by any developer.
It should also work on OpenShift if you add the right exception there.

[miguelaeh]

Thank you very much @Cellebyte for testing that, I have been testing the chart and it is working fine.
As a suggestion I would add the annotation to track changes in the configmap, it is done adding something like the following in the deployment and in the daemonset:

checksum/configmap: {{ include (print $.Template.BasePath "/configmap.yaml") . | sha256sum }}

[miguelaeh]

Apart from that suggestion,
LGTM!!!
Thank you very much for addressing all the changes

[Cellebyte]

Thank you very much @Cellebyte for testing that, I have been testing the chart and it is working fine.
As a suggestion I would add the annotation to track changes in the configmap, it is done adding something like the following in the deployment and in the daemonset:

We should not implement that.
Because the configmap is reloaded by MetalLB itself.
The only think to keep in mind, that after a configuration change you should look into log if the new configuration got applied or you messed something up.

metallb/metallb#348

I think it will be better handled when they introduce a CRD for handling config.

[miguelaeh]

Thank you for the clarification @Cellebyte , we will take care of add this to our internal CI/CD before merge the Chart.

[miguelaeh]

Hi @Cellebyte ,
I was adding the chart to our CI/CD and faced an issue performing the test that I wpould like to check with you. Is intended to have the file system as read-only? Do you know if there is any use case where it is needed to create files indide the container?
I checked the values and controller.securityContext.readOnlyRootFilesystem and speaker.securityContext.readOnlyRootFilesystem are set to true by default

[Cellebyte]

Hi @miguelaeh,
I think it is there to read the available host nics from the host fs.
It should never be allowed to write something to the host fs.

https://github.com/metallb/metallb/blob/v0.9.3/manifests/metallb.yaml

Also the default deployment is mounting / as readOnlyRootFilesystem: true

[AntonioMeireles]

(hey all!) thanks you all your great work! one small question - when this will be actually usable ? as at sight, afaict, it's not yet consumable from https://charts.bitnami.com/bitnami ...

[miguelaeh]

Hi @AntonioMeireles ,
Thanks for your interest, we are making the release in our internal CI/CD, it should be available in the index soon, probably today.

[miguelaeh]

Hi @AntonioMeireles ,
The chart has just been published to the Helm index, to get it execute:

helm repo update