Skip to content

Commit 6ce0980

Browse files
pwittrockdroot
authored andcommitted
Add validation for informers and rbac annotations.
- Codegeneration to validate rbac annotations exist for informers - Libraries to validate the informer was registered
1 parent 6e01188 commit 6ce0980

File tree

3 files changed

+82
-11
lines changed

3 files changed

+82
-11
lines changed

cmd/internal/codegen/parse/parser.go

Lines changed: 74 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -17,15 +17,16 @@ limitations under the License.
1717
package parse
1818

1919
import (
20+
"bufio"
2021
"go/build"
22+
"log"
23+
"os"
2124
"path/filepath"
2225
"strings"
23-
"os"
24-
"bufio"
2526

2627
"github.com/golang/glog"
27-
2828
"github.com/kubernetes-sigs/kubebuilder/cmd/internal/codegen"
29+
"github.com/markbates/inflect"
2930
"github.com/pkg/errors"
3031
rbacv1 "k8s.io/api/rbac/v1"
3132
"k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -68,11 +69,77 @@ func NewAPIs(context *generator.Context, arguments *args.GeneratorArgs) *APIs {
6869
b.parseControllers()
6970
b.parseRBAC()
7071
b.parseInformers()
72+
b.verifyRBACAnnotations()
7173
b.parseAPIs()
7274
b.parseCRDs()
7375
return b
7476
}
7577

78+
// verifyRBACAnnotations verifies that there are corresponding RBAC annotations for
79+
// each informer annotation.
80+
// e.g. if there is an // +kubebuilder:informer annotation for Pods, then there
81+
// should also be a // +kubebuilder:rbac annotation for Pods
82+
func (b *APIs) verifyRBACAnnotations() {
83+
rs := inflect.NewDefaultRuleset()
84+
85+
// For each informer, look for the RBAC annotation
86+
for gvk := range b.Informers {
87+
found := false
88+
89+
// Search all RBAC rules for one that matches the informer group and resource
90+
for _, rule := range b.Rules {
91+
92+
// Check if the group matches the informer group
93+
groupFound := false
94+
for _, g := range rule.APIGroups {
95+
// RBAC has the full group with domain, whereas informers do not. Strip the domain
96+
// from the group before comparing.
97+
parts := strings.Split(g, ".")
98+
group := parts[len(parts)-1]
99+
100+
// If the RBAC group is wildcard or matches, it is a match
101+
if g == "*" || group == gvk.Group {
102+
groupFound = true
103+
break
104+
}
105+
// Edge case where "core" and "" are equivalent
106+
if (group == "core" || group == "") && (gvk.Group == "core" || gvk.Group == "") {
107+
groupFound = true
108+
break
109+
}
110+
}
111+
if !groupFound {
112+
continue
113+
}
114+
115+
// Check if the resource matches the informer resource
116+
resourceFound := false
117+
for _, k := range rule.Resources {
118+
// The resource name is the lower-plural of the Kind
119+
resource := rs.Pluralize(strings.ToLower(gvk.Kind))
120+
121+
// If the RBAC resource is a wildcard or matches the informer resource, it is a match
122+
if k == "*" || k == resource {
123+
resourceFound = true
124+
break
125+
}
126+
}
127+
if !resourceFound {
128+
continue
129+
}
130+
131+
// Found a matching RBAC rule
132+
found = true
133+
break
134+
}
135+
if !found {
136+
log.Fatalf("Missing rbac rule for %s.%s. Add with // +kubebuilder:rbac:groups=%s,"+
137+
"resources=%s,verbs=get;list;watch", gvk.Group, gvk.Kind, gvk.Group,
138+
inflect.NewDefaultRuleset().Pluralize(strings.ToLower(gvk.Kind)))
139+
}
140+
}
141+
}
142+
76143
// parseGroupNames initializes b.GroupNames with the set of all groups
77144
func (b *APIs) parseGroupNames() {
78145
b.GroupNames = sets.String{}
@@ -129,10 +196,10 @@ func (b *APIs) parseDomain() {
129196

130197
func parseDomainFromFiles(paths []string) string {
131198
var domain string
132-
for _, path := range paths {
133-
if strings.HasSuffix(path, "pkg/apis") {
134-
filePath := strings.Join([]string{build.Default.GOPATH, "src", path, "doc.go"}, "/")
135-
lines := []string{}
199+
for _, path := range paths {
200+
if strings.HasSuffix(path, "pkg/apis") {
201+
filePath := strings.Join([]string{build.Default.GOPATH, "src", path, "doc.go"}, "/")
202+
lines := []string{}
136203

137204
file, err := os.Open(filePath)
138205
if err != nil {

cmd/internal/codegen/parse/rbac.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ func parseRBACTag(tag string) rbacv1.PolicyRule {
7272
normalized = append(normalized, v)
7373
}
7474
}
75-
result.APIGroups = values
75+
result.APIGroups = normalized
7676
case "resources":
7777
result.Resources = values
7878
case "verbs":

pkg/controller/manager.go

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,11 @@ func (m *ControllerManager) GetInformer(object metav1.Object) cache.SharedInform
8282
// GetInformerProvider returns the InformerProvider for the object type
8383
func (m *ControllerManager) GetInformerProvider(object metav1.Object) informers.InformerProvider {
8484
m.once.Do(m.init)
85-
return m.sharedInformersByResource.GetInformerProvider(object)
85+
si := m.sharedInformersByResource.GetInformerProvider(object)
86+
if si == nil {
87+
warningMissingInformer(object)
88+
}
89+
return si
8690
}
8791

8892
// GetInformerProvider returns the InformerProvider for the object type.
@@ -152,8 +156,8 @@ func warningMissingInformer(obj interface{}) {
152156

153157
// Create a helpful error message
154158
msg := fmt.Sprintf("\nWARNING: %s\nWARNING: Informer for %s.%s.%s not registered! "+
155-
"Must register informer with a // +kubebuilder:informers:group=%s,version=%s,kind=%s annotation on the"+
156-
"Controller struct.\n",
159+
"Must register informer with a // +kubebuilder:informers:group=%s,version=%s,kind=%s annotation on the "+
160+
"Controller struct and then run `kubebuilder generate`.\n",
157161
provideControllerLine(), group, version, kind, group, version, kind)
158162
fmt.Fprint(os.Stderr, msg)
159163
}

0 commit comments

Comments
 (0)