Skip to content

Commit

Permalink
Added ENV flag to disable Application CR lister (#18259)
Browse files Browse the repository at this point in the history
* Added ENV flag to disable Application CR lister

* added tests

* image bump
  • Loading branch information
mfaizanse authored Oct 5, 2023
1 parent 21b126a commit b0bd82f
Show file tree
Hide file tree
Showing 11 changed files with 321 additions and 219 deletions.
17 changes: 12 additions & 5 deletions pkg/cloudevents/builder/generic.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,10 @@ func NewGenericBuilder(typePrefix string, cleaner cleaner.Cleaner, applicationLi
}
}

func (gb *GenericBuilder) isApplicationListerEnabled() bool {
return gb.applicationLister != nil
}

func (gb *GenericBuilder) Build(event cev2event.Event) (*cev2event.Event, error) {
// format logger
namedLogger := gb.namedLogger(event.Source(), event.Type())
Expand Down Expand Up @@ -77,12 +81,15 @@ func (gb *GenericBuilder) getFinalSubject(source, eventType string) string {
// GetAppNameOrSource returns the application name if exists, otherwise returns source name.
func (gb *GenericBuilder) GetAppNameOrSource(source string, namedLogger *zap.SugaredLogger) string {
var appName = source
if appObj, err := gb.applicationLister.Get(source); err == nil && appObj != nil {
appName = application.GetTypeOrName(appObj)
namedLogger.With("application", source).Debug("Using application name: %s as source.", appName)
} else {
namedLogger.With("application", source).Debug("Cannot find application.")
if gb.isApplicationListerEnabled() {
if appObj, err := gb.applicationLister.Get(source); err == nil && appObj != nil {
appName = application.GetTypeOrName(appObj)
namedLogger.With("application", source).Debug("Using application name: %s as source.", appName)
} else {
namedLogger.With("application", source).Debug("Cannot find application.")
}
}

return appName
}

Expand Down
67 changes: 42 additions & 25 deletions pkg/cloudevents/builder/generic_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -149,38 +149,51 @@ func Test_GetAppNameOrSource(t *testing.T) {
}

testCases := []struct {
name string
givenApplicationName string
givenApplicationLabels map[string]string
givenSource string
wantSource string
name string
givenApplicationName string
givenApplicationLabels map[string]string
givenSource string
givenApplicationListerEnabled bool
wantSource string
}{
{
name: "should return application name instead of source name",
givenSource: "appName1",
givenApplicationName: "appName1",
wantSource: "appName1",
name: "should return application name instead of source name",
givenSource: "appName1",
givenApplicationName: "appName1",
givenApplicationListerEnabled: true,
wantSource: "appName1",
},
{
name: "should return application label instead of source name or app name",
givenSource: "appName1",
givenApplicationName: "appName1",
givenApplicationLabels: map[string]string{application.TypeLabel: "testapptype"},
wantSource: "testapptype",
name: "should return application label instead of source name or app name",
givenSource: "appName1",
givenApplicationName: "appName1",
givenApplicationListerEnabled: true,
givenApplicationLabels: map[string]string{application.TypeLabel: "testapptype"},
wantSource: "testapptype",
},
{
name: "should return non-clean application label",
givenSource: "appName1",
givenApplicationName: "appName1",
givenApplicationLabels: map[string]string{application.TypeLabel: "t..e--s__t!!a@@p##p%%t^^y&&p**e"},
wantSource: "t..e--s__t!!a@@p##p%%t^^y&&p**e",
name: "should return non-clean application label",
givenSource: "appName1",
givenApplicationName: "appName1",
givenApplicationListerEnabled: true,
givenApplicationLabels: map[string]string{application.TypeLabel: "t..e--s__t!!a@@p##p%%t^^y&&p**e"},
wantSource: "t..e--s__t!!a@@p##p%%t^^y&&p**e",
},
{
name: "should return source name as application does not exists",
givenSource: "noapp1",
givenApplicationName: "appName1",
givenApplicationLabels: map[string]string{application.TypeLabel: "testapptype"},
wantSource: "noapp1",
name: "should return source name as application does not exists",
givenSource: "noapp1",
givenApplicationName: "appName1",
givenApplicationListerEnabled: true,
givenApplicationLabels: map[string]string{application.TypeLabel: "testapptype"},
wantSource: "noapp1",
},
{
name: "should return source name when application lister is disabled",
givenSource: "appName1",
givenApplicationName: "appName1",
givenApplicationListerEnabled: false,
givenApplicationLabels: map[string]string{application.TypeLabel: "testapptype"},
wantSource: "appName1",
},
}
for _, testCase := range testCases {
Expand All @@ -189,7 +202,11 @@ func Test_GetAppNameOrSource(t *testing.T) {
t.Parallel()

app := applicationtest.NewApplication(tc.givenApplicationName, tc.givenApplicationLabels)
appLister := fake.NewApplicationListerOrDie(context.Background(), app)

var appLister *application.Lister
if tc.givenApplicationListerEnabled {
appLister = fake.NewApplicationListerOrDie(context.Background(), app)
}

genericBuilder := &GenericBuilder{
applicationLister: appLister,
Expand Down
2 changes: 1 addition & 1 deletion pkg/cloudevents/builder/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ type CloudEventBuilder interface {

type GenericBuilder struct {
typePrefix string
applicationLister *application.Lister
applicationLister *application.Lister // applicationLister will be nil when disabled.
cleaner cleaner.Cleaner
logger *logger.Logger
}
Expand Down
19 changes: 12 additions & 7 deletions pkg/cloudevents/eventtype/clean.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ type Cleaner interface {

type cleaner struct {
eventTypePrefix string
applicationLister *application.Lister
applicationLister *application.Lister // applicationLister will be nil when disabled.
logger *logger.Logger
}

Expand All @@ -39,6 +39,10 @@ func NewCleaner(eventTypePrefix string, applicationLister *application.Lister, l
return &cleaner{eventTypePrefix: eventTypePrefix, applicationLister: applicationLister, logger: logger}
}

func (c *cleaner) isApplicationListerEnabled() bool {
return c.applicationLister != nil
}

// Clean cleans the event-type from none-alphanumeric characters and returns it
// or returns an error if the event-type parsing failed.
func (c *cleaner) Clean(eventType string) (string, error) {
Expand All @@ -51,12 +55,13 @@ func (c *cleaner) Clean(eventType string) (string, error) {
}

// clean the application name
var eventTypeClean string
if appObj, err := c.applicationLister.Get(appName); err != nil {
namedLogger.With("application", appName).Debug("Cannot find application")
eventTypeClean = build(c.eventTypePrefix, application.GetCleanName(appName), event, version)
} else {
eventTypeClean = build(c.eventTypePrefix, application.GetCleanTypeOrName(appObj), event, version)
eventTypeClean := build(c.eventTypePrefix, application.GetCleanName(appName), event, version)
if c.isApplicationListerEnabled() {
if appObj, err := c.applicationLister.Get(appName); err == nil {
eventTypeClean = build(c.eventTypePrefix, application.GetCleanTypeOrName(appObj), event, version)
} else {
namedLogger.With("application", appName).Debug("Cannot find application")
}
}

// clean the event-type segments
Expand Down
Loading

0 comments on commit b0bd82f

Please sign in to comment.