Skip to content

Commit

Permalink
added unit test for RBAC annotation validation
Browse files Browse the repository at this point in the history
  • Loading branch information
droot committed May 18, 2018
1 parent 6ce0980 commit 9025ebf
Show file tree
Hide file tree
Showing 4 changed files with 119 additions and 8 deletions.
20 changes: 14 additions & 6 deletions cmd/internal/codegen/parse/parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package parse

import (
"bufio"
"fmt"
"go/build"
"log"
"os"
Expand Down Expand Up @@ -80,14 +81,21 @@ func NewAPIs(context *generator.Context, arguments *args.GeneratorArgs) *APIs {
// e.g. if there is an // +kubebuilder:informer annotation for Pods, then there
// should also be a // +kubebuilder:rbac annotation for Pods
func (b *APIs) verifyRBACAnnotations() {
err := rbacMatchesInformers(b.Informers, b.Rules)
if err != nil {
log.Fatal(err)
}
}

func rbacMatchesInformers(informers map[v1.GroupVersionKind]bool, rbacRules []rbacv1.PolicyRule) error {
rs := inflect.NewDefaultRuleset()

// For each informer, look for the RBAC annotation
for gvk := range b.Informers {
for gvk := range informers {
found := false

// Search all RBAC rules for one that matches the informer group and resource
for _, rule := range b.Rules {
for _, rule := range rbacRules {

// Check if the group matches the informer group
groupFound := false
Expand All @@ -112,12 +120,11 @@ func (b *APIs) verifyRBACAnnotations() {
continue
}

// The resource name is the lower-plural of the Kind
resource := rs.Pluralize(strings.ToLower(gvk.Kind))
// Check if the resource matches the informer resource
resourceFound := false
for _, k := range rule.Resources {
// The resource name is the lower-plural of the Kind
resource := rs.Pluralize(strings.ToLower(gvk.Kind))

// If the RBAC resource is a wildcard or matches the informer resource, it is a match
if k == "*" || k == resource {
resourceFound = true
Expand All @@ -133,11 +140,12 @@ func (b *APIs) verifyRBACAnnotations() {
break
}
if !found {
log.Fatalf("Missing rbac rule for %s.%s. Add with // +kubebuilder:rbac:groups=%s,"+
return fmt.Errorf("Missing rbac rule for %s.%s. Add with // +kubebuilder:rbac:groups=%s,"+
"resources=%s,verbs=get;list;watch", gvk.Group, gvk.Kind, gvk.Group,
inflect.NewDefaultRuleset().Pluralize(strings.ToLower(gvk.Kind)))
}
}
return nil
}

// parseGroupNames initializes b.GroupNames with the set of all groups
Expand Down
96 changes: 96 additions & 0 deletions cmd/internal/codegen/parse/parser_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
/*
Copyright 2018 The Kubernetes Authors.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/
package parse

import (
"testing"

rbacv1 "k8s.io/api/rbac/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1"
)

func TestrbacMatchesInformers(t *testing.T) {
tests := []struct {
informers map[v1.GroupVersionKind]bool
rbacRules []rbacv1.PolicyRule
expErr bool
}{
{
// informer resource matches the RBAC rule
informers: map[v1.GroupVersionKind]bool{
v1.GroupVersionKind{Group: "apps", Version: "v1", Kind: "Deployment"}: true,
},
rbacRules: []rbacv1.PolicyRule{
{APIGroups: []string{"apps"}, Resources: []string{"deployments"}},
},
expErr: false,
},
{
// RBAC rule does not match the informer resource because of missing pluralization in RBAC rules
informers: map[v1.GroupVersionKind]bool{
v1.GroupVersionKind{Group: "apps", Version: "v1", Kind: "Deployment"}: true,
},
rbacRules: []rbacv1.PolicyRule{
{APIGroups: []string{"apps"}, Resources: []string{"Deployment"}},
},
expErr: true,
},
{
// wild-card RBAC rule should match any resource in the group
informers: map[v1.GroupVersionKind]bool{
v1.GroupVersionKind{Group: "apps", Version: "v1", Kind: "StatefulSet"}: true,
v1.GroupVersionKind{Group: "apps", Version: "v1", Kind: "Deployment"}: true,
},
rbacRules: []rbacv1.PolicyRule{
{APIGroups: []string{"apps"}, Resources: []string{"*"}},
},
expErr: false,
},
{
// empty group name is normalized to "core"
informers: map[v1.GroupVersionKind]bool{
v1.GroupVersionKind{Group: "core", Version: "v1", Kind: "Pod"}: true,
},
rbacRules: []rbacv1.PolicyRule{
{APIGroups: []string{""}, Resources: []string{"pods"}},
},
expErr: false,
},
{
// empty group name is normalized to "core"
informers: map[v1.GroupVersionKind]bool{
v1.GroupVersionKind{Group: "", Version: "v1", Kind: "Pod"}: true,
},
rbacRules: []rbacv1.PolicyRule{
{APIGroups: []string{"core"}, Resources: []string{"pods"}},
},
expErr: false,
},
}

for _, test := range tests {
err := checkRBACMatchesInformers(test.informers, test.rbacRules)
if test.expErr {
if err == nil {
t.Errorf("RBAC rules %+v shouldn't match with informers %+v", test.rbacRules, test.informers)
}
} else {
if err != nil {
t.Errorf("RBAC rules %+v should match informers %+v, but got a mismatch error: %v", test.rbacRules, test.informers, err)
}
}
}
}
1 change: 1 addition & 0 deletions cmd/kubebuilder/create/controller/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ func (bc *{{.Kind}}Controller) Reconcile(k types.ReconcileKey) error {
return nil
}
{{if .CoreType}}// +kubebuilder:informers:group={{ .Group }},version={{ .Version }},kind={{ .Kind }}{{end}}
{{if .CoreType}}// +kubebuilder:rbac:groups={{ .Group }},resources={{ .Resource }},verbs=get;watch;list{{end}}
// +kubebuilder:controller:group={{ .Group }},version={{ .Version }},kind={{ .Kind}},resource={{ .Resource }}
type {{.Kind}}Controller struct {
// INSERT ADDITIONAL FIELDS HERE
Expand Down
10 changes: 8 additions & 2 deletions test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -136,14 +136,18 @@ function prepare_testdir_under_gopath {
header_text "running kubebuilder commands in test directory $kb_test_dir"
}

function generate_crd_resources {
header_text "generating CRD resources and code"
function setup_envs {
header_text "setting up env vars"

# Setup env vars
export PATH=/tmp/kubebuilder/bin/:$PATH
export TEST_ASSET_KUBECTL=/tmp/kubebuilder/bin/kubectl
export TEST_ASSET_KUBE_APISERVER=/tmp/kubebuilder/bin/kube-apiserver
export TEST_ASSET_ETCD=/tmp/kubebuilder/bin/etcd
}

function generate_crd_resources {
header_text "generating CRD resources and code"

# Run the commands
kubebuilder init repo --domain sample.kubernetes.io
Expand Down Expand Up @@ -582,6 +586,8 @@ prepare_staging_dir
fetch_tools
build_kb

setup_envs

prepare_testdir_under_gopath
generate_crd_resources
generate_controller
Expand Down

0 comments on commit 9025ebf

Please sign in to comment.