Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add basic TLS support for TiDB cluster #750

Merged
merged 26 commits into from
Sep 4, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
f31172f
discovery: add HTTPS URL support
AstroProfundis Aug 5, 2019
5bb2384
tls: add basic support of certis
AstroProfundis Aug 5, 2019
99956b7
tls: support tls client for pd
AstroProfundis Aug 6, 2019
84cc8bd
tls: support tls client for controller
AstroProfundis Aug 6, 2019
71bf08d
tls: update startup script templates
AstroProfundis Aug 7, 2019
ecfe87e
tls: fix configs
AstroProfundis Aug 7, 2019
926073e
tls: fix health check for tidb
AstroProfundis Aug 8, 2019
e4b319a
tls: create new pd client when scheme changed
AstroProfundis Aug 8, 2019
4da9be3
tls: fix return value when loading CAs
AstroProfundis Aug 9, 2019
451011a
Merge branch 'master' into tls-stage1
AstroProfundis Aug 9, 2019
b6cd879
tls: fix test errors
AstroProfundis Aug 9, 2019
030a04c
tls: fix typo in scripts
AstroProfundis Aug 12, 2019
b392aa2
Merge branch 'master' into tls-stage1
cofyc Aug 16, 2019
4b27ddd
fix test errors
AstroProfundis Aug 17, 2019
665e2fa
Merge branch 'tls-stage1' of github.com:AstroProfundis/tidb-operator …
AstroProfundis Aug 17, 2019
57ff597
tls: change config name and use better layout
AstroProfundis Aug 21, 2019
d8e982e
tls: refine scheme detection
AstroProfundis Aug 21, 2019
4f9bea5
tls: fix keys in templates
AstroProfundis Aug 21, 2019
04a9b59
tls: fix test cases
AstroProfundis Aug 21, 2019
0602bb5
Merge remote-tracking branch 'upstream' into tls-stage1
AstroProfundis Aug 23, 2019
eea2feb
Merge branch 'master' into tls-stage1
AstroProfundis Aug 26, 2019
7d9d03b
tls: update coding styles
AstroProfundis Aug 27, 2019
990b357
Merge branch 'master' into tls-stage1
AstroProfundis Aug 28, 2019
7f1e876
Merge branch 'master' into tls-stage1
cofyc Sep 2, 2019
0dca5a7
Merge branch 'master' into tls-stage1
AstroProfundis Sep 2, 2019
eb628d7
Merge branch 'master' into tls-stage1
tennix Sep 4, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 28 additions & 0 deletions charts/tidb-cluster/templates/_helpers.tpl
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,13 @@ config-file: |-
{{- if .Values.pd.config }}
{{ .Values.pd.config | indent 2 }}
{{- end -}}
{{- if .Values.enableTLSCluster }}
[security]
cacert-path = "/var/run/secrets/kubernetes.io/serviceaccount/ca.crt"
cert-path = "/var/lib/pd-tls/pd.crt"
key-path = "/var/lib/pd-tls/pd.key"
{{- end -}}

{{- end -}}

{{- define "pd-configmap.data-digest" -}}
Expand All @@ -53,6 +60,13 @@ config-file: |-
{{- if .Values.tikv.config }}
{{ .Values.tikv.config | indent 2 }}
{{- end -}}
{{- if .Values.enableTLSCluster }}
[security]
ca-path = "/var/run/secrets/kubernetes.io/serviceaccount/ca.crt"
cert-path = "/var/lib/tikv-tls/tikv.crt"
key-path = "/var/lib/tikv-tls/tikv.key"
{{- end -}}

{{- end -}}

{{- define "tikv-configmap.data-digest" -}}
Expand All @@ -73,6 +87,20 @@ config-file: |-
{{- if .Values.tidb.config }}
{{ .Values.tidb.config | indent 2 }}
{{- end -}}
{{- if or .Values.enableTLSCluster .Values.enableTLSClient }}
[security]
{{- end -}}
{{- if .Values.enableTLSCluster }}
cluster-ssl-ca = "/var/run/secrets/kubernetes.io/serviceaccount/ca.crt"
cluster-ssl-cert = "/var/lib/tidb-tls/tidb.crt"
cluster-ssl-key = "/var/lib/tidb-tls/tidb.key"
cofyc marked this conversation as resolved.
Show resolved Hide resolved
{{- end -}}
{{- if .Values.tidb.enableTLSClient }}
ssl-ca = "/var/run/secrets/kubernetes.io/serviceaccount/ca.crt"
ssl-cert = "/var/lib/tidb-tls/tidb.crt"
ssl-key = "/var/lib/tidb-tls/tidb.key"
Copy link
Contributor

@cofyc cofyc Aug 17, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we support users to configure their own MySQL certificates? Users may want to configure an existing certificate in the new cluster.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cc @tennix @weekface What do you think?
One case is when a user migrates a MySQL instance which already has TLS enabled but doesn't want to update application certificates. The user must configure exiting certificates into tidb-server.
If we're going to support this, do we need to support users to configure secret with an arbitrary name?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm thinking about the same thing too, but AFAIK we don't have any plan to support custom certs yet, if we do, custom CA importing would also need to be added, and we need to validate user uploaded certs to ensure they are signed by the uploaded CA, that would need more code changes and seems better to be another PR.

Copy link
Contributor

@cofyc cofyc Aug 21, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

agree. however, it seems not right to use cluster TLS certs as tidb-server TLS server for MySQL clients, focusing on cluster TLS support only in this PR?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we did not support the custom certs now, I don't think it is necessary to expose these parameters to the user, which will make the user confused or misconfigured.

{{- end -}}

{{- end -}}

{{- define "tidb-configmap.data-digest" -}}
Expand Down
12 changes: 11 additions & 1 deletion charts/tidb-cluster/templates/discovery-deployment.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -45,4 +45,14 @@ spec:
valueFrom:
fieldRef:
fieldPath: metadata.namespace

{{- if .Values.enableTLSCluster }}
volumeMounts:
- mountPath: /var/lib/tls
name: tls
readOnly: true
volumes:
- name: tls
secret:
defaultMode: 420
secretName: client-tls
{{- end -}}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIRC, have you changed the way the certs was injected, by injecting certs in the program instead of mount certs into pod?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it's been changed in #782

10 changes: 6 additions & 4 deletions charts/tidb-cluster/templates/scripts/_start_pd.sh.tpl
Original file line number Diff line number Diff line change
Expand Up @@ -58,12 +58,14 @@ while true; do
fi
done

SCHEME={{ if .Values.enableTLSCluster }}"https"{{ else }}"http"{{ end }}

ARGS="--data-dir=/var/lib/pd \
--name=${POD_NAME} \
--peer-urls=http://0.0.0.0:2380 \
--advertise-peer-urls=http://${domain}:2380 \
--client-urls=http://0.0.0.0:2379 \
--advertise-client-urls=http://${domain}:2379 \
--peer-urls=${SCHEME}://0.0.0.0:2380 \
--advertise-peer-urls=${SCHEME}://${domain}:2380 \
--client-urls=${SCHEME}://0.0.0.0:2379 \
--advertise-client-urls=${SCHEME}://${domain}:2379 \
--config=/etc/pd/pd.toml \
"

Expand Down
4 changes: 3 additions & 1 deletion charts/tidb-cluster/templates/scripts/_start_tikv.sh.tpl
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,11 @@ then
tail -f /dev/null
fi

SCHEME={{ if .Values.enableTLSCluster }}"https"{{ else }}"http"{{ end }}

# Use HOSTNAME if POD_NAME is unset for backward compatibility.
POD_NAME=${POD_NAME:-$HOSTNAME}
ARGS="--pd=${CLUSTER_NAME}-pd:2379 \
ARGS="--pd=${SCHEME}://${CLUSTER_NAME}-pd:2379 \
--advertise-addr=${POD_NAME}.${HEADLESS_SERVICE_NAME}.${NAMESPACE}.svc:20160 \
--addr=0.0.0.0:20160 \
--status-addr=0.0.0.0:20180 \
Expand Down
2 changes: 2 additions & 0 deletions charts/tidb-cluster/templates/tidb-cluster.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ metadata:
spec:
pvReclaimPolicy: {{ .Values.pvReclaimPolicy }}
timezone: {{ .Values.timezone | default "UTC" }}
enableTLSCluster: {{ .Values.enableTLSCluster | default false }}
enableTLSClient: {{ .Values.enableTLSClient | default false }}
services:
{{ toYaml .Values.services | indent 4 }}
schedulerName: {{ .Values.schedulerName | default "default-scheduler" }}
Expand Down
12 changes: 12 additions & 0 deletions charts/tidb-cluster/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,11 @@ discovery:
# if the ConfigMap was not changed.
enableConfigMapRollout: true

# Whether enable TLS connections between server nodes.
# When enabled, PD/TiDB/TiKV will use TLS encrypted connections to transfer data between each node,
# certificates will be generated automatically (if not already present).
enableTLSCluster: false

pd:
# Please refer to https://github.com/pingcap/pd/blob/master/conf/config.toml for the default
# pd configurations (change to the tags of your pd version),
Expand Down Expand Up @@ -247,6 +252,7 @@ tidb:
config: |
[log]
level = "info"

# # Here are some parameters you MUST customize (Please configure in the above 'tidb.config' section):
# [performance]
# # Normally it should be tuned to `tidb.resources.limits.cpu`, for example: 16000m -> 16
Expand Down Expand Up @@ -330,6 +336,12 @@ tidb:
# the start argument to specify the plugin id (name "-" version) that needs to be loaded, e.g. 'conn_limit-1'.
list: ["whitelist-1"]

# Whether enable TLS connection between TiDB server and MySQL client.
# When enabled, TiDB will accept TLS encrypted connections from MySQL client, certificates will be generated
# automatically.
# Note: TLS connection is not forced on the server side, plain connections are also accepted after enableing.
enableTLSClient: false

# mysqlClient is used to set password for TiDB
# it must has Python MySQL client installed
mysqlClient:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,15 @@ spec:
fieldPath: metadata.namespace
- name: TZ
value: {{ .Values.timezone | default "UTC" }}
volumeMounts:
- mountPath: /var/lib/tls
name: tls
readOnly: true
volumes:
- name: tls
secret:
defaultMode: 420
secretName: client-tls
cofyc marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to create the client-tls secret manually?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As of this PR, Yes, and the cert need to be signed by the k8s CA by kubectl certificate approve, as described in the PR OP.

PR #782 implements the automatic process of generating certs.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

make this optional, otherwise, it will fail when users don't need to enable TLS

{{- with .Values.controllerManager.nodeSelector }}
nodeSelector:
{{ toYaml . | indent 8 }}
Expand All @@ -67,4 +76,4 @@ spec:
{{- with .Values.controllerManager.tolerations }}
tolerations:
{{ toYaml . | indent 8 }}
{{- end }}
{{- end }}
7 changes: 7 additions & 0 deletions pkg/apis/pingcap.com/v1alpha1/tidbcluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -158,3 +158,10 @@ func (tc *TidbCluster) TiKVIsAvailable() bool {
func (tc *TidbCluster) GetClusterID() string {
return tc.Status.ClusterID
}

func (tc *TidbCluster) Scheme() string {
if tc.Spec.EnableTLSCluster {
return "https"
}
return "http"
}
9 changes: 6 additions & 3 deletions pkg/apis/pingcap.com/v1alpha1/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,8 @@ type TidbClusterSpec struct {
Services []Service `json:"services,omitempty"`
PVReclaimPolicy corev1.PersistentVolumeReclaimPolicy `json:"pvReclaimPolicy,omitempty"`
Timezone string `json:"timezone,omitempty"`
// Enable TLS connection between TiDB server compoments
EnableTLSCluster bool `json:"enableTLSCluster,omitempty"`
}

// TidbClusterStatus represents the current status of a tidb cluster.
Expand All @@ -103,7 +105,7 @@ type TidbClusterStatus struct {
TiDB TiDBStatus `json:"tidb,omitempty"`
}

// PDSpec contains details of PD member
// PDSpec contains details of PD members
type PDSpec struct {
ContainerSpec
Replicas int32 `json:"replicas"`
Expand All @@ -115,7 +117,7 @@ type PDSpec struct {
StorageClassName string `json:"storageClassName,omitempty"`
}

// TiDBSpec contains details of PD member
// TiDBSpec contains details of TiDB members
type TiDBSpec struct {
ContainerSpec
Replicas int32 `json:"replicas"`
Expand All @@ -129,14 +131,15 @@ type TiDBSpec struct {
MaxFailoverCount int32 `json:"maxFailoverCount,omitempty"`
SeparateSlowLog bool `json:"separateSlowLog,omitempty"`
SlowLogTailer TiDBSlowLogTailerSpec `json:"slowLogTailer,omitempty"`
EnableTLSClient bool `json:"enableTLSClient,omitempty"`
}

// TiDBSlowLogTailerSpec represents an optional log tailer sidecar with TiDB
type TiDBSlowLogTailerSpec struct {
ContainerSpec
}

// TiKVSpec contains details of PD member
// TiKVSpec contains details of TiKV members
type TiKVSpec struct {
ContainerSpec
Replicas int32 `json:"replicas"`
Expand Down
2 changes: 1 addition & 1 deletion pkg/controller/pd_control.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import (

// GetPDClient gets the pd client from the TidbCluster
func GetPDClient(pdControl pdapi.PDControlInterface, tc *v1alpha1.TidbCluster) pdapi.PDClient {
return pdControl.GetPDClient(pdapi.Namespace(tc.GetNamespace()), tc.GetName())
return pdControl.GetPDClient(pdapi.Namespace(tc.GetNamespace()), tc.GetName(), tc.Spec.EnableTLSCluster)
}

// NewFakePDClient creates a fake pdclient that is set as the pd client
Expand Down
3 changes: 2 additions & 1 deletion pkg/controller/pod_control.go
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,8 @@ func (rpc *realPodControl) UpdateMetaInfo(tc *v1alpha1.TidbCluster, pod *corev1.
clusterID := labels[label.ClusterIDLabelKey]
memberID := labels[label.MemberIDLabelKey]
storeID := labels[label.StoreIDLabelKey]
pdClient := rpc.pdControl.GetPDClient(pdapi.Namespace(tc.GetNamespace()), tcName)

pdClient := rpc.pdControl.GetPDClient(pdapi.Namespace(tc.GetNamespace()), tcName, tc.Spec.EnableTLSCluster)
if labels[label.ClusterIDLabelKey] == "" {
cluster, err := pdClient.GetCluster()
if err != nil {
Expand Down
44 changes: 38 additions & 6 deletions pkg/controller/tidb_control.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
package controller

import (
"crypto/tls"
"encoding/json"
"fmt"
"io/ioutil"
Expand Down Expand Up @@ -55,18 +56,37 @@ type defaultTiDBControl struct {

// NewDefaultTiDBControl returns a defaultTiDBControl instance
func NewDefaultTiDBControl() TiDBControlInterface {
httpClient := &http.Client{Timeout: timeout}
return &defaultTiDBControl{httpClient: httpClient}
return &defaultTiDBControl{httpClient: &http.Client{Timeout: timeout}}
}
AstroProfundis marked this conversation as resolved.
Show resolved Hide resolved

func (tdc *defaultTiDBControl) useTLSHTTPClient(enableTLS bool) error {
if enableTLS {
rootCAs, err := httputil.ReadCACerts()
if err != nil {
return err
}
config := &tls.Config{
RootCAs: rootCAs,
}
tdc.httpClient.Transport = &http.Transport{TLSClientConfig: config}
}
return nil
}

func (tdc *defaultTiDBControl) GetHealth(tc *v1alpha1.TidbCluster) map[string]bool {
tcName := tc.GetName()
ns := tc.GetNamespace()
scheme := tc.Scheme()

result := map[string]bool{}

if err := tdc.useTLSHTTPClient(tc.Spec.EnableTLSCluster); err != nil {
return result
}

for i := 0; i < int(tc.TiDBRealReplicas()); i++ {
hostName := fmt.Sprintf("%s-%d", TiDBMemberName(tcName), i)
url := fmt.Sprintf("http://%s.%s.%s:10080/status", hostName, TiDBPeerMemberName(tcName), ns)
url := fmt.Sprintf("%s://%s.%s.%s:10080/status", scheme, hostName, TiDBPeerMemberName(tcName), ns)
_, err := tdc.getBodyOK(url)
if err != nil {
result[hostName] = false
Expand All @@ -80,9 +100,13 @@ func (tdc *defaultTiDBControl) GetHealth(tc *v1alpha1.TidbCluster) map[string]bo
func (tdc *defaultTiDBControl) ResignDDLOwner(tc *v1alpha1.TidbCluster, ordinal int32) (bool, error) {
tcName := tc.GetName()
ns := tc.GetNamespace()
scheme := tc.Scheme()
if err := tdc.useTLSHTTPClient(tc.Spec.EnableTLSCluster); err != nil {
return false, err
}

hostName := fmt.Sprintf("%s-%d", TiDBMemberName(tcName), ordinal)
url := fmt.Sprintf("http://%s.%s.%s:10080/ddl/owner/resign", hostName, TiDBPeerMemberName(tcName), ns)
url := fmt.Sprintf("%s://%s.%s.%s:10080/ddl/owner/resign", scheme, hostName, TiDBPeerMemberName(tcName), ns)
req, err := http.NewRequest("POST", url, nil)
if err != nil {
return false, err
Expand All @@ -105,9 +129,13 @@ func (tdc *defaultTiDBControl) ResignDDLOwner(tc *v1alpha1.TidbCluster, ordinal
func (tdc *defaultTiDBControl) GetInfo(tc *v1alpha1.TidbCluster, ordinal int32) (*dbInfo, error) {
tcName := tc.GetName()
ns := tc.GetNamespace()
scheme := tc.Scheme()
if err := tdc.useTLSHTTPClient(tc.Spec.EnableTLSCluster); err != nil {
return nil, err
}

hostName := fmt.Sprintf("%s-%d", TiDBMemberName(tcName), ordinal)
url := fmt.Sprintf("http://%s.%s.%s:10080/info", hostName, TiDBPeerMemberName(tcName), ns)
url := fmt.Sprintf("%s://%s.%s.%s:10080/info", scheme, hostName, TiDBPeerMemberName(tcName), ns)
req, err := http.NewRequest("POST", url, nil)
if err != nil {
return nil, err
Expand Down Expand Up @@ -136,9 +164,13 @@ func (tdc *defaultTiDBControl) GetInfo(tc *v1alpha1.TidbCluster, ordinal int32)
func (tdc *defaultTiDBControl) GetSettings(tc *v1alpha1.TidbCluster, ordinal int32) (*config.Config, error) {
tcName := tc.GetName()
ns := tc.GetNamespace()
scheme := tc.Scheme()
if err := tdc.useTLSHTTPClient(tc.Spec.EnableTLSCluster); err != nil {
return nil, err
}

hostName := fmt.Sprintf("%s-%d", TiDBMemberName(tcName), ordinal)
url := fmt.Sprintf("http://%s.%s.%s:10080/settings", hostName, TiDBPeerMemberName(tcName), ns)
url := fmt.Sprintf("%s://%s.%s.%s:10080/settings", scheme, hostName, TiDBPeerMemberName(tcName), ns)
req, err := http.NewRequest("GET", url, nil)
if err != nil {
return nil, err
Expand Down
4 changes: 2 additions & 2 deletions pkg/discovery/discovery.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,10 +94,10 @@ func (td *tidbDiscovery) Discover(advertisePeerUrl string) (string, error) {

if len(currentCluster.peers) == int(replicas) {
delete(currentCluster.peers, podName)
return fmt.Sprintf("--initial-cluster=%s=http://%s", podName, advertisePeerUrl), nil
return fmt.Sprintf("--initial-cluster=%s=%s://%s", podName, tc.Scheme(), advertisePeerUrl), nil
}

pdClient := td.pdControl.GetPDClient(pdapi.Namespace(tc.GetNamespace()), tc.GetName())
pdClient := td.pdControl.GetPDClient(pdapi.Namespace(tc.GetNamespace()), tc.GetName(), tc.Spec.EnableTLSCluster)
membersInfo, err := pdClient.GetMembers()
if err != nil {
return "", err
Expand Down
Loading