Skip to content

Commit 7483e2f

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request kubernetes-sigs#7242 from sbueringer/pr-improve-patch-selector-validation
🌱 ClusterClass: make patch selector validation more robust
2 parents b608605 + a4231a6 commit 7483e2f

File tree

2 files changed

+226
-24
lines changed

2 files changed

+226
-24
lines changed

‎internal/webhooks/patch_validation.go‎

Lines changed: 35 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -166,44 +166,58 @@ func validateSelectors(selector clusterv1.PatchSelector, class *clusterv1.Cluste
166166
"no selector enabled",
167167
))
168168
}
169+
169170
if selector.MatchResources.InfrastructureCluster {
170-
if selectorMatchTemplate(selector, class.Spec.Infrastructure.Ref) {
171-
return nil
171+
if !selectorMatchTemplate(selector, class.Spec.Infrastructure.Ref) {
172+
allErrs = append(allErrs, field.Invalid(
173+
path.Child("matchResources", "infrastructureCluster"),
174+
selector.MatchResources.InfrastructureCluster,
175+
"selector is enabled but does not match the infrastructure ref",
176+
))
172177
}
173178
}
179+
174180
if selector.MatchResources.ControlPlane {
181+
match := false
175182
if selectorMatchTemplate(selector, class.Spec.ControlPlane.Ref) {
176-
return nil
183+
match = true
177184
}
178-
}
179-
180-
if selector.MatchResources.ControlPlane && class.Spec.ControlPlane.MachineInfrastructure != nil {
181-
if selectorMatchTemplate(selector, class.Spec.ControlPlane.MachineInfrastructure.Ref) {
182-
return nil
185+
if class.Spec.ControlPlane.MachineInfrastructure != nil &&
186+
selectorMatchTemplate(selector, class.Spec.ControlPlane.MachineInfrastructure.Ref) {
187+
match = true
188+
}
189+
if !match {
190+
allErrs = append(allErrs, field.Invalid(
191+
path.Child("matchResources", "controlPlane"),
192+
selector.MatchResources.ControlPlane,
193+
"selector is enabled but matches neither the controlPlane ref nor the controlPlane machineInfrastructure ref",
194+
))
183195
}
184196
}
185197

186198
if selector.MatchResources.MachineDeploymentClass != nil && len(selector.MatchResources.MachineDeploymentClass.Names) > 0 {
187-
for _, name := range selector.MatchResources.MachineDeploymentClass.Names {
199+
for i, name := range selector.MatchResources.MachineDeploymentClass.Names {
200+
match := false
188201
for _, md := range class.Spec.Workers.MachineDeployments {
189202
if md.Class == name {
190-
if selectorMatchTemplate(selector, md.Template.Infrastructure.Ref) {
191-
return nil
192-
}
193-
if selectorMatchTemplate(selector, md.Template.Bootstrap.Ref) {
194-
return nil
203+
if selectorMatchTemplate(selector, md.Template.Infrastructure.Ref) ||
204+
selectorMatchTemplate(selector, md.Template.Bootstrap.Ref) {
205+
match = true
206+
break
195207
}
196208
}
197209
}
210+
if !match {
211+
allErrs = append(allErrs, field.Invalid(
212+
path.Child("matchResources", "machineDeploymentClass", "names").Index(i),
213+
name,
214+
"selector is enabled but matches neither the bootstrap ref nor the infrastructure ref of a MachineDeployment class",
215+
))
216+
}
198217
}
199218
}
200-
// if the code has not returned at this point there is no matching template in the ClusterClass. Return an error.
201-
return append(allErrs,
202-
field.Invalid(
203-
path,
204-
prettyPrint(selector),
205-
"selector did not match any template in the ClusterClass",
206-
))
219+
220+
return allErrs
207221
}
208222

209223
// selectorMatchTemplate returns true if APIVersion and Kind for the given selector match the reference.

‎internal/webhooks/patch_validation_test.go‎

Lines changed: 191 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1538,7 +1538,7 @@ func Test_validateSelectors(t *testing.T) {
15381538
wantErr bool
15391539
}{
15401540
{
1541-
name: "error if no selectors are all set to false or empty",
1541+
name: "error if selectors are all set to false or empty",
15421542
selector: clusterv1.PatchSelector{
15431543
APIVersion: "infrastructure.cluster.x-k8s.io/v1beta1",
15441544
Kind: "InfrastructureClusterTemplate",
@@ -1787,6 +1787,65 @@ func Test_validateSelectors(t *testing.T) {
17871787
Build(),
17881788
wantErr: true,
17891789
},
1790+
{
1791+
name: "fail if selector targets ControlPlane Machine Infrastructure but does not have MatchResources.ControlPlane enabled",
1792+
selector: clusterv1.PatchSelector{
1793+
APIVersion: "infrastructure.cluster.x-k8s.io/v1beta1",
1794+
Kind: "InfrastructureMachineTemplate",
1795+
MatchResources: clusterv1.PatchSelectorMatch{
1796+
MachineDeploymentClass: &clusterv1.PatchSelectorMatchMachineDeploymentClass{
1797+
Names: []string{"bb"},
1798+
},
1799+
ControlPlane: false,
1800+
},
1801+
},
1802+
clusterClass: builder.ClusterClass(metav1.NamespaceDefault, "class1").
1803+
WithControlPlaneInfrastructureMachineTemplate(
1804+
refToUnstructured(
1805+
&corev1.ObjectReference{
1806+
APIVersion: "infrastructure.cluster.x-k8s.io/v1beta1",
1807+
Kind: "InfrastructureMachineTemplate",
1808+
}),
1809+
).
1810+
Build(),
1811+
wantErr: true,
1812+
},
1813+
// The following tests have selectors which match multiple resources at the same time.
1814+
{
1815+
name: "fail if selector targets a matching infrastructureCluster reference and a not matching control plane",
1816+
selector: clusterv1.PatchSelector{
1817+
APIVersion: "infrastructure.cluster.x-k8s.io/v1beta1",
1818+
Kind: "InfrastructureClusterTemplate",
1819+
MatchResources: clusterv1.PatchSelectorMatch{
1820+
InfrastructureCluster: true,
1821+
ControlPlane: true,
1822+
},
1823+
},
1824+
clusterClass: builder.ClusterClass(metav1.NamespaceDefault, "class1").
1825+
WithInfrastructureClusterTemplate(
1826+
refToUnstructured(
1827+
&corev1.ObjectReference{
1828+
APIVersion: "infrastructure.cluster.x-k8s.io/v1beta1",
1829+
Kind: "InfrastructureClusterTemplate",
1830+
}),
1831+
).
1832+
WithControlPlaneTemplate(
1833+
refToUnstructured(
1834+
&corev1.ObjectReference{
1835+
APIVersion: "controlplane.cluster.x-k8s.io/v1beta1",
1836+
Kind: "NonMatchingControlPlaneTemplate",
1837+
}),
1838+
).
1839+
WithControlPlaneInfrastructureMachineTemplate(
1840+
refToUnstructured(
1841+
&corev1.ObjectReference{
1842+
APIVersion: "infrastructure.cluster.x-k8s.io/v1beta1",
1843+
Kind: "NonMatchingInfrastructureMachineTemplate",
1844+
}),
1845+
).
1846+
Build(),
1847+
wantErr: true,
1848+
},
17901849
{
17911850
name: "pass if selector targets BOTH an existing ControlPlane MachineInfrastructureTemplate and an existing MachineDeploymentClass InfrastructureTemplate",
17921851
selector: clusterv1.PatchSelector{
@@ -1837,25 +1896,154 @@ func Test_validateSelectors(t *testing.T) {
18371896
wantErr: false,
18381897
},
18391898
{
1840-
name: "fail if selector targets ControlPlane Machine Infrastructure but does not have MatchResources.ControlPlane enabled",
1899+
name: "fail if selector targets BOTH an existing ControlPlane MachineInfrastructureTemplate and an existing MachineDeploymentClass InfrastructureTemplate but does not match all MachineDeployment classes",
1900+
selector: clusterv1.PatchSelector{
1901+
APIVersion: "infrastructure.cluster.x-k8s.io/v1beta1",
1902+
Kind: "InfrastructureMachineTemplate",
1903+
MatchResources: clusterv1.PatchSelectorMatch{
1904+
MachineDeploymentClass: &clusterv1.PatchSelectorMatchMachineDeploymentClass{
1905+
Names: []string{"aa", "bb"},
1906+
},
1907+
ControlPlane: true,
1908+
},
1909+
},
1910+
clusterClass: builder.ClusterClass(metav1.NamespaceDefault, "class1").
1911+
WithControlPlaneTemplate(
1912+
refToUnstructured(
1913+
&corev1.ObjectReference{
1914+
APIVersion: "controlplane.cluster.x-k8s.io/v1beta1",
1915+
Kind: "NonMatchingControlPlaneTemplate",
1916+
}),
1917+
).
1918+
WithControlPlaneInfrastructureMachineTemplate(
1919+
refToUnstructured(
1920+
&corev1.ObjectReference{
1921+
APIVersion: "infrastructure.cluster.x-k8s.io/v1beta1",
1922+
Kind: "InfrastructureMachineTemplate",
1923+
}),
1924+
).
1925+
WithWorkerMachineDeploymentClasses(
1926+
*builder.MachineDeploymentClass("aa").
1927+
WithInfrastructureTemplate(
1928+
refToUnstructured(&corev1.ObjectReference{
1929+
APIVersion: "infrastructure.cluster.x-k8s.io/v1beta1",
1930+
Kind: "NonMatchingInfrastructureMachineTemplate",
1931+
})).
1932+
WithBootstrapTemplate(
1933+
refToUnstructured(&corev1.ObjectReference{
1934+
APIVersion: "bootstrap.cluster.x-k8s.io/v1beta1",
1935+
Kind: "BootstrapTemplate",
1936+
})).
1937+
Build(),
1938+
*builder.MachineDeploymentClass("bb").
1939+
WithInfrastructureTemplate(
1940+
refToUnstructured(&corev1.ObjectReference{
1941+
APIVersion: "infrastructure.cluster.x-k8s.io/v1beta1",
1942+
Kind: "InfrastructureMachineTemplate",
1943+
})).
1944+
WithBootstrapTemplate(
1945+
refToUnstructured(&corev1.ObjectReference{
1946+
APIVersion: "bootstrap.cluster.x-k8s.io/v1beta1",
1947+
Kind: "BootstrapTemplate",
1948+
})).
1949+
Build(),
1950+
).
1951+
Build(),
1952+
wantErr: true,
1953+
},
1954+
{
1955+
name: "fail if selector targets BOTH an existing ControlPlane MachineInfrastructureTemplate and an existing MachineDeploymentClass InfrastructureTemplate but matches only one",
18411956
selector: clusterv1.PatchSelector{
18421957
APIVersion: "infrastructure.cluster.x-k8s.io/v1beta1",
18431958
Kind: "InfrastructureMachineTemplate",
18441959
MatchResources: clusterv1.PatchSelectorMatch{
18451960
MachineDeploymentClass: &clusterv1.PatchSelectorMatchMachineDeploymentClass{
18461961
Names: []string{"bb"},
18471962
},
1848-
ControlPlane: false,
1963+
ControlPlane: true,
1964+
},
1965+
},
1966+
clusterClass: builder.ClusterClass(metav1.NamespaceDefault, "class1").
1967+
WithControlPlaneTemplate(
1968+
refToUnstructured(
1969+
&corev1.ObjectReference{
1970+
APIVersion: "controlplane.cluster.x-k8s.io/v1beta1",
1971+
Kind: "NonMatchingControlPlaneTemplate",
1972+
}),
1973+
).
1974+
WithControlPlaneInfrastructureMachineTemplate(
1975+
refToUnstructured(
1976+
&corev1.ObjectReference{
1977+
APIVersion: "infrastructure.cluster.x-k8s.io/v1beta1",
1978+
Kind: "InfrastructureMachineTemplate",
1979+
}),
1980+
).
1981+
WithWorkerMachineDeploymentClasses(
1982+
*builder.MachineDeploymentClass("bb").
1983+
WithInfrastructureTemplate(
1984+
refToUnstructured(&corev1.ObjectReference{
1985+
APIVersion: "infrastructure.cluster.x-k8s.io/v1beta1",
1986+
Kind: "OtherInfrastructureMachineTemplate",
1987+
})).
1988+
WithBootstrapTemplate(
1989+
refToUnstructured(&corev1.ObjectReference{
1990+
APIVersion: "bootstrap.cluster.x-k8s.io/v1beta1",
1991+
Kind: "BootstrapTemplate",
1992+
})).
1993+
Build(),
1994+
).
1995+
Build(),
1996+
wantErr: true,
1997+
},
1998+
{
1999+
name: "fail if selector targets everything but nothing matches",
2000+
selector: clusterv1.PatchSelector{
2001+
APIVersion: "infrastructure.cluster.x-k8s.io/v1beta1",
2002+
Kind: "NotMatchingInfrastructureMachineTemplate",
2003+
MatchResources: clusterv1.PatchSelectorMatch{
2004+
ControlPlane: true,
2005+
InfrastructureCluster: true,
2006+
MachineDeploymentClass: &clusterv1.PatchSelectorMatchMachineDeploymentClass{
2007+
Names: []string{"bb"},
2008+
},
18492009
},
18502010
},
18512011
clusterClass: builder.ClusterClass(metav1.NamespaceDefault, "class1").
2012+
WithInfrastructureClusterTemplate(
2013+
refToUnstructured(
2014+
&corev1.ObjectReference{
2015+
APIVersion: "infrastructure.cluster.x-k8s.io/v1beta1",
2016+
Kind: "InfrastructureClusterTemplate",
2017+
}),
2018+
).
2019+
WithControlPlaneTemplate(
2020+
refToUnstructured(
2021+
&corev1.ObjectReference{
2022+
APIVersion: "controlplane.cluster.x-k8s.io/v1beta1",
2023+
Kind: "NonMatchingControlPlaneTemplate",
2024+
}),
2025+
).
18522026
WithControlPlaneInfrastructureMachineTemplate(
18532027
refToUnstructured(
18542028
&corev1.ObjectReference{
18552029
APIVersion: "infrastructure.cluster.x-k8s.io/v1beta1",
18562030
Kind: "InfrastructureMachineTemplate",
18572031
}),
18582032
).
2033+
WithWorkerMachineDeploymentClasses(
2034+
*builder.MachineDeploymentClass("bb").
2035+
WithInfrastructureTemplate(
2036+
refToUnstructured(&corev1.ObjectReference{
2037+
APIVersion: "infrastructure.cluster.x-k8s.io/v1beta1",
2038+
Kind: "OtherInfrastructureMachineTemplate",
2039+
})).
2040+
WithBootstrapTemplate(
2041+
refToUnstructured(&corev1.ObjectReference{
2042+
APIVersion: "bootstrap.cluster.x-k8s.io/v1beta1",
2043+
Kind: "BootstrapTemplate",
2044+
})).
2045+
Build(),
2046+
).
18592047
Build(),
18602048
wantErr: true,
18612049
},

0 commit comments

Comments
 (0)