Skip to content

Commit 0119de7

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request kubernetes-sigs#6930 from valaparthvi/5648-allow-matching-all-mdc
✨ ClusterClass patches: Allow matching all MachineDeploymentClasses
2 parents 6a82514 + df9283c commit 0119de7

File tree

5 files changed

+364
-5
lines changed

5 files changed

+364
-5
lines changed

‎internal/controllers/topology/cluster/patches/inline/json_patch_generator.go‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ import (
2525
"strings"
2626
"text/template"
2727

28-
sprig "github.com/Masterminds/sprig/v3"
28+
"github.com/Masterminds/sprig/v3"
2929
"github.com/pkg/errors"
3030
apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
3131
kerrors "k8s.io/apimachinery/pkg/util/errors"
@@ -170,7 +170,14 @@ func matchesSelector(req *runtimehooksv1.GeneratePatchesRequestItem, templateVar
170170
// If templateMDClass matches one of the configured MachineDeploymentClasses.
171171
for _, mdClass := range selector.MatchResources.MachineDeploymentClass.Names {
172172
// We have to quote mdClass as templateMDClassJSON is a JSON string (e.g. "default-worker").
173-
if string(templateMDClassJSON.Raw) == strconv.Quote(mdClass) {
173+
if mdClass == "*" || string(templateMDClassJSON.Raw) == strconv.Quote(mdClass) {
174+
return true
175+
}
176+
unquoted, _ := strconv.Unquote(string(templateMDClassJSON.Raw))
177+
if strings.HasPrefix(mdClass, "*") && strings.HasSuffix(unquoted, strings.TrimPrefix(mdClass, "*")) {
178+
return true
179+
}
180+
if strings.HasSuffix(mdClass, "*") && strings.HasPrefix(unquoted, strings.TrimSuffix(mdClass, "*")) {
174181
return true
175182
}
176183
}

‎internal/controllers/topology/cluster/patches/inline/json_patch_generator_test.go‎

Lines changed: 164 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -617,6 +617,170 @@ func TestMatchesSelector(t *testing.T) {
617617
},
618618
match: true,
619619
},
620+
{
621+
name: "Match all MD BootstrapTemplate",
622+
req: &runtimehooksv1.GeneratePatchesRequestItem{
623+
Object: runtime.RawExtension{
624+
Object: &unstructured.Unstructured{
625+
Object: map[string]interface{}{
626+
"apiVersion": "bootstrap.cluster.x-k8s.io/v1beta1",
627+
"kind": "BootstrapTemplate",
628+
},
629+
},
630+
},
631+
HolderReference: runtimehooksv1.HolderReference{
632+
APIVersion: clusterv1.GroupVersion.String(),
633+
Kind: "MachineDeployment",
634+
Name: "my-md-0",
635+
Namespace: "default",
636+
FieldPath: "spec.template.spec.bootstrap.configRef",
637+
},
638+
},
639+
templateVariables: map[string]apiextensionsv1.JSON{
640+
"builtin": {Raw: []byte(`{"machineDeployment":{"class":"classA"}}`)},
641+
},
642+
selector: clusterv1.PatchSelector{
643+
APIVersion: "bootstrap.cluster.x-k8s.io/v1beta1",
644+
Kind: "BootstrapTemplate",
645+
MatchResources: clusterv1.PatchSelectorMatch{
646+
MachineDeploymentClass: &clusterv1.PatchSelectorMatchMachineDeploymentClass{
647+
Names: []string{"*"},
648+
},
649+
},
650+
},
651+
match: true,
652+
},
653+
{
654+
name: "Glob match MD BootstrapTemplate with <string>-*",
655+
req: &runtimehooksv1.GeneratePatchesRequestItem{
656+
Object: runtime.RawExtension{
657+
Object: &unstructured.Unstructured{
658+
Object: map[string]interface{}{
659+
"apiVersion": "bootstrap.cluster.x-k8s.io/v1beta1",
660+
"kind": "BootstrapTemplate",
661+
},
662+
},
663+
},
664+
HolderReference: runtimehooksv1.HolderReference{
665+
APIVersion: clusterv1.GroupVersion.String(),
666+
Kind: "MachineDeployment",
667+
Name: "my-md-0",
668+
Namespace: "default",
669+
FieldPath: "spec.template.spec.bootstrap.configRef",
670+
},
671+
},
672+
templateVariables: map[string]apiextensionsv1.JSON{
673+
"builtin": {Raw: []byte(`{"machineDeployment":{"class":"class-A"}}`)},
674+
},
675+
selector: clusterv1.PatchSelector{
676+
APIVersion: "bootstrap.cluster.x-k8s.io/v1beta1",
677+
Kind: "BootstrapTemplate",
678+
MatchResources: clusterv1.PatchSelectorMatch{
679+
MachineDeploymentClass: &clusterv1.PatchSelectorMatchMachineDeploymentClass{
680+
Names: []string{"class-*"},
681+
},
682+
},
683+
},
684+
match: true,
685+
},
686+
{
687+
name: "Glob match MD BootstrapTemplate with *-<string>",
688+
req: &runtimehooksv1.GeneratePatchesRequestItem{
689+
Object: runtime.RawExtension{
690+
Object: &unstructured.Unstructured{
691+
Object: map[string]interface{}{
692+
"apiVersion": "bootstrap.cluster.x-k8s.io/v1beta1",
693+
"kind": "BootstrapTemplate",
694+
},
695+
},
696+
},
697+
HolderReference: runtimehooksv1.HolderReference{
698+
APIVersion: clusterv1.GroupVersion.String(),
699+
Kind: "MachineDeployment",
700+
Name: "my-md-0",
701+
Namespace: "default",
702+
FieldPath: "spec.template.spec.bootstrap.configRef",
703+
},
704+
},
705+
templateVariables: map[string]apiextensionsv1.JSON{
706+
"builtin": {Raw: []byte(`{"machineDeployment":{"class":"class-A"}}`)},
707+
},
708+
selector: clusterv1.PatchSelector{
709+
APIVersion: "bootstrap.cluster.x-k8s.io/v1beta1",
710+
Kind: "BootstrapTemplate",
711+
MatchResources: clusterv1.PatchSelectorMatch{
712+
MachineDeploymentClass: &clusterv1.PatchSelectorMatchMachineDeploymentClass{
713+
Names: []string{"*-A"},
714+
},
715+
},
716+
},
717+
match: true,
718+
},
719+
720+
{
721+
name: "Don't match BootstrapTemplate, .matchResources.machineDeploymentClass.names is empty",
722+
req: &runtimehooksv1.GeneratePatchesRequestItem{
723+
Object: runtime.RawExtension{
724+
Object: &unstructured.Unstructured{
725+
Object: map[string]interface{}{
726+
"apiVersion": "bootstrap.cluster.x-k8s.io/v1beta1",
727+
"kind": "BootstrapTemplate",
728+
},
729+
},
730+
},
731+
HolderReference: runtimehooksv1.HolderReference{
732+
APIVersion: clusterv1.GroupVersion.String(),
733+
Kind: "MachineDeployment",
734+
Name: "my-md-0",
735+
Namespace: "default",
736+
FieldPath: "spec.template.spec.bootstrap.configRef",
737+
},
738+
},
739+
templateVariables: map[string]apiextensionsv1.JSON{
740+
"builtin": {Raw: []byte(`{"machineDeployment":{"class":"classA"}}`)},
741+
},
742+
selector: clusterv1.PatchSelector{
743+
APIVersion: "bootstrap.cluster.x-k8s.io/v1beta1",
744+
Kind: "BootstrapTemplate",
745+
MatchResources: clusterv1.PatchSelectorMatch{
746+
MachineDeploymentClass: &clusterv1.PatchSelectorMatchMachineDeploymentClass{
747+
Names: []string{},
748+
},
749+
},
750+
},
751+
match: false,
752+
},
753+
{
754+
name: "Do not match BootstrapTemplate, .matchResources.machineDeploymentClass is set to nil",
755+
req: &runtimehooksv1.GeneratePatchesRequestItem{
756+
Object: runtime.RawExtension{
757+
Object: &unstructured.Unstructured{
758+
Object: map[string]interface{}{
759+
"apiVersion": "bootstrap.cluster.x-k8s.io/v1beta1",
760+
"kind": "BootstrapTemplate",
761+
},
762+
},
763+
},
764+
HolderReference: runtimehooksv1.HolderReference{
765+
APIVersion: clusterv1.GroupVersion.String(),
766+
Kind: "MachineDeployment",
767+
Name: "my-md-0",
768+
Namespace: "default",
769+
FieldPath: "spec.template.spec.bootstrap.configRef",
770+
},
771+
},
772+
templateVariables: map[string]apiextensionsv1.JSON{
773+
"builtin": {Raw: []byte(`{"machineDeployment":{"class":"classA"}}`)},
774+
},
775+
selector: clusterv1.PatchSelector{
776+
APIVersion: "bootstrap.cluster.x-k8s.io/v1beta1",
777+
Kind: "BootstrapTemplate",
778+
MatchResources: clusterv1.PatchSelectorMatch{
779+
MachineDeploymentClass: nil,
780+
},
781+
},
782+
match: false,
783+
},
620784
{
621785
name: "Don't match BootstrapTemplate, .matchResources.machineDeploymentClass not set",
622786
req: &runtimehooksv1.GeneratePatchesRequestItem{

‎internal/webhooks/patch_validation.go‎

Lines changed: 40 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,10 +23,11 @@ import (
2323
"strings"
2424
"text/template"
2525

26-
sprig "github.com/Masterminds/sprig/v3"
26+
"github.com/Masterminds/sprig/v3"
2727
"github.com/pkg/errors"
2828
corev1 "k8s.io/api/core/v1"
2929
"k8s.io/apimachinery/pkg/util/sets"
30+
"k8s.io/apimachinery/pkg/util/validation"
3031
"k8s.io/apimachinery/pkg/util/validation/field"
3132

3233
clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
@@ -198,8 +199,45 @@ func validateSelectors(selector clusterv1.PatchSelector, class *clusterv1.Cluste
198199
if selector.MatchResources.MachineDeploymentClass != nil && len(selector.MatchResources.MachineDeploymentClass.Names) > 0 {
199200
for i, name := range selector.MatchResources.MachineDeploymentClass.Names {
200201
match := false
202+
if strings.Contains(name, "*") {
203+
// selector can at most have a single * rune
204+
if strings.Count(name, "*") > 1 {
205+
allErrs = append(allErrs, field.Invalid(
206+
path.Child("matchResources", "machineDeploymentClass", "names").Index(i),
207+
name,
208+
"selector can at most contain a single \"*\" rune"))
209+
break
210+
}
211+
212+
// the * rune can appear only at the beginning, or ending of the selector.
213+
if strings.Contains(name, "*") && !(strings.HasPrefix(name, "*") || strings.HasSuffix(name, "*")) {
214+
// templateMDClass can only have "*" rune at the start or end of the string
215+
allErrs = append(allErrs, field.Invalid(
216+
path.Child("matchResources", "machineDeploymentClass", "names").Index(i),
217+
name,
218+
"\"*\" rune can only appear at the beginning, or ending of the selector"))
219+
break
220+
}
221+
// a valid selector without "*" should comply with Kubernetes naming standards.
222+
if validation.IsQualifiedName(strings.ReplaceAll(name, "*", "a")) != nil {
223+
allErrs = append(allErrs, field.Invalid(
224+
path.Child("matchResources", "machineDeploymentClass", "names").Index(i),
225+
name,
226+
"selector does not comply with the Kubernetes naming standards"))
227+
break
228+
}
229+
}
201230
for _, md := range class.Spec.Workers.MachineDeployments {
202-
if md.Class == name {
231+
var matches bool
232+
if md.Class == name || name == "*" {
233+
matches = true
234+
} else if strings.HasPrefix(name, "*") && strings.HasSuffix(md.Class, strings.TrimPrefix(name, "*")) {
235+
matches = true
236+
} else if strings.HasSuffix(name, "*") && strings.HasPrefix(md.Class, strings.TrimSuffix(name, "*")) {
237+
matches = true
238+
}
239+
240+
if matches {
203241
if selectorMatchTemplate(selector, md.Template.Infrastructure.Ref) ||
204242
selectorMatchTemplate(selector, md.Template.Bootstrap.Ref) {
205243
match = true

0 commit comments

Comments
 (0)