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

Update MatchesConsul to normalize partitions during comparison. #3284

Merged
merged 2 commits into from
Dec 1, 2023
Merged

Update MatchesConsul to normalize partitions during comparison. #3284

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b44a7b7
Update MatchesConsul to normalize partitions during comparison.
thisisnotashwin Nov 29, 2023
0e60896
Update test cases with valid datasets
thisisnotashwin Nov 30, 2023
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
3 changes: 3 additions & 0 deletions .changelog/3284.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:bug-fix
control-plane: normalize the `partition` and `namespace` fields in V1 CRDs when comparing with saved version of the config-entry.
```
12 changes: 10 additions & 2 deletions control-plane/api/v1alpha1/exportedservices_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,15 @@ import (

"github.com/google/go-cmp/cmp"
"github.com/google/go-cmp/cmp/cmpopts"
"github.com/hashicorp/consul-k8s/control-plane/api/common"
"github.com/hashicorp/consul/api"
capi "github.com/hashicorp/consul/api"
corev1 "k8s.io/api/core/v1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/apimachinery/pkg/util/validation/field"

"github.com/hashicorp/consul-k8s/control-plane/api/common"
)

const ExportedServicesKubeKind = "exportedservices"
Expand Down Expand Up @@ -189,8 +190,15 @@ func (in *ExportedServices) MatchesConsul(candidate api.ConfigEntry) bool {
if !ok {
return false
}

specialEquality := cmp.Options{
cmp.FilterPath(func(path cmp.Path) bool {
return path.String() == "Services.Consumers.Partition"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exports are kinda weird -- I'd be surprised if they need to be normalized. Was it actually being modified during the wanfed sync?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i didnt check.. based on the conversation with Dhia, i kinda blanket changed everywhere that one could specify a partition.

}, cmp.Transformer("NormalizePartition", normalizeEmptyToDefault)),
}

// No datacenter is passed to ToConsul as we ignore the Meta field when checking for equality.
return cmp.Equal(in.ToConsul(""), configEntry, cmpopts.IgnoreFields(capi.ExportedServicesConfigEntry{}, "Partition", "Meta", "ModifyIndex", "CreateIndex"), cmpopts.IgnoreUnexported(), cmpopts.EquateEmpty())
return cmp.Equal(in.ToConsul(""), configEntry, cmpopts.IgnoreFields(capi.ExportedServicesConfigEntry{}, "Partition", "Meta", "ModifyIndex", "CreateIndex"), cmpopts.IgnoreUnexported(), cmpopts.EquateEmpty(), specialEquality)

}

Expand Down
4 changes: 3 additions & 1 deletion control-plane/api/v1alpha1/exportedservices_types_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,12 @@ import (
"testing"
"time"

"github.com/hashicorp/consul-k8s/control-plane/api/common"
capi "github.com/hashicorp/consul/api"
"github.com/stretchr/testify/require"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"

"github.com/hashicorp/consul-k8s/control-plane/api/common"
)

// Test MatchesConsul for cases that should return true.
Expand Down Expand Up @@ -103,6 +104,7 @@ func TestExportedServices_MatchesConsul(t *testing.T) {
},
{
SamenessGroup: "sg1",
Partition: "default",
},
},
},
Expand Down
18 changes: 15 additions & 3 deletions control-plane/api/v1alpha1/ingressgateway_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,18 @@ package v1alpha1
import (
"encoding/json"
"fmt"

"github.com/google/go-cmp/cmp"
"github.com/google/go-cmp/cmp/cmpopts"
"github.com/hashicorp/consul-k8s/control-plane/api/common"
"github.com/hashicorp/consul-k8s/control-plane/namespaces"
capi "github.com/hashicorp/consul/api"
corev1 "k8s.io/api/core/v1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/apimachinery/pkg/util/validation/field"

"github.com/hashicorp/consul-k8s/control-plane/api/common"
"github.com/hashicorp/consul-k8s/control-plane/namespaces"
)

const (
Expand Down Expand Up @@ -268,8 +270,18 @@ func (in *IngressGateway) MatchesConsul(candidate capi.ConfigEntry) bool {
if !ok {
return false
}

specialEquality := cmp.Options{
cmp.FilterPath(func(path cmp.Path) bool {
return path.String() == "Listeners.Services.Namespace"
}, cmp.Transformer("NormalizeNamespace", normalizeEmptyToDefault)),
cmp.FilterPath(func(path cmp.Path) bool {
return path.String() == "Listeners.Services.Partition"
}, cmp.Transformer("NormalizePartition", normalizeEmptyToDefault)),
}

// No datacenter is passed to ToConsul as we ignore the Meta field when checking for equality.
return cmp.Equal(in.ToConsul(""), configEntry, cmpopts.IgnoreFields(capi.IngressGatewayConfigEntry{}, "Partition", "Namespace", "Meta", "ModifyIndex", "CreateIndex"), cmpopts.IgnoreUnexported(), cmpopts.EquateEmpty())
return cmp.Equal(in.ToConsul(""), configEntry, cmpopts.IgnoreFields(capi.IngressGatewayConfigEntry{}, "Partition", "Namespace", "Meta", "ModifyIndex", "CreateIndex"), cmpopts.IgnoreUnexported(), cmpopts.EquateEmpty(), specialEquality)
}

func (in *IngressGateway) Validate(consulMeta common.ConsulMeta) error {
Expand Down
4 changes: 2 additions & 2 deletions control-plane/api/v1alpha1/ingressgateway_types_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,13 @@ import (
"time"

"github.com/google/go-cmp/cmp"
"github.com/hashicorp/consul-k8s/control-plane/api/common"
capi "github.com/hashicorp/consul/api"
"github.com/stretchr/testify/require"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/utils/pointer"

"github.com/hashicorp/consul-k8s/control-plane/api/common"
)

func TestIngressGateway_MatchesConsul(t *testing.T) {
Expand Down Expand Up @@ -102,7 +103,6 @@ func TestIngressGateway_MatchesConsul(t *testing.T) {
Name: "name1",
Hosts: []string{"host1_1", "host1_2"},
Namespace: "ns1",
Partition: "default",
IngressServiceConfig: IngressServiceConfig{
MaxConnections: &maxConnections,
MaxPendingRequests: &maxPendingRequests,
Expand Down
13 changes: 10 additions & 3 deletions control-plane/api/v1alpha1/samenessgroup_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,18 @@ package v1alpha1

import (
"encoding/json"

"github.com/google/go-cmp/cmp"
"github.com/google/go-cmp/cmp/cmpopts"
"github.com/hashicorp/consul-k8s/control-plane/api/common"
"github.com/hashicorp/consul/api"
capi "github.com/hashicorp/consul/api"
corev1 "k8s.io/api/core/v1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/apimachinery/pkg/util/validation/field"

"github.com/hashicorp/consul-k8s/control-plane/api/common"
)

const (
Expand Down Expand Up @@ -166,8 +168,13 @@ func (in *SamenessGroup) MatchesConsul(candidate api.ConfigEntry) bool {
if !ok {
return false
}
return cmp.Equal(in.ToConsul(""), configEntry, cmpopts.IgnoreFields(capi.SamenessGroupConfigEntry{}, "Partition", "Meta", "ModifyIndex", "CreateIndex"), cmpopts.IgnoreUnexported(), cmpopts.EquateEmpty(),
cmp.Comparer(transparentProxyConfigComparer))

specialEquality := cmp.Options{
cmp.FilterPath(func(path cmp.Path) bool {
return path.String() == "Members.Partition"
Copy link
Member

@hashi-derek hashi-derek Nov 30, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sameness groups are enterprise-only, so I'd be surprised if they actually sync through wanfed and modify fields. I guess it doesn't hurt to leave this in place, though.

}, cmp.Transformer("NormalizePartition", normalizeEmptyToDefault)),
}
return cmp.Equal(in.ToConsul(""), configEntry, cmpopts.IgnoreFields(capi.SamenessGroupConfigEntry{}, "Partition", "Meta", "ModifyIndex", "CreateIndex"), cmpopts.IgnoreUnexported(), cmpopts.EquateEmpty(), specialEquality)
}

func (in *SamenessGroup) Validate(consulMeta common.ConsulMeta) error {
Expand Down
15 changes: 12 additions & 3 deletions control-plane/api/v1alpha1/samenessgroup_types_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,15 @@
package v1alpha1

import (
"github.com/hashicorp/consul-k8s/control-plane/api/common"
"testing"
"time"

capi "github.com/hashicorp/consul/api"
"github.com/stretchr/testify/require"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"testing"
"time"

"github.com/hashicorp/consul-k8s/control-plane/api/common"
)

func TestSamenessGroups_ToConsul(t *testing.T) {
Expand Down Expand Up @@ -120,6 +122,9 @@ func TestSamenessGroups_MatchesConsul(t *testing.T) {
{
Partition: "p2",
},
{
Peer: "test-peer",
},
},
},
},
Expand All @@ -139,6 +144,10 @@ func TestSamenessGroups_MatchesConsul(t *testing.T) {
{
Partition: "p2",
},
{
Peer: "test-peer",
Partition: "default",
},
},
},
true,
Expand Down
14 changes: 12 additions & 2 deletions control-plane/api/v1alpha1/servicedefaults_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -703,9 +703,19 @@ func (in *ServiceDefaults) MatchesConsul(candidate capi.ConfigEntry) bool {
if !ok {
return false
}

specialEquality := cmp.Options{
cmp.FilterPath(func(path cmp.Path) bool {
return path.String() == "UpstreamConfig.Overrides.Namespace"
}, cmp.Transformer("NormalizeNamespace", normalizeEmptyToDefault)),
cmp.FilterPath(func(path cmp.Path) bool {
return path.String() == "UpstreamConfig.Overrides.Partition"
}, cmp.Transformer("NormalizePartition", normalizeEmptyToDefault)),
cmp.Comparer(transparentProxyConfigComparer),
}

// No datacenter is passed to ToConsul as we ignore the Meta field when checking for equality.
return cmp.Equal(in.ToConsul(""), configEntry, cmpopts.IgnoreFields(capi.ServiceConfigEntry{}, "Partition", "Namespace", "Meta", "ModifyIndex", "CreateIndex"), cmpopts.IgnoreUnexported(), cmpopts.EquateEmpty(),
cmp.Comparer(transparentProxyConfigComparer))
return cmp.Equal(in.ToConsul(""), configEntry, cmpopts.IgnoreFields(capi.ServiceConfigEntry{}, "Partition", "Namespace", "Meta", "ModifyIndex", "CreateIndex"), cmpopts.IgnoreUnexported(), cmpopts.EquateEmpty(), specialEquality)
}

func (in *ServiceDefaults) ConsulGlobalResource() bool {
Expand Down
4 changes: 2 additions & 2 deletions control-plane/api/v1alpha1/servicedefaults_types_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -565,7 +565,6 @@ func TestServiceDefaults_MatchesConsul(t *testing.T) {
},
{
Name: "upstream-default",
Namespace: "ns",
EnvoyListenerJSON: `{"key": "value"}`,
EnvoyClusterJSON: `{"key": "value"}`,
Protocol: "http2",
Expand Down Expand Up @@ -707,7 +706,8 @@ func TestServiceDefaults_MatchesConsul(t *testing.T) {
},
{
Name: "upstream-default",
Namespace: "ns",
Namespace: "default",
Partition: "default",
EnvoyListenerJSON: `{"key": "value"}`,
EnvoyClusterJSON: `{"key": "value"}`,
Protocol: "http2",
Expand Down
21 changes: 19 additions & 2 deletions control-plane/api/v1alpha1/serviceresolver_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,10 @@ import (
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/apimachinery/pkg/util/validation/field"

"github.com/hashicorp/consul-k8s/control-plane/api/common"
capi "github.com/hashicorp/consul/api"
"github.com/hashicorp/go-bexpr"

"github.com/hashicorp/consul-k8s/control-plane/api/common"
)

const ServiceResolverKubeKind string = "serviceresolver"
Expand Down Expand Up @@ -325,8 +326,24 @@ func (in *ServiceResolver) MatchesConsul(candidate capi.ConfigEntry) bool {
if !ok {
return false
}

specialEquality := cmp.Options{
cmp.FilterPath(func(path cmp.Path) bool {
return path.String() == "Redirect.Namespace"
}, cmp.Transformer("NormalizeNamespace", normalizeEmptyToDefault)),
cmp.FilterPath(func(path cmp.Path) bool {
return path.String() == "Redirect.Partition"
}, cmp.Transformer("NormalizePartition", normalizeEmptyToDefault)),
cmp.FilterPath(func(path cmp.Path) bool {
return path.String() == "Failover.Targets.Namespace"
}, cmp.Transformer("NormalizeNamespace", normalizeEmptyToDefault)),
cmp.FilterPath(func(path cmp.Path) bool {
return path.String() == "Failover.Targets.Partition"
}, cmp.Transformer("NormalizePartition", normalizeEmptyToDefault)),
}

// No datacenter is passed to ToConsul as we ignore the Meta field when checking for equality.
return cmp.Equal(in.ToConsul(""), configEntry, cmpopts.IgnoreFields(capi.ServiceResolverConfigEntry{}, "Partition", "Namespace", "Meta", "ModifyIndex", "CreateIndex"), cmpopts.IgnoreUnexported(), cmpopts.EquateEmpty())
return cmp.Equal(in.ToConsul(""), configEntry, cmpopts.IgnoreFields(capi.ServiceResolverConfigEntry{}, "Partition", "Namespace", "Meta", "ModifyIndex", "CreateIndex"), cmpopts.IgnoreUnexported(), cmpopts.EquateEmpty(), specialEquality)
}

func (in *ServiceResolver) ConsulGlobalResource() bool {
Expand Down
6 changes: 5 additions & 1 deletion control-plane/api/v1alpha1/serviceresolver_types_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,13 @@ import (
"testing"
"time"

"github.com/hashicorp/consul-k8s/control-plane/api/common"
capi "github.com/hashicorp/consul/api"
"github.com/stretchr/testify/require"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/util/validation/field"

"github.com/hashicorp/consul-k8s/control-plane/api/common"
)

func TestServiceResolver_MatchesConsul(t *testing.T) {
Expand Down Expand Up @@ -63,6 +64,7 @@ func TestServiceResolver_MatchesConsul(t *testing.T) {
Service: "redirect",
ServiceSubset: "redirect_subset",
Namespace: "redirect_namespace",
Partition: "default",
Datacenter: "redirect_datacenter",
Peer: "redirect_peer",
},
Expand Down Expand Up @@ -96,6 +98,7 @@ func TestServiceResolver_MatchesConsul(t *testing.T) {
Targets: []ServiceResolverFailoverTarget{
{Peer: "failover_peer3"},
{Partition: "failover_partition3", Namespace: "failover_namespace3"},
{Peer: "failover_peer4"},
},
Policy: &FailoverPolicy{
Mode: "order-by-locality",
Expand Down Expand Up @@ -181,6 +184,7 @@ func TestServiceResolver_MatchesConsul(t *testing.T) {
Targets: []capi.ServiceResolverFailoverTarget{
{Peer: "failover_peer3"},
{Partition: "failover_partition3", Namespace: "failover_namespace3"},
{Peer: "failover_peer4", Partition: "default", Namespace: "default"},
},
Policy: &capi.ServiceResolverFailoverPolicy{
Mode: "order-by-locality",
Expand Down
17 changes: 14 additions & 3 deletions control-plane/api/v1alpha1/servicerouter_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,15 @@ import (

"github.com/google/go-cmp/cmp"
"github.com/google/go-cmp/cmp/cmpopts"
"github.com/hashicorp/consul-k8s/control-plane/api/common"
"github.com/hashicorp/consul-k8s/control-plane/namespaces"
capi "github.com/hashicorp/consul/api"
corev1 "k8s.io/api/core/v1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/apimachinery/pkg/util/validation/field"

"github.com/hashicorp/consul-k8s/control-plane/api/common"
"github.com/hashicorp/consul-k8s/control-plane/namespaces"
)

func init() {
Expand Down Expand Up @@ -253,8 +254,18 @@ func (in *ServiceRouter) MatchesConsul(candidate capi.ConfigEntry) bool {
if !ok {
return false
}

specialEquality := cmp.Options{
cmp.FilterPath(func(path cmp.Path) bool {
return path.String() == "Routes.Destination.Namespace"
}, cmp.Transformer("NormalizeNamespace", normalizeEmptyToDefault)),
cmp.FilterPath(func(path cmp.Path) bool {
return path.String() == "Routes.Destination.Partition"
}, cmp.Transformer("NormalizePartition", normalizeEmptyToDefault)),
}

// No datacenter is passed to ToConsul as we ignore the Meta field when checking for equality.
return cmp.Equal(in.ToConsul(""), configEntry, cmpopts.IgnoreFields(capi.ServiceRouterConfigEntry{}, "Partition", "Namespace", "Meta", "ModifyIndex", "CreateIndex"), cmpopts.IgnoreUnexported(), cmpopts.EquateEmpty())
return cmp.Equal(in.ToConsul(""), configEntry, cmpopts.IgnoreFields(capi.ServiceRouterConfigEntry{}, "Partition", "Namespace", "Meta", "ModifyIndex", "CreateIndex"), cmpopts.IgnoreUnexported(), cmpopts.EquateEmpty(), specialEquality)
}

func (in *ServiceRouter) Validate(consulMeta common.ConsulMeta) error {
Expand Down
4 changes: 3 additions & 1 deletion control-plane/api/v1alpha1/servicerouter_types_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,12 @@ import (
"time"

"github.com/google/go-cmp/cmp"
"github.com/hashicorp/consul-k8s/control-plane/api/common"
capi "github.com/hashicorp/consul/api"
"github.com/stretchr/testify/require"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"

"github.com/hashicorp/consul-k8s/control-plane/api/common"
)

// Test MatchesConsul.
Expand Down Expand Up @@ -158,6 +159,7 @@ func TestServiceRouter_MatchesConsul(t *testing.T) {
Service: "service",
ServiceSubset: "serviceSubset",
Namespace: "namespace",
Partition: "default",
PrefixRewrite: "prefixRewrite",
IdleTimeout: 1 * time.Second,
RequestTimeout: 1 * time.Second,
Expand Down
Loading