Promote SubjectAccessReview to v1 #40709

liggitt · 2017-01-30T20:57:58Z

We have multiple features that depend on this API:

SubjectAccessReview

webhook authorization
kubelet delegated authorization
add-on API server delegated authorization

The API has been in use since 1.3 in beta status (v1beta1) with negligible changes:

Added a status field for reporting errors evaluating access
A typo was discovered in the SubjectAccessReviewSpec Groups field name

This PR promotes the existing v1beta1 API to v1, with the only change being the typo fix to the groups field. (fixes #32709)

Because the API does not persist data (it is a query/response-style API), there are no data migration concerns.

This positions us to promote the features that depend on this API to stable in 1.7

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

The authorization.k8s.io API group was promoted to v1

k8s-reviewable · 2017-01-30T20:58:06Z

This change is

deads2k · 2017-01-30T21:11:52Z

cmd/libs/go2idl/informer-gen/generators/packages.go

-		cmdArgs += fmt.Sprintf("--%s=%s ", f.Name, f.Value)
-	})
-	return fmt.Sprintf("\n// This file was automatically generated by informer-gen with arguments: %s\n\n", cmdArgs)
+	return fmt.Sprintf("\n// This file was automatically generated by informer-gen\n\n")


@ncdc I don't care, do you?

No, I already gave the ok in #40700

deads2k · 2017-01-30T21:14:03Z

pkg/apis/authorization/install/install.go

 			AddInternalObjectsToScheme: authorization.AddToScheme,
 		},
 		announced.VersionToSchemeFunc{
+			v1.SchemeGroupVersion.Version:      v1.AddToScheme,


need to fix up the version preference order above.

deads2k · 2017-01-30T21:14:36Z

Is there a simple way for you to confirm that the preference order is correct in discovery?

deads2k · 2017-01-30T21:14:50Z

minor (but important) comment. lgtm otherwise.

liggitt · 2017-01-30T22:32:34Z

@k8s-bot bazel test this

liggitt · 2017-02-01T14:37:23Z

@mikedanese, any clue what the issue is causing this bazel error:

W0201 14:25:24.575] ERROR: /workspace/k8s.io/kubernetes/BUILD.bazel:35:1: Target '//pkg:all-srcs' is not visible from target '//:all-srcs'. Check the visibility declaration of the former target if you think the dependency is legitimate.
W0201 14:25:24.894] ERROR: Analysis of target '//hack:verify-boilerplate' failed; build aborted.
W0201 14:25:24.894] ____Elapsed time: 13.606s
W0201 14:25:24.895] ERROR: Couldn't start the build. Unable to run tests.
W0201 14:25:24.897] make: *** [bazel-test] Error 1
I0201 14:25:24.997] Makefile:501: recipe for target 'bazel-test' failed
E0201 14:25:25.050] Build failed
I0201 14:25:25.050] process 468 exited with code 2 after 9.0m
E0201 14:25:25.050] FAIL: pull-kubernetes-bazel

liggitt · 2017-02-02T04:00:33Z

#39471 flake
@k8s-bot unit test this
@k8s-bot cross build this

liggitt · 2017-02-02T04:01:22Z

@kubernetes/api-approvers for approval

liggitt · 2017-02-02T17:02:09Z

flake #38518
@k8s-bot unit test this

smarterclayton · 2017-02-02T20:57:50Z

I'm ok with this being promoted - it fits the requirements, we've had it baked for a long time, and we are testing and using it regularly. @erictune can you weigh in whether you concur?

erictune · 2017-02-06T18:07:57Z

/lgtm

k8s-ci-robot · 2017-02-06T18:07:58Z

@erictune: you can't LGTM a PR unless you are an assignee.

In response to this comment:

/lgtm

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. I understand the commands that are listed here.

erictune · 2017-02-06T18:09:27Z

@cjcullen FYI. subjectaccessreview going GA. looked good to me. now would be good time to comment if you had issues with it.

cjcullen · 2017-02-06T18:20:39Z

No worries about it from me...

smarterclayton · 2017-02-06T21:06:04Z

/lgtm

smarterclayton · 2017-02-06T21:30:24Z

/approve

Use specific v1beta1 Authorization client Add client expansions

liggitt · 2017-02-06T23:17:09Z

regenerated conflicting pb files

k8s-github-robot · 2017-02-06T23:27:31Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

The following people have approved this PR: liggitt

Needs approval from an approver in each of these OWNERS Files:

OWNERS
~~pkg/client/OWNERS~~ [liggitt]
~~test/OWNERS~~ [liggitt]

We suggest the following people:
cc @smarterclayton
You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

liggitt · 2017-02-07T00:42:33Z

@k8s-bot gce etcd3 e2e test this

k8s-ci-robot · 2017-02-07T01:35:50Z

@liggitt: The following test(s) failed:

Test name	Commit	Details	Rerun command
Jenkins CRI GCE e2e	bb7b0e4bec92c427438a8db71bde88dc47de0508	link	`@k8s-bot cri e2e test this`

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

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. I understand the commands that are listed here.

k8s-github-robot · 2017-02-07T04:57:14Z

Automatic merge from submit-queue (batch tested with PRs 40971, 41027, 40709, 40903, 39369)

Automatic merge from submit-queue (batch tested with PRs 41112, 41201, 41058, 40650, 40926) Promote TokenReview to v1 Peer to #40709 We have multiple features that depend on this API: - [webhook authentication](https://kubernetes.io/docs/admin/authentication/#webhook-token-authentication) - [kubelet delegated authentication](https://kubernetes.io/docs/admin/kubelet-authentication-authorization/#kubelet-authentication) - add-on API server delegated authentication The API has been in use since 1.3 in beta status (v1beta1) with negligible changes: - Added a status field for reporting errors evaluating the token This PR promotes the existing v1beta1 API to v1 with no changes Because the API does not persist data (it is a query/response-style API), there are no data migration concerns. This positions us to promote the features that depend on this API to stable in 1.7 cc @kubernetes/sig-auth-api-reviews @kubernetes/sig-auth-misc ```release-note The authentication.k8s.io API group was promoted to v1 ```

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Jan 30, 2017

k8s-github-robot assigned timothysc Jan 30, 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 size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Jan 30, 2017

liggitt added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed release-note-label-needed labels Jan 30, 2017

deads2k reviewed Jan 30, 2017

View reviewed changes

liggitt force-pushed the v1-sar branch 3 times, most recently from e7b5b11 to 752978f Compare January 30, 2017 22:09

k8s-github-robot assigned bgrant0607 and smarterclayton and unassigned timothysc and bgrant0607 Jan 30, 2017

apelisse assigned timothysc Jan 31, 2017

timothysc removed their assignment Jan 31, 2017

liggitt force-pushed the v1-sar branch 6 times, most recently from db5b393 to 40ef8f7 Compare February 1, 2017 14:16

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

k8s-github-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 6, 2017

liggitt added 4 commits February 6, 2017 17:43

Copy v1beta1 authorization types to v1

e4d6c48

Fix typo in v1beta1 field name

ae73f24

Generated files

31d45fe

install authorization.k8s.io/v1, add tests

8706bc9

Use specific v1beta1 Authorization client Add client expansions

liggitt force-pushed the v1-sar branch from bb7b0e4 to 8706bc9 Compare February 6, 2017 23:17

k8s-github-robot merged commit 460f443 into kubernetes:master Feb 7, 2017

liggitt mentioned this pull request Feb 7, 2017

Promote TokenReview to v1 #41058

Merged

liggitt deleted the v1-sar branch February 10, 2017 15:46

Promote SubjectAccessReview to v1 #40709

Promote SubjectAccessReview to v1 #40709

Uh oh!

Conversation

liggitt commented Jan 30, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

k8s-reviewable commented Jan 30, 2017

Uh oh!

deads2k Jan 30, 2017

Choose a reason for hiding this comment

Uh oh!

ncdc Jan 30, 2017

Choose a reason for hiding this comment

Uh oh!

deads2k Jan 30, 2017

Choose a reason for hiding this comment

Uh oh!

liggitt Jan 30, 2017

Choose a reason for hiding this comment

Uh oh!

deads2k commented Jan 30, 2017

Uh oh!

deads2k commented Jan 30, 2017

Uh oh!

liggitt commented Jan 30, 2017

Uh oh!

liggitt commented Feb 1, 2017

Uh oh!

liggitt commented Feb 2, 2017

Uh oh!

liggitt commented Feb 2, 2017

Uh oh!

liggitt commented Feb 2, 2017

Uh oh!

smarterclayton commented Feb 2, 2017

Uh oh!

erictune commented Feb 6, 2017

Uh oh!

k8s-ci-robot commented Feb 6, 2017

Uh oh!

erictune commented Feb 6, 2017

Uh oh!

cjcullen commented Feb 6, 2017

Uh oh!

smarterclayton commented Feb 6, 2017

Uh oh!

smarterclayton commented Feb 6, 2017

Uh oh!

liggitt commented Feb 6, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

k8s-github-robot commented Feb 6, 2017

Uh oh!

liggitt commented Feb 7, 2017

Uh oh!

k8s-ci-robot commented Feb 7, 2017

Uh oh!

k8s-github-robot commented Feb 7, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

12 participants

liggitt commented Jan 30, 2017 •

edited

Loading

liggitt commented Feb 6, 2017 •

edited

Loading