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

Skip the TLS of the PD dashboard when the TiDB version < v4.0.0 #2389

Merged
merged 12 commits into from
May 8, 2020
11 changes: 11 additions & 0 deletions pkg/apis/pingcap/v1alpha1/tidbcluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ package v1alpha1
import (
"encoding/json"
"fmt"
"strings"

"github.com/pingcap/advanced-statefulset/pkg/apis/apps/v1/helper"
"github.com/pingcap/tidb-operator/pkg/label"
Expand Down Expand Up @@ -55,6 +56,16 @@ func (tc *TidbCluster) PDImage() string {
return image
}

func (tc *TidbCluster) PDVersion() string {
weekface marked this conversation as resolved.
Show resolved Hide resolved
image := tc.PDImage()
colonIdx := strings.LastIndexByte(image, ':')
if colonIdx >= 0 {
return image[colonIdx+1:]
}

return "latest"
}

func (tc *TidbCluster) TiKVImage() string {
image := tc.Spec.TiKV.Image
baseImage := tc.Spec.TiKV.BaseImage
Expand Down
41 changes: 41 additions & 0 deletions pkg/apis/pingcap/v1alpha1/tidbcluster_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -436,6 +436,47 @@ func TestHelperImagePullPolicy(t *testing.T) {
}
}

func TestPDVersion(t *testing.T) {
g := NewGomegaWithT(t)

type testcase struct {
name string
update func(*TidbCluster)
expectFn func(*GomegaWithT, *TidbCluster)
}
testFn := func(test *testcase, t *testing.T) {
t.Log(test.name)

tc := newTidbCluster()
test.update(tc)
test.expectFn(g, tc)
}
tests := []testcase{
{
name: "has tag",
update: func(tc *TidbCluster) {
tc.Spec.PD.Image = "pingcap/pd:v3.1.0"
},
expectFn: func(g *GomegaWithT, tc *TidbCluster) {
g.Expect(tc.PDVersion()).To(Equal("v3.1.0"))
},
},
{
name: "don't have tag",
update: func(tc *TidbCluster) {
tc.Spec.PD.Image = "pingcap/pd"
},
expectFn: func(g *GomegaWithT, tc *TidbCluster) {
g.Expect(tc.PDVersion()).To(Equal("latest"))
},
},
}

for i := range tests {
testFn(&tests[i], t)
}
}

func newTidbCluster() *TidbCluster {
return &TidbCluster{
TypeMeta: metav1.TypeMeta{
Expand Down
27 changes: 24 additions & 3 deletions pkg/manager/member/pd_member_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"strconv"
"strings"

"github.com/Masterminds/semver"
"github.com/pingcap/tidb-operator/pkg/apis/pingcap/v1alpha1"
"github.com/pingcap/tidb-operator/pkg/controller"
"github.com/pingcap/tidb-operator/pkg/label"
Expand Down Expand Up @@ -537,6 +538,11 @@ func getNewPDSetForTidbCluster(tc *v1alpha1.TidbCluster, cm *corev1.ConfigMap) (
pdConfigMap = cm.Name
}

clusterVersionGE4, err := clusterVersionGreaterThanOrEqualTo4(tc.PDVersion())
if err != nil {
klog.Warningf("cluster version: %s is not semantic versioning compatible", tc.PDVersion())
}

annMount, annVolume := annotationsMountVolume()
volMounts := []corev1.VolumeMount{
annMount,
Expand All @@ -549,7 +555,7 @@ func getNewPDSetForTidbCluster(tc *v1alpha1.TidbCluster, cm *corev1.ConfigMap) (
Name: "pd-tls", ReadOnly: true, MountPath: "/var/lib/pd-tls",
})
}
if tc.Spec.TiDB.IsTLSClientEnabled() && !tc.SkipTLSWhenConnectTiDB() {
if tc.Spec.TiDB.IsTLSClientEnabled() && !tc.SkipTLSWhenConnectTiDB() && clusterVersionGE4 {
volMounts = append(volMounts, corev1.VolumeMount{
Name: "tidb-client-tls", ReadOnly: true, MountPath: tidbClientCertPath,
})
Expand Down Expand Up @@ -587,7 +593,7 @@ func getNewPDSetForTidbCluster(tc *v1alpha1.TidbCluster, cm *corev1.ConfigMap) (
},
})
}
if tc.Spec.TiDB.IsTLSClientEnabled() && !tc.SkipTLSWhenConnectTiDB() {
if tc.Spec.TiDB.IsTLSClientEnabled() && !tc.SkipTLSWhenConnectTiDB() && clusterVersionGE4 {
vols = append(vols, corev1.Volume{
Name: "tidb-client-tls", VolumeSource: corev1.VolumeSource{
Secret: &corev1.SecretVolumeSource{
Expand Down Expand Up @@ -724,6 +730,11 @@ func getPDConfigMap(tc *v1alpha1.TidbCluster) (*corev1.ConfigMap, error) {
return nil, nil
}

clusterVersionGE4, err := clusterVersionGreaterThanOrEqualTo4(tc.PDVersion())
if err != nil {
klog.Warningf("cluster version: %s is not semantic versioning compatible", tc.PDVersion())
}

// override CA if tls enabled
if tc.IsTLSClusterEnabled() {
if config.Security == nil {
Expand All @@ -733,7 +744,8 @@ func getPDConfigMap(tc *v1alpha1.TidbCluster) (*corev1.ConfigMap, error) {
config.Security.CertPath = path.Join(pdClusterCertPath, corev1.TLSCertKey)
config.Security.KeyPath = path.Join(pdClusterCertPath, corev1.TLSPrivateKeyKey)
}
if tc.Spec.TiDB.IsTLSClientEnabled() && !tc.SkipTLSWhenConnectTiDB() {
// Versions below v4.0 do not support Dashboard
if tc.Spec.TiDB.IsTLSClientEnabled() && !tc.SkipTLSWhenConnectTiDB() && clusterVersionGE4 {
if config.Dashboard == nil {
config.Dashboard = &v1alpha1.DashboardConfig{}
}
Expand Down Expand Up @@ -773,6 +785,15 @@ func getPDConfigMap(tc *v1alpha1.TidbCluster) (*corev1.ConfigMap, error) {
return cm, nil
}

func clusterVersionGreaterThanOrEqualTo4(version string) (bool, error) {
v, err := semver.NewVersion(version)
if err != nil {
return true, err
}
weekface marked this conversation as resolved.
Show resolved Hide resolved

return v.Major() >= 4, nil
}

func (pmm *pdMemberManager) collectUnjoinedMembers(tc *v1alpha1.TidbCluster, set *apps.StatefulSet, pdStatus map[string]v1alpha1.PDMember) error {
podSelector, podSelectErr := metav1.LabelSelectorAsSelector(set.Spec.Selector)
if podSelectErr != nil {
Expand Down
182 changes: 182 additions & 0 deletions pkg/manager/member/pd_member_manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1113,6 +1113,58 @@ func TestGetNewPDSetForTidbCluster(t *testing.T) {
},
}),
},
{
name: "tidb version v3.1.0, tidb client tls is enabled",
tc: v1alpha1.TidbCluster{
ObjectMeta: metav1.ObjectMeta{
Name: "tls-v3",
Namespace: "ns",
},
Spec: v1alpha1.TidbClusterSpec{
PD: v1alpha1.PDSpec{
ComponentSpec: v1alpha1.ComponentSpec{
Image: "pingcap/pd:v3.1.0",
},
},
TiDB: v1alpha1.TiDBSpec{
TLSClient: &v1alpha1.TiDBTLSClient{
Enabled: true,
},
},
},
},
testSts: func(sts *apps.StatefulSet) {
g := NewGomegaWithT(t)
g.Expect(hasTLSVol(sts)).To(BeFalse())
g.Expect(hasTLSVolMount(sts)).To(BeFalse())
},
},
{
name: "tidb version v4.0.0-rc.1, tidb client tls is enabled",
weekface marked this conversation as resolved.
Show resolved Hide resolved
tc: v1alpha1.TidbCluster{
ObjectMeta: metav1.ObjectMeta{
Name: "tls-v4",
Namespace: "ns",
},
Spec: v1alpha1.TidbClusterSpec{
PD: v1alpha1.PDSpec{
ComponentSpec: v1alpha1.ComponentSpec{
Image: "pingcap/pd:v4.0.0-rc.1",
},
},
TiDB: v1alpha1.TiDBSpec{
TLSClient: &v1alpha1.TiDBTLSClient{
Enabled: true,
},
},
},
},
testSts: func(sts *apps.StatefulSet) {
g := NewGomegaWithT(t)
g.Expect(hasTLSVol(sts)).To(BeTrue())
g.Expect(hasTLSVolMount(sts)).To(BeTrue())
},
},
// TODO add more tests
}

Expand Down Expand Up @@ -1204,6 +1256,114 @@ func TestGetPDConfigMap(t *testing.T) {
[replication]
max-replicas = 5
location-labels = ["node", "rack"]
`,
},
},
},
{
name: "tidb version v3.1.0, tidb client tls is enabled",
tc: v1alpha1.TidbCluster{
ObjectMeta: metav1.ObjectMeta{
Name: "tls-v3",
Namespace: "ns",
},
Spec: v1alpha1.TidbClusterSpec{
PD: v1alpha1.PDSpec{
ComponentSpec: v1alpha1.ComponentSpec{
Image: "pingcap/pd:v3.1.0",
},
Config: &v1alpha1.PDConfig{},
},
TiDB: v1alpha1.TiDBSpec{
TLSClient: &v1alpha1.TiDBTLSClient{
Enabled: true,
},
},
},
},
expected: &corev1.ConfigMap{
ObjectMeta: metav1.ObjectMeta{
Name: "tls-v3-pd",
Namespace: "ns",
Labels: map[string]string{
"app.kubernetes.io/name": "tidb-cluster",
"app.kubernetes.io/managed-by": "tidb-operator",
"app.kubernetes.io/instance": "tls-v3",
"app.kubernetes.io/component": "pd",
},
OwnerReferences: []metav1.OwnerReference{
{
APIVersion: "pingcap.com/v1alpha1",
Kind: "TidbCluster",
Name: "tls-v3",
UID: "",
Controller: func(b bool) *bool {
return &b
}(true),
BlockOwnerDeletion: func(b bool) *bool {
return &b
}(true),
},
},
},
Data: map[string]string{
"startup-script": "",
"config-file": "",
},
},
},
{
name: "tidb version v4.0.0-rc.1, tidb client tls is enabled",
tc: v1alpha1.TidbCluster{
ObjectMeta: metav1.ObjectMeta{
Name: "tls-v4",
Namespace: "ns",
},
Spec: v1alpha1.TidbClusterSpec{
PD: v1alpha1.PDSpec{
ComponentSpec: v1alpha1.ComponentSpec{
Image: "pingcap/pd:v4.0.0-rc.1",
},
Config: &v1alpha1.PDConfig{},
},
TiDB: v1alpha1.TiDBSpec{
TLSClient: &v1alpha1.TiDBTLSClient{
Enabled: true,
},
},
},
},
expected: &corev1.ConfigMap{
ObjectMeta: metav1.ObjectMeta{
Name: "tls-v4-pd",
weekface marked this conversation as resolved.
Show resolved Hide resolved
Namespace: "ns",
Labels: map[string]string{
"app.kubernetes.io/name": "tidb-cluster",
"app.kubernetes.io/managed-by": "tidb-operator",
"app.kubernetes.io/instance": "tls-v4",
"app.kubernetes.io/component": "pd",
},
OwnerReferences: []metav1.OwnerReference{
{
APIVersion: "pingcap.com/v1alpha1",
Kind: "TidbCluster",
Name: "tls-v4",
UID: "",
Controller: func(b bool) *bool {
return &b
}(true),
BlockOwnerDeletion: func(b bool) *bool {
return &b
}(true),
},
},
},
Data: map[string]string{
"startup-script": "",
"config-file": `[dashboard]
tidb-cacert-path = "/var/lib/tidb-client-tls/ca.crt"
tidb-cert-path = "/var/lib/tidb-client-tls/tls.crt"
tidb-key-path = "/var/lib/tidb-client-tls/tls.key"
`,
},
},
Expand Down Expand Up @@ -1883,3 +2043,25 @@ func TestPDShouldRecover(t *testing.T) {
})
}
}

func hasTLSVol(sts *apps.StatefulSet) bool {
for _, vol := range sts.Spec.Template.Spec.Volumes {
if vol.Name == "tidb-client-tls" {
return true
}
}
return false
}

func hasTLSVolMount(sts *apps.StatefulSet) bool {
for _, container := range sts.Spec.Template.Spec.Containers {
if container.Name == v1alpha1.PDMemberType.String() {
for _, vm := range container.VolumeMounts {
if vm.Name == "tidb-client-tls" {
return true
}
}
}
}
return false
}