From aa4c3ae713032c8609b1a61c3cb211b5d0cabdf8 Mon Sep 17 00:00:00 2001 From: Leon Date: Wed, 22 Jan 2025 17:33:21 +0800 Subject: [PATCH] update tls certs --- .../transformer_cluster_sharding_tls.go | 4 +- .../component/component_controller_test.go | 3 +- .../component/transformer_component_tls.go | 82 ++++++++++------ .../transformer_component_tls_test.go | 94 +------------------ pkg/controller/plan/tls.go | 45 +++------ pkg/controller/plan/tls_test.go | 60 ++++++------ 6 files changed, 100 insertions(+), 188 deletions(-) diff --git a/controllers/apps/cluster/transformer_cluster_sharding_tls.go b/controllers/apps/cluster/transformer_cluster_sharding_tls.go index 690898c572f..5b95afdbd71 100644 --- a/controllers/apps/cluster/transformer_cluster_sharding_tls.go +++ b/controllers/apps/cluster/transformer_cluster_sharding_tls.go @@ -137,7 +137,7 @@ func (t *clusterShardingTLSTransformer) buildTLSSecret(transCtx *clusterTransfor Name: sharding.Name, } secret := t.newTLSSecret(transCtx, sharding, compDef) - return plan.ComposeTLSSecret(compDef, synthesizedComp, secret) + return plan.ComposeTLSCertsWithSecret(compDef, synthesizedComp, secret) } func (t *clusterShardingTLSTransformer) newTLSSecret(transCtx *clusterTransformContext, @@ -157,7 +157,7 @@ func (t *clusterShardingTLSTransformer) newTLSSecret(transCtx *clusterTransformC AddLabelsInMap(compDef.Spec.Labels). AddAnnotationsInMap(sharding.Template.Annotations). AddAnnotationsInMap(compDef.Spec.Annotations). - SetStringData(map[string]string{}). + SetData(map[string][]byte{}). GetObject() } diff --git a/controllers/apps/component/component_controller_test.go b/controllers/apps/component/component_controller_test.go index b7152afc9df..1fb7a11d535 100644 --- a/controllers/apps/component/component_controller_test.go +++ b/controllers/apps/component/component_controller_test.go @@ -48,7 +48,6 @@ import ( "github.com/apecloud/kubeblocks/pkg/constant" "github.com/apecloud/kubeblocks/pkg/controller/builder" "github.com/apecloud/kubeblocks/pkg/controller/component" - "github.com/apecloud/kubeblocks/pkg/controller/plan" intctrlutil "github.com/apecloud/kubeblocks/pkg/controllerutil" "github.com/apecloud/kubeblocks/pkg/generics" kbacli "github.com/apecloud/kubeblocks/pkg/kbagent/client" @@ -1154,7 +1153,7 @@ var _ = Describe("Component Controller", func() { By("check TLS secret") secretKey := types.NamespacedName{ Namespace: compObj.Namespace, - Name: plan.GenerateTLSSecretName(clusterKey.Name, compName), + Name: tlsSecretName(clusterKey.Name, compName), } Eventually(testapps.CheckObj(&testCtx, secretKey, func(g Gomega, secret *corev1.Secret) { g.Expect(secret.Data).Should(HaveKey(*tls.CAFile)) diff --git a/controllers/apps/component/transformer_component_tls.go b/controllers/apps/component/transformer_component_tls.go index 2462e9220d9..83d00983b1d 100644 --- a/controllers/apps/component/transformer_component_tls.go +++ b/controllers/apps/component/transformer_component_tls.go @@ -31,6 +31,8 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" appsv1 "github.com/apecloud/kubeblocks/apis/apps/v1" + "github.com/apecloud/kubeblocks/pkg/constant" + "github.com/apecloud/kubeblocks/pkg/controller/builder" "github.com/apecloud/kubeblocks/pkg/controller/component" "github.com/apecloud/kubeblocks/pkg/controller/graph" "github.com/apecloud/kubeblocks/pkg/controller/model" @@ -97,7 +99,7 @@ func (t *componentTLSTransformer) enabled(compDef *appsv1.ComponentDefinition, if tls.Issuer == nil { return false, fmt.Errorf("the issuer shouldn't be nil when the TLS is enabled") } - if slices.Contains([]appsv1.IssuerName{appsv1.IssuerUserProvided, appsv1.IssuerKubeBlocks}, tls.Issuer.Name) { + if !slices.Contains([]appsv1.IssuerName{appsv1.IssuerUserProvided, appsv1.IssuerKubeBlocks}, tls.Issuer.Name) { return false, fmt.Errorf("unknown TLS issuer %s", tls.Issuer.Name) } if compDef.Spec.TLS == nil { @@ -110,7 +112,7 @@ func (t *componentTLSTransformer) secretObject(transCtx *componentTransformConte synthesizedComp *component.SynthesizedComponent) (*corev1.Secret, error) { secretKey := types.NamespacedName{ Namespace: synthesizedComp.Namespace, - Name: plan.GenerateTLSSecretName(synthesizedComp.ClusterName, synthesizedComp.Name), + Name: tlsSecretName(synthesizedComp.ClusterName, synthesizedComp.Name), } secret := &corev1.Secret{} err := transCtx.Client.Get(transCtx.Context, secretKey, secret) @@ -168,7 +170,7 @@ func (t *componentTLSTransformer) handleDelete(ctx context.Context, cli client.R } if secret != nil { graphCli, _ := cli.(model.GraphClient) - graphCli.Delete(dag, secret) + graphCli.Delete(dag, secret) // TODO: notify the pods } return nil } @@ -181,7 +183,7 @@ func (t *componentTLSTransformer) handleUpdate(ctx context.Context, cli client.R } if secret != nil { graphCli, _ := cli.(model.GraphClient) - graphCli.Update(dag, secretObj, secret) + graphCli.Update(dag, secretObj, secret) // TODO: notify the pods } return nil } @@ -192,7 +194,8 @@ type tlsIssuerKubeBlocks struct { } func (i *tlsIssuerKubeBlocks) create(ctx context.Context, cli client.Reader) (*corev1.Secret, error) { - return plan.ComposeTLSSecret(i.compDef, *i.synthesizedComp, nil) + proto := newTLSSecret(i.synthesizedComp) + return plan.ComposeTLSCertsWithSecret(i.compDef, *i.synthesizedComp, proto) } func (i *tlsIssuerKubeBlocks) delete(ctx context.Context, cli client.Reader, secret *corev1.Secret) (*corev1.Secret, error) { @@ -200,9 +203,9 @@ func (i *tlsIssuerKubeBlocks) delete(ctx context.Context, cli client.Reader, sec } func (i *tlsIssuerKubeBlocks) update(ctx context.Context, cli client.Reader, secret *corev1.Secret) (*corev1.Secret, error) { - proto := plan.BuildTLSSecret(*i.synthesizedComp) + proto := newTLSSecret(i.synthesizedComp) - // TODO: update secret data if needed + // For TLS certs generated by KubeBlocks, we only support updating labels and annotations. secretCopy := secret.DeepCopy() secretCopy.Labels = proto.Labels secretCopy.Annotations = proto.Annotations @@ -219,12 +222,38 @@ type tlsIssuerUserProvided struct { } func (i *tlsIssuerUserProvided) create(ctx context.Context, cli client.Reader) (*corev1.Secret, error) { + return i.proto(ctx, cli) +} + +func (i *tlsIssuerUserProvided) delete(ctx context.Context, cli client.Reader, secret *corev1.Secret) (*corev1.Secret, error) { + return secret, nil +} + +func (i *tlsIssuerUserProvided) update(ctx context.Context, cli client.Reader, secret *corev1.Secret) (*corev1.Secret, error) { + proto, err := i.proto(ctx, cli) + if err != nil { + // the referenced secret not existing should not affect the reconciliation + return nil, client.IgnoreNotFound(err) + } + + secretCopy := secret.DeepCopy() + secretCopy.Labels = proto.Labels + secretCopy.Annotations = proto.Annotations + secretCopy.Data = proto.Data + + if !reflect.DeepEqual(secret, secretCopy) { + return secretCopy, nil + } + return nil, nil +} + +func (i *tlsIssuerUserProvided) proto(ctx context.Context, cli client.Reader) (*corev1.Secret, error) { secret, err := i.referenced(ctx, cli) if err != nil { return nil, err } - proto := plan.BuildTLSSecret(*i.synthesizedComp) + proto := newTLSSecret(i.synthesizedComp) secretRef := i.synthesizedComp.TLSConfig.Issuer.SecretRef if i.compDef.Spec.TLS.CAFile != nil { @@ -240,24 +269,6 @@ func (i *tlsIssuerUserProvided) create(ctx context.Context, cli client.Reader) ( return proto, nil } -func (i *tlsIssuerUserProvided) delete(ctx context.Context, cli client.Reader, secret *corev1.Secret) (*corev1.Secret, error) { - return secret, nil -} - -func (i *tlsIssuerUserProvided) update(ctx context.Context, cli client.Reader, secret *corev1.Secret) (*corev1.Secret, error) { - proto := plan.BuildTLSSecret(*i.synthesizedComp) - - // TODO: update secret data if needed - secretCopy := secret.DeepCopy() - secretCopy.Labels = proto.Labels - secretCopy.Annotations = proto.Annotations - - if !reflect.DeepEqual(secret, secretCopy) { - return secretCopy, nil - } - return nil, nil -} - func (i *tlsIssuerUserProvided) referenced(ctx context.Context, cli client.Reader) (*corev1.Secret, error) { var ( secretRef = i.synthesizedComp.TLSConfig.Issuer.SecretRef @@ -326,7 +337,7 @@ func (t *componentTLSTransformer) composeTLSVolume(compDef *appsv1.ComponentDefi tls := synthesizedComp.TLSConfig switch tls.Issuer.Name { case appsv1.IssuerKubeBlocks: - secretName = plan.GenerateTLSSecretName(synthesizedComp.ClusterName, synthesizedComp.Name) + secretName = tlsSecretName(synthesizedComp.ClusterName, synthesizedComp.Name) ca = compDef.Spec.TLS.CAFile cert = compDef.Spec.TLS.CertFile key = compDef.Spec.TLS.KeyFile @@ -404,3 +415,20 @@ func (t *componentTLSTransformer) removeVolumeNVolumeMount(compDef *appsv1.Compo return nil } + +func tlsSecretName(clusterName, compName string) string { + return clusterName + "-" + compName + "-tls-certs" +} + +func newTLSSecret(synthesizedComp *component.SynthesizedComponent) *corev1.Secret { + secretName := tlsSecretName(synthesizedComp.ClusterName, synthesizedComp.Name) + return builder.NewSecretBuilder(synthesizedComp.Namespace, secretName). + // priority: static < dynamic < built-in + AddLabelsInMap(synthesizedComp.StaticLabels). + AddLabelsInMap(synthesizedComp.DynamicLabels). + AddLabelsInMap(constant.GetCompLabels(synthesizedComp.ClusterName, synthesizedComp.Name)). + AddAnnotationsInMap(synthesizedComp.StaticAnnotations). + AddAnnotationsInMap(synthesizedComp.DynamicAnnotations). + SetData(map[string][]byte{}). + GetObject() +} diff --git a/controllers/apps/component/transformer_component_tls_test.go b/controllers/apps/component/transformer_component_tls_test.go index 48ebb661f89..aeeb2d92fd0 100644 --- a/controllers/apps/component/transformer_component_tls_test.go +++ b/controllers/apps/component/transformer_component_tls_test.go @@ -21,33 +21,11 @@ package component import ( . "github.com/onsi/ginkgo/v2" - . "github.com/onsi/gomega" - corev1 "k8s.io/api/core/v1" - "k8s.io/utils/ptr" - "sigs.k8s.io/controller-runtime/pkg/client" - - appsv1 "github.com/apecloud/kubeblocks/apis/apps/v1" - "github.com/apecloud/kubeblocks/pkg/constant" - "github.com/apecloud/kubeblocks/pkg/controller/component" - "github.com/apecloud/kubeblocks/pkg/controller/plan" testapps "github.com/apecloud/kubeblocks/pkg/testutil/apps" ) var _ = Describe("TLS self-signed cert function", func() { - const ( - compDefName = "test-compdef" - clusterNamePrefix = "test-cluster" - defaultCompName = "mysql" - caFile = "ca.pem" - certFile = "cert.pem" - keyFile = "key.pem" - ) - - var ( - compDefObj *appsv1.ComponentDefinition - ) - cleanEnv := func() { // must wait until resources deleted and no longer exist before the testcases start, // otherwise if later it needs to create some new resource objects with the same name, @@ -62,75 +40,5 @@ var _ = Describe("TLS self-signed cert function", func() { AfterEach(cleanEnv) - Context("tls is enabled/disabled", func() { - BeforeEach(func() { - By("Create a componentDefinition obj") - compDefObj = testapps.NewComponentDefinitionFactory(compDefName). - WithRandomName(). - AddAnnotations(constant.SkipImmutableCheckAnnotationKey, "true"). - SetDefaultSpec(). - Create(&testCtx). - GetObject() - }) - - Context("when issuer is UserProvided", func() { - var ( - synthesizedComp *component.SynthesizedComponent - secretObj *corev1.Secret - ) - - BeforeEach(func() { - // prepare self provided tls certs secret - var err error - compDef := &appsv1.ComponentDefinition{ - Spec: appsv1.ComponentDefinitionSpec{ - TLS: &appsv1.TLS{ - CAFile: ptr.To(caFile), - CertFile: ptr.To(certFile), - KeyFile: ptr.To(keyFile), - }, - }, - } - synthesizedComp = &component.SynthesizedComponent{ - Namespace: testCtx.DefaultNamespace, - ClusterName: "test", - Name: "self-provided", - FullCompName: "test-self-provided", - CompDefName: compDefObj.Name, - } - secretObj, err = plan.ComposeTLSSecret(compDef, *synthesizedComp, nil) - Expect(err).Should(BeNil()) - Expect(k8sClient.Create(testCtx.Ctx, secretObj)).Should(Succeed()) - }) - - AfterEach(func() { - // delete self provided tls secret - Expect(k8sClient.Delete(testCtx.Ctx, secretObj)).Should(Succeed()) - Eventually(testapps.CheckObjExists(&testCtx, client.ObjectKeyFromObject(secretObj), secretObj, false)).Should(Succeed()) - }) - - It("should create the component when secret referenced exist", func() { - issuer := &appsv1.Issuer{ - Name: appsv1.IssuerUserProvided, - SecretRef: &appsv1.TLSSecretReference{ - SecretReference: corev1.SecretReference{ - Namespace: testCtx.DefaultNamespace, - Name: secretObj.Name, - }, - CA: caFile, - Cert: certFile, - Key: keyFile, - }, - } - By("create component obj") - compObj := testapps.NewComponentFactory(synthesizedComp.Namespace, synthesizedComp.FullCompName, synthesizedComp.CompDefName). - WithRandomName(). - SetReplicas(3). - SetTLSConfig(true, issuer). - Create(&testCtx). - GetObject() - Eventually(k8sClient.Get(testCtx.Ctx, client.ObjectKeyFromObject(compObj), compObj)).Should(Succeed()) - }) - }) - }) + // TODO: test }) diff --git a/pkg/controller/plan/tls.go b/pkg/controller/plan/tls.go index 345fb8b8857..f3f98b022ce 100644 --- a/pkg/controller/plan/tls.go +++ b/pkg/controller/plan/tls.go @@ -26,46 +26,22 @@ import ( "text/template" "github.com/Masterminds/sprig/v3" + "github.com/pkg/errors" + corev1 "k8s.io/api/core/v1" + appsv1 "github.com/apecloud/kubeblocks/apis/apps/v1" - "github.com/apecloud/kubeblocks/pkg/constant" - "github.com/apecloud/kubeblocks/pkg/controller/builder" "github.com/apecloud/kubeblocks/pkg/controller/component" - "github.com/pkg/errors" - v1 "k8s.io/api/core/v1" ) -func GenerateTLSSecretName(clusterName, componentName string) string { - return clusterName + "-" + componentName + "-tls-certs" -} - -func BuildTLSSecret(synthesizedComp component.SynthesizedComponent) *v1.Secret { - name := GenerateTLSSecretName(synthesizedComp.ClusterName, synthesizedComp.Name) - return builder.NewSecretBuilder(synthesizedComp.Namespace, name). - // Priority: static < dynamic < built-in - AddLabelsInMap(synthesizedComp.StaticLabels). - AddLabelsInMap(synthesizedComp.DynamicLabels). - AddLabelsInMap(constant.GetCompLabels(synthesizedComp.ClusterName, synthesizedComp.Name)). - AddAnnotationsInMap(synthesizedComp.StaticAnnotations). - AddAnnotationsInMap(synthesizedComp.DynamicAnnotations). - SetStringData(map[string]string{}). - SetData(map[string][]byte{}). - GetObject() -} - -// ComposeTLSSecret composes a TSL secret object. -// REVIEW/TODO: -// 1. missing public function doc -// 2. should avoid using Go template to call a function, this is too hacky & costly, -// should just call underlying registered Go template function. -func ComposeTLSSecret(compDef *appsv1.ComponentDefinition, synthesizedComp component.SynthesizedComponent, secret *v1.Secret) (*v1.Secret, error) { +func ComposeTLSCertsWithSecret(compDef *appsv1.ComponentDefinition, + synthesizedComp component.SynthesizedComponent, secret *corev1.Secret) (*corev1.Secret, error) { var ( namespace = synthesizedComp.Namespace clusterName = synthesizedComp.ClusterName compName = synthesizedComp.Name ) - if secret == nil { - secret = BuildTLSSecret(synthesizedComp) - } + + // TODO: should avoid using Go template to call a function, this is too hacky & costly, should just call underlying registered Go template function. // use ca gen cert // IP: 127.0.0.1 and ::1 // DNS: localhost and *.--headless..svc.cluster.local @@ -83,19 +59,20 @@ func ComposeTLSSecret(compDef *appsv1.ComponentDefinition, synthesizedComp compo if err != nil { return nil, err } + parts := strings.Split(out, spliter) if len(parts) != 3 { return nil, errors.Errorf("generate TLS certificates failed with cluster name %s, component name %s in namespace %s", clusterName, compName, namespace) } if compDef.Spec.TLS.CAFile != nil { - secret.StringData[*compDef.Spec.TLS.CAFile] = parts[0] + secret.Data[*compDef.Spec.TLS.CAFile] = []byte(parts[0]) } if compDef.Spec.TLS.CertFile != nil { - secret.StringData[*compDef.Spec.TLS.CertFile] = parts[1] + secret.Data[*compDef.Spec.TLS.CertFile] = []byte(parts[1]) } if compDef.Spec.TLS.KeyFile != nil { - secret.StringData[*compDef.Spec.TLS.KeyFile] = parts[2] + secret.Data[*compDef.Spec.TLS.KeyFile] = []byte(parts[2]) } return secret, nil } diff --git a/pkg/controller/plan/tls_test.go b/pkg/controller/plan/tls_test.go index 907a43767f3..907f6375bbc 100644 --- a/pkg/controller/plan/tls_test.go +++ b/pkg/controller/plan/tls_test.go @@ -23,42 +23,42 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/utils/ptr" appsv1 "github.com/apecloud/kubeblocks/apis/apps/v1" - "github.com/apecloud/kubeblocks/pkg/constant" "github.com/apecloud/kubeblocks/pkg/controller/component" ) -var _ = Describe("TLSUtilsTest", func() { - Context("ComposeTLSSecret function", func() { - It("should work well", func() { - compDef := &appsv1.ComponentDefinition{ - Spec: appsv1.ComponentDefinitionSpec{ - TLS: &appsv1.TLS{ - CAFile: ptr.To("ca.pem"), - CertFile: ptr.To("cert.pem"), - KeyFile: ptr.To("key.pem"), - }, +var _ = Describe("TLS test", func() { + It("ComposeTLSCertsWithSecret", func() { + compDef := &appsv1.ComponentDefinition{ + Spec: appsv1.ComponentDefinitionSpec{ + TLS: &appsv1.TLS{ + CAFile: ptr.To("ca.pem"), + CertFile: ptr.To("cert.pem"), + KeyFile: ptr.To("key.pem"), }, - } - synthesizedComp := component.SynthesizedComponent{ - Namespace: testCtx.DefaultNamespace, - ClusterName: "bar", - Name: "test", - } - secret, err := ComposeTLSSecret(compDef, synthesizedComp, nil) - Expect(err).Should(BeNil()) - Expect(secret).ShouldNot(BeNil()) - Expect(secret.Name).Should(Equal(GenerateTLSSecretName(synthesizedComp.ClusterName, synthesizedComp.Name))) - Expect(secret.Labels).ShouldNot(BeNil()) - Expect(secret.Labels[constant.AppInstanceLabelKey]).Should(Equal(synthesizedComp.ClusterName)) - Expect(secret.Labels[constant.AppManagedByLabelKey]).Should(Equal(constant.AppName)) - Expect(secret.Labels[constant.KBAppComponentLabelKey]).Should(Equal(synthesizedComp.Name)) - Expect(secret.StringData).ShouldNot(BeNil()) - Expect(secret.StringData[*compDef.Spec.TLS.CAFile]).ShouldNot(BeZero()) - Expect(secret.StringData[*compDef.Spec.TLS.CertFile]).ShouldNot(BeZero()) - Expect(secret.StringData[*compDef.Spec.TLS.KeyFile]).ShouldNot(BeZero()) - }) + }, + } + synthesizedComp := component.SynthesizedComponent{ + Namespace: testCtx.DefaultNamespace, + ClusterName: "foo", + Name: "bar", + } + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: testCtx.DefaultNamespace, + Name: "foo-bar-tls", + }, + Data: map[string][]byte{}, + } + _, err := ComposeTLSCertsWithSecret(compDef, synthesizedComp, secret) + Expect(err).Should(BeNil()) + Expect(secret.Data).ShouldNot(BeNil()) + Expect(secret.Data[*compDef.Spec.TLS.CAFile]).ShouldNot(BeZero()) + Expect(secret.Data[*compDef.Spec.TLS.CertFile]).ShouldNot(BeZero()) + Expect(secret.Data[*compDef.Spec.TLS.KeyFile]).ShouldNot(BeZero()) }) })