knative
/
pkg
mirror of https://github.com/knative/pkg.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

codegen - support multiple annotation keys (#2350) 

* allow multiple values for comment tag keys

this will allow us to support multiple annotation keys

* add OR filter chain

* update codegen to support multiple annotation keys

* Preserve previous code & formatting if the number of annotation keys is 1

- preserve the order of the annotations vs. sorting it
- deprecate and don't remove ClassAnnotationKey to allow migration to happen smoothly

* fix default value for krshape

* include clarifying comment
This commit is contained in:
Dave Protasowski 2021-11-17 16:53:28 -05:00 committed by GitHub
parent 22c0eba0ca
commit 5708c4c442
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 178 additions and 73 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

57
codegen/cmd/injection-gen/generators/comment_parser.go
View File

@ -19,25 +19,41 @@ import "strings"
// Adapted from the k8s.io comment parser https://github.com/kubernetes/gengo/blob/master/types/comments.go
// CommentsTags maps marker prefixes to a set of tags containing keys and values
type CommentTags map[string]CommentTag
// CommentTags maps keys to a list of values
type CommentTag map[string][]string
// ExtractCommentTags parses comments for lines of the form:
//
// 'marker' + ':' "key=value,key2=value2".
// "marker" + "prefix" + ':' + "key=value,key2=value2".
//
// Values are optional; empty map is the default. A tag can be specified more than
// In the following example the marker is '+' and the prefix is 'foo':
// +foo:key=value1,key2=value2,key=value3
//
// Values are optional; empty map is the default. A tag can be specified more than
// one time and all values are returned. If the resulting map has an entry for
// a key, the value (a slice) is guaranteed to have at least 1 element.
//
// Example: if you pass "+" for 'marker', and the following lines are in
// the comments:
// +foo:key=value1,key2=value2
// +foo:key=value1,key2=value2,key=value3
// +bar
//
// Then this function will return:
// map[string]map[string]string{"foo":{"key":value1","key2":"value2"}, "bar": nil}
// map[string]map[string]string{
// "foo":{
// "key": []string{"value1", "value3"},
// "key2": []string{"value2"}
// },
// "bar": {},
// }
//
// Users are not expected to repeat values.
func ExtractCommentTags(marker string, lines []string) map[string]map[string]string {
out := map[string]map[string]string{}
func ExtractCommentTags(marker string, lines []string) CommentTags {
out := CommentTags{}
for _, line := range lines {
line = strings.TrimSpace(line)
if len(line) == 0 || !strings.HasPrefix(line, marker) {
@ -45,29 +61,32 @@ func ExtractCommentTags(marker string, lines []string) map[string]map[string]str
}
options := strings.SplitN(line[len(marker):], ":", 2)
prefix := options[0]
if len(options) == 2 {
vals := strings.Split(options[1], ",")
opts := out[options[0]]
opts := out[prefix]
if opts == nil {
opts = make(map[string]string, len(vals))
opts = make(CommentTag, len(vals))
out[prefix] = opts
}
for _, pair := range vals {
if kv := strings.SplitN(pair, "=", 2); len(kv) == 2 {
opts[kv[0]] = kv[1]
} else if kv[0] != "" {
opts[kv[0]] = ""
kv := strings.SplitN(pair, "=", 2)
if len(kv) == 1 && kv[0] == "" {
continue
}
if _, ok := opts[kv[0]]; !ok {
opts[kv[0]] = []string{}
}
if len(kv) == 2 {
opts[kv[0]] = append(opts[kv[0]], kv[1])
}
}
if len(opts) == 0 {
out[options[0]] = nil
} else {
out[options[0]] = opts
}
} else if len(options) == 1 && options[0] != "" {
if _, has := out[options[0]]; !has {
out[options[0]] = nil
if _, has := out[prefix]; !has {
out[prefix] = CommentTag{}
}
}
}

66
codegen/cmd/injection-gen/generators/comment_parser_test.go
View File

@ -15,7 +15,11 @@ limitations under the License.
*/
package generators
import "testing"
import (
"testing"
"github.com/google/go-cmp/cmp"
)
func TestParseComments(t *testing.T) {
comment := []string{
@ -26,28 +30,31 @@ func TestParseComments(t *testing.T) {
"+with:option",
"+pair:key=value",
"+manypairs:key1=value1,key2=value2",
"+duplicate:key1=value1,key1=value2",
}
extracted := ExtractCommentTags("+", comment)
if val, ok := extracted["foo"]; !ok || val != nil {
t.Errorf("Failed to extract single key got=%t,%v want=true,nil", ok, val)
expected := CommentTags{
"foo": {},
"bar": {},
"with": {
"option": {},
},
"pair": {
"key": []string{"value"},
},
"manypairs": {
"key1": []string{"value1"},
"key2": []string{"value2"},
},
"duplicate": {
"key1": []string{"value1", "value2"},
},
}
if val, ok := extracted["bar"]; !ok || val != nil {
t.Errorf("Failed to extract single key got=%t,%v want=true,nil", ok, val)
}
if val, ok := extracted["with"]; !ok || val["option"] != "" {
t.Errorf(`Failed to extract single key got=%t,%v want=true,{"option":""}`, ok, val)
}
if val, ok := extracted["pair"]; !ok || val["key"] != "value" {
t.Errorf(`Failed to extract single key got=%t,%v want=true,{"key":"value"}`, ok, val)
}
if val, ok := extracted["manypairs"]; !ok || val["key1"] != "value1" || val["key2"] != "value2" {
t.Errorf(`Failed to extract single key got=%t,%v want=true,{"key":"value"}`, ok, val)
if diff := cmp.Diff(expected, extracted); diff != "" {
t.Error("diff (-want, +got): ", diff)
}
}
@ -61,24 +68,25 @@ func TestMergeDuplicates(t *testing.T) {
"+bar",
"+manypairs:key1=value1",
"+manypairs:key2=value2",
"+manypairs:key1=value3",
"+oops:,,,",
}
extracted := ExtractCommentTags("+", comment)
if val, ok := extracted["foo"]; !ok || val != nil {
t.Errorf("Failed to extract single key got=%t,%v want=true,nil", ok, val)
expected := CommentTags{
"foo": {},
"bar": {
"key": []string{"value"},
},
"manypairs": {
"key1": []string{"value1", "value3"},
"key2": []string{"value2"},
},
"oops": {},
}
if val, ok := extracted["bar"]; !ok || val["key"] != "value" {
t.Errorf(`Failed to extract single key got=%t,%v want=true,{"key":"value"}`, ok, val)
}
if val, ok := extracted["manypairs"]; !ok || val["key1"] != "value1" || val["key2"] != "value2" {
t.Errorf(`Failed to extract single key got=%t,%v want=true,{"key":"value"}`, ok, val)
}
if val, ok := extracted["oops"]; !ok || val != nil {
t.Errorf(`Failed to extract single key got=%t,%v want=true,{"oops":nil}`, ok, val)
if diff := cmp.Diff(expected, extracted); diff != "" {
t.Error("diff (-want, +got): ", diff)
}
}

33
codegen/cmd/injection-gen/generators/packages.go
View File

@ -193,29 +193,36 @@ func MustParseClientGenTags(lines []string) Tags {
return ret
}
func extractCommentTags(t *types.Type) map[string]map[string]string {
func extractCommentTags(t *types.Type) CommentTags {
comments := append(append([]string{}, t.SecondClosestCommentLines...), t.CommentLines...)
return ExtractCommentTags("+", comments)
}
func extractReconcilerClassTag(tags map[string]map[string]string) (string, bool) {
func extractReconcilerClassesTag(tags CommentTags) ([]string, bool) {
vals, ok := tags["genreconciler"]
if !ok {
return "", false
return nil, false
}
classname, has := vals["class"]
return classname, has
classnames, has := vals["class"]
if has && len(classnames) == 0 {
return nil, false
}
return classnames, has
}
func isKRShaped(tags map[string]map[string]string) bool {
func isKRShaped(tags CommentTags) bool {
vals, has := tags["genreconciler"]
if !has {
return false
}
return vals["krshapedlogic"] != "false"
stringVals, has := vals["krshapedlogic"]
if !has || len(vals) == 0 {
return true // Default is true
}
return stringVals[0] != "false"
}
func isNonNamespaced(tags map[string]map[string]string) bool {
func isNonNamespaced(tags CommentTags) bool {
vals, has := tags["genclient"]
if !has {
return false
@ -224,7 +231,7 @@ func isNonNamespaced(tags map[string]map[string]string) bool {
return has
}
func stubs(tags map[string]map[string]string) bool {
func stubs(tags CommentTags) bool {
vals, has := tags["genreconciler"]
if !has {
return false
@ -545,7 +552,7 @@ func reconcilerPackages(basePackage string, groupPkgName string, gv clientgentyp
// Fix for golang iterator bug.
t := t
extracted := extractCommentTags(t)
reconcilerClass, hasReconcilerClass := extractReconcilerClassTag(extracted)
reconcilerClasses, hasReconcilerClass := extractReconcilerClassesTag(extracted)
nonNamespaced := isNonNamespaced(extracted)
isKRShaped := isKRShaped(extracted)
stubs := stubs(extracted)
@ -573,7 +580,7 @@ func reconcilerPackages(basePackage string, groupPkgName string, gv clientgentyp
clientPkg: clientPackagePath,
informerPackagePath: informerPackagePath,
schemePkg: filepath.Join(customArgs.VersionedClientSetPackage, "scheme"),
reconcilerClass: reconcilerClass,
reconcilerClasses: reconcilerClasses,
hasReconcilerClass: hasReconcilerClass,
hasStatus: hasStatus(t),
})
@ -602,7 +609,7 @@ func reconcilerPackages(basePackage string, groupPkgName string, gv clientgentyp
outputPackage: filepath.Join(packagePath, "stub"),
imports: generator.NewImportTracker(),
informerPackagePath: informerPackagePath,
reconcilerClass: reconcilerClass,
reconcilerClasses: reconcilerClasses,
hasReconcilerClass: hasReconcilerClass,
})
return generators
@ -633,7 +640,7 @@ func reconcilerPackages(basePackage string, groupPkgName string, gv clientgentyp
listerPkg: listerPackagePath,
groupGoName: groupGoName,
groupVersion: gv,
reconcilerClass: reconcilerClass,
reconcilerClasses: reconcilerClasses,
hasReconcilerClass: hasReconcilerClass,
nonNamespaced: nonNamespaced,
isKRShaped: isKRShaped,

20
codegen/cmd/injection-gen/generators/reconciler_controller.go
View File

@ -38,7 +38,7 @@ type reconcilerControllerGenerator struct {
schemePkg string
informerPackagePath string
reconcilerClass string
reconcilerClasses []string
hasReconcilerClass bool
hasStatus bool
}
@ -69,7 +69,7 @@ func (g *reconcilerControllerGenerator) GenerateType(c *generator.Context, t *ty
m := map[string]interface{}{
"type": t,
"group": g.groupName,
"class": g.reconcilerClass,
"classes": g.reconcilerClasses,
"hasClass": g.hasReconcilerClass,
"hasStatus": g.hasStatus,
"controllerImpl": c.Universe.Type(types.Name{
@ -195,12 +195,24 @@ var reconcilerControllerNewImpl = `
const (
defaultControllerAgentName = "{{.type|lowercaseSingular}}-controller"
defaultFinalizerName = "{{.type|allLowercasePlural}}.{{.group}}"
{{if .hasClass}}
{{if .hasClass }}
// ClassAnnotationKey points to the annotation for the class of this resource.
ClassAnnotationKey = "{{ .class }}"
{{if gt (.classes | len) 1 }}// Deprecated: Use ClassAnnotationKeys given multiple keys exist
{{end -}}
ClassAnnotationKey = "{{ index .classes 0 }}"
{{end}}
)
{{if gt (.classes | len) 1 }}
var (
// ClassAnnotationKeys points to the annotation for the class of this resource.
ClassAnnotationKeys = []string{
{{range $class := .classes}}"{{$class}}",
{{end}}
}
)
{{end}}
// NewImpl returns a {{.controllerImpl|raw}} that handles queuing and feeding work from
// the queue through an implementation of {{.controllerReconciler|raw}}, delegating to
// the provided Interface and optional Finalizer methods. OptionsFn is used to return

22
codegen/cmd/injection-gen/generators/reconciler_controller_stub.go
View File

@ -35,7 +35,7 @@ type reconcilerControllerStubGenerator struct {
reconcilerPkg string
informerPackagePath string
reconcilerClass string
reconcilerClasses []string
hasReconcilerClass bool
}
@ -64,7 +64,7 @@ func (g *reconcilerControllerStubGenerator) GenerateType(c *generator.Context, t
m := map[string]interface{}{
"type": t,
"class": g.reconcilerClass,
"classes": g.reconcilerClasses,
"hasClass": g.hasReconcilerClass,
"informerGet": c.Universe.Function(types.Name{
Package: g.informerPackagePath,
@ -91,10 +91,18 @@ func (g *reconcilerControllerStubGenerator) GenerateType(c *generator.Context, t
Package: g.reconcilerPkg,
Name: "ClassAnnotationKey",
}),
"classAnnotationKeys": c.Universe.Variable(types.Name{
Package: g.reconcilerPkg,
Name: "ClassAnnotationKeys",
}),
"annotationFilterFunc": c.Universe.Function(types.Name{
Package: "knative.dev/pkg/reconciler",
Name: "AnnotationFilterFunc",
}),
"orFilterFunc": c.Universe.Function(types.Name{
Package: "knative.dev/pkg/reconciler",
Name: "Or",
}),
"filterHandler": c.Universe.Type(types.Name{
Package: "k8s.io/client-go/tools/cache",
Name: "FilteringResourceEventHandler",
@ -120,7 +128,17 @@ func NewController(
{{if .hasClass}}
classValue := "default" // TODO: update this to the appropriate value.
{{if len .classes | eq 1 }}
classFilter := {{.annotationFilterFunc|raw}}({{.classAnnotationKey|raw}}, classValue, false /*allowUnset*/)
{{else if gt (len .classes) 1 }}
filterFuncs := make([]func(interface{}) bool, 0, len({{.classAnnotationKeys|raw}}))
for _, key := range {{.classAnnotationKeys|raw}} {
filterFuncs = append(filterFuncs, {{.annotationFilterFunc|raw}}(key, classValue, false /*allowUnset*/))
}
classFilter := {{.orFilterFunc|raw}}(filterFuncs...)
{{end}}
{{end}}
// TODO: setup additional informers here.

41
codegen/cmd/injection-gen/generators/reconciler_reconciler.go
View File

@ -37,7 +37,7 @@ type reconcilerReconcilerGenerator struct {
listerName string
listerPkg string
reconcilerClass string
reconcilerClasses []string
hasReconcilerClass bool
nonNamespaced bool
isKRShaped bool
@ -74,7 +74,7 @@ func (g *reconcilerReconcilerGenerator) GenerateType(c *generator.Context, t *ty
"type": t,
"group": namer.IC(g.groupGoName),
"version": namer.IC(g.groupVersion.Version.String()),
"class": g.reconcilerClass,
"classes": g.reconcilerClasses,
"hasClass": g.hasReconcilerClass,
"isKRShaped": g.isKRShaped,
"hasStatus": g.hasStatus,
@ -219,6 +219,9 @@ func (g *reconcilerReconcilerGenerator) GenerateType(c *generator.Context, t *ty
sw.Do(reconcilerInterfaceFactory, m)
sw.Do(reconcilerNewReconciler, m)
sw.Do(reconcilerImplFactory, m)
if len(g.reconcilerClasses) > 1 {
sw.Do(reconcilerLookupClass, m)
}
if g.hasStatus {
sw.Do(reconcilerStatusFactory, m)
}
@ -227,6 +230,17 @@ func (g *reconcilerReconcilerGenerator) GenerateType(c *generator.Context, t *ty
return sw.Error()
}
var reconcilerLookupClass = `
func lookupClass(annotations map[string]string) (string, bool) {
for _, key := range ClassAnnotationKeys {
if val, ok := annotations[key]; ok {
return val, true
}
}
return "", false
}
`
var reconcilerInterfaceFactory = `
// Interface defines the strongly typed interfaces to be implemented by a
// controller reconciling {{.type|raw}}.
@ -293,8 +307,15 @@ type reconcilerImpl struct {
skipStatusUpdates bool
{{end}}
{{if .hasClass}}
// classValue is the resource annotation[{{ .class }}] instance value this reconciler instance filters on.
{{if len .classes | eq 1 }}
// classValue is the resource annotation[{{ index .classes 0 }}] instance value this reconciler instance filters on.
classValue string
{{else if gt (len .classes) 1 }}
// classValue is the resource annotation instance value this reconciler instance filters on.
// The annotations key are:
{{- range $class := .classes}}
// {{$class}}
{{- end}}
classValue string
{{end}}
}
@ -416,14 +437,22 @@ func (r *reconcilerImpl) Reconcile(ctx {{.contextContext|raw}}, key string) erro
} else if err != nil {
return err
}
{{if .hasClass}}
{{if len .classes | eq 1 }}
if classValue, found := original.GetAnnotations()[ClassAnnotationKey]; !found || classValue != r.classValue {
logger.Debugw("Skip reconciling resource, class annotation value does not match reconciler instance value.",
zap.String("classKey", ClassAnnotationKey),
zap.String("issue", classValue+"!="+r.classValue))
return nil
}
{{end}}
{{else if gt (len .classes) 1 }}
if classValue, found := lookupClass(original.GetAnnotations()); !found || classValue != r.classValue {
logger.Debugw("Skip reconciling resource, class annotation value does not match reconciler instance value.",
zap.Strings("classKeys", ClassAnnotationKeys),
zap.String("issue", classValue+"!="+r.classValue))
return nil
}
{{end}}
// Don't modify the informers copy.
resource := original.DeepCopy()

12
reconciler/filter.go
View File

@ -77,6 +77,18 @@ func Not(f func(interface{}) bool) func(interface{}) bool {
}
}
// Or creates a FilterFunc which performs an OR of the passed in FilterFuncs
func Or(funcs ...func(interface{}) bool) func(interface{}) bool {
return func(obj interface{}) bool {
for _, f := range funcs {
if f(obj) {
return true
}
}
return false
}
}
// ChainFilterFuncs creates a FilterFunc which performs an AND of the passed FilterFuncs.
func ChainFilterFuncs(funcs ...func(interface{}) bool) func(interface{}) bool {
return func(obj interface{}) bool {