address comments

Signed-off-by: Yun-Tang Hsu <[email protected]>

build/charts/flow-aggregator/README.md

@@ -26,9 +26,13 @@ Kubernetes: `>= 1.16.0-0`
 | clickHouse.commitInterval | string | `"8s"` | CommitInterval is the periodical interval between batch commit of flow records to DB. Valid time units are "ns", "us" (or "µs"), "ms", "s", "m", "h". |
 | clickHouse.compress | bool | `true` | Compress enables lz4 compression when committing flow records. |
 | clickHouse.connectionSecret | object | `{"password":"clickhouse_operator_password","username":"clickhouse_operator"}` | Credentials to connect to ClickHouse. They will be stored in a Secret. |
-| clickHouse.databaseURL | string | `"tcp://clickhouse-clickhouse.flow-visibility.svc:9000"` | DatabaseURL is the url to the database. TCP protocol is required. |
+| clickHouse.databaseURL | string | `"tcp://clickhouse-clickhouse.flow-visibility.svc:9000"` | DatabaseURL is the url to the database. TCP/HTTP/HTTPS protocols are supported. |
 | clickHouse.debug | bool | `false` | Debug enables debug logs from ClickHouse sql driver. |
 | clickHouse.enable | bool | `false` | Determine whether to enable exporting flow records to ClickHouse. |
+| clickHouse.tls | object | `{"customCACert":false,"enable":false,"insecureSkipVerify":false}` | tls contains tls related configuration options, which is used to connect to clickhouse service. |
+| clickHouse.tls.customCACert | bool | `false` | Indicates whether to use custom CA certificate. Default root CAs will be used if this field is false. If true, a Secret named "clickhouse-ca" must be provided with the following keys: ca.crt: <CA certificate> |
+| clickHouse.tls.enable | bool | `false` | Determine whether to enable tls. |
+| clickHouse.tls.insecureSkipVerify | bool | `false` | Determine whether to skip the verification of the server's certificate chain and host name. Default is false. |
 | flowAggregatorAddress | string | `""` | Provide an extra DNS name or IP address of flow aggregator for generating TLS certificate. |
 | flowCollector.address | string | `""` | Provide the flow collector address as string with format <IP>:<port>[:<proto>],  where proto is tcp or udp. If no L4 transport proto is given, we consider tcp as default. |
 | flowCollector.enable | bool | `false` | Determine whether to enable exporting flow records to external flow collector. |

build/charts/flow-aggregator/conf/flow-aggregator.conf

@@ -80,7 +80,7 @@ clickHouse:
     # Enable is the switch to enable TLS when connecting to ClickHouse server.
     enable: {{ .Values.clickHouse.tls.enable }}
 
-    # InsecureSkipVerify determine whether a client verifies the server's certificate chain and host name.
+    # InsecureSkipVerify determine whether to skip the verification of the server's certificate chain and host name.
     # Default is false.
     insecureSkipVerify: {{ .Values.clickHouse.tls.insecureSkipVerify }}
 

build/charts/flow-aggregator/templates/deployment.yaml

@@ -79,8 +79,10 @@ spec:
           readOnly: true
         - mountPath: /var/log/antrea/flow-aggregator
           name: host-var-log-antrea-flow-aggregator
+        {{- if and .Values.clickHouse.tls.enable .Values.clickHouse.tls.customCACert }}
         - name: clickhouse-ca
           mountPath: /etc/ssl/certs
+        {{- end }}
       nodeSelector:
         kubernetes.io/os: linux
         kubernetes.io/arch: amd64
@@ -93,9 +95,11 @@ spec:
         hostPath:
           path: /var/log/antrea/flow-aggregator
           type: DirectoryOrCreate
+      {{- if and .Values.clickHouse.tls.enable .Values.clickHouse.tls.customCACert }}
       # Make it optional as we only read it when customCACert=true.
       - name: clickhouse-ca
         secret:
           secretName: clickhouse-ca
           defaultMode: 0400
           optional: true
+      {{- end }}

build/charts/flow-aggregator/values.yaml

@@ -51,7 +51,7 @@ clickHouse:
   tls:
     # -- Determine whether to enable tls.
     enable: false
-    # -- Determine whether a client verifies the server's certificate chain and host name. Default is false.
+    # -- Determine whether to skip the verification of the server's certificate chain and host name. Default is false.
     insecureSkipVerify: false
     # -- Indicates whether to use custom CA certificate. Default root CAs will be used if this field is false.
     # If true, a Secret named "clickhouse-ca" must be provided with the following keys:

build/yamls/flow-aggregator.yml

@@ -232,7 +232,7 @@ data:
         # Enable is the switch to enable TLS when connecting to ClickHouse server.
         enable: false
 
-        # InsecureSkipVerify determine whether a client verifies the server's certificate chain and host name.
+        # InsecureSkipVerify determine whether to skip the verification of the server's certificate chain and host name.
         # Default is false.
         insecureSkipVerify: false
 
@@ -448,8 +448,6 @@ spec:
           readOnly: true
         - mountPath: /var/log/antrea/flow-aggregator
           name: host-var-log-antrea-flow-aggregator
-        - mountPath: /etc/ssl/certs
-          name: clickhouse-ca
       nodeSelector:
         kubernetes.io/arch: amd64
         kubernetes.io/os: linux
@@ -462,8 +460,3 @@ spec:
           path: /var/log/antrea/flow-aggregator
           type: DirectoryOrCreate
         name: host-var-log-antrea-flow-aggregator
-      - name: clickhouse-ca
-        secret:
-          defaultMode: 256
-          optional: true
-          secretName: clickhouse-ca

docs/network-flow-visibility.md

@@ -291,13 +291,15 @@ the URL for `clickHouse.databaseURL` in the following format:
 
 #### Support secure connections to ClickHouse database
 
-Since Antrea v1.13, you can enable TLS when connecting to ClickHouse Server by setting `clickHouse.tls.enable` to `true`.
-You can also change the value of `clickHouse.tls.insecureSkipVerify` to determine whether a client verifies the
-server's certificate.
-If you want to provide a custom CA certificate, you can set `clickHouse.tls.customizedCACert` to `true` and
-the flow-aggregator will read the certificate key pair from the `clickhouse-ca` Secret.
+Since Antrea v1.13, you can enable TLS when connecting to ClickHouse Server by
+setting `clickHouse.tls.enable` to `true`. You can also change the value of
+`clickHouse.tls.insecureSkipVerify` to determine whether a client verifies the
+server's certificate. If you want to provide a custom CA certificate, you can
+set `clickHouse.tls.customizedCACert` to `true` and the flow-aggregator will
+read the certificate key pair from the `clickhouse-ca` Secret.
 
-Make sure to follow the following form when creating the `clickhouse-ca` Secret with the custom CA certificate:
+Make sure to follow the following form when creating the `clickhouse-ca` Secret
+with the custom CA certificate:
 
 ```yaml
 apiVersion: v1
@@ -311,16 +313,17 @@ data:
 
 #### Using kubectl
 
-You can use `kubectl apply -f <PATH TO SECRET YAML>` to create the above secret, or use `kubectl create secret`:
+You can use `kubectl apply -f <PATH TO SECRET YAML>` to create the above secret
+, or use `kubectl create secret`:
 
 ```bash
 kubectl create secret generic clickhouse-ca -n flow-aggregator --from-file=ca.crt=<PATH TO CA CERTIFICATE>
 ```
 
 #### Configuration for ClickHouse pre Antrea v1.13
 
-We don't support secure connection to ClickHouse database prior to Antrea v1.13. We only support TCP when connecting to
-ClickHouse server from Flow-Aggregator.
+We don't support secure connection to ClickHouse database prior to Antrea v1.13.
+We only support TCP when connecting to ClickHouse server from Flow-Aggregator.
 
 #### Example of flow-aggregator.conf
 
@@ -404,7 +407,7 @@ flow-aggregator.conf: |
       # Enable is the switch to enable TLS when connecting to ClickHouse server.
       enable: false
 
-      # InsecureSkipVerify determine whether a client verifies the server's certificate chain and host name.
+      # InsecureSkipVerify determine whether to skip the verification of the server's certificate chain and host name.
       # Default is false.
       insecureSkipVerify: false
 

pkg/config/flowaggregator/config.go

@@ -113,7 +113,7 @@ type ClickHouseConfig struct {
 type TLSConfig struct {
 	// Enable is the switch to enable TLS when connecting to ClickHouse server.
 	Enable bool `yaml:"enable,omitempty"`
-	// InsecureSkipVerify determine whether a client verifies the server's certificate chain and host name.
+	// InsecureSkipVerify determine whether to skip the verification of the server's certificate chain and host name.
 	// Default is false.
 	InsecureSkipVerify bool `yaml:"insecureSkipVerify,omitempty"`
 	// CustomCACert determine whether to use custom CA certificate. Default root CAs will be used if false.

pkg/flowaggregator/clickhouseclient/clickhouseclient.go

@@ -34,6 +34,7 @@ import (
 )
 
 const (
+	ProtocolUnknown   = -1
 	maxQueueSize      = 1 << 19 // 524288. ~500MB assuming 1KB per record
 	queueFlushTimeout = 10 * time.Second
 	insertQuery       = `INSERT INTO flows (
@@ -475,22 +476,22 @@ func parseDatabaseURL(dbUrl string, secure bool) (clickhouse.Protocol, string, e
 		return -1, "", fmt.Errorf("failed to parse ClickHouse database URL: %w", err)
 	}
 	if u.Path != "" || u.RawQuery != "" || u.User != nil {
-		return -1, "", fmt.Errorf("invalid ClickHouse database URL '%s': path, query string or user info should not be set", dbUrl)
+		return ProtocolUnknown, "", fmt.Errorf("invalid ClickHouse database URL '%s': path, query string or user info should not be set", dbUrl)
 	}
 	switch u.Scheme {
 	case "http":
 		if secure {
-			return -1, u.Host, fmt.Errorf("invalid ClickHouse setting: http with TLS enabled")
+			return ProtocolUnknown, u.Host, fmt.Errorf("invalid ClickHouse setting: http with TLS enabled")
 		}
 		return clickhouse.HTTP, u.Host, nil
 	case "https":
 		if !secure {
-			return -1, u.Host, fmt.Errorf("invalid ClickHouse setting: https without TLS enabled")
+			return ProtocolUnknown, u.Host, fmt.Errorf("invalid ClickHouse setting: https without TLS enabled")
 		}
 		return clickhouse.HTTP, u.Host, nil
 	case "tcp":
 		return clickhouse.Native, u.Host, nil
 	default:
-		return -1, "", fmt.Errorf("connection over %s transport protocol is not supported", u.Scheme)
+		return ProtocolUnknown, "", fmt.Errorf("connection over %s transport protocol is not supported", u.Scheme)
 	}
 }