refactor: extract CharacterType from TLS configuration (#8388)

apecloud · Nov 12, 2024 · 7ec2c5c · 7ec2c5c
1 parent c3861c2
commit 7ec2c5c
Show file tree

Hide file tree

Showing 4 changed files with 42 additions and 107 deletions.
diff --git a/controllers/apps/transformer_component_tls.go b/controllers/apps/transformer_component_tls.go
@@ -23,7 +23,6 @@ import (
 	"context"
 	"fmt"
 	"reflect"
-	"strings"
 
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/errors"
@@ -71,69 +70,37 @@ func (t *componentTLSTransformer) Transform(ctx graph.TransformContext, dag *gra
 
 // a hack way to notify the configuration controller to re-render config
 func checkAndTriggerReRender(ctx context.Context, synthesizedComp component.SynthesizedComponent, cli client.Client) error {
-	cm := &corev1.ConfigMap{}
-	if len(synthesizedComp.ConfigTemplates) == 0 {
-		return nil
+	tls := synthesizedComp.TLSConfig
+	conf := &appsv1alpha1.Configuration{}
+	confKey := types.NamespacedName{Namespace: synthesizedComp.Namespace, Name: cfgcore.GenerateComponentConfigurationName(synthesizedComp.ClusterName, synthesizedComp.Name)}
+	if err := cli.Get(ctx, confKey, conf); err != nil {
+		return client.IgnoreNotFound(err)
 	}
-
-	// TODO: (good-first-issue) don't hard code the tls keyword
-	// TODO(v1.0): character-type
-	tlsKeyword := plan.GetTLSKeyWord(synthesizedComp.ServiceKind)
-	if tlsKeyword == "unsupported-character-type" {
+	// update payload for tls
+	confCopy := conf.DeepCopy()
+	// confCopy.Spec.ConfigItemDetails[0].Version = fmt.Sprint(time.Now().UnixMilli())
+	if len(confCopy.Spec.ConfigItemDetails) == 0 {
 		return nil
 	}
-
-	// we assume the database config is always the first item of configSpecs, this is true for now
-	cmName := cfgcore.GetComponentCfgName(synthesizedComp.ClusterName, synthesizedComp.Name, synthesizedComp.ConfigTemplates[0].Name)
-	if err := cli.Get(ctx, types.NamespacedName{Namespace: synthesizedComp.Namespace, Name: cmName}, cm); err != nil {
-		return client.IgnoreNotFound(err)
-	}
-
-	tlsEnabledInCM := false
-	// search all config files
-	// NODE: The check logic may have bugs and the parameters may be commented.
-	for _, configData := range cm.Data {
-		if strings.Index(configData, tlsKeyword) > 0 {
-			tlsEnabledInCM = true
-			break
-		}
+	updated, err := intctrlutil.CheckAndPatchPayload(&confCopy.Spec.ConfigItemDetails[0], constant.TLSPayload, tls)
+	if err != nil {
+		return err
 	}
-
-	tls := synthesizedComp.TLSConfig
-	if ((tls == nil || !tls.Enable) && tlsEnabledInCM) ||
-		(tls != nil && tls.Enable && !tlsEnabledInCM) {
-		// tls config changed
-		conf := &appsv1alpha1.Configuration{}
-		confKey := types.NamespacedName{Namespace: synthesizedComp.Namespace, Name: cfgcore.GenerateComponentConfigurationName(synthesizedComp.ClusterName, synthesizedComp.Name)}
-		if err := cli.Get(ctx, confKey, conf); err != nil {
-			return client.IgnoreNotFound(err)
-		}
-		// update payload for tls
-		confCopy := conf.DeepCopy()
-		// confCopy.Spec.ConfigItemDetails[0].Version = fmt.Sprint(time.Now().UnixMilli())
-		updated, err := intctrlutil.CheckAndPatchPayload(&confCopy.Spec.ConfigItemDetails[0], constant.TLSPayload, tls)
-		if err != nil {
-			return err
-		}
-		if !updated {
-			return nil
-		}
-
-		// NODE: The check logic may have bugs, the configuration requires that it can only be updated through patch
-		// bad case:
-		// thread1: fetch latest configuration(id: 1000)  // e.g cluster reconcile thread
-		// thread2: fetch latest configuration(id: 1000), // e.g reconfiguring operation
-		// thread1: update payload without submit
-		// thread2: update configuration.Spec.ConfigItemDetails[*].configFileParams[*]
-		// thread2: patch configuration(id: 1001)
-		// thread1: submit configuration
-		// result: thread2's update will be lost
-		// graphCli, _ := cli.(model.GraphClient)
-		// graphCli.Update(dag, conf, confCopy)
-		return cli.Patch(ctx, confCopy, client.MergeFrom(conf.DeepCopy()))
+	if !updated {
+		return nil
 	}
-
-	return nil
+	// NOTE: The check logic may have bugs, the configuration requires that it can only be updated through patch
+	// bad case:
+	// thread1: fetch latest configuration(id: 1000)  // e.g cluster reconcile thread
+	// thread2: fetch latest configuration(id: 1000), // e.g reconfiguring operation
+	// thread1: update payload without submit
+	// thread2: update configuration.Spec.ConfigItemDetails[*].configFileParams[*]
+	// thread2: patch configuration(id: 1001)
+	// thread1: submit configuration
+	// result: thread2's update will be lost
+	// graphCli, _ := cli.(model.GraphClient)
+	// graphCli.Update(dag, conf, confCopy)
+	return cli.Patch(ctx, confCopy, client.MergeFrom(conf.DeepCopy()))
 }
 
 func buildTLSCert(ctx context.Context, cli client.Reader, synthesizedComp component.SynthesizedComponent, dag *graph.DAG) error {

diff --git a/controllers/apps/transformer_component_tls_test.go b/controllers/apps/transformer_component_tls_test.go
@@ -21,7 +21,6 @@ package apps
 
 import (
 	"context"
-	"strings"
 
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
@@ -32,15 +31,15 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
 	appsv1 "github.com/apecloud/kubeblocks/apis/apps/v1"
+	appsv1alpha1 "github.com/apecloud/kubeblocks/apis/apps/v1alpha1"
 	appsv1beta1 "github.com/apecloud/kubeblocks/apis/apps/v1beta1"
-	cfgcore "github.com/apecloud/kubeblocks/pkg/configuration/core"
+	"github.com/apecloud/kubeblocks/pkg/configuration/core"
 	"github.com/apecloud/kubeblocks/pkg/constant"
 	"github.com/apecloud/kubeblocks/pkg/controller/component"
 	"github.com/apecloud/kubeblocks/pkg/controller/graph"
 	"github.com/apecloud/kubeblocks/pkg/controller/plan"
 	"github.com/apecloud/kubeblocks/pkg/generics"
 	testapps "github.com/apecloud/kubeblocks/pkg/testutil/apps"
-	testk8s "github.com/apecloud/kubeblocks/pkg/testutil/k8s"
 )
 
 var _ = Describe("TLS self-signed cert function", func() {
@@ -172,22 +171,22 @@ var _ = Describe("TLS self-signed cert function", func() {
 				Eventually(k8sClient.Get(ctx, clusterKey, clusterObj)).Should(Succeed())
 				Eventually(testapps.ClusterReconciled(&testCtx, clusterKey)).Should(BeTrue())
 				Eventually(testapps.GetClusterPhase(&testCtx, clusterKey)).Should(Equal(appsv1.CreatingClusterPhase))
-
-				itsList := testk8s.ListAndCheckInstanceSet(&testCtx, clusterKey)
-				its := itsList.Items[0]
-				cmName := cfgcore.GetInstanceCMName(&its, &compDefObj.Spec.Configs[0].ComponentTemplateSpec)
-				cmKey := client.ObjectKey{Namespace: its.Namespace, Name: cmName}
+				cfgKey := client.ObjectKey{
+					Name:      core.GenerateComponentConfigurationName(clusterObj.Name, defaultCompName),
+					Namespace: testCtx.DefaultNamespace,
+				}
 				hasTLSSettings := func() bool {
-					cm := &corev1.ConfigMap{}
-					Expect(k8sClient.Get(ctx, cmKey, cm)).Should(Succeed())
-					tlsKeyWord := plan.GetTLSKeyWord(serviceKind)
-					for _, cfgFile := range cm.Data {
-						index := strings.Index(cfgFile, tlsKeyWord)
-						if index >= 0 {
-							return true
-						}
+					conf := &appsv1alpha1.Configuration{}
+					Expect(k8sClient.Get(ctx, cfgKey, conf)).Should(Succeed())
+					item := &conf.Spec.ConfigItemDetails[0]
+					if item.Payload.Data == nil {
+						return false
+					}
+					payload, ok := item.Payload.Data[constant.TLSPayload]
+					if !ok || payload == nil {
+						return false
 					}
-					return false
+					return true
 				}
 
 				Eventually(hasTLSSettings).Should(BeFalse())

diff --git a/pkg/controller/plan/tls_utils.go b/pkg/controller/plan/tls_utils.go
@@ -120,16 +120,3 @@ func CheckTLSSecretRef(ctx context.Context, cli client.Reader, namespace string,
 	}
 	return nil
 }
-
-func GetTLSKeyWord(kind string) string {
-	switch strings.ToLower(kind) {
-	case "mysql":
-		return "ssl_cert"
-	case "postgresql":
-		return "ssl_cert_file"
-	case "redis":
-		return "tls-cert-file"
-	default:
-		return "unsupported-character-type"
-	}
-}
diff --git a/pkg/controller/plan/tls_utils_test.go b/pkg/controller/plan/tls_utils_test.go
@@ -119,23 +119,5 @@ var _ = Describe("TLSUtilsTest", func() {
 				}).Times(1)
 			Expect(CheckTLSSecretRef(ctx, k8sMock, namespace, secretRef)).Should(Succeed())
 		})
-
-		Context("GetTLSKeyWord function", func() {
-			It("should work well", func() {
-				suite := []struct {
-					input    string
-					expected string
-				}{
-					{input: "mysql", expected: "ssl_cert"},
-					{input: "postgresql", expected: "ssl_cert_file"},
-					{input: "redis", expected: "tls-cert-file"},
-					{input: "others", expected: "unsupported-character-type"},
-				}
-
-				for _, s := range suite {
-					Expect(GetTLSKeyWord(s.input)).Should(Equal(s.expected))
-				}
-			})
-		})
 	})
 })