Skip to content

Commit 00dc1b7

Browse files
leon-apeleon-ape
authored
chore: validate PD config template references (#10390)
1 parent 19e9f14 commit 00dc1b7

3 files changed

Lines changed: 243 additions & 38 deletions

File tree

‎controllers/parameters/parametersdefinition_controller.go‎

Lines changed: 118 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -23,20 +23,22 @@ import (
2323
"context"
2424
"fmt"
2525
"reflect"
26-
"time"
2726

28-
"github.com/pkg/errors"
2927
corev1 "k8s.io/api/core/v1"
3028
"k8s.io/apimachinery/pkg/runtime"
29+
"k8s.io/apimachinery/pkg/types"
3130
"k8s.io/client-go/tools/record"
3231
ctrl "sigs.k8s.io/controller-runtime"
3332
"sigs.k8s.io/controller-runtime/pkg/client"
33+
"sigs.k8s.io/controller-runtime/pkg/handler"
3434
"sigs.k8s.io/controller-runtime/pkg/log"
35+
"sigs.k8s.io/controller-runtime/pkg/reconcile"
3536

37+
appsv1 "github.com/apecloud/kubeblocks/apis/apps/v1"
3638
parametersv1alpha1 "github.com/apecloud/kubeblocks/apis/parameters/v1alpha1"
3739
"github.com/apecloud/kubeblocks/pkg/constant"
3840
intctrlutil "github.com/apecloud/kubeblocks/pkg/controllerutil"
39-
"github.com/apecloud/kubeblocks/pkg/parameters"
41+
paramutil "github.com/apecloud/kubeblocks/pkg/parameters"
4042
cfgcore "github.com/apecloud/kubeblocks/pkg/parameters/core"
4143
"github.com/apecloud/kubeblocks/pkg/parameters/openapi"
4244
"github.com/apecloud/kubeblocks/pkg/parameters/validate"
@@ -84,31 +86,129 @@ func (r *ParametersDefinitionReconciler) Reconcile(ctx context.Context, req ctrl
8486
func (r *ParametersDefinitionReconciler) SetupWithManager(mgr ctrl.Manager) error {
8587
return intctrlutil.NewControllerManagedBy(mgr).
8688
For(&parametersv1alpha1.ParametersDefinition{}).
89+
Watches(
90+
&appsv1.ComponentDefinition{},
91+
handler.EnqueueRequestsFromMapFunc(r.mapCmpdToPDs),
92+
).
8793
Complete(r)
8894
}
8995

9096
func (r *ParametersDefinitionReconciler) reconcile(reqCtx intctrlutil.RequestCtx, parametersDef *parametersv1alpha1.ParametersDefinition) (ctrl.Result, error) {
9197

92-
if parameters.ParametersDefinitionTerminalPhases(parametersDef.Status, parametersDef.Generation) {
93-
return intctrlutil.Reconciled()
94-
}
95-
96-
if ok, err := checkParametersSchema(reqCtx, parametersDef); !ok || err != nil {
97-
return intctrlutil.RequeueAfter(time.Second, reqCtx.Log, "ValidateConfigurationTemplate")
98+
if err := r.validate(reqCtx, parametersDef); err != nil {
99+
return r.failed(reqCtx, parametersDef, err)
98100
}
99101

100102
// Automatically convert cue to openAPISchema.
101103
if err := updateParametersSchema(parametersDef, r.Client, reqCtx.Ctx); err != nil {
102-
return intctrlutil.CheckedRequeueWithError(err, reqCtx.Log, errors.Wrap(err, "failed to generate openAPISchema").Error())
104+
return r.failed(reqCtx, parametersDef, err)
103105
}
104106

105-
if err := updateParamDefinitionStatus(r.Client, reqCtx, parametersDef, parametersv1alpha1.PDAvailablePhase); err != nil {
107+
phaseChanged, err := r.status(reqCtx, parametersDef, parametersv1alpha1.PDAvailablePhase)
108+
if err != nil {
106109
return intctrlutil.CheckedRequeueWithError(err, reqCtx.Log, "")
107110
}
108-
intctrlutil.RecordCreatedEvent(r.Recorder, parametersDef)
111+
if phaseChanged {
112+
intctrlutil.RecordCreatedEvent(r.Recorder, parametersDef)
113+
}
109114
return intctrlutil.Reconciled()
110115
}
111116

117+
func (r *ParametersDefinitionReconciler) validate(reqCtx intctrlutil.RequestCtx, parametersDef *parametersv1alpha1.ParametersDefinition) error {
118+
if err := validateSchema(parametersDef); err != nil {
119+
return err
120+
}
121+
if err := r.validateTemplateName(reqCtx.Ctx, parametersDef); err != nil {
122+
return err
123+
}
124+
return nil
125+
}
126+
127+
func (r *ParametersDefinitionReconciler) failed(reqCtx intctrlutil.RequestCtx, parametersDef *parametersv1alpha1.ParametersDefinition, err error) (ctrl.Result, error) {
128+
if _, err1 := r.status(reqCtx, parametersDef, parametersv1alpha1.PDUnavailablePhase); err1 != nil {
129+
return intctrlutil.CheckedRequeueWithError(err1, reqCtx.Log, "")
130+
}
131+
return intctrlutil.CheckedRequeueWithError(err, reqCtx.Log, "")
132+
}
133+
134+
func (r *ParametersDefinitionReconciler) status(reqCtx intctrlutil.RequestCtx, parametersDef *parametersv1alpha1.ParametersDefinition, phase parametersv1alpha1.ParametersDescPhase) (bool, error) {
135+
base := parametersDef.DeepCopy()
136+
patch := client.MergeFrom(base)
137+
phaseChanged := parametersDef.Status.Phase != phase
138+
parametersDef.Status.Phase = phase
139+
parametersDef.Status.ObservedGeneration = parametersDef.Generation
140+
if reflect.DeepEqual(parametersDef.Status, base.Status) {
141+
return false, nil
142+
}
143+
return phaseChanged, r.Client.Status().Patch(reqCtx.Ctx, parametersDef, patch)
144+
}
145+
146+
func (r *ParametersDefinitionReconciler) mapCmpdToPDs(ctx context.Context, obj client.Object) []reconcile.Request {
147+
cmpd, ok := obj.(*appsv1.ComponentDefinition)
148+
if !ok {
149+
return nil
150+
}
151+
paramsDefList := &parametersv1alpha1.ParametersDefinitionList{}
152+
if err := r.Client.List(ctx, paramsDefList); err != nil {
153+
log.FromContext(ctx).WithName("ParametersDefinitionReconcile").Error(err,
154+
"failed to list ParametersDefinitions for ComponentDefinition watch", "ComponentDefinition", cmpd.Name)
155+
return nil
156+
}
157+
requests := make([]reconcile.Request, 0, len(paramsDefList.Items))
158+
for i := range paramsDefList.Items {
159+
paramsDef := &paramsDefList.Items[i]
160+
matched, err := paramutil.MatchParametersDefinition(cmpd, paramsDef)
161+
if err != nil {
162+
log.FromContext(ctx).WithName("ParametersDefinitionReconcile").Error(err,
163+
"failed to match ParametersDefinition for ComponentDefinition watch",
164+
"ParametersDefinition", paramsDef.Name,
165+
"ComponentDefinition", cmpd.Name)
166+
continue
167+
}
168+
if !matched {
169+
continue
170+
}
171+
requests = append(requests, reconcile.Request{
172+
NamespacedName: types.NamespacedName{Name: paramsDef.Name},
173+
})
174+
}
175+
return requests
176+
}
177+
178+
func (r *ParametersDefinitionReconciler) validateTemplateName(ctx context.Context, parametersDef *parametersv1alpha1.ParametersDefinition) error {
179+
if parametersDef.Spec.ComponentDef == "" || parametersDef.Spec.TemplateName == "" {
180+
return nil
181+
}
182+
cmpdList := &appsv1.ComponentDefinitionList{}
183+
if err := r.Client.List(ctx, cmpdList); err != nil {
184+
return err
185+
}
186+
for i := range cmpdList.Items {
187+
cmpd := &cmpdList.Items[i]
188+
matched, err := paramutil.MatchParametersDefinition(cmpd, parametersDef)
189+
if err != nil {
190+
return err
191+
}
192+
if !matched {
193+
continue
194+
}
195+
if !hasConfigTemplate(cmpd, parametersDef.Spec.TemplateName) {
196+
return fmt.Errorf("parametersdefinition[%s] references config template[%s], but matched ComponentDefinition[%s] does not define it",
197+
parametersDef.Name, parametersDef.Spec.TemplateName, cmpd.Name)
198+
}
199+
}
200+
return nil
201+
}
202+
203+
func hasConfigTemplate(cmpd *appsv1.ComponentDefinition, templateName string) bool {
204+
for _, config := range cmpd.Spec.Configs {
205+
if config.Name == templateName {
206+
return true
207+
}
208+
}
209+
return false
210+
}
211+
112212
func (r *ParametersDefinitionReconciler) deletionHandler(parametersDef *parametersv1alpha1.ParametersDefinition, reqCtx intctrlutil.RequestCtx) func() (*ctrl.Result, error) {
113213
recordEvent := func() {
114214
r.Recorder.Event(parametersDef, corev1.EventTypeWarning, "ExistsReferencedResources",
@@ -117,7 +217,7 @@ func (r *ParametersDefinitionReconciler) deletionHandler(parametersDef *paramete
117217

118218
return func() (*ctrl.Result, error) {
119219
if parametersDef.Status.Phase != parametersv1alpha1.PDDeletingPhase {
120-
err := updateParamDefinitionStatus(r.Client, reqCtx, parametersDef, parametersv1alpha1.PDDeletingPhase)
220+
_, err := r.status(reqCtx, parametersDef, parametersv1alpha1.PDDeletingPhase)
121221
if err != nil {
122222
return nil, err
123223
}
@@ -131,30 +231,12 @@ func (r *ParametersDefinitionReconciler) deletionHandler(parametersDef *paramete
131231
}
132232
}
133233

134-
func updateParamDefinitionStatus(cli client.Client, ctx intctrlutil.RequestCtx, parametersDef *parametersv1alpha1.ParametersDefinition, phase parametersv1alpha1.ParametersDescPhase) error {
135-
patch := client.MergeFrom(parametersDef.DeepCopy())
136-
parametersDef.Status.Phase = phase
137-
parametersDef.Status.ObservedGeneration = parametersDef.Generation
138-
return cli.Status().Patch(ctx.Ctx, parametersDef, patch)
139-
}
140-
141-
func checkParametersSchema(ctx intctrlutil.RequestCtx, parametersDef *parametersv1alpha1.ParametersDefinition) (bool, error) {
142-
// validate configuration template
143-
validateConfigSchema := func(ccSchema *parametersv1alpha1.ParametersSchema) (bool, error) {
144-
if ccSchema == nil || len(ccSchema.CUE) == 0 {
145-
return true, nil
146-
}
147-
err := validate.CueValidate(ccSchema.CUE)
148-
return err == nil, err
149-
}
150-
151-
// validate schema
152-
if ok, err := validateConfigSchema(parametersDef.Spec.ParametersSchema); !ok || err != nil {
153-
ctx.Log.Error(err, "failed to validate template schema!",
154-
"configMapName", fmt.Sprintf("%v", parametersDef.Spec.ParametersSchema))
155-
return ok, err
234+
func validateSchema(parametersDef *parametersv1alpha1.ParametersDefinition) error {
235+
schema := parametersDef.Spec.ParametersSchema
236+
if schema == nil || len(schema.CUE) == 0 {
237+
return nil
156238
}
157-
return true, nil
239+
return validate.CueValidate(schema.CUE)
158240
}
159241

160242
func updateParametersSchema(parametersDef *parametersv1alpha1.ParametersDefinition, cli client.Client, ctx context.Context) error {

‎controllers/parameters/parametersdefinition_controller_test.go‎

Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ import (
2727

2828
"sigs.k8s.io/controller-runtime/pkg/client"
2929

30+
appsv1 "github.com/apecloud/kubeblocks/apis/apps/v1"
3031
appsv1alpha1 "github.com/apecloud/kubeblocks/apis/apps/v1alpha1"
3132
parametersv1alpha1 "github.com/apecloud/kubeblocks/apis/parameters/v1alpha1"
3233
"github.com/apecloud/kubeblocks/pkg/constant"
@@ -50,6 +51,7 @@ var _ = Describe("ConfigConstraint Controller", func() {
5051
inNS := client.InNamespace(testCtx.DefaultNamespace)
5152
ml := client.HasLabels{testCtx.TestObjLabelKey}
5253
// non-namespaced
54+
testapps.ClearResources(&testCtx, intctrlutil.ComponentDefinitionSignature, ml)
5355
testapps.ClearResources(&testCtx, intctrlutil.ParametersDefinitionSignature, ml)
5456
// namespaced
5557
testapps.ClearResourcesWithRemoveFinalizerOption(&testCtx, intctrlutil.ConfigMapSignature, true, inNS, ml)
@@ -130,4 +132,124 @@ client?: {
130132
})).Should(Succeed())
131133
})
132134
})
135+
136+
Context("Validate referenced config template", func() {
137+
It("Should not be available when templateName is not defined in matched ComponentDefinition configs", func() {
138+
configmap := testparameters.NewComponentTemplateFactory("pd-template-check-cm",
139+
testCtx.DefaultNamespace).
140+
Create(&testCtx).
141+
GetObject()
142+
143+
compDef := testapps.NewComponentDefinitionFactory("pd-template-check-compdef").
144+
SetDefaultSpec().
145+
AddConfigTemplate(configSpecName, configmap.Name, testCtx.DefaultNamespace, configVolumeName, false).
146+
Create(&testCtx).
147+
GetObject()
148+
Expect(testapps.GetAndChangeObjStatus(&testCtx, client.ObjectKeyFromObject(compDef), func(obj *appsv1.ComponentDefinition) {
149+
obj.Status.Phase = appsv1.AvailablePhase
150+
})()).Should(Succeed())
151+
152+
parametersDef := testparameters.NewParametersDefinitionFactory("pd-template-check-params").
153+
SetComponentDefinition(compDef.Name).
154+
SetTemplateName("missing-config").
155+
Create(&testCtx).
156+
GetObject()
157+
158+
Eventually(testapps.CheckObj(&testCtx, client.ObjectKeyFromObject(parametersDef),
159+
func(g Gomega, tpl *parametersv1alpha1.ParametersDefinition) {
160+
g.Expect(tpl.Status.Phase).Should(BeEquivalentTo(parametersv1alpha1.PDUnavailablePhase))
161+
})).Should(Succeed())
162+
163+
Expect(testapps.GetAndChangeObj(&testCtx, client.ObjectKeyFromObject(parametersDef), func(obj *parametersv1alpha1.ParametersDefinition) {
164+
obj.Spec.TemplateName = configSpecName
165+
})()).Should(Succeed())
166+
167+
Eventually(testapps.CheckObj(&testCtx, client.ObjectKeyFromObject(parametersDef),
168+
func(g Gomega, tpl *parametersv1alpha1.ParametersDefinition) {
169+
g.Expect(tpl.Status.Phase).Should(BeEquivalentTo(parametersv1alpha1.PDAvailablePhase))
170+
})).Should(Succeed())
171+
172+
Expect(testapps.GetAndChangeObj(&testCtx, client.ObjectKeyFromObject(compDef), func(obj *appsv1.ComponentDefinition) {
173+
obj.Spec.Configs = nil
174+
})()).Should(Succeed())
175+
176+
Eventually(testapps.CheckObj(&testCtx, client.ObjectKeyFromObject(parametersDef),
177+
func(g Gomega, tpl *parametersv1alpha1.ParametersDefinition) {
178+
g.Expect(tpl.Status.Phase).Should(BeEquivalentTo(parametersv1alpha1.PDUnavailablePhase))
179+
})).Should(Succeed())
180+
})
181+
182+
It("Should not be available when any matched ComponentDefinition misses templateName", func() {
183+
configmap := testparameters.NewComponentTemplateFactory("pd-template-check-cm2",
184+
testCtx.DefaultNamespace).
185+
Create(&testCtx).
186+
GetObject()
187+
188+
validCompDef := testapps.NewComponentDefinitionFactory("pd-template-check-valid").
189+
SetDefaultSpec().
190+
AddConfigTemplate(configSpecName, configmap.Name, testCtx.DefaultNamespace, configVolumeName, false).
191+
Create(&testCtx).
192+
GetObject()
193+
Expect(testapps.GetAndChangeObjStatus(&testCtx, client.ObjectKeyFromObject(validCompDef), func(obj *appsv1.ComponentDefinition) {
194+
obj.Status.Phase = appsv1.AvailablePhase
195+
})()).Should(Succeed())
196+
197+
missingCompDef := testapps.NewComponentDefinitionFactory("pd-template-check-missing").
198+
SetDefaultSpec().
199+
Create(&testCtx).
200+
GetObject()
201+
Expect(testapps.GetAndChangeObjStatus(&testCtx, client.ObjectKeyFromObject(missingCompDef), func(obj *appsv1.ComponentDefinition) {
202+
obj.Status.Phase = appsv1.AvailablePhase
203+
})()).Should(Succeed())
204+
205+
parametersDef := testparameters.NewParametersDefinitionFactory("pd-template-check-params2").
206+
SetComponentDefinition("pd-template-check").
207+
SetTemplateName(configSpecName).
208+
Create(&testCtx).
209+
GetObject()
210+
211+
Eventually(testapps.CheckObj(&testCtx, client.ObjectKeyFromObject(parametersDef),
212+
func(g Gomega, tpl *parametersv1alpha1.ParametersDefinition) {
213+
g.Expect(tpl.Status.Phase).Should(BeEquivalentTo(parametersv1alpha1.PDUnavailablePhase))
214+
})).Should(Succeed())
215+
})
216+
217+
It("Should ignore ComponentDefinitions with unmatched serviceVersion", func() {
218+
configmap := testparameters.NewComponentTemplateFactory("pd-template-check-cm3",
219+
testCtx.DefaultNamespace).
220+
Create(&testCtx).
221+
GetObject()
222+
223+
validCompDef := testapps.NewComponentDefinitionFactory("pd-template-check-v8").
224+
SetDefaultSpec().
225+
SetServiceVersion("8.0").
226+
AddConfigTemplate(configSpecName, configmap.Name, testCtx.DefaultNamespace, configVolumeName, false).
227+
Create(&testCtx).
228+
GetObject()
229+
Expect(testapps.GetAndChangeObjStatus(&testCtx, client.ObjectKeyFromObject(validCompDef), func(obj *appsv1.ComponentDefinition) {
230+
obj.Status.Phase = appsv1.AvailablePhase
231+
})()).Should(Succeed())
232+
233+
unmatchedCompDef := testapps.NewComponentDefinitionFactory("pd-template-check-v9").
234+
SetDefaultSpec().
235+
SetServiceVersion("9.0").
236+
Create(&testCtx).
237+
GetObject()
238+
Expect(testapps.GetAndChangeObjStatus(&testCtx, client.ObjectKeyFromObject(unmatchedCompDef), func(obj *appsv1.ComponentDefinition) {
239+
obj.Status.Phase = appsv1.AvailablePhase
240+
})()).Should(Succeed())
241+
242+
parametersDef := testparameters.NewParametersDefinitionFactory("pd-template-check-params3").
243+
SetComponentDefinition("pd-template-check").
244+
SetServiceVersion("8.0").
245+
SetTemplateName(configSpecName).
246+
Create(&testCtx).
247+
GetObject()
248+
249+
Eventually(testapps.CheckObj(&testCtx, client.ObjectKeyFromObject(parametersDef),
250+
func(g Gomega, tpl *parametersv1alpha1.ParametersDefinition) {
251+
g.Expect(tpl.Status.Phase).Should(BeEquivalentTo(parametersv1alpha1.PDAvailablePhase))
252+
})).Should(Succeed())
253+
})
254+
})
133255
})

‎pkg/parameters/config_util.go‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -313,7 +313,7 @@ func ResolveCmpdParametersDefs(ctx context.Context, reader client.Reader, cmpd *
313313
coveredFiles := make(map[string]string)
314314
for i := range paramsDefList.Items {
315315
paramsDef := &paramsDefList.Items[i]
316-
matched, err := matchParametersDefinition(cmpd, paramsDef)
316+
matched, err := MatchParametersDefinition(cmpd, paramsDef)
317317
if err != nil {
318318
return nil, nil, err
319319
}
@@ -385,7 +385,8 @@ func resolveCmpdParametersDefsByConfigRender(ctx context.Context, reader client.
385385
return slices.Clone(configRender.Spec.Configs), paramsDefs, nil
386386
}
387387

388-
func matchParametersDefinition(cmpd *appsv1.ComponentDefinition, paramsDef *parametersv1alpha1.ParametersDefinition) (bool, error) {
388+
// MatchParametersDefinition reports whether a ParametersDefinition applies to the ComponentDefinition.
389+
func MatchParametersDefinition(cmpd *appsv1.ComponentDefinition, paramsDef *parametersv1alpha1.ParametersDefinition) (bool, error) {
389390
if cmpd == nil || paramsDef == nil {
390391
return false, nil
391392
}

0 commit comments

Comments
 (0)