Skip to content

Commit

Permalink
chore: remove conn credential from comp definition (#6000)
Browse files Browse the repository at this point in the history
  • Loading branch information
leon-inf authored Dec 6, 2023
1 parent 8902ee7 commit bd89124
Show file tree
Hide file tree
Showing 27 changed files with 62 additions and 474 deletions.
5 changes: 0 additions & 5 deletions apis/apps/v1alpha1/clusterdefinition_webhook_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -443,11 +443,6 @@ spec:
- mountPath: /data/config/mysql
name: mysql-config
env:
- name: "MYSQL_ROOT_PASSWORD"
valueFrom:
secretKeyRef:
name: $(CONN_CREDENTIAL_SECRET_NAME)
key: password
command: ["/usr/bin/bash", "-c"]
`, name)
clusterDefinition := &ClusterDefinition{}
Expand Down
5 changes: 0 additions & 5 deletions apis/apps/v1alpha1/componentdefinition_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -173,11 +173,6 @@ type ComponentDefinitionSpec struct {
// +optional
SystemAccounts []SystemAccount `json:"systemAccounts,omitempty"`

// ConnectionCredentials defines the default connection credentials that can be used to access the component service.
// Cannot be updated.
// +optional
ConnectionCredentials []ConnectionCredential `json:"connectionCredentials,omitempty"`

// UpdateStrategy defines the strategy for updating the component instance.
// Cannot be updated.
// +kubebuilder:default=Serial
Expand Down
5 changes: 0 additions & 5 deletions apis/apps/v1alpha1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

34 changes: 0 additions & 34 deletions config/crd/bases/apps.kubeblocks.io_componentdefinitions.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -162,40 +162,6 @@ spec:
x-kubernetes-list-map-keys:
- name
x-kubernetes-list-type: map
connectionCredentials:
description: ConnectionCredentials defines the default connection
credentials that can be used to access the component service. Cannot
be updated.
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
description:
description: Description is a brief description of the component.
maxLength: 256
Expand Down
2 changes: 0 additions & 2 deletions controllers/apps/component_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -140,8 +140,6 @@ func (r *ComponentReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
&componentAccountTransformer{},
// provision component system accounts
&componentAccountProvisionTransformer{},
// handle component connection credentials
&componentCredentialTransformer{},
// handle tls volume and cert
&componentTLSTransformer{},
// handle component custom volumes
Expand Down
37 changes: 0 additions & 37 deletions controllers/apps/component_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1140,39 +1140,6 @@ var _ = Describe("Component Controller", func() {
})).Should(Succeed())
}

testCompConnCredential := func(compName, compDefName string) {
createClusterObjV2(compName, compDefObj.Name, nil)

By("check root conn credential")
serviceName := constant.GenerateComponentServiceName(clusterObj.Name, compName, "rw")
servicePort := "3306"
endpoint := fmt.Sprintf("%s:%s", serviceName, servicePort)
rootSecretKey := types.NamespacedName{
Namespace: compObj.Namespace,
Name: constant.GenerateComponentConnCredential(clusterObj.Name, compName, "root"),
}
Eventually(testapps.CheckObj(&testCtx, rootSecretKey, func(g Gomega, secret *corev1.Secret) {
g.Expect(secret.Data).Should(HaveKeyWithValue("endpoint", []byte(endpoint)))
g.Expect(secret.Data).Should(HaveKeyWithValue("host", []byte(serviceName)))
g.Expect(secret.Data).Should(HaveKeyWithValue("port", []byte(servicePort)))
g.Expect(secret.Data).Should(HaveKeyWithValue(constant.AccountNameForSecret, []byte("root")))
g.Expect(secret.Data).Should(HaveKey(constant.AccountPasswdForSecret))
})).Should(Succeed())

By("check admin conn credential")
adminSecretKey := types.NamespacedName{
Namespace: compObj.Namespace,
Name: constant.GenerateComponentConnCredential(clusterObj.Name, compName, "admin"),
}
Eventually(testapps.CheckObj(&testCtx, adminSecretKey, func(g Gomega, secret *corev1.Secret) {
g.Expect(secret.Data).Should(HaveKeyWithValue("endpoint", []byte(endpoint)))
g.Expect(secret.Data).Should(HaveKeyWithValue("host", []byte(serviceName)))
g.Expect(secret.Data).Should(HaveKeyWithValue("port", []byte(servicePort)))
g.Expect(secret.Data).Should(HaveKeyWithValue(constant.AccountNameForSecret, []byte("admin")))
g.Expect(secret.Data).Should(HaveKey(constant.AccountPasswdForSecret))
})).Should(Succeed())
}

testCompVars := func(compName, compDefName string) {
compDefKey := client.ObjectKeyFromObject(compDefObj)
Eventually(testapps.GetAndChangeObj(&testCtx, compDefKey, func(compDef *appsv1alpha1.ComponentDefinition) {
Expand Down Expand Up @@ -1979,10 +1946,6 @@ var _ = Describe("Component Controller", func() {
testCompSystemAccount(defaultCompName, compDefName)
})

It("with component conn credentials", func() {
testCompConnCredential(defaultCompName, compDefName)
})

It("with component vars", func() {
testCompVars(defaultCompName, compDefName)
})
Expand Down
74 changes: 0 additions & 74 deletions controllers/apps/componentdefinition_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,6 @@ func (r *ComponentDefinitionReconciler) validate(cli client.Client, rctx intctrl
r.validatePolicyRules,
r.validateLabels,
r.validateSystemAccounts,
r.validateConnectionCredentials,
r.validateReplicaRoles,
r.validateLifecycleActions,
r.validateComponentDefRef,
Expand Down Expand Up @@ -270,79 +269,6 @@ func (r *ComponentDefinitionReconciler) validateSystemAccounts(cli client.Client
return nil
}

func (r *ComponentDefinitionReconciler) validateConnectionCredentials(cli client.Client, rctx intctrlutil.RequestCtx,
cmpd *appsv1alpha1.ComponentDefinition) error {
if !checkUniqueItemWithValue(cmpd.Spec.ConnectionCredentials, "Name", nil) {
return fmt.Errorf("duplicate connection credential names are not allowed")
}
for _, cc := range cmpd.Spec.ConnectionCredentials {
if err := r.validateConnectionCredential(cli, rctx, cmpd, cc); err != nil {
return err
}
}
return nil
}

func (r *ComponentDefinitionReconciler) validateConnectionCredential(cli client.Client, rctx intctrlutil.RequestCtx,
cmpd *appsv1alpha1.ComponentDefinition, cc appsv1alpha1.ConnectionCredential) error {
if err := r.validateConnectionCredentialService(cmpd, cc); err != nil {
return err
}
if err := r.validateConnectionCredentialAccount(cmpd, cc); err != nil {
return err
}
return nil
}

func (r *ComponentDefinitionReconciler) validateConnectionCredentialService(cmpd *appsv1alpha1.ComponentDefinition,
cc appsv1alpha1.ConnectionCredential) error {
if len(cc.ServiceName) == 0 {
return fmt.Errorf("there is no component service name defined for connection credential: %s", cc.Name)
}
for _, svc := range cmpd.Spec.Services {
if svc.Name == cc.ServiceName {
return r.validateConnectionCredentialPort(cmpd, cc, svc.Spec.Ports)
}
}
return fmt.Errorf("there is no matched service for connection credential: %s", cc.Name)
}

func (r *ComponentDefinitionReconciler) validateConnectionCredentialPort(cmpd *appsv1alpha1.ComponentDefinition,
cc appsv1alpha1.ConnectionCredential, ports []corev1.ServicePort) error {
if len(cc.PortName) == 0 {
switch len(ports) {
case 0:
return fmt.Errorf("there is no port defined for connection credential: %s", cc.Name)
case 1:
return nil
default:
return fmt.Errorf("there are multiple ports defined, it must be specified a port for connection credential: %s", cc.Name)
}
}
for _, port := range ports {
if port.Name == cc.PortName {
return nil
}
}
return fmt.Errorf("there is no matched port for connection credential: %s", cc.Name)
}

func (r *ComponentDefinitionReconciler) validateConnectionCredentialAccount(cmpd *appsv1alpha1.ComponentDefinition,
cc appsv1alpha1.ConnectionCredential) error {
if len(cc.AccountName) == 0 {
return nil
}
if cmpd.Spec.SystemAccounts == nil {
return fmt.Errorf("there is no account defined for connection credential: %s", cc.Name)
}
for _, account := range cmpd.Spec.SystemAccounts {
if account.Name == cc.AccountName {
return nil
}
}
return fmt.Errorf("there is no matched account for connection credential: %s", cc.Name)
}

func (r *ComponentDefinitionReconciler) validateReplicaRoles(cli client.Client, reqCtx intctrlutil.RequestCtx,
cmpd *appsv1alpha1.ComponentDefinition) error {
if !checkUniqueItemWithValue(cmpd.Spec.Roles, "Name", nil) {
Expand Down
15 changes: 0 additions & 15 deletions controllers/apps/componentdefinition_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -319,21 +319,6 @@ var _ = Describe("ComponentDefinition Controller", func() {
})
})

Context("connection credential", func() {
It("ok", func() {
By("create a ComponentDefinition obj")
componentDefObj := testapps.NewComponentDefinitionFactory(componentDefName).
SetRuntime(nil).
AddService("default", "", 3306, "").
AddSystemAccount(string(appsv1alpha1.AdminAccount), false, "create user").
AddConnectionCredential("default", "default", "", string(appsv1alpha1.AdminAccount)).
SetLifecycleAction("AccountProvision", defaultActionHandler).
Create(&testCtx).GetObject()

checkObjectStatus(componentDefObj, appsv1alpha1.AvailablePhase)
})
})

Context("replica roles", func() {
It("ok", func() {
By("create a ComponentDefinition obj")
Expand Down
2 changes: 1 addition & 1 deletion controllers/apps/systemaccount_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,7 @@ func (r *SystemAccountReconciler) Reconcile(ctx context.Context, req ctrl.Reques
reqCtx.Log.V(1).Info("detected database facts", "cluster", req.NamespacedName, "accounts", detectedEngineFacts)

// replace KubeBlocks ENVs.
replaceEnvsValues(cluster.Name, compDef.SystemAccounts)
replaceEnvsValues(cluster.Name, compDef.SystemAccounts, nil)

for _, account := range compDef.SystemAccounts.Accounts {
accountID := account.Name.GetAccountID()
Expand Down
22 changes: 6 additions & 16 deletions controllers/apps/systemaccount_util.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,34 +78,24 @@ func newCustomizedEngine(execConfig *appsv1alpha1.CmdExecutorConfig, dbcluster *
}
}

func replaceEnvsValues(clusterName string, sysAccounts *appsv1alpha1.SystemAccountSpec) {
connCredentialPlaceHolderMap := componetutil.GetEnvReplacementMapForConnCredential(clusterName)
compConnCredentialPlaceHolderMap := componetutil.GetEnvReplacementMapForConnCredential(clusterName)

mergeMap := func(map1, map2 map[string]string) map[string]string {
mergedMap := make(map[string]string)
for k, v := range map1 {
mergedMap[k] = v
}
for k, v := range map2 {
mergedMap[k] = v
}
return mergedMap
func replaceEnvsValues(clusterName string, sysAccounts *appsv1alpha1.SystemAccountSpec, placeholders map[string]string) {
mergedPlaceholders := componetutil.GetEnvReplacementMapForConnCredential(clusterName)
for k, v := range placeholders {
mergedPlaceholders[k] = v
}

// replace systemAccounts.cmdExecutorConfig.env[].valueFrom.secretKeyRef.name variables
cmdConfig := sysAccounts.CmdExecutorConfig
if cmdConfig != nil {
cmdConfig.Env = componetutil.ReplaceSecretEnvVars(mergeMap(connCredentialPlaceHolderMap, compConnCredentialPlaceHolderMap), cmdConfig.Env)
cmdConfig.Env = componetutil.ReplaceSecretEnvVars(mergeMap(connCredentialPlaceHolderMap, compConnCredentialPlaceHolderMap), cmdConfig.Env)
cmdConfig.Env = componetutil.ReplaceSecretEnvVars(mergedPlaceholders, cmdConfig.Env)
}

accounts := sysAccounts.Accounts
for _, acc := range accounts {
if acc.ProvisionPolicy.Type == appsv1alpha1.ReferToExisting {
// replace systemAccounts.accounts[*].provisionPolicy.secretRef.name variables
secretRef := acc.ProvisionPolicy.SecretRef
name := componetutil.ReplaceNamedVars(mergeMap(connCredentialPlaceHolderMap, compConnCredentialPlaceHolderMap), secretRef.Name, 1, false)
name := componetutil.ReplaceNamedVars(mergedPlaceholders, secretRef.Name, 1, false)
if name != secretRef.Name {
secretRef.Name = name
}
Expand Down
15 changes: 10 additions & 5 deletions controllers/apps/systemaccount_util_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ along with this program. If not, see <http://www.gnu.org/licenses/>.
package apps

import (
"fmt"
"math/rand"
"reflect"
"strings"
Expand All @@ -29,11 +30,15 @@ import (
corev1 "k8s.io/api/core/v1"

appsv1alpha1 "github.com/apecloud/kubeblocks/apis/apps/v1alpha1"
"github.com/apecloud/kubeblocks/pkg/constant"
testapps "github.com/apecloud/kubeblocks/pkg/testutil/apps"
viper "github.com/apecloud/kubeblocks/pkg/viperx"
)

var (
secret4Referent = "secret-4-referent"
secret4ReferentPlaceholder = fmt.Sprintf("$(%s)", secret4Referent)
)

func mockSystemAccountsSpec() *appsv1alpha1.SystemAccountSpec {
var (
mysqlClientImage = "docker.io/mysql:8.0.30"
Expand Down Expand Up @@ -99,7 +104,7 @@ func mockCreateByRefSystemAccount(name appsv1alpha1.AccountName, scope appsv1alp
Scope: scope,
SecretRef: &appsv1alpha1.ProvisionSecretRef{
Namespace: testCtx.DefaultNamespace,
Name: constant.KBConnCredentialPlaceHolder,
Name: secret4ReferentPlaceholder,
},
},
}
Expand Down Expand Up @@ -170,7 +175,7 @@ func TestRenderJob(t *testing.T) {
}

accountsSetting := clusterDef.Spec.ComponentDefs[0].SystemAccounts
replaceEnvsValues(cluster.Name, accountsSetting)
replaceEnvsValues(cluster.Name, accountsSetting, map[string]string{secret4ReferentPlaceholder: secret4Referent})
cmdExecutorConfig := accountsSetting.CmdExecutorConfig

engine := newCustomizedEngine(cmdExecutorConfig, cluster, mysqlCompName)
Expand Down Expand Up @@ -243,7 +248,7 @@ func TestRenderJob(t *testing.T) {
assert.Contains(t, tolerationKeys, testDataPlaneTolerationKey)
assert.Contains(t, tolerationKeys, toleration[0].Key)
case appsv1alpha1.ReferToExisting:
assert.False(t, strings.Contains(acc.ProvisionPolicy.SecretRef.Name, constant.KBConnCredentialPlaceHolder))
assert.False(t, strings.Contains(acc.ProvisionPolicy.SecretRef.Name, secret4ReferentPlaceholder))
}
}
}
Expand Down Expand Up @@ -328,7 +333,7 @@ func TestRenderCreationStmt(t *testing.T) {
assert.NotNil(t, compDef.SystemAccounts)

accountsSetting := compDef.SystemAccounts
replaceEnvsValues(clusterName, accountsSetting)
replaceEnvsValues(clusterName, accountsSetting, nil)

compKey := componentUniqueKey{
namespace: testCtx.DefaultNamespace,
Expand Down
Loading

0 comments on commit bd89124

Please sign in to comment.