-
Notifications
You must be signed in to change notification settings - Fork 321
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Prevent extra-config from being loaded twice (and erroring for segmen…
…t config) on clients and servers (#3337) * wip: testing with server works when you add segments as extraValues. Todos: * make similar changes to clients * potentially upgrade test? * consider locality having its own volume, rather than 2 volumes with extra in them * move extra-config out of /consul/config so it does not get applied twice * add comments about use of additional config maps * remove temporary inclusion of values.yaml in root that was used for hand off * get rid of temporary config.file * add segments test * test using 3 servers in a single cluster * add changelog * fix linting issues. * add comment to test. remove extra lines from config map. * fix bats tests --------- Co-authored-by: Nitya Dhanushkodi <nitya@hashicorp.com>
- Loading branch information
1 parent
e2dc674
commit b81182f
Showing
16 changed files
with
363 additions
and
38 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| ```release-note:bug-fix | ||
| mesh: prevent extra-config from being loaded twice (and erroring for segment config) on clients and servers. | ||
| ``` |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,18 @@ | ||
| // Copyright (c) HashiCorp, Inc. | ||
| // SPDX-License-Identifier: MPL-2.0 | ||
|
|
||
| package segments | ||
|
|
||
| import ( | ||
| "os" | ||
| "testing" | ||
|
|
||
| testsuite "github.com/hashicorp/consul-k8s/acceptance/framework/suite" | ||
| ) | ||
|
|
||
| var suite testsuite.Suite | ||
|
|
||
| func TestMain(m *testing.M) { | ||
| suite = testsuite.NewSuite(m) | ||
| os.Exit(suite.Run()) | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,187 @@ | ||
| // Copyright (c) HashiCorp, Inc. | ||
| // SPDX-License-Identifier: MPL-2.0 | ||
|
|
||
| package segments | ||
|
|
||
| import ( | ||
| "context" | ||
| "testing" | ||
|
|
||
| "github.com/stretchr/testify/require" | ||
| metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" | ||
|
|
||
| "github.com/hashicorp/consul-k8s/acceptance/framework/connhelper" | ||
| "github.com/hashicorp/consul-k8s/acceptance/framework/consul" | ||
| "github.com/hashicorp/consul-k8s/acceptance/framework/helpers" | ||
| "github.com/hashicorp/consul-k8s/acceptance/framework/k8s" | ||
| "github.com/hashicorp/consul-k8s/acceptance/framework/logger" | ||
| ) | ||
|
|
||
| // TestSegments_MeshWithAgentfulClients is a simple test that verifies that | ||
| // the Consul service mesh can be configured to use segments with: | ||
| // - one cluster with an alpha segment configured on the servers. | ||
| // - clients enabled and joining the alpha segment. | ||
| // - static client can communicate with static server. | ||
| func TestSegments_MeshWithAgentfulClients(t *testing.T) { | ||
| cases := map[string]struct { | ||
| secure bool | ||
| }{ | ||
| "not-secure": {secure: false}, | ||
| "secure": {secure: true}, | ||
| } | ||
|
|
||
| for name, c := range cases { | ||
| t.Run(name, func(t *testing.T) { | ||
| cfg := suite.Config() | ||
| if !cfg.EnableEnterprise { | ||
| t.Skipf("skipping this test because -enable-enterprise is not set") | ||
| } | ||
| ctx := suite.Environment().DefaultContext(t) | ||
|
|
||
| releaseName := helpers.RandomName() | ||
|
|
||
| helmValues := map[string]string{ | ||
| "connectInject.enabled": "true", | ||
|
|
||
| "server.replicas": "3", | ||
| "server.extraConfig": `"{\"segments\": [{\"name\":\"alpha1\"\,\"bind\":\"0.0.0.0\"\,\"port\":8303}]}"`, | ||
|
|
||
| "client.enabled": "true", | ||
| // need to configure clients to connect to port 8303 that the alpha segment was configured on rather than | ||
| // the standard serf LAN port. | ||
| "client.join[0]": "${CONSUL_FULLNAME}-server-0.${CONSUL_FULLNAME}-server.${NAMESPACE}.svc:8303", | ||
| "client.join[1]": "${CONSUL_FULLNAME}-server-1.${CONSUL_FULLNAME}-server.${NAMESPACE}.svc:8303", | ||
| "client.join[2]": "${CONSUL_FULLNAME}-server-2.${CONSUL_FULLNAME}-server.${NAMESPACE}.svc:8303", | ||
| "client.extraConfig": `"{\"segment\": \"alpha1\"}"`, | ||
| } | ||
|
|
||
| connHelper := connhelper.ConnectHelper{ | ||
| ClusterKind: consul.Helm, | ||
| Secure: c.secure, | ||
| ReleaseName: releaseName, | ||
| Ctx: ctx, | ||
| UseAppNamespace: cfg.EnableRestrictedPSAEnforcement, | ||
| Cfg: cfg, | ||
| HelmValues: helmValues, | ||
| } | ||
|
|
||
| connHelper.Setup(t) | ||
|
|
||
| connHelper.Install(t) | ||
| connHelper.DeployClientAndServer(t) | ||
| if c.secure { | ||
| connHelper.TestConnectionFailureWithoutIntention(t, connhelper.ConnHelperOpts{}) | ||
| connHelper.CreateIntention(t, connhelper.IntentionOpts{}) | ||
| } | ||
|
|
||
| connHelper.TestConnectionSuccess(t, connhelper.ConnHelperOpts{}) | ||
| connHelper.TestConnectionFailureWhenUnhealthy(t) | ||
| }) | ||
| } | ||
| } | ||
|
|
||
| // TestSegments_MeshWithAgentfulClientsMultiCluster is a simple test that verifies that | ||
| // the Consul service mesh can be configured to use segments with: | ||
| // - one cluster with an alpha segment configured on the servers. | ||
| // - clients enabled on another cluster and joining the alpha segment. | ||
| // - static client can communicate with static server. | ||
| func TestSegments_MeshWithAgentfulClientsMultiCluster(t *testing.T) { | ||
| cases := map[string]struct { | ||
| secure bool | ||
| }{ | ||
| "not-secure": {secure: false}, | ||
| "secure": {secure: true}, | ||
| } | ||
|
|
||
| for name, c := range cases { | ||
| t.Run(name, func(t *testing.T) { | ||
| cfg := suite.Config() | ||
| if !cfg.EnableEnterprise { | ||
| t.Skipf("skipping this test because -enable-enterprise is not set") | ||
| } | ||
| releaseName := helpers.RandomName() | ||
|
|
||
| // deploy server cluster | ||
| serverClusterContext := suite.Environment().DefaultContext(t) | ||
| serverClusterHelmValues := map[string]string{ | ||
| "connectInject.enabled": "true", | ||
|
|
||
| "server.replicas": "3", | ||
| "server.extraConfig": `"{\"segments\": [{\"name\":\"alpha1\"\,\"bind\":\"0.0.0.0\"\,\"port\":8303}]}"`, | ||
|
|
||
| "client.enabled": "true", | ||
| "client.join[0]": "${CONSUL_FULLNAME}-server-0.${CONSUL_FULLNAME}-server.${NAMESPACE}.svc:8303", | ||
| "client.join[1]": "${CONSUL_FULLNAME}-server-1.${CONSUL_FULLNAME}-server.${NAMESPACE}.svc:8303", | ||
| "client.join[2]": "${CONSUL_FULLNAME}-server-2.${CONSUL_FULLNAME}-server.${NAMESPACE}.svc:8303", | ||
| "client.extraConfig": `"{\"segment\": \"alpha1\"}"`, | ||
| } | ||
|
|
||
| serverConnHelper := connhelper.ConnectHelper{ | ||
| ClusterKind: consul.Helm, | ||
| Secure: c.secure, | ||
| ReleaseName: releaseName, | ||
| Ctx: serverClusterContext, | ||
| UseAppNamespace: cfg.EnableRestrictedPSAEnforcement, | ||
| Cfg: cfg, | ||
| HelmValues: serverClusterHelmValues, | ||
| } | ||
|
|
||
| serverConnHelper.Setup(t) | ||
| serverConnHelper.Install(t) | ||
| serverConnHelper.DeployServer(t) | ||
|
|
||
| // deploy client cluster | ||
| clientClusterContext := suite.Environment().Context(t, 1) | ||
| clientClusterHelmValues := map[string]string{ | ||
| "connectInject.enabled": "true", | ||
|
|
||
| "server.enabled": "false", | ||
|
|
||
| "client.enabled": "true", | ||
| "client.join[0]": "${CONSUL_FULLNAME}-server-0.${CONSUL_FULLNAME}-server.${NAMESPACE}.svc:8303", | ||
| "client.join[1]": "${CONSUL_FULLNAME}-server-1.${CONSUL_FULLNAME}-server.${NAMESPACE}.svc:8303", | ||
| "client.join[2]": "${CONSUL_FULLNAME}-server-2.${CONSUL_FULLNAME}-server.${NAMESPACE}.svc:8303", | ||
| "client.extraConfig": `"{\"segment\": \"alpha1\"}"`, | ||
| } | ||
|
|
||
| clientClusterConnHelper := connhelper.ConnectHelper{ | ||
| ClusterKind: consul.Helm, | ||
| Secure: c.secure, | ||
| ReleaseName: releaseName, | ||
| Ctx: clientClusterContext, | ||
| UseAppNamespace: cfg.EnableRestrictedPSAEnforcement, | ||
| Cfg: cfg, | ||
| HelmValues: clientClusterHelmValues, | ||
| } | ||
|
|
||
| clientClusterConnHelper.Setup(t) | ||
| clientClusterConnHelper.Install(t) | ||
| logger.Log(t, "creating static-client deployments in client cluster") | ||
| opts := clientClusterConnHelper.KubectlOptsForApp(t) | ||
|
|
||
| if cfg.EnableTransparentProxy { | ||
| k8s.DeployKustomize(t, opts, cfg.NoCleanupOnFailure, cfg.NoCleanup, cfg.DebugDirectory, "../fixtures/cases/static-client-tproxy") | ||
| } else { | ||
| k8s.DeployKustomize(t, opts, cfg.NoCleanupOnFailure, cfg.NoCleanup, cfg.DebugDirectory, "../fixtures/cases/static-client-inject") | ||
| } | ||
|
|
||
| // Check that the static-client has been injected and now have 2 containers in client cluster. | ||
| for _, labelSelector := range []string{"app=static-client"} { | ||
| podList, err := clientClusterContext.KubernetesClient(t).CoreV1().Pods(metav1.NamespaceAll).List(context.Background(), metav1.ListOptions{ | ||
| LabelSelector: labelSelector, | ||
| }) | ||
| require.NoError(t, err) | ||
| require.Len(t, podList.Items, 1) | ||
| require.Len(t, podList.Items[0].Spec.Containers, 2) | ||
| } | ||
|
|
||
| //if c.secure { | ||
| // connHelper.TestConnectionFailureWithoutIntention(t, connhelper.ConnHelperOpts{}) | ||
| // connHelper.CreateIntention(t, connhelper.IntentionOpts{}) | ||
| //} | ||
| // | ||
| //connHelper.TestConnectionSuccess(t, connhelper.ConnHelperOpts{}) | ||
| //connHelper.TestConnectionFailureWhenUnhealthy(t) | ||
| }) | ||
| } | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
21 changes: 21 additions & 0 deletions
21
charts/consul/templates/client-tmp-extra-config-configmap.yaml
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,21 @@ | ||
| {{- if (or (and (ne (.Values.client.enabled | toString) "-") .Values.client.enabled) (and (eq (.Values.client.enabled | toString) "-") .Values.global.enabled)) }} | ||
| # ConfigMap that is used as a temporary landing spot so that the container command | ||
| # in the client-daemonset where it needs to be transformed. ConfigMaps create | ||
| # read only volumes so it needs to be copied and transformed to the extra-config | ||
| # emptyDir volume where all final extra cofngi lives for use in consul. (locality-init | ||
| # also writes to extra-config volume.) | ||
| apiVersion: v1 | ||
| kind: ConfigMap | ||
| metadata: | ||
| name: {{ template "consul.fullname" . }}-client-tmp-extra-config | ||
| namespace: {{ .Release.Namespace }} | ||
| labels: | ||
| app: {{ template "consul.name" . }} | ||
| chart: {{ template "consul.chart" . }} | ||
| heritage: {{ .Release.Service }} | ||
| release: {{ .Release.Name }} | ||
| component: client | ||
| data: | ||
| extra-from-values.json: |- | ||
| {{ tpl .Values.client.extraConfig . | trimAll "\"" | indent 4 }} | ||
| {{- end }} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
21 changes: 21 additions & 0 deletions
21
charts/consul/templates/server-tmp-extra-config-configmap.yaml
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,21 @@ | ||
| {{- if (or (and (ne (.Values.server.enabled | toString) "-") .Values.server.enabled) (and (eq (.Values.server.enabled | toString) "-") .Values.global.enabled)) }} | ||
| # ConfigMap that is used as a temporary landing spot so that the container command | ||
| # in the server-stateful set where it needs to be transformed. ConfigMaps create | ||
| # read only volumes so it needs to be copied and transformed to the extra-config | ||
| # emptyDir volume where all final extra cofngi lives for use in consul. (locality-init | ||
| # also writes to extra-config volume.) | ||
| apiVersion: v1 | ||
| kind: ConfigMap | ||
| metadata: | ||
| name: {{ template "consul.fullname" . }}-server-tmp-extra-config | ||
| namespace: {{ .Release.Namespace }} | ||
| labels: | ||
| app: {{ template "consul.name" . }} | ||
| chart: {{ template "consul.chart" . }} | ||
| heritage: {{ .Release.Service }} | ||
| release: {{ .Release.Name }} | ||
| component: server | ||
| data: | ||
| extra-from-values.json: |- | ||
| {{ tpl .Values.server.extraConfig . | trimAll "\"" | indent 4 }} | ||
| {{- end }} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.