[milvus] 22 proxy tls support

[VincentDu2021]

What this PR does / why we need it:

Checklist

[Place an '[x]' (no spaces) in all applicable fields. Please remove unrelated fields.]

• Chart Version bumped
• Variables are documented in the README.md
• Title of the PR starts with chart name (e.g. [mychartname])
• PR only contains changes for one chart

[haorenfsa]

This feature is not directly tested in CI, adding a seperated values file in charts/milvus/ci would be better, save us time of manual testing to confirm the correctness.
What do you think @LoveEachDay ?

[haorenfsa]

@VincentDu2021 Hi, thank you for providing this patch. Sry for the delay of review for most of our reviewers are on vocation. We'll be testing it soon.
By the way, don't forget to sign off your commit, and better to squash all commits into 1. And plz rebase and update the version once again once we decided to merge it.

[VincentDu2021]

7b2b4e8

Thanks @haorenfsa , I just squashed commits with signoff, and forced push again.

[haorenfsa]

@VincentDu2021 the test still failed

[VincentDu2021]

@VincentDu2021 the test still failed

My appologies @haorenfsa. I did further testing

1. if I do not turn on proxy.tls and proxy.volumeMounts etc. in the values file. The new chart just deploy as normal and would work.
2. To fully test the tls, secret creation and mounting, I had to make changes to the values.yaml and did find some issue. I have corrected those here and my local deploy (into an EKS cluster) is working:

helm upgrade --install milvus milvus-4.1.4.tgz -n test-milvus -f milvus/values.yaml --create-namespace
Release "milvus" does not exist. Installing it now.
NAME: milvus
LAST DEPLOYED: Thu Oct  5 16:10:42 2023
NAMESPACE: test-milvus
STATUS: deployed
REVISION: 1
TEST SUITE: None
---
 k get po -n test-milvus
NAME                                 READY   STATUS      RESTARTS   AGE
milvus-datacoord-5bc469bd98-8c7fq    1/1     Running     0          23m
milvus-datanode-bd6fd755d-n89rw      1/1     Running     0          23m
milvus-etcd-0                        1/1     Running     0          23m
milvus-etcd-1                        1/1     Running     0          23m
milvus-etcd-2                        1/1     Running     0          23m
milvus-indexcoord-589b758fbb-pz8zk   1/1     Running     0          23m
milvus-indexnode-8955566f7-8pccf     1/1     Running     0          23m
milvus-minio-0                       1/1     Running     0          23m
milvus-minio-1                       1/1     Running     0          23m
milvus-minio-2                       1/1     Running     0          23m
milvus-minio-3                       1/1     Running     0          23m
milvus-proxy-8464b7df79-77lsh        1/1     Running     0          23m
milvus-pulsar-bookie-0               1/1     Running     0          23m                                                                                                                                                                                                                                                    milvus-pulsar-bookie-1               1/1     Running     0          23m
milvus-pulsar-bookie-2               1/1     Running     0          23m
milvus-pulsar-bookie-init-p98hx      0/1     Completed   0          23m
milvus-pulsar-broker-0               1/1     Running     0          23m
milvus-pulsar-proxy-0                1/1     Running     0          23m
milvus-pulsar-pulsar-init-cxvj4      0/1     Completed   0          23m
milvus-pulsar-recovery-0             1/1     Running     0          23m
milvus-pulsar-zookeeper-0            1/1     Running     0          23m
milvus-pulsar-zookeeper-1            1/1     Running     0          23m
milvus-pulsar-zookeeper-2            1/1     Running     0          22m
milvus-querycoord-b8db7f98c-lgfr8    1/1     Running     0          23m
milvus-querynode-5d569749f5-xg9cf    1/1     Running     0          23m
milvus-rootcoord-64c5c5c5f7-pntgk    1/1     Running     0          23m
---
k logs -f milvus-proxy-8464b7df79-77lsh -n test-milvus
Defaulted container "proxy" out of: proxy, config (init)
2023/10/05 16:10:46 maxprocs: Leaving GOMAXPROCS=8: CPU quota undefined

    __  _________ _   ____  ______
   /  |/  /  _/ /| | / / / / / __/
  / /|_/ // // /_| |/ / /_/ /\ \
 /_/  /_/___/____/___/\____/___/

Welcome to use Milvus!
Version:   v2.3.1
Built:     Fri Sep 22 11:37:42 UTC 2023
GitCommit: 70cf65b05
GoVersion: go version go1.20.7 linux/amd64
...
[2023/10/05 16:14:24.556 +00:00] [INFO] [sessionutil/session_util.go:435] ["put session key into etcd"] [key=by-dev/meta/session/proxy-33] [value="{\"ServerID\":33,\"ServerName\":\"proxy\",\"Address\":\"10.0.39.162:19529\",\"TriggerKill\":true,\"Version\":\"2.3.1\",\"LeaseID\":8122676237663639430}"]
[2023/10/05 16:14:24.556 +00:00] [INFO] [sessionutil/session_util.go:445] ["Service registered successfully"] [ServerName=proxy] [serverID=33]
[2023/10/05 16:14:24.556 +00:00] [INFO] [proxy/proxy.go:143] ["Proxy Register Finished"]
[2023/10/05 16:14:24.559 +00:00] [DEBUG] [proxy/service.go:366] ["start Proxy server done"]
[2023/10/05 16:14:24.559 +00:00] [INFO] [components/proxy.go:55] ["Proxy successfully started"]
[2023/10/05 16:14:24.559 +00:00] [INFO] [logutil/logutil.go:163] ["Log directory"] [configDir=]
[2023/10/05 16:14:24.559 +00:00] [INFO] [logutil/logutil.go:164] ["Set log file to "] [path=]
[2023/10/05 16:14:24.559 +00:00] [INFO] [tracer/tracer.go:71] ["Init tracer finished"] [Exporter=stdout]
[2023/10/05 16:14:24.577 +00:00] [INFO] [sessionutil/session_util.go:852] ["register session success"] [role=proxy] [key=by-dev/meta/session/proxy-33]


---
k get secret -n test-milvus milvus-tls -o yaml
apiVersion: v1
data:
  tls.crt: LS0tLS1CRU--REDUCT
  tls.key: LS0tLS1CRUd--REDUCT
kind: Secret
...
  labels:
    app.kubernetes.io/managed-by: Helm
  name: milvus-tls
  namespace: test-milvus
  resourceVersion: "21054694"
  uid: 457dba7e-14aa-4724-936b-3210c0fbcade
type: kubernetes.io/tls
---
k get deploy -n test-milvus milvus-proxy -o json | jq .spec.template.spec.volumes[2]
{
  "name": "milvus-tls",
  "secret": {
    "defaultMode": 420,
    "secretName": "milvus-tls"
  }
}
---
 k get deploy -n test-milvus milvus-proxy -o json | jq .spec.template.spec.containers[0].volumeMounts[3]
{
  "mountPath": "/etc/milvus/certs/",
  "name": "milvus-tls"
}

To make above change happen, I did these changes in the values.yaml, The tls.crt tls.key values are populated from
https://github.com/milvus-io/milvus/blob/master/configs/cert/server.key
https://github.com/milvus-io/milvus/blob/master/configs/cert/server.pem
and basically using $(cat $file | base64 -w 0)

diff --git a/charts/milvus/values.yaml b/charts/milvus/values.yaml
index 4be4a4b..12b4783 100644
--- a/charts/milvus/values.yaml
+++ b/charts/milvus/values.yaml
@@ -51,12 +51,12 @@ extraConfigFiles:
     #      http:
     #        enabled: true
     #  Enable tlsMode and set the tls cert and key
-    #  tls:
-    #    serverPemPath: /etc/milvus/certs/tls.crt
-    #    serverKeyPath: /etc/milvus/certs/tls.key
-    #  common:
-    #    security:
-    #      tlsMode: 1
+       tls:
+        serverPemPath: /etc/milvus/certs/tls.crt
+        serverKeyPath: /etc/milvus/certs/tls.key
+       common:
+         security:
+           tlsMode: 1

 ## Expose the Milvus service to be accessed from outside the cluster (LoadBalancer service).
 ## or access it from within the cluster (ClusterIP service). Set the service type and the port to serve it.
@@ -237,19 +237,19 @@ proxy:
     debugMode:
       enabled: false
   # Mount a TLS secret into proxy pod
-  ## tls:
-  ##   enabled: true
-  ##   secretName: milvus-tls
-  # expecting base64 encoded values here: i.e. $(cat tls.crt | base64 -w 0) and $(cat tls.key | base64 -w 0)
-  ##   cert:
-  ##   key:
-  ## volume:
-  ## - secret:
-  ##     secretName: milvus-tls-cert
-  ##   name: milvus-tls
-  ## volumeMounts:
-  ## - mountPath: /etc/milvus/certs/
-  ##   name: milvus-tls
+  tls:
+    enabled: true
+    secretName: milvus-tls
+  #expecting base64 encoded values here: i.e. $(cat tls.crt | base64 -w 0) and $(cat tls.key | base64 -w 0)
+    key: LS0tLS1CRU-REDUCT
+    crt: LS0tLS1CRUdJTiB-REDUCT
+  volumes:
+  - secret:
+      secretName: milvus-tls
+    name: milvus-tls
+  volumeMounts:
+  - mountPath: /etc/milvus/certs/
+    name: milvus-tls

I have fixes ready, before pushing agian here are my questions:

1. How do I run your ci-test in my own fork?
2. I updated chart README.md, added instruction on how to set the secret values etc., and decided to leave proxy.tls.enabled=false. To test this feature, the CI needs to using the values file or (--set inline).

[VincentDu2021]

@VincentDu2021

How do I run your ci-test in my own fork?

cd to /charts/milvus, then use ct install --chart-dirs . --charts ., check https://github.com/helm/chart-testing/blob/main/doc/ct_install.md for more information

I updated chart README.md, added instruction on how to set the secret values etc., and decided to leave proxy.tls.enabled=false. To test this feature, the CI needs to using the values file or (--set inline).

Add a new xxx-values.yaml in /charts/milvus/ci/, and run ct again to run test with that values.

You could also includ the proper test value file in this patch.

hi @haorenfsa with 491289b, the ci lint-test is passing in my fork.

The only way to test this feature without breaking the default bevoir is to turn off the proxy.tls, secret creation and mount entirely in the default values.yaml, then adding an overwrite section to turn them on in the ci/cluster-values.yaml. I tried to add an "proxytls-values.yaml" with only the overwrite section and it is not working for me. The test log was huge and probably got truncated in the end, therefore it is hard for me find out what's wrong.

I turned on proxy.tls, enabled tls secret creation and mount in the default values.yaml without the overwrite section in ci/cluster-values.yaml and the lint-test would also pass in my fork, in a separate test

[haorenfsa]

@VincentDu2021 Hi, thank you very much for taking time testing this, it looks good. I'll check later how to include a separated test values file. For now please undo the changes you made on cluster-values.yaml, and we're ready to merge this. Btw, the next version should be 4.0.6.

[VincentDu2021]

@VincentDu2021 Hi, thank you very much for taking time testing this, it looks good. I'll check later how to include a separated test values file. For now please undo the changes you made on cluster-values.yaml, and we're ready to merge this. Btw, the next version should be 4.0.6.

@haorenfsa Hi, my pleasure. Done both as requested

[haorenfsa]

/lgtm

[haorenfsa]

/cc @LoveEachDay

[haorenfsa]

/lgtm
/approve

[LoveEachDay]

/lgtm
/approve

[sre-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: haorenfsa, LoveEachDay, VincentDu2021

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [LoveEachDay]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment