Skip to content

Commit

Permalink
Merge pull request brancz#32 from stlaz/lib_go_validation
Browse files Browse the repository at this point in the history
move username and groupname validation to apiserver-library-go
  • Loading branch information
openshift-merge-robot authored Dec 4, 2020
2 parents 7a3ac7c + f3e956b commit 9abb36a
Show file tree
Hide file tree
Showing 10 changed files with 261 additions and 44 deletions.
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ require (
github.com/google/uuid v1.1.1
github.com/jteeuwen/go-bindata v3.0.8-0.20151023091102-a0ff2567cfb7+incompatible
github.com/openshift/api v0.0.0-20201120165435-072a4cd8ca42
github.com/openshift/apiserver-library-go v0.0.0-20201204115753-d48a1b462aa6
github.com/openshift/build-machinery-go v0.0.0-20200917070002-f171684f77ab
github.com/openshift/client-go v0.0.0-20201120192246-6aaf557bace9
github.com/openshift/library-go v0.0.0-20201102091359-c4fa0f5b3a08
Expand All @@ -25,7 +26,6 @@ require (
k8s.io/client-go v0.19.2
k8s.io/code-generator v0.19.2
k8s.io/component-base v0.19.2
k8s.io/klog/v2 v2.3.0
k8s.io/kube-aggregator v0.19.2
k8s.io/kube-openapi v0.0.0-20200805222855-6aeccd4b50c6
k8s.io/kubernetes v1.19.2
Expand Down
6 changes: 6 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -466,16 +466,20 @@ github.com/opencontainers/selinux v1.5.2/go.mod h1:yTcKuYAh6R95iDpefGLQaPaRwJFwy
github.com/openshift/api v0.0.0-20201019163320-c6a5ec25f267/go.mod h1:RDvBcRQMGLa3aNuDuejVBbTEQj/2i14NXdpOLqbNBvM=
github.com/openshift/api v0.0.0-20201120165435-072a4cd8ca42 h1:meFswbseUxIkrfb2+g91gHbPwh+16Kj/8E1AiR1jv6A=
github.com/openshift/api v0.0.0-20201120165435-072a4cd8ca42/go.mod h1:RDvBcRQMGLa3aNuDuejVBbTEQj/2i14NXdpOLqbNBvM=
github.com/openshift/apiserver-library-go v0.0.0-20201204115753-d48a1b462aa6 h1:FmkTaJKn0c6gKq8Ryn3KYjQG9lCHKfprC0bq+1fln/g=
github.com/openshift/apiserver-library-go v0.0.0-20201204115753-d48a1b462aa6/go.mod h1:fGp6VNAiZ3Uzcyxgj5CahXv/+BrxyaAhXXRWWffgxwc=
github.com/openshift/build-machinery-go v0.0.0-20200917070002-f171684f77ab h1:lBrojddP6C9C2p67EMs2vcdpC8eF+H0DDom+fgI2IF0=
github.com/openshift/build-machinery-go v0.0.0-20200917070002-f171684f77ab/go.mod h1:b1BuldmJlbA/xYtdZvKi+7j5YGB44qJUJDZ9zwiNCfE=
github.com/openshift/client-go v0.0.0-20201020074620-f8fd44879f7c h1:NB9g4Y/aegId7fyNqYyGxEfyNOytYFT5dxWJtfOJFQs=
github.com/openshift/client-go v0.0.0-20201020074620-f8fd44879f7c/go.mod h1:yZ3u8vgWC19I9gbDMRk8//9JwG/0Sth6v7C+m6R8HXs=
github.com/openshift/client-go v0.0.0-20201020082437-7737f16e53fc/go.mod h1:yZ3u8vgWC19I9gbDMRk8//9JwG/0Sth6v7C+m6R8HXs=
github.com/openshift/client-go v0.0.0-20201120192246-6aaf557bace9 h1:jqa3ZnPt/jDKvKrkgNJfyDBChB8Qw3A2aXUSIzrgCXk=
github.com/openshift/client-go v0.0.0-20201120192246-6aaf557bace9/go.mod h1:Zwzg4+Ye3sD5Df2SMB/XVU42TenqXLBF8T7F/wi7lGo=
github.com/openshift/kubernetes-apiserver v0.0.0-20201118100029-304f639eba13 h1:vl8/Ex1dQNoWOc+YrHTeBfuD1Ap1sBGkmSIDu8mDXxA=
github.com/openshift/kubernetes-apiserver v0.0.0-20201118100029-304f639eba13/go.mod h1:FreAq0bJ2vtZFj9Ago/X0oNGC51GfubKK/ViOKfVAOA=
github.com/openshift/kubernetes-client-go v0.0.0-20201104094117-806c7d66cfea h1:MY3sLcj2kfsjN36hEs0736bcyNFdUAOQLHXNL9u3+bc=
github.com/openshift/kubernetes-client-go v0.0.0-20201104094117-806c7d66cfea/go.mod h1:S5wPhCqyDNAlzM9CnEdgTGV4OqhsW3jGO1UM1epwfJA=
github.com/openshift/library-go v0.0.0-20201020083322-646ad9742a1e/go.mod h1:qbwvTwCy4btqEcqU3oI59CopNgcRgZUPXG4Y2jc+B4E=
github.com/openshift/library-go v0.0.0-20201102091359-c4fa0f5b3a08 h1:Z+8t3ooTH2T+J/GoCZbgaOk5WqNZgPuHlUAKMfG1FEk=
github.com/openshift/library-go v0.0.0-20201102091359-c4fa0f5b3a08/go.mod h1:1xYaYQcQsn+AyCRsvOU+Qn5z6GGiCmcblXkT/RZLVfo=
github.com/pborman/uuid v1.2.0/go.mod h1:X/NO0urCmaxf9VXbdlT7C2Yzkj2IKimNn4k+gtPdI/k=
Expand Down Expand Up @@ -607,6 +611,8 @@ go.uber.org/atomic v1.4.0 h1:cxzIVoETapQEqDhQu3QfnvXAV4AlzcvUCxkVUFw3+EU=
go.uber.org/atomic v1.4.0/go.mod h1:gD2HeocX3+yG+ygLZcrzQJaqmWj9AIm7n08wl/qW/PE=
go.uber.org/multierr v1.1.0 h1:HoEmRHQPVSqub6w2z2d2EOVs2fjyFRGyofhKuyDq0QI=
go.uber.org/multierr v1.1.0/go.mod h1:wR5kodmAFQ0UK8QlbwjlSNy0Z68gJhDJUG5sjR94q/0=
go.uber.org/multierr v1.1.1-0.20180122172545-ddea229ff1df h1:ijDSp1iOMDAWixcZLxdmOBE0N7YTvtV4s2HWE3U1CoQ=
go.uber.org/multierr v1.1.1-0.20180122172545-ddea229ff1df/go.mod h1:wR5kodmAFQ0UK8QlbwjlSNy0Z68gJhDJUG5sjR94q/0=
go.uber.org/zap v1.10.0 h1:ORx85nbTijNz8ljznvCMR1ZBIPKFn3jQrag10X2AsuM=
go.uber.org/zap v1.10.0/go.mod h1:vwi/ZaCAaUcBkycHslxD9B2zi4UTXhF60s6SWpuDF0Q=
golang.org/x/crypto v0.0.0-20180904163835-0709b304e793/go.mod h1:6SG95UA2DQfeDnfUPMdvaQW0Q7yPrPDi9nlGo2tz2b4=
Expand Down
4 changes: 2 additions & 2 deletions pkg/oauth/apis/oauth/validation/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,10 @@ import (
"k8s.io/apiserver/pkg/authentication/serviceaccount"

routev1 "github.com/openshift/api/route/v1"
"github.com/openshift/apiserver-library-go/pkg/apivalidation"
bootstrap "github.com/openshift/library-go/pkg/authentication/bootstrapauthenticator"
scopemetadata "github.com/openshift/library-go/pkg/authorization/scopemetadata"
oauthapi "github.com/openshift/oauth-apiserver/pkg/oauth/apis/oauth"
uservalidation "github.com/openshift/oauth-apiserver/pkg/user/apis/user/validation"
)

const (
Expand Down Expand Up @@ -342,7 +342,7 @@ func ValidateUserNameField(value string, fldPath *field.Path) field.ErrorList {
if value == bootstrap.BootstrapUser {
return field.ErrorList{}
}
if reasons := uservalidation.ValidateUserName(value, false); len(reasons) != 0 {
if reasons := apivalidation.ValidateUserName(value, false); len(reasons) != 0 {
return field.ErrorList{field.Invalid(fldPath, value, strings.Join(reasons, ", "))}
}
return field.ErrorList{}
Expand Down
41 changes: 7 additions & 34 deletions pkg/user/apis/user/validation/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,23 +8,10 @@ import (
"k8s.io/apimachinery/pkg/api/validation/path"
"k8s.io/apimachinery/pkg/util/validation/field"

"github.com/openshift/apiserver-library-go/pkg/apivalidation"
userapi "github.com/openshift/oauth-apiserver/pkg/user/apis/user"
)

func ValidateUserName(name string, _ bool) []string {
if reasons := path.ValidatePathSegmentName(name, false); len(reasons) != 0 {
return reasons
}

if strings.Contains(name, ":") {
return []string{`may not contain ":"`}
}
if name == "~" {
return []string{`may not equal "~"`}
}
return nil
}

func ValidateIdentityName(name string, _ bool) []string {
if reasons := path.ValidatePathSegmentName(name, false); len(reasons) != 0 {
return reasons
Expand All @@ -43,20 +30,6 @@ func ValidateIdentityName(name string, _ bool) []string {
return nil
}

func ValidateGroupName(name string, _ bool) []string {
if reasons := path.ValidatePathSegmentName(name, false); len(reasons) != 0 {
return reasons
}

if strings.Contains(name, ":") {
return []string{`may not contain ":"`}
}
if name == "~" {
return []string{`may not equal "~"`}
}
return nil
}

// if you change this, update the peer in oauth admission validation. also, don't change this.
func ValidateIdentityProviderName(name string) []string {
if reasons := path.ValidatePathSegmentName(name, false); len(reasons) != 0 {
Expand All @@ -71,11 +44,11 @@ func ValidateIdentityProviderName(name string) []string {

func ValidateIdentityProviderUserName(name string) []string {
// Any provider user name must be a valid user name
return ValidateUserName(name, false)
return apivalidation.ValidateUserName(name, false)
}

func ValidateGroup(group *userapi.Group) field.ErrorList {
allErrs := kvalidation.ValidateObjectMeta(&group.ObjectMeta, false, ValidateGroupName, field.NewPath("metadata"))
allErrs := kvalidation.ValidateObjectMeta(&group.ObjectMeta, false, apivalidation.ValidateGroupName, field.NewPath("metadata"))

userPath := field.NewPath("user")
for index, user := range group.Users {
Expand All @@ -84,7 +57,7 @@ func ValidateGroup(group *userapi.Group) field.ErrorList {
allErrs = append(allErrs, field.Invalid(idxPath, user, "may not be empty"))
continue
}
if reasons := ValidateUserName(user, false); len(reasons) != 0 {
if reasons := apivalidation.ValidateUserName(user, false); len(reasons) != 0 {
allErrs = append(allErrs, field.Invalid(idxPath, user, strings.Join(reasons, ", ")))
}
}
Expand All @@ -99,7 +72,7 @@ func ValidateGroupUpdate(group *userapi.Group, old *userapi.Group) field.ErrorLi
}

func ValidateUser(user *userapi.User) field.ErrorList {
allErrs := kvalidation.ValidateObjectMeta(&user.ObjectMeta, false, ValidateUserName, field.NewPath("metadata"))
allErrs := kvalidation.ValidateObjectMeta(&user.ObjectMeta, false, apivalidation.ValidateUserName, field.NewPath("metadata"))
identitiesPath := field.NewPath("identities")
for index, identity := range user.Identities {
idxPath := identitiesPath.Index(index)
Expand Down Expand Up @@ -145,7 +118,7 @@ func ValidateIdentity(identity *userapi.Identity) field.ErrorList {
}

userPath := field.NewPath("user")
if reasons := ValidateUserName(identity.User.Name, false); len(reasons) != 0 {
if reasons := apivalidation.ValidateUserName(identity.User.Name, false); len(reasons) != 0 {
allErrs = append(allErrs, field.Invalid(userPath.Child("name"), identity.User.Name, strings.Join(reasons, ", ")))
}
if len(identity.User.Name) == 0 && len(identity.User.UID) != 0 {
Expand Down Expand Up @@ -184,7 +157,7 @@ func ValidateUserIdentityMapping(mapping *userapi.UserIdentityMapping) field.Err

if len(mapping.User.Name) == 0 {
allErrs = append(allErrs, field.Required(field.NewPath("user", "name"), ""))
} else if reasons := ValidateUserName(mapping.User.Name, false); len(reasons) != 0 {
} else if reasons := apivalidation.ValidateUserName(mapping.User.Name, false); len(reasons) != 0 {
allErrs = append(allErrs, field.Invalid(field.NewPath("user", "name"), mapping.User.Name, strings.Join(reasons, ", ")))
}

Expand Down
6 changes: 3 additions & 3 deletions pkg/user/apiserver/registry/user/etcd/etcd.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,10 @@ import (
"k8s.io/apiserver/pkg/registry/rest"

usergroup "github.com/openshift/api/user"
"github.com/openshift/apiserver-library-go/pkg/apivalidation"
"github.com/openshift/oauth-apiserver/pkg/printers"
"github.com/openshift/oauth-apiserver/pkg/printerstorage"
userapi "github.com/openshift/oauth-apiserver/pkg/user/apis/user"
"github.com/openshift/oauth-apiserver/pkg/user/apis/user/validation"
"github.com/openshift/oauth-apiserver/pkg/user/apiserver/registry/user"
userprinters "github.com/openshift/oauth-apiserver/pkg/user/printers/internalversion"
)
Expand Down Expand Up @@ -68,7 +68,7 @@ func (r *REST) Get(ctx context.Context, name string, options *metav1.GetOptions)
// build a virtual user object using the context data
virtualUser := &userapi.User{ObjectMeta: metav1.ObjectMeta{Name: name, UID: types.UID(user.GetUID())}, Groups: contextGroups}

if reasons := validation.ValidateUserName(name, false); len(reasons) != 0 {
if reasons := apivalidation.ValidateUserName(name, false); len(reasons) != 0 {
// The user the authentication layer has identified cannot be a valid persisted user
// Return an API representation of the virtual user
return virtualUser, nil
Expand Down Expand Up @@ -101,7 +101,7 @@ func (r *REST) Get(ctx context.Context, name string, options *metav1.GetOptions)

// do not bother looking up users that cannot be persisted
// make sure we return a status error otherwise the API server will complain
if reasons := validation.ValidateUserName(name, false); len(reasons) != 0 {
if reasons := apivalidation.ValidateUserName(name, false); len(reasons) != 0 {
err := field.Invalid(field.NewPath("metadata", "name"), name, strings.Join(reasons, ", "))
return nil, kerrs.NewInvalid(usergroup.Kind("User"), name, field.ErrorList{err})
}
Expand Down
Loading

0 comments on commit 9abb36a

Please sign in to comment.