Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

✨ ClusterClass patches: Allow matching all MachineDeploymentClasses #6930

Merged
merged 1 commit into from
Jan 16, 2023
Merged

✨ ClusterClass patches: Allow matching all MachineDeploymentClasses #6930

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions internal/controllers/topology/cluster/patches/inline/json_patch_generator.go
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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) ||
sbueringer marked this conversation as resolved.
Show resolved Hide resolved
selectorMatchTemplate(selector, md.Template.Bootstrap.Ref) {
match = true
Expand Down
Loading