Merge pull request #60898 from lnsp/master

Automatic merge from submit-queue (batch tested with PRs 60898, 60912, 60753, 61002, 60796). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

Add ranges field to PSP group Validate error details

**What this PR does / why we need it**: When the PodSecurityPolicy provider validation of a `fsGroup` fails, it throws a very non-descriptive error that redundantly announces that the group ID is not allowed. Since the group ID is already contained in the error stack, this is not required. This PR fixes it by storing the allowed ranges in the error details and therefore improves the debugging experience.

**Which issue(s) this PR fixes**:
Fixes #60847 

**Release note**:

```release-note
NONE
```
pull/8/head
Kubernetes Submit Queue 2018-03-20 17:37:07 -07:00 committed by GitHub
commit d7c863e082
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 18 additions and 14 deletions

View File

@ -70,7 +70,7 @@ func (s *mustRunAs) Validate(_ *api.Pod, groups []int64) field.ErrorList {
for _, group := range groups { for _, group := range groups {
if !s.isGroupValid(group) { if !s.isGroupValid(group) {
detail := fmt.Sprintf("%d is not an allowed group", group) detail := fmt.Sprintf("group %d must be in the ranges: %v", group, s.ranges)
allErrs = append(allErrs, field.Invalid(field.NewPath(s.field), groups, detail)) allErrs = append(allErrs, field.Invalid(field.NewPath(s.field), groups, detail))
} }
} }

View File

@ -17,8 +17,10 @@ limitations under the License.
package group package group
import ( import (
"k8s.io/kubernetes/pkg/apis/extensions" "strings"
"testing" "testing"
"k8s.io/kubernetes/pkg/apis/extensions"
) )
func TestMustRunAsOptions(t *testing.T) { func TestMustRunAsOptions(t *testing.T) {
@ -110,17 +112,19 @@ func TestValidate(t *testing.T) {
tests := map[string]struct { tests := map[string]struct {
ranges []extensions.GroupIDRange ranges []extensions.GroupIDRange
groups []int64 groups []int64
pass bool expectedError string
}{ }{
"nil security context": { "nil security context": {
ranges: []extensions.GroupIDRange{ ranges: []extensions.GroupIDRange{
{Min: 1, Max: 3}, {Min: 1, Max: 3},
}, },
expectedError: "unable to validate empty groups against required ranges",
}, },
"empty groups": { "empty groups": {
ranges: []extensions.GroupIDRange{ ranges: []extensions.GroupIDRange{
{Min: 1, Max: 3}, {Min: 1, Max: 3},
}, },
expectedError: "unable to validate empty groups against required ranges",
}, },
"not in range": { "not in range": {
groups: []int64{5}, groups: []int64{5},
@ -128,34 +132,31 @@ func TestValidate(t *testing.T) {
{Min: 1, Max: 3}, {Min: 1, Max: 3},
{Min: 4, Max: 4}, {Min: 4, Max: 4},
}, },
expectedError: "group 5 must be in the ranges: [{1 3} {4 4}]",
}, },
"in range 1": { "in range 1": {
groups: []int64{2}, groups: []int64{2},
ranges: []extensions.GroupIDRange{ ranges: []extensions.GroupIDRange{
{Min: 1, Max: 3}, {Min: 1, Max: 3},
}, },
pass: true,
}, },
"in range boundary min": { "in range boundary min": {
groups: []int64{1}, groups: []int64{1},
ranges: []extensions.GroupIDRange{ ranges: []extensions.GroupIDRange{
{Min: 1, Max: 3}, {Min: 1, Max: 3},
}, },
pass: true,
}, },
"in range boundary max": { "in range boundary max": {
groups: []int64{3}, groups: []int64{3},
ranges: []extensions.GroupIDRange{ ranges: []extensions.GroupIDRange{
{Min: 1, Max: 3}, {Min: 1, Max: 3},
}, },
pass: true,
}, },
"singular range": { "singular range": {
groups: []int64{4}, groups: []int64{4},
ranges: []extensions.GroupIDRange{ ranges: []extensions.GroupIDRange{
{Min: 4, Max: 4}, {Min: 4, Max: 4},
}, },
pass: true,
}, },
} }
@ -165,11 +166,14 @@ func TestValidate(t *testing.T) {
t.Errorf("error creating strategy for %s: %v", k, err) t.Errorf("error creating strategy for %s: %v", k, err)
} }
errs := s.Validate(nil, v.groups) errs := s.Validate(nil, v.groups)
if v.pass && len(errs) > 0 { if v.expectedError == "" && len(errs) > 0 {
t.Errorf("unexpected errors for %s: %v", k, errs) t.Errorf("unexpected errors for %s: %v", k, errs)
} }
if !v.pass && len(errs) == 0 { if v.expectedError != "" && len(errs) == 0 {
t.Errorf("expected no errors for %s but got: %v", k, errs) t.Errorf("expected errors for %s but got: %v", k, errs)
}
if v.expectedError != "" && len(errs) > 0 && !strings.Contains(errs[0].Error(), v.expectedError) {
t.Errorf("expected error for %s: %v, but got: %v", k, v.expectedError, errs[0])
} }
} }
} }

View File

@ -291,7 +291,7 @@ func TestValidatePodSecurityContextFailures(t *testing.T) {
"failSupplementalGroupOutOfRange": { "failSupplementalGroupOutOfRange": {
pod: failSupplementalGroupPod, pod: failSupplementalGroupPod,
psp: failSupplementalGroupPSP, psp: failSupplementalGroupPSP,
expectedError: "999 is not an allowed group", expectedError: "group 999 must be in the ranges: [{1 1}]",
}, },
"failSupplementalGroupEmpty": { "failSupplementalGroupEmpty": {
pod: defaultPod(), pod: defaultPod(),
@ -301,7 +301,7 @@ func TestValidatePodSecurityContextFailures(t *testing.T) {
"failFSGroupOutOfRange": { "failFSGroupOutOfRange": {
pod: failFSGroupPod, pod: failFSGroupPod,
psp: failFSGroupPSP, psp: failFSGroupPSP,
expectedError: "999 is not an allowed group", expectedError: "group 999 must be in the ranges: [{1 1}]",
}, },
"failFSGroupEmpty": { "failFSGroupEmpty": {
pod: defaultPod(), pod: defaultPod(),