Handle empty array for shadowsocks.dnsServers

[alexandre-abrioux]

This PR makes sure that an empty array [] for the value of shadowsocks.dnsServers will create an empty DNS_ADDRS environment variable.

[predatorray]

You forgot the parentheses here. This will cause the following exception:

Error: template: shadowsocks/templates/deployment.yaml:58:30: executing "shadowsocks/templates/deployment.yaml" at <join>: wrong number of args for join: want 2 got 0

[alexandre-abrioux]

Hi! Thanks for the feedback, I got confused in the tests, my change wasn't even working.

I force-pushed a fix. To test it please create a value-test.yaml with the following content:

shadowsocks:
  dnsServers: []

; and then run helm install --dry-run --values values-test.yaml --debug test .
You should get the following:

env:
  - ...
  - name: DNS_ADDRS
    value: ""

[predatorray]

I think we should probably surround this part with {{- if .Values.shadowsocks.dnsServers }} ... {{- end }}. Passing an empty DNS_ADDRS value to the shadowsocks/shadowsocks-libev image would get the following error:

2021-12-16 14:08:40 ERROR: failed to set nameservers

[alexandre-abrioux]

@predatorray You are right, there is a bug in the shadowsocks/shadowsocks-libev docker image. For the moment you are not able to use an empty DNS_ADDRS value, which means you are not able to use Kubernetes's internal DNS server (the one that is already setup in /etc/resolv.conf). I've submitted a fix on the same day that I've opened this PR, but it hasn't been merged yet: shadowsocks/shadowsocks-libev#2866. I will keep you posted once it's released!

[predatorray]

I don't quite understand the bug you mentioned in that issue. As I tested on my laptop, the process can be started if we either provide a non-empty DNS_ADDRS or just leave it unset. The problem only occurs if we provide an empty string. Maybe the simplest way is the solution I suggested in the previous comment.

[alexandre-abrioux]

The solution you suggested won't work. Let me rephrase: the goal of this PR is to be able to set an empty string for DNS_ADDRS in order to use Kubernetes' internal DNS server, instead of the default 8.8.8.8. Doing so will allow shadowsocks proxy users to access any of the cluster's services, like a VPN hosted on the cluster for instance.

To use Kubernetes' internal DNS server, I need to set the DNS_ADDRS env var as an empty string. That way, the -d option of ss-server won't be used, and the DNS server IP address will be fetched from /etc/resolv.conf.

See the documentation of ss-server:

-d <addr>
Setup name servers for internal DNS resolver (libc-ares). The default server is fetched from /etc/resolv.conf.

The bug is that, as of today, even if you pass an empty string for DNS_ADDRS in the docker image, the -d option is still used, so you get

ERROR: failed to set nameservers.

This will be fixed in shadowsocks/shadowsocks-libev#2866

[predatorray]

Thanks for your explanation! Yes, this is indeed a problem. The image does not provide any mechanism to reset the -d option to its default behavior but instead using 8.8.8.8,8.8.4.4 as its default value. Let's see what is the suggestion from the maintainer.

[alexandre-abrioux]

Hi @predatorray! The fix for the Docker image was merged in shadowsocks/shadowsocks-libev#2866. The proposed change for the Helm Chart is now relevant 🙂 I am using it in production, and it works like a charm!

[predatorray]

Thanks @alexandre-abrioux for your efforts to push this feature forward. Considering it as an incompatible change, since setting DNS_ADDRS to be an empty will fail using the latest stable image, let's wait for the release of a new stable image for this and change the appVersion in the Chart.yaml accordingly.