From 6c8c7ee9b6c889b8b9fcb695fc7de74ab9f90d47 Mon Sep 17 00:00:00 2001 From: Nitya Dhanushkodi Date: Sun, 12 Jun 2022 22:19:11 -0700 Subject: [PATCH 1/2] add a helm value for cluster peering --- .../templates/connect-inject-deployment.yaml | 3 ++ .../templates/crd-peeringacceptors.yaml | 2 +- .../consul/templates/crd-peeringdialers.yaml | 2 +- .../test/unit/connect-inject-deployment.bats | 27 +++++++++++ charts/consul/values.yaml | 7 +++ .../subcommand/inject-connect/command.go | 45 +++++++++++-------- 6 files changed, 65 insertions(+), 21 deletions(-) diff --git a/charts/consul/templates/connect-inject-deployment.yaml b/charts/consul/templates/connect-inject-deployment.yaml index 9865ff0b01..42a589188e 100644 --- a/charts/consul/templates/connect-inject-deployment.yaml +++ b/charts/consul/templates/connect-inject-deployment.yaml @@ -111,6 +111,9 @@ spec: {{- else }} -default-enable-transparent-proxy=false \ {{- end }} + {{- if .Values.global.peering.enabled }} + -enable-peering=true \ + {{- end }} {{- if .Values.global.openshift.enabled }} -enable-openshift \ {{- end }} diff --git a/charts/consul/templates/crd-peeringacceptors.yaml b/charts/consul/templates/crd-peeringacceptors.yaml index 0350d64c51..a5242ad311 100644 --- a/charts/consul/templates/crd-peeringacceptors.yaml +++ b/charts/consul/templates/crd-peeringacceptors.yaml @@ -1,4 +1,4 @@ -{{- if .Values.connectInject.enabled }} +{{- if and .Values.connectInject.enabled .Values.global.peering.enabled }} --- apiVersion: apiextensions.k8s.io/v1 kind: CustomResourceDefinition diff --git a/charts/consul/templates/crd-peeringdialers.yaml b/charts/consul/templates/crd-peeringdialers.yaml index 7dbcdb9402..f03a1790ad 100644 --- a/charts/consul/templates/crd-peeringdialers.yaml +++ b/charts/consul/templates/crd-peeringdialers.yaml @@ -1,4 +1,4 @@ -{{- if .Values.connectInject.enabled }} +{{- if and .Values.connectInject.enabled .Values.global.peering.enabled }} --- apiVersion: apiextensions.k8s.io/v1 kind: CustomResourceDefinition diff --git a/charts/consul/test/unit/connect-inject-deployment.bats b/charts/consul/test/unit/connect-inject-deployment.bats index 5dfa8d6d89..91c9380df9 100755 --- a/charts/consul/test/unit/connect-inject-deployment.bats +++ b/charts/consul/test/unit/connect-inject-deployment.bats @@ -1730,6 +1730,33 @@ EOF [ "${actual}" = "true" ] } +#-------------------------------------------------------------------- +# peering + +@test "connectInject/Deployment: peering is not set by default" { + cd `chart_dir` + local actual=$(helm template \ + -s templates/connect-inject-deployment.yaml \ + --set 'connectInject.enabled=true' \ + . | tee /dev/stderr | + yq '.spec.template.spec.containers[0].command | any(contains("-enable-peering=true"))' | tee /dev/stderr) + + [ "${actual}" = "false" ] +} + +@test "connectInject/Deployment: -enable-peering=true is set when global.peering.enabled is true" { + cd `chart_dir` + local actual=$(helm template \ + -s templates/connect-inject-deployment.yaml \ + --set 'connectInject.enabled=true' \ + --set 'global.peering.enabled=true' \ + . | tee /dev/stderr | + yq '.spec.template.spec.containers[0].command | any(contains("-enable-peering=true"))' | tee /dev/stderr) + + [ "${actual}" = "true" ] +} + + #-------------------------------------------------------------------- # openshift diff --git a/charts/consul/values.yaml b/charts/consul/values.yaml index 81de83386a..5506a5734a 100644 --- a/charts/consul/values.yaml +++ b/charts/consul/values.yaml @@ -29,6 +29,13 @@ global: # Consul into Kubernetes will have, e.g. `service-name.service.consul`. domain: consul + # Configures the Cluster Peering feature. + peering: + # If true, the Helm chart will enable Cluster Peering for the cluster. This will enable peering controllers and + # allow use of the PeeringAcceptor and PeeringDialer CRDs to establish peerings for service mesh. + # @type boolean + enabled: false + # [Enterprise Only] Enabling `adminPartitions` allows creation of Admin Partitions in Kubernetes clusters. # It additionally indicates that you are running Consul Enterprise v1.11+ with a valid Consul Enterprise # license. Admin partitions enables deploying services across partitions, while sharing diff --git a/control-plane/subcommand/inject-connect/command.go b/control-plane/subcommand/inject-connect/command.go index c19d37c697..e6ec337197 100644 --- a/control-plane/subcommand/inject-connect/command.go +++ b/control-plane/subcommand/inject-connect/command.go @@ -93,6 +93,9 @@ type Command struct { flagDefaultEnableTransparentProxy bool flagTransparentProxyDefaultOverwriteProbes bool + // Peering flags. + flagEnablePeering bool + // Consul DNS flags. flagEnableConsulDNS bool flagResourcePrefix string @@ -133,6 +136,7 @@ func (c *Command) init() { "Docker image for Envoy.") c.flagSet.StringVar(&c.flagConsulK8sImage, "consul-k8s-image", "", "Docker image for consul-k8s. Used for the connect sidecar.") + c.flagSet.BoolVar(&c.flagEnablePeering, "enable-peering", false, "Enable cluster peering controllers.") c.flagSet.StringVar(&c.flagEnvoyExtraArgs, "envoy-extra-args", "", "Extra envoy command line args to be set when starting envoy (e.g \"--log-level debug --disable-hot-restart\").") c.flagSet.StringVar(&c.flagACLAuthMethod, "acl-auth-method", "", @@ -426,26 +430,29 @@ func (c *Command) Run(args []string) int { return 1 } - if err = (&connectinject.PeeringAcceptorController{ - Client: mgr.GetClient(), - ConsulClient: c.consulClient, - Log: ctrl.Log.WithName("controller").WithName("peering-acceptor"), - Scheme: mgr.GetScheme(), - Context: ctx, - }).SetupWithManager(mgr); err != nil { - setupLog.Error(err, "unable to create controller", "controller", "peering-acceptor") - return 1 - } - if err = (&connectinject.PeeringDialerController{ - Client: mgr.GetClient(), - ConsulClient: c.consulClient, - Log: ctrl.Log.WithName("controller").WithName("peering-dialer"), - Scheme: mgr.GetScheme(), - Context: ctx, - }).SetupWithManager(mgr); err != nil { - setupLog.Error(err, "unable to create controller", "controller", "peering-dialer") - return 1 + if c.flagEnablePeering { + if err = (&connectinject.PeeringAcceptorController{ + Client: mgr.GetClient(), + ConsulClient: c.consulClient, + Log: ctrl.Log.WithName("controller").WithName("peering-acceptor"), + Scheme: mgr.GetScheme(), + Context: ctx, + }).SetupWithManager(mgr); err != nil { + setupLog.Error(err, "unable to create controller", "controller", "peering-acceptor") + return 1 + } + if err = (&connectinject.PeeringDialerController{ + Client: mgr.GetClient(), + ConsulClient: c.consulClient, + Log: ctrl.Log.WithName("controller").WithName("peering-dialer"), + Scheme: mgr.GetScheme(), + Context: ctx, + }).SetupWithManager(mgr); err != nil { + setupLog.Error(err, "unable to create controller", "controller", "peering-dialer") + return 1 + } } + mgr.GetWebhookServer().CertDir = c.flagCertDir mgr.GetWebhookServer().Register("/mutate", From 665446e019679dd0e5b131ac692b25a96ca64654 Mon Sep 17 00:00:00 2001 From: Nitya Dhanushkodi Date: Mon, 13 Jun 2022 11:05:03 -0700 Subject: [PATCH 2/2] add experimental tag to values and crd fields, fail if connect inject is not set when peering is --- charts/consul/templates/connect-inject-deployment.yaml | 1 + charts/consul/templates/crd-exportedservices.yaml | 4 ++-- charts/consul/templates/crd-serviceintentions.yaml | 3 ++- charts/consul/test/unit/connect-inject-deployment.bats | 10 ++++++++++ charts/consul/values.yaml | 2 +- control-plane/api/v1alpha1/exportedservices_types.go | 2 +- control-plane/api/v1alpha1/serviceintentions_types.go | 2 +- .../bases/consul.hashicorp.com_exportedservices.yaml | 4 ++-- .../bases/consul.hashicorp.com_serviceintentions.yaml | 3 ++- 9 files changed, 22 insertions(+), 9 deletions(-) diff --git a/charts/consul/templates/connect-inject-deployment.yaml b/charts/consul/templates/connect-inject-deployment.yaml index 42a589188e..8a9b79f286 100644 --- a/charts/consul/templates/connect-inject-deployment.yaml +++ b/charts/consul/templates/connect-inject-deployment.yaml @@ -1,3 +1,4 @@ +{{- if and .Values.global.peering.enabled (not .Values.connectInject.enabled) }}{{ fail "setting global.peering.enabled to true requires connectInject.enabled to be true" }}{{ end }} {{- if (or (and (ne (.Values.connectInject.enabled | toString) "-") .Values.connectInject.enabled) (and (eq (.Values.connectInject.enabled | toString) "-") .Values.global.enabled)) }} {{- if not (or (and (ne (.Values.client.enabled | toString) "-") .Values.client.enabled) (and (eq (.Values.client.enabled | toString) "-") .Values.global.enabled)) }}{{ fail "clients must be enabled for connect injection" }}{{ end }} {{- if not .Values.client.grpc }}{{ fail "client.grpc must be true for connect injection" }}{{ end }} diff --git a/charts/consul/templates/crd-exportedservices.yaml b/charts/consul/templates/crd-exportedservices.yaml index d6453b6305..6ed5406cba 100644 --- a/charts/consul/templates/crd-exportedservices.yaml +++ b/charts/consul/templates/crd-exportedservices.yaml @@ -76,8 +76,8 @@ spec: the service to. type: string peerName: - description: PeerName is the name of the peer to export - the service to. + description: '[Experimental] PeerName is the name of the + peer to export the service to.' type: string type: object type: array diff --git a/charts/consul/templates/crd-serviceintentions.yaml b/charts/consul/templates/crd-serviceintentions.yaml index 255dffc25b..827fcfe2a9 100644 --- a/charts/consul/templates/crd-serviceintentions.yaml +++ b/charts/consul/templates/crd-serviceintentions.yaml @@ -102,7 +102,8 @@ spec: description: Partition is the Admin Partition for the Name parameter. type: string peer: - description: Peer is the peer name for the Name parameter. + description: '[Experimental] Peer is the peer name for the Name + parameter.' type: string permissions: description: Permissions is the list of all additional L7 attributes diff --git a/charts/consul/test/unit/connect-inject-deployment.bats b/charts/consul/test/unit/connect-inject-deployment.bats index 91c9380df9..09bd62c327 100755 --- a/charts/consul/test/unit/connect-inject-deployment.bats +++ b/charts/consul/test/unit/connect-inject-deployment.bats @@ -1756,6 +1756,16 @@ EOF [ "${actual}" = "true" ] } +@test "connectInject/Deployment: fails if peering is enabled but connect inject is not" { + cd `chart_dir` + run helm template \ + -s templates/connect-inject-deployment.yaml \ + --set 'connectInject.enabled=false' \ + --set 'global.peering.enabled=true' . + [ "$status" -eq 1 ] + [[ "$output" =~ "setting global.peering.enabled to true requires connectInject.enabled to be true" ]] +} + #-------------------------------------------------------------------- # openshift diff --git a/charts/consul/values.yaml b/charts/consul/values.yaml index 5506a5734a..67b06174d5 100644 --- a/charts/consul/values.yaml +++ b/charts/consul/values.yaml @@ -29,7 +29,7 @@ global: # Consul into Kubernetes will have, e.g. `service-name.service.consul`. domain: consul - # Configures the Cluster Peering feature. + # [Experimental] Configures the Cluster Peering feature. Requires Consul v1.13+ and Consul-K8s v0.45+. peering: # If true, the Helm chart will enable Cluster Peering for the cluster. This will enable peering controllers and # allow use of the PeeringAcceptor and PeeringDialer CRDs to establish peerings for service mesh. diff --git a/control-plane/api/v1alpha1/exportedservices_types.go b/control-plane/api/v1alpha1/exportedservices_types.go index 2816f03497..477574d945 100644 --- a/control-plane/api/v1alpha1/exportedservices_types.go +++ b/control-plane/api/v1alpha1/exportedservices_types.go @@ -68,7 +68,7 @@ type ExportedService struct { type ServiceConsumer struct { // Partition is the admin partition to export the service to. Partition string `json:"partition,omitempty"` - // PeerName is the name of the peer to export the service to. + // [Experimental] PeerName is the name of the peer to export the service to. PeerName string `json:"peerName,omitempty"` } diff --git a/control-plane/api/v1alpha1/serviceintentions_types.go b/control-plane/api/v1alpha1/serviceintentions_types.go index 09528dda3c..b2e5cf4872 100644 --- a/control-plane/api/v1alpha1/serviceintentions_types.go +++ b/control-plane/api/v1alpha1/serviceintentions_types.go @@ -78,7 +78,7 @@ type SourceIntention struct { Name string `json:"name,omitempty"` // Namespace is the namespace for the Name parameter. Namespace string `json:"namespace,omitempty"` - // Peer is the peer name for the Name parameter. + // [Experimental] Peer is the peer name for the Name parameter. Peer string `json:"peer,omitempty"` // Partition is the Admin Partition for the Name parameter. Partition string `json:"partition,omitempty"` diff --git a/control-plane/config/crd/bases/consul.hashicorp.com_exportedservices.yaml b/control-plane/config/crd/bases/consul.hashicorp.com_exportedservices.yaml index 3dd8e32471..ddcc51b2b8 100644 --- a/control-plane/config/crd/bases/consul.hashicorp.com_exportedservices.yaml +++ b/control-plane/config/crd/bases/consul.hashicorp.com_exportedservices.yaml @@ -69,8 +69,8 @@ spec: the service to. type: string peerName: - description: PeerName is the name of the peer to export - the service to. + description: '[Experimental] PeerName is the name of the + peer to export the service to.' type: string type: object type: array diff --git a/control-plane/config/crd/bases/consul.hashicorp.com_serviceintentions.yaml b/control-plane/config/crd/bases/consul.hashicorp.com_serviceintentions.yaml index 5a36e7e5d7..a0cc7a6343 100644 --- a/control-plane/config/crd/bases/consul.hashicorp.com_serviceintentions.yaml +++ b/control-plane/config/crd/bases/consul.hashicorp.com_serviceintentions.yaml @@ -95,7 +95,8 @@ spec: description: Partition is the Admin Partition for the Name parameter. type: string peer: - description: Peer is the peer name for the Name parameter. + description: '[Experimental] Peer is the peer name for the Name + parameter.' type: string permissions: description: Permissions is the list of all additional L7 attributes