Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Default broker annotations from the broker defaults configmap #2724

Merged
merged 2 commits into from
Mar 8, 2020
Merged

Default broker annotations from the broker defaults configmap #2724

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
65884ef
Default broker annotations from the broker defaults configmap
vaikas Mar 7, 2020
9e2a987
appease the robot overlord
vaikas Mar 7, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions Gopkg.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

37 changes: 31 additions & 6 deletions pkg/apis/config/defaults.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,11 +70,19 @@ func NewDefaultsConfigFromConfigMap(config *corev1.ConfigMap) (*Defaults, error)
type Defaults struct {
// NamespaceDefaultsConfig are the default Broker Configs for each namespace.
// Namespace is the key, the value is the KReference to the config.
NamespaceDefaultsConfig map[string]*duckv1.KReference `json:"namespaceDefaults,omitempty"`
NamespaceDefaultsConfig map[string]*ClassAndKRef `json:"namespaceDefaults,omitempty"`

// ClusterDefaultBrokerConfig is the default broker config for all the namespaces that
// are not in NamespaceDefaultBrokerConfigs.
ClusterDefault *duckv1.KReference `json:"clusterDefault,omitempty"`
ClusterDefault *ClassAndKRef `json:"clusterDefault,omitempty"`
}

// ClassAndKRef contains configuration for a given namespace for broker. Allows
// configuring both the Class of the Broker as well as the reference to the
// config it should use
type ClassAndKRef struct {
BrokerClass string `json:"brokerClass,omitempty"`
*duckv1.KReference `json:",inline"`
}

// GetBrokerConfig returns a namespace specific Broker Configuration, and if
Expand All @@ -85,11 +93,28 @@ func (d *Defaults) GetBrokerConfig(ns string) (*duckv1.KReference, error) {
return nil, errors.New("Defaults are nil")
}
value, present := d.NamespaceDefaultsConfig[ns]
if present {
return value, nil
if present && value.KReference != nil {
return value.KReference, nil
}
if d.ClusterDefault != nil {
return d.ClusterDefault, nil
if d.ClusterDefault != nil && d.ClusterDefault.KReference != nil {
return d.ClusterDefault.KReference, nil
}
return nil, errors.New("Defaults for Broker Configurations have not been set up.")
}

// GetBrokerClass returns a namespace specific Broker Class, and if
// that doesn't exist, return a Cluster Default and if that doesn't exist
// return an error.
func (d *Defaults) GetBrokerClass(ns string) (string, error) {
if d == nil {
return "", errors.New("Defaults are nil")
}
value, present := d.NamespaceDefaultsConfig[ns]
if present && value.BrokerClass != "" {
return value.BrokerClass, nil
}
if d.ClusterDefault != nil && d.ClusterDefault.BrokerClass != "" {
return d.ClusterDefault.BrokerClass, nil
}
return "", errors.New("Defaults for Broker Configurations have not been set up.")
}
240 changes: 212 additions & 28 deletions pkg/apis/config/defaults_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ import (
"knative.dev/pkg/kmp"
"knative.dev/pkg/system"

"knative.dev/eventing/pkg/apis/eventing"

. "knative.dev/pkg/configmap/testing"
_ "knative.dev/pkg/system/testing"
)
Expand Down Expand Up @@ -56,6 +58,64 @@ func TestGetBrokerConfig(t *testing.T) {
if c.Name != "someothername" {
t.Errorf("GetBrokerConfig Failed, wanted someothername, got: %s", c.Name)
}

// Nil and empty tests
var nilDefaults *Defaults
_, err = nilDefaults.GetBrokerConfig("rando")
if err == nil {
t.Errorf("GetBrokerConfig did not fail with nil")
}
if err.Error() != "Defaults are nil" {
t.Errorf("GetBrokerConfig did not fail with nil msg, got %v", err)
}
emptyDefaults := Defaults{}
_, err = emptyDefaults.GetBrokerConfig("rando")
if err == nil {
t.Errorf("GetBrokerConfig did not fail with empty")
}
if err.Error() != "Defaults for Broker Configurations have not been set up." {
t.Errorf("GetBrokerConfig did not fail with non-setup msg, got %v", err)
}
}

func TestGetBrokerClass(t *testing.T) {
_, example := ConfigMapsFromTestFile(t, DefaultsConfigName)
defaults, err := NewDefaultsConfigFromConfigMap(example)
if err != nil {
t.Errorf("NewDefaultsConfigFromConfigMap(example) = %v", err)
}
c, err := defaults.GetBrokerClass("rando")
if err != nil {
t.Errorf("GetBrokerClass Failed = %v", err)
}
if c != eventing.ChannelBrokerClassValue {
t.Errorf("GetBrokerClass Failed, wanted somename, got: %s", c)
}
c, err = defaults.GetBrokerClass("some-namespace")
if err != nil {
t.Errorf("GetBrokerClass Failed = %v", err)
}
if c != "someotherbrokerclass" {
t.Errorf("GetBrokerClass Failed, wanted someothername, got: %s", c)
}

// Nil and empty tests
var nilDefaults *Defaults
_, err = nilDefaults.GetBrokerClass("rando")
if err == nil {
t.Errorf("GetBrokerClass did not fail with nil")
}
if err.Error() != "Defaults are nil" {
t.Errorf("GetBrokerClass did not fail with nil msg, got %v", err)
}
emptyDefaults := Defaults{}
_, err = emptyDefaults.GetBrokerClass("rando")
if err == nil {
t.Errorf("GetBrokerClass did not fail with empty")
}
if err.Error() != "Defaults for Broker Configurations have not been set up." {
t.Errorf("GetBrokerClass did not fail with non-setup msg, got %v", err)
}
}

func TestDefaultsConfiguration(t *testing.T) {
Expand All @@ -78,25 +138,34 @@ func TestDefaultsConfiguration(t *testing.T) {
name: "all specified values",
wantErr: false,
wantDefaults: &Defaults{
NamespaceDefaultsConfig: map[string]*duckv1.KReference{
NamespaceDefaultsConfig: map[string]*ClassAndKRef{
"some-namespace": {
APIVersion: "v1",
Kind: "ConfigMap",
Name: "someothername",
Namespace: "someothernamespace",
BrokerClass: "somenamespaceclass",
KReference: &duckv1.KReference{
APIVersion: "v1",
Kind: "ConfigMap",
Name: "someothername",
Namespace: "someothernamespace",
},
},
"some-namespace-too": {
APIVersion: "v1",
Kind: "ConfigMap",
Name: "someothernametoo",
Namespace: "someothernamespacetoo",
BrokerClass: "somenamespaceclasstoo",
KReference: &duckv1.KReference{
APIVersion: "v1",
Kind: "ConfigMap",
Name: "someothernametoo",
Namespace: "someothernamespacetoo",
},
},
},
ClusterDefault: &duckv1.KReference{
Kind: "ConfigMap",
APIVersion: "v1",
Namespace: "knative-eventing",
Name: "somename",
ClusterDefault: &ClassAndKRef{
BrokerClass: "clusterbrokerclass",
KReference: &duckv1.KReference{
Kind: "ConfigMap",
APIVersion: "v1",
Namespace: "knative-eventing",
Name: "somename",
},
},
},
config: &corev1.ConfigMap{
Expand All @@ -107,17 +176,20 @@ func TestDefaultsConfiguration(t *testing.T) {
Data: map[string]string{
"default-br-config": `
clusterDefault:
brokerClass: clusterbrokerclass
apiVersion: v1
kind: ConfigMap
name: somename
namespace: knative-eventing
namespaceDefaults:
some-namespace:
brokerClass: somenamespaceclass
apiVersion: v1
kind: ConfigMap
name: someothername
namespace: someothernamespace
some-namespace-too:
brokerClass: somenamespaceclasstoo
apiVersion: v1
kind: ConfigMap
name: someothernametoo
Expand All @@ -130,11 +202,14 @@ func TestDefaultsConfiguration(t *testing.T) {
wantErr: false,
wantDefaults: &Defaults{
// NamespaceDefaultsConfig: map[string]*duckv1.KReference{},
ClusterDefault: &duckv1.KReference{
Kind: "ConfigMap",
APIVersion: "v1",
Namespace: "knative-eventing",
Name: "somename",
ClusterDefault: &ClassAndKRef{
BrokerClass: "clusterbrokerclass",
KReference: &duckv1.KReference{
Kind: "ConfigMap",
APIVersion: "v1",
Namespace: "knative-eventing",
Name: "somename",
},
},
},
config: &corev1.ConfigMap{
Expand All @@ -145,6 +220,7 @@ func TestDefaultsConfiguration(t *testing.T) {
Data: map[string]string{
"default-br-config": `
clusterDefault:
brokerClass: clusterbrokerclass
apiVersion: v1
kind: ConfigMap
name: somename
Expand All @@ -156,18 +232,73 @@ func TestDefaultsConfiguration(t *testing.T) {
name: "only namespace defaults",
wantErr: false,
wantDefaults: &Defaults{
NamespaceDefaultsConfig: map[string]*duckv1.KReference{
NamespaceDefaultsConfig: map[string]*ClassAndKRef{
"some-namespace": {
APIVersion: "v1",
Kind: "ConfigMap",
Name: "someothername",
Namespace: "someothernamespace",
BrokerClass: "brokerclassnamespace",
KReference: &duckv1.KReference{
APIVersion: "v1",
Kind: "ConfigMap",
Name: "someothername",
Namespace: "someothernamespace",
},
},
"some-namespace-too": {
APIVersion: "v1",
Kind: "ConfigMap",
Name: "someothernametoo",
Namespace: "someothernamespacetoo",
BrokerClass: "brokerclassnamespacetoo",
KReference: &duckv1.KReference{
APIVersion: "v1",
Kind: "ConfigMap",
Name: "someothernametoo",
Namespace: "someothernamespacetoo",
},
},
},
},
config: &corev1.ConfigMap{
ObjectMeta: metav1.ObjectMeta{
Namespace: system.Namespace(),
Name: DefaultsConfigName,
},
Data: map[string]string{
"default-br-config": `
namespaceDefaults:
some-namespace:
brokerClass: brokerclassnamespace
apiVersion: v1
kind: ConfigMap
name: someothername
namespace: someothernamespace
some-namespace-too:
brokerClass: brokerclassnamespacetoo
apiVersion: v1
kind: ConfigMap
name: someothernametoo
namespace: someothernamespacetoo
`,
},
},
}, {
name: "only namespace config default, cluster brokerclass",
wantErr: false,
wantDefaults: &Defaults{
ClusterDefault: &ClassAndKRef{
BrokerClass: "clusterbrokerclass",
},
NamespaceDefaultsConfig: map[string]*ClassAndKRef{
"some-namespace": {
KReference: &duckv1.KReference{
APIVersion: "v1",
Kind: "ConfigMap",
Name: "someothername",
Namespace: "someothernamespace",
},
},
"some-namespace-too": {
KReference: &duckv1.KReference{
APIVersion: "v1",
Kind: "ConfigMap",
Name: "someothernametoo",
Namespace: "someothernamespacetoo",
},
},
},
},
Expand All @@ -178,8 +309,61 @@ func TestDefaultsConfiguration(t *testing.T) {
},
Data: map[string]string{
"default-br-config": `
clusterDefault:
brokerClass: clusterbrokerclass
namespaceDefaults:
some-namespace:
apiVersion: v1
kind: ConfigMap
name: someothername
namespace: someothernamespace
some-namespace-too:
apiVersion: v1
kind: ConfigMap
name: someothernametoo
namespace: someothernamespacetoo
`,
},
},
}, {
name: "one namespace config default, namespace config default with class, cluster brokerclass",
wantErr: false,
wantDefaults: &Defaults{
ClusterDefault: &ClassAndKRef{
BrokerClass: "clusterbrokerclass",
},
NamespaceDefaultsConfig: map[string]*ClassAndKRef{
"some-namespace": {
BrokerClass: "namespacebrokerclass",
KReference: &duckv1.KReference{
APIVersion: "v1",
Kind: "ConfigMap",
Name: "someothername",
Namespace: "someothernamespace",
},
},
"some-namespace-too": {
KReference: &duckv1.KReference{
APIVersion: "v1",
Kind: "ConfigMap",
Name: "someothernametoo",
Namespace: "someothernamespacetoo",
},
},
},
},
config: &corev1.ConfigMap{
ObjectMeta: metav1.ObjectMeta{
Namespace: system.Namespace(),
Name: DefaultsConfigName,
},
Data: map[string]string{
"default-br-config": `
clusterDefault:
brokerClass: clusterbrokerclass
namespaceDefaults:
some-namespace:
brokerClass: namespacebrokerclass
apiVersion: v1
kind: ConfigMap
name: someothername
Expand Down
Loading