Introduce PodSecurityPolicy in the policy/v1beta1 API group #54933

php-coder · 2017-11-01T11:40:50Z

Types/constants are completely the same as in extensions/v1beta1 except that they are located outside of the extensions API group.

What this PR does / why we need it:
This is the first step for migrating PSP-related stuff away of extensions group. See #43214 for more information.

Also it related to kubernetes/enhancements#5

Example:

$ cat restricted2.yaml 
apiVersion: policy/v1beta1
kind: PodSecurityPolicy
metadata:
  name: restricted2
...
$ kubectl create -f restricted.yaml 
podsecuritypolicy "restricted2" created
$ kubectl get psp restricted2 -o yaml
apiVersion: extensions/v1beta1
kind: PodSecurityPolicy
...

Release note:

The `PodSecurityPolicy` API has been moved to the `policy/v1beta1` API group. The `PodSecurityPolicy` API in the `extensions/v1beta1` API group is deprecated and will be removed in a future release. Authorizations for using pod security policy resources should change to reference the `policy` API group after upgrading to 1.11.

dims · 2017-11-01T11:46:28Z

/ok-to-test

@php-coder you should apply for membership to the kubernetes org :)

php-coder · 2017-11-01T11:46:30Z

pkg/apis/psp/v1beta1/doc.go

+*/
+
+// +k8s:conversion-gen=k8s.io/kubernetes/pkg/apis/psp
+// +k8s:conversion-gen=k8s.io/kubernetes/pkg/apis/extensions


@sttts I'm not sure whether I need this line or not. My theory is that it should generate conversions from/to extensions to psp types but I'm not sure. Could you tell me what it does?

From Makefile.generated_files:

# # Conversion generation # # Any package that wants conversion functions generated must include one or # more comment-tags in any .go file, in column 0, of the form: # // +k8s:conversion-gen=<CONVERSION_TARGET_DIR> # # The CONVERSION_TARGET_DIR is a project-local path to another directory which # should be considered when evaluating peer types for conversions. Types which # are found in the source package (where conversions are being generated) # but do not have a peer in one of the target directories will not have # conversions generated.

I expect that you don't need this if all your types are in the new pkg/apis directory. But if you share the internal types with pkg/apis/extensions, you will need this tag.

php-coder · 2017-11-01T11:48:34Z

pkg/apis/psp/OWNERS

@@ -0,0 +1,39 @@
+reviewers:


I copied OWNERS file from pkg/apis/extensions/OWNERS. Is it OK or we should start an empty OWNERS file?

that's a huge list. OWNERS should be more focused.

May I just remove OWNERS files?

If it does not exist, the everything from the parent dir is inherited. But having a handful or less of reviewers here makes certainly sense.

@sttts Ok, thanks!

I decided to include only myself as a reviewer because I don't know who also want to be a reviewer for this stuff.

@pweil- @simo5 @adelton @enj Let me know if you want to be reviewers and I'll add you to this list.

Sure. Please include @liggitt @tallclair as well.

Thanks! Added.

php-coder · 2017-11-01T11:54:07Z

make update fails with the following error:

Running update-generated-protobuf
Running update-codegen
Running update-generated-docs
# k8s.io/kubernetes/vendor/k8s.io/client-go/kubernetes
vendor/k8s.io/client-go/kubernetes/clientset.go:90: duplicate method PolicyV1beta1
vendor/k8s.io/client-go/kubernetes/clientset.go:95: duplicate method Policy
vendor/k8s.io/client-go/kubernetes/clientset.go:135: duplicate field policyV1beta1
vendor/k8s.io/client-go/kubernetes/clientset.go:292: ambiguous selector c.policyV1beta1
vendor/k8s.io/client-go/kubernetes/clientset.go:298: ambiguous selector c.policyV1beta1
vendor/k8s.io/client-go/kubernetes/clientset.go:302: (*Clientset).PolicyV1beta1 redeclared in this block
	previous declaration at vendor/k8s.io/client-go/kubernetes/clientset.go:291
vendor/k8s.io/client-go/kubernetes/clientset.go:303: ambiguous selector c.policyV1beta1
vendor/k8s.io/client-go/kubernetes/clientset.go:308: (*Clientset).Policy redeclared in this block
	previous declaration at vendor/k8s.io/client-go/kubernetes/clientset.go:297
vendor/k8s.io/client-go/kubernetes/clientset.go:309: ambiguous selector c.policyV1beta1
vendor/k8s.io/client-go/kubernetes/clientset.go:455: ambiguous selector cs.policyV1beta1
vendor/k8s.io/client-go/kubernetes/clientset.go:455: too many errors
# k8s.io/kubernetes/vendor/k8s.io/client-go/listers/policy/v1beta1
vendor/k8s.io/client-go/listers/policy/v1beta1/eviction.go:34: undefined: EvictionListerExpansion
# k8s.io/kubernetes/pkg/client/listers/policy/internalversion
pkg/client/listers/policy/internalversion/eviction.go:34: undefined: EvictionListerExpansion
!!! [1101 12:47:43] Call tree:
!!! [1101 12:47:43]  1: /home/coder/git/src/home/kubernetes/hack/lib/golang.sh:705 kube::golang::build_binaries_for_platform(...)
!!! [1101 12:47:43]  2: hack/make-rules/build.sh:27 kube::golang::build_binaries(...)
!!! [1101 12:47:43] Call tree:
!!! [1101 12:47:43]  1: hack/make-rules/build.sh:27 kube::golang::build_binaries(...)
!!! [1101 12:47:43] Call tree:
!!! [1101 12:47:43]  1: hack/make-rules/build.sh:27 kube::golang::build_binaries(...)
make[1]: *** [all] Error 1

@sttts Could you look at this PR and give me a hint what's wrong here? For some reason, generators adds generated code to the existing files and it leads to duplicated definitions. I couldn't find why they are doing this.

It also not clear where I should put the files for policy.authorization.k8s.io. The policy directory is already used, so I put the files into the psp directory. Is it correct?

sttts · 2017-11-01T12:02:05Z

pkg/apis/psp/types.go

@@ -0,0 +1,283 @@
+/*


will you delete the old internal types for PSPs?

I'm following a roadmap that @soltysh gave me: in this Kubernetes release, I only need to introduce a new API that will be stored as old PSP types. So, in the current iteration, I won't remove anything from PSP.

Which doubles the code for the internal types. You can easily share them by removing the old extensions types and let the extensions group point to your new ones in the new group. Compare #50327.

Maybe I'm missing something here, but I don't see how it doubles the code for the internal types. New API group is using the old internal types.

sttts · 2017-11-01T12:07:52Z

For some reason, generators adds generated code to the existing files and it leads to duplicated definitions. I couldn't find why they are doing this.

It also not clear where I should put the files for policy.authorization.k8s.io. The policy directory is already used, so I put the files into the psp directory. Is it correct?

Looks like clientgen uses the first segment of the group name for the identifiers in the clientset. @mfojtik can your exception file fix this?

enj · 2017-11-01T14:49:01Z

Is policy.authorization.k8s.io specific enough? RBAC could easily be thought to fit in this group (even though it does not).

liggitt · 2017-11-01T15:01:22Z

yeah, I'm unsure of the group as well. what would be the unifying characteristic of objects in the policy.authorization.k8s.io group? For example, the NetworkPolicy object is in the networking.k8s.io group.

some possibilities:

the policy group: already contains PodDisruptionBudget objects which are policies around pod eviction
podsecuritypolicy.admission.k8s.io - objects specific to this admission plugin

php-coder · 2017-11-01T15:29:03Z

podsecuritypolicy.admission.k8s.io - objects specific to this admission plugin

This sounds better to me. I'll do update on next Monday to use it, unless someone object against.

sttts · 2017-11-01T18:56:11Z

#54950 adds the missing support for non-unique GroupName prefixes to client-gen and informer-gen.

sttts · 2017-11-01T18:58:43Z

It also not clear where I should put the files for policy.authorization.k8s.io. The policy directory is already used, so I put the files into the psp directory. Is it correct?

There is no strict rule. Policy is for the pod policy types. Are PSPs the only types in the new group? Maybe authpolicy might make sense.

liggitt · 2018-02-19T19:10:46Z

pkg/apis/policy/v1beta1/defaults.go

@@ -0,0 +1,35 @@
+/*
+Copyright 2015 The Kubernetes Authors.


nit, 2018 in all new files

There was only one single file.

PSP are completely the same as in extensions/v1beta1 except that they are located outside of the extensions API group.

liggitt · 2018-02-19T19:19:17Z

one nit on copyright dates in new files, lgtm otherwise

cc @kubernetes/sig-auth-api-reviews @kubernetes/api-approvers

php-coder · 2018-02-19T19:48:14Z

@liggitt The comment was addressed. Thank you for guidance and patience!

liggitt · 2018-02-20T21:48:22Z

/lgtm
/hold for ack from @kubernetes/api-approvers

bgrant0607 · 2018-02-20T21:51:53Z

@liggitt ACK that policy is fine. Thanks much

liggitt · 2018-02-20T21:52:29Z

/hold cancel

php-coder · 2018-02-20T22:01:00Z

@liggitt Are we waiting for someone else LGTM from API approvers or I can ask examples/ owners to review their part?

smarterclayton · 2018-02-20T22:03:46Z

/approve

examples

k8s-ci-robot · 2018-02-20T22:03:53Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: liggitt, php-coder, smarterclayton, soltysh

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:

~~api/OWNERS~~ [liggitt,smarterclayton]
~~cmd/kube-apiserver/OWNERS~~ [liggitt,smarterclayton]
~~docs/api-reference/OWNERS~~ [liggitt,smarterclayton]
~~examples/OWNERS~~ [smarterclayton]
~~pkg/api/OWNERS~~ [liggitt,smarterclayton]
~~pkg/apis/policy/OWNERS~~ [liggitt,smarterclayton]
~~pkg/kubeapiserver/OWNERS~~ [liggitt,smarterclayton]
~~pkg/registry/OWNERS~~ [liggitt,smarterclayton]
~~staging/src/k8s.io/api/policy/OWNERS~~ [liggitt,smarterclayton]
~~staging/src/k8s.io/client-go/OWNERS~~ [liggitt,smarterclayton]
~~test/OWNERS~~ [liggitt,smarterclayton,soltysh]

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

k8s-github-robot · 2018-02-20T22:56:30Z

/test all [submit-queue is verifying that this PR is safe to merge]

k8s-github-robot · 2018-02-20T23:44:50Z

Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions here.

Automatic merge from submit-queue. 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>. PSP plugin: allow authorizing via "use" verb in policy API group **What this PR does / why we need it**: In order to determine whether a service account/user has access to PSP, PodSecurityPolicy admission plugin tests whether a service account/user is authorized for "use" verb in `extensions` API group. As PSP is being migrated to `policy` API group, we need to support its new location. This PR adds such a support by checking in both API groups. **Which issue(s) this PR fixes**: Addressed to: kubernetes/enhancements#5 Follow-up to: #54933

Automatic merge from submit-queue (batch tested with PRs 60475, 60514, 60506, 59903, 60319). 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>. E2E: add tests for PSP in the policy API Group **What this PR does / why we need it**: E2E tests were added for testing PSP from the "policy" API Group. They are similar to the tests for PSP from the "extensions" API Group. **Which issue(s) this PR fixes**: Addressed to: kubernetes/enhancements#5 Follow-up to: #54933 and #60145

k8s-github-robot assigned yifan-gu and gmarek Nov 1, 2017

k8s-github-robot added kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API kind/new-api labels Nov 1, 2017

php-coder force-pushed the psp_introduce_new_api_group branch from cf6e369 to e37e86e Compare November 1, 2017 11:44

k8s-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Nov 1, 2017

k8s-ci-robot removed the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Nov 1, 2017

php-coder commented Nov 1, 2017

View reviewed changes

sttts reviewed Nov 1, 2017

View reviewed changes

k8s-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Nov 1, 2017

php-coder force-pushed the psp_introduce_new_api_group branch from a6b0f13 to e334fe4 Compare November 8, 2017 16:23

php-coder changed the title ~~[WIP] Introduce policy.authorization.k8s.io/v1beta1~~ [WIP] Introduce podsecuritypolicy.admission.k8s.io/v1beta1 Nov 8, 2017

k8s-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Nov 8, 2017

php-coder force-pushed the psp_introduce_new_api_group branch from d7edfa3 to f253101 Compare February 19, 2018 18:39

liggitt reviewed Feb 19, 2018

View reviewed changes

php-coder added 3 commits February 19, 2018 20:14

Introduce PodSecurityPolicy in the policy/v1beta1 API group.

379683d

PSP are completely the same as in extensions/v1beta1 except that they are located outside of the extensions API group.

Update examples to use PSPs from the policy API group.

6410448

Update generated files.

29514f2

k8s-ci-robot added the kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API label Feb 19, 2018

liggitt assigned tallclair Feb 19, 2018

php-coder force-pushed the psp_introduce_new_api_group branch from f253101 to 29514f2 Compare February 19, 2018 19:46

k8s-ci-robot assigned liggitt Feb 20, 2018

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 20, 2018

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 20, 2018

k8s-github-robot merged commit f829870 into kubernetes:master Feb 20, 2018

php-coder deleted the psp_introduce_new_api_group branch February 21, 2018 08:08

php-coder mentioned this pull request Feb 21, 2018

PSP plugin: allow authorizing via "use" verb in policy API group #60145

Merged

php-coder mentioned this pull request Feb 23, 2018

E2E: add tests for PSP in the policy API Group #60319

Merged

This was referenced Feb 28, 2018

staging/podsecuritypolicy/rbac: use PSP from policy API group kubernetes/examples#206

Merged

Use PSP from policy API group kubernetes/website#7562

Merged

yue9944882 mentioned this pull request Aug 27, 2018

[WIP] admission/podsecuritypolicy: externalize podsecuritypolicy admission controller #67846

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Introduce PodSecurityPolicy in the policy/v1beta1 API group #54933

Introduce PodSecurityPolicy in the policy/v1beta1 API group #54933

php-coder commented Nov 1, 2017 •

edited by liggitt

dims commented Nov 1, 2017

php-coder Nov 1, 2017

sttts Nov 1, 2017

sttts Nov 1, 2017

php-coder Nov 1, 2017 •

edited

sttts Nov 1, 2017

php-coder Nov 1, 2017

sttts Nov 1, 2017

php-coder Nov 1, 2017 •

edited

pweil- Nov 1, 2017

php-coder Nov 1, 2017

php-coder commented Nov 1, 2017

sttts Nov 1, 2017

php-coder Nov 1, 2017

sttts Nov 1, 2017

php-coder Nov 1, 2017

sttts commented Nov 1, 2017

enj commented Nov 1, 2017

liggitt commented Nov 1, 2017

php-coder commented Nov 1, 2017

sttts commented Nov 1, 2017

sttts commented Nov 1, 2017

liggitt Feb 19, 2018

php-coder Feb 19, 2018

liggitt commented Feb 19, 2018 •

edited

php-coder commented Feb 19, 2018

liggitt commented Feb 20, 2018

bgrant0607 commented Feb 20, 2018

liggitt commented Feb 20, 2018

php-coder commented Feb 20, 2018

smarterclayton commented Feb 20, 2018

k8s-ci-robot commented Feb 20, 2018

k8s-github-robot commented Feb 20, 2018

k8s-github-robot commented Feb 20, 2018

Introduce PodSecurityPolicy in the policy/v1beta1 API group #54933

Introduce PodSecurityPolicy in the policy/v1beta1 API group #54933

Conversation

php-coder commented Nov 1, 2017 • edited by liggitt

dims commented Nov 1, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

php-coder Nov 1, 2017 • edited

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

php-coder Nov 1, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

php-coder commented Nov 1, 2017

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

sttts commented Nov 1, 2017

enj commented Nov 1, 2017

liggitt commented Nov 1, 2017

php-coder commented Nov 1, 2017

sttts commented Nov 1, 2017

sttts commented Nov 1, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

liggitt commented Feb 19, 2018 • edited

php-coder commented Feb 19, 2018

liggitt commented Feb 20, 2018

bgrant0607 commented Feb 20, 2018

liggitt commented Feb 20, 2018

php-coder commented Feb 20, 2018

smarterclayton commented Feb 20, 2018

k8s-ci-robot commented Feb 20, 2018

k8s-github-robot commented Feb 20, 2018

k8s-github-robot commented Feb 20, 2018

php-coder commented Nov 1, 2017 •

edited by liggitt

php-coder Nov 1, 2017 •

edited

php-coder Nov 1, 2017 •

edited

liggitt commented Feb 19, 2018 •

edited