hanahmily commented on code in PR #7:
URL: 
https://github.com/apache/skywalking-banyandb-helm/pull/7#discussion_r1537654012


##########
README.md:
##########
@@ -118,6 +118,31 @@ $ helm install my-release banyandb -f values.yaml
 ## Use external certificate authorities for TLS
 If you'd like to use external certificate authorities, such as Vault, 
corresponding annotations can be injected into 
[banyandb](./chart/templates/statefulset.yaml).
 
+## Setup certificate for etcd TLS
+To establish secure communication for etcd, you can leverage cert-manager to 
generate the necessary TLS certificates. This tool simplifies the process of 
creating and managing certificates.
+
+```yaml
+apiVersion: cert-manager.io/v1
+kind: Certificate
+metadata:
+  name: etcd-client
+  namespace: banyandb
+spec:
+  secretName: etcd-client-tls
+  duration: 17520h
+  renewBefore: 4320h
+  issuerRef:
+    name: banyandb-issuer
+    kind: Issuer
+  usages:
+    - server auth
+    - client auth
+  commonName: banyandb-etcd
+  dnsNames:
+    - "*.banyandb-etcd.default.svc.cluster.local"

Review Comment:
   Please add a section explaining the reason for setting this specific URL.



##########
README.md:
##########
@@ -118,6 +118,31 @@ $ helm install my-release banyandb -f values.yaml
 ## Use external certificate authorities for TLS
 If you'd like to use external certificate authorities, such as Vault, 
corresponding annotations can be injected into 
[banyandb](./chart/templates/statefulset.yaml).
 
+## Setup certificate for etcd TLS
+To establish secure communication for etcd, you can leverage cert-manager to 
generate the necessary TLS certificates. This tool simplifies the process of 
creating and managing certificates.

Review Comment:
   Can you please add a section that clearly outlines the steps involved in the 
installation of cert manager?



##########
chart/templates/statefulset.yaml:
##########
@@ -228,4 +229,291 @@ spec:
                   {{- end }}
             {{- end }}
         {{- end }}
-      {{- end }}
\ No newline at end of file
+      {{- end }}
+{{- end }}
+
+{{- if and .Values.cluster.enabled .Values.cluster.data }}
+apiVersion: apps/v1
+kind: StatefulSet
+metadata:
+  labels:
+    app: {{ template "banyandb.name" . }}
+    chart: {{ .Chart.Name }}-{{ .Chart.Version }}
+    component: "{{ .Values.cluster.data.name }}"
+    heritage: {{ .Release.Service }}
+    release: {{ .Release.Name }}
+  name: {{ template "banyandb.fullname" . }}
+spec:
+  serviceName: banyandb
+  replicas: {{ .Values.cluster.data.replicas }}
+  selector:
+    matchLabels:
+      app: {{ template "banyandb.name" . }}
+      component: "{{ .Values.cluster.data.name }}"
+      release: {{ .Release.Name }}
+      role: {{ .Values.cluster.data.role }}
+  template:
+    metadata:
+      labels:
+        app: {{ template "banyandb.name" . }}
+        component: "{{ .Values.cluster.data.name }}"
+        release: {{ .Release.Name }}
+        role: {{ .Values.cluster.data.role }}
+      {{- if .Values.cluster.data.podAnnotations }}
+      annotations:
+{{ toYaml .Values.cluster.data.podAnnotations | indent 8 }}
+      {{- end }}
+    spec:
+      serviceAccountName: {{ template "banyandb.serviceAccountName" . }}
+      {{- with .Values.cluster.data.securityContext }}
+      securityContext:
+        {{- toYaml . | nindent 8 }}
+      {{- end }}
+      priorityClassName: {{ .Values.cluster.data.priorityClassName }}
+      containers:
+        - name: {{ .Values.cluster.data.name }}
+          image: {{ .Values.cluster.image.repository }}:{{ required 
"banyandb.image.tag is required" .Values.cluster.image.tag }}
+          imagePullPolicy: {{ .Values.cluster.image.pullPolicy }}
+          env:
+            {{- range $env := .Values.cluster.data.env }}
+            - name: {{ $env.name }}
+              value: {{ $env.value }}
+            {{- end }}
+            {{- if .Values.cluster.data.tls.grpcSecretName }}
+            - name: BYDB_TLS
+              value: "true"
+            - name: BYDB_CERT_FILE
+              value: "/etc/tls/{{ .Values.cluster.data.tls.grpcSecretName 
}}/tls.crt"
+            - name: BYDB_KEY_FILE
+              value: "/etc/tls/{{ .Values.cluster.data.tls.grpcSecretName 
}}/tls.key"
+            - name: BYDB_HTTP_GRPC_CERT_FILE
+              value: "/etc/tls/{{ .Values.cluster.data.tls.grpcSecretName 
}}/tls.crt"
+            {{- end }}
+            {{- if and .Values.etcd.auth.rbac.create (not 
.Values.etcd.auth.rbac.allowNoneAuthentication) }}
+            - name: BYDB_ETCD_USERNAME
+              value: "root"
+            - name: BYDB_ETCD_PASSWORD
+              value: {{ .Values.etcd.auth.rbac.rootPassword }}
+            {{- end }}
+            {{- if .Values.etcd.auth.client.secureTransport }}
+            - name: BYDB_ETCD_TLS_CA_FILE
+              value: "/etc/tls/{{ .Values.cluster.data.tls.etcdSecretName 
}}/ca.crt"
+            {{- end }}
+            {{- if .Values.etcd.auth.client.enableAuthentication }}
+            - name: BYDB_ETCD_TLS_CERT_FILE
+              value: "/etc/tls/{{ .Values.cluster.data.tls.etcdSecretName 
}}/tls.crt"
+            - name: BYDB_ETCD_TLS_KEY_FILE
+              value: "/etc/tls/{{ .Values.cluster.data.tls.etcdSecretName 
}}/tls.key"
+            {{- end }}
+            {{- if and (not .Values.etcd.enabled) 
.Values.cluster.etcdEndpoints }}
+            - name: BYDB_ETCD_ENDPOINTS
+              value: "{{- .Values.cluster.etcdEndpoints | join "," -}}"
+            {{- else }}
+            {{- $endpoints := list }}
+            {{- $replicaCount := int .Values.etcd.replicaCount }}
+            {{- $releaseName := .Release.Name }}
+            {{- $namespace := .Release.Namespace }}
+            {{- range $i := until $replicaCount }}
+              {{- $endpoint := printf "%s-etcd-%d.%s-etcd-headless.%s:2379" 
$releaseName $i $releaseName $namespace }}
+              {{- $endpoints = append $endpoints $endpoint }}

Review Comment:
   You should move this piece to `_helpers.tpl` to reuse it for deployment.



##########
README.md:
##########
@@ -118,6 +118,31 @@ $ helm install my-release banyandb -f values.yaml
 ## Use external certificate authorities for TLS
 If you'd like to use external certificate authorities, such as Vault, 
corresponding annotations can be injected into 
[banyandb](./chart/templates/statefulset.yaml).
 
+## Setup certificate for etcd TLS
+To establish secure communication for etcd, you can leverage cert-manager to 
generate the necessary TLS certificates. This tool simplifies the process of 
creating and managing certificates.
+
+```yaml
+apiVersion: cert-manager.io/v1
+kind: Certificate
+metadata:
+  name: etcd-client
+  namespace: banyandb
+spec:
+  secretName: etcd-client-tls
+  duration: 17520h
+  renewBefore: 4320h
+  issuerRef:
+    name: banyandb-issuer
+    kind: Issuer
+  usages:
+    - server auth
+    - client auth
+  commonName: banyandb-etcd
+  dnsNames:
+    - "*.banyandb-etcd.default.svc.cluster.local"

Review Comment:
   When the embed etcd is enabled, liaison and data nodes use headless services 
to access etcd. What is the common service used for?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscr...@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to