Skip to content

Commit c3484c2

Browse files
openshift-merge-robotopenshift-merge-robot
authored
Merge pull request #17288 from deads2k/admission-01-external-easy
Automatic merge from submit-queue. switch the easy admission plugins to external types I also picked up the user change before I realized how far it rippled.
2 parents 5eb8353 + faf4959 commit c3484c2

File tree

19 files changed

+110
-56
lines changed

19 files changed

+110
-56
lines changed

‎pkg/auth/userregistry/identitymapper/interfaces.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
package identitymapper
22

33
import (
4-
userapi "github.com/openshift/origin/pkg/user/apis/user"
4+
userapi "github.com/openshift/origin/pkg/user/apis/user/v1"
55
)
66

77
type UserToGroupMapper interface {

‎pkg/authorization/admission/restrictusers/groupcache_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
package restrictusers
22

33
import (
4-
userapi "github.com/openshift/origin/pkg/user/apis/user"
4+
userapi "github.com/openshift/origin/pkg/user/apis/user/v1"
55
)
66

77
type fakeGroupCache struct {

‎pkg/authorization/admission/restrictusers/restrictusers.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -14,13 +14,13 @@ import (
1414
"k8s.io/kubernetes/pkg/apis/rbac"
1515
kadmission "k8s.io/kubernetes/pkg/kubeapiserver/admission"
1616

17-
authorizationclient "github.com/openshift/origin/pkg/authorization/generated/internalclientset"
18-
authorizationtypedclient "github.com/openshift/origin/pkg/authorization/generated/internalclientset/typed/authorization/internalversion"
17+
authorizationclient "github.com/openshift/origin/pkg/authorization/generated/clientset"
18+
authorizationtypedclient "github.com/openshift/origin/pkg/authorization/generated/clientset/typed/authorization/v1"
1919
oadmission "github.com/openshift/origin/pkg/cmd/server/admission"
20-
userapi "github.com/openshift/origin/pkg/user/apis/user"
20+
userapi "github.com/openshift/origin/pkg/user/apis/user/v1"
2121
usercache "github.com/openshift/origin/pkg/user/cache"
22-
userinformer "github.com/openshift/origin/pkg/user/generated/informers/internalversion"
23-
userclient "github.com/openshift/origin/pkg/user/generated/internalclientset"
22+
userclient "github.com/openshift/origin/pkg/user/generated/clientset"
23+
userinformer "github.com/openshift/origin/pkg/user/generated/informers/externalversions"
2424
)
2525

2626
func Register(plugins *admission.Plugins) {
@@ -72,7 +72,7 @@ func (q *restrictUsersAdmission) SetOpenshiftInternalUserClient(userClient userc
7272
}
7373

7474
func (q *restrictUsersAdmission) SetUserInformer(userInformers userinformer.SharedInformerFactory) {
75-
q.groupCache = usercache.NewGroupCache(userInformers.User().InternalVersion().Groups())
75+
q.groupCache = usercache.NewGroupCache(userInformers.User().V1().Groups())
7676
}
7777

7878
// subjectsDelta returns the relative complement of elementsToIgnore in

‎pkg/authorization/admission/restrictusers/restrictusers_test.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,11 +15,11 @@ import (
1515
"k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset/fake"
1616
kadmission "k8s.io/kubernetes/pkg/kubeapiserver/admission"
1717

18-
authorizationapi "github.com/openshift/origin/pkg/authorization/apis/authorization"
19-
fakeauthorizationclient "github.com/openshift/origin/pkg/authorization/generated/internalclientset/fake"
18+
authorizationapi "github.com/openshift/origin/pkg/authorization/apis/authorization/v1"
19+
fakeauthorizationclient "github.com/openshift/origin/pkg/authorization/generated/clientset/fake"
2020
oadmission "github.com/openshift/origin/pkg/cmd/server/admission"
21-
userapi "github.com/openshift/origin/pkg/user/apis/user"
22-
fakeuserclient "github.com/openshift/origin/pkg/user/generated/internalclientset/fake"
21+
userapi "github.com/openshift/origin/pkg/user/apis/user/v1"
22+
fakeuserclient "github.com/openshift/origin/pkg/user/generated/clientset/fake"
2323
)
2424

2525
func TestAdmission(t *testing.T) {

‎pkg/authorization/admission/restrictusers/subjectchecker.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,9 @@ import (
99
"k8s.io/kubernetes/pkg/apis/rbac"
1010
kclientset "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset"
1111

12-
authorizationapi "github.com/openshift/origin/pkg/authorization/apis/authorization"
13-
userapi "github.com/openshift/origin/pkg/user/apis/user"
14-
userclient "github.com/openshift/origin/pkg/user/generated/internalclientset/typed/user/internalversion"
12+
authorizationapi "github.com/openshift/origin/pkg/authorization/apis/authorization/v1"
13+
userapi "github.com/openshift/origin/pkg/user/apis/user/v1"
14+
userclient "github.com/openshift/origin/pkg/user/generated/clientset/typed/user/v1"
1515
)
1616

1717
// SubjectChecker determines whether rolebindings on a subject (user, group, or
@@ -47,7 +47,7 @@ func (checkers UnionSubjectChecker) Allowed(subject rbac.Subject, ctx *RoleBindi
4747
// RoleBindingRestrictionContext holds context that is used when determining
4848
// whether a RoleBindingRestriction allows rolebindings on a particular subject.
4949
type RoleBindingRestrictionContext struct {
50-
userClient userclient.UserInterface
50+
userClient userclient.UserV1Interface
5151
kclient kclientset.Interface
5252

5353
// groupCache maps user name to groups.
@@ -66,7 +66,7 @@ type RoleBindingRestrictionContext struct {
6666

6767
// NewRoleBindingRestrictionContext returns a new RoleBindingRestrictionContext
6868
// object.
69-
func NewRoleBindingRestrictionContext(ns string, kc kclientset.Interface, userClient userclient.UserInterface, groupCache GroupCache) (*RoleBindingRestrictionContext, error) {
69+
func NewRoleBindingRestrictionContext(ns string, kc kclientset.Interface, userClient userclient.UserV1Interface, groupCache GroupCache) (*RoleBindingRestrictionContext, error) {
7070
return &RoleBindingRestrictionContext{
7171
namespace: ns,
7272
kclient: kc,

‎pkg/authorization/admission/restrictusers/subjectchecker_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,9 @@ import (
1010
"k8s.io/kubernetes/pkg/apis/rbac"
1111
"k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset/fake"
1212

13-
authorizationapi "github.com/openshift/origin/pkg/authorization/apis/authorization"
14-
userapi "github.com/openshift/origin/pkg/user/apis/user"
15-
fakeuserclient "github.com/openshift/origin/pkg/user/generated/internalclientset/fake"
13+
authorizationapi "github.com/openshift/origin/pkg/authorization/apis/authorization/v1"
14+
userapi "github.com/openshift/origin/pkg/user/apis/user/v1"
15+
fakeuserclient "github.com/openshift/origin/pkg/user/generated/clientset/fake"
1616
)
1717

1818
func mustNewSubjectChecker(t *testing.T, spec *authorizationapi.RoleBindingRestrictionSpec) SubjectChecker {

‎pkg/build/admission/strategyrestrictions/admission.go

Lines changed: 28 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
99
"k8s.io/apimachinery/pkg/runtime/schema"
1010
"k8s.io/apiserver/pkg/admission"
11+
kapi "k8s.io/kubernetes/pkg/api"
1112
kapihelper "k8s.io/kubernetes/pkg/api/helper"
1213
"k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset"
1314
authorizationclient "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset/typed/authorization/internalversion"
@@ -17,7 +18,7 @@ import (
1718
authorizationapi "github.com/openshift/origin/pkg/authorization/apis/authorization"
1819
"github.com/openshift/origin/pkg/authorization/util"
1920
buildapi "github.com/openshift/origin/pkg/build/apis/build"
20-
buildclient "github.com/openshift/origin/pkg/build/generated/internalclientset"
21+
buildclient "github.com/openshift/origin/pkg/build/generated/clientset"
2122
oadmission "github.com/openshift/origin/pkg/cmd/server/admission"
2223
"k8s.io/kubernetes/pkg/apis/authorization"
2324
)
@@ -182,13 +183,21 @@ func (a *buildByStrategy) checkBuildRequestAuthorization(req *buildapi.BuildRequ
182183
if err != nil {
183184
return admission.NewForbidden(attr, err)
184185
}
185-
return a.checkBuildAuthorization(build, attr)
186+
internalBuild := &buildapi.Build{}
187+
if err := kapi.Scheme.Convert(build, internalBuild, nil); err != nil {
188+
return admission.NewForbidden(attr, err)
189+
}
190+
return a.checkBuildAuthorization(internalBuild, attr)
186191
case buildapi.IsResourceOrLegacy("buildconfigs", gr):
187-
build, err := a.buildClient.Build().BuildConfigs(attr.GetNamespace()).Get(req.Name, metav1.GetOptions{})
192+
buildConfig, err := a.buildClient.Build().BuildConfigs(attr.GetNamespace()).Get(req.Name, metav1.GetOptions{})
188193
if err != nil {
189194
return admission.NewForbidden(attr, err)
190195
}
191-
return a.checkBuildConfigAuthorization(build, attr)
196+
internalBuildConfig := &buildapi.BuildConfig{}
197+
if err := kapi.Scheme.Convert(buildConfig, internalBuildConfig, nil); err != nil {
198+
return admission.NewForbidden(attr, err)
199+
}
200+
return a.checkBuildConfigAuthorization(internalBuildConfig, attr)
192201
default:
193202
return admission.NewForbidden(attr, fmt.Errorf("Unknown resource type %s for BuildRequest", attr.GetResource()))
194203
}
@@ -206,5 +215,19 @@ func (a *buildByStrategy) checkAccess(strategy buildapi.BuildStrategy, subjectAc
206215
}
207216

208217
func notAllowed(strategy buildapi.BuildStrategy, attr admission.Attributes) error {
209-
return admission.NewForbidden(attr, fmt.Errorf("build strategy %s is not allowed", buildapi.StrategyType(strategy)))
218+
return admission.NewForbidden(attr, fmt.Errorf("build strategy %s is not allowed", strategyTypeString(strategy)))
219+
}
220+
221+
func strategyTypeString(strategy buildapi.BuildStrategy) string {
222+
switch {
223+
case strategy.DockerStrategy != nil:
224+
return "Docker"
225+
case strategy.CustomStrategy != nil:
226+
return "Custom"
227+
case strategy.SourceStrategy != nil:
228+
return "Source"
229+
case strategy.JenkinsPipelineStrategy != nil:
230+
return "JenkinsPipeline"
231+
}
232+
return ""
210233
}

‎pkg/build/admission/strategyrestrictions/admission_test.go

Lines changed: 28 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -11,13 +11,17 @@ import (
1111
"k8s.io/apiserver/pkg/admission"
1212
"k8s.io/apiserver/pkg/authentication/user"
1313
clientgotesting "k8s.io/client-go/testing"
14+
kapi "k8s.io/kubernetes/pkg/api"
1415
"k8s.io/kubernetes/pkg/apis/authorization"
1516
fakekubeclient "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset/fake"
1617
kubeadmission "k8s.io/kubernetes/pkg/kubeapiserver/admission"
1718

1819
buildapi "github.com/openshift/origin/pkg/build/apis/build"
19-
fakebuildclient "github.com/openshift/origin/pkg/build/generated/internalclientset/fake"
20+
buildapiv1 "github.com/openshift/origin/pkg/build/apis/build/v1"
21+
fakebuildclient "github.com/openshift/origin/pkg/build/generated/clientset/fake"
2022
oadmission "github.com/openshift/origin/pkg/cmd/server/admission"
23+
24+
_ "github.com/openshift/origin/pkg/build/apis/build/install"
2125
)
2226

2327
func TestBuildAdmission(t *testing.T) {
@@ -48,7 +52,7 @@ func TestBuildAdmission(t *testing.T) {
4852
{
4953
name: "allowed source build clone",
5054
object: testBuildRequest("test-build"),
51-
responseObject: testBuild(buildapi.BuildStrategy{SourceStrategy: &buildapi.SourceBuildStrategy{}}),
55+
responseObject: asV1Build(testBuild(buildapi.BuildStrategy{SourceStrategy: &buildapi.SourceBuildStrategy{}})),
5256
kind: buildapi.Kind("Build"),
5357
resource: buildapi.Resource("builds"),
5458
subResource: "clone",
@@ -70,7 +74,7 @@ func TestBuildAdmission(t *testing.T) {
7074
{
7175
name: "denied docker build clone",
7276
object: testBuildRequest("buildname"),
73-
responseObject: testBuild(buildapi.BuildStrategy{DockerStrategy: &buildapi.DockerBuildStrategy{}}),
77+
responseObject: asV1Build(testBuild(buildapi.BuildStrategy{DockerStrategy: &buildapi.DockerBuildStrategy{}})),
7478
kind: buildapi.Kind("Build"),
7579
resource: buildapi.Resource("builds"),
7680
subResource: "clone",
@@ -101,7 +105,7 @@ func TestBuildAdmission(t *testing.T) {
101105
},
102106
{
103107
name: "allowed build config instantiate",
104-
responseObject: testBuildConfig(buildapi.BuildStrategy{DockerStrategy: &buildapi.DockerBuildStrategy{}}),
108+
responseObject: asV1BuildConfig(testBuildConfig(buildapi.BuildStrategy{DockerStrategy: &buildapi.DockerBuildStrategy{}})),
105109
object: testBuildRequest("test-buildconfig"),
106110
kind: buildapi.Kind("Build"),
107111
resource: buildapi.Resource("buildconfigs"),
@@ -123,7 +127,7 @@ func TestBuildAdmission(t *testing.T) {
123127
},
124128
{
125129
name: "forbidden build config instantiate",
126-
responseObject: testBuildConfig(buildapi.BuildStrategy{CustomStrategy: &buildapi.CustomBuildStrategy{}}),
130+
responseObject: asV1BuildConfig(testBuildConfig(buildapi.BuildStrategy{CustomStrategy: &buildapi.CustomBuildStrategy{}})),
127131
object: testBuildRequest("buildname"),
128132
kind: buildapi.Kind("Build"),
129133
resource: buildapi.Resource("buildconfigs"),
@@ -164,7 +168,7 @@ func TestBuildAdmission(t *testing.T) {
164168
{
165169
name: "allowed jenkins pipeline build clone",
166170
object: testBuildRequest("test-build"),
167-
responseObject: testBuild(buildapi.BuildStrategy{JenkinsPipelineStrategy: &buildapi.JenkinsPipelineBuildStrategy{}}),
171+
responseObject: asV1Build(testBuild(buildapi.BuildStrategy{JenkinsPipelineStrategy: &buildapi.JenkinsPipelineBuildStrategy{}})),
168172
kind: buildapi.Kind("Build"),
169173
resource: buildapi.Resource("builds"),
170174
subResource: "clone",
@@ -252,6 +256,15 @@ func testBuild(strategy buildapi.BuildStrategy) *buildapi.Build {
252256
}
253257
}
254258

259+
func asV1Build(in *buildapi.Build) *buildapiv1.Build {
260+
out := &buildapiv1.Build{}
261+
err := kapi.Scheme.Convert(in, out, nil)
262+
if err != nil {
263+
panic(err)
264+
}
265+
return out
266+
}
267+
255268
func testBuildConfig(strategy buildapi.BuildStrategy) *buildapi.BuildConfig {
256269
return &buildapi.BuildConfig{
257270
ObjectMeta: metav1.ObjectMeta{
@@ -266,6 +279,15 @@ func testBuildConfig(strategy buildapi.BuildStrategy) *buildapi.BuildConfig {
266279
}
267280
}
268281

282+
func asV1BuildConfig(in *buildapi.BuildConfig) *buildapiv1.BuildConfig {
283+
out := &buildapiv1.BuildConfig{}
284+
err := kapi.Scheme.Convert(in, out, nil)
285+
if err != nil {
286+
panic(err)
287+
}
288+
return out
289+
}
290+
269291
func reviewResponse(allowed bool, msg string) *authorization.SubjectAccessReview {
270292
return &authorization.SubjectAccessReview{
271293
Status: authorization.SubjectAccessReviewStatus{

‎pkg/build/apis/build/v1/register.go

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,17 @@ func Resource(resource string) schema.GroupResource {
2727
return SchemeGroupVersion.WithResource(resource).GroupResource()
2828
}
2929

30+
// LegacyResource takes an unqualified resource and returns back a Group qualified GroupResource
31+
func LegacyResource(resource string) schema.GroupResource {
32+
return LegacySchemeGroupVersion.WithResource(resource).GroupResource()
33+
}
34+
35+
// IsResourceOrLegacy checks if the provided GroupResources matches with the given
36+
// resource by looking up the API group and also the legacy API.
37+
func IsResourceOrLegacy(resource string, gr schema.GroupResource) bool {
38+
return gr == Resource(resource) || gr == LegacyResource(resource)
39+
}
40+
3041
// addKnownTypes adds types to API group
3142
func addKnownTypes(scheme *runtime.Scheme) error {
3243
scheme.AddKnownTypes(SchemeGroupVersion,

‎pkg/cmd/server/admission/init.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,8 @@ import (
88
kubeapiserveradmission "k8s.io/kubernetes/pkg/kubeapiserver/admission"
99
"k8s.io/kubernetes/pkg/quota"
1010

11-
authorizationclient "github.com/openshift/origin/pkg/authorization/generated/internalclientset"
12-
buildclient "github.com/openshift/origin/pkg/build/generated/internalclientset"
11+
authorizationclient "github.com/openshift/origin/pkg/authorization/generated/clientset"
12+
buildclient "github.com/openshift/origin/pkg/build/generated/clientset"
1313
configapi "github.com/openshift/origin/pkg/cmd/server/api"
1414
imageapi "github.com/openshift/origin/pkg/image/apis/image"
1515
imageclient "github.com/openshift/origin/pkg/image/generated/internalclientset"
@@ -19,8 +19,8 @@ import (
1919
quotaclient "github.com/openshift/origin/pkg/quota/generated/internalclientset"
2020
securityinformer "github.com/openshift/origin/pkg/security/generated/informers/internalversion"
2121
templateclient "github.com/openshift/origin/pkg/template/generated/internalclientset"
22-
userinformer "github.com/openshift/origin/pkg/user/generated/informers/internalversion"
23-
userclient "github.com/openshift/origin/pkg/user/generated/internalclientset"
22+
userclient "github.com/openshift/origin/pkg/user/generated/clientset"
23+
userinformer "github.com/openshift/origin/pkg/user/generated/informers/externalversions"
2424
)
2525

2626
type PluginInitializer struct {

0 commit comments

Comments
 (0)