Skip to content

Commit

Permalink
Merge pull request #6930 from valaparthvi/5648-allow-matching-all-mdc
Browse files Browse the repository at this point in the history 
✨ ClusterClass patches: Allow matching all MachineDeploymentClasses
  • Loading branch information
@k8s-ci-robot
k8s-ci-robot authored Jan 16, 2023
2 parents 6a82514 + df9283c commit 0119de7
Show file tree
Hide file tree
Showing 5 changed files with 364 additions and 5 deletions.
11 changes: 9 additions & 2 deletions internal/controllers/topology/cluster/patches/inline/json_patch_generator.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import (
"strings"
"text/template"

sprig "github.com/Masterminds/sprig/v3"
"github.com/Masterminds/sprig/v3"
"github.com/pkg/errors"
apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
kerrors "k8s.io/apimachinery/pkg/util/errors"
Expand Down Expand Up @@ -170,7 +170,14 @@ func matchesSelector(req *runtimehooksv1.GeneratePatchesRequestItem, templateVar
// If templateMDClass matches one of the configured MachineDeploymentClasses.
for _, mdClass := range selector.MatchResources.MachineDeploymentClass.Names {
// We have to quote mdClass as templateMDClassJSON is a JSON string (e.g. "default-worker").
if string(templateMDClassJSON.Raw) == strconv.Quote(mdClass) {
if mdClass == "*" || string(templateMDClassJSON.Raw) == strconv.Quote(mdClass) {
return true
}
unquoted, _ := strconv.Unquote(string(templateMDClassJSON.Raw))
if strings.HasPrefix(mdClass, "*") && strings.HasSuffix(unquoted, strings.TrimPrefix(mdClass, "*")) {
return true
}
if strings.HasSuffix(mdClass, "*") && strings.HasPrefix(unquoted, strings.TrimSuffix(mdClass, "*")) {
return true
}
}
Expand Down
164 changes: 164 additions & 0 deletions internal/controllers/topology/cluster/patches/inline/json_patch_generator_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -617,6 +617,170 @@ func TestMatchesSelector(t *testing.T) {
},
match: true,
},
{
name: "Match all MD BootstrapTemplate",
req: &runtimehooksv1.GeneratePatchesRequestItem{
Object: runtime.RawExtension{
Object: &unstructured.Unstructured{
Object: map[string]interface{}{
"apiVersion": "bootstrap.cluster.x-k8s.io/v1beta1",
"kind": "BootstrapTemplate",
},
},
},
HolderReference: runtimehooksv1.HolderReference{
APIVersion: clusterv1.GroupVersion.String(),
Kind: "MachineDeployment",
Name: "my-md-0",
Namespace: "default",
FieldPath: "spec.template.spec.bootstrap.configRef",
},
},
templateVariables: map[string]apiextensionsv1.JSON{
"builtin": {Raw: []byte(`{"machineDeployment":{"class":"classA"}}`)},
},
selector: clusterv1.PatchSelector{
APIVersion: "bootstrap.cluster.x-k8s.io/v1beta1",
Kind: "BootstrapTemplate",
MatchResources: clusterv1.PatchSelectorMatch{
MachineDeploymentClass: &clusterv1.PatchSelectorMatchMachineDeploymentClass{
Names: []string{"*"},
},
},
},
match: true,
},
{
name: "Glob match MD BootstrapTemplate with <string>-*",
req: &runtimehooksv1.GeneratePatchesRequestItem{
Object: runtime.RawExtension{
Object: &unstructured.Unstructured{
Object: map[string]interface{}{
"apiVersion": "bootstrap.cluster.x-k8s.io/v1beta1",
"kind": "BootstrapTemplate",
},
},
},
HolderReference: runtimehooksv1.HolderReference{
APIVersion: clusterv1.GroupVersion.String(),
Kind: "MachineDeployment",
Name: "my-md-0",
Namespace: "default",
FieldPath: "spec.template.spec.bootstrap.configRef",
},
},
templateVariables: map[string]apiextensionsv1.JSON{
"builtin": {Raw: []byte(`{"machineDeployment":{"class":"class-A"}}`)},
},
selector: clusterv1.PatchSelector{
APIVersion: "bootstrap.cluster.x-k8s.io/v1beta1",
Kind: "BootstrapTemplate",
MatchResources: clusterv1.PatchSelectorMatch{
MachineDeploymentClass: &clusterv1.PatchSelectorMatchMachineDeploymentClass{
Names: []string{"class-*"},
},
},
},
match: true,
},
{
name: "Glob match MD BootstrapTemplate with *-<string>",
req: &runtimehooksv1.GeneratePatchesRequestItem{
Object: runtime.RawExtension{
Object: &unstructured.Unstructured{
Object: map[string]interface{}{
"apiVersion": "bootstrap.cluster.x-k8s.io/v1beta1",
"kind": "BootstrapTemplate",
},
},
},
HolderReference: runtimehooksv1.HolderReference{
APIVersion: clusterv1.GroupVersion.String(),
Kind: "MachineDeployment",
Name: "my-md-0",
Namespace: "default",
FieldPath: "spec.template.spec.bootstrap.configRef",
},
},
templateVariables: map[string]apiextensionsv1.JSON{
"builtin": {Raw: []byte(`{"machineDeployment":{"class":"class-A"}}`)},
},
selector: clusterv1.PatchSelector{
APIVersion: "bootstrap.cluster.x-k8s.io/v1beta1",
Kind: "BootstrapTemplate",
MatchResources: clusterv1.PatchSelectorMatch{
MachineDeploymentClass: &clusterv1.PatchSelectorMatchMachineDeploymentClass{
Names: []string{"*-A"},
},
},
},
match: true,
},

{
name: "Don't match BootstrapTemplate, .matchResources.machineDeploymentClass.names is empty",
req: &runtimehooksv1.GeneratePatchesRequestItem{
Object: runtime.RawExtension{
Object: &unstructured.Unstructured{
Object: map[string]interface{}{
"apiVersion": "bootstrap.cluster.x-k8s.io/v1beta1",
"kind": "BootstrapTemplate",
},
},
},
HolderReference: runtimehooksv1.HolderReference{
APIVersion: clusterv1.GroupVersion.String(),
Kind: "MachineDeployment",
Name: "my-md-0",
Namespace: "default",
FieldPath: "spec.template.spec.bootstrap.configRef",
},
},
templateVariables: map[string]apiextensionsv1.JSON{
"builtin": {Raw: []byte(`{"machineDeployment":{"class":"classA"}}`)},
},
selector: clusterv1.PatchSelector{
APIVersion: "bootstrap.cluster.x-k8s.io/v1beta1",
Kind: "BootstrapTemplate",
MatchResources: clusterv1.PatchSelectorMatch{
MachineDeploymentClass: &clusterv1.PatchSelectorMatchMachineDeploymentClass{
Names: []string{},
},
},
},
match: false,
},
{
name: "Do not match BootstrapTemplate, .matchResources.machineDeploymentClass is set to nil",
req: &runtimehooksv1.GeneratePatchesRequestItem{
Object: runtime.RawExtension{
Object: &unstructured.Unstructured{
Object: map[string]interface{}{
"apiVersion": "bootstrap.cluster.x-k8s.io/v1beta1",
"kind": "BootstrapTemplate",
},
},
},
HolderReference: runtimehooksv1.HolderReference{
APIVersion: clusterv1.GroupVersion.String(),
Kind: "MachineDeployment",
Name: "my-md-0",
Namespace: "default",
FieldPath: "spec.template.spec.bootstrap.configRef",
},
},
templateVariables: map[string]apiextensionsv1.JSON{
"builtin": {Raw: []byte(`{"machineDeployment":{"class":"classA"}}`)},
},
selector: clusterv1.PatchSelector{
APIVersion: "bootstrap.cluster.x-k8s.io/v1beta1",
Kind: "BootstrapTemplate",
MatchResources: clusterv1.PatchSelectorMatch{
MachineDeploymentClass: nil,
},
},
match: false,
},
{
name: "Don't match BootstrapTemplate, .matchResources.machineDeploymentClass not set",
req: &runtimehooksv1.GeneratePatchesRequestItem{
Expand Down
42 changes: 40 additions & 2 deletions internal/webhooks/patch_validation.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,11 @@ import (
"strings"
"text/template"

sprig "github.com/Masterminds/sprig/v3"
"github.com/Masterminds/sprig/v3"
"github.com/pkg/errors"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/util/sets"
"k8s.io/apimachinery/pkg/util/validation"
"k8s.io/apimachinery/pkg/util/validation/field"

clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
Expand Down Expand Up @@ -198,8 +199,45 @@ func validateSelectors(selector clusterv1.PatchSelector, class *clusterv1.Cluste
if selector.MatchResources.MachineDeploymentClass != nil && len(selector.MatchResources.MachineDeploymentClass.Names) > 0 {
for i, name := range selector.MatchResources.MachineDeploymentClass.Names {
match := false
if strings.Contains(name, "*") {
// selector can at most have a single * rune
if strings.Count(name, "*") > 1 {
allErrs = append(allErrs, field.Invalid(
path.Child("matchResources", "machineDeploymentClass", "names").Index(i),
name,
"selector can at most contain a single \"*\" rune"))
break
}

// the * rune can appear only at the beginning, or ending of the selector.
if strings.Contains(name, "*") && !(strings.HasPrefix(name, "*") || strings.HasSuffix(name, "*")) {
// templateMDClass can only have "*" rune at the start or end of the string
allErrs = append(allErrs, field.Invalid(
path.Child("matchResources", "machineDeploymentClass", "names").Index(i),
name,
"\"*\" rune can only appear at the beginning, or ending of the selector"))
break
}
// a valid selector without "*" should comply with Kubernetes naming standards.
if validation.IsQualifiedName(strings.ReplaceAll(name, "*", "a")) != nil {
allErrs = append(allErrs, field.Invalid(
path.Child("matchResources", "machineDeploymentClass", "names").Index(i),
name,
"selector does not comply with the Kubernetes naming standards"))
break
}
}
for _, md := range class.Spec.Workers.MachineDeployments {
if md.Class == name {
var matches bool
if md.Class == name || name == "*" {
matches = true
} else if strings.HasPrefix(name, "*") && strings.HasSuffix(md.Class, strings.TrimPrefix(name, "*")) {
matches = true
} else if strings.HasSuffix(name, "*") && strings.HasPrefix(md.Class, strings.TrimSuffix(name, "*")) {
matches = true
}

if matches {
if selectorMatchTemplate(selector, md.Template.Infrastructure.Ref) ||
selectorMatchTemplate(selector, md.Template.Bootstrap.Ref) {
match = true
Expand Down
Loading

0 comments on commit 0119de7

Please sign in to comment.