From 63b035c554db4176760b4bcd1df7f11508751ac4 Mon Sep 17 00:00:00 2001 From: Ashwin Venkatesh Date: Tue, 29 Sep 2020 16:40:28 -0400 Subject: [PATCH] Restructure MatchesConsul test to test against mismatched type - This ensures we dont have to necessarily write failing tests every time we add a field --- api/v1alpha1/proxydefaults_types_test.go | 33 +++- api/v1alpha1/servicedefaults_types_test.go | 218 ++------------------- api/v1alpha1/serviceresolver_types_test.go | 36 +++- api/v1alpha1/servicerouter_types_test.go | 55 ++---- api/v1alpha1/servicesplitter_types_test.go | 55 ++---- main.go | 3 +- 6 files changed, 108 insertions(+), 292 deletions(-) diff --git a/api/v1alpha1/proxydefaults_types_test.go b/api/v1alpha1/proxydefaults_types_test.go index 300297a93b..b14f211f9d 100644 --- a/api/v1alpha1/proxydefaults_types_test.go +++ b/api/v1alpha1/proxydefaults_types_test.go @@ -15,10 +15,11 @@ import ( // Test MatchesConsul for cases that should return true. func TestProxyDefaults_MatchesConsul(t *testing.T) { cases := map[string]struct { - Ours ProxyDefaults - Theirs *capi.ProxyConfigEntry + Ours ProxyDefaults + Theirs capi.ConfigEntry + Matches bool }{ - "empty fields": { + "empty fields matches": { Ours: ProxyDefaults{ ObjectMeta: metav1.ObjectMeta{ Name: common.Global, @@ -26,11 +27,15 @@ func TestProxyDefaults_MatchesConsul(t *testing.T) { Spec: ProxyDefaultsSpec{}, }, Theirs: &capi.ProxyConfigEntry{ - Name: common.Global, - Kind: capi.ProxyDefaults, + Name: common.Global, + Kind: capi.ProxyDefaults, + Namespace: "default", + CreateIndex: 1, + ModifyIndex: 2, }, + Matches: true, }, - "all fields set": { + "all fields set matches": { Ours: ProxyDefaults{ ObjectMeta: metav1.ObjectMeta{ Name: common.Global, @@ -86,11 +91,25 @@ func TestProxyDefaults_MatchesConsul(t *testing.T) { }, }, }, + Matches: true, + }, + "mismatched types does not match": { + Ours: ProxyDefaults{ + ObjectMeta: metav1.ObjectMeta{ + Name: common.Global, + }, + Spec: ProxyDefaultsSpec{}, + }, + Theirs: &capi.ServiceConfigEntry{ + Name: common.Global, + Kind: capi.ProxyDefaults, + }, + Matches: false, }, } for name, c := range cases { t.Run(name, func(t *testing.T) { - require.True(t, c.Ours.MatchesConsul(c.Theirs)) + require.Equal(t, c.Ours.MatchesConsul(c.Theirs), c.Matches) }) } } diff --git a/api/v1alpha1/servicedefaults_types_test.go b/api/v1alpha1/servicedefaults_types_test.go index 373416e8c3..a70bae7fa6 100644 --- a/api/v1alpha1/servicedefaults_types_test.go +++ b/api/v1alpha1/servicedefaults_types_test.go @@ -9,7 +9,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) -func TestToConsul(t *testing.T) { +func TestServiceDefaults_ToConsul(t *testing.T) { cases := map[string]struct { input *ServiceDefaults expected *capi.ServiceConfigEntry @@ -286,12 +286,13 @@ func TestToConsul(t *testing.T) { } } -func TestMatchesConsul_Matches(t *testing.T) { +func TestServiceDefaults_MatchesConsul(t *testing.T) { cases := map[string]struct { internal *ServiceDefaults - consul *capi.ServiceConfigEntry + consul capi.ConfigEntry + matches bool }{ - "empty fields": { + "empty fields matches": { &ServiceDefaults{ ObjectMeta: metav1.ObjectMeta{ Name: "my-test-service", @@ -305,8 +306,9 @@ func TestMatchesConsul_Matches(t *testing.T) { CreateIndex: 1, ModifyIndex: 2, }, + true, }, - "all fields populated": { + "all fields populated matches": { &ServiceDefaults{ ObjectMeta: metav1.ObjectMeta{ Name: "my-test-service", @@ -360,217 +362,29 @@ func TestMatchesConsul_Matches(t *testing.T) { }, ExternalSNI: "sni-value", }, + true, }, - } - - for name, testCase := range cases { - t.Run(name, func(t *testing.T) { - require.True(t, testCase.internal.MatchesConsul(testCase.consul)) - }) - } -} - -func TestMatchesConsul_NotMatch(t *testing.T) { - cases := map[string]struct { - internal *ServiceDefaults - consul *capi.ServiceConfigEntry - }{ - "name:mismatched": { + "mismatched types does not match": { &ServiceDefaults{ ObjectMeta: metav1.ObjectMeta{ Name: "my-test-service", }, Spec: ServiceDefaultsSpec{}, }, - &capi.ServiceConfigEntry{ - Kind: capi.ServiceDefaults, - Name: "differently-named-service", - }, - }, - "protocol:mismatched": { - &ServiceDefaults{ - Spec: ServiceDefaultsSpec{ - Protocol: "http", - }, - }, - &capi.ServiceConfigEntry{ - Kind: capi.ServiceDefaults, - Protocol: "https", - }, - }, - "gatewayConfig:mismatched": { - &ServiceDefaults{ - Spec: ServiceDefaultsSpec{ - MeshGateway: MeshGatewayConfig{ - Mode: "remote", - }, - }, - }, - &capi.ServiceConfigEntry{ - Kind: capi.ServiceDefaults, - MeshGateway: capi.MeshGatewayConfig{ - Mode: capi.MeshGatewayModeLocal, - }, - }, - }, - "externalSNI:mismatched": { - &ServiceDefaults{ - Spec: ServiceDefaultsSpec{ - ExternalSNI: "test-external-sni", - }, - }, - &capi.ServiceConfigEntry{ + &capi.ProxyConfigEntry{ Kind: capi.ServiceDefaults, - ExternalSNI: "different-external-sni", - }, - }, - "expose.checks:mismatched": { - &ServiceDefaults{ - Spec: ServiceDefaultsSpec{ - Expose: ExposeConfig{ - Checks: true, - }, - }, - }, - &capi.ServiceConfigEntry{ - Kind: capi.ServiceDefaults, - Expose: capi.ExposeConfig{ - Checks: false, - }, - }, - }, - "expose.paths.listenerPort:mismatched": { - &ServiceDefaults{ - Spec: ServiceDefaultsSpec{ - Expose: ExposeConfig{ - Paths: []ExposePath{ - { - ListenerPort: 80, - }, - }, - }, - }, - }, - &capi.ServiceConfigEntry{ - Kind: capi.ServiceDefaults, - Expose: capi.ExposeConfig{ - Paths: []capi.ExposePath{ - { - ListenerPort: 81, - }, - }, - }, - }, - }, - "expose.paths.path:mismatched": { - &ServiceDefaults{ - Spec: ServiceDefaultsSpec{ - Expose: ExposeConfig{ - Paths: []ExposePath{ - { - Path: "/test/path", - }, - }, - }, - }, - }, - &capi.ServiceConfigEntry{ - Kind: capi.ServiceDefaults, - Expose: capi.ExposeConfig{ - Paths: []capi.ExposePath{ - { - Path: "/differnt/path", - }, - }, - }, - }, - }, - "expose.paths.localPathPort:mismatched": { - &ServiceDefaults{ - Spec: ServiceDefaultsSpec{ - Expose: ExposeConfig{ - Paths: []ExposePath{ - { - LocalPathPort: 42, - }, - }, - }, - }, - }, - &capi.ServiceConfigEntry{ - Kind: capi.ServiceDefaults, - Expose: capi.ExposeConfig{ - Paths: []capi.ExposePath{ - { - LocalPathPort: 21, - }, - }, - }, - }, - }, - "expose.paths.protocol:mismatched": { - &ServiceDefaults{ - Spec: ServiceDefaultsSpec{ - Expose: ExposeConfig{ - Paths: []ExposePath{ - { - Protocol: "tcp", - }, - }, - }, - }, - }, - &capi.ServiceConfigEntry{ - Kind: capi.ServiceDefaults, - Expose: capi.ExposeConfig{ - Paths: []capi.ExposePath{ - { - Protocol: "https", - }, - }, - }, - }, - }, - "expose.paths:mismatched when path lengths are different": { - &ServiceDefaults{ - Spec: ServiceDefaultsSpec{ - Expose: ExposeConfig{ - Paths: []ExposePath{ - { - ListenerPort: 8080, - Path: "/second/test/path", - LocalPathPort: 11, - Protocol: "https", - }, - { - ListenerPort: 80, - Path: "/test/path", - LocalPathPort: 42, - Protocol: "tcp", - }, - }, - }, - }, - }, - &capi.ServiceConfigEntry{ - Kind: capi.ServiceDefaults, - Expose: capi.ExposeConfig{ - Paths: []capi.ExposePath{ - { - ListenerPort: 8080, - Path: "/second/test/path", - LocalPathPort: 11, - Protocol: "https", - }, - }, - }, + Name: "my-test-service", + Namespace: "namespace", + CreateIndex: 1, + ModifyIndex: 2, }, + false, }, } for name, testCase := range cases { t.Run(name, func(t *testing.T) { - require.False(t, testCase.internal.MatchesConsul(testCase.consul)) + require.Equal(t, testCase.internal.MatchesConsul(testCase.consul), testCase.matches) }) } } diff --git a/api/v1alpha1/serviceresolver_types_test.go b/api/v1alpha1/serviceresolver_types_test.go index 8ed05d7b39..a796ecf004 100644 --- a/api/v1alpha1/serviceresolver_types_test.go +++ b/api/v1alpha1/serviceresolver_types_test.go @@ -12,10 +12,11 @@ import ( func TestServiceResolver_MatchesConsul(t *testing.T) { cases := map[string]struct { - Ours ServiceResolver - Theirs *capi.ServiceResolverConfigEntry + Ours ServiceResolver + Theirs capi.ConfigEntry + Matches bool }{ - "empty fields": { + "empty fields matches": { Ours: ServiceResolver{ ObjectMeta: metav1.ObjectMeta{ Name: "name", @@ -23,11 +24,15 @@ func TestServiceResolver_MatchesConsul(t *testing.T) { Spec: ServiceResolverSpec{}, }, Theirs: &capi.ServiceResolverConfigEntry{ - Name: "name", - Kind: capi.ServiceResolver, + Name: "name", + Kind: capi.ServiceResolver, + Namespace: "foobar", + CreateIndex: 1, + ModifyIndex: 2, }, + Matches: true, }, - "all fields set": { + "all fields set matches": { Ours: ServiceResolver{ ObjectMeta: metav1.ObjectMeta{ Name: "name", @@ -149,11 +154,28 @@ func TestServiceResolver_MatchesConsul(t *testing.T) { }, }, }, + Matches: true, + }, + "different types does not match": { + Ours: ServiceResolver{ + ObjectMeta: metav1.ObjectMeta{ + Name: "name", + }, + Spec: ServiceResolverSpec{}, + }, + Theirs: &capi.ProxyConfigEntry{ + Name: "name", + Kind: capi.ServiceResolver, + Namespace: "foobar", + CreateIndex: 1, + ModifyIndex: 2, + }, + Matches: false, }, } for name, c := range cases { t.Run(name, func(t *testing.T) { - require.True(t, c.Ours.MatchesConsul(c.Theirs)) + require.Equal(t, c.Ours.MatchesConsul(c.Theirs), c.Matches) }) } } diff --git a/api/v1alpha1/servicerouter_types_test.go b/api/v1alpha1/servicerouter_types_test.go index 4361df5c3a..c390ac4fe4 100644 --- a/api/v1alpha1/servicerouter_types_test.go +++ b/api/v1alpha1/servicerouter_types_test.go @@ -10,13 +10,14 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) -// Test MatchesConsul for cases that should return true. -func TestServiceRouter_MatchesConsulTrue(t *testing.T) { +// Test MatchesConsul. +func TestServiceRouter_MatchesConsul(t *testing.T) { cases := map[string]struct { - Ours ServiceRouter - Theirs *capi.ServiceRouterConfigEntry + Ours ServiceRouter + Theirs capi.ConfigEntry + Matches bool }{ - "empty fields": { + "empty fields matches": { Ours: ServiceRouter{ ObjectMeta: metav1.ObjectMeta{ Name: "name", @@ -30,8 +31,9 @@ func TestServiceRouter_MatchesConsulTrue(t *testing.T) { CreateIndex: 1, ModifyIndex: 2, }, + Matches: true, }, - "all fields set": { + "all fields set matches": { Ours: ServiceRouter{ ObjectMeta: metav1.ObjectMeta{ Name: "name", @@ -125,49 +127,28 @@ func TestServiceRouter_MatchesConsulTrue(t *testing.T) { }, }, }, + Matches: true, }, - } - for name, c := range cases { - t.Run(name, func(t *testing.T) { - require.True(t, c.Ours.MatchesConsul(c.Theirs)) - }) - } -} - -// Test MatchesConsul for cases that should return false. -func TestServiceRouter_MatchesConsulFalse(t *testing.T) { - cases := map[string]struct { - Ours ServiceRouter - Theirs capi.ConfigEntry - }{ - "different type": { + "mismatched type does not match": { Ours: ServiceRouter{ ObjectMeta: metav1.ObjectMeta{ Name: "name", }, Spec: ServiceRouterSpec{}, }, - Theirs: &capi.ServiceConfigEntry{ - Name: "name", - Kind: capi.ServiceRouter, - }, - }, - "different name": { - Ours: ServiceRouter{ - ObjectMeta: metav1.ObjectMeta{ - Name: "name", - }, - Spec: ServiceRouterSpec{}, - }, - Theirs: &capi.ServiceRouterConfigEntry{ - Name: "other_name", - Kind: capi.ServiceRouter, + Theirs: &capi.ProxyConfigEntry{ + Kind: capi.ServiceRouter, + Name: "name", + Namespace: "namespace", + CreateIndex: 1, + ModifyIndex: 2, }, + Matches: false, }, } for name, c := range cases { t.Run(name, func(t *testing.T) { - require.False(t, c.Ours.MatchesConsul(c.Theirs)) + require.Equal(t, c.Ours.MatchesConsul(c.Theirs), c.Matches) }) } } diff --git a/api/v1alpha1/servicesplitter_types_test.go b/api/v1alpha1/servicesplitter_types_test.go index 2e33c1fc99..7416b89ba3 100644 --- a/api/v1alpha1/servicesplitter_types_test.go +++ b/api/v1alpha1/servicesplitter_types_test.go @@ -9,13 +9,14 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) -// Test MatchesConsul for cases that should return true. -func TestServiceSplitter_MatchesConsulTrue(t *testing.T) { +// Test MatchesConsul. +func TestServiceSplitter_MatchesConsul(t *testing.T) { cases := map[string]struct { - Ours ServiceSplitter - Theirs *capi.ServiceSplitterConfigEntry + Ours ServiceSplitter + Theirs capi.ConfigEntry + Matches bool }{ - "empty fields": { + "empty fields matches": { Ours: ServiceSplitter{ ObjectMeta: metav1.ObjectMeta{ Name: "name", @@ -29,8 +30,9 @@ func TestServiceSplitter_MatchesConsulTrue(t *testing.T) { CreateIndex: 1, ModifyIndex: 2, }, + Matches: true, }, - "all fields set": { + "all fields set matches": { Ours: ServiceSplitter{ ObjectMeta: metav1.ObjectMeta{ Name: "name", @@ -58,49 +60,28 @@ func TestServiceSplitter_MatchesConsulTrue(t *testing.T) { }, }, }, + Matches: true, }, - } - for name, c := range cases { - t.Run(name, func(t *testing.T) { - require.True(t, c.Ours.MatchesConsul(c.Theirs)) - }) - } -} - -// Test MatchesConsul for cases that should return false. -func TestServiceSplitter_MatchesConsulFalse(t *testing.T) { - cases := map[string]struct { - Ours ServiceSplitter - Theirs capi.ConfigEntry - }{ - "different type": { + "different types does not match": { Ours: ServiceSplitter{ ObjectMeta: metav1.ObjectMeta{ Name: "name", }, Spec: ServiceSplitterSpec{}, }, - Theirs: &capi.ServiceConfigEntry{ - Name: "name", - Kind: capi.ServiceSplitter, - }, - }, - "different name": { - Ours: ServiceSplitter{ - ObjectMeta: metav1.ObjectMeta{ - Name: "name", - }, - Spec: ServiceSplitterSpec{}, - }, - Theirs: &capi.ServiceSplitterConfigEntry{ - Name: "other_name", - Kind: capi.ServiceSplitter, + Theirs: &capi.ProxyConfigEntry{ + Kind: capi.ServiceSplitter, + Name: "name", + Namespace: "namespace", + CreateIndex: 1, + ModifyIndex: 2, }, + Matches: false, }, } for name, c := range cases { t.Run(name, func(t *testing.T) { - require.False(t, c.Ours.MatchesConsul(c.Theirs)) + require.Equal(t, c.Ours.MatchesConsul(c.Theirs), c.Matches) }) } } diff --git a/main.go b/main.go index d820362398..8c3eb4e0e9 100644 --- a/main.go +++ b/main.go @@ -4,9 +4,8 @@ import ( "log" "os" - "github.com/mitchellh/cli" - "github.com/hashicorp/consul-k8s/version" + "github.com/mitchellh/cli" ) func main() {