diff --git a/staging/src/k8s.io/apiserver/pkg/apis/audit/types.go b/staging/src/k8s.io/apiserver/pkg/apis/audit/types.go index cfea0550661..d6d15adb583 100644 --- a/staging/src/k8s.io/apiserver/pkg/apis/audit/types.go +++ b/staging/src/k8s.io/apiserver/pkg/apis/audit/types.go @@ -215,10 +215,17 @@ type GroupResources struct { // The empty string represents the core API group. // +optional Group string - // Resources is a list of resources within the API group. - // Any empty list implies every resource kind in the API group. + // Resources is a list of resources within the API group. Subresources are + // matched using a "/" to indicate the subresource. For example, "pods/logs" + // would match request to the logs subresource of pods. The top level resource + // does not match subresources, "pods" doesn't match "pods/logs". // +optional Resources []string + // ResourceNames is a list of resource instance names that the policy matches. + // Using this field requires Resources to be specified. + // An empty list implies that every instance of the resource is matched. + // +optional + ResourceNames []string } // ObjectReference contains enough information to let you inspect or modify the referred object. diff --git a/staging/src/k8s.io/apiserver/pkg/apis/audit/v1alpha1/types.go b/staging/src/k8s.io/apiserver/pkg/apis/audit/v1alpha1/types.go index e2022656828..9eaaa111136 100644 --- a/staging/src/k8s.io/apiserver/pkg/apis/audit/v1alpha1/types.go +++ b/staging/src/k8s.io/apiserver/pkg/apis/audit/v1alpha1/types.go @@ -216,10 +216,17 @@ type GroupResources struct { // The empty string represents the core API group. // +optional Group string `json:"group,omitempty" protobuf:"bytes,1,opt,name=group"` - // Resources is a list of resources within the API group. - // Any empty list implies every resource kind in the API group. + // Resources is a list of resources within the API group. Subresources are + // matched using a "/" to indicate the subresource. For example, "pods/logs" + // would match request to the logs subresource of pods. The top level resource + // does not match subresources, "pods" doesn't match "pods/logs". // +optional Resources []string `json:"resources,omitempty" protobuf:"bytes,2,rep,name=resources"` + // ResourceNames is a list of resource instance names that the policy matches. + // Using this field requires Resources to be specified. + // An empty list implies that every instance of the resource is matched. + // +optional + ResourceNames []string `json:"resourceNames,omitempty" protobuf:"bytes,3,rep,name=resourceNames"` } // ObjectReference contains enough information to let you inspect or modify the referred object. diff --git a/staging/src/k8s.io/apiserver/pkg/apis/audit/v1beta1/types.go b/staging/src/k8s.io/apiserver/pkg/apis/audit/v1beta1/types.go index 1ecac858854..b60b78ec49f 100644 --- a/staging/src/k8s.io/apiserver/pkg/apis/audit/v1beta1/types.go +++ b/staging/src/k8s.io/apiserver/pkg/apis/audit/v1beta1/types.go @@ -209,10 +209,17 @@ type GroupResources struct { // The empty string represents the core API group. // +optional Group string `json:"group,omitempty" protobuf:"bytes,1,opt,name=group"` - // Resources is a list of resources within the API group. - // Any empty list implies every resource kind in the API group. + // Resources is a list of resources within the API group. Subresources are + // matched using a "/" to indicate the subresource. For example, "pods/logs" + // would match request to the logs subresource of pods. The top level resource + // does not match subresources, "pods" doesn't match "pods/logs". // +optional Resources []string `json:"resources,omitempty" protobuf:"bytes,2,rep,name=resources"` + // ResourceNames is a list of resource instance names that the policy matches. + // Using this field requires Resources to be specified. + // An empty list implies that every instance of the resource is matched. + // +optional + ResourceNames []string `json:"resourceNames,omitempty" protobuf:"bytes,3,rep,name=resourceNames"` } // ObjectReference contains enough information to let you inspect or modify the referred object. diff --git a/staging/src/k8s.io/apiserver/pkg/apis/audit/validation/validation.go b/staging/src/k8s.io/apiserver/pkg/apis/audit/validation/validation.go index 80b73f851cb..0db2030433a 100644 --- a/staging/src/k8s.io/apiserver/pkg/apis/audit/validation/validation.go +++ b/staging/src/k8s.io/apiserver/pkg/apis/audit/validation/validation.go @@ -88,16 +88,18 @@ func validateResources(groupResources []audit.GroupResources, fldPath *field.Pat var allErrs field.ErrorList for _, groupResource := range groupResources { // The empty string represents the core API group. - if len(groupResource.Group) == 0 { - continue + if len(groupResource.Group) != 0 { + // Group names must be lower case and be valid DNS subdomains. + // reference: https://github.com/kubernetes/community/blob/master/contributors/devel/api-conventions.md + // an error is returned for group name like rbac.authorization.k8s.io/v1beta1 + // rbac.authorization.k8s.io is the valid one + if msgs := validation.NameIsDNSSubdomain(groupResource.Group, false); len(msgs) != 0 { + allErrs = append(allErrs, field.Invalid(fldPath.Child("group"), groupResource.Group, strings.Join(msgs, ","))) + } } - // Group names must be lower case and be valid DNS subdomains. - // reference: https://github.com/kubernetes/community/blob/master/contributors/devel/api-conventions.md - // an error is returned for group name like rbac.authorization.k8s.io/v1beta1 - // rbac.authorization.k8s.io is the valid one - if msgs := validation.NameIsDNSSubdomain(groupResource.Group, false); len(msgs) != 0 { - allErrs = append(allErrs, field.Invalid(fldPath.Child("group"), groupResource.Group, strings.Join(msgs, ","))) + if len(groupResource.ResourceNames) > 0 && len(groupResource.Resources) == 0 { + allErrs = append(allErrs, field.Invalid(fldPath.Child("resourceNames"), groupResource.ResourceNames, "using resourceNames requires at least one resource")) } } return allErrs diff --git a/staging/src/k8s.io/apiserver/pkg/apis/audit/validation/validation_test.go b/staging/src/k8s.io/apiserver/pkg/apis/audit/validation/validation_test.go index 99692157791..3acb9598bd0 100644 --- a/staging/src/k8s.io/apiserver/pkg/apis/audit/validation/validation_test.go +++ b/staging/src/k8s.io/apiserver/pkg/apis/audit/validation/validation_test.go @@ -102,6 +102,12 @@ func TestValidatePolicy(t *testing.T) { "/metrics", }, }, + { // ResourceNames without Resources + Level: audit.LevelMetadata, + Verbs: []string{"get"}, + Resources: []audit.GroupResources{{ResourceNames: []string{"leader"}}}, + Namespaces: []string{"kube-system"}, + }, } errorCases := []audit.Policy{} for _, rule := range invalidRules { diff --git a/staging/src/k8s.io/apiserver/pkg/audit/policy/checker.go b/staging/src/k8s.io/apiserver/pkg/audit/policy/checker.go index 526710c2c6f..8ebe69ca136 100644 --- a/staging/src/k8s.io/apiserver/pkg/audit/policy/checker.go +++ b/staging/src/k8s.io/apiserver/pkg/audit/policy/checker.go @@ -143,6 +143,15 @@ func ruleMatchesResource(r *audit.PolicyRule, attrs authorizer.Attributes) bool apiGroup := attrs.GetAPIGroup() resource := attrs.GetResource() + // If subresource, the resource in the policy must match "(resource)/(subresource)" + // + // TODO: consider adding options like "pods/*" to match all subresources. + if sr := attrs.GetSubresource(); sr != "" { + resource = resource + "/" + sr + } + + name := attrs.GetName() + for _, gr := range r.Resources { if gr.Group == apiGroup { if len(gr.Resources) == 0 { @@ -150,7 +159,9 @@ func ruleMatchesResource(r *audit.PolicyRule, attrs authorizer.Attributes) bool } for _, res := range gr.Resources { if res == resource { - return true + if len(gr.ResourceNames) == 0 || hasString(gr.ResourceNames, name) { + return true + } } } } diff --git a/staging/src/k8s.io/apiserver/pkg/audit/policy/checker_test.go b/staging/src/k8s.io/apiserver/pkg/audit/policy/checker_test.go index 29cf1a4689a..015e23beb7e 100644 --- a/staging/src/k8s.io/apiserver/pkg/audit/policy/checker_test.go +++ b/staging/src/k8s.io/apiserver/pkg/audit/policy/checker_test.go @@ -61,6 +61,18 @@ func TestChecker(t *testing.T) { ResourceRequest: false, Path: "/logs/kubelet.log", }, + "subresource": &authorizer.AttributesRecord{ + User: tim, + Verb: "get", + Namespace: "default", + APIGroup: "", // Core + APIVersion: "v1", + Resource: "pods", + Subresource: "log", + Name: "busybox", + ResourceRequest: true, + Path: "/api/v1/namespaces/default/pods/busybox", + }, } rules := map[string]audit.PolicyRule{ @@ -88,6 +100,11 @@ func TestChecker(t *testing.T) { Verbs: []string{"get"}, Resources: []audit.GroupResources{{Resources: []string{"pods"}}}, }, + "getPodLogs": { + Level: audit.LevelRequest, + Verbs: []string{"get"}, + Resources: []audit.GroupResources{{Resources: []string{"pods/log"}}}, + }, "getClusterRoles": { Level: audit.LevelRequestResponse, Verbs: []string{"get"}, @@ -111,6 +128,14 @@ func TestChecker(t *testing.T) { "/metrics", }, }, + "clusterRoleEdit": { + Level: audit.LevelRequest, + Resources: []audit.GroupResources{{ + Group: "rbac.authorization.k8s.io", + Resources: []string{"clusterroles"}, + ResourceNames: []string{"edit"}, + }}, + }, } test := func(req string, expected audit.Level, ruleNames ...string) { @@ -135,6 +160,7 @@ func TestChecker(t *testing.T) { test("namespaced", audit.LevelNone, "getMetrics") test("namespaced", audit.LevelMetadata, "getMetrics", "serviceAccounts", "default") test("namespaced", audit.LevelRequestResponse, "getMetrics", "getPods", "default") + test("namespaced", audit.LevelRequestResponse, "getPodLogs", "getPods") test("cluster", audit.LevelMetadata, "default") test("cluster", audit.LevelNone, "create") @@ -143,10 +169,12 @@ func TestChecker(t *testing.T) { test("cluster", audit.LevelNone, "serviceAccounts") test("cluster", audit.LevelNone, "getPods") test("cluster", audit.LevelRequestResponse, "getClusterRoles") + test("cluster", audit.LevelRequest, "clusterRoleEdit", "getClusterRoles") test("cluster", audit.LevelNone, "getLogs") test("cluster", audit.LevelNone, "getMetrics") test("cluster", audit.LevelMetadata, "getMetrics", "serviceAccounts", "default") test("cluster", audit.LevelRequestResponse, "getMetrics", "getClusterRoles", "default") + test("cluster", audit.LevelNone, "getPodLogs", "getPods") test("nonResource", audit.LevelMetadata, "default") test("nonResource", audit.LevelNone, "create") @@ -159,4 +187,8 @@ func TestChecker(t *testing.T) { test("nonResource", audit.LevelNone, "getMetrics") test("nonResource", audit.LevelMetadata, "getMetrics", "serviceAccounts", "default") test("nonResource", audit.LevelRequestResponse, "getLogs", "getClusterRoles", "default") + test("nonResource", audit.LevelNone, "getPodLogs", "getPods") + + test("subresource", audit.LevelRequest, "getPodLogs", "getPods") + test("subresource", audit.LevelRequest, "getPods", "getPodLogs") }