Merge pull request #128637 from jpbetz/fix-mutating-admission-defaulting

Bug fix: MutatingAdmissionPolicy should default builtin types after each mutation
This commit is contained in:
Kubernetes Prow Robot 2024-11-07 12:33:30 +00:00 committed by GitHub
commit 9729ac8c6f
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 53 additions and 1 deletions

View File

@ -27,6 +27,7 @@ import (
k8serrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/api/meta"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/apimachinery/pkg/types"
"k8s.io/apiserver/pkg/admission"
@ -249,9 +250,20 @@ func (d *dispatcher) dispatchOne(
return err
}
switch versionedAttributes.VersionedObject.(type) {
case *unstructured.Unstructured:
// No conversion needed before defaulting for the patch object if the admitted object is unstructured.
default:
// Before defaulting, if the admitted object is a typed object, convert unstructured patch result back to a typed object.
newVersionedObject, err = o.GetObjectConvertor().ConvertToVersion(newVersionedObject, versionedAttributes.GetKind().GroupVersion())
if err != nil {
return err
}
}
o.GetObjectDefaulter().Default(newVersionedObject)
versionedAttributes.Dirty = true
versionedAttributes.VersionedObject = newVersionedObject
o.GetObjectDefaulter().Default(newVersionedObject)
return nil
}

View File

@ -122,6 +122,9 @@ func TestDispatcher(t *testing.T) {
Volumes: []corev1.Volume{{Name: "x"}},
},
},
Strategy: appsv1.DeploymentStrategy{
Type: appsv1.RollingUpdateDeploymentStrategyType,
},
}},
},
{
@ -144,6 +147,9 @@ func TestDispatcher(t *testing.T) {
Volumes: []corev1.Volume{{Name: "x"}},
},
},
Strategy: appsv1.DeploymentStrategy{
Type: appsv1.RollingUpdateDeploymentStrategyType,
},
}},
params: []runtime.Object{
&corev1.ConfigMap{
@ -212,6 +218,9 @@ func TestDispatcher(t *testing.T) {
Volumes: []corev1.Volume{{Name: "x"}},
},
},
Strategy: appsv1.DeploymentStrategy{
Type: appsv1.RollingUpdateDeploymentStrategyType,
},
}},
},
{
@ -233,6 +242,9 @@ func TestDispatcher(t *testing.T) {
Volumes: []corev1.Volume{{Name: "x"}},
},
},
Strategy: appsv1.DeploymentStrategy{
Type: appsv1.RollingUpdateDeploymentStrategyType,
},
}},
policyHooks: []generic.PolicyHook[*Policy, *PolicyBinding, PolicyEvaluator]{
{
@ -317,6 +329,9 @@ func TestDispatcher(t *testing.T) {
Volumes: []corev1.Volume{{Name: "x"}},
},
},
Strategy: appsv1.DeploymentStrategy{
Type: appsv1.RollingUpdateDeploymentStrategyType,
},
}},
},
{
@ -338,6 +353,9 @@ func TestDispatcher(t *testing.T) {
Volumes: []corev1.Volume{{Name: "x"}},
},
},
Strategy: appsv1.DeploymentStrategy{
Type: appsv1.RollingUpdateDeploymentStrategyType,
},
}},
policyHooks: []generic.PolicyHook[*Policy, *PolicyBinding, PolicyEvaluator]{
{
@ -444,6 +462,9 @@ func TestDispatcher(t *testing.T) {
Volumes: []corev1.Volume{{Name: "x"}},
},
},
Strategy: appsv1.DeploymentStrategy{
Type: appsv1.RollingUpdateDeploymentStrategyType,
},
}},
},
{
@ -466,6 +487,9 @@ func TestDispatcher(t *testing.T) {
Volumes: []corev1.Volume{{Name: "x"}},
},
},
Strategy: appsv1.DeploymentStrategy{
Type: appsv1.RollingUpdateDeploymentStrategyType,
},
}},
policyHooks: []generic.PolicyHook[*Policy, *PolicyBinding, PolicyEvaluator]{
{
@ -571,6 +595,9 @@ func TestDispatcher(t *testing.T) {
Volumes: []corev1.Volume{{Name: "x"}},
},
},
Strategy: appsv1.DeploymentStrategy{
Type: appsv1.RollingUpdateDeploymentStrategyType,
},
}},
},
}
@ -597,6 +624,11 @@ func TestDispatcher(t *testing.T) {
t.Fatal(err)
}
// Register a fake defaulter since registering the full defaulter adds noise
// and creates dep cycles.
scheme.AddTypeDefaultingFunc(&appsv1.Deployment{},
func(obj interface{}) { fakeSetDefaultForDeployment(obj.(*appsv1.Deployment)) })
objectInterfaces := admission.NewObjectInterfacesFromScheme(scheme)
for _, tc := range testCases {
@ -673,3 +705,11 @@ func (t testParamScope) Name() meta.RESTScopeName {
}
var _ meta.RESTScope = testParamScope{}
func fakeSetDefaultForDeployment(obj *appsv1.Deployment) {
// Just default strategy type so the tests have a defaulted field to observe
strategy := &obj.Spec.Strategy
if strategy.Type == "" {
strategy.Type = appsv1.RollingUpdateDeploymentStrategyType
}
}