Add APIGroup ratcheting validation to PVC.DataSource

This commit is contained in:
carlory 2023-07-05 15:31:15 +08:00
parent 160fe010f3
commit 5fcffcf4e4
2 changed files with 144 additions and 4 deletions

View File

@ -2022,6 +2022,8 @@ type PersistentVolumeClaimSpecValidationOptions struct {
EnableRecoverFromExpansionFailure bool
// Allow to validate the label value of the label selector
AllowInvalidLabelValueInSelector bool
// Allow to validate the API group of the data source and data source reference
AllowInvalidAPIGroupInDataSourceOrRef bool
}
func ValidationOptionsForPersistentVolumeClaim(pvc, oldPvc *core.PersistentVolumeClaim) PersistentVolumeClaimSpecValidationOptions {
@ -2034,6 +2036,10 @@ func ValidationOptionsForPersistentVolumeClaim(pvc, oldPvc *core.PersistentVolum
// If there's no old PVC, use the options based solely on feature enablement
return opts
}
// If the old object had an invalid API group in the data source or data source reference, continue to allow it in the new object
opts.AllowInvalidAPIGroupInDataSourceOrRef = allowInvalidAPIGroupInDataSourceOrRef(&oldPvc.Spec)
labelSelectorValidationOpts := unversionedvalidation.LabelSelectorValidationOptions{
AllowInvalidLabelValueInSelector: opts.AllowInvalidLabelValueInSelector,
}
@ -2077,6 +2083,17 @@ func ValidationOptionsForPersistentVolumeClaimTemplate(claimTemplate, oldClaimTe
return opts
}
// allowInvalidAPIGroupInDataSourceOrRef returns true if the spec contains a data source or data source reference with an API group
func allowInvalidAPIGroupInDataSourceOrRef(spec *core.PersistentVolumeClaimSpec) bool {
if spec.DataSource != nil && spec.DataSource.APIGroup != nil {
return true
}
if spec.DataSourceRef != nil && spec.DataSourceRef.APIGroup != nil {
return true
}
return false
}
// ValidatePersistentVolumeClaim validates a PersistentVolumeClaim
func ValidatePersistentVolumeClaim(pvc *core.PersistentVolumeClaim, opts PersistentVolumeClaimSpecValidationOptions) field.ErrorList {
allErrs := ValidateObjectMeta(&pvc.ObjectMeta, true, ValidatePersistentVolumeName, field.NewPath("metadata"))
@ -2085,7 +2102,7 @@ func ValidatePersistentVolumeClaim(pvc *core.PersistentVolumeClaim, opts Persist
}
// validateDataSource validates a DataSource/DataSourceRef in a PersistentVolumeClaimSpec
func validateDataSource(dataSource *core.TypedLocalObjectReference, fldPath *field.Path) field.ErrorList {
func validateDataSource(dataSource *core.TypedLocalObjectReference, fldPath *field.Path, allowInvalidAPIGroupInDataSourceOrRef bool) field.ErrorList {
allErrs := field.ErrorList{}
if len(dataSource.Name) == 0 {
@ -2101,12 +2118,17 @@ func validateDataSource(dataSource *core.TypedLocalObjectReference, fldPath *fie
if len(apiGroup) == 0 && dataSource.Kind != "PersistentVolumeClaim" {
allErrs = append(allErrs, field.Invalid(fldPath, dataSource.Kind, "must be 'PersistentVolumeClaim' when referencing the default apiGroup"))
}
if len(apiGroup) > 0 && !allowInvalidAPIGroupInDataSourceOrRef {
for _, errString := range validation.IsDNS1123Subdomain(apiGroup) {
allErrs = append(allErrs, field.Invalid(fldPath.Child("apiGroup"), apiGroup, errString))
}
}
return allErrs
}
// validateDataSourceRef validates a DataSourceRef in a PersistentVolumeClaimSpec
func validateDataSourceRef(dataSourceRef *core.TypedObjectReference, fldPath *field.Path) field.ErrorList {
func validateDataSourceRef(dataSourceRef *core.TypedObjectReference, fldPath *field.Path, allowInvalidAPIGroupInDataSourceOrRef bool) field.ErrorList {
allErrs := field.ErrorList{}
if len(dataSourceRef.Name) == 0 {
@ -2122,6 +2144,11 @@ func validateDataSourceRef(dataSourceRef *core.TypedObjectReference, fldPath *fi
if len(apiGroup) == 0 && dataSourceRef.Kind != "PersistentVolumeClaim" {
allErrs = append(allErrs, field.Invalid(fldPath, dataSourceRef.Kind, "must be 'PersistentVolumeClaim' when referencing the default apiGroup"))
}
if len(apiGroup) > 0 && !allowInvalidAPIGroupInDataSourceOrRef {
for _, errString := range validation.IsDNS1123Subdomain(apiGroup) {
allErrs = append(allErrs, field.Invalid(fldPath.Child("apiGroup"), apiGroup, errString))
}
}
if dataSourceRef.Namespace != nil && len(*dataSourceRef.Namespace) > 0 {
for _, msg := range ValidateNameFunc(ValidateNamespaceName)(*dataSourceRef.Namespace, false) {
@ -2185,10 +2212,10 @@ func ValidatePersistentVolumeClaimSpec(spec *core.PersistentVolumeClaimSpec, fld
}
if spec.DataSource != nil {
allErrs = append(allErrs, validateDataSource(spec.DataSource, fldPath.Child("dataSource"))...)
allErrs = append(allErrs, validateDataSource(spec.DataSource, fldPath.Child("dataSource"), opts.AllowInvalidAPIGroupInDataSourceOrRef)...)
}
if spec.DataSourceRef != nil {
allErrs = append(allErrs, validateDataSourceRef(spec.DataSourceRef, fldPath.Child("dataSourceRef"))...)
allErrs = append(allErrs, validateDataSourceRef(spec.DataSourceRef, fldPath.Child("dataSourceRef"), opts.AllowInvalidAPIGroupInDataSourceOrRef)...)
}
if spec.DataSourceRef != nil && spec.DataSourceRef.Namespace != nil && len(*spec.DataSourceRef.Namespace) > 0 {
if spec.DataSource != nil {

View File

@ -996,6 +996,21 @@ func pvcTemplateWithAccessModes(accessModes []core.PersistentVolumeAccessMode) *
}
}
func pvcWithDataSource(dataSource *core.TypedLocalObjectReference) *core.PersistentVolumeClaim {
return &core.PersistentVolumeClaim{
Spec: core.PersistentVolumeClaimSpec{
DataSource: dataSource,
},
}
}
func pvcWithDataSourceRef(ref *core.TypedObjectReference) *core.PersistentVolumeClaim {
return &core.PersistentVolumeClaim{
Spec: core.PersistentVolumeClaimSpec{
DataSourceRef: ref,
},
}
}
func testLocalVolume(path string, affinity *core.VolumeNodeAffinity) core.PersistentVolumeSpec {
return core.PersistentVolumeSpec{
Capacity: core.ResourceList{
@ -1545,6 +1560,7 @@ func testVolumeClaimStorageClassInAnnotationAndNilInSpec(name, namespace, scName
func testValidatePVC(t *testing.T, ephemeral bool) {
invalidClassName := "-invalid-"
validClassName := "valid"
invalidAPIGroup := "^invalid"
invalidMode := core.PersistentVolumeMode("fakeVolumeMode")
validMode := core.PersistentVolumeFilesystem
goodName := "foo"
@ -1934,6 +1950,42 @@ func testValidatePVC(t *testing.T, ephemeral bool) {
},
}),
},
"invaild-apigroup-in-data-source": {
isExpectedFailure: true,
claim: testVolumeClaim(goodName, goodNS, core.PersistentVolumeClaimSpec{
AccessModes: []core.PersistentVolumeAccessMode{
core.ReadWriteOnce,
},
Resources: core.VolumeResourceRequirements{
Requests: core.ResourceList{
core.ResourceName(core.ResourceStorage): resource.MustParse("10G"),
},
},
DataSource: &core.TypedLocalObjectReference{
APIGroup: &invalidAPIGroup,
Kind: "Foo",
Name: "foo1",
},
}),
},
"invaild-apigroup-in-data-source-ref": {
isExpectedFailure: true,
claim: testVolumeClaim(goodName, goodNS, core.PersistentVolumeClaimSpec{
AccessModes: []core.PersistentVolumeAccessMode{
core.ReadWriteOnce,
},
Resources: core.VolumeResourceRequirements{
Requests: core.ResourceList{
core.ResourceName(core.ResourceStorage): resource.MustParse("10G"),
},
},
DataSourceRef: &core.TypedObjectReference{
APIGroup: &invalidAPIGroup,
Kind: "Foo",
Name: "foo1",
},
}),
},
}
for name, scenario := range scenarios {
@ -2057,6 +2109,7 @@ func TestAlphaPVVolumeModeUpdate(t *testing.T) {
func TestValidatePersistentVolumeClaimUpdate(t *testing.T) {
block := core.PersistentVolumeBlock
file := core.PersistentVolumeFilesystem
invaildAPIGroup := "^invalid"
validClaim := testVolumeClaimWithStatus("foo", "ns", core.PersistentVolumeClaimSpec{
AccessModes: []core.PersistentVolumeAccessMode{
@ -2427,6 +2480,42 @@ func TestValidatePersistentVolumeClaimUpdate(t *testing.T) {
},
})
invalidClaimDataSourceAPIGroup := testVolumeClaim("foo", "ns", core.PersistentVolumeClaimSpec{
AccessModes: []core.PersistentVolumeAccessMode{
core.ReadWriteOnce,
},
VolumeMode: &file,
Resources: core.VolumeResourceRequirements{
Requests: core.ResourceList{
core.ResourceName(core.ResourceStorage): resource.MustParse("10G"),
},
},
VolumeName: "volume",
DataSource: &core.TypedLocalObjectReference{
APIGroup: &invaildAPIGroup,
Kind: "Foo",
Name: "foo",
},
})
invalidClaimDataSourceRefAPIGroup := testVolumeClaim("foo", "ns", core.PersistentVolumeClaimSpec{
AccessModes: []core.PersistentVolumeAccessMode{
core.ReadWriteOnce,
},
VolumeMode: &file,
Resources: core.VolumeResourceRequirements{
Requests: core.ResourceList{
core.ResourceName(core.ResourceStorage): resource.MustParse("10G"),
},
},
VolumeName: "volume",
DataSourceRef: &core.TypedObjectReference{
APIGroup: &invaildAPIGroup,
Kind: "Foo",
Name: "foo",
},
})
scenarios := map[string]struct {
isExpectedFailure bool
oldClaim *core.PersistentVolumeClaim
@ -2631,6 +2720,16 @@ func TestValidatePersistentVolumeClaimUpdate(t *testing.T) {
enableRecoverFromExpansion: true,
isExpectedFailure: true,
},
"allow-update-pvc-when-data-source-used": {
oldClaim: invalidClaimDataSourceAPIGroup,
newClaim: invalidClaimDataSourceAPIGroup,
isExpectedFailure: false,
},
"allow-update-pvc-when-data-source-ref-used": {
oldClaim: invalidClaimDataSourceRefAPIGroup,
newClaim: invalidClaimDataSourceRefAPIGroup,
isExpectedFailure: false,
},
}
for name, scenario := range scenarios {
@ -2651,6 +2750,8 @@ func TestValidatePersistentVolumeClaimUpdate(t *testing.T) {
}
func TestValidationOptionsForPersistentVolumeClaim(t *testing.T) {
invaildAPIGroup := "^invalid"
tests := map[string]struct {
oldPvc *core.PersistentVolumeClaim
enableReadWriteOncePod bool
@ -2696,6 +2797,18 @@ func TestValidationOptionsForPersistentVolumeClaim(t *testing.T) {
EnableRecoverFromExpansionFailure: false,
},
},
"invaild apiGroup in dataSource allowed because the old pvc is used": {
oldPvc: pvcWithDataSource(&core.TypedLocalObjectReference{APIGroup: &invaildAPIGroup}),
expectValidationOpts: PersistentVolumeClaimSpecValidationOptions{
AllowInvalidAPIGroupInDataSourceOrRef: true,
},
},
"invaild apiGroup in dataSourceRef allowed because the old pvc is used": {
oldPvc: pvcWithDataSourceRef(&core.TypedObjectReference{APIGroup: &invaildAPIGroup}),
expectValidationOpts: PersistentVolumeClaimSpecValidationOptions{
AllowInvalidAPIGroupInDataSourceOrRef: true,
},
},
}
for name, tc := range tests {