Skip to content

Commit

Permalink
Add partition support to ingress and terminating gateways
Browse files Browse the repository at this point in the history
  • Loading branch information
Ashwin Venkatesh committed Sep 17, 2021
1 parent 37bdad3 commit 683642d
Show file tree
Hide file tree
Showing 17 changed files with 144 additions and 53 deletions.
4 changes: 2 additions & 2 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ executors:
- image: docker.mirror.hashicorp.services/circleci/golang:1.16
environment:
TEST_RESULTS: /tmp/test-results # path to where test results are saved
CONSUL_VERSION: 1.10.2 # Consul's OSS version to use in tests
CONSUL_ENT_VERSION: 1.10.2+ent # Consul's enterprise version to use in tests
CONSUL_VERSION: 1.11.0-alpha # Consul's OSS version to use in tests
CONSUL_ENT_VERSION: 1.11.0+ent-alpha # Consul's enterprise version to use in tests

control-plane-path : &control-plane-path control-plane
acceptance-test-path: &acceptance-test-path charts/consul/test/acceptance
Expand Down
2 changes: 1 addition & 1 deletion charts/consul/templates/connect-inject-deployment.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ spec:
{{- end }}
{{- if .Values.global.adminPartitions.enabled }}
-enable-partitions=true \
-partition-name={{ .Values.global.adminPartitions.name }} \
-partition={{ .Values.global.adminPartitions.name }} \
{{- end }}
{{- if .Values.global.enableConsulNamespaces }}
-enable-namespaces=true \
Expand Down
10 changes: 10 additions & 0 deletions charts/consul/templates/ingress-gateways-deployment.yaml
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
{{- if .Values.ingressGateways.enabled }}
{{- if not .Values.connectInject.enabled }}{{ fail "connectInject.enabled must be true" }}{{ end -}}
{{- if not .Values.client.grpc }}{{ fail "client.grpc must be true" }}{{ end -}}
{{- if and .Values.global.adminPartitions.enabled (not .Values.global.enableConsulNamespaces) }}{{ fail "global.enableConsulNamespaces must be true if global.adminPartitions.enabled=true" }}{{ end }}
{{- 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" }}{{ end -}}
{{- if .Values.global.lifecycleSidecarContainer }}{{ fail "global.lifecycleSidecarContainer has been renamed to global.consulSidecarContainer. Please set values using global.consulSidecarContainer." }}{{ end }}

Expand Down Expand Up @@ -217,6 +218,9 @@ spec:
{{- if $root.Values.global.enableConsulNamespaces }}
namespace = "{{ (default $defaults.consulNamespace .consulNamespace) }}"
{{- end }}
{{- if $root.Values.global.adminPartitions.enabled }}
partition = "{{ $root.Values.global.adminPartitions.name }}"
{{- end }}
port = ${WAN_PORT}
address = "${WAN_ADDR}"
tagged_addresses {
Expand Down Expand Up @@ -340,6 +344,9 @@ spec:
{{- if $root.Values.global.enableConsulNamespaces }}
- -namespace={{ default $defaults.consulNamespace .consulNamespace }}
{{- end }}
{{- if $root.Values.global.adminPartitions.enabled }}
- -partition={{ $root.Values.global.adminPartitions.name }}
{{- end }}
livenessProbe:
tcpSocket:
port: 21000
Expand Down Expand Up @@ -374,6 +381,9 @@ spec:
{{- if $root.Values.global.enableConsulNamespaces }}
-namespace={{ default $defaults.consulNamespace .consulNamespace }} \
{{- end }}
{{- if $root.Values.global.adminPartitions.enabled }}
-partition={{ $root.Values.global.adminPartitions.name }} \
{{- end }}
-id="${POD_NAME}"
# consul-sidecar ensures the ingress gateway is always registered with
Expand Down
10 changes: 10 additions & 0 deletions charts/consul/templates/terminating-gateways-deployment.yaml
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
{{- if .Values.terminatingGateways.enabled }}
{{- if not .Values.connectInject.enabled }}{{ fail "connectInject.enabled must be true" }}{{ end -}}
{{- if not .Values.client.grpc }}{{ fail "client.grpc must be true" }}{{ end -}}
{{- if and .Values.global.adminPartitions.enabled (not .Values.global.enableConsulNamespaces) }}{{ fail "global.enableConsulNamespaces must be true if global.adminPartitions.enabled=true" }}{{ end }}
{{- 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" }}{{ end -}}
{{- if .Values.global.lifecycleSidecarContainer }}{{ fail "global.lifecycleSidecarContainer has been renamed to global.consulSidecarContainer. Please set values using global.consulSidecarContainer." }}{{ end }}

Expand Down Expand Up @@ -183,6 +184,9 @@ spec:
{{- if $root.Values.global.enableConsulNamespaces }}
namespace = "{{ (default $defaults.consulNamespace .consulNamespace) }}"
{{- end }}
{{- if $root.Values.global.adminPartitions.enabled }}
partition = "{{ $root.Values.global.adminPartitions.name }}"
{{- end }}
address = "${POD_IP}"
port = 8443
{{- if (and $root.Values.global.metrics.enabled $root.Values.global.metrics.enableGatewayMetrics) }}
Expand Down Expand Up @@ -290,6 +294,9 @@ spec:
{{- if $root.Values.global.enableConsulNamespaces }}
- -namespace={{ default $defaults.consulNamespace .consulNamespace }}
{{- end }}
{{- if $root.Values.global.adminPartitions.enabled }}
- -partition={{ $root.Values.global.adminPartitions.name }}
{{- end }}
livenessProbe:
tcpSocket:
port: 8443
Expand Down Expand Up @@ -320,6 +327,9 @@ spec:
{{- if $root.Values.global.enableConsulNamespaces }}
-namespace={{ default $defaults.consulNamespace .consulNamespace }} \
{{- end }}
{{- if $root.Values.global.adminPartitions.enabled }}
-partition={{ $root.Values.global.adminPartitions.name }} \
{{- end }}
-id="${POD_NAME}"
# consul-sidecar ensures the terminating gateway is always registered with
Expand Down
12 changes: 0 additions & 12 deletions charts/consul/test/unit/connect-inject-deployment.bats
Original file line number Diff line number Diff line change
Expand Up @@ -714,18 +714,6 @@ EOF
[ "${actual}" = "true" ]
}

@test "connectInject/Deployment: partition name set with .global.adminPartitions.enabled=true" {
cd `chart_dir`
local actual=$(helm template \
-s templates/connect-inject-deployment.yaml \
--set 'connectInject.enabled=true' \
--set 'global.adminPartitions.enabled=true' \
. | tee /dev/stderr |
yq '.spec.template.spec.containers[0].command | any(contains("partition-name=default"))' | tee /dev/stderr)

[ "${actual}" = "true" ]
}

@test "connectInject/Deployment: fails if namespaces are disabled and .global.adminPartitions.enabled=true" {
cd `chart_dir`
run helm template \
Expand Down
51 changes: 51 additions & 0 deletions charts/consul/test/unit/ingress-gateways-deployment.bats
Original file line number Diff line number Diff line change
Expand Up @@ -1405,6 +1405,57 @@ EOF
[ "${actual}" = "true" ]
}

#--------------------------------------------------------------------
# partitions

@test "ingressGateways/Deployment: partition command flag is not present by default" {
cd `chart_dir`
local object=$(helm template \
-s templates/ingress-gateways-deployment.yaml \
--set 'ingressGateways.enabled=true' \
--set 'connectInject.enabled=true' \
. | tee /dev/stderr |
yq -s -r '.[0].spec.template.spec.containers[0]' | tee /dev/stderr)

local actual=$(echo $object | yq -r '.command | any(contains("-partition"))' | tee /dev/stderr)
[ "${actual}" = "false" ]

local actual=$(echo $object | yq -r '.lifecycle.preStop.exec.command | any(contains("-partition"))' | tee /dev/stderr)
[ "${actual}" = "false" ]
}

@test "ingressGateways/Deployment: partition command flag is specified through partition name" {
cd `chart_dir`
local object=$(helm template \
-s templates/ingress-gateways-deployment.yaml \
--set 'ingressGateways.enabled=true' \
--set 'connectInject.enabled=true' \
--set 'global.enableConsulNamespaces=true' \
--set 'global.adminPartitions.enabled=true' \
--set 'global.adminPartitions.name=default' \
. | tee /dev/stderr |
yq -s -r '.[0].spec.template.spec.containers[0]' | tee /dev/stderr)

local actual=$(echo $object | yq -r '.command | any(contains("-partition=default"))' | tee /dev/stderr)
[ "${actual}" = "true" ]

local actual=$(echo $object | yq -r '.lifecycle.preStop.exec.command | any(contains("-partition=default"))' | tee /dev/stderr)
[ "${actual}" = "true" ]
}

@test "ingressGateways/Deployment: fails if admin partitions are enabled but namespaces aren't" {
cd `chart_dir`
run helm template \
-s templates/ingress-gateways-deployment.yaml \
--set 'ingressGateways.enabled=true' \
--set 'connectInject.enabled=true' \
--set 'global.enableConsulNamespaces=false' \
--set 'global.adminPartitions.enabled=true' .

[ "$status" -eq 1 ]
[[ "$output" =~ "global.enableConsulNamespaces must be true if global.adminPartitions.enabled=true" ]]
}

#--------------------------------------------------------------------
# multiple gateways

Expand Down
51 changes: 51 additions & 0 deletions charts/consul/test/unit/terminating-gateways-deployment.bats
Original file line number Diff line number Diff line change
Expand Up @@ -1215,6 +1215,57 @@ EOF
[ "${actual}" = "true" ]
}

#--------------------------------------------------------------------
# partitions

@test "terminatingGateways/Deployment: partition command flag is not present by default" {
cd `chart_dir`
local object=$(helm template \
-s templates/terminating-gateways-deployment.yaml \
--set 'terminatingGateways.enabled=true' \
--set 'connectInject.enabled=true' \
. | tee /dev/stderr |
yq -s -r '.[0].spec.template.spec.containers[0]' | tee /dev/stderr)

local actual=$(echo $object | yq -r '.command | any(contains("-partition"))' | tee /dev/stderr)
[ "${actual}" = "false" ]

local actual=$(echo $object | yq -r '.lifecycle.preStop.exec.command | any(contains("-partition"))' | tee /dev/stderr)
[ "${actual}" = "false" ]
}

@test "terminatingGateways/Deployment: partition command flag is specified through partition name" {
cd `chart_dir`
local object=$(helm template \
-s templates/terminating-gateways-deployment.yaml \
--set 'terminatingGateways.enabled=true' \
--set 'connectInject.enabled=true' \
--set 'global.enableConsulNamespaces=true' \
--set 'global.adminPartitions.enabled=true' \
--set 'global.adminPartitions.name=default' \
. | tee /dev/stderr |
yq -s -r '.[0].spec.template.spec.containers[0]' | tee /dev/stderr)

local actual=$(echo $object | yq -r '.command | any(contains("-partition=default"))' | tee /dev/stderr)
[ "${actual}" = "true" ]

local actual=$(echo $object | yq -r '.lifecycle.preStop.exec.command | any(contains("-partition=default"))' | tee /dev/stderr)
[ "${actual}" = "true" ]
}

@test "terminatingGateways/Deployment: fails if admin partitions are enabled but namespaces aren't" {
cd `chart_dir`
run helm template \
-s templates/terminating-gateways-deployment.yaml \
--set 'terminatingGateways.enabled=true' \
--set 'connectInject.enabled=true' \
--set 'global.enableConsulNamespaces=false' \
--set 'global.adminPartitions.enabled=true' .

[ "$status" -eq 1 ]
[[ "$output" =~ "global.enableConsulNamespaces must be true if global.adminPartitions.enabled=true" ]]
}

#--------------------------------------------------------------------
# multiple gateways

Expand Down
19 changes: 0 additions & 19 deletions control-plane/connect-inject/endpoints_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -351,25 +351,6 @@ func TestProcessUpstreams(t *testing.T) {
consulNamespacesEnabled: true,
consulPartitionsEnabled: true,
},
{
name: "single upstream with partition",
pod: func() *corev1.Pod {
pod1 := createPod("pod1", "1.2.3.4", true, true)
pod1.Annotations[annotationUpstreams] = "upstream.default.bar:1234"
return pod1
},
expected: []api.Upstream{
{
DestinationType: api.UpstreamDestTypeService,
DestinationName: "upstream",
LocalBindPort: 1234,
DestinationNamespace: "default",
DestinationPartition: "bar",
},
},
consulNamespacesEnabled: false,
consulPartitionsEnabled: true,
},
{
name: "multiple upstreams",
pod: func() *corev1.Pod {
Expand Down
2 changes: 1 addition & 1 deletion control-plane/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ require (
github.com/google/go-cmp v0.5.6
github.com/google/go-querystring v1.0.0 // indirect
github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510
github.com/hashicorp/consul/api v1.10.1-0.20210913215352-5b658d2f392d
github.com/hashicorp/consul/api v1.10.1-0.20210915232521-e0a7900f52bf
github.com/hashicorp/consul/sdk v0.8.0
github.com/hashicorp/go-cleanhttp v0.5.2 // indirect
github.com/hashicorp/go-discover v0.0.0-20200812215701-c4b85f6ed31f
Expand Down
2 changes: 2 additions & 0 deletions control-plane/go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -286,6 +286,8 @@ github.com/grpc-ecosystem/grpc-gateway v1.9.5/go.mod h1:vNeuVxBJEsws4ogUvrchl83t
github.com/hashicorp/consul/api v1.1.0/go.mod h1:VmuI/Lkw1nC05EYQWNKwWGbkg+FbDBtguAZLlVdkD9Q=
github.com/hashicorp/consul/api v1.10.1-0.20210913215352-5b658d2f392d h1:IBMYvG34CbxQqM55tBk8aVtmIQxvcczI0BqyxmbQDBs=
github.com/hashicorp/consul/api v1.10.1-0.20210913215352-5b658d2f392d/go.mod h1:sDjTOq0yUyv5G4h+BqSea7Fn6BU+XbolEz1952UB+mk=
github.com/hashicorp/consul/api v1.10.1-0.20210915232521-e0a7900f52bf h1:fouyN8SkrE4py09XaOru4PCM9zunem39CjOrMJMrKsc=
github.com/hashicorp/consul/api v1.10.1-0.20210915232521-e0a7900f52bf/go.mod h1:sDjTOq0yUyv5G4h+BqSea7Fn6BU+XbolEz1952UB+mk=
github.com/hashicorp/consul/sdk v0.1.1/go.mod h1:VKf9jXwCTEY1QZP2MOLRhb5i/I/ssyNV1vwHyQBF0x8=
github.com/hashicorp/consul/sdk v0.7.0/go.mod h1:fY08Y9z5SvJqevyZNy6WWPXiG3KwBPAvlcdx16zZ0fM=
github.com/hashicorp/consul/sdk v0.8.0 h1:OJtKBtEjboEZvG6AOUdh4Z1Zbyu0WcxQ0qatRrZHTVU=
Expand Down
3 changes: 0 additions & 3 deletions control-plane/subcommand/connect-init/command.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ type Command struct {
UI cli.Ui

flagACLAuthMethod string // Auth Method to use for ACLs, if enabled.
flagPartition string // Admin Partition name. Consul Enterprise 1.11+ feature.
flagPodName string // Pod name.
flagPodNamespace string // Pod namespace.
flagAuthMethodNamespace string // Consul namespace the auth-method is defined in.
Expand All @@ -58,7 +57,6 @@ type Command struct {
func (c *Command) init() {
c.flagSet = flag.NewFlagSet("", flag.ContinueOnError)
c.flagSet.StringVar(&c.flagACLAuthMethod, "acl-auth-method", "", "Name of the auth method to login to.")
c.flagSet.StringVar(&c.flagPartition, "partition", "", "Name of the Admin Partition of deployment.")
c.flagSet.StringVar(&c.flagPodName, "pod-name", "", "Name of the pod.")
c.flagSet.StringVar(&c.flagPodNamespace, "pod-namespace", "", "Name of the pod namespace.")
c.flagSet.StringVar(&c.flagAuthMethodNamespace, "auth-method-namespace", "", "Consul namespace the auth-method is defined in")
Expand Down Expand Up @@ -120,7 +118,6 @@ func (c *Command) Run(args []string) int {
}
}
cfg := api.DefaultConfig()
cfg.Partition = c.flagPartition
cfg.Namespace = c.flagConsulServiceNamespace
c.http.MergeOntoConfig(cfg)
consulClient, err := consul.NewClient(cfg)
Expand Down
1 change: 0 additions & 1 deletion control-plane/subcommand/connect-init/command_ent_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,6 @@ func TestRun_ServicePollingWithACLsAndTLSWithNamespaces(t *testing.T) {
tokenSinkFile: tokenFile,
proxyIDFile: proxyFile,
serviceRegistrationPollingAttempts: 5,
flagPartition: c.adminPartition,
}
// We build the http-addr because normally it's defined by the init container setting
// CONSUL_HTTP_ADDR when it processes the command template.
Expand Down
3 changes: 0 additions & 3 deletions control-plane/subcommand/controller/command.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ type Command struct {
flagEnableLeaderElection bool
flagEnableWebhooks bool
flagDatacenter string
flagPartition string
flagLogLevel string
flagLogJSON bool

Expand Down Expand Up @@ -65,8 +64,6 @@ func (c *Command) init() {
"Enabling this will ensure there is only one active controller manager.")
c.flagSet.StringVar(&c.flagDatacenter, "datacenter", "",
"Name of the Consul datacenter the controller is operating in. This is added as metadata on managed custom resources.")
c.flagSet.StringVar(&c.flagPartition, "partition", "",
"Name of the Consul Admin Partition the controller is operating in. The config entries are created in this partition.")
c.flagSet.BoolVar(&c.flagEnableNamespaces, "enable-namespaces", false,
"[Enterprise Only] Enables Consul Enterprise namespaces, in either a single Consul namespace or mirrored.")
c.flagSet.StringVar(&c.flagConsulDestinationNamespace, "consul-destination-namespace", "default",
Expand Down
6 changes: 5 additions & 1 deletion control-plane/subcommand/flags/http.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ func (f *HTTPFlags) Flags() *flag.FlagSet {
"The server name to use as the SNI host when connecting via TLS. This "+
"can also be specified via the CONSUL_TLS_SERVER_NAME environment variable.")
fs.Var(&f.partition, "partition",
"[Enterprise Only] Name of the Consul Admin Partition the controller is operating in. The config entries are created in this partition.")
"[Enterprise Only] Name of the Consul Admin Partition to query. Default to \"default\" if Admin Partitions are enabled.")
return fs
}

Expand Down Expand Up @@ -96,6 +96,10 @@ func (f *HTTPFlags) ReadTokenFile() (string, error) {
return strings.TrimSpace(string(data)), nil
}

func (f *HTTPFlags) Partition() string {
return f.partition.String()
}

func (f *HTTPFlags) APIClient() (*api.Client, error) {
c := api.DefaultConfig()

Expand Down
4 changes: 4 additions & 0 deletions control-plane/subcommand/flags/usage_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,10 @@ HTTP API Options
can also be set to HTTPS by setting the environment variable
CONSUL_HTTP_SSL=true.
-partition=<value>
[Enterprise Only] Name of the Consul Admin Partition to query.
Default to "default" if Admin Partitions are enabled.
-tls-server-name=<value>
The server name to use as the SNI host when connecting via
TLS. This can also be specified via the CONSUL_TLS_SERVER_NAME
Expand Down
Loading

0 comments on commit 683642d

Please sign in to comment.