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

Add Peer to serviceDefaults #1853

Merged
merged 2 commits into from
Feb 3, 2023
Merged

Add Peer to serviceDefaults #1853

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
0e67116
feat: add peer to service-defaults overrides
DanStough Jan 26, 2023
d2b8c9c
chore: fix generation for peering CRDs
DanStough Jan 26, 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
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ IMPROVEMENTS:
* Add the `accessLogs` field to the `ProxyDefaults` CRD. [[GH-1816](https://github.com/hashicorp/consul-k8s/pull/1816)]
* Add the `envoyExtensions` field to the `ProxyDefaults` and `ServiceDefaults` CRD. [[GH-1823]](https://github.com/hashicorp/consul-k8s/pull/1823)
* Add the `balanceInboundConnections` field to the `ServiceDefaults` CRD. [[GH-1823]](https://github.com/hashicorp/consul-k8s/pull/1823)
* Add the `upstreamConfig.overrides[].peer` field to the `ServiceDefaults` CRD. [[GH-1853]](https://github.com/hashicorp/consul-k8s/pull/1853)
* Control-Plane
* Add support for the annotation `consul.hashicorp.com/use-proxy-health-check`. When this annotation is used by a service, it configures a readiness endpoint on Consul Dataplane and queries it instead of the proxy's inbound port which forwards requests to the application. [[GH-1824](https://github.com/hashicorp/consul-k8s/pull/1824)], [[GH-1841](https://github.com/hashicorp/consul-k8s/pull/1841)]
* Add health check for synced services based on the status of the Kubernetes readiness probe on synced pod. [[GH-1821](https://github.com/hashicorp/consul-k8s/pull/1821)]
Expand Down
20 changes: 14 additions & 6 deletions charts/consul/templates/crd-servicedefaults.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -258,15 +258,15 @@ spec:
type: string
type: object
name:
description: Name is only accepted within a service-defaults
description: Name is only accepted within service ServiceDefaultsSpec.UpstreamConfig.Overrides
config entry.
type: string
namespace:
description: Namespace is only accepted within a service-defaults
description: Namespace is only accepted within service ServiceDefaultsSpec.UpstreamConfig.Overrides
config entry.
type: string
partition:
description: Partition is only accepted within a service-defaults
description: Partition is only accepted within service ServiceDefaultsSpec.UpstreamConfig.Overrides
config entry.
type: string
passiveHealthCheck:
Expand All @@ -291,6 +291,10 @@ spec:
format: int32
type: integer
type: object
peer:
description: Peer is only accepted within service ServiceDefaultsSpec.UpstreamConfig.Overrides
config entry.
type: string
protocol:
description: Protocol describes the upstream's service protocol.
Valid values are "tcp", "http" and "grpc". Anything else
Expand Down Expand Up @@ -357,15 +361,15 @@ spec:
type: string
type: object
name:
description: Name is only accepted within a service-defaults
description: Name is only accepted within service ServiceDefaultsSpec.UpstreamConfig.Overrides
config entry.
type: string
namespace:
description: Namespace is only accepted within a service-defaults
description: Namespace is only accepted within service ServiceDefaultsSpec.UpstreamConfig.Overrides
config entry.
type: string
partition:
description: Partition is only accepted within a service-defaults
description: Partition is only accepted within service ServiceDefaultsSpec.UpstreamConfig.Overrides
config entry.
type: string
passiveHealthCheck:
Expand All @@ -392,6 +396,10 @@ spec:
format: int32
type: integer
type: object
peer:
description: Peer is only accepted within service ServiceDefaultsSpec.UpstreamConfig.Overrides
config entry.
type: string
protocol:
description: Protocol describes the upstream's service protocol.
Valid values are "tcp", "http" and "grpc". Anything else
Expand Down
27 changes: 23 additions & 4 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 @@ -7,14 +7,15 @@ import (

"github.com/google/go-cmp/cmp"
"github.com/google/go-cmp/cmp/cmpopts"
"github.com/hashicorp/consul-k8s/control-plane/api/common"
capi "github.com/hashicorp/consul/api"
"github.com/miekg/dns"
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 @@ -113,12 +114,14 @@ type Upstreams struct {
}

type Upstream struct {
// Name is only accepted within a service-defaults config entry.
// Name is only accepted within service ServiceDefaultsSpec.UpstreamConfig.Overrides config entry.
Name string `json:"name,omitempty"`
// Namespace is only accepted within a service-defaults config entry.
// Namespace is only accepted within service ServiceDefaultsSpec.UpstreamConfig.Overrides config entry.
Namespace string `json:"namespace,omitempty"`
// Partition is only accepted within a service-defaults config entry.
// Partition is only accepted within service ServiceDefaultsSpec.UpstreamConfig.Overrides config entry.
Partition string `json:"partition,omitempty"`
// Peer is only accepted within service ServiceDefaultsSpec.UpstreamConfig.Overrides config entry.
Peer string `json:"peer,omitempty"`
// EnvoyListenerJSON is a complete override ("escape hatch") for the upstream's
// listener.
// Note: This escape hatch is NOT compatible with the discovery chain and
Expand Down Expand Up @@ -374,10 +377,25 @@ func (in *Upstream) validate(path *field.Path, kind string, partitionsEnabled bo
if in.Name != "" {
errs = append(errs, field.Invalid(path.Child("name"), in.Name, "upstream.name for a default upstream must be \"\""))
}
if in.Namespace != "" {
DanStough marked this conversation as resolved.
Show resolved Hide resolved
errs = append(errs, field.Invalid(path.Child("namespace"), in.Namespace, "upstream.namespace for a default upstream must be \"\""))
}
if in.Partition != "" {
errs = append(errs, field.Invalid(path.Child("partition"), in.Partition, "upstream.partition for a default upstream must be \"\""))
}
if in.Peer != "" {
errs = append(errs, field.Invalid(path.Child("peer"), in.Peer, "upstream.peer for a default upstream must be \"\""))
}
} else if kind == overrideUpstream {
if in.Name == "" {
errs = append(errs, field.Invalid(path.Child("name"), in.Name, "upstream.name for an override upstream cannot be \"\""))
}
if in.Namespace != "" && in.Peer != "" {
errs = append(errs, field.Invalid(path, in, "both namespace and peer cannot be specified."))
}
if in.Partition != "" && in.Peer != "" {
errs = append(errs, field.Invalid(path, in, "both partition and peer cannot be specified."))
}
}
if !partitionsEnabled && in.Partition != "" {
errs = append(errs, field.Invalid(path.Child("partition"), in.Partition, "Consul Enterprise Admin Partitions must be enabled to set upstream.partition"))
Expand All @@ -396,6 +414,7 @@ func (in *Upstream) toConsul() *capi.UpstreamConfig {
Name: in.Name,
Namespace: in.Namespace,
Partition: in.Partition,
Peer: in.Peer,
DanStough marked this conversation as resolved.
Show resolved Hide resolved
EnvoyListenerJSON: in.EnvoyListenerJSON,
EnvoyClusterJSON: in.EnvoyClusterJSON,
Protocol: in.Protocol,
Expand Down
73 changes: 71 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 @@ -5,12 +5,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/utils/pointer"

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

func TestServiceDefaults_ToConsul(t *testing.T) {
Expand Down Expand Up @@ -854,6 +855,21 @@ func TestServiceDefaults_Validate(t *testing.T) {
},
expectedErrMsg: `servicedefaults.consul.hashicorp.com "my-service" is invalid: spec.upstreamConfig.defaults.name: Invalid value: "foobar": upstream.name for a default upstream must be ""`,
},
"upstreamConfig.defaults.namespace": {
input: &ServiceDefaults{
ObjectMeta: metav1.ObjectMeta{
Name: "my-service",
},
Spec: ServiceDefaultsSpec{
UpstreamConfig: &Upstreams{
Defaults: &Upstream{
Namespace: "foobar",
},
},
},
},
expectedErrMsg: `servicedefaults.consul.hashicorp.com "my-service" is invalid: spec.upstreamConfig.defaults.namespace: Invalid value: "foobar": upstream.namespace for a default upstream must be ""`,
},
"upstreamConfig.defaults.partition": {
input: &ServiceDefaults{
ObjectMeta: metav1.ObjectMeta{
Expand All @@ -868,7 +884,22 @@ func TestServiceDefaults_Validate(t *testing.T) {
},
},
partitionsEnabled: false,
expectedErrMsg: `servicedefaults.consul.hashicorp.com "my-service" is invalid: spec.upstreamConfig.defaults.partition: Invalid value: "upstream": Consul Enterprise Admin Partitions must be enabled to set upstream.partition`,
expectedErrMsg: `servicedefaults.consul.hashicorp.com "my-service" is invalid: [spec.upstreamConfig.defaults.partition: Invalid value: "upstream": upstream.partition for a default upstream must be "", spec.upstreamConfig.defaults.partition: Invalid value: "upstream": Consul Enterprise Admin Partitions must be enabled to set upstream.partition]`,
},
"upstreamConfig.defaults.peer": {
input: &ServiceDefaults{
ObjectMeta: metav1.ObjectMeta{
Name: "my-service",
},
Spec: ServiceDefaultsSpec{
UpstreamConfig: &Upstreams{
Defaults: &Upstream{
Peer: "foobar",
},
},
},
},
expectedErrMsg: `servicedefaults.consul.hashicorp.com "my-service" is invalid: spec.upstreamConfig.defaults.peer: Invalid value: "foobar": upstream.peer for a default upstream must be ""`,
},
"upstreamConfig.overrides.meshGateway": {
input: &ServiceDefaults{
Expand Down Expand Up @@ -925,6 +956,44 @@ func TestServiceDefaults_Validate(t *testing.T) {
},
expectedErrMsg: `servicedefaults.consul.hashicorp.com "my-service" is invalid: spec.upstreamConfig.overrides[0].partition: Invalid value: "upstream": Consul Enterprise Admin Partitions must be enabled to set upstream.partition`,
},
"upstreamConfig.overrides.partition and namespace": {
input: &ServiceDefaults{
ObjectMeta: metav1.ObjectMeta{
Name: "my-service",
},
Spec: ServiceDefaultsSpec{
UpstreamConfig: &Upstreams{
Overrides: []*Upstream{
{
Name: "service",
Namespace: "namespace",
Peer: "peer",
},
},
},
},
},
expectedErrMsg: `servicedefaults.consul.hashicorp.com "my-service" is invalid: spec.upstreamConfig.overrides[0]: Invalid value: v1alpha1.Upstream{Name:"service", Namespace:"namespace", Partition:"", Peer:"peer", EnvoyListenerJSON:"", EnvoyClusterJSON:"", Protocol:"", ConnectTimeoutMs:0, Limits:(*v1alpha1.UpstreamLimits)(nil), PassiveHealthCheck:(*v1alpha1.PassiveHealthCheck)(nil), MeshGateway:v1alpha1.MeshGateway{Mode:""}}: both namespace and peer cannot be specified.`,
},
"upstreamConfig.overrides.partition and peer": {
input: &ServiceDefaults{
ObjectMeta: metav1.ObjectMeta{
Name: "my-service",
},
Spec: ServiceDefaultsSpec{
UpstreamConfig: &Upstreams{
Overrides: []*Upstream{
{
Name: "service",
Partition: "upstream",
Peer: "peer",
},
},
},
},
},
expectedErrMsg: `servicedefaults.consul.hashicorp.com "my-service" is invalid: [spec.upstreamConfig.overrides[0]: Invalid value: v1alpha1.Upstream{Name:"service", Namespace:"", Partition:"upstream", Peer:"peer", EnvoyListenerJSON:"", EnvoyClusterJSON:"", Protocol:"", ConnectTimeoutMs:0, Limits:(*v1alpha1.UpstreamLimits)(nil), PassiveHealthCheck:(*v1alpha1.PassiveHealthCheck)(nil), MeshGateway:v1alpha1.MeshGateway{Mode:""}}: both partition and peer cannot be specified., spec.upstreamConfig.overrides[0].partition: Invalid value: "upstream": Consul Enterprise Admin Partitions must be enabled to set upstream.partition]`,
},
"multi-error": {
input: &ServiceDefaults{
ObjectMeta: metav1.ObjectMeta{
Expand Down
20 changes: 14 additions & 6 deletions control-plane/config/crd/bases/consul.hashicorp.com_servicedefaults.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -251,15 +251,15 @@ spec:
type: string
type: object
name:
description: Name is only accepted within a service-defaults
description: Name is only accepted within service ServiceDefaultsSpec.UpstreamConfig.Overrides
config entry.
type: string
namespace:
description: Namespace is only accepted within a service-defaults
description: Namespace is only accepted within service ServiceDefaultsSpec.UpstreamConfig.Overrides
config entry.
type: string
partition:
description: Partition is only accepted within a service-defaults
description: Partition is only accepted within service ServiceDefaultsSpec.UpstreamConfig.Overrides
config entry.
type: string
passiveHealthCheck:
Expand All @@ -284,6 +284,10 @@ spec:
format: int32
type: integer
type: object
peer:
description: Peer is only accepted within service ServiceDefaultsSpec.UpstreamConfig.Overrides
config entry.
type: string
protocol:
description: Protocol describes the upstream's service protocol.
Valid values are "tcp", "http" and "grpc". Anything else
Expand Down Expand Up @@ -350,15 +354,15 @@ spec:
type: string
type: object
name:
description: Name is only accepted within a service-defaults
description: Name is only accepted within service ServiceDefaultsSpec.UpstreamConfig.Overrides
config entry.
type: string
namespace:
description: Namespace is only accepted within a service-defaults
description: Namespace is only accepted within service ServiceDefaultsSpec.UpstreamConfig.Overrides
config entry.
type: string
partition:
description: Partition is only accepted within a service-defaults
description: Partition is only accepted within service ServiceDefaultsSpec.UpstreamConfig.Overrides
config entry.
type: string
passiveHealthCheck:
Expand All @@ -385,6 +389,10 @@ spec:
format: int32
type: integer
type: object
peer:
description: Peer is only accepted within service ServiceDefaultsSpec.UpstreamConfig.Overrides
config entry.
type: string
protocol:
description: Protocol describes the upstream's service protocol.
Valid values are "tcp", "http" and "grpc". Anything else
Expand Down
2 changes: 1 addition & 1 deletion control-plane/go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ require (
github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510
github.com/hashicorp/consul-k8s/control-plane/cni v0.0.0-20220831174802-b8af65262de8
github.com/hashicorp/consul-server-connection-manager v0.1.0
github.com/hashicorp/consul/api v1.10.1-0.20230106171340-8d923c178919
github.com/hashicorp/consul/api v1.10.1-0.20230203155153-2f149d60ccbf
github.com/hashicorp/consul/sdk v0.13.0
github.com/hashicorp/go-discover v0.0.0-20200812215701-c4b85f6ed31f
github.com/hashicorp/go-hclog v1.2.2
Expand Down
4 changes: 4 additions & 0 deletions control-plane/go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -348,6 +348,10 @@ github.com/hashicorp/consul-server-connection-manager v0.1.0/go.mod h1:XVVlO+Yk7
github.com/hashicorp/consul/api v1.1.0/go.mod h1:VmuI/Lkw1nC05EYQWNKwWGbkg+FbDBtguAZLlVdkD9Q=
github.com/hashicorp/consul/api v1.10.1-0.20230106171340-8d923c178919 h1:8aVegJMSv7PIAAa1zqQQ0CT4TKv+Nf7I4rhE6+uDa1U=
github.com/hashicorp/consul/api v1.10.1-0.20230106171340-8d923c178919/go.mod h1:c1u8FzGHcavbEtRW/p1YditvfMgn4QsKNgz2rnCDF7c=
github.com/hashicorp/consul/api v1.10.1-0.20230126204442-a43eadd1225b h1:dNIQYhru10Hg+E1oEL8f9CX6MC+8CW5JuQ4jk3g70LA=
github.com/hashicorp/consul/api v1.10.1-0.20230126204442-a43eadd1225b/go.mod h1:c1u8FzGHcavbEtRW/p1YditvfMgn4QsKNgz2rnCDF7c=
github.com/hashicorp/consul/api v1.10.1-0.20230203155153-2f149d60ccbf h1:vvsHghmX3LyNUaDe7onYKHyDiny+ystdHKIEujbNj4Q=
github.com/hashicorp/consul/api v1.10.1-0.20230203155153-2f149d60ccbf/go.mod h1:c1u8FzGHcavbEtRW/p1YditvfMgn4QsKNgz2rnCDF7c=
github.com/hashicorp/consul/proto-public v0.1.0 h1:O0LSmCqydZi363hsqc6n2v5sMz3usQMXZF6ziK3SzXU=
github.com/hashicorp/consul/proto-public v0.1.0/go.mod h1:vs2KkuWwtjkIgA5ezp4YKPzQp4GitV+q/+PvksrA92k=
github.com/hashicorp/consul/sdk v0.4.1-0.20221021205723-cc843c4be892 h1:jw0NwPmNPr5CxAU04hACdj61JSaJBKZ0FdBo+kwfNp4=
Expand Down
17 changes: 15 additions & 2 deletions hack/copy-crds-to-chart/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,14 @@ import (
"strings"
)

var (
// HACK IT!
requiresPeering = map[string]struct{}{
"consul.hashicorp.com_peeringacceptors.yaml": {},
"consul.hashicorp.com_peeringdialers.yaml": {},
}
DanStough marked this conversation as resolved.
Show resolved Hide resolved
)

func main() {
if len(os.Args) != 1 {
fmt.Println("Usage: go run ./...")
Expand Down Expand Up @@ -43,8 +51,13 @@ func realMain(helmPath string) error {
// Strip leading newline.
contents = strings.TrimPrefix(contents, "\n")

// Add {{- if .Values.connectInject.enabled }} {{- end }} wrapper.
contents = fmt.Sprintf("{{- if .Values.connectInject.enabled }}\n%s{{- end }}\n", contents)
if _, ok := requiresPeering[info.Name()]; ok {
// Add {{- if and .Values.connectInject.enabled .Values.global.peering.enabled }} {{- end }} wrapper.
contents = fmt.Sprintf("{{- if and .Values.connectInject.enabled .Values.global.peering.enabled }}\n%s{{- end }}\n", contents)
} else {
// Add {{- if .Values.connectInject.enabled }} {{- end }} wrapper.
contents = fmt.Sprintf("{{- if .Values.connectInject.enabled }}\n%s{{- end }}\n", contents)
}

// Add labels, this is hacky because we're relying on the line number
// but it means we don't need to regex or yaml parse.
Expand Down