Skip to content

Commit

Permalink
rm DryRun options
Browse files Browse the repository at this point in the history
  • Loading branch information
p0lyn0mial committed Aug 30, 2019
1 parent 1825e53 commit 7b50f4a
Show file tree
Hide file tree
Showing 2 changed files with 135 additions and 2 deletions.
23 changes: 21 additions & 2 deletions pkg/securitycontextconstraints/sccadmission/scc_exec.go
Expand Up @@ -3,8 +3,8 @@ package sccadmission
import (
"fmt"
"io"

metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apiserver/pkg/admission"
"k8s.io/apiserver/pkg/admission/initializer"
"k8s.io/apiserver/pkg/authorization/authorizer"
Expand Down Expand Up @@ -61,7 +61,7 @@ func (d *sccExecRestrictions) Validate(a admission.Attributes, o admission.Objec

// TODO, if we want to actually limit who can use which service account, then we'll need to add logic here to make sure that
// we're allowed to use the SA the pod is using. Otherwise, user-A creates pod and user-B (who can't use the SA) can exec into it.
createAttributes := admission.NewAttributesRecord(internalPod, nil, coreapi.Kind("Pod").WithVersion(""), a.GetNamespace(), a.GetName(), a.GetResource(), "", admission.Create, a.GetOperationOptions(), false, a.GetUserInfo())
createAttributes := admission.NewAttributesRecord(internalPod, nil, coreapi.Kind("Pod").WithVersion(""), a.GetNamespace(), a.GetName(), a.GetResource(), "", admission.Create, removeDryRunOptionsFromOperationOptions(a.GetOperationOptions()), false, a.GetUserInfo())
// call SCC.Admit instead of SCC.Validate because we accept that a different SCC is chosen. SCC.Validate would require
// that the chosen SCC (stored in the "openshift.io/scc" annotation) does not change.
if err := d.constraintAdmission.Admit(createAttributes, o); err != nil {
Expand Down Expand Up @@ -96,3 +96,22 @@ func (d *sccExecRestrictions) SetAuthorizer(authorizer authorizer.Authorizer) {
func (d *sccExecRestrictions) ValidateInitialization() error {
return d.constraintAdmission.ValidateInitialization()
}

// as a security precaution, we remove DryRun options from the operation options
func removeDryRunOptionsFromOperationOptions(rawOptions runtime.Object) runtime.Object {
if rawOptions == nil {
return nil
}

switch options := rawOptions.(type) {
case *metav1.CreateOptions:
o := options.DeepCopy()
o.DryRun = []string{}
return o
case *metav1.DeleteOptions:
o := options.DeepCopy()
o.DryRun = []string{}
return o
}
return rawOptions
}
114 changes: 114 additions & 0 deletions pkg/securitycontextconstraints/sccadmission/scc_exec_test.go
Expand Up @@ -4,8 +4,10 @@ import (
"testing"

corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/equality"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/util/diff"
"k8s.io/apiserver/pkg/admission"
"k8s.io/apiserver/pkg/authentication/user"
"k8s.io/client-go/kubernetes/fake"
Expand Down Expand Up @@ -120,3 +122,115 @@ func TestExecAdmit(t *testing.T) {
}
}
}

func TestRemoveDryRunOptionsFromOperationOptions(t *testing.T) {
scenarios := []struct {
name string
input runtime.Object
validateFunc func(ts *testing.T, inputCpy, rawInput, rawOutput runtime.Object)
}{
{
name: "checks if DryRun options are removed from CreateOptions",
input: &metav1.CreateOptions{
TypeMeta: metav1.TypeMeta{},
DryRun: []string{"one", "two"},
FieldManager: "some.field.path",
},
validateFunc: func(ts *testing.T, inputCpy, rawInput, rawOutput runtime.Object) {
input, ok := rawInput.(*metav1.CreateOptions)
if !ok {
ts.Fatalf("rawInput has incorrect type %T, expected *metav1.CreateOptions", rawInput)
}
if !equality.Semantic.DeepEqual(inputCpy, input) {
ts.Errorf("original object was modified, diff = %v", diff.ObjectDiff(inputCpy, input))
}

output, ok := rawOutput.(*metav1.CreateOptions)
if !ok {
ts.Fatalf("rawOutput has incorrect type %T, expected *metav1.CreateOptions", rawOutput)
}
input.DryRun = []string{}
if !equality.Semantic.DeepEqual(input, output) {
ts.Errorf(diff.ObjectDiff(input, output))
}
},
},

{
name: "checks if DryRun options are removed from DeleteOptions",
input: &metav1.DeleteOptions{
TypeMeta: metav1.TypeMeta{},
DryRun: []string{"one", "two"},
GracePeriodSeconds: func() *int64 {
number := int64(5)
return &number
}(),
},
validateFunc: func(ts *testing.T, inputCpy, rawInput, rawOutput runtime.Object) {
input, ok := rawInput.(*metav1.DeleteOptions)
if !ok {
ts.Fatalf("rawInput has incorrect type %T, expected *metav1.DeleteOptions", rawInput)
}
if !equality.Semantic.DeepEqual(inputCpy, input) {
ts.Errorf("original object was modified, diff = %v", diff.ObjectDiff(inputCpy, input))
}

output, ok := rawOutput.(*metav1.DeleteOptions)
if !ok {
ts.Fatalf("rawOutput has incorrect type %T, expected *metav1.DeleteOptions", rawOutput)
}
input.DryRun = []string{}
if !equality.Semantic.DeepEqual(input, output) {
ts.Errorf(diff.ObjectDiff(input, output))
}
},
},

{
name: "no-op for UpdateOptions",
input: &metav1.UpdateOptions{
TypeMeta: metav1.TypeMeta{},
DryRun: []string{"one", "two"},
FieldManager: "some.field.path",
},
validateFunc: func(ts *testing.T, inputCpy, rawInput, rawOutput runtime.Object) {
input, ok := rawInput.(*metav1.UpdateOptions)
if !ok {
ts.Fatalf("rawInput has incorrect type %T, expected *metav1.UpdateOptions", rawInput)
}
if !equality.Semantic.DeepEqual(inputCpy, input) {
ts.Errorf("original object was modified, diff = %v", diff.ObjectDiff(inputCpy, input))
}

output, ok := rawOutput.(*metav1.UpdateOptions)
if !ok {
ts.Fatalf("rawOutput has incorrect type %T, expected *metav1.UpdateOptions", rawOutput)
}
if !equality.Semantic.DeepEqual(input, output) {
ts.Errorf(diff.ObjectDiff(input, output))
}
},
},

{
name: "checks if passing nil doesn't blow up",
input: nil,
validateFunc: func(ts *testing.T, inputCpy, rawInput, rawOutput runtime.Object) {
if inputCpy != nil || rawInput != nil || rawOutput != nil {
ts.Fatalf("something went wrong, inputCpy, rawInput and rawOutput shouldn't be nil")

}
},
},
}
for _, scenario := range scenarios {
t.Run(scenario.name, func(t *testing.T) {
var inputCpy runtime.Object
if scenario.input != nil {
inputCpy = scenario.input.DeepCopyObject()
}
updatedRawOptions := removeDryRunOptionsFromOperationOptions(scenario.input)
scenario.validateFunc(t, inputCpy, scenario.input, updatedRawOptions)
})
}
}

0 comments on commit 7b50f4a

Please sign in to comment.