Merge pull request #41995 from roidelapluie/41990-a

Automatic merge from submit-queue

Improvements to mustrunas_test.go

refs #41990

Signed-off-by: Julien Pivotto <roidelapluie@inuits.eu>



**What this PR does / why we need it**:

**Which issue this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close that issue when PR gets merged)*: fixes #41990

**Special notes for your reviewer**:

**Release note**:

```release-note
```

pkg/security/podsecuritypolicy/capabilities/mustrunas_test.go

@@ -25,14 +25,11 @@ import (
 
 func TestGenerateAdds(t *testing.T) {
 	tests := map[string]struct {
-		defaultAddCaps   []api.Capability
+		defaultAddCaps []api.Capability
-		requiredDropCaps []api.Capability
+		containerCaps  *api.Capabilities
-		containerCaps    *api.Capabilities
+		expectedCaps   *api.Capabilities
-		expectedCaps     *api.Capabilities
 	}{
-		"no required, no container requests": {
+		"no required, no container requests": {},
-			expectedCaps: nil,
-		},
 		"required, no container requests": {
 			defaultAddCaps: []api.Capability{"foo"},
 			expectedCaps: &api.Capabilities{
@@ -93,7 +90,7 @@ func TestGenerateAdds(t *testing.T) {
 			},
 		}
 
-		strategy, err := NewDefaultCapabilities(v.defaultAddCaps, v.requiredDropCaps, nil)
+		strategy, err := NewDefaultCapabilities(v.defaultAddCaps, nil, nil)
 		if err != nil {
 			t.Errorf("%s failed: %v", k, err)
 			continue
@@ -216,23 +213,19 @@ func TestGenerateDrops(t *testing.T) {
 
 func TestValidateAdds(t *testing.T) {
 	tests := map[string]struct {
-		defaultAddCaps   []api.Capability
+		defaultAddCaps []api.Capability
-		requiredDropCaps []api.Capability
+		allowedCaps    []api.Capability
-		allowedCaps      []api.Capability
+		containerCaps  *api.Capabilities
-		containerCaps    *api.Capabilities
+		expectedError  string
-		shouldPass       bool
 	}{
 		// no container requests
-		"no required, no allowed, no container requests": {
+		"no required, no allowed, no container requests": {},
-			shouldPass: true,
-		},
 		"no required, allowed, no container requests": {
 			allowedCaps: []api.Capability{"foo"},
-			shouldPass:  true,
 		},
 		"required, no allowed, no container requests": {
 			defaultAddCaps: []api.Capability{"foo"},
-			shouldPass:     false,
+			expectedError:  `capabilities: Invalid value: "null": required capabilities are not set on the securityContext`,
 		},
 
 		// container requests match required
@@ -241,14 +234,13 @@ func TestValidateAdds(t *testing.T) {
 			containerCaps: &api.Capabilities{
 				Add: []api.Capability{"foo"},
 			},
-			shouldPass: true,
 		},
 		"required, no allowed, container requests invalid": {
 			defaultAddCaps: []api.Capability{"foo"},
 			containerCaps: &api.Capabilities{
 				Add: []api.Capability{"bar"},
 			},
-			shouldPass: false,
+			expectedError: `capabilities.add: Invalid value: "bar": capability may not be added`,
 		},
 
 		// container requests match allowed
@@ -257,14 +249,13 @@ func TestValidateAdds(t *testing.T) {
 			containerCaps: &api.Capabilities{
 				Add: []api.Capability{"foo"},
 			},
-			shouldPass: true,
 		},
 		"no required, allowed, container requests invalid": {
 			allowedCaps: []api.Capability{"foo"},
 			containerCaps: &api.Capabilities{
 				Add: []api.Capability{"bar"},
 			},
-			shouldPass: false,
+			expectedError: `capabilities.add: Invalid value: "bar": capability may not be added`,
 		},
 
 		// required and allowed
@@ -274,7 +265,6 @@ func TestValidateAdds(t *testing.T) {
 			containerCaps: &api.Capabilities{
 				Add: []api.Capability{"foo"},
 			},
-			shouldPass: true,
 		},
 		"required, allowed, container requests valid allowed": {
 			defaultAddCaps: []api.Capability{"foo"},
@@ -282,7 +272,6 @@ func TestValidateAdds(t *testing.T) {
 			containerCaps: &api.Capabilities{
 				Add: []api.Capability{"bar"},
 			},
-			shouldPass: true,
 		},
 		"required, allowed, container requests invalid": {
 			defaultAddCaps: []api.Capability{"foo"},
@@ -290,14 +279,14 @@ func TestValidateAdds(t *testing.T) {
 			containerCaps: &api.Capabilities{
 				Add: []api.Capability{"baz"},
 			},
-			shouldPass: false,
+			expectedError: `capabilities.add: Invalid value: "baz": capability may not be added`,
 		},
 		"validation is case sensitive": {
 			defaultAddCaps: []api.Capability{"foo"},
 			containerCaps: &api.Capabilities{
 				Add: []api.Capability{"FOO"},
 			},
-			shouldPass: false,
+			expectedError: `capabilities.add: Invalid value: "FOO": capability may not be added`,
 		},
 	}
 
@@ -308,36 +297,41 @@ func TestValidateAdds(t *testing.T) {
 			},
 		}
 
-		strategy, err := NewDefaultCapabilities(v.defaultAddCaps, v.requiredDropCaps, v.allowedCaps)
+		strategy, err := NewDefaultCapabilities(v.defaultAddCaps, nil, v.allowedCaps)
 		if err != nil {
 			t.Errorf("%s failed: %v", k, err)
 			continue
 		}
 		errs := strategy.Validate(nil, container)
-		if v.shouldPass && len(errs) > 0 {
+		if v.expectedError == "" && len(errs) > 0 {
 			t.Errorf("%s should have passed but had errors %v", k, errs)
 			continue
 		}
-		if !v.shouldPass && len(errs) == 0 {
+		if v.expectedError != "" && len(errs) == 0 {
 			t.Errorf("%s should have failed but received no errors", k)
+			continue
+		}
+		if len(errs) == 1 && errs[0].Error() != v.expectedError {
+			t.Errorf("%s should have failed with %v but received %v", k, v.expectedError, errs[0])
+			continue
+		}
+		if len(errs) > 1 {
+			t.Errorf("%s should have failed with at most one error, but received %v: %v", k, len(errs), errs)
 		}
 	}
 }
 
 func TestValidateDrops(t *testing.T) {
 	tests := map[string]struct {
-		defaultAddCaps   []api.Capability
 		requiredDropCaps []api.Capability
 		containerCaps    *api.Capabilities
-		shouldPass       bool
+		expectedError    string
 	}{
 		// no container requests
-		"no required, no container requests": {
+		"no required, no container requests": {},
-			shouldPass: true,
-		},
 		"required, no container requests": {
 			requiredDropCaps: []api.Capability{"foo"},
-			shouldPass:       false,
+			expectedError:    `capabilities: Invalid value: "null": required capabilities are not set on the securityContext`,
 		},
 
 		// container requests match required
@@ -346,21 +340,20 @@ func TestValidateDrops(t *testing.T) {
 			containerCaps: &api.Capabilities{
 				Drop: []api.Capability{"foo"},
 			},
-			shouldPass: true,
 		},
 		"required, container requests invalid": {
 			requiredDropCaps: []api.Capability{"foo"},
 			containerCaps: &api.Capabilities{
 				Drop: []api.Capability{"bar"},
 			},
-			shouldPass: false,
+			expectedError: `capabilities.drop: Invalid value: []api.Capability{"bar"}: foo is required to be dropped but was not found`,
 		},
 		"validation is case sensitive": {
 			requiredDropCaps: []api.Capability{"foo"},
 			containerCaps: &api.Capabilities{
 				Drop: []api.Capability{"FOO"},
 			},
-			shouldPass: false,
+			expectedError: `capabilities.drop: Invalid value: []api.Capability{"FOO"}: foo is required to be dropped but was not found`,
 		},
 	}
 
@@ -371,18 +364,26 @@ func TestValidateDrops(t *testing.T) {
 			},
 		}
 
-		strategy, err := NewDefaultCapabilities(v.defaultAddCaps, v.requiredDropCaps, nil)
+		strategy, err := NewDefaultCapabilities(nil, v.requiredDropCaps, nil)
 		if err != nil {
 			t.Errorf("%s failed: %v", k, err)
 			continue
 		}
 		errs := strategy.Validate(nil, container)
-		if v.shouldPass && len(errs) > 0 {
+		if v.expectedError == "" && len(errs) > 0 {
 			t.Errorf("%s should have passed but had errors %v", k, errs)
 			continue
 		}
-		if !v.shouldPass && len(errs) == 0 {
+		if v.expectedError != "" && len(errs) == 0 {
 			t.Errorf("%s should have failed but received no errors", k)
+			continue
+		}
+		if len(errs) == 1 && errs[0].Error() != v.expectedError {
+			t.Errorf("%s should have failed with %v but received %v", k, v.expectedError, errs[0])
+			continue
+		}
+		if len(errs) > 1 {
+			t.Errorf("%s should have failed with at most one error, but received %v: %v", k, len(errs), errs)
 		}
 	}
 }