Merge pull request #51782 from charrywanganthony/audit-1

Automatic merge from submit-queue (batch tested with PRs 51900, 51782, 52030) A policy with 0 rules should return an error **Which issue this PR fixes** [isuue#51565](#51565) **Release note**: ``` An audit policy file with 0 rule returns an error. ```
kubernetes · Sep 8, 2017 · 4a72b32 · 4a72b32
2 parents 63d6bdb + 0ad4282
commit 4a72b32
Show file tree

Hide file tree

Showing 2 changed files with 46 additions and 18 deletions.
diff --git a/staging/src/k8s.io/apiserver/pkg/audit/policy/reader.go b/staging/src/k8s.io/apiserver/pkg/audit/policy/reader.go
@@ -49,6 +49,10 @@ func LoadPolicyFromFile(filePath string) (*auditinternal.Policy, error) {
 		return nil, err.ToAggregate()
 	}
 
-	glog.V(4).Infof("Loaded %d audit policy rules from file %s\n", len(policy.Rules), filePath)
+	policyCnt := len(policy.Rules)
+	if policyCnt == 0 {
+		return nil, fmt.Errorf("loaded illegal policy with 0 rules from file %s", filePath)
+	}
+	glog.V(4).Infof("Loaded %d audit policy rules from file %s", policyCnt, filePath)
 	return policy, nil
 }
diff --git a/staging/src/k8s.io/apiserver/pkg/audit/policy/reader_test.go b/staging/src/k8s.io/apiserver/pkg/audit/policy/reader_test.go
@@ -32,7 +32,7 @@ import (
 )
 
 const policyDefV1alpha1 = `
-apiVersion: audit.k8s.io/v1beta1
+apiVersion: audit.k8s.io/v1alpha1
 kind: Policy
 rules:
   - level: None
@@ -91,16 +91,11 @@ var expectedPolicy = &audit.Policy{
 }
 
 func TestParserV1alpha1(t *testing.T) {
-	// Create a policy file.
-	f, err := ioutil.TempFile("", "policy.yaml")
-	require.NoError(t, err)
-	defer os.Remove(f.Name())
-
-	_, err = f.WriteString(policyDefV1alpha1)
+	f, err := writePolicy(policyDefV1alpha1, t)
 	require.NoError(t, err)
-	require.NoError(t, f.Close())
+	defer os.Remove(f)
 
-	policy, err := LoadPolicyFromFile(f.Name())
+	policy, err := LoadPolicyFromFile(f)
 	require.NoError(t, err)
 
 	assert.Len(t, policy.Rules, 3) // Sanity check.
@@ -110,20 +105,49 @@ func TestParserV1alpha1(t *testing.T) {
 }
 
 func TestParserV1beta1(t *testing.T) {
-	// Create a policy file.
-	f, err := ioutil.TempFile("", "policy.yaml")
+	f, err := writePolicy(policyDefV1beta1, t)
 	require.NoError(t, err)
-	defer os.Remove(f.Name())
+	defer os.Remove(f)
 
-	_, err = f.WriteString(policyDefV1beta1)
-	require.NoError(t, err)
-	require.NoError(t, f.Close())
-
-	policy, err := LoadPolicyFromFile(f.Name())
+	policy, err := LoadPolicyFromFile(f)
 	require.NoError(t, err)
 
 	assert.Len(t, policy.Rules, 3) // Sanity check.
 	if !reflect.DeepEqual(policy, expectedPolicy) {
 		t.Errorf("Unexpected policy! Diff:\n%s", diff.ObjectDiff(policy, expectedPolicy))
 	}
 }
+
+func TestPolicyCntCheck(t *testing.T) {
+	//a set of testCases
+	var testCases = []struct {
+		caseName, policy string
+	}{
+		{
+			"policyWithNoRule",
+			`apiVersion: audit.k8s.io/v1beta1
+kind: Policy`,
+		},
+		{"emptyPolicyFile", ""},
+	}
+
+	for _, tc := range testCases {
+		f, err := writePolicy(tc.policy, t)
+		require.NoError(t, err)
+		defer os.Remove(f)
+
+		_, err = LoadPolicyFromFile(f)
+		assert.Errorf(t, err, "loaded illegal policy with 0 rules from testCase %s", tc.caseName)
+	}
+}
+
+func writePolicy(policy string, t *testing.T) (string, error) {
+	f, err := ioutil.TempFile("", "policy.yaml")
+	require.NoError(t, err)
+
+	_, err = f.WriteString(policy)
+	require.NoError(t, err)
+	require.NoError(t, f.Close())
+
+	return f.Name(), nil
+}