[PodSecurity] Metrics improvements #105898

tallclair · 2021-10-26T07:31:49Z

What type of PR is this?

/kind bug

What this PR does / why we need it:

Fix up current metrics:
- Register metrics on the global handler
- Expose a /metrics endpoint on the webhook
Make metrics conform to the spec defined in https://github.com/kubernetes/enhancements/blob/master/keps/sig-auth/2579-psp-replacement/README.md#monitoring:
- operation -> request_operation for the label name
- resource should be either pod or controller (see comment on below)
- operation should be lower case
Add test cases
Add the error & exemption metrics

Which issue(s) this PR fixes:

Special notes for your reviewer:

Question 1: Do we want to consider the short-circuit case an evaluation, for monitoring purposes? doing so adds a second alloc in the common path, and doubles the evaluation time. See benchmarks below.

Question 2: I was corrected in the docs PR (kubernetes/website#29124 (review), @sftim ) that we should refer to the "pod template resources" as workload resources. Do we want to update the resource label on the metric to match (controller -> workload)?

Benchmarks:

# Original (without this PR):

BenchmarkVerifyPod/enforce-implicit_pod-16         	 4421935	       266.8 ns/op	     112 B/op	       1 allocs/op
BenchmarkVerifyPod/enforce-implicit_deployment-16  	 5054050	       267.9 ns/op	     112 B/op	       1 allocs/op
BenchmarkVerifyPod/enforce-privileged_pod-16       	 5110650	       276.4 ns/op	     112 B/op	       1 allocs/op
BenchmarkVerifyPod/enforce-privileged_deployment-16         	 4437080	       280.7 ns/op	     112 B/op	       1 allocs/op
BenchmarkVerifyPod/enforce-baseline_pod-16                  	  315181	      4174 ns/op	    3504 B/op	      19 allocs/op
BenchmarkVerifyPod/enforce-baseline_deployment-16           	 3700531	       303.5 ns/op	     112 B/op	       1 allocs/op
BenchmarkVerifyPod/enforce-restricted_pod-16                	  292236	      4702 ns/op	    3504 B/op	      19 allocs/op
BenchmarkVerifyPod/enforce-restricted_deployment-16         	 4413544	       299.9 ns/op	     112 B/op	       1 allocs/op
BenchmarkVerifyPod/warn-baseline_pod-16                     	  274810	      4191 ns/op	    3504 B/op	      19 allocs/op
BenchmarkVerifyPod/warn-baseline_deployment-16              	  304512	      3667 ns/op	    3552 B/op	      19 allocs/op
BenchmarkVerifyPod/warn-restricted_pod-16                   	  283743	      4435 ns/op	    3504 B/op	      19 allocs/op
BenchmarkVerifyPod/warn-restricted_deployment-16            	  263958	      3974 ns/op	    3552 B/op	      19 allocs/op
BenchmarkVerifyPod/enforce-warn-audit-baseline_pod-16       	  187177	      6975 ns/op	    6000 B/op	      29 allocs/op
BenchmarkVerifyPod/enforce-warn-audit-baseline_deployment-16         	  256402	      5064 ns/op	    4800 B/op	      24 allocs/op
BenchmarkVerifyPod/warn-baseline-audit-restricted_pod-16             	  204177	      5673 ns/op	    4752 B/op	      24 allocs/op
BenchmarkVerifyPod/warn-baseline-audit-restricted_deployment-16      	  226470	      5445 ns/op	    4800 B/op	      24 allocs/op

## With this PR

BenchmarkVerifyPod/enforce-implicit_pod-16         	 1946442	       588.9 ns/op	     224 B/op	       2 allocs/op
BenchmarkVerifyPod/enforce-implicit_deployment-16  	 4798802	       256.9 ns/op	     112 B/op	       1 allocs/op
BenchmarkVerifyPod/enforce-privileged_pod-16       	 1862803	       626.7 ns/op	     224 B/op	       2 allocs/op
BenchmarkVerifyPod/enforce-privileged_deployment-16         	 4398561	       283.9 ns/op	     112 B/op	       1 allocs/op
BenchmarkVerifyPod/enforce-baseline_pod-16                  	  301028	      4009 ns/op	    3480 B/op	      18 allocs/op
BenchmarkVerifyPod/enforce-baseline_deployment-16           	 4139908	       284.3 ns/op	     112 B/op	       1 allocs/op
BenchmarkVerifyPod/enforce-restricted_pod-16                	  297513	      4582 ns/op	    3480 B/op	      18 allocs/op
BenchmarkVerifyPod/enforce-restricted_deployment-16         	 3554356	       309.7 ns/op	     112 B/op	       1 allocs/op
BenchmarkVerifyPod/warn-baseline_pod-16                     	  295238	      4026 ns/op	    3480 B/op	      18 allocs/op
BenchmarkVerifyPod/warn-baseline_deployment-16              	  294944	      3670 ns/op	    3552 B/op	      19 allocs/op
BenchmarkVerifyPod/warn-restricted_pod-16                   	  298869	      4262 ns/op	    3480 B/op	      18 allocs/op
BenchmarkVerifyPod/warn-restricted_deployment-16            	  333310	      3916 ns/op	    3552 B/op	      19 allocs/op
BenchmarkVerifyPod/enforce-warn-audit-baseline_pod-16       	  166224	      6843 ns/op	    5976 B/op	      28 allocs/op
BenchmarkVerifyPod/enforce-warn-audit-baseline_deployment-16         	  257518	      4938 ns/op	    4800 B/op	      24 allocs/op
BenchmarkVerifyPod/warn-baseline-audit-restricted_pod-16             	  219632	      5800 ns/op	    4728 B/op	      23 allocs/op
BenchmarkVerifyPod/warn-baseline-audit-restricted_deployment-16      	  231926	      5616 ns/op	    4800 B/op	      24 allocs/op

## With this PR, but dropping the short-circuit eval count:

BenchmarkVerifyPod/enforce-implicit_pod-16         	 4651302	       247.4 ns/op	     112 B/op	       1 allocs/op
BenchmarkVerifyPod/enforce-implicit_deployment-16  	 4379590	       265.1 ns/op	     112 B/op	       1 allocs/op
BenchmarkVerifyPod/enforce-privileged_pod-16       	 4283446	       273.9 ns/op	     112 B/op	       1 allocs/op
BenchmarkVerifyPod/enforce-privileged_deployment-16         	 4164979	       282.0 ns/op	     112 B/op	       1 allocs/op
BenchmarkVerifyPod/enforce-baseline_pod-16                  	  307122	      3885 ns/op	    3480 B/op	      18 allocs/op
BenchmarkVerifyPod/enforce-baseline_deployment-16           	 4469337	       289.3 ns/op	     112 B/op	       1 allocs/op
BenchmarkVerifyPod/enforce-restricted_pod-16                	  300816	      4306 ns/op	    3480 B/op	      18 allocs/op
BenchmarkVerifyPod/enforce-restricted_deployment-16         	 4040166	       283.6 ns/op	     112 B/op	       1 allocs/op
BenchmarkVerifyPod/warn-baseline_pod-16                     	  307626	      3977 ns/op	    3480 B/op	      18 allocs/op
BenchmarkVerifyPod/warn-baseline_deployment-16              	  293635	      3664 ns/op	    3552 B/op	      19 allocs/op
BenchmarkVerifyPod/warn-restricted_pod-16                   	  308919	      4307 ns/op	    3480 B/op	      18 allocs/op
BenchmarkVerifyPod/warn-restricted_deployment-16            	  317541	      3943 ns/op	    3552 B/op	      19 allocs/op
BenchmarkVerifyPod/enforce-warn-audit-baseline_pod-16       	  181440	      6677 ns/op	    5976 B/op	      28 allocs/op
BenchmarkVerifyPod/enforce-warn-audit-baseline_deployment-16         	  233512	      5131 ns/op	    4800 B/op	      24 allocs/op
BenchmarkVerifyPod/warn-baseline-audit-restricted_pod-16             	  228664	      5582 ns/op	    4728 B/op	      23 allocs/op
BenchmarkVerifyPod/warn-baseline-audit-restricted_deployment-16      	  241128	      5634 ns/op	    4800 B/op	      24 allocs/op

goos: darwin
goarch: amd64
pkg: k8s.io/kubernetes/plugin/pkg/admission/security/podsecurity
cpu: Intel(R) Core(TM) i9-9980HK CPU @ 2.40GHz

Does this PR introduce a user-facing change?

Add Pod Security admission metrics: pod_security_evaluations_total, pod_security_exemptions_total, pod_security_errors_total

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

- [KEP]: https://github.com/kubernetes/enhancements/blob/master/keps/sig-auth/2579-psp-replacement/README.md#monitoring

/sig auth
/milestone v1.23
/assign @liggitt
/cc @jyz0309

k8s-ci-robot · 2021-10-26T07:31:51Z

@tallclair: GitHub didn't allow me to request PR reviews from the following users: jyz0309.

Note that only kubernetes members and repo collaborators can review this PR, and authors cannot review their own PRs.

In response to this:

What type of PR is this?

/kind bug

What this PR does / why we need it:

Fix up current metrics:
- Register metrics on the global handler
- Expose a /metrics endpoint on the webhook
Make metrics conform to the spec defined in https://github.com/kubernetes/enhancements/blob/master/keps/sig-auth/2579-psp-replacement/README.md#monitoring:
- operation -> request_operation for the label name
- resource should be either pod or controller (see comment on below)
- operation should be lower case
Add test cases
Add the error & exemption metrics

Which issue(s) this PR fixes:

Fixes #103207

Special notes for your reviewer:

Question 1: Do we want to consider the short-circuit case an evaluation, for monitoring purposes? doing so adds a second alloc in the common path, and doubles the evaluation time. See benchmarks below.

Question 2: I was corrected in the docs PR (kubernetes/website#29124 (review), @sftim ) that we should refer to the "pod template resources" as workload resources. Do we want to update the resource label on the metric to match (controller -> workload)?

Benchmarks:

# Original (without this PR):

BenchmarkVerifyPod/enforce-implicit_pod-16         	 4421935	       266.8 ns/op	     112 B/op	       1 allocs/op
BenchmarkVerifyPod/enforce-implicit_deployment-16  	 5054050	       267.9 ns/op	     112 B/op	       1 allocs/op
BenchmarkVerifyPod/enforce-privileged_pod-16       	 5110650	       276.4 ns/op	     112 B/op	       1 allocs/op
BenchmarkVerifyPod/enforce-privileged_deployment-16         	 4437080	       280.7 ns/op	     112 B/op	       1 allocs/op
BenchmarkVerifyPod/enforce-baseline_pod-16                  	  315181	      4174 ns/op	    3504 B/op	      19 allocs/op
BenchmarkVerifyPod/enforce-baseline_deployment-16           	 3700531	       303.5 ns/op	     112 B/op	       1 allocs/op
BenchmarkVerifyPod/enforce-restricted_pod-16                	  292236	      4702 ns/op	    3504 B/op	      19 allocs/op
BenchmarkVerifyPod/enforce-restricted_deployment-16         	 4413544	       299.9 ns/op	     112 B/op	       1 allocs/op
BenchmarkVerifyPod/warn-baseline_pod-16                     	  274810	      4191 ns/op	    3504 B/op	      19 allocs/op
BenchmarkVerifyPod/warn-baseline_deployment-16              	  304512	      3667 ns/op	    3552 B/op	      19 allocs/op
BenchmarkVerifyPod/warn-restricted_pod-16                   	  283743	      4435 ns/op	    3504 B/op	      19 allocs/op
BenchmarkVerifyPod/warn-restricted_deployment-16            	  263958	      3974 ns/op	    3552 B/op	      19 allocs/op
BenchmarkVerifyPod/enforce-warn-audit-baseline_pod-16       	  187177	      6975 ns/op	    6000 B/op	      29 allocs/op
BenchmarkVerifyPod/enforce-warn-audit-baseline_deployment-16         	  256402	      5064 ns/op	    4800 B/op	      24 allocs/op
BenchmarkVerifyPod/warn-baseline-audit-restricted_pod-16             	  204177	      5673 ns/op	    4752 B/op	      24 allocs/op
BenchmarkVerifyPod/warn-baseline-audit-restricted_deployment-16      	  226470	      5445 ns/op	    4800 B/op	      24 allocs/op

## With this PR

BenchmarkVerifyPod/enforce-implicit_pod-16         	 1946442	       588.9 ns/op	     224 B/op	       2 allocs/op
BenchmarkVerifyPod/enforce-implicit_deployment-16  	 4798802	       256.9 ns/op	     112 B/op	       1 allocs/op
BenchmarkVerifyPod/enforce-privileged_pod-16       	 1862803	       626.7 ns/op	     224 B/op	       2 allocs/op
BenchmarkVerifyPod/enforce-privileged_deployment-16         	 4398561	       283.9 ns/op	     112 B/op	       1 allocs/op
BenchmarkVerifyPod/enforce-baseline_pod-16                  	  301028	      4009 ns/op	    3480 B/op	      18 allocs/op
BenchmarkVerifyPod/enforce-baseline_deployment-16           	 4139908	       284.3 ns/op	     112 B/op	       1 allocs/op
BenchmarkVerifyPod/enforce-restricted_pod-16                	  297513	      4582 ns/op	    3480 B/op	      18 allocs/op
BenchmarkVerifyPod/enforce-restricted_deployment-16         	 3554356	       309.7 ns/op	     112 B/op	       1 allocs/op
BenchmarkVerifyPod/warn-baseline_pod-16                     	  295238	      4026 ns/op	    3480 B/op	      18 allocs/op
BenchmarkVerifyPod/warn-baseline_deployment-16              	  294944	      3670 ns/op	    3552 B/op	      19 allocs/op
BenchmarkVerifyPod/warn-restricted_pod-16                   	  298869	      4262 ns/op	    3480 B/op	      18 allocs/op
BenchmarkVerifyPod/warn-restricted_deployment-16            	  333310	      3916 ns/op	    3552 B/op	      19 allocs/op
BenchmarkVerifyPod/enforce-warn-audit-baseline_pod-16       	  166224	      6843 ns/op	    5976 B/op	      28 allocs/op
BenchmarkVerifyPod/enforce-warn-audit-baseline_deployment-16         	  257518	      4938 ns/op	    4800 B/op	      24 allocs/op
BenchmarkVerifyPod/warn-baseline-audit-restricted_pod-16             	  219632	      5800 ns/op	    4728 B/op	      23 allocs/op
BenchmarkVerifyPod/warn-baseline-audit-restricted_deployment-16      	  231926	      5616 ns/op	    4800 B/op	      24 allocs/op

## With this PR, but dropping the short-circuit eval count:

BenchmarkVerifyPod/enforce-implicit_pod-16         	 4651302	       247.4 ns/op	     112 B/op	       1 allocs/op
BenchmarkVerifyPod/enforce-implicit_deployment-16  	 4379590	       265.1 ns/op	     112 B/op	       1 allocs/op
BenchmarkVerifyPod/enforce-privileged_pod-16       	 4283446	       273.9 ns/op	     112 B/op	       1 allocs/op
BenchmarkVerifyPod/enforce-privileged_deployment-16         	 4164979	       282.0 ns/op	     112 B/op	       1 allocs/op
BenchmarkVerifyPod/enforce-baseline_pod-16                  	  307122	      3885 ns/op	    3480 B/op	      18 allocs/op
BenchmarkVerifyPod/enforce-baseline_deployment-16           	 4469337	       289.3 ns/op	     112 B/op	       1 allocs/op
BenchmarkVerifyPod/enforce-restricted_pod-16                	  300816	      4306 ns/op	    3480 B/op	      18 allocs/op
BenchmarkVerifyPod/enforce-restricted_deployment-16         	 4040166	       283.6 ns/op	     112 B/op	       1 allocs/op
BenchmarkVerifyPod/warn-baseline_pod-16                     	  307626	      3977 ns/op	    3480 B/op	      18 allocs/op
BenchmarkVerifyPod/warn-baseline_deployment-16              	  293635	      3664 ns/op	    3552 B/op	      19 allocs/op
BenchmarkVerifyPod/warn-restricted_pod-16                   	  308919	      4307 ns/op	    3480 B/op	      18 allocs/op
BenchmarkVerifyPod/warn-restricted_deployment-16            	  317541	      3943 ns/op	    3552 B/op	      19 allocs/op
BenchmarkVerifyPod/enforce-warn-audit-baseline_pod-16       	  181440	      6677 ns/op	    5976 B/op	      28 allocs/op
BenchmarkVerifyPod/enforce-warn-audit-baseline_deployment-16         	  233512	      5131 ns/op	    4800 B/op	      24 allocs/op
BenchmarkVerifyPod/warn-baseline-audit-restricted_pod-16             	  228664	      5582 ns/op	    4728 B/op	      23 allocs/op
BenchmarkVerifyPod/warn-baseline-audit-restricted_deployment-16      	  241128	      5634 ns/op	    4800 B/op	      24 allocs/op

goos: darwin
goarch: amd64
pkg: k8s.io/kubernetes/plugin/pkg/admission/security/podsecurity
cpu: Intel(R) Core(TM) i9-9980HK CPU @ 2.40GHz

Does this PR introduce a user-facing change?

Add Pod Security admission metrics: pod_security_evaluations_total, pod_security_exemptions_total, pod_security_errors_total

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

- [KEP]: https://github.com/kubernetes/enhancements/blob/master/keps/sig-auth/2579-psp-replacement/README.md#monitoring

/sig auth
/milestone v1.23
/assign @liggitt
/cc @jyz0309

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

k8s-ci-robot · 2021-10-26T07:31:56Z

@tallclair: This issue is currently awaiting triage.

If a SIG or subproject determines this is a relevant issue, they will accept it by applying the triage/accepted label and provide further guidance.

The triage/accepted label can be added by org members by writing /triage accepted in a comment.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

staging/src/k8s.io/pod-security-admission/admission/admission.go

tallclair · 2021-10-26T07:34:15Z

staging/src/k8s.io/pod-security-admission/metrics/metrics.go

+		return "namespace"
+	default:
+		// Assume any other resource is a valid input to pod-security, and therefore a controller.
+		return "controller"


This matches the spec described in the KEP, but should we call this workload instead, to match the term used in the documentation?

I'm fine with there being some mismatches - especially if that's what's in the KEP.
The workload resources page is at the URL https://kubernetes.io/docs/concepts/workloads/controllers/

Am I right that each of these “controller” checks is really looking at a pod template? If I were going to rename, I'd rename to something that mentions pod_template. Just a personal opinion here.

I don't feel strongly about the controller vs workload label value. Calling it pod_template might be confusing, since there's a specific PodTemplate kind

staging/src/k8s.io/pod-security-admission/metrics/metrics.go

staging/src/k8s.io/pod-security-admission/admission/admission.go

liggitt · 2021-10-26T14:20:04Z

Overall lgtm. Since the metrics calls are pretty dispersed, how's the unit test coverage of the various places we record them?

dgrisonnet · 2021-10-26T21:39:21Z

Looking good from a metrics perspective.

dgrisonnet · 2021-10-26T21:10:09Z

staging/src/k8s.io/pod-security-admission/cmd/webhook/server/server.go

@@ -117,6 +119,11 @@ func (s *Server) Start(ctx context.Context) error {
 	// debugging or proxy purposes. The API server will not connect to an http webhook.
 	mux.HandleFunc("/", s.HandleValidate)

+	// Serve the global metrics registry.
+	metrics.LegacyMustRegister()


Do we really need the global registry here? Since you already added the logic to handle the /metrics endpoint, it would be a good opportunity to move away from the global registry and add a local KubeRegistry. This would prevent being polluted by metrics registered to the legacyregistry that wouldn't be meaningful to the webhook.

Good call. I dumped the metrics from the webhook, and there was a lot of irrelevant stuff there. For now I'm using a new registry with just the pod_security metrics. We may want to add back some of the relevant pieces later though (go metrics, server metrics).

You can add the go metrics, right now by registering the client_golang GoCollector.

As for server metrics, it is a bit more complex since they aren't dissociated from the legacyregistry, so if we really want them the best would be to still serve metrics from the global registry. That's what we are doing in metrics-server which is an aggregated API for which server metrics are really useful: https://github.com/kubernetes-sigs/metrics-server/blob/bfa76316118b4e4993fd16c1e6df501109b0cc18/pkg/server/config.go#L95-L110. So we end up serving metrics from both the legacylegistry and the KubeRegistry, but having them separated into two registries should make it easier to integrate server metrics once they are decoupled from the global registry.

dgrisonnet · 2021-10-26T21:34:40Z

staging/src/k8s.io/pod-security-admission/metrics/metrics.go

 }

-func (r PrometheusRecorder) RecordEvaluation(decision Decision, policy api.LevelVersion, evalMode Mode, attrs api.Attributes) {
+func (r *PrometheusRecorder) MustRegister(registerFunc func(...metrics.Registerable)) {
+	r.registerOnce.Do(func() {


I don't have too much knowledge about the PodSecurity plugin, but can we really have multiple plugins registering the metrics on the same server?

Are you asking why registerOnce is necessary here? TBH, I'm not sure, but I kept getting panics in the integration testing. Maybe something to do with the way integration tests are run in process?

Yes, that's why I was asking cause from an external point of view, it didn't seem possible to have the metrics being registered more than once. If you were seeing panics in the integration tests, I guess it means that it is relevant here 🤷‍♂️

it's not really the recorder's responsibility to limit itself to registering once... I suggested moving this to the kube-apiserver single registration to the legacy registry

tallclair · 2021-10-27T01:52:38Z

Ready for another round of reviews.

tallclair · 2021-10-27T17:28:02Z

Updated tests to explicitly expect the cached metrics, and eliminate the dependency on the prometheus model.

staging/src/k8s.io/pod-security-admission/metrics/metrics.go

liggitt · 2021-10-27T21:36:13Z

staging/src/k8s.io/pod-security-admission/metrics/metrics.go

 }

-func (r PrometheusRecorder) RecordEvaluation(decision Decision, policy api.LevelVersion, evalMode Mode, attrs api.Attributes) {
+func (r *PrometheusRecorder) MustRegister(registerFunc func(...metrics.Registerable)) {
+	r.registerOnce.Do(func() {


it's not really the recorder's responsibility to limit itself to registering once... I suggested moving this to the kube-apiserver single registration to the legacy registry

staging/src/k8s.io/pod-security-admission/metrics/metrics.go

staging/src/k8s.io/pod-security-admission/cmd/webhook/server/server.go

tallclair · 2021-10-29T18:24:13Z

/hold
Needs squash before merging.

tallclair · 2021-10-29T20:58:34Z

This is ready for another review.

liggitt

needs update-vendor.sh run

staging/src/k8s.io/pod-security-admission/api/helpers.go

liggitt · 2021-11-01T12:51:45Z

lgtm otherwise

tallclair · 2021-11-01T16:45:43Z

Done. Also squashed commits & rebased.

/hold cancel

liggitt · 2021-11-01T19:52:40Z

/retest

Update pod security metrics to match the spec in the KEP.

tallclair · 2021-11-01T21:24:02Z

Rebased. Also had to update the test to check specifically for audit-violations in the annotations to construct the metrics expectations.

liggitt · 2021-11-01T21:29:55Z

/lgtm

liggitt · 2021-11-01T23:21:52Z

/approve

k8s-ci-robot · 2021-11-01T23:22:15Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: liggitt, tallclair

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~plugin/pkg/admission/OWNERS~~ [liggitt]
~~staging/src/k8s.io/pod-security-admission/OWNERS~~ [liggitt,tallclair]
~~test/OWNERS~~ [liggitt]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

tallclair added this to In Review in SIG-Auth: PodSecurity via automation Oct 26, 2021

k8s-ci-robot assigned liggitt Oct 26, 2021

k8s-ci-robot added the release-note Denotes a PR that will be considered when it comes time to generate release notes. label Oct 26, 2021

k8s-ci-robot added kind/bug Categorizes issue or PR as related to a bug. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. sig/auth Categorizes an issue or PR as relevant to SIG Auth. labels Oct 26, 2021

k8s-ci-robot added this to the v1.23 milestone Oct 26, 2021

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Oct 26, 2021

k8s-ci-robot added needs-priority Indicates a PR lacks a `priority/foo` label and requires one. area/test sig/instrumentation Categorizes an issue or PR as relevant to SIG Instrumentation. sig/testing Categorizes an issue or PR as relevant to SIG Testing. labels Oct 26, 2021

tallclair commented Oct 26, 2021

View reviewed changes

staging/src/k8s.io/pod-security-admission/admission/admission.go Show resolved Hide resolved

tallclair commented Oct 26, 2021

View reviewed changes

liggitt reviewed Oct 26, 2021

View reviewed changes

staging/src/k8s.io/pod-security-admission/metrics/metrics.go Outdated Show resolved Hide resolved

staging/src/k8s.io/pod-security-admission/metrics/metrics.go Outdated Show resolved Hide resolved

staging/src/k8s.io/pod-security-admission/admission/admission.go Show resolved Hide resolved

tallclair changed the title ~~Ps metrics~~ [PodSecurity] Metrics improvements Oct 26, 2021

dgrisonnet reviewed Oct 26, 2021

View reviewed changes

k8s-ci-robot added the area/dependency Issues or PRs related to dependency changes label Oct 27, 2021

tallclair force-pushed the ps-metrics branch from d2aeaf8 to 79195d5 Compare October 27, 2021 17:25

liggitt reviewed Oct 27, 2021

View reviewed changes

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 29, 2021

k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 29, 2021

tallclair force-pushed the ps-metrics branch 2 times, most recently from c543dc4 to ade88ed Compare October 29, 2021 20:54

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 29, 2021

liggitt reviewed Nov 1, 2021

View reviewed changes

staging/src/k8s.io/pod-security-admission/api/helpers.go Outdated Show resolved Hide resolved

enj added this to Needs Triage in SIG Auth Old Nov 1, 2021

ritazh moved this from Needs Triage to In Review in SIG Auth Old Nov 1, 2021

tallclair force-pushed the ps-metrics branch from ade88ed to a67c710 Compare November 1, 2021 16:45

k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 1, 2021

liggitt moved this from In Review to Done (1.23, Beta) in SIG-Auth: PodSecurity Nov 1, 2021

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 1, 2021

tallclair added 4 commits November 1, 2021 14:11

[PodSecurity] Fix up metrics & add tests

e46928c

Update pod security metrics to match the spec in the KEP.

[PodSecurity] Add error & exemption metrics

21692e1

[PodSecurity] Avoid the LegcayRegistry for metrics serving

6c27302

Cache fast-path metrics & update unit tests

afad341

tallclair force-pushed the ps-metrics branch from a67c710 to afad341 Compare November 1, 2021 21:23

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 1, 2021

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 1, 2021

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 1, 2021

tallclair mentioned this pull request Nov 1, 2021

[PodSecurity] Expand unit test coverage and fix error cases #106017

Merged

k8s-ci-robot merged commit 349758d into kubernetes:master Nov 2, 2021

SIG Auth Old automation moved this from In Review to Closed / Done Nov 2, 2021

liggitt mentioned this pull request Nov 3, 2021

PodSecurity admission (PodSecurityPolicy replacement) kubernetes/enhancements#2579

Closed

12 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[PodSecurity] Metrics improvements #105898

[PodSecurity] Metrics improvements #105898

tallclair commented Oct 26, 2021

k8s-ci-robot commented Oct 26, 2021

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR fixes:

Special notes for your reviewer:

Does this PR introduce a user-facing change?

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

k8s-ci-robot commented Oct 26, 2021

tallclair Oct 26, 2021

sftim Oct 26, 2021

liggitt Oct 26, 2021

liggitt commented Oct 26, 2021

dgrisonnet commented Oct 26, 2021

dgrisonnet Oct 26, 2021

tallclair Oct 27, 2021

dgrisonnet Oct 27, 2021

dgrisonnet Oct 26, 2021

tallclair Oct 26, 2021

dgrisonnet Oct 27, 2021

liggitt Oct 27, 2021

tallclair commented Oct 27, 2021

tallclair commented Oct 27, 2021

liggitt Oct 27, 2021

tallclair commented Oct 29, 2021

tallclair commented Oct 29, 2021

liggitt left a comment

liggitt commented Nov 1, 2021

tallclair commented Nov 1, 2021

liggitt commented Nov 1, 2021

tallclair commented Nov 1, 2021

liggitt commented Nov 1, 2021

liggitt commented Nov 1, 2021

k8s-ci-robot commented Nov 1, 2021

[PodSecurity] Metrics improvements #105898

[PodSecurity] Metrics improvements #105898

Conversation

tallclair commented Oct 26, 2021

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR fixes:

Special notes for your reviewer:

Does this PR introduce a user-facing change?

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

k8s-ci-robot commented Oct 26, 2021

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR fixes:

Special notes for your reviewer:

Does this PR introduce a user-facing change?

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

k8s-ci-robot commented Oct 26, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

liggitt commented Oct 26, 2021

dgrisonnet commented Oct 26, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tallclair commented Oct 27, 2021

tallclair commented Oct 27, 2021

Choose a reason for hiding this comment

tallclair commented Oct 29, 2021

tallclair commented Oct 29, 2021

liggitt left a comment

Choose a reason for hiding this comment

liggitt commented Nov 1, 2021

tallclair commented Nov 1, 2021

liggitt commented Nov 1, 2021

tallclair commented Nov 1, 2021

liggitt commented Nov 1, 2021

liggitt commented Nov 1, 2021

k8s-ci-robot commented Nov 1, 2021