From 8bda420ae4b6c860427f3761c794089a540c52cb Mon Sep 17 00:00:00 2001 From: Leon Date: Mon, 18 Dec 2023 21:22:00 +0800 Subject: [PATCH] chore: remove new cluster conn-credential API (#6098) (cherry picked from commit 794b49a4da60d86bbe13a2a5c18ddc55c480baae) --- apis/apps/v1alpha1/cluster_types.go | 17 +- apis/apps/v1alpha1/type.go | 31 --- apis/apps/v1alpha1/zz_generated.deepcopy.go | 20 -- .../bases/apps.kubeblocks.io_clusters.yaml | 34 --- controllers/apps/cluster_controller.go | 2 +- controllers/apps/cluster_controller_test.go | 7 - .../apps/transformer_cluster_credential.go | 204 +++--------------- .../crds/apps.kubeblocks.io_clusters.yaml | 34 --- pkg/constant/pattern.go | 10 +- .../component_definition_convertor.go | 8 - .../component_definition_convertor_test.go | 7 - pkg/controller/factory/builder.go | 10 - 12 files changed, 33 insertions(+), 351 deletions(-) diff --git a/apis/apps/v1alpha1/cluster_types.go b/apis/apps/v1alpha1/cluster_types.go index 60e4664ee98..085fa4aace4 100644 --- a/apis/apps/v1alpha1/cluster_types.go +++ b/apis/apps/v1alpha1/cluster_types.go @@ -72,10 +72,14 @@ type ClusterSpec struct { // +optional Services []Service `json:"services,omitempty"` - // connectionCredentials defines the credentials used to access a cluster. + // affinity is a group of affinity scheduling rules. + // +optional + Affinity *Affinity `json:"affinity,omitempty"` + + // tolerations are attached to tolerate any taint that matches the triple `key,value,effect` using the matching operator `operator`. // +kubebuilder:pruning:PreserveUnknownFields // +optional - ConnectionCredentials []ConnectionCredential `json:"connectionCredentials,omitempty"` + Tolerations []corev1.Toleration `json:"tolerations,omitempty"` // tenancy describes how pods are distributed across node. // SharedNode means multiple pods may share the same node. @@ -87,15 +91,6 @@ type ClusterSpec struct { // +optional AvailabilityPolicy AvailabilityPolicyType `json:"availabilityPolicy,omitempty"` - // affinity is a group of affinity scheduling rules. - // +optional - Affinity *Affinity `json:"affinity,omitempty"` - - // tolerations are attached to tolerate any taint that matches the triple `key,value,effect` using the matching operator `operator`. - // +kubebuilder:pruning:PreserveUnknownFields - // +optional - Tolerations []corev1.Toleration `json:"tolerations,omitempty"` - // replicas specifies the replicas of the first componentSpec, if the replicas of the first componentSpec is specified, this value will be ignored. // +optional Replicas *int32 `json:"replicas,omitempty"` diff --git a/apis/apps/v1alpha1/type.go b/apis/apps/v1alpha1/type.go index 21e4733efbd..9def65488a8 100644 --- a/apis/apps/v1alpha1/type.go +++ b/apis/apps/v1alpha1/type.go @@ -622,37 +622,6 @@ type Service struct { RoleSelector string `json:"roleSelector,omitempty"` } -type ConnectionCredential struct { - // The name of the ConnectionCredential. - // Cannot be updated. - // +required - Name string `json:"name"` - - // ServiceName specifies the name of service to use for accessing. - // Cannot be updated. - // +optional - ServiceName string `json:"serviceName,omitempty"` - - // PortName specifies the name of the port to access the service. - // If the service has multiple ports, a specific port must be specified to use here. - // Otherwise, the unique port of the service will be used. - // Cannot be updated. - // +optional - PortName string `json:"portName,omitempty"` - - // ComponentName specifies the name of component where the account defined. - // For cluster-level connection credential, this field is required. - // Cannot be updated. - // +optional - ComponentName string `json:"componentName,omitempty"` - - // AccountName specifies the name of account used to access the service. - // If specified, the account must be defined in @SystemAccounts. - // Cannot be updated. - // +optional - AccountName string `json:"accountName,omitempty"` -} - // List of all the built-in variables provided by KubeBlocks. // These variables are automatically available when building environment variables for Pods and Actions, as well as // rendering templates for config and script. Users can directly use these variables without explicit declaration. diff --git a/apis/apps/v1alpha1/zz_generated.deepcopy.go b/apis/apps/v1alpha1/zz_generated.deepcopy.go index a145cce6045..5193080ad12 100644 --- a/apis/apps/v1alpha1/zz_generated.deepcopy.go +++ b/apis/apps/v1alpha1/zz_generated.deepcopy.go @@ -1043,11 +1043,6 @@ func (in *ClusterSpec) DeepCopyInto(out *ClusterSpec) { (*in)[i].DeepCopyInto(&(*out)[i]) } } - if in.ConnectionCredentials != nil { - in, out := &in.ConnectionCredentials, &out.ConnectionCredentials - *out = make([]ConnectionCredential, len(*in)) - copy(*out, *in) - } if in.Affinity != nil { in, out := &in.Affinity, &out.Affinity *out = new(Affinity) @@ -2684,21 +2679,6 @@ func (in *ConfigurationStatus) DeepCopy() *ConfigurationStatus { return out } -// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *ConnectionCredential) DeepCopyInto(out *ConnectionCredential) { - *out = *in -} - -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ConnectionCredential. -func (in *ConnectionCredential) DeepCopy() *ConnectionCredential { - if in == nil { - return nil - } - out := new(ConnectionCredential) - in.DeepCopyInto(out) - return out -} - // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ConnectionCredentialAuth) DeepCopyInto(out *ConnectionCredentialAuth) { *out = *in diff --git a/config/crd/bases/apps.kubeblocks.io_clusters.yaml b/config/crd/bases/apps.kubeblocks.io_clusters.yaml index 97cf78cff99..78d7598ccfb 100644 --- a/config/crd/bases/apps.kubeblocks.io_clusters.yaml +++ b/config/crd/bases/apps.kubeblocks.io_clusters.yaml @@ -898,40 +898,6 @@ spec: x-kubernetes-validations: - message: duplicated component rule: self.all(x, size(self.filter(c, c.name == x.name)) == 1) - connectionCredentials: - description: connectionCredentials defines the credentials used to - access a cluster. - items: - properties: - accountName: - description: AccountName specifies the name of account used - to access the service. If specified, the account must be defined - in @SystemAccounts. Cannot be updated. - type: string - componentName: - description: ComponentName specifies the name of component where - the account defined. For cluster-level connection credential, - this field is required. Cannot be updated. - type: string - name: - description: The name of the ConnectionCredential. Cannot be - updated. - type: string - portName: - description: PortName specifies the name of the port to access - the service. If the service has multiple ports, a specific - port must be specified to use here. Otherwise, the unique - port of the service will be used. Cannot be updated. - type: string - serviceName: - description: ServiceName specifies the name of service to use - for accessing. Cannot be updated. - type: string - required: - - name - type: object - type: array - x-kubernetes-preserve-unknown-fields: true monitor: description: monitor specifies the configuration of monitor properties: diff --git a/controllers/apps/cluster_controller.go b/controllers/apps/cluster_controller.go index 7077c42205f..540f6e793b5 100644 --- a/controllers/apps/cluster_controller.go +++ b/controllers/apps/cluster_controller.go @@ -147,7 +147,7 @@ func (r *ClusterReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct // update cluster components' status &clusterComponentStatusTransformer{}, // create default cluster connection credential secret object - &clusterCredentialTransformer{}, + &clusterConnCredentialTransformer{}, // build backuppolicy and backupschedule from backupPolicyTemplate &clusterBackupPolicyTransformer{}, // add our finalizer to all objects diff --git a/controllers/apps/cluster_controller_test.go b/controllers/apps/cluster_controller_test.go index 7492695d38d..c7128b5348b 100644 --- a/controllers/apps/cluster_controller_test.go +++ b/controllers/apps/cluster_controller_test.go @@ -319,9 +319,6 @@ var _ = Describe("Cluster Controller", func() { })).Should(Succeed()) } - testClusterCredential := func(compName, compDefName string, createObj func(string, string, func(*testapps.MockClusterFactory))) { - } - testClusterAffinityNToleration := func(compName, compDefName string, createObj func(string, string, func(*testapps.MockClusterFactory))) { const ( clusterTopologyKey = "testClusterTopologyKey" @@ -752,10 +749,6 @@ var _ = Describe("Cluster Controller", func() { testClusterService(consensusCompName, compDefName, createObj) }) - It("with cluster credential set", func() { - testClusterCredential(consensusCompName, compDefName, createObj) - }) - It("with cluster affinity and toleration set", func() { testClusterAffinityNToleration(consensusCompName, compDefName, createObj) }) diff --git a/controllers/apps/transformer_cluster_credential.go b/controllers/apps/transformer_cluster_credential.go index 3f1acd9b322..9d25330252b 100644 --- a/controllers/apps/transformer_cluster_credential.go +++ b/controllers/apps/transformer_cluster_credential.go @@ -20,40 +20,34 @@ along with this program. If not, see . package apps import ( - "fmt" - "reflect" - - "golang.org/x/exp/maps" corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" - "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" - appsv1alpha1 "github.com/apecloud/kubeblocks/apis/apps/v1alpha1" - "github.com/apecloud/kubeblocks/pkg/constant" "github.com/apecloud/kubeblocks/pkg/controller/component" "github.com/apecloud/kubeblocks/pkg/controller/factory" "github.com/apecloud/kubeblocks/pkg/controller/graph" "github.com/apecloud/kubeblocks/pkg/controller/model" ) -// clusterCredentialTransformer creates the connection credential secret -type clusterCredentialTransformer struct{} +// clusterCredentialTransformer creates the default cluster connection credential secret +type clusterConnCredentialTransformer struct{} -var _ graph.Transformer = &clusterCredentialTransformer{} +var _ graph.Transformer = &clusterConnCredentialTransformer{} -func (t *clusterCredentialTransformer) Transform(ctx graph.TransformContext, dag *graph.DAG) error { +func (t *clusterConnCredentialTransformer) Transform(ctx graph.TransformContext, dag *graph.DAG) error { transCtx, _ := ctx.(*clusterTransformContext) if model.IsObjectDeleting(transCtx.OrigCluster) { return nil } - if t.isLegacyCluster(transCtx) { - return t.transformClusterCredentialLegacy(transCtx, dag) + if !t.isLegacyCluster(transCtx) { + return nil } - return t.transformClusterCredential(transCtx, dag) + return t.buildClusterConnCredential(transCtx, dag) } -func (t *clusterCredentialTransformer) isLegacyCluster(transCtx *clusterTransformContext) bool { +func (t *clusterConnCredentialTransformer) isLegacyCluster(transCtx *clusterTransformContext) bool { for _, comp := range transCtx.ComponentSpecs { compDef, ok := transCtx.ComponentDefs[comp.ComponentDef] if ok && (len(compDef.UID) > 0 || !compDef.CreationTimestamp.IsZero()) { @@ -63,19 +57,27 @@ func (t *clusterCredentialTransformer) isLegacyCluster(transCtx *clusterTransfor return true } -func (t *clusterCredentialTransformer) transformClusterCredentialLegacy(transCtx *clusterTransformContext, dag *graph.DAG) error { +func (t *clusterConnCredentialTransformer) buildClusterConnCredential(transCtx *clusterTransformContext, dag *graph.DAG) error { graphCli, _ := transCtx.Client.(model.GraphClient) - synthesizedComponent := t.buildSynthesizedComponentLegacy(transCtx) - if synthesizedComponent != nil { - secret := factory.BuildConnCredential(transCtx.ClusterDef, transCtx.Cluster, synthesizedComponent) - if secret != nil { - graphCli.Create(dag, secret) - } + synthesizedComponent := t.buildSynthesizedComponent(transCtx) + if synthesizedComponent == nil { + return nil + } + secret := factory.BuildConnCredential(transCtx.ClusterDef, transCtx.Cluster, synthesizedComponent) + if secret == nil { + return nil + } + err := transCtx.Client.Get(transCtx.Context, client.ObjectKeyFromObject(secret), &corev1.Secret{}) + if err != nil && !apierrors.IsNotFound(err) { + return err + } + if apierrors.IsNotFound(err) { + graphCli.Create(dag, secret) } return nil } -func (t *clusterCredentialTransformer) buildSynthesizedComponentLegacy(transCtx *clusterTransformContext) *component.SynthesizedComponent { +func (t *clusterConnCredentialTransformer) buildSynthesizedComponent(transCtx *clusterTransformContext) *component.SynthesizedComponent { for _, compDef := range transCtx.ClusterDef.Spec.ComponentDefs { if compDef.Service == nil { continue @@ -92,159 +94,3 @@ func (t *clusterCredentialTransformer) buildSynthesizedComponentLegacy(transCtx } return nil } - -func (t *clusterCredentialTransformer) transformClusterCredential(transCtx *clusterTransformContext, dag *graph.DAG) error { - graphCli, _ := transCtx.Client.(model.GraphClient) - for _, credential := range transCtx.Cluster.Spec.ConnectionCredentials { - secret, err := t.buildClusterCredential(transCtx, dag, credential) - if err != nil { - return err - } - if err = createOrUpdateCredentialSecret(transCtx, dag, graphCli, secret); err != nil { - return err - } - } - return nil -} - -func (t *clusterCredentialTransformer) buildClusterCredential(transCtx *clusterTransformContext, dag *graph.DAG, - credential appsv1alpha1.ConnectionCredential) (*corev1.Secret, error) { - var ( - cluster = transCtx.Cluster - compDef *appsv1alpha1.ComponentDefinition - ) - - if len(credential.ComponentName) > 0 { - for _, compSpec := range transCtx.ComponentSpecs { - if compSpec.Name != credential.ComponentName { - compDef = transCtx.ComponentDefs[compSpec.ComponentDef] - break - } - } - } - - data := make(map[string][]byte) - if len(credential.ServiceName) > 0 { - if err := t.buildCredentialEndpoint(cluster, compDef, credential, &data); err != nil { - return nil, err - } - } - if len(credential.ComponentName) > 0 && len(credential.AccountName) > 0 { - if err := buildCredentialAccountFromSecret(transCtx, dag, cluster.Namespace, cluster.Name, - credential.ComponentName, credential.AccountName, &data); err != nil { - return nil, err - } - } - - secret := factory.BuildConnCredential4Cluster(cluster, credential.Name, data) - return secret, nil -} - -func (t *clusterCredentialTransformer) buildCredentialEndpoint(cluster *appsv1alpha1.Cluster, - compDef *appsv1alpha1.ComponentDefinition, credential appsv1alpha1.ConnectionCredential, data *map[string][]byte) error { - serviceName, ports := t.lookupMatchedService(cluster, compDef, credential) - if len(serviceName) == 0 { - return fmt.Errorf("cluster credential references a service which is not definied: %s-%s", cluster.Name, credential.Name) - } - if len(ports) == 0 { - return fmt.Errorf("cluster credential references a service which doesn't define any ports: %s-%s", cluster.Name, credential.Name) - } - if len(credential.PortName) == 0 && len(ports) > 1 { - return fmt.Errorf("cluster credential should specify which port to use for the referenced service: %s-%s", cluster.Name, credential.Name) - } - - buildCredentialEndpointFromService(credential, serviceName, ports, data) - return nil -} - -func (t *clusterCredentialTransformer) lookupMatchedService(cluster *appsv1alpha1.Cluster, - compDef *appsv1alpha1.ComponentDefinition, credential appsv1alpha1.ConnectionCredential) (string, []corev1.ServicePort) { - for i, svc := range cluster.Spec.Services { - if svc.Name == credential.ServiceName { - serviceName := constant.GenerateClusterServiceName(cluster.Name, svc.ServiceName) - return serviceName, cluster.Spec.Services[i].Spec.Ports - } - } - if len(credential.ComponentName) > 0 && compDef != nil { - for i, svc := range compDef.Spec.Services { - if svc.Name == credential.ServiceName { - serviceName := constant.GenerateComponentServiceName(cluster.Name, credential.ComponentName, svc.ServiceName) - return serviceName, compDef.Spec.Services[i].Spec.Ports - } - } - } - return "", nil -} - -func buildCredentialEndpointFromService(credential appsv1alpha1.ConnectionCredential, - serviceName string, ports []corev1.ServicePort, data *map[string][]byte) { - port := int32(0) - if len(credential.PortName) == 0 { - port = ports[0].Port - } else { - for _, servicePort := range ports { - if servicePort.Name == credential.PortName { - port = servicePort.Port - break - } - } - } - // TODO(component): define the service and port pattern - (*data)["endpoint"] = []byte(fmt.Sprintf("%s:%d", serviceName, port)) - (*data)["host"] = []byte(serviceName) - (*data)["port"] = []byte(fmt.Sprintf("%d", port)) -} - -func buildCredentialAccountFromSecret(ctx graph.TransformContext, dag *graph.DAG, - namespace, clusterName, compName, accountName string, data *map[string][]byte) error { - secretKey := types.NamespacedName{ - Namespace: namespace, - Name: constant.GenerateAccountSecretName(clusterName, compName, accountName), - } - secret := &corev1.Secret{} - if err := ctx.GetClient().Get(ctx.GetContext(), secretKey, secret); err != nil { - if !apierrors.IsNotFound(err) { - return err - } - if secret, err = getAccountSecretFromLocalCache(ctx, dag, secretKey); err != nil { - return err - } - } - maps.Copy(*data, secret.Data) - return nil -} - -func getAccountSecretFromLocalCache(ctx graph.TransformContext, dag *graph.DAG, secretKey types.NamespacedName) (*corev1.Secret, error) { - graphCli, _ := ctx.GetClient().(model.GraphClient) - secrets := graphCli.FindAll(dag, &corev1.Secret{}) - for i, obj := range secrets { - if obj.GetNamespace() == secretKey.Namespace && obj.GetName() == secretKey.Name { - return secrets[i].(*corev1.Secret), nil - } - } - return nil, fmt.Errorf("the account secret referenced is not found: %s", secretKey.String()) -} - -func createOrUpdateCredentialSecret(ctx graph.TransformContext, dag *graph.DAG, graphCli model.GraphClient, secret *corev1.Secret) error { - key := types.NamespacedName{ - Namespace: secret.Namespace, - Name: secret.Name, - } - obj := &corev1.Secret{} - if err := ctx.GetClient().Get(ctx.GetContext(), key, obj); err != nil { - if apierrors.IsNotFound(err) { - graphCli.Create(dag, secret) - return nil - } - return err - } - objCopy := obj.DeepCopy() - objCopy.Immutable = secret.Immutable - objCopy.Data = secret.Data - objCopy.StringData = secret.StringData - objCopy.Type = secret.Type - if !reflect.DeepEqual(obj, objCopy) { - graphCli.Update(dag, obj, objCopy) - } - return nil -} diff --git a/deploy/helm/crds/apps.kubeblocks.io_clusters.yaml b/deploy/helm/crds/apps.kubeblocks.io_clusters.yaml index 97cf78cff99..78d7598ccfb 100644 --- a/deploy/helm/crds/apps.kubeblocks.io_clusters.yaml +++ b/deploy/helm/crds/apps.kubeblocks.io_clusters.yaml @@ -898,40 +898,6 @@ spec: x-kubernetes-validations: - message: duplicated component rule: self.all(x, size(self.filter(c, c.name == x.name)) == 1) - connectionCredentials: - description: connectionCredentials defines the credentials used to - access a cluster. - items: - properties: - accountName: - description: AccountName specifies the name of account used - to access the service. If specified, the account must be defined - in @SystemAccounts. Cannot be updated. - type: string - componentName: - description: ComponentName specifies the name of component where - the account defined. For cluster-level connection credential, - this field is required. Cannot be updated. - type: string - name: - description: The name of the ConnectionCredential. Cannot be - updated. - type: string - portName: - description: PortName specifies the name of the port to access - the service. If the service has multiple ports, a specific - port must be specified to use here. Otherwise, the unique - port of the service will be used. Cannot be updated. - type: string - serviceName: - description: ServiceName specifies the name of service to use - for accessing. Cannot be updated. - type: string - required: - - name - type: object - type: array - x-kubernetes-preserve-unknown-fields: true monitor: description: monitor specifies the configuration of monitor properties: diff --git a/pkg/constant/pattern.go b/pkg/constant/pattern.go index a495e5b32bd..8e8ed7d0681 100644 --- a/pkg/constant/pattern.go +++ b/pkg/constant/pattern.go @@ -78,15 +78,7 @@ func GenerateDefaultComponentHeadlessServiceName(clusterName, compName string) s // GenerateDefaultConnCredential generates the default connection credential name for cluster. // TODO: deprecated, will be removed later. func GenerateDefaultConnCredential(clusterName string) string { - return GenerateClusterConnCredential(clusterName, "") -} - -// GenerateClusterConnCredential generates the connection credential name for cluster. -func GenerateClusterConnCredential(clusterName, name string) string { - if len(name) == 0 { - name = "conn-credential" - } - return fmt.Sprintf("%s-%s", clusterName, name) + return fmt.Sprintf("%s-conn-credential", clusterName) } // GenerateClusterComponentEnvPattern generates cluster and component pattern diff --git a/pkg/controller/component/component_definition_convertor.go b/pkg/controller/component/component_definition_convertor.go index ab0680da372..cff76258748 100644 --- a/pkg/controller/component/component_definition_convertor.go +++ b/pkg/controller/component/component_definition_convertor.go @@ -56,7 +56,6 @@ func buildComponentDefinitionByConversion(clusterCompDef *appsv1alpha1.ClusterCo "labels": &compDefLabelsConvertor{}, "replicasLimit": &compDefReplicasLimitConvertor{}, "systemaccounts": &compDefSystemAccountsConvertor{}, - "connectioncredentials": &compDefConnCredentialsConvertor{}, "updatestrategy": &compDefUpdateStrategyConvertor{}, "roles": &compDefRolesConvertor{}, "rolearbitrator": &compDefRoleArbitratorConvertor{}, @@ -324,13 +323,6 @@ func (c *compDefSystemAccountsConvertor) convert(args ...any) (any, error) { return accounts, nil } -// compDefConnCredentialsConvertor is an implementation of the convertor interface, used to convert the given object into ComponentDefinition.Spec.ConnectionCredentials. -type compDefConnCredentialsConvertor struct{} - -func (c *compDefConnCredentialsConvertor) convert(args ...any) (any, error) { - return nil, nil -} - // compDefUpdateStrategyConvertor is an implementation of the convertor interface, used to convert the given object into ComponentDefinition.Spec.UpdateStrategy. type compDefUpdateStrategyConvertor struct{} diff --git a/pkg/controller/component/component_definition_convertor_test.go b/pkg/controller/component/component_definition_convertor_test.go index 67a9c38c15d..6fda5a4a749 100644 --- a/pkg/controller/component/component_definition_convertor_test.go +++ b/pkg/controller/component/component_definition_convertor_test.go @@ -591,13 +591,6 @@ var _ = Describe("Component Definition Convertor", func() { }) }) - It("connection credentials", func() { - convertor := &compDefConnCredentialsConvertor{} - res, err := convertor.convert(clusterCompDef) - Expect(err).Should(Succeed()) - Expect(res).Should(BeNil()) - }) - Context("update strategy", func() { It("w/o workload spec", func() { clusterCompDef.ConsensusSpec = nil diff --git a/pkg/controller/factory/builder.go b/pkg/controller/factory/builder.go index b3674f59c57..90696aca2a1 100644 --- a/pkg/controller/factory/builder.go +++ b/pkg/controller/factory/builder.go @@ -276,16 +276,6 @@ func BuildConnCredential(clusterDefinition *appsv1alpha1.ClusterDefinition, clus return connCredential } -func BuildConnCredential4Cluster(cluster *appsv1alpha1.Cluster, name string, data map[string][]byte) *corev1.Secret { - secretName := constant.GenerateClusterConnCredential(cluster.Name, name) - labels := constant.GetClusterWellKnownLabels(cluster.Name) - return builder.NewSecretBuilder(cluster.Namespace, secretName). - AddLabelsInMap(labels). - SetData(data). - SetImmutable(true). - GetObject() -} - func BuildPDB(synthesizedComp *component.SynthesizedComponent) *policyv1.PodDisruptionBudget { var ( namespace = synthesizedComp.Namespace