From 35b2213e058aebfde274844844ea66ee9556fd3f Mon Sep 17 00:00:00 2001 From: Jonathan Herlin Date: Wed, 11 Mar 2020 11:32:13 +0100 Subject: [PATCH 1/6] Fix typo in values file (#861) * Fix typo Co-authored-by: Jonathan Herlin --- charts/postgres-operator/values-crd.yaml | 2 +- charts/postgres-operator/values.yaml | 2 +- docs/reference/operator_parameters.md | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/charts/postgres-operator/values-crd.yaml b/charts/postgres-operator/values-crd.yaml index b5d561807..d170e0b77 100644 --- a/charts/postgres-operator/values-crd.yaml +++ b/charts/postgres-operator/values-crd.yaml @@ -218,7 +218,7 @@ configLogicalBackup: logical_backup_s3_endpoint: "" # S3 Secret Access Key logical_backup_s3_secret_access_key: "" - # S3 server side encription + # S3 server side encryption logical_backup_s3_sse: "AES256" # backup schedule in the cron format logical_backup_schedule: "30 00 * * *" diff --git a/charts/postgres-operator/values.yaml b/charts/postgres-operator/values.yaml index 07ba76285..b6f18f305 100644 --- a/charts/postgres-operator/values.yaml +++ b/charts/postgres-operator/values.yaml @@ -209,7 +209,7 @@ configLogicalBackup: logical_backup_s3_endpoint: "" # S3 Secret Access Key logical_backup_s3_secret_access_key: "" - # S3 server side encription + # S3 server side encryption logical_backup_s3_sse: "AES256" # backup schedule in the cron format logical_backup_schedule: "30 00 * * *" diff --git a/docs/reference/operator_parameters.md b/docs/reference/operator_parameters.md index ad519b657..ba8e73cf8 100644 --- a/docs/reference/operator_parameters.md +++ b/docs/reference/operator_parameters.md @@ -284,7 +284,7 @@ configuration they are grouped under the `kubernetes` key. used for AWS volume resizing and not required if you don't need that capability. The default is `false`. - * **master_pod_move_timeout** +* **master_pod_move_timeout** The period of time to wait for the success of migration of master pods from an unschedulable node. The migration includes Patroni switchovers to respective replicas on healthy nodes. The situation where master pods still @@ -472,7 +472,7 @@ grouped under the `logical_backup` key. When using non-AWS S3 storage, endpoint can be set as a ENV variable. The default is empty. * **logical_backup_s3_sse** - Specify server side encription that S3 storage is using. If empty string + Specify server side encryption that S3 storage is using. If empty string is specified, no argument will be passed to `aws s3` command. Default: "AES256". * **logical_backup_s3_access_key_id** From cde61f3f0b9d91863ac16cab89463c647d297517 Mon Sep 17 00:00:00 2001 From: Felix Kunde Date: Wed, 11 Mar 2020 14:08:54 +0100 Subject: [PATCH 2/6] e2e: wait for pods after disabling anti affinity (#862) --- e2e/tests/test_e2e.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/e2e/tests/test_e2e.py b/e2e/tests/test_e2e.py index 6760e815d..f6be8a600 100644 --- a/e2e/tests/test_e2e.py +++ b/e2e/tests/test_e2e.py @@ -441,13 +441,15 @@ class EndToEndTestCase(unittest.TestCase): self.assert_failover( master_node, len(replica_nodes), failover_targets, cluster_label) - # disable pod anti affintiy again + # now disable pod anti affintiy again which will cause yet another failover patch_disable_antiaffinity = { "data": { "enable_pod_antiaffinity": "false" } } k8s.update_config(patch_disable_antiaffinity) + k8s.wait_for_pod_start('spilo-role=master') + k8s.wait_for_pod_start('spilo-role=replica') class K8sApi: From 650b8daf77f35d03b0fbd989908c3dea4a78108f Mon Sep 17 00:00:00 2001 From: grantlanglois Date: Thu, 12 Mar 2020 04:12:53 -0700 Subject: [PATCH 3/6] add json:omitempty option to ClusterDomain (#851) Co-authored-by: mlu42 --- pkg/apis/acid.zalan.do/v1/operator_configuration_type.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/apis/acid.zalan.do/v1/operator_configuration_type.go b/pkg/apis/acid.zalan.do/v1/operator_configuration_type.go index ded5261fb..8463be6bd 100644 --- a/pkg/apis/acid.zalan.do/v1/operator_configuration_type.go +++ b/pkg/apis/acid.zalan.do/v1/operator_configuration_type.go @@ -53,7 +53,7 @@ type KubernetesMetaConfiguration struct { EnableInitContainers *bool `json:"enable_init_containers,omitempty"` EnableSidecars *bool `json:"enable_sidecars,omitempty"` SecretNameTemplate config.StringTemplate `json:"secret_name_template,omitempty"` - ClusterDomain string `json:"cluster_domain"` + ClusterDomain string `json:"cluster_domain,omitempty"` OAuthTokenSecretName spec.NamespacedName `json:"oauth_token_secret_name,omitempty"` InfrastructureRolesSecretName spec.NamespacedName `json:"infrastructure_roles_secret_name,omitempty"` PodRoleLabel string `json:"pod_role_label,omitempty"` From 65fb2ce1a622109d4ec0f9cc40bf8590d8647659 Mon Sep 17 00:00:00 2001 From: zimbatm Date: Fri, 13 Mar 2020 11:44:38 +0100 Subject: [PATCH 4/6] add support for custom TLS certificates (#798) * add support for custom TLS certificates --- docs/reference/cluster_manifest.md | 21 ++++ docs/user.md | 47 ++++++++ go.mod | 1 + go.sum | 1 + manifests/complete-postgres-manifest.yaml | 7 ++ manifests/postgresql.crd.yaml | 13 +++ pkg/apis/acid.zalan.do/v1/crds.go | 18 ++++ pkg/apis/acid.zalan.do/v1/postgresql_type.go | 8 ++ .../acid.zalan.do/v1/zz_generated.deepcopy.go | 21 ++++ pkg/cluster/k8sres.go | 100 +++++++++++++++--- pkg/cluster/k8sres_test.go | 67 +++++++++++- 11 files changed, 285 insertions(+), 19 deletions(-) diff --git a/docs/reference/cluster_manifest.md b/docs/reference/cluster_manifest.md index 7b049b6fa..92e457d7e 100644 --- a/docs/reference/cluster_manifest.md +++ b/docs/reference/cluster_manifest.md @@ -359,3 +359,24 @@ CPU and memory limits for the sidecar container. * **memory** memory limits for the sidecar container. Optional, overrides the `default_memory_limits` operator configuration parameter. Optional. + +## Custom TLS certificates + +Those parameters are grouped under the `tls` top-level key. + +* **secretName** + By setting the `secretName` value, the cluster will switch to load the given + Kubernetes Secret into the container as a volume and uses that as the + certificate instead. It is up to the user to create and manage the + Kubernetes Secret either by hand or using a tool like the CertManager + operator. + +* **certificateFile** + Filename of the certificate. Defaults to "tls.crt". + +* **privateKeyFile** + Filename of the private key. Defaults to "tls.key". + +* **caFile** + Optional filename to the CA certificate. Useful when the client connects + with `sslmode=verify-ca` or `sslmode=verify-full`. diff --git a/docs/user.md b/docs/user.md index 295c149bd..6e71d0404 100644 --- a/docs/user.md +++ b/docs/user.md @@ -511,3 +511,50 @@ monitoring is outside the scope of operator responsibilities. See [configuration reference](reference/cluster_manifest.md) and [administrator documentation](administrator.md) for details on how backups are executed. + +## Custom TLS certificates + +By default, the spilo image generates its own TLS certificate during startup. +This certificate is not secure since it cannot be verified and thus doesn't +protect from active MITM attacks. In this section we show how a Kubernete +Secret resources can be loaded with a custom TLS certificate. + +Before applying these changes, the operator must also be configured with the +`spilo_fsgroup` set to the GID matching the postgres user group. If the value +is not provided, the cluster will default to `103` which is the GID from the +default spilo image. + +Upload the cert as a kubernetes secret: +```sh +kubectl create secret tls pg-tls \ + --key pg-tls.key \ + --cert pg-tls.crt +``` + +Or with a CA: +```sh +kubectl create secret generic pg-tls \ + --from-file=tls.crt=server.crt \ + --from-file=tls.key=server.key \ + --from-file=ca.crt=ca.crt +``` + +Alternatively it is also possible to use +[cert-manager](https://cert-manager.io/docs/) to generate these secrets. + +Then configure the postgres resource with the TLS secret: + +```yaml +apiVersion: "acid.zalan.do/v1" +kind: postgresql + +metadata: + name: acid-test-cluster +spec: + tls: + secretName: "pg-tls" + caFile: "ca.crt" # add this if the secret is configured with a CA +``` + +Certificate rotation is handled in the spilo image which checks every 5 +minutes if the certificates have changed and reloads postgres accordingly. diff --git a/go.mod b/go.mod index 36686dcf6..be1ec32d4 100644 --- a/go.mod +++ b/go.mod @@ -11,6 +11,7 @@ require ( github.com/lib/pq v1.2.0 github.com/motomux/pretty v0.0.0-20161209205251-b2aad2c9a95d github.com/sirupsen/logrus v1.4.2 + github.com/stretchr/testify v1.4.0 golang.org/x/crypto v0.0.0-20191206172530-e9b2fee46413 // indirect golang.org/x/net v0.0.0-20191209160850-c0dbc17a3553 // indirect golang.org/x/sys v0.0.0-20191210023423-ac6580df4449 // indirect diff --git a/go.sum b/go.sum index f85dd060f..0737d3a5d 100644 --- a/go.sum +++ b/go.sum @@ -275,6 +275,7 @@ github.com/stretchr/objx v0.1.1/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+ github.com/stretchr/objx v0.2.0/go.mod h1:qt09Ya8vawLte6SNmTgCsAVtYtaKzEcn8ATUoHMkEqE= github.com/stretchr/testify v0.0.0-20151208002404-e3a8ff8ce365/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs= github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs= +github.com/stretchr/testify v1.3.0 h1:TivCn/peBQ7UY8ooIcPgZFpTNSz0Q2U6UrFlUfqbe0Q= github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI= github.com/stretchr/testify v1.4.0 h1:2E4SXV/wtOkTonXsotYi4li6zVWxYlZuYNCXe9XRJyk= github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81PSLYec5m4= diff --git a/manifests/complete-postgres-manifest.yaml b/manifests/complete-postgres-manifest.yaml index 5ae817ca3..5ba1fd1bc 100644 --- a/manifests/complete-postgres-manifest.yaml +++ b/manifests/complete-postgres-manifest.yaml @@ -100,3 +100,10 @@ spec: # env: # - name: "USEFUL_VAR" # value: "perhaps-true" + +# Custom TLS certificate. Disabled unless tls.secretName has a value. + tls: + secretName: "" # should correspond to a Kubernetes Secret resource to load + certificateFile: "tls.crt" + privateKeyFile: "tls.key" + caFile: "" # optionally configure Postgres with a CA certificate diff --git a/manifests/postgresql.crd.yaml b/manifests/postgresql.crd.yaml index 453916b26..04c789fb9 100644 --- a/manifests/postgresql.crd.yaml +++ b/manifests/postgresql.crd.yaml @@ -251,6 +251,19 @@ spec: type: string teamId: type: string + tls: + type: object + required: + - secretName + properties: + secretName: + type: string + certificateFile: + type: string + privateKeyFile: + type: string + caFile: + type: string tolerations: type: array items: diff --git a/pkg/apis/acid.zalan.do/v1/crds.go b/pkg/apis/acid.zalan.do/v1/crds.go index 28dfa1566..eff47c255 100644 --- a/pkg/apis/acid.zalan.do/v1/crds.go +++ b/pkg/apis/acid.zalan.do/v1/crds.go @@ -417,6 +417,24 @@ var PostgresCRDResourceValidation = apiextv1beta1.CustomResourceValidation{ "teamId": { Type: "string", }, + "tls": { + Type: "object", + Required: []string{"secretName"}, + Properties: map[string]apiextv1beta1.JSONSchemaProps{ + "secretName": { + Type: "string", + }, + "certificateFile": { + Type: "string", + }, + "privateKeyFile": { + Type: "string", + }, + "caFile": { + Type: "string", + }, + }, + }, "tolerations": { Type: "array", Items: &apiextv1beta1.JSONSchemaPropsOrArray{ diff --git a/pkg/apis/acid.zalan.do/v1/postgresql_type.go b/pkg/apis/acid.zalan.do/v1/postgresql_type.go index 07b42d4d4..862db6a4e 100644 --- a/pkg/apis/acid.zalan.do/v1/postgresql_type.go +++ b/pkg/apis/acid.zalan.do/v1/postgresql_type.go @@ -61,6 +61,7 @@ type PostgresSpec struct { StandbyCluster *StandbyDescription `json:"standby"` PodAnnotations map[string]string `json:"podAnnotations"` ServiceAnnotations map[string]string `json:"serviceAnnotations"` + TLS *TLSDescription `json:"tls"` // deprecated json tags InitContainersOld []v1.Container `json:"init_containers,omitempty"` @@ -126,6 +127,13 @@ type StandbyDescription struct { S3WalPath string `json:"s3_wal_path,omitempty"` } +type TLSDescription struct { + SecretName string `json:"secretName,omitempty"` + CertificateFile string `json:"certificateFile,omitempty"` + PrivateKeyFile string `json:"privateKeyFile,omitempty"` + CAFile string `json:"caFile,omitempty"` +} + // CloneDescription describes which cluster the new should clone and up to which point in time type CloneDescription struct { ClusterName string `json:"cluster,omitempty"` diff --git a/pkg/apis/acid.zalan.do/v1/zz_generated.deepcopy.go b/pkg/apis/acid.zalan.do/v1/zz_generated.deepcopy.go index aaae1f04b..753b0490c 100644 --- a/pkg/apis/acid.zalan.do/v1/zz_generated.deepcopy.go +++ b/pkg/apis/acid.zalan.do/v1/zz_generated.deepcopy.go @@ -521,6 +521,11 @@ func (in *PostgresSpec) DeepCopyInto(out *PostgresSpec) { (*out)[key] = val } } + if in.TLS != nil { + in, out := &in.TLS, &out.TLS + *out = new(TLSDescription) + **out = **in + } if in.InitContainersOld != nil { in, out := &in.InitContainersOld, &out.InitContainersOld *out = make([]corev1.Container, len(*in)) @@ -752,6 +757,22 @@ func (in *StandbyDescription) DeepCopy() *StandbyDescription { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *TLSDescription) DeepCopyInto(out *TLSDescription) { + *out = *in + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new TLSDescription. +func (in *TLSDescription) DeepCopy() *TLSDescription { + if in == nil { + return nil + } + out := new(TLSDescription) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *TeamsAPIConfiguration) DeepCopyInto(out *TeamsAPIConfiguration) { *out = *in diff --git a/pkg/cluster/k8sres.go b/pkg/cluster/k8sres.go index e2251a67c..84c407700 100644 --- a/pkg/cluster/k8sres.go +++ b/pkg/cluster/k8sres.go @@ -3,6 +3,7 @@ package cluster import ( "encoding/json" "fmt" + "path" "sort" "github.com/sirupsen/logrus" @@ -30,7 +31,10 @@ const ( patroniPGBinariesParameterName = "bin_dir" patroniPGParametersParameterName = "parameters" patroniPGHBAConfParameterName = "pg_hba" - localHost = "127.0.0.1/32" + + // the gid of the postgres user in the default spilo image + spiloPostgresGID = 103 + localHost = "127.0.0.1/32" ) type pgUser struct { @@ -446,6 +450,7 @@ func generatePodTemplate( podAntiAffinityTopologyKey string, additionalSecretMount string, additionalSecretMountPath string, + volumes []v1.Volume, ) (*v1.PodTemplateSpec, error) { terminateGracePeriodSeconds := terminateGracePeriod @@ -464,6 +469,7 @@ func generatePodTemplate( InitContainers: initContainers, Tolerations: *tolerationsSpec, SecurityContext: &securityContext, + Volumes: volumes, } if shmVolume != nil && *shmVolume { @@ -724,6 +730,7 @@ func (c *Cluster) generateStatefulSet(spec *acidv1.PostgresSpec) (*appsv1.Statef sidecarContainers []v1.Container podTemplate *v1.PodTemplateSpec volumeClaimTemplate *v1.PersistentVolumeClaim + volumes []v1.Volume ) // Improve me. Please. @@ -840,21 +847,76 @@ func (c *Cluster) generateStatefulSet(spec *acidv1.PostgresSpec) (*appsv1.Statef } // generate environment variables for the spilo container - spiloEnvVars := deduplicateEnvVars( - c.generateSpiloPodEnvVars(c.Postgresql.GetUID(), spiloConfiguration, &spec.Clone, - spec.StandbyCluster, customPodEnvVarsList), c.containerName(), c.logger) + spiloEnvVars := c.generateSpiloPodEnvVars( + c.Postgresql.GetUID(), + spiloConfiguration, + &spec.Clone, + spec.StandbyCluster, + customPodEnvVarsList, + ) // pickup the docker image for the spilo container effectiveDockerImage := util.Coalesce(spec.DockerImage, c.OpConfig.DockerImage) + // determine the FSGroup for the spilo pod + effectiveFSGroup := c.OpConfig.Resources.SpiloFSGroup + if spec.SpiloFSGroup != nil { + effectiveFSGroup = spec.SpiloFSGroup + } + volumeMounts := generateVolumeMounts(spec.Volume) + // configure TLS with a custom secret volume + if spec.TLS != nil && spec.TLS.SecretName != "" { + if effectiveFSGroup == nil { + c.logger.Warnf("Setting the default FSGroup to satisfy the TLS configuration") + fsGroup := int64(spiloPostgresGID) + effectiveFSGroup = &fsGroup + } + // this is combined with the FSGroup above to give read access to the + // postgres user + defaultMode := int32(0640) + volumes = append(volumes, v1.Volume{ + Name: "tls-secret", + VolumeSource: v1.VolumeSource{ + Secret: &v1.SecretVolumeSource{ + SecretName: spec.TLS.SecretName, + DefaultMode: &defaultMode, + }, + }, + }) + + mountPath := "/tls" + volumeMounts = append(volumeMounts, v1.VolumeMount{ + MountPath: mountPath, + Name: "tls-secret", + ReadOnly: true, + }) + + // use the same filenames as Secret resources by default + certFile := ensurePath(spec.TLS.CertificateFile, mountPath, "tls.crt") + privateKeyFile := ensurePath(spec.TLS.PrivateKeyFile, mountPath, "tls.key") + spiloEnvVars = append( + spiloEnvVars, + v1.EnvVar{Name: "SSL_CERTIFICATE_FILE", Value: certFile}, + v1.EnvVar{Name: "SSL_PRIVATE_KEY_FILE", Value: privateKeyFile}, + ) + + if spec.TLS.CAFile != "" { + caFile := ensurePath(spec.TLS.CAFile, mountPath, "") + spiloEnvVars = append( + spiloEnvVars, + v1.EnvVar{Name: "SSL_CA_FILE", Value: caFile}, + ) + } + } + // generate the spilo container c.logger.Debugf("Generating Spilo container, environment variables: %v", spiloEnvVars) spiloContainer := generateContainer(c.containerName(), &effectiveDockerImage, resourceRequirements, - spiloEnvVars, + deduplicateEnvVars(spiloEnvVars, c.containerName(), c.logger), volumeMounts, c.OpConfig.Resources.SpiloPrivileged, ) @@ -893,16 +955,10 @@ func (c *Cluster) generateStatefulSet(spec *acidv1.PostgresSpec) (*appsv1.Statef tolerationSpec := tolerations(&spec.Tolerations, c.OpConfig.PodToleration) effectivePodPriorityClassName := util.Coalesce(spec.PodPriorityClassName, c.OpConfig.PodPriorityClassName) - // determine the FSGroup for the spilo pod - effectiveFSGroup := c.OpConfig.Resources.SpiloFSGroup - if spec.SpiloFSGroup != nil { - effectiveFSGroup = spec.SpiloFSGroup - } - annotations := c.generatePodAnnotations(spec) // generate pod template for the statefulset, based on the spilo container and sidecars - if podTemplate, err = generatePodTemplate( + podTemplate, err = generatePodTemplate( c.Namespace, c.labelsSet(true), annotations, @@ -920,10 +976,9 @@ func (c *Cluster) generateStatefulSet(spec *acidv1.PostgresSpec) (*appsv1.Statef c.OpConfig.EnablePodAntiAffinity, c.OpConfig.PodAntiAffinityTopologyKey, c.OpConfig.AdditionalSecretMount, - c.OpConfig.AdditionalSecretMountPath); err != nil { - return nil, fmt.Errorf("could not generate pod template: %v", err) - } - + c.OpConfig.AdditionalSecretMountPath, + volumes, + ) if err != nil { return nil, fmt.Errorf("could not generate pod template: %v", err) } @@ -1539,7 +1594,8 @@ func (c *Cluster) generateLogicalBackupJob() (*batchv1beta1.CronJob, error) { false, "", c.OpConfig.AdditionalSecretMount, - c.OpConfig.AdditionalSecretMountPath); err != nil { + c.OpConfig.AdditionalSecretMountPath, + nil); err != nil { return nil, fmt.Errorf("could not generate pod template for logical backup pod: %v", err) } @@ -1671,3 +1727,13 @@ func (c *Cluster) generateLogicalBackupPodEnvVars() []v1.EnvVar { func (c *Cluster) getLogicalBackupJobName() (jobName string) { return "logical-backup-" + c.clusterName().Name } + +func ensurePath(file string, defaultDir string, defaultFile string) string { + if file == "" { + return path.Join(defaultDir, defaultFile) + } + if !path.IsAbs(file) { + return path.Join(defaultDir, file) + } + return file +} diff --git a/pkg/cluster/k8sres_test.go b/pkg/cluster/k8sres_test.go index e8fe05456..7fd4cd3e6 100644 --- a/pkg/cluster/k8sres_test.go +++ b/pkg/cluster/k8sres_test.go @@ -3,16 +3,17 @@ package cluster import ( "reflect" - v1 "k8s.io/api/core/v1" - "testing" + "github.com/stretchr/testify/assert" + acidv1 "github.com/zalando/postgres-operator/pkg/apis/acid.zalan.do/v1" "github.com/zalando/postgres-operator/pkg/util" "github.com/zalando/postgres-operator/pkg/util/config" "github.com/zalando/postgres-operator/pkg/util/constants" "github.com/zalando/postgres-operator/pkg/util/k8sutil" + v1 "k8s.io/api/core/v1" policyv1beta1 "k8s.io/api/policy/v1beta1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/intstr" @@ -451,3 +452,65 @@ func TestSecretVolume(t *testing.T) { } } } + +func TestTLS(t *testing.T) { + var err error + var spec acidv1.PostgresSpec + var cluster *Cluster + + makeSpec := func(tls acidv1.TLSDescription) acidv1.PostgresSpec { + return acidv1.PostgresSpec{ + TeamID: "myapp", NumberOfInstances: 1, + Resources: acidv1.Resources{ + ResourceRequests: acidv1.ResourceDescription{CPU: "1", Memory: "10"}, + ResourceLimits: acidv1.ResourceDescription{CPU: "1", Memory: "10"}, + }, + Volume: acidv1.Volume{ + Size: "1G", + }, + TLS: &tls, + } + } + + cluster = New( + Config{ + OpConfig: config.Config{ + PodManagementPolicy: "ordered_ready", + ProtectedRoles: []string{"admin"}, + Auth: config.Auth{ + SuperUsername: superUserName, + ReplicationUsername: replicationUserName, + }, + }, + }, k8sutil.KubernetesClient{}, acidv1.Postgresql{}, logger) + spec = makeSpec(acidv1.TLSDescription{SecretName: "my-secret", CAFile: "ca.crt"}) + s, err := cluster.generateStatefulSet(&spec) + if err != nil { + assert.NoError(t, err) + } + + fsGroup := int64(103) + assert.Equal(t, &fsGroup, s.Spec.Template.Spec.SecurityContext.FSGroup, "has a default FSGroup assigned") + + defaultMode := int32(0640) + volume := v1.Volume{ + Name: "tls-secret", + VolumeSource: v1.VolumeSource{ + Secret: &v1.SecretVolumeSource{ + SecretName: "my-secret", + DefaultMode: &defaultMode, + }, + }, + } + assert.Contains(t, s.Spec.Template.Spec.Volumes, volume, "the pod gets a secret volume") + + assert.Contains(t, s.Spec.Template.Spec.Containers[0].VolumeMounts, v1.VolumeMount{ + MountPath: "/tls", + Name: "tls-secret", + ReadOnly: true, + }, "the volume gets mounted in /tls") + + assert.Contains(t, s.Spec.Template.Spec.Containers[0].Env, v1.EnvVar{Name: "SSL_CERTIFICATE_FILE", Value: "/tls/tls.crt"}) + assert.Contains(t, s.Spec.Template.Spec.Containers[0].Env, v1.EnvVar{Name: "SSL_PRIVATE_KEY_FILE", Value: "/tls/tls.key"}) + assert.Contains(t, s.Spec.Template.Spec.Containers[0].Env, v1.EnvVar{Name: "SSL_CA_FILE", Value: "/tls/ca.crt"}) +} From b66734a0a9a94147bc99bffa6844e146b036a7f7 Mon Sep 17 00:00:00 2001 From: Felix Kunde Date: Fri, 13 Mar 2020 11:48:19 +0100 Subject: [PATCH 5/6] omit PgVersion diff on sync (#860) * use PostgresParam.PgVersion everywhere * on sync compare pgVersion with SpiloConfiguration * update getNewPgVersion and added tests --- kubectl-pg/cmd/list.go | 25 ++++--- pkg/cluster/cluster.go | 7 +- pkg/cluster/k8sres.go | 48 +++++++++++++- pkg/cluster/k8sres_test.go | 129 +++++++++++++++++++++++++++++++++++++ pkg/cluster/sync.go | 12 ++++ 5 files changed, 208 insertions(+), 13 deletions(-) diff --git a/kubectl-pg/cmd/list.go b/kubectl-pg/cmd/list.go index df827ffaf..f4dea882d 100644 --- a/kubectl-pg/cmd/list.go +++ b/kubectl-pg/cmd/list.go @@ -24,13 +24,14 @@ package cmd import ( "fmt" - "github.com/spf13/cobra" - "github.com/zalando/postgres-operator/pkg/apis/acid.zalan.do/v1" - PostgresqlLister "github.com/zalando/postgres-operator/pkg/generated/clientset/versioned/typed/acid.zalan.do/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "log" "strconv" "time" + + "github.com/spf13/cobra" + v1 "github.com/zalando/postgres-operator/pkg/apis/acid.zalan.do/v1" + PostgresqlLister "github.com/zalando/postgres-operator/pkg/generated/clientset/versioned/typed/acid.zalan.do/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) const ( @@ -95,8 +96,12 @@ func listAll(listPostgres *v1.PostgresqlList) { template := "%-32s%-16s%-12s%-12s%-12s%-12s%-12s\n" fmt.Printf(template, "NAME", "STATUS", "INSTANCES", "VERSION", "AGE", "VOLUME", "NAMESPACE") for _, pgObjs := range listPostgres.Items { - fmt.Printf(template, pgObjs.Name, pgObjs.Status.PostgresClusterStatus, strconv.Itoa(int(pgObjs.Spec.NumberOfInstances)), - pgObjs.Spec.PgVersion, time.Since(pgObjs.CreationTimestamp.Time).Truncate(TrimCreateTimestamp), pgObjs.Spec.Size, pgObjs.Namespace) + fmt.Printf(template, pgObjs.Name, + pgObjs.Status.PostgresClusterStatus, + strconv.Itoa(int(pgObjs.Spec.NumberOfInstances)), + pgObjs.Spec.PostgresqlParam.PgVersion, + time.Since(pgObjs.CreationTimestamp.Time).Truncate(TrimCreateTimestamp), + pgObjs.Spec.Size, pgObjs.Namespace) } } @@ -104,8 +109,12 @@ func listWithNamespace(listPostgres *v1.PostgresqlList) { template := "%-32s%-16s%-12s%-12s%-12s%-12s\n" fmt.Printf(template, "NAME", "STATUS", "INSTANCES", "VERSION", "AGE", "VOLUME") for _, pgObjs := range listPostgres.Items { - fmt.Printf(template, pgObjs.Name, pgObjs.Status.PostgresClusterStatus, strconv.Itoa(int(pgObjs.Spec.NumberOfInstances)), - pgObjs.Spec.PgVersion, time.Since(pgObjs.CreationTimestamp.Time).Truncate(TrimCreateTimestamp), pgObjs.Spec.Size) + fmt.Printf(template, pgObjs.Name, + pgObjs.Status.PostgresClusterStatus, + strconv.Itoa(int(pgObjs.Spec.NumberOfInstances)), + pgObjs.Spec.PostgresqlParam.PgVersion, + time.Since(pgObjs.CreationTimestamp.Time).Truncate(TrimCreateTimestamp), + pgObjs.Spec.Size) } } diff --git a/pkg/cluster/cluster.go b/pkg/cluster/cluster.go index 91e7a5195..d740260d2 100644 --- a/pkg/cluster/cluster.go +++ b/pkg/cluster/cluster.go @@ -554,10 +554,11 @@ func (c *Cluster) Update(oldSpec, newSpec *acidv1.Postgresql) error { } }() - if oldSpec.Spec.PgVersion != newSpec.Spec.PgVersion { // PG versions comparison - c.logger.Warningf("postgresql version change(%q -> %q) has no effect", oldSpec.Spec.PgVersion, newSpec.Spec.PgVersion) + if oldSpec.Spec.PostgresqlParam.PgVersion != newSpec.Spec.PostgresqlParam.PgVersion { // PG versions comparison + c.logger.Warningf("postgresql version change(%q -> %q) has no effect", + oldSpec.Spec.PostgresqlParam.PgVersion, newSpec.Spec.PostgresqlParam.PgVersion) //we need that hack to generate statefulset with the old version - newSpec.Spec.PgVersion = oldSpec.Spec.PgVersion + newSpec.Spec.PostgresqlParam.PgVersion = oldSpec.Spec.PostgresqlParam.PgVersion } // Service diff --git a/pkg/cluster/k8sres.go b/pkg/cluster/k8sres.go index 84c407700..aaa27384a 100644 --- a/pkg/cluster/k8sres.go +++ b/pkg/cluster/k8sres.go @@ -27,7 +27,7 @@ import ( ) const ( - pgBinariesLocationTemplate = "/usr/lib/postgresql/%s/bin" + pgBinariesLocationTemplate = "/usr/lib/postgresql/%v/bin" patroniPGBinariesParameterName = "bin_dir" patroniPGParametersParameterName = "parameters" patroniPGHBAConfParameterName = "pg_hba" @@ -722,6 +722,50 @@ func makeResources(cpuRequest, memoryRequest, cpuLimit, memoryLimit string) acid } } +func extractPgVersionFromBinPath(binPath string, template string) (string, error) { + var pgVersion float32 + _, err := fmt.Sscanf(binPath, template, &pgVersion) + if err != nil { + return "", err + } + return fmt.Sprintf("%v", pgVersion), nil +} + +func (c *Cluster) getNewPgVersion(container v1.Container, newPgVersion string) (string, error) { + var ( + spiloConfiguration spiloConfiguration + runningPgVersion string + err error + ) + + for _, env := range container.Env { + if env.Name != "SPILO_CONFIGURATION" { + continue + } + err = json.Unmarshal([]byte(env.Value), &spiloConfiguration) + if err != nil { + return newPgVersion, err + } + } + + if len(spiloConfiguration.PgLocalConfiguration) > 0 { + currentBinPath := fmt.Sprintf("%v", spiloConfiguration.PgLocalConfiguration[patroniPGBinariesParameterName]) + runningPgVersion, err = extractPgVersionFromBinPath(currentBinPath, pgBinariesLocationTemplate) + if err != nil { + return "", fmt.Errorf("could not extract Postgres version from %v in SPILO_CONFIGURATION", currentBinPath) + } + } else { + return "", fmt.Errorf("could not find %q setting in SPILO_CONFIGURATION", patroniPGBinariesParameterName) + } + + if runningPgVersion != newPgVersion { + c.logger.Warningf("postgresql version change(%q -> %q) has no effect", runningPgVersion, newPgVersion) + newPgVersion = runningPgVersion + } + + return newPgVersion, nil +} + func (c *Cluster) generateStatefulSet(spec *acidv1.PostgresSpec) (*appsv1.StatefulSet, error) { var ( @@ -1680,7 +1724,7 @@ func (c *Cluster) generateLogicalBackupPodEnvVars() []v1.EnvVar { // Postgres env vars { Name: "PG_VERSION", - Value: c.Spec.PgVersion, + Value: c.Spec.PostgresqlParam.PgVersion, }, { Name: "PGPORT", diff --git a/pkg/cluster/k8sres_test.go b/pkg/cluster/k8sres_test.go index 7fd4cd3e6..25e0f7af4 100644 --- a/pkg/cluster/k8sres_test.go +++ b/pkg/cluster/k8sres_test.go @@ -382,6 +382,135 @@ func TestCloneEnv(t *testing.T) { } } +func TestExtractPgVersionFromBinPath(t *testing.T) { + testName := "TestExtractPgVersionFromBinPath" + tests := []struct { + subTest string + binPath string + template string + expected string + }{ + { + subTest: "test current bin path with decimal against hard coded template", + binPath: "/usr/lib/postgresql/9.6/bin", + template: pgBinariesLocationTemplate, + expected: "9.6", + }, + { + subTest: "test current bin path against hard coded template", + binPath: "/usr/lib/postgresql/12/bin", + template: pgBinariesLocationTemplate, + expected: "12", + }, + { + subTest: "test alternative bin path against a matching template", + binPath: "/usr/pgsql-12/bin", + template: "/usr/pgsql-%v/bin", + expected: "12", + }, + } + + for _, tt := range tests { + pgVersion, err := extractPgVersionFromBinPath(tt.binPath, tt.template) + if err != nil { + t.Errorf("Unexpected error: %v", err) + } + if pgVersion != tt.expected { + t.Errorf("%s %s: Expected version %s, have %s instead", + testName, tt.subTest, tt.expected, pgVersion) + } + } +} + +func TestGetPgVersion(t *testing.T) { + testName := "TestGetPgVersion" + tests := []struct { + subTest string + pgContainer v1.Container + currentPgVersion string + newPgVersion string + }{ + { + subTest: "new version with decimal point differs from current SPILO_CONFIGURATION", + pgContainer: v1.Container{ + Name: "postgres", + Env: []v1.EnvVar{ + { + Name: "SPILO_CONFIGURATION", + Value: "{\"postgresql\": {\"bin_dir\": \"/usr/lib/postgresql/9.6/bin\"}}", + }, + }, + }, + currentPgVersion: "9.6", + newPgVersion: "12", + }, + { + subTest: "new version differs from current SPILO_CONFIGURATION", + pgContainer: v1.Container{ + Name: "postgres", + Env: []v1.EnvVar{ + { + Name: "SPILO_CONFIGURATION", + Value: "{\"postgresql\": {\"bin_dir\": \"/usr/lib/postgresql/11/bin\"}}", + }, + }, + }, + currentPgVersion: "11", + newPgVersion: "12", + }, + { + subTest: "new version is lower than the one found in current SPILO_CONFIGURATION", + pgContainer: v1.Container{ + Name: "postgres", + Env: []v1.EnvVar{ + { + Name: "SPILO_CONFIGURATION", + Value: "{\"postgresql\": {\"bin_dir\": \"/usr/lib/postgresql/12/bin\"}}", + }, + }, + }, + currentPgVersion: "12", + newPgVersion: "11", + }, + { + subTest: "new version is the same like in the current SPILO_CONFIGURATION", + pgContainer: v1.Container{ + Name: "postgres", + Env: []v1.EnvVar{ + { + Name: "SPILO_CONFIGURATION", + Value: "{\"postgresql\": {\"bin_dir\": \"/usr/lib/postgresql/12/bin\"}}", + }, + }, + }, + currentPgVersion: "12", + newPgVersion: "12", + }, + } + + var cluster = New( + Config{ + OpConfig: config.Config{ + ProtectedRoles: []string{"admin"}, + Auth: config.Auth{ + SuperUsername: superUserName, + ReplicationUsername: replicationUserName, + }, + }, + }, k8sutil.KubernetesClient{}, acidv1.Postgresql{}, logger) + + for _, tt := range tests { + pgVersion, err := cluster.getNewPgVersion(tt.pgContainer, tt.newPgVersion) + if err != nil { + t.Errorf("Unexpected error: %v", err) + } + if pgVersion != tt.currentPgVersion { + t.Errorf("%s %s: Expected version %s, have %s instead", + testName, tt.subTest, tt.currentPgVersion, pgVersion) + } + } +} + func TestSecretVolume(t *testing.T) { testName := "TestSecretVolume" tests := []struct { diff --git a/pkg/cluster/sync.go b/pkg/cluster/sync.go index 053db9ff7..b04ff863b 100644 --- a/pkg/cluster/sync.go +++ b/pkg/cluster/sync.go @@ -285,6 +285,18 @@ func (c *Cluster) syncStatefulSet() error { // statefulset is already there, make sure we use its definition in order to compare with the spec. c.Statefulset = sset + // check if there is no Postgres version mismatch + for _, container := range c.Statefulset.Spec.Template.Spec.Containers { + if container.Name != "postgres" { + continue + } + pgVersion, err := c.getNewPgVersion(container, c.Spec.PostgresqlParam.PgVersion) + if err != nil { + return fmt.Errorf("could not parse current Postgres version: %v", err) + } + c.Spec.PostgresqlParam.PgVersion = pgVersion + } + desiredSS, err := c.generateStatefulSet(&c.Spec) if err != nil { return fmt.Errorf("could not generate statefulset: %v", err) From d666c521726eaf3ceaba8ac24171f290d4277742 Mon Sep 17 00:00:00 2001 From: Dmitry Dolgov <9erthalion6@gmail.com> Date: Fri, 13 Mar 2020 11:51:39 +0100 Subject: [PATCH 6/6] ClusterDomain default (#863) * add json:omitempty option to ClusterDomain * Add default value for ClusterDomain Unfortunately, omitempty in operator configuration CRD doesn't mean that defauls from operator config object will be picked up automatically. Make sure that ClusterDomain default is specified, so that even when someone will set cluster_domain = "", it will be overwritted with a default value. Co-authored-by: mlu42 --- pkg/controller/operator_config.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pkg/controller/operator_config.go b/pkg/controller/operator_config.go index c6f10faa0..03602c3bd 100644 --- a/pkg/controller/operator_config.go +++ b/pkg/controller/operator_config.go @@ -6,6 +6,7 @@ import ( "time" acidv1 "github.com/zalando/postgres-operator/pkg/apis/acid.zalan.do/v1" + "github.com/zalando/postgres-operator/pkg/util" "github.com/zalando/postgres-operator/pkg/util/config" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -50,7 +51,7 @@ func (c *Controller) importConfigurationFromCRD(fromCRD *acidv1.OperatorConfigur result.PodTerminateGracePeriod = time.Duration(fromCRD.Kubernetes.PodTerminateGracePeriod) result.SpiloPrivileged = fromCRD.Kubernetes.SpiloPrivileged result.SpiloFSGroup = fromCRD.Kubernetes.SpiloFSGroup - result.ClusterDomain = fromCRD.Kubernetes.ClusterDomain + result.ClusterDomain = util.Coalesce(fromCRD.Kubernetes.ClusterDomain, "cluster.local") result.WatchedNamespace = fromCRD.Kubernetes.WatchedNamespace result.PDBNameFormat = fromCRD.Kubernetes.PDBNameFormat result.EnablePodDisruptionBudget = fromCRD.Kubernetes.EnablePodDisruptionBudget