From 2cc3fe323aebca351580a2c6440ed3ad69db56af Mon Sep 17 00:00:00 2001 From: jm96441n Date: Tue, 1 Oct 2024 13:23:08 -0400 Subject: [PATCH 01/10] purge services on disable of sync catalog --- .../templates/sync-catalog-deployment.yaml | 4 +- charts/consul/values.yaml | 4 ++ .../subcommand/sync-catalog/command.go | 71 +++---------------- 3 files changed, 16 insertions(+), 63 deletions(-) diff --git a/charts/consul/templates/sync-catalog-deployment.yaml b/charts/consul/templates/sync-catalog-deployment.yaml index 94260b5e44..8a54dd83b0 100644 --- a/charts/consul/templates/sync-catalog-deployment.yaml +++ b/charts/consul/templates/sync-catalog-deployment.yaml @@ -1,4 +1,5 @@ -{{- if (or (and (ne (.Values.syncCatalog.enabled | toString) "-") .Values.syncCatalog.enabled) (and (eq (.Values.syncCatalog.enabled | toString) "-") .Values.global.enabled)) }} +{{ - $syncCatalogEnabled := (or (and (ne (.Values.syncCatalog.enabled | toString) "-") .Values.syncCatalog.enabled) (and (eq (.Values.syncCatalog.enabled | toString) "-") .Values.global.enabled)) }} +{{- if (or $syncCatalogEnabled .Values.syncCatalog.purgeServicesOnDisable) }} {{- template "consul.reservedNamesFailer" (list .Values.syncCatalog.consulNamespaces.consulDestinationNamespace "syncCatalog.consulNamespaces.consulDestinationNamespace") }} {{ template "consul.validateRequiredCloudSecretsExist" . }} {{ template "consul.validateCloudSecretKeys" . }} @@ -213,6 +214,7 @@ spec: -metrics-port={{ .Values.syncCatalog.metrics.port }} \ {{- end }} -prometheus-retention-time={{ .Values.global.metrics.agentMetricsRetentionTime }} \ + -purgeServicesOnDisable={{ and (not $syncCatalogEnabled) .Values.syncCatalog.purgeServicesOnDisable }} \ livenessProbe: httpGet: path: /health/ready diff --git a/charts/consul/values.yaml b/charts/consul/values.yaml index 4a6aa20060..66115bad7a 100644 --- a/charts/consul/values.yaml +++ b/charts/consul/values.yaml @@ -2090,6 +2090,10 @@ syncCatalog: # global.enabled. enabled: false + # True if you want to deregister all services in this cluster from consul that have been registered by the catalog sync when the `enabled` flag is set to false. + # @type: boolean + purgeServicesOnDisable: false + # The name of the Docker image (including any tag) for consul-k8s-control-plane # to run the sync program. # @type: string diff --git a/control-plane/subcommand/sync-catalog/command.go b/control-plane/subcommand/sync-catalog/command.go index c3965165ed..2ab7247da2 100644 --- a/control-plane/subcommand/sync-catalog/command.go +++ b/control-plane/subcommand/sync-catalog/command.go @@ -12,20 +12,17 @@ import ( "os" "os/signal" "regexp" - "strings" "sync" "syscall" "time" "github.com/armon/go-metrics/prometheus" - "github.com/cenkalti/backoff" mapset "github.com/deckarep/golang-set" "github.com/hashicorp/consul-server-connection-manager/discovery" "github.com/hashicorp/consul/api" "github.com/hashicorp/go-hclog" "github.com/mitchellh/cli" "github.com/prometheus/client_golang/prometheus/promhttp" - "golang.org/x/sync/errgroup" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/kubernetes" _ "k8s.io/client-go/plugin/pkg/client/auth/gcp" @@ -37,7 +34,6 @@ import ( "github.com/hashicorp/consul-k8s/control-plane/helper/controller" "github.com/hashicorp/consul-k8s/control-plane/subcommand" "github.com/hashicorp/consul-k8s/control-plane/subcommand/common" - metricsutil "github.com/hashicorp/consul-k8s/control-plane/subcommand/common" "github.com/hashicorp/consul-k8s/control-plane/subcommand/flags" ) @@ -67,7 +63,7 @@ type Command struct { flagAddK8SNamespaceSuffix bool flagLogLevel string flagLogJSON bool - flagPurgeK8SServicesFromNode string + flagPurgeK8SServicesFromNode bool flagFilter string // Flags to support namespaces @@ -156,8 +152,8 @@ func (c *Command) init() { "\"debug\", \"info\", \"warn\", and \"error\".") c.flags.BoolVar(&c.flagLogJSON, "log-json", false, "Enable or disable JSON output format for logging.") - c.flags.StringVar(&c.flagPurgeK8SServicesFromNode, "purge-k8s-services-from-node", "", - "Specifies the name of the Consul node for which to deregister synced Kubernetes services.") + c.flags.BoolVar(&c.flagPurgeK8SServicesFromNode, "purge-k8s-services-from-node", false, + "Specifies if services should be purged from the Consul node. If set, all K8S services will be removed from the Consul node.") c.flags.StringVar(&c.flagFilter, "filter", "", "Specifies the expression used to filter the services on the Consul node that will be deregistered. "+ "The syntax for this filter is the same as the syntax used in the List Services for Node API in the Consul catalog.") @@ -462,64 +458,15 @@ func (c *Command) Run(args []string) int { // remove all k8s services from Consul. func (c *Command) removeAllK8SServicesFromConsulNode(consulClient *api.Client) error { - node, _, err := consulClient.Catalog().NodeServiceList(c.flagPurgeK8SServicesFromNode, &api.QueryOptions{Filter: c.flagFilter}) + _, err := consulClient.Catalog().Deregister(&api.CatalogDeregistration{ + Node: c.flagConsulNodeName, + Partition: c.consul.Partition, + }, nil) if err != nil { + c.UI.Error(fmt.Sprintf("unable to deregister all K8S services from Consul: %s", err)) return err } - var firstErr error - services := node.Services - batchSize := 300 - maxRetries := 2 - retryDelay := 200 * time.Millisecond - - // Ask for user confirmation before purging services - for { - c.UI.Info(fmt.Sprintf("Are you sure you want to delete %v K8S services from %v? (y/n): ", len(services), c.flagPurgeK8SServicesFromNode)) - var input string - fmt.Scanln(&input) - if input = strings.ToLower(input); input == "y" { - break - } else if input == "n" { - return nil - } else { - c.UI.Info("Invalid input. Please enter 'y' or 'n'.") - } - } - - for i := 0; i < len(services); i += batchSize { - end := i + batchSize - if end > len(services) { - end = len(services) - } - - var eg errgroup.Group - for _, service := range services[i:end] { - s := service - eg.Go(func() error { - var b backoff.BackOff = backoff.NewConstantBackOff(retryDelay) - b = backoff.WithMaxRetries(b, uint64(maxRetries)) - return backoff.Retry(func() error { - _, err := consulClient.Catalog().Deregister(&api.CatalogDeregistration{ - Node: c.flagPurgeK8SServicesFromNode, - ServiceID: s.ID, - }, nil) - return err - }, b) - }) - } - if err := eg.Wait(); err != nil { - if firstErr == nil { - c.UI.Info("Some K8S services were not deregistered from Consul") - firstErr = err - } - } - c.UI.Info(fmt.Sprintf("Processed %v K8S services from %v", end-i, c.flagPurgeK8SServicesFromNode)) - } - - if firstErr != nil { - return firstErr - } c.UI.Info("All K8S services were deregistered from Consul") return nil } @@ -570,7 +517,7 @@ func (c *Command) validateFlags() error { } if c.flagMetricsPort != "" { - if _, valid := metricsutil.ParseScrapePort(c.flagMetricsPort); !valid { + if _, valid := common.ParseScrapePort(c.flagMetricsPort); !valid { return errors.New("-metrics-port must be a valid unprivileged port number") } } From ca3df7cc345520f7e95123b1814f047fc584100a Mon Sep 17 00:00:00 2001 From: jm96441n Date: Tue, 1 Oct 2024 13:37:08 -0400 Subject: [PATCH 02/10] gofumpt --- control-plane/subcommand/sync-catalog/command.go | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/control-plane/subcommand/sync-catalog/command.go b/control-plane/subcommand/sync-catalog/command.go index 2ab7247da2..ba664645e7 100644 --- a/control-plane/subcommand/sync-catalog/command.go +++ b/control-plane/subcommand/sync-catalog/command.go @@ -500,7 +500,7 @@ func (c *Command) validateFlags() error { // For the Consul node name to be discoverable via DNS, it must contain only // dashes and alphanumeric characters. Length is also constrained. // These restrictions match those defined in Consul's agent definition. - var invalidDnsRe = regexp.MustCompile(`[^A-Za-z0-9\\-]+`) + invalidDnsRe := regexp.MustCompile(`[^A-Za-z0-9\\-]+`) const maxDNSLabelLength = 63 if invalidDnsRe.MatchString(c.flagConsulNodeName) { @@ -533,7 +533,7 @@ func (c *Command) recordMetrics() (*prometheus.PrometheusSink, error) { return &prometheus.PrometheusSink{}, err } - var counters = [][]prometheus.CounterDefinition{ + counters := [][]prometheus.CounterDefinition{ catalogtoconsul.SyncToConsulCounters, catalogtok8s.SyncToK8sCounters, } @@ -568,8 +568,9 @@ func (c *Command) authorizeMiddleware() func(http.Handler) http.Handler { } } -const synopsis = "Sync Kubernetes services and Consul services." -const help = ` +const ( + synopsis = "Sync Kubernetes services and Consul services." + help = ` Usage: consul-k8s-control-plane sync-catalog [options] Sync K8S pods, services, and more with the Consul service catalog. @@ -578,3 +579,4 @@ Usage: consul-k8s-control-plane sync-catalog [options] K8S services. ` +) From 0ecfe3538c87ece60025f58f734c525c1a0f5b05 Mon Sep 17 00:00:00 2001 From: jm96441n Date: Tue, 1 Oct 2024 14:37:23 -0400 Subject: [PATCH 03/10] update tests for sync catalog --- .../subcommand/sync-catalog/command.go | 2 +- .../subcommand/sync-catalog/command_test.go | 271 +++++++++--------- 2 files changed, 131 insertions(+), 142 deletions(-) diff --git a/control-plane/subcommand/sync-catalog/command.go b/control-plane/subcommand/sync-catalog/command.go index ba664645e7..36bb996f2f 100644 --- a/control-plane/subcommand/sync-catalog/command.go +++ b/control-plane/subcommand/sync-catalog/command.go @@ -275,7 +275,7 @@ func (c *Command) Run(args []string) int { } c.ready = true - if c.flagPurgeK8SServicesFromNode != "" { + if c.flagPurgeK8SServicesFromNode { consulClient, err := consul.NewClientFromConnMgr(consulConfig, c.connMgr) if err != nil { c.UI.Error(fmt.Sprintf("unable to instantiate consul client: %s", err)) diff --git a/control-plane/subcommand/sync-catalog/command_test.go b/control-plane/subcommand/sync-catalog/command_test.go index b6aa63a903..94f5452ad9 100644 --- a/control-plane/subcommand/sync-catalog/command_test.go +++ b/control-plane/subcommand/sync-catalog/command_test.go @@ -560,160 +560,149 @@ func TestRun_ToConsulChangingFlags(t *testing.T) { // Test services could be de-registered from Consul. func TestRemoveAllK8SServicesFromConsul(t *testing.T) { - t.Parallel() - - k8s, testClient := completeSetup(t) - consulClient := testClient.APIClient - - // Create a mock reader to simulate user input - input := "y\n" - reader, writer, err := os.Pipe() - require.NoError(t, err) - oldStdin := os.Stdin - os.Stdin = reader - defer func() { os.Stdin = oldStdin }() - - // Write the simulated user input to the mock reader - go func() { - defer writer.Close() - _, err := writer.WriteString(input) - require.NoError(t, err) - }() - - // Run the command. - ui := cli.NewMockUi() - cmd := Command{ - UI: ui, - clientset: k8s, - logger: hclog.New(&hclog.LoggerOptions{ - Name: t.Name(), - Level: hclog.Debug, - }), - flagAllowK8sNamespacesList: []string{"*"}, - connMgr: testClient.Watcher, + testCases := map[string]struct { + nodeName string + partitionName string + }{ + "default Node name in default partition": { + nodeName: "k8s-sync", + partitionName: "default", + }, + "non-default Node name in default partition": { + nodeName: "custom-node", + partitionName: "default", + }, + "default Node name in non-default partition": { + nodeName: "k8s-sync", + partitionName: "part-1", + }, + "non-default Node name in non-default partition": { + nodeName: "custom-node", + partitionName: "part-1", + }, } - // create two services in k8s - _, err = k8s.CoreV1().Services("bar").Create(context.Background(), lbService("foo", "1.1.1.1"), metav1.CreateOptions{}) - require.NoError(t, err) + for name, tc := range testCases { + tc := tc + t.Run(name, func(t *testing.T) { + t.Parallel() + var err error - _, err = k8s.CoreV1().Services("baz").Create(context.Background(), lbService("foo", "2.2.2.2"), metav1.CreateOptions{}) - require.NoError(t, err) + k8s, testClient := completeSetup(t) + consulClient := testClient.APIClient - longRunningChan := runCommandAsynchronously(&cmd, []string{ - "-addresses", "127.0.0.1", - "-http-port", strconv.Itoa(testClient.Cfg.HTTPPort), - "-consul-write-interval", "100ms", - "-add-k8s-namespace-suffix", - }) - defer stopCommand(t, &cmd, longRunningChan) + // Create a mock reader to simulate user input + // Run the command. + ui := cli.NewMockUi() + cmd := Command{ + UI: ui, + clientset: k8s, + logger: hclog.New(&hclog.LoggerOptions{ + Name: t.Name(), + Level: hclog.Debug, + }), + flagAllowK8sNamespacesList: []string{"*"}, + connMgr: testClient.Watcher, + } - // check that the two K8s services have been synced into Consul - retry.Run(t, func(r *retry.R) { - svc, _, err := consulClient.Catalog().Service("foo-bar", "k8s", nil) - require.NoError(r, err) - require.Len(r, svc, 1) - require.Equal(r, "1.1.1.1", svc[0].ServiceAddress) - svc, _, err = consulClient.Catalog().Service("foo-baz", "k8s", nil) - require.NoError(r, err) - require.Len(r, svc, 1) - require.Equal(r, "2.2.2.2", svc[0].ServiceAddress) - }) + otherPartitionName := "the-other-one" + + // create another partition and register 2 services there to the same node to show that we won't delete them + _, _, err = consulClient.Partitions().Create(context.Background(), &api.Partition{Name: otherPartitionName}, nil) + _, err = consulClient.Catalog().Register( + &api.CatalogRegistration{ + Node: tc.nodeName, + Address: "5.5.5.5", + Service: &api.AgentService{ + ID: "other-service-1", + Service: "other-service-1", + Tags: []string{"other-k8s-cluster"}, + Meta: map[string]string{}, + Port: 0, + Address: "5.5.5.5", + Partition: otherPartitionName, + }, + Partition: otherPartitionName, + }, + &api.WriteOptions{Partition: otherPartitionName}, + ) + require.NoError(t, err) + + _, err = consulClient.Catalog().Register( + &api.CatalogRegistration{ + Node: tc.nodeName, + Address: "6.6.6.6", + Service: &api.AgentService{ + ID: "other-service-2", + Service: "other-service-2", + Tags: []string{"other-k8s-cluster"}, + Meta: map[string]string{}, + Port: 0, + Address: "6.6.6.6", + Partition: otherPartitionName, + }, + Partition: otherPartitionName, + }, + &api.WriteOptions{Partition: otherPartitionName}, + ) + require.NoError(t, err) + + // create partition if it does not exist + if tc.partitionName != "default" { + p, _, err := consulClient.Partitions().Create(context.Background(), &api.Partition{Name: tc.partitionName}, nil) + require.NoError(t, err) + require.NotNil(t, p) + } - exitChan := runCommandAsynchronously(&cmd, []string{ - "-addresses", "127.0.0.1", - "-http-port", strconv.Itoa(testClient.Cfg.HTTPPort), - "-purge-k8s-services-from-node=k8s-sync", - }) - stopCommand(t, &cmd, exitChan) + // create two services in k8s + _, err = k8s.CoreV1().Services("bar").Create(context.Background(), lbService("foo", "1.1.1.1"), metav1.CreateOptions{}) + require.NoError(t, err) - retry.Run(t, func(r *retry.R) { - serviceList, _, err := consulClient.Catalog().NodeServiceList("k8s-sync", &api.QueryOptions{AllowStale: false}) - require.NoError(r, err) - require.Len(r, serviceList.Services, 0) - }) -} + _, err = k8s.CoreV1().Services("baz").Create(context.Background(), lbService("foo", "2.2.2.2"), metav1.CreateOptions{}) + require.NoError(t, err) -// Test services could be de-registered from Consul with filter. -func TestRemoveAllK8SServicesFromConsulWithFilter(t *testing.T) { - t.Parallel() + longRunningChan := runCommandAsynchronously(&cmd, []string{ + "-addresses", "127.0.0.1", + "-http-port", strconv.Itoa(testClient.Cfg.HTTPPort), + "-consul-write-interval", "100ms", + "-partition", tc.partitionName, + "-consul-node-name", tc.nodeName, + "-add-k8s-namespace-suffix", + }) - k8s, testClient := completeSetup(t) - consulClient := testClient.APIClient + // check that the two K8s services have been synced into Consul + retry.Run(t, func(r *retry.R) { + svc, _, err := consulClient.Catalog().Service("foo-bar", "k8s", &api.QueryOptions{Partition: tc.partitionName}) + require.NoError(r, err) + require.Len(r, svc, 1) + require.Equal(r, "1.1.1.1", svc[0].ServiceAddress) + svc, _, err = consulClient.Catalog().Service("foo-baz", "k8s", &api.QueryOptions{Partition: tc.partitionName}) + require.NoError(r, err) + require.Len(r, svc, 1) + require.Equal(r, "2.2.2.2", svc[0].ServiceAddress) + }) - // Create a mock reader to simulate user input - input := "y\n" - reader, writer, err := os.Pipe() - require.NoError(t, err) - oldStdin := os.Stdin - os.Stdin = reader - defer func() { os.Stdin = oldStdin }() + defer stopCommand(t, &cmd, longRunningChan) - // Write the simulated user input to the mock reader - go func() { - defer writer.Close() - _, err := writer.WriteString(input) - require.NoError(t, err) - }() + exitChan := runCommandAsynchronously(&cmd, []string{ + "-addresses", "127.0.0.1", + "-http-port", strconv.Itoa(testClient.Cfg.HTTPPort), + "-purge-k8s-services-from-node=true", + "-partition", tc.partitionName, + "-consul-node-name", tc.nodeName, + }) + stopCommand(t, &cmd, exitChan) - // Run the command. - ui := cli.NewMockUi() - cmd := Command{ - UI: ui, - clientset: k8s, - logger: hclog.New(&hclog.LoggerOptions{ - Name: t.Name(), - Level: hclog.Debug, - }), - flagAllowK8sNamespacesList: []string{"*"}, - connMgr: testClient.Watcher, + retry.Run(t, func(r *retry.R) { + serviceList, _, err := consulClient.Catalog().NodeServiceList(tc.nodeName, &api.QueryOptions{AllowStale: false, Partition: tc.partitionName}) + require.NoError(r, err) + require.Len(r, serviceList.Services, 0) + otherPartitionServiceList, _, err := consulClient.Catalog().NodeServiceList(tc.nodeName, &api.QueryOptions{AllowStale: false, Partition: otherPartitionName}) + require.NoError(r, err) + require.Len(r, otherPartitionServiceList.Services, 2) + }) + }) } - - // create two services in k8s - _, err = k8s.CoreV1().Services("bar").Create(context.Background(), lbService("foo", "1.1.1.1"), metav1.CreateOptions{}) - require.NoError(t, err) - _, err = k8s.CoreV1().Services("baz").Create(context.Background(), lbService("foo", "2.2.2.2"), metav1.CreateOptions{}) - require.NoError(t, err) - _, err = k8s.CoreV1().Services("bat").Create(context.Background(), lbService("foo", "3.3.3.3"), metav1.CreateOptions{}) - require.NoError(t, err) - - longRunningChan := runCommandAsynchronously(&cmd, []string{ - "-addresses", "127.0.0.1", - "-http-port", strconv.Itoa(testClient.Cfg.HTTPPort), - "-consul-write-interval", "100ms", - "-add-k8s-namespace-suffix", - }) - defer stopCommand(t, &cmd, longRunningChan) - - // check that the name of the service is namespaced - retry.Run(t, func(r *retry.R) { - svc, _, err := consulClient.Catalog().Service("foo-bar", "k8s", nil) - require.NoError(r, err) - require.Len(r, svc, 1) - require.Equal(r, "1.1.1.1", svc[0].ServiceAddress) - svc, _, err = consulClient.Catalog().Service("foo-baz", "k8s", nil) - require.NoError(r, err) - require.Len(r, svc, 1) - require.Equal(r, "2.2.2.2", svc[0].ServiceAddress) - svc, _, err = consulClient.Catalog().Service("foo-bat", "k8s", nil) - require.NoError(r, err) - require.Len(r, svc, 1) - require.Equal(r, "3.3.3.3", svc[0].ServiceAddress) - }) - - exitChan := runCommandAsynchronously(&cmd, []string{ - "-addresses", "127.0.0.1", - "-http-port", strconv.Itoa(testClient.Cfg.HTTPPort), - "-purge-k8s-services-from-node=k8s-sync", - "-filter=baz in ID", - }) - stopCommand(t, &cmd, exitChan) - - retry.Run(t, func(r *retry.R) { - serviceList, _, err := consulClient.Catalog().NodeServiceList("k8s-sync", &api.QueryOptions{AllowStale: false}) - require.NoError(r, err) - require.Len(r, serviceList.Services, 2) - }) } // Set up test consul agent and fake kubernetes cluster client. From b6678602c757c3bce29e1cc6b56bcc876abe7684 Mon Sep 17 00:00:00 2001 From: jm96441n Date: Thu, 10 Oct 2024 10:37:33 -0400 Subject: [PATCH 04/10] Added changelog --- .changelog/4378.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 .changelog/4378.txt diff --git a/.changelog/4378.txt b/.changelog/4378.txt new file mode 100644 index 0000000000..30a4287816 --- /dev/null +++ b/.changelog/4378.txt @@ -0,0 +1,3 @@ +```release-note:enhancement +catalog-sync: Added field to helm chart to purge all services registered with catalog-sync from consul on disabling of catalog-sync. +``` From 0fd8e01ac7c1b51b28ea473b73cfc50460b4d5e7 Mon Sep 17 00:00:00 2001 From: jm96441n Date: Thu, 10 Oct 2024 16:55:27 -0400 Subject: [PATCH 05/10] create separate jobs to handle deregistering services when disabling sync catalog --- ...sync-catalog-cleanup-on-uninstall-job.yaml | 123 +++++++++++++++++ .../sync-catalog-cleanup-on-upgrade-job.yaml | 124 ++++++++++++++++++ ...ync-catalog-cleanup-podsecuritypolicy.yaml | 32 +++++ .../sync-catalog-cleanup-serviceaccount.yaml | 19 +++ .../templates/sync-catalog-deployment.yaml | 5 +- 5 files changed, 300 insertions(+), 3 deletions(-) create mode 100644 charts/consul/templates/sync-catalog-cleanup-on-uninstall-job.yaml create mode 100644 charts/consul/templates/sync-catalog-cleanup-on-upgrade-job.yaml create mode 100644 charts/consul/templates/sync-catalog-cleanup-podsecuritypolicy.yaml create mode 100644 charts/consul/templates/sync-catalog-cleanup-serviceaccount.yaml diff --git a/charts/consul/templates/sync-catalog-cleanup-on-uninstall-job.yaml b/charts/consul/templates/sync-catalog-cleanup-on-uninstall-job.yaml new file mode 100644 index 0000000000..0e4c6c3fff --- /dev/null +++ b/charts/consul/templates/sync-catalog-cleanup-on-uninstall-job.yaml @@ -0,0 +1,123 @@ +{{- if .Values.syncCatalog.purgeServicesOnDisable }} +apiVersion: batch/v1 +kind: Job +metadata: + name: {{ template "consul.fullname" . }}-sync-catalog-cleanup-on-uninstall + namespace: {{ .Release.Namespace }} + labels: + app: {{ template "consul.name" . }} + chart: {{ template "consul.chart" . }} + heritage: {{ .Release.Service }} + release: {{ .Release.Name }} + component: sync-catalog-cleanup + {{- if .Values.global.extraLabels }} + {{- toYaml .Values.global.extraLabels | nindent 4 }} + {{- end }} + annotations: + "helm.sh/hook": pre-delete + "helm.sh/hook-weight": "0" + "helm.sh/hook-delete-policy": hook-succeeded,hook-failed +spec: + template: + metadata: + name: {{ template "consul.fullname" . }}-sync-catalog-cleanup-on-uninstall + labels: + app: {{ template "consul.name" . }} + chart: {{ template "consul.chart" . }} + release: {{ .Release.Name }} + component: sync-catalog-cleanup + {{- if .Values.global.extraLabels }} + {{- toYaml .Values.global.extraLabels | nindent 8 }} + {{- end }} + annotations: + "consul.hashicorp.com/connect-inject": "false" + "consul.hashicorp.com/mesh-inject": "false" + spec: + restartPolicy: Never + serviceAccountName: {{ template "consul.fullname" . }}-sync-catalog-cleanup + containers: + - name: sync-catalog-cleanup-job + image: "{{ default .Values.global.imageK8S .Values.syncCatalog.image }}" + {{ template "consul.imagePullPolicy" . }} + {{- include "consul.restrictedSecurityContext" . | nindent 8 }} + env: + {{- include "consul.consulK8sConsulServerEnvVars" . | nindent 8 }} + {{- if .Values.global.acls.manageSystemACLs }} + - name: CONSUL_LOGIN_AUTH_METHOD + {{- if and .Values.global.federation.enabled .Values.global.federation.primaryDatacenter .Values.global.enableConsulNamespaces }} + value: {{ template "consul.fullname" . }}-k8s-component-auth-method-{{ .Values.global.datacenter }} + {{- else }} + value: {{ template "consul.fullname" . }}-k8s-component-auth-method + {{- end }} + - name: CONSUL_LOGIN_DATACENTER + {{- if and .Values.global.federation.enabled .Values.global.federation.primaryDatacenter .Values.global.enableConsulNamespaces }} + value: {{ .Values.global.federation.primaryDatacenter }} + {{- else }} + value: {{ .Values.global.datacenter }} + {{- end }} + - name: CONSUL_LOGIN_META + value: "component=sync-catalog,pod=$(NAMESPACE)/$(POD_NAME)" + {{- end }} + - name: NAMESPACE + valueFrom: + fieldRef: + fieldPath: metadata.namespace + {{- if (and .Values.syncCatalog.aclSyncToken.secretName .Values.syncCatalog.aclSyncToken.secretKey) }} + - name: CONSUL_ACL_TOKEN + valueFrom: + secretKeyRef: + name: {{ .Values.syncCatalog.aclSyncToken.secretName }} + key: {{ .Values.syncCatalog.aclSyncToken.secretKey }} + {{- end }} + volumeMounts: + {{- if .Values.global.tls.enabled }} + {{- if not (or (and .Values.externalServers.enabled .Values.externalServers.useSystemRoots) .Values.global.secretsBackend.vault.enabled) }} + - name: consul-ca-cert + mountPath: /consul/tls/ca + readOnly: true + {{- end }} + {{- end }} + command: + - "/bin/sh" + - "-ec" + - | + exec consul-k8s-control-plane sync-catalog \ + -log-level={{ default .Values.global.logLevel .Values.syncCatalog.logLevel }} \ + -log-json={{ .Values.global.logJSON }} \ + -k8s-default-sync={{ .Values.syncCatalog.default }} \ + {{- if .Values.global.adminPartitions.enabled }} + -partition={{ .Values.global.adminPartitions.name }} \ + {{- end }} + {{- if .Values.syncCatalog.metrics.enabled | default .Values.global.metrics.enabled }} + -enable-metrics \ + {{- end }} + {{- if .Values.syncCatalog.metrics.path }} + -metrics-path={{ .Values.syncCatalog.metrics.path }} \ + {{- end }} + {{- if .Values.syncCatalog.metrics.port }} + -metrics-port={{ .Values.syncCatalog.metrics.port }} \ + {{- end }} + -prometheus-retention-time={{ .Values.global.metrics.agentMetricsRetentionTime }} \ + {{- if .Release.IsUpgrade }} + -purge-k8s-services-from-node + {{- end }} + {{- if .Values.global.acls.tolerations }} + tolerations: + {{ tpl .Values.global.acls.tolerations . | indent 8 | trim }} + {{- end }} + volumes: + {{- if .Values.global.tls.enabled }} + {{- if not (or (and .Values.externalServers.enabled .Values.externalServers.useSystemRoots) .Values.global.secretsBackend.vault.enabled) }} + - name: consul-ca-cert + secret: + {{- if .Values.global.tls.caCert.secretName }} + secretName: {{ .Values.global.tls.caCert.secretName }} + {{- else }} + secretName: {{ template "consul.fullname" . }}-ca-cert + {{- end }} + items: + - key: {{ default "tls.crt" .Values.global.tls.caCert.secretKey }} + path: tls.crt + {{- end }} + {{- end }} +{{- end }} diff --git a/charts/consul/templates/sync-catalog-cleanup-on-upgrade-job.yaml b/charts/consul/templates/sync-catalog-cleanup-on-upgrade-job.yaml new file mode 100644 index 0000000000..f9dfd1cccc --- /dev/null +++ b/charts/consul/templates/sync-catalog-cleanup-on-upgrade-job.yaml @@ -0,0 +1,124 @@ +{{- $syncCatalogEnabled := (or (and (ne (.Values.syncCatalog.enabled | toString) "-") .Values.syncCatalog.enabled) (and (eq (.Values.syncCatalog.enabled | toString) "-") .Values.global.enabled)) }} +{{- if and (not $syncCatalogEnabled) .Values.syncCatalog.purgeServicesOnDisable }} +apiVersion: batch/v1 +kind: Job +metadata: + name: {{ template "consul.fullname" . }}-sync-catalog-cleanup-on-upgrade + namespace: {{ .Release.Namespace }} + labels: + app: {{ template "consul.name" . }} + chart: {{ template "consul.chart" . }} + heritage: {{ .Release.Service }} + release: {{ .Release.Name }} + component: sync-catalog-cleanup + {{- if .Values.global.extraLabels }} + {{- toYaml .Values.global.extraLabels | nindent 4 }} + {{- end }} + annotations: + "helm.sh/hook": pre-upgrade + "helm.sh/hook-weight": "0" + "helm.sh/hook-delete-policy": hook-succeeded,hook-failed +spec: + template: + metadata: + name: {{ template "consul.fullname" . }}-sync-catalog-cleanup-on-upgrade + labels: + app: {{ template "consul.name" . }} + chart: {{ template "consul.chart" . }} + release: {{ .Release.Name }} + component: sync-catalog-cleanup + {{- if .Values.global.extraLabels }} + {{- toYaml .Values.global.extraLabels | nindent 8 }} + {{- end }} + annotations: + "consul.hashicorp.com/connect-inject": "false" + "consul.hashicorp.com/mesh-inject": "false" + spec: + restartPolicy: Never + serviceAccountName: {{ template "consul.fullname" . }}-sync-catalog-cleanup + containers: + - name: sync-catalog-cleanup-job + image: "{{ default .Values.global.imageK8S .Values.syncCatalog.image }}" + {{ template "consul.imagePullPolicy" . }} + {{- include "consul.restrictedSecurityContext" . | nindent 8 }} + env: + {{- include "consul.consulK8sConsulServerEnvVars" . | nindent 8 }} + {{- if .Values.global.acls.manageSystemACLs }} + - name: CONSUL_LOGIN_AUTH_METHOD + {{- if and .Values.global.federation.enabled .Values.global.federation.primaryDatacenter .Values.global.enableConsulNamespaces }} + value: {{ template "consul.fullname" . }}-k8s-component-auth-method-{{ .Values.global.datacenter }} + {{- else }} + value: {{ template "consul.fullname" . }}-k8s-component-auth-method + {{- end }} + - name: CONSUL_LOGIN_DATACENTER + {{- if and .Values.global.federation.enabled .Values.global.federation.primaryDatacenter .Values.global.enableConsulNamespaces }} + value: {{ .Values.global.federation.primaryDatacenter }} + {{- else }} + value: {{ .Values.global.datacenter }} + {{- end }} + - name: CONSUL_LOGIN_META + value: "component=sync-catalog,pod=$(NAMESPACE)/$(POD_NAME)" + {{- end }} + - name: NAMESPACE + valueFrom: + fieldRef: + fieldPath: metadata.namespace + {{- if (and .Values.syncCatalog.aclSyncToken.secretName .Values.syncCatalog.aclSyncToken.secretKey) }} + - name: CONSUL_ACL_TOKEN + valueFrom: + secretKeyRef: + name: {{ .Values.syncCatalog.aclSyncToken.secretName }} + key: {{ .Values.syncCatalog.aclSyncToken.secretKey }} + {{- end }} + volumeMounts: + {{- if .Values.global.tls.enabled }} + {{- if not (or (and .Values.externalServers.enabled .Values.externalServers.useSystemRoots) .Values.global.secretsBackend.vault.enabled) }} + - name: consul-ca-cert + mountPath: /consul/tls/ca + readOnly: true + {{- end }} + {{- end }} + command: + - "/bin/sh" + - "-ec" + - | + exec consul-k8s-control-plane sync-catalog \ + -log-level={{ default .Values.global.logLevel .Values.syncCatalog.logLevel }} \ + -log-json={{ .Values.global.logJSON }} \ + -k8s-default-sync={{ .Values.syncCatalog.default }} \ + {{- if .Values.global.adminPartitions.enabled }} + -partition={{ .Values.global.adminPartitions.name }} \ + {{- end }} + {{- if .Values.syncCatalog.metrics.enabled | default .Values.global.metrics.enabled }} + -enable-metrics \ + {{- end }} + {{- if .Values.syncCatalog.metrics.path }} + -metrics-path={{ .Values.syncCatalog.metrics.path }} \ + {{- end }} + {{- if .Values.syncCatalog.metrics.port }} + -metrics-port={{ .Values.syncCatalog.metrics.port }} \ + {{- end }} + -prometheus-retention-time={{ .Values.global.metrics.agentMetricsRetentionTime }} \ + {{- if .Release.IsUpgrade }} + -purge-k8s-services-from-node + {{- end }} + {{- if .Values.global.acls.tolerations }} + tolerations: + {{ tpl .Values.global.acls.tolerations . | indent 8 | trim }} + {{- end }} + volumes: + {{- if .Values.global.tls.enabled }} + {{- if not (or (and .Values.externalServers.enabled .Values.externalServers.useSystemRoots) .Values.global.secretsBackend.vault.enabled) }} + - name: consul-ca-cert + secret: + {{- if .Values.global.tls.caCert.secretName }} + secretName: {{ .Values.global.tls.caCert.secretName }} + {{- else }} + secretName: {{ template "consul.fullname" . }}-ca-cert + {{- end }} + items: + - key: {{ default "tls.crt" .Values.global.tls.caCert.secretKey }} + path: tls.crt + {{- end }} + {{- end }} +{{- end }} diff --git a/charts/consul/templates/sync-catalog-cleanup-podsecuritypolicy.yaml b/charts/consul/templates/sync-catalog-cleanup-podsecuritypolicy.yaml new file mode 100644 index 0000000000..c3c847b4ae --- /dev/null +++ b/charts/consul/templates/sync-catalog-cleanup-podsecuritypolicy.yaml @@ -0,0 +1,32 @@ +{{- if and .Values.syncCatalog.purgeServicesOnDisable .Values.global.enablePodSecurityPolicies }} +apiVersion: policy/v1beta1 +kind: PodSecurityPolicy +metadata: + name: {{ template "consul.fullname" . }}-sync-catalog-cleanup + namespace: {{ .Release.Namespace }} + labels: + app: {{ template "consul.name" . }} + chart: {{ template "consul.chart" . }} + heritage: {{ .Release.Service }} + release: {{ .Release.Name }} + component: sync-catalog-cleanup +spec: + privileged: false + allowPrivilegeEscalation: false + # This is redundant with non-root + disallow privilege escalation, + # but we can provide it for defense in depth. + requiredDropCapabilities: + - ALL + hostNetwork: false + hostIPC: false + hostPID: false + runAsUser: + rule: 'RunAsAny' + seLinux: + rule: 'RunAsAny' + supplementalGroups: + rule: 'RunAsAny' + fsGroup: + rule: 'RunAsAny' + readOnlyRootFilesystem: false +{{- end }} diff --git a/charts/consul/templates/sync-catalog-cleanup-serviceaccount.yaml b/charts/consul/templates/sync-catalog-cleanup-serviceaccount.yaml new file mode 100644 index 0000000000..162cdf6905 --- /dev/null +++ b/charts/consul/templates/sync-catalog-cleanup-serviceaccount.yaml @@ -0,0 +1,19 @@ +{{- if .Values.syncCatalog.purgeServicesOnDisable }} +apiVersion: v1 +kind: ServiceAccount +metadata: + name: {{ template "consul.fullname" . }}-sync-catalog-cleanup + namespace: {{ .Release.Namespace }} + labels: + app: {{ template "consul.name" . }} + chart: {{ template "consul.chart" . }} + heritage: {{ .Release.Service }} + release: {{ .Release.Name }} + component: sync-catalog-cleanup +{{- with .Values.global.imagePullSecrets }} +imagePullSecrets: +{{- range . }} + - name: {{ .name }} +{{- end }} +{{- end }} +{{- end }} diff --git a/charts/consul/templates/sync-catalog-deployment.yaml b/charts/consul/templates/sync-catalog-deployment.yaml index 8a54dd83b0..5e69110810 100644 --- a/charts/consul/templates/sync-catalog-deployment.yaml +++ b/charts/consul/templates/sync-catalog-deployment.yaml @@ -1,5 +1,5 @@ -{{ - $syncCatalogEnabled := (or (and (ne (.Values.syncCatalog.enabled | toString) "-") .Values.syncCatalog.enabled) (and (eq (.Values.syncCatalog.enabled | toString) "-") .Values.global.enabled)) }} -{{- if (or $syncCatalogEnabled .Values.syncCatalog.purgeServicesOnDisable) }} +{{- $syncCatalogEnabled := (or (and (ne (.Values.syncCatalog.enabled | toString) "-") .Values.syncCatalog.enabled) (and (eq (.Values.syncCatalog.enabled | toString) "-") .Values.global.enabled)) }} +{{- if $syncCatalogEnabled }} {{- template "consul.reservedNamesFailer" (list .Values.syncCatalog.consulNamespaces.consulDestinationNamespace "syncCatalog.consulNamespaces.consulDestinationNamespace") }} {{ template "consul.validateRequiredCloudSecretsExist" . }} {{ template "consul.validateCloudSecretKeys" . }} @@ -214,7 +214,6 @@ spec: -metrics-port={{ .Values.syncCatalog.metrics.port }} \ {{- end }} -prometheus-retention-time={{ .Values.global.metrics.agentMetricsRetentionTime }} \ - -purgeServicesOnDisable={{ and (not $syncCatalogEnabled) .Values.syncCatalog.purgeServicesOnDisable }} \ livenessProbe: httpGet: path: /health/ready From 9f491d4375b4af2a90c6b182ecbcda9ec5879e63 Mon Sep 17 00:00:00 2001 From: jm96441n Date: Thu, 17 Oct 2024 11:22:01 -0400 Subject: [PATCH 06/10] Check error --- control-plane/subcommand/sync-catalog/command_test.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/control-plane/subcommand/sync-catalog/command_test.go b/control-plane/subcommand/sync-catalog/command_test.go index 94f5452ad9..8c06c80f12 100644 --- a/control-plane/subcommand/sync-catalog/command_test.go +++ b/control-plane/subcommand/sync-catalog/command_test.go @@ -609,6 +609,8 @@ func TestRemoveAllK8SServicesFromConsul(t *testing.T) { // create another partition and register 2 services there to the same node to show that we won't delete them _, _, err = consulClient.Partitions().Create(context.Background(), &api.Partition{Name: otherPartitionName}, nil) + require.NoError(t, err) + _, err = consulClient.Catalog().Register( &api.CatalogRegistration{ Node: tc.nodeName, From bde9f054da6c79cc0dc6b2444be1cc6527745d68 Mon Sep 17 00:00:00 2001 From: jm96441n Date: Mon, 21 Oct 2024 12:33:44 -0400 Subject: [PATCH 07/10] fix tests --- .../sync-catalog/command_ent_test.go | 150 +++++++++++++++++- .../subcommand/sync-catalog/command_test.go | 85 ++++------ 2 files changed, 178 insertions(+), 57 deletions(-) diff --git a/control-plane/subcommand/sync-catalog/command_ent_test.go b/control-plane/subcommand/sync-catalog/command_ent_test.go index f8bdccda6b..231b2755c6 100644 --- a/control-plane/subcommand/sync-catalog/command_ent_test.go +++ b/control-plane/subcommand/sync-catalog/command_ent_test.go @@ -272,7 +272,6 @@ func TestRun_ToConsulMirroringNamespaces(t *testing.T) { } } - }) }) } @@ -706,3 +705,152 @@ func TestRun_ToConsulNamespacesACLs(t *testing.T) { }) } } + +// Test services could be de-registered from Consul. +func TestRemoveAllK8SServicesFromConsulWithPartitions(t *testing.T) { + testCases := map[string]struct { + nodeName string + partitionName string + }{ + "default Node name in default partition": { + nodeName: "k8s-sync", + partitionName: "default", + }, + "non-default Node name in default partition": { + nodeName: "custom-node", + partitionName: "default", + }, + "default Node name in non-default partition": { + nodeName: "k8s-sync", + partitionName: "part-1", + }, + "non-default Node name in non-default partition": { + nodeName: "custom-node", + partitionName: "part-1", + }, + } + + for name, tc := range testCases { + tc := tc + t.Run(name, func(t *testing.T) { + t.Parallel() + var err error + + k8s, testClient := completeSetup(t) + consulClient := testClient.APIClient + + // Create a mock reader to simulate user input + // Run the command. + ui := cli.NewMockUi() + cmd := Command{ + UI: ui, + clientset: k8s, + logger: hclog.New(&hclog.LoggerOptions{ + Name: t.Name(), + Level: hclog.Debug, + }), + flagAllowK8sNamespacesList: []string{"*"}, + connMgr: testClient.Watcher, + } + + otherPartitionName := "the-other-one" + + // create another partition and register 2 services there to the same node to show that we won't delete them + _, _, err = consulClient.Partitions().Create(context.Background(), &api.Partition{Name: otherPartitionName}, nil) + require.NoError(t, err) + + _, err = consulClient.Catalog().Register( + &api.CatalogRegistration{ + Node: tc.nodeName, + Address: "5.5.5.5", + Service: &api.AgentService{ + ID: "other-service-1", + Service: "other-service-1", + Tags: []string{"other-k8s-cluster"}, + Meta: map[string]string{}, + Port: 0, + Address: "5.5.5.5", + Partition: otherPartitionName, + }, + Partition: otherPartitionName, + }, + &api.WriteOptions{Partition: otherPartitionName}, + ) + require.NoError(t, err) + + _, err = consulClient.Catalog().Register( + &api.CatalogRegistration{ + Node: tc.nodeName, + Address: "6.6.6.6", + Service: &api.AgentService{ + ID: "other-service-2", + Service: "other-service-2", + Tags: []string{"other-k8s-cluster"}, + Meta: map[string]string{}, + Port: 0, + Address: "6.6.6.6", + Partition: otherPartitionName, + }, + Partition: otherPartitionName, + }, + &api.WriteOptions{Partition: otherPartitionName}, + ) + require.NoError(t, err) + + // create partition if it does not exist + if tc.partitionName != "default" { + p, _, err := consulClient.Partitions().Create(context.Background(), &api.Partition{Name: tc.partitionName}, nil) + require.NoError(t, err) + require.NotNil(t, p) + } + + // create two services in k8s + _, err = k8s.CoreV1().Services("bar").Create(context.Background(), lbService("foo", "1.1.1.1"), metav1.CreateOptions{}) + require.NoError(t, err) + + _, err = k8s.CoreV1().Services("baz").Create(context.Background(), lbService("foo", "2.2.2.2"), metav1.CreateOptions{}) + require.NoError(t, err) + + longRunningChan := runCommandAsynchronously(&cmd, []string{ + "-addresses", "127.0.0.1", + "-http-port", strconv.Itoa(testClient.Cfg.HTTPPort), + "-consul-write-interval", "100ms", + "-partition", tc.partitionName, + "-consul-node-name", tc.nodeName, + "-add-k8s-namespace-suffix", + }) + + // check that the two K8s services have been synced into Consul + retry.Run(t, func(r *retry.R) { + svc, _, err := consulClient.Catalog().Service("foo-bar", "k8s", &api.QueryOptions{Partition: tc.partitionName}) + require.NoError(r, err) + require.Len(r, svc, 1) + require.Equal(r, "1.1.1.1", svc[0].ServiceAddress) + svc, _, err = consulClient.Catalog().Service("foo-baz", "k8s", &api.QueryOptions{Partition: tc.partitionName}) + require.NoError(r, err) + require.Len(r, svc, 1) + require.Equal(r, "2.2.2.2", svc[0].ServiceAddress) + }) + + defer stopCommand(t, &cmd, longRunningChan) + + exitChan := runCommandAsynchronously(&cmd, []string{ + "-addresses", "127.0.0.1", + "-http-port", strconv.Itoa(testClient.Cfg.HTTPPort), + "-purge-k8s-services-from-node=true", + "-partition", tc.partitionName, + "-consul-node-name", tc.nodeName, + }) + stopCommand(t, &cmd, exitChan) + + retry.Run(t, func(r *retry.R) { + serviceList, _, err := consulClient.Catalog().NodeServiceList(tc.nodeName, &api.QueryOptions{AllowStale: false, Partition: tc.partitionName}) + require.NoError(r, err) + require.Len(r, serviceList.Services, 0) + otherPartitionServiceList, _, err := consulClient.Catalog().NodeServiceList(tc.nodeName, &api.QueryOptions{AllowStale: false, Partition: otherPartitionName}) + require.NoError(r, err) + require.Len(r, otherPartitionServiceList.Services, 2) + }) + }) + } +} diff --git a/control-plane/subcommand/sync-catalog/command_test.go b/control-plane/subcommand/sync-catalog/command_test.go index 8c06c80f12..d4d011b7e0 100644 --- a/control-plane/subcommand/sync-catalog/command_test.go +++ b/control-plane/subcommand/sync-catalog/command_test.go @@ -561,27 +561,18 @@ func TestRun_ToConsulChangingFlags(t *testing.T) { // Test services could be de-registered from Consul. func TestRemoveAllK8SServicesFromConsul(t *testing.T) { testCases := map[string]struct { - nodeName string - partitionName string + nodeToDeregisterName string }{ "default Node name in default partition": { - nodeName: "k8s-sync", - partitionName: "default", + nodeToDeregisterName: "k8s-sync", }, "non-default Node name in default partition": { - nodeName: "custom-node", - partitionName: "default", - }, - "default Node name in non-default partition": { - nodeName: "k8s-sync", - partitionName: "part-1", - }, - "non-default Node name in non-default partition": { - nodeName: "custom-node", - partitionName: "part-1", + nodeToDeregisterName: "custom-node", }, } + otherNodeName := "other-node" + for name, tc := range testCases { tc := tc t.Run(name, func(t *testing.T) { @@ -605,57 +596,40 @@ func TestRemoveAllK8SServicesFromConsul(t *testing.T) { connMgr: testClient.Watcher, } - otherPartitionName := "the-other-one" - - // create another partition and register 2 services there to the same node to show that we won't delete them - _, _, err = consulClient.Partitions().Create(context.Background(), &api.Partition{Name: otherPartitionName}, nil) - require.NoError(t, err) - _, err = consulClient.Catalog().Register( &api.CatalogRegistration{ - Node: tc.nodeName, + Node: otherNodeName, Address: "5.5.5.5", Service: &api.AgentService{ - ID: "other-service-1", - Service: "other-service-1", - Tags: []string{"other-k8s-cluster"}, - Meta: map[string]string{}, - Port: 0, - Address: "5.5.5.5", - Partition: otherPartitionName, + ID: "other-service-1", + Service: "other-service-1", + Tags: []string{"other-k8s-cluster"}, + Meta: map[string]string{}, + Port: 0, + Address: "5.5.5.5", }, - Partition: otherPartitionName, }, - &api.WriteOptions{Partition: otherPartitionName}, + &api.WriteOptions{}, ) require.NoError(t, err) _, err = consulClient.Catalog().Register( &api.CatalogRegistration{ - Node: tc.nodeName, + Node: otherNodeName, Address: "6.6.6.6", Service: &api.AgentService{ - ID: "other-service-2", - Service: "other-service-2", - Tags: []string{"other-k8s-cluster"}, - Meta: map[string]string{}, - Port: 0, - Address: "6.6.6.6", - Partition: otherPartitionName, + ID: "other-service-2", + Service: "other-service-2", + Tags: []string{"other-k8s-cluster"}, + Meta: map[string]string{}, + Port: 0, + Address: "6.6.6.6", }, - Partition: otherPartitionName, }, - &api.WriteOptions{Partition: otherPartitionName}, + &api.WriteOptions{}, ) require.NoError(t, err) - // create partition if it does not exist - if tc.partitionName != "default" { - p, _, err := consulClient.Partitions().Create(context.Background(), &api.Partition{Name: tc.partitionName}, nil) - require.NoError(t, err) - require.NotNil(t, p) - } - // create two services in k8s _, err = k8s.CoreV1().Services("bar").Create(context.Background(), lbService("foo", "1.1.1.1"), metav1.CreateOptions{}) require.NoError(t, err) @@ -667,18 +641,17 @@ func TestRemoveAllK8SServicesFromConsul(t *testing.T) { "-addresses", "127.0.0.1", "-http-port", strconv.Itoa(testClient.Cfg.HTTPPort), "-consul-write-interval", "100ms", - "-partition", tc.partitionName, - "-consul-node-name", tc.nodeName, + "-consul-node-name", tc.nodeToDeregisterName, "-add-k8s-namespace-suffix", }) // check that the two K8s services have been synced into Consul retry.Run(t, func(r *retry.R) { - svc, _, err := consulClient.Catalog().Service("foo-bar", "k8s", &api.QueryOptions{Partition: tc.partitionName}) + svc, _, err := consulClient.Catalog().Service("foo-bar", "k8s", &api.QueryOptions{}) require.NoError(r, err) require.Len(r, svc, 1) require.Equal(r, "1.1.1.1", svc[0].ServiceAddress) - svc, _, err = consulClient.Catalog().Service("foo-baz", "k8s", &api.QueryOptions{Partition: tc.partitionName}) + svc, _, err = consulClient.Catalog().Service("foo-baz", "k8s", &api.QueryOptions{}) require.NoError(r, err) require.Len(r, svc, 1) require.Equal(r, "2.2.2.2", svc[0].ServiceAddress) @@ -690,18 +663,18 @@ func TestRemoveAllK8SServicesFromConsul(t *testing.T) { "-addresses", "127.0.0.1", "-http-port", strconv.Itoa(testClient.Cfg.HTTPPort), "-purge-k8s-services-from-node=true", - "-partition", tc.partitionName, - "-consul-node-name", tc.nodeName, + "-consul-node-name", tc.nodeToDeregisterName, }) stopCommand(t, &cmd, exitChan) retry.Run(t, func(r *retry.R) { - serviceList, _, err := consulClient.Catalog().NodeServiceList(tc.nodeName, &api.QueryOptions{AllowStale: false, Partition: tc.partitionName}) + serviceList, _, err := consulClient.Catalog().NodeServiceList(tc.nodeToDeregisterName, &api.QueryOptions{AllowStale: false}) require.NoError(r, err) require.Len(r, serviceList.Services, 0) - otherPartitionServiceList, _, err := consulClient.Catalog().NodeServiceList(tc.nodeName, &api.QueryOptions{AllowStale: false, Partition: otherPartitionName}) + + otherNodeServiceList, _, err := consulClient.Catalog().NodeServiceList(otherNodeName, &api.QueryOptions{AllowStale: false}) require.NoError(r, err) - require.Len(r, otherPartitionServiceList.Services, 2) + require.Len(r, otherNodeServiceList.Services, 2) }) }) } From 681eee15d4042f82162453affa7e897c69b6a63f Mon Sep 17 00:00:00 2001 From: jm96441n Date: Mon, 21 Oct 2024 15:27:46 -0400 Subject: [PATCH 08/10] rename field in values file, remove references to pod security policy --- .../templates/sync-catalog-cleanup-on-uninstall-job.yaml | 4 +--- .../consul/templates/sync-catalog-cleanup-on-upgrade-job.yaml | 2 +- .../templates/sync-catalog-cleanup-podsecuritypolicy.yaml | 2 +- .../consul/templates/sync-catalog-cleanup-serviceaccount.yaml | 2 +- charts/consul/values.yaml | 2 +- 5 files changed, 5 insertions(+), 7 deletions(-) diff --git a/charts/consul/templates/sync-catalog-cleanup-on-uninstall-job.yaml b/charts/consul/templates/sync-catalog-cleanup-on-uninstall-job.yaml index 0e4c6c3fff..626b325985 100644 --- a/charts/consul/templates/sync-catalog-cleanup-on-uninstall-job.yaml +++ b/charts/consul/templates/sync-catalog-cleanup-on-uninstall-job.yaml @@ -1,4 +1,4 @@ -{{- if .Values.syncCatalog.purgeServicesOnDisable }} +{{- if .Values.syncCatalog.cleanupNodeOnRemoval }} apiVersion: batch/v1 kind: Job metadata: @@ -98,9 +98,7 @@ spec: -metrics-port={{ .Values.syncCatalog.metrics.port }} \ {{- end }} -prometheus-retention-time={{ .Values.global.metrics.agentMetricsRetentionTime }} \ - {{- if .Release.IsUpgrade }} -purge-k8s-services-from-node - {{- end }} {{- if .Values.global.acls.tolerations }} tolerations: {{ tpl .Values.global.acls.tolerations . | indent 8 | trim }} diff --git a/charts/consul/templates/sync-catalog-cleanup-on-upgrade-job.yaml b/charts/consul/templates/sync-catalog-cleanup-on-upgrade-job.yaml index f9dfd1cccc..a14d4a9a9f 100644 --- a/charts/consul/templates/sync-catalog-cleanup-on-upgrade-job.yaml +++ b/charts/consul/templates/sync-catalog-cleanup-on-upgrade-job.yaml @@ -1,5 +1,5 @@ {{- $syncCatalogEnabled := (or (and (ne (.Values.syncCatalog.enabled | toString) "-") .Values.syncCatalog.enabled) (and (eq (.Values.syncCatalog.enabled | toString) "-") .Values.global.enabled)) }} -{{- if and (not $syncCatalogEnabled) .Values.syncCatalog.purgeServicesOnDisable }} +{{- if and (not $syncCatalogEnabled) .Values.syncCatalog.cleanupNodeOnRemoval }} apiVersion: batch/v1 kind: Job metadata: diff --git a/charts/consul/templates/sync-catalog-cleanup-podsecuritypolicy.yaml b/charts/consul/templates/sync-catalog-cleanup-podsecuritypolicy.yaml index c3c847b4ae..7050250653 100644 --- a/charts/consul/templates/sync-catalog-cleanup-podsecuritypolicy.yaml +++ b/charts/consul/templates/sync-catalog-cleanup-podsecuritypolicy.yaml @@ -1,4 +1,4 @@ -{{- if and .Values.syncCatalog.purgeServicesOnDisable .Values.global.enablePodSecurityPolicies }} +{{- if and .Values.syncCatalog.cleanupNodeOnRemoval }} apiVersion: policy/v1beta1 kind: PodSecurityPolicy metadata: diff --git a/charts/consul/templates/sync-catalog-cleanup-serviceaccount.yaml b/charts/consul/templates/sync-catalog-cleanup-serviceaccount.yaml index 162cdf6905..b4bd141ed7 100644 --- a/charts/consul/templates/sync-catalog-cleanup-serviceaccount.yaml +++ b/charts/consul/templates/sync-catalog-cleanup-serviceaccount.yaml @@ -1,4 +1,4 @@ -{{- if .Values.syncCatalog.purgeServicesOnDisable }} +{{- if .Values.syncCatalog.cleanupNodeOnRemoval }} apiVersion: v1 kind: ServiceAccount metadata: diff --git a/charts/consul/values.yaml b/charts/consul/values.yaml index 94b025c535..13615e716c 100644 --- a/charts/consul/values.yaml +++ b/charts/consul/values.yaml @@ -2092,7 +2092,7 @@ syncCatalog: # True if you want to deregister all services in this cluster from consul that have been registered by the catalog sync when the `enabled` flag is set to false. # @type: boolean - purgeServicesOnDisable: false + cleanupNodeOnRemoval: false # The name of the Docker image (including any tag) for consul-k8s-control-plane # to run the sync program. From 8dd18f4070630a3432cdedc346cb0f9dec3e390b Mon Sep 17 00:00:00 2001 From: jm96441n Date: Mon, 21 Oct 2024 15:38:03 -0400 Subject: [PATCH 09/10] remove psp --- ...ync-catalog-cleanup-podsecuritypolicy.yaml | 32 ------------------- 1 file changed, 32 deletions(-) delete mode 100644 charts/consul/templates/sync-catalog-cleanup-podsecuritypolicy.yaml diff --git a/charts/consul/templates/sync-catalog-cleanup-podsecuritypolicy.yaml b/charts/consul/templates/sync-catalog-cleanup-podsecuritypolicy.yaml deleted file mode 100644 index 7050250653..0000000000 --- a/charts/consul/templates/sync-catalog-cleanup-podsecuritypolicy.yaml +++ /dev/null @@ -1,32 +0,0 @@ -{{- if and .Values.syncCatalog.cleanupNodeOnRemoval }} -apiVersion: policy/v1beta1 -kind: PodSecurityPolicy -metadata: - name: {{ template "consul.fullname" . }}-sync-catalog-cleanup - namespace: {{ .Release.Namespace }} - labels: - app: {{ template "consul.name" . }} - chart: {{ template "consul.chart" . }} - heritage: {{ .Release.Service }} - release: {{ .Release.Name }} - component: sync-catalog-cleanup -spec: - privileged: false - allowPrivilegeEscalation: false - # This is redundant with non-root + disallow privilege escalation, - # but we can provide it for defense in depth. - requiredDropCapabilities: - - ALL - hostNetwork: false - hostIPC: false - hostPID: false - runAsUser: - rule: 'RunAsAny' - seLinux: - rule: 'RunAsAny' - supplementalGroups: - rule: 'RunAsAny' - fsGroup: - rule: 'RunAsAny' - readOnlyRootFilesystem: false -{{- end }} From 6a2b63d5114f2ffe1d7166e6c68a149ed6bb370b Mon Sep 17 00:00:00 2001 From: jm96441n Date: Mon, 21 Oct 2024 16:33:17 -0400 Subject: [PATCH 10/10] added bats testing --- ...sync-catalog-cleanup-on-uninstall-job.yaml | 57 ++++++++++++++++- .../sync-catalog-cleanup-on-upgrade-job.yaml | 61 +++++++++++++++++-- .../sync-catalog-cleanup-serviceaccount.yaml | 4 ++ 3 files changed, 115 insertions(+), 7 deletions(-) diff --git a/charts/consul/templates/sync-catalog-cleanup-on-uninstall-job.yaml b/charts/consul/templates/sync-catalog-cleanup-on-uninstall-job.yaml index 626b325985..955042dd0a 100644 --- a/charts/consul/templates/sync-catalog-cleanup-on-uninstall-job.yaml +++ b/charts/consul/templates/sync-catalog-cleanup-on-uninstall-job.yaml @@ -26,12 +26,42 @@ spec: chart: {{ template "consul.chart" . }} release: {{ .Release.Name }} component: sync-catalog-cleanup + {{- if .Values.syncCatalog.extraLabels }} + {{- toYaml .Values.syncCatalog.extraLabels | nindent 8 }} + {{- end }} {{- if .Values.global.extraLabels }} {{- toYaml .Values.global.extraLabels | nindent 8 }} {{- end }} annotations: "consul.hashicorp.com/connect-inject": "false" "consul.hashicorp.com/mesh-inject": "false" + {{- if .Values.syncCatalog.annotations }} + {{- tpl .Values.syncCatalog.annotations . | nindent 8 }} + {{- end }} + {{- if (and .Values.global.secretsBackend.vault.enabled .Values.global.tls.enabled) }} + "vault.hashicorp.com/agent-init-first": "true" + "vault.hashicorp.com/agent-inject": "true" + "vault.hashicorp.com/role": {{ .Values.global.secretsBackend.vault.consulCARole }} + "vault.hashicorp.com/agent-inject-secret-serverca.crt": {{ .Values.global.tls.caCert.secretName }} + "vault.hashicorp.com/agent-inject-template-serverca.crt": {{ template "consul.serverTLSCATemplate" . }} + {{- if and .Values.global.secretsBackend.vault.ca.secretName .Values.global.secretsBackend.vault.ca.secretKey }} + "vault.hashicorp.com/agent-extra-secret": "{{ .Values.global.secretsBackend.vault.ca.secretName }}" + "vault.hashicorp.com/ca-cert": "/vault/custom/{{ .Values.global.secretsBackend.vault.ca.secretKey }}" + {{- end }} + {{- if .Values.global.secretsBackend.vault.agentAnnotations }} + {{ tpl .Values.global.secretsBackend.vault.agentAnnotations . | nindent 8 | trim }} + {{- end }} + {{- if (and (.Values.global.secretsBackend.vault.vaultNamespace) (not (hasKey (default "" .Values.global.secretsBackend.vault.agentAnnotations | fromYaml) "vault.hashicorp.com/namespace")))}} + "vault.hashicorp.com/namespace": "{{ .Values.global.secretsBackend.vault.vaultNamespace }}" + {{- end }} + {{- end }} + {{- if .Values.syncCatalog.metrics.enabled | default .Values.global.metrics.enabled }} + "prometheus.io/scrape": "true" + {{- if not (hasKey (default "" .Values.syncCatalog.annotations | fromYaml) "prometheus.io/path")}} + "prometheus.io/path": {{ default "/metrics" .Values.syncCatalog.metrics.path }} + {{- end }} + "prometheus.io/port": {{ .Values.syncCatalog.metrics.port | default "20300" | quote }} + {{- end }} spec: restartPolicy: Never serviceAccountName: {{ template "consul.fullname" . }}-sync-catalog-cleanup @@ -88,6 +118,9 @@ spec: {{- if .Values.global.adminPartitions.enabled }} -partition={{ .Values.global.adminPartitions.name }} \ {{- end }} + {{- if .Values.syncCatalog.consulNodeName }} + -consul-node-name={{ .Values.syncCatalog.consulNodeName }} \ + {{- end }} {{- if .Values.syncCatalog.metrics.enabled | default .Values.global.metrics.enabled }} -enable-metrics \ {{- end }} @@ -99,9 +132,29 @@ spec: {{- end }} -prometheus-retention-time={{ .Values.global.metrics.agentMetricsRetentionTime }} \ -purge-k8s-services-from-node - {{- if .Values.global.acls.tolerations }} + {{- with .Values.syncCatalog.resources }} + resources: + {{- toYaml . | nindent 10 }} + {{- end }} + {{- if or (eq (.Values.syncCatalog.metrics.enabled | toString) "-") .Values.syncCatalog.metrics.enabled .Values.global.metrics.enabled }} + ports: + - name: prometheus + containerPort: {{ .Values.syncCatalog.metrics.port | default "20300" | int }} + {{- end }} + {{- if .Values.syncCatalog.priorityClassName }} + priorityClassName: {{ .Values.syncCatalog.priorityClassName | quote }} + {{- end }} + {{- if .Values.syncCatalog.nodeSelector }} + nodeSelector: + {{ tpl .Values.syncCatalog.nodeSelector . | indent 8 | trim }} + {{- end }} + {{- if .Values.syncCatalog.affinity }} + affinity: + {{ tpl .Values.syncCatalog.affinity . | indent 8 | trim }} + {{- end }} + {{- if .Values.syncCatalog.tolerations }} tolerations: - {{ tpl .Values.global.acls.tolerations . | indent 8 | trim }} + {{ tpl .Values.syncCatalog.tolerations . | indent 8 | trim }} {{- end }} volumes: {{- if .Values.global.tls.enabled }} diff --git a/charts/consul/templates/sync-catalog-cleanup-on-upgrade-job.yaml b/charts/consul/templates/sync-catalog-cleanup-on-upgrade-job.yaml index a14d4a9a9f..2349c513d4 100644 --- a/charts/consul/templates/sync-catalog-cleanup-on-upgrade-job.yaml +++ b/charts/consul/templates/sync-catalog-cleanup-on-upgrade-job.yaml @@ -1,5 +1,5 @@ {{- $syncCatalogEnabled := (or (and (ne (.Values.syncCatalog.enabled | toString) "-") .Values.syncCatalog.enabled) (and (eq (.Values.syncCatalog.enabled | toString) "-") .Values.global.enabled)) }} -{{- if and (not $syncCatalogEnabled) .Values.syncCatalog.cleanupNodeOnRemoval }} +{{- if and (not $syncCatalogEnabled) .Values.syncCatalog.cleanupNodeOnRemoval .Release.IsUpgrade }} apiVersion: batch/v1 kind: Job metadata: @@ -27,12 +27,42 @@ spec: chart: {{ template "consul.chart" . }} release: {{ .Release.Name }} component: sync-catalog-cleanup + {{- if .Values.syncCatalog.extraLabels }} + {{- toYaml .Values.syncCatalog.extraLabels | nindent 8 }} + {{- end }} {{- if .Values.global.extraLabels }} {{- toYaml .Values.global.extraLabels | nindent 8 }} {{- end }} annotations: "consul.hashicorp.com/connect-inject": "false" "consul.hashicorp.com/mesh-inject": "false" + {{- if .Values.syncCatalog.annotations }} + {{- tpl .Values.syncCatalog.annotations . | nindent 8 }} + {{- end }} + {{- if (and .Values.global.secretsBackend.vault.enabled .Values.global.tls.enabled) }} + "vault.hashicorp.com/agent-init-first": "true" + "vault.hashicorp.com/agent-inject": "true" + "vault.hashicorp.com/role": {{ .Values.global.secretsBackend.vault.consulCARole }} + "vault.hashicorp.com/agent-inject-secret-serverca.crt": {{ .Values.global.tls.caCert.secretName }} + "vault.hashicorp.com/agent-inject-template-serverca.crt": {{ template "consul.serverTLSCATemplate" . }} + {{- if and .Values.global.secretsBackend.vault.ca.secretName .Values.global.secretsBackend.vault.ca.secretKey }} + "vault.hashicorp.com/agent-extra-secret": "{{ .Values.global.secretsBackend.vault.ca.secretName }}" + "vault.hashicorp.com/ca-cert": "/vault/custom/{{ .Values.global.secretsBackend.vault.ca.secretKey }}" + {{- end }} + {{- if .Values.global.secretsBackend.vault.agentAnnotations }} + {{ tpl .Values.global.secretsBackend.vault.agentAnnotations . | nindent 8 | trim }} + {{- end }} + {{- if (and (.Values.global.secretsBackend.vault.vaultNamespace) (not (hasKey (default "" .Values.global.secretsBackend.vault.agentAnnotations | fromYaml) "vault.hashicorp.com/namespace")))}} + "vault.hashicorp.com/namespace": "{{ .Values.global.secretsBackend.vault.vaultNamespace }}" + {{- end }} + {{- end }} + {{- if .Values.syncCatalog.metrics.enabled | default .Values.global.metrics.enabled }} + "prometheus.io/scrape": "true" + {{- if not (hasKey (default "" .Values.syncCatalog.annotations | fromYaml) "prometheus.io/path")}} + "prometheus.io/path": {{ default "/metrics" .Values.syncCatalog.metrics.path }} + {{- end }} + "prometheus.io/port": {{ .Values.syncCatalog.metrics.port | default "20300" | quote }} + {{- end }} spec: restartPolicy: Never serviceAccountName: {{ template "consul.fullname" . }}-sync-catalog-cleanup @@ -89,6 +119,9 @@ spec: {{- if .Values.global.adminPartitions.enabled }} -partition={{ .Values.global.adminPartitions.name }} \ {{- end }} + {{- if .Values.syncCatalog.consulNodeName }} + -consul-node-name={{ .Values.syncCatalog.consulNodeName }} \ + {{- end }} {{- if .Values.syncCatalog.metrics.enabled | default .Values.global.metrics.enabled }} -enable-metrics \ {{- end }} @@ -99,12 +132,30 @@ spec: -metrics-port={{ .Values.syncCatalog.metrics.port }} \ {{- end }} -prometheus-retention-time={{ .Values.global.metrics.agentMetricsRetentionTime }} \ - {{- if .Release.IsUpgrade }} -purge-k8s-services-from-node - {{- end }} - {{- if .Values.global.acls.tolerations }} + {{- with .Values.syncCatalog.resources }} + resources: + {{- toYaml . | nindent 10 }} + {{- end }} + {{- if or (eq (.Values.syncCatalog.metrics.enabled | toString) "-") .Values.syncCatalog.metrics.enabled .Values.global.metrics.enabled }} + ports: + - name: prometheus + containerPort: {{ .Values.syncCatalog.metrics.port | default "20300" | int }} + {{- end }} + {{- if .Values.syncCatalog.priorityClassName }} + priorityClassName: {{ .Values.syncCatalog.priorityClassName | quote }} + {{- end }} + {{- if .Values.syncCatalog.nodeSelector }} + nodeSelector: + {{ tpl .Values.syncCatalog.nodeSelector . | indent 8 | trim }} + {{- end }} + {{- if .Values.syncCatalog.affinity }} + affinity: + {{ tpl .Values.syncCatalog.affinity . | indent 8 | trim }} + {{- end }} + {{- if .Values.syncCatalog.tolerations }} tolerations: - {{ tpl .Values.global.acls.tolerations . | indent 8 | trim }} + {{ tpl .Values.syncCatalog.tolerations . | indent 8 | trim }} {{- end }} volumes: {{- if .Values.global.tls.enabled }} diff --git a/charts/consul/templates/sync-catalog-cleanup-serviceaccount.yaml b/charts/consul/templates/sync-catalog-cleanup-serviceaccount.yaml index b4bd141ed7..c49852b14c 100644 --- a/charts/consul/templates/sync-catalog-cleanup-serviceaccount.yaml +++ b/charts/consul/templates/sync-catalog-cleanup-serviceaccount.yaml @@ -10,6 +10,10 @@ metadata: heritage: {{ .Release.Service }} release: {{ .Release.Name }} component: sync-catalog-cleanup + {{- if .Values.syncCatalog.serviceAccount.annotations }} + annotations: + {{ tpl .Values.syncCatalog.serviceAccount.annotations . | nindent 4 | trim }} + {{- end }} {{- with .Values.global.imagePullSecrets }} imagePullSecrets: {{- range . }}