From 7cf2fae6df5dfde03bd9ac87e9ab4493bab4aeef Mon Sep 17 00:00:00 2001 From: Dmitry Dolgov <9erthalion6@gmail.com> Date: Wed, 5 Aug 2020 14:18:56 +0200 Subject: [PATCH] [WIP] Extend infrastructure roles handling (#1064) Extend infrastructure roles handling Postgres Operator uses infrastructure roles to provide access to a database for external users e.g. for monitoring purposes. Such infrastructure roles are expected to be present in the form of k8s secrets with the following content: inrole1: some_encrypted_role password1: some_encrypted_password user1: some_entrypted_name inrole2: some_encrypted_role password2: some_encrypted_password user2: some_entrypted_name The format of this content is implied implicitly and not flexible enough. In case if we do not have possibility to change the format of a secret we want to use in the Operator, we need to recreate it in this format. To address this lets make the format of secret content explicitly. The idea is to introduce a new configuration option for the Operator. infrastructure_roles_secrets: - secretname: k8s_secret_name userkey: some_encrypted_name passwordkey: some_encrypted_password rolekey: some_encrypted_role - secretname: k8s_secret_name userkey: some_encrypted_name passwordkey: some_encrypted_password rolekey: some_encrypted_role This would allow Operator to use any avalable secrets to prepare infrastructure roles. To make it backward compatible simulate the old behaviour if the new option is not present. The new configuration option is intended be used mainly from CRD, but it's also available via Operator ConfigMap in a limited fashion. For ConfigMap one can put there only a string with one secret definition in the following format (as a string): infrastructure_roles_secrets: | secretname: k8s_secret_name, userkey: some_encrypted_name, passwordkey: some_encrypted_password, rolekey: some_encrypted_role Note than only one secret could be specified this way, no multiple secrets are allowed. Eventually the resulting list of infrastructure roles would be a total sum of all supported ways to describe it, namely legacy via infrastructure_roles_secret_name and infrastructure_roles_secrets from both ConfigMap and CRD. --- .../crds/operatorconfigurations.yaml | 22 + docs/reference/operator_parameters.md | 10 +- docs/user.md | 61 +- e2e/Dockerfile | 3 + e2e/exec.sh | 2 + e2e/tests/test_e2e.py | 1038 +++++++++-------- manifests/configmap.yaml | 3 +- manifests/infrastructure-roles-new.yaml | 14 + manifests/operatorconfiguration.crd.yaml | 22 + ...gresql-operator-default-configuration.yaml | 8 + pkg/apis/acid.zalan.do/v1/crds.go | 31 +- .../v1/operator_configuration_type.go | 45 +- .../acid.zalan.do/v1/zz_generated.deepcopy.go | 12 + pkg/controller/controller.go | 3 +- pkg/controller/operator_config.go | 15 + pkg/controller/util.go | 292 ++++- pkg/controller/util_test.go | 299 ++++- pkg/spec/types.go | 4 + pkg/util/config/config.go | 42 +- pkg/util/k8sutil/k8sutil.go | 70 +- 20 files changed, 1375 insertions(+), 621 deletions(-) create mode 100755 e2e/exec.sh create mode 100644 manifests/infrastructure-roles-new.yaml diff --git a/charts/postgres-operator/crds/operatorconfigurations.yaml b/charts/postgres-operator/crds/operatorconfigurations.yaml index 89f495367..3218decd7 100644 --- a/charts/postgres-operator/crds/operatorconfigurations.yaml +++ b/charts/postgres-operator/crds/operatorconfigurations.yaml @@ -131,6 +131,28 @@ spec: type: boolean infrastructure_roles_secret_name: type: string + infrastructure_roles_secrets: + type: array + nullable: true + items: + type: object + required: + - secretname + - userkey + - passwordkey + properties: + secretname: + type: string + userkey: + type: string + passwordkey: + type: string + rolekey: + type: string + details: + type: string + template: + type: boolean inherited_labels: type: array items: diff --git a/docs/reference/operator_parameters.md b/docs/reference/operator_parameters.md index 7e5196d56..20771078f 100644 --- a/docs/reference/operator_parameters.md +++ b/docs/reference/operator_parameters.md @@ -252,8 +252,14 @@ configuration they are grouped under the `kubernetes` key. teams API. The default is `postgresql-operator`. * **infrastructure_roles_secret_name** - namespaced name of the secret containing infrastructure roles names and - passwords. + *deprecated*: namespaced name of the secret containing infrastructure roles + with user names, passwords and role membership. + +* **infrastructure_roles_secrets** + array of infrastructure role definitions which reference existing secrets + and specify the key names from which user name, password and role membership + are extracted. For the ConfigMap this has to be a string which allows + referencing only one infrastructure roles secret. The default is empty. * **pod_role_label** name of the label assigned to the Postgres pods (and services/endpoints) by diff --git a/docs/user.md b/docs/user.md index 3683fdf61..a4b1424b8 100644 --- a/docs/user.md +++ b/docs/user.md @@ -150,23 +150,62 @@ user. There are two ways to define them: #### Infrastructure roles secret -The infrastructure roles secret is specified by the `infrastructure_roles_secret_name` -parameter. The role definition looks like this (values are base64 encoded): +Infrastructure roles can be specified by the `infrastructure_roles_secrets` +parameter where you can reference multiple existing secrets. Prior to `v1.6.0` +the operator could only reference one secret with the +`infrastructure_roles_secret_name` option. However, this secret could contain +multiple roles using the same set of keys plus incrementing index. ```yaml -user1: ZGJ1c2Vy -password1: c2VjcmV0 -inrole1: b3BlcmF0b3I= +apiVersion: v1 +kind: Secret +metadata: + name: postgresql-infrastructure-roles +data: + user1: ZGJ1c2Vy + password1: c2VjcmV0 + inrole1: b3BlcmF0b3I= + user2: ... ``` The block above describes the infrastructure role 'dbuser' with password -'secret' that is a member of the 'operator' role. For the following definitions -one must increase the index, i.e. the next role will be defined as 'user2' and -so on. The resulting role will automatically be a login role. +'secret' that is a member of the 'operator' role. The resulting role will +automatically be a login role. -Note that with definitions that solely use the infrastructure roles secret -there is no way to specify role options (like superuser or nologin) or role -memberships. This is where the ConfigMap comes into play. +With the new option users can configure the names of secret keys that contain +the user name, password etc. The secret itself is referenced by the +`secretname` key. If the secret uses a template for multiple roles as described +above list them separately. + +```yaml +apiVersion: v1 +kind: OperatorConfiguration +metadata: + name: postgresql-operator-configuration +configuration: + kubernetes: + infrastructure_roles_secrets: + - secretname: "postgresql-infrastructure-roles" + userkey: "user1" + passwordkey: "password1" + rolekey: "inrole1" + - secretname: "postgresql-infrastructure-roles" + userkey: "user2" + ... +``` + +Note, only the CRD-based configuration allows for referencing multiple secrets. +As of now, the ConfigMap is restricted to either one or the existing template +option with `infrastructure_roles_secret_name`. Please, refer to the example +manifests to understand how `infrastructure_roles_secrets` has to be configured +for the [configmap](../manifests/configmap.yaml) or [CRD configuration](../manifests/postgresql-operator-default-configuration.yaml). + +If both `infrastructure_roles_secret_name` and `infrastructure_roles_secrets` +are defined the operator will create roles for both of them. So make sure, +they do not collide. Note also, that with definitions that solely use the +infrastructure roles secret there is no way to specify role options (like +superuser or nologin) or role memberships. This is where the additional +ConfigMap comes into play. #### Secret plus ConfigMap diff --git a/e2e/Dockerfile b/e2e/Dockerfile index 236942d04..a250ea9cb 100644 --- a/e2e/Dockerfile +++ b/e2e/Dockerfile @@ -1,7 +1,10 @@ +# An image to perform the actual test. Do not forget to copy all necessary test +# files here. FROM ubuntu:18.04 LABEL maintainer="Team ACID @ Zalando " COPY manifests ./manifests +COPY exec.sh ./exec.sh COPY requirements.txt tests ./ RUN apt-get update \ diff --git a/e2e/exec.sh b/e2e/exec.sh new file mode 100755 index 000000000..56276bc3c --- /dev/null +++ b/e2e/exec.sh @@ -0,0 +1,2 @@ +#!/usr/bin/env bash +kubectl exec -it $1 -- sh -c "$2" diff --git a/e2e/tests/test_e2e.py b/e2e/tests/test_e2e.py index 18b9852c4..4cd1c6a30 100644 --- a/e2e/tests/test_e2e.py +++ b/e2e/tests/test_e2e.py @@ -1,3 +1,4 @@ +import json import unittest import time import timeout_decorator @@ -50,7 +51,8 @@ class EndToEndTestCase(unittest.TestCase): for filename in ["operator-service-account-rbac.yaml", "configmap.yaml", - "postgres-operator.yaml"]: + "postgres-operator.yaml", + "infrastructure-roles-new.yaml"]: result = k8s.create_with_kubectl("manifests/" + filename) print("stdout: {}, stderr: {}".format(result.stdout, result.stderr)) @@ -69,507 +71,548 @@ class EndToEndTestCase(unittest.TestCase): print('Operator log: {}'.format(k8s.get_operator_log())) raise + # @timeout_decorator.timeout(TEST_TIMEOUT_SEC) + # def test_enable_disable_connection_pooler(self): + # ''' + # For a database without connection pooler, then turns it on, scale up, + # turn off and on again. Test with different ways of doing this (via + # enableConnectionPooler or connectionPooler configuration section). At + # the end turn connection pooler off to not interfere with other tests. + # ''' + # k8s = self.k8s + # service_labels = { + # 'cluster-name': 'acid-minimal-cluster', + # } + # pod_labels = dict({ + # 'connection-pooler': 'acid-minimal-cluster-pooler', + # }) + + # pod_selector = to_selector(pod_labels) + # service_selector = to_selector(service_labels) + + # try: + # # enable connection pooler + # k8s.api.custom_objects_api.patch_namespaced_custom_object( + # 'acid.zalan.do', 'v1', 'default', + # 'postgresqls', 'acid-minimal-cluster', + # { + # 'spec': { + # 'enableConnectionPooler': True, + # } + # }) + # k8s.wait_for_pod_start(pod_selector) + + # pods = k8s.api.core_v1.list_namespaced_pod( + # 'default', label_selector=pod_selector + # ).items + + # self.assertTrue(pods, 'No connection pooler pods') + + # k8s.wait_for_service(service_selector) + # services = k8s.api.core_v1.list_namespaced_service( + # 'default', label_selector=service_selector + # ).items + # services = [ + # s for s in services + # if s.metadata.name.endswith('pooler') + # ] + + # self.assertTrue(services, 'No connection pooler service') + + # # scale up connection pooler deployment + # k8s.api.custom_objects_api.patch_namespaced_custom_object( + # 'acid.zalan.do', 'v1', 'default', + # 'postgresqls', 'acid-minimal-cluster', + # { + # 'spec': { + # 'connectionPooler': { + # 'numberOfInstances': 2, + # }, + # } + # }) + + # k8s.wait_for_running_pods(pod_selector, 2) + + # # turn it off, keeping configuration section + # k8s.api.custom_objects_api.patch_namespaced_custom_object( + # 'acid.zalan.do', 'v1', 'default', + # 'postgresqls', 'acid-minimal-cluster', + # { + # 'spec': { + # 'enableConnectionPooler': False, + # } + # }) + # k8s.wait_for_pods_to_stop(pod_selector) + + # except timeout_decorator.TimeoutError: + # print('Operator log: {}'.format(k8s.get_operator_log())) + # raise + + # @timeout_decorator.timeout(TEST_TIMEOUT_SEC) + # def test_enable_load_balancer(self): + # ''' + # Test if services are updated when enabling/disabling load balancers + # ''' + + # k8s = self.k8s + # cluster_label = 'application=spilo,cluster-name=acid-minimal-cluster' + + # # enable load balancer services + # pg_patch_enable_lbs = { + # "spec": { + # "enableMasterLoadBalancer": True, + # "enableReplicaLoadBalancer": True + # } + # } + # k8s.api.custom_objects_api.patch_namespaced_custom_object( + # "acid.zalan.do", "v1", "default", "postgresqls", "acid-minimal-cluster", pg_patch_enable_lbs) + # # wait for service recreation + # time.sleep(60) + + # master_svc_type = k8s.get_service_type(cluster_label + ',spilo-role=master') + # self.assertEqual(master_svc_type, 'LoadBalancer', + # "Expected LoadBalancer service type for master, found {}".format(master_svc_type)) + + # repl_svc_type = k8s.get_service_type(cluster_label + ',spilo-role=replica') + # self.assertEqual(repl_svc_type, 'LoadBalancer', + # "Expected LoadBalancer service type for replica, found {}".format(repl_svc_type)) + + # # disable load balancer services again + # pg_patch_disable_lbs = { + # "spec": { + # "enableMasterLoadBalancer": False, + # "enableReplicaLoadBalancer": False + # } + # } + # k8s.api.custom_objects_api.patch_namespaced_custom_object( + # "acid.zalan.do", "v1", "default", "postgresqls", "acid-minimal-cluster", pg_patch_disable_lbs) + # # wait for service recreation + # time.sleep(60) + + # master_svc_type = k8s.get_service_type(cluster_label + ',spilo-role=master') + # self.assertEqual(master_svc_type, 'ClusterIP', + # "Expected ClusterIP service type for master, found {}".format(master_svc_type)) + + # repl_svc_type = k8s.get_service_type(cluster_label + ',spilo-role=replica') + # self.assertEqual(repl_svc_type, 'ClusterIP', + # "Expected ClusterIP service type for replica, found {}".format(repl_svc_type)) + + # @timeout_decorator.timeout(TEST_TIMEOUT_SEC) + # def test_lazy_spilo_upgrade(self): + # ''' + # Test lazy upgrade for the Spilo image: operator changes a stateful set but lets pods run with the old image + # until they are recreated for reasons other than operator's activity. That works because the operator configures + # stateful sets to use "onDelete" pod update policy. + + # The test covers: + # 1) enabling lazy upgrade in existing operator deployment + # 2) forcing the normal rolling upgrade by changing the operator configmap and restarting its pod + # ''' + + # k8s = self.k8s + + # # update docker image in config and enable the lazy upgrade + # conf_image = "registry.opensource.zalan.do/acid/spilo-cdp-12:1.6-p114" + # patch_lazy_spilo_upgrade = { + # "data": { + # "docker_image": conf_image, + # "enable_lazy_spilo_upgrade": "true" + # } + # } + # k8s.update_config(patch_lazy_spilo_upgrade) + + # pod0 = 'acid-minimal-cluster-0' + # pod1 = 'acid-minimal-cluster-1' + + # # restart the pod to get a container with the new image + # k8s.api.core_v1.delete_namespaced_pod(pod0, 'default') + # time.sleep(60) + + # # lazy update works if the restarted pod and older pods run different Spilo versions + # new_image = k8s.get_effective_pod_image(pod0) + # old_image = k8s.get_effective_pod_image(pod1) + # self.assertNotEqual(new_image, old_image, "Lazy updated failed: pods have the same image {}".format(new_image)) + + # # sanity check + # assert_msg = "Image {} of a new pod differs from {} in operator conf".format(new_image, conf_image) + # self.assertEqual(new_image, conf_image, assert_msg) + + # # clean up + # unpatch_lazy_spilo_upgrade = { + # "data": { + # "enable_lazy_spilo_upgrade": "false", + # } + # } + # k8s.update_config(unpatch_lazy_spilo_upgrade) + + # # at this point operator will complete the normal rolling upgrade + # # so we additonally test if disabling the lazy upgrade - forcing the normal rolling upgrade - works + + # # XXX there is no easy way to wait until the end of Sync() + # time.sleep(60) + + # image0 = k8s.get_effective_pod_image(pod0) + # image1 = k8s.get_effective_pod_image(pod1) + + # assert_msg = "Disabling lazy upgrade failed: pods still have different images {} and {}".format(image0, image1) + # self.assertEqual(image0, image1, assert_msg) + + # @timeout_decorator.timeout(TEST_TIMEOUT_SEC) + # def test_logical_backup_cron_job(self): + # ''' + # Ensure we can (a) create the cron job at user request for a specific PG cluster + # (b) update the cluster-wide image for the logical backup pod + # (c) delete the job at user request + + # Limitations: + # (a) Does not run the actual batch job because there is no S3 mock to upload backups to + # (b) Assumes 'acid-minimal-cluster' exists as defined in setUp + # ''' + + # k8s = self.k8s + + # # create the cron job + # schedule = "7 7 7 7 *" + # pg_patch_enable_backup = { + # "spec": { + # "enableLogicalBackup": True, + # "logicalBackupSchedule": schedule + # } + # } + # k8s.api.custom_objects_api.patch_namespaced_custom_object( + # "acid.zalan.do", "v1", "default", "postgresqls", "acid-minimal-cluster", pg_patch_enable_backup) + # k8s.wait_for_logical_backup_job_creation() + + # jobs = k8s.get_logical_backup_job().items + # self.assertEqual(1, len(jobs), "Expected 1 logical backup job, found {}".format(len(jobs))) + + # job = jobs[0] + # self.assertEqual(job.metadata.name, "logical-backup-acid-minimal-cluster", + # "Expected job name {}, found {}" + # .format("logical-backup-acid-minimal-cluster", job.metadata.name)) + # self.assertEqual(job.spec.schedule, schedule, + # "Expected {} schedule, found {}" + # .format(schedule, job.spec.schedule)) + + # # update the cluster-wide image of the logical backup pod + # image = "test-image-name" + # patch_logical_backup_image = { + # "data": { + # "logical_backup_docker_image": image, + # } + # } + # k8s.update_config(patch_logical_backup_image) + + # jobs = k8s.get_logical_backup_job().items + # actual_image = jobs[0].spec.job_template.spec.template.spec.containers[0].image + # self.assertEqual(actual_image, image, + # "Expected job image {}, found {}".format(image, actual_image)) + + # # delete the logical backup cron job + # pg_patch_disable_backup = { + # "spec": { + # "enableLogicalBackup": False, + # } + # } + # k8s.api.custom_objects_api.patch_namespaced_custom_object( + # "acid.zalan.do", "v1", "default", "postgresqls", "acid-minimal-cluster", pg_patch_disable_backup) + # k8s.wait_for_logical_backup_job_deletion() + # jobs = k8s.get_logical_backup_job().items + # self.assertEqual(0, len(jobs), + # "Expected 0 logical backup jobs, found {}".format(len(jobs))) + + # @timeout_decorator.timeout(TEST_TIMEOUT_SEC) + # def test_min_resource_limits(self): + # ''' + # Lower resource limits below configured minimum and let operator fix it + # ''' + # k8s = self.k8s + # cluster_label = 'application=spilo,cluster-name=acid-minimal-cluster' + # labels = 'spilo-role=master,' + cluster_label + # _, failover_targets = k8s.get_pg_nodes(cluster_label) + + # # configure minimum boundaries for CPU and memory limits + # minCPULimit = '500m' + # minMemoryLimit = '500Mi' + # patch_min_resource_limits = { + # "data": { + # "min_cpu_limit": minCPULimit, + # "min_memory_limit": minMemoryLimit + # } + # } + # k8s.update_config(patch_min_resource_limits) + + # # lower resource limits below minimum + # pg_patch_resources = { + # "spec": { + # "resources": { + # "requests": { + # "cpu": "10m", + # "memory": "50Mi" + # }, + # "limits": { + # "cpu": "200m", + # "memory": "200Mi" + # } + # } + # } + # } + # k8s.api.custom_objects_api.patch_namespaced_custom_object( + # "acid.zalan.do", "v1", "default", "postgresqls", "acid-minimal-cluster", pg_patch_resources) + # k8s.wait_for_pod_failover(failover_targets, labels) + # k8s.wait_for_pod_start('spilo-role=replica') + + # pods = k8s.api.core_v1.list_namespaced_pod( + # 'default', label_selector=labels).items + # self.assert_master_is_unique() + # masterPod = pods[0] + + # self.assertEqual(masterPod.spec.containers[0].resources.limits['cpu'], minCPULimit, + # "Expected CPU limit {}, found {}" + # .format(minCPULimit, masterPod.spec.containers[0].resources.limits['cpu'])) + # self.assertEqual(masterPod.spec.containers[0].resources.limits['memory'], minMemoryLimit, + # "Expected memory limit {}, found {}" + # .format(minMemoryLimit, masterPod.spec.containers[0].resources.limits['memory'])) + + # @timeout_decorator.timeout(TEST_TIMEOUT_SEC) + # def test_multi_namespace_support(self): + # ''' + # Create a customized Postgres cluster in a non-default namespace. + # ''' + # k8s = self.k8s + + # with open("manifests/complete-postgres-manifest.yaml", 'r+') as f: + # pg_manifest = yaml.safe_load(f) + # pg_manifest["metadata"]["namespace"] = self.namespace + # yaml.dump(pg_manifest, f, Dumper=yaml.Dumper) + + # k8s.create_with_kubectl("manifests/complete-postgres-manifest.yaml") + # k8s.wait_for_pod_start("spilo-role=master", self.namespace) + # self.assert_master_is_unique(self.namespace, "acid-test-cluster") + + # @timeout_decorator.timeout(TEST_TIMEOUT_SEC) + # def test_node_readiness_label(self): + # ''' + # Remove node readiness label from master node. This must cause a failover. + # ''' + # k8s = self.k8s + # cluster_label = 'application=spilo,cluster-name=acid-minimal-cluster' + # readiness_label = 'lifecycle-status' + # readiness_value = 'ready' + + # # get nodes of master and replica(s) (expected target of new master) + # current_master_node, current_replica_nodes = k8s.get_pg_nodes(cluster_label) + # num_replicas = len(current_replica_nodes) + # failover_targets = self.get_failover_targets(current_master_node, current_replica_nodes) + + # # add node_readiness_label to potential failover nodes + # patch_readiness_label = { + # "metadata": { + # "labels": { + # readiness_label: readiness_value + # } + # } + # } + # for failover_target in failover_targets: + # k8s.api.core_v1.patch_node(failover_target, patch_readiness_label) + + # # define node_readiness_label in config map which should trigger a failover of the master + # patch_readiness_label_config = { + # "data": { + # "node_readiness_label": readiness_label + ':' + readiness_value, + # } + # } + # k8s.update_config(patch_readiness_label_config) + # new_master_node, new_replica_nodes = self.assert_failover( + # current_master_node, num_replicas, failover_targets, cluster_label) + + # # patch also node where master ran before + # k8s.api.core_v1.patch_node(current_master_node, patch_readiness_label) + + # # wait a little before proceeding with the pod distribution test + # time.sleep(30) + + # # toggle pod anti affinity to move replica away from master node + # self.assert_distributed_pods(new_master_node, new_replica_nodes, cluster_label) + + # @timeout_decorator.timeout(TEST_TIMEOUT_SEC) + # def test_scaling(self): + # ''' + # Scale up from 2 to 3 and back to 2 pods by updating the Postgres manifest at runtime. + # ''' + # k8s = self.k8s + # labels = "application=spilo,cluster-name=acid-minimal-cluster" + + # k8s.wait_for_pg_to_scale(3) + # self.assertEqual(3, k8s.count_pods_with_label(labels)) + # self.assert_master_is_unique() + + # k8s.wait_for_pg_to_scale(2) + # self.assertEqual(2, k8s.count_pods_with_label(labels)) + # self.assert_master_is_unique() + + # @timeout_decorator.timeout(TEST_TIMEOUT_SEC) + # def test_service_annotations(self): + # ''' + # Create a Postgres cluster with service annotations and check them. + # ''' + # k8s = self.k8s + # patch_custom_service_annotations = { + # "data": { + # "custom_service_annotations": "foo:bar", + # } + # } + # k8s.update_config(patch_custom_service_annotations) + + # pg_patch_custom_annotations = { + # "spec": { + # "serviceAnnotations": { + # "annotation.key": "value", + # "foo": "bar", + # } + # } + # } + # k8s.api.custom_objects_api.patch_namespaced_custom_object( + # "acid.zalan.do", "v1", "default", "postgresqls", "acid-minimal-cluster", pg_patch_custom_annotations) + + # # wait a little before proceeding + # time.sleep(30) + # annotations = { + # "annotation.key": "value", + # "foo": "bar", + # } + # self.assertTrue(k8s.check_service_annotations( + # "cluster-name=acid-minimal-cluster,spilo-role=master", annotations)) + # self.assertTrue(k8s.check_service_annotations( + # "cluster-name=acid-minimal-cluster,spilo-role=replica", annotations)) + + # # clean up + # unpatch_custom_service_annotations = { + # "data": { + # "custom_service_annotations": "", + # } + # } + # k8s.update_config(unpatch_custom_service_annotations) + + # @timeout_decorator.timeout(TEST_TIMEOUT_SEC) + # def test_statefulset_annotation_propagation(self): + # ''' + # Inject annotation to Postgresql CRD and check it's propagation to stateful set + # ''' + # k8s = self.k8s + # cluster_label = 'application=spilo,cluster-name=acid-minimal-cluster' + + # patch_sset_propagate_annotations = { + # "data": { + # "downscaler_annotations": "deployment-time,downscaler/*", + # } + # } + # k8s.update_config(patch_sset_propagate_annotations) + + # pg_crd_annotations = { + # "metadata": { + # "annotations": { + # "deployment-time": "2020-04-30 12:00:00", + # "downscaler/downtime_replicas": "0", + # }, + # } + # } + # k8s.api.custom_objects_api.patch_namespaced_custom_object( + # "acid.zalan.do", "v1", "default", "postgresqls", "acid-minimal-cluster", pg_crd_annotations) + + # # wait a little before proceeding + # time.sleep(60) + # annotations = { + # "deployment-time": "2020-04-30 12:00:00", + # "downscaler/downtime_replicas": "0", + # } + # self.assertTrue(k8s.check_statefulset_annotations(cluster_label, annotations)) + + # @timeout_decorator.timeout(TEST_TIMEOUT_SEC) + # def test_taint_based_eviction(self): + # ''' + # Add taint "postgres=:NoExecute" to node with master. This must cause a failover. + # ''' + # k8s = self.k8s + # cluster_label = 'application=spilo,cluster-name=acid-minimal-cluster' + + # # get nodes of master and replica(s) (expected target of new master) + # current_master_node, current_replica_nodes = k8s.get_pg_nodes(cluster_label) + # num_replicas = len(current_replica_nodes) + # failover_targets = self.get_failover_targets(current_master_node, current_replica_nodes) + + # # taint node with postgres=:NoExecute to force failover + # body = { + # "spec": { + # "taints": [ + # { + # "effect": "NoExecute", + # "key": "postgres" + # } + # ] + # } + # } + + # # patch node and test if master is failing over to one of the expected nodes + # k8s.api.core_v1.patch_node(current_master_node, body) + # new_master_node, new_replica_nodes = self.assert_failover( + # current_master_node, num_replicas, failover_targets, cluster_label) + + # # add toleration to pods + # patch_toleration_config = { + # "data": { + # "toleration": "key:postgres,operator:Exists,effect:NoExecute" + # } + # } + # k8s.update_config(patch_toleration_config) + + # # wait a little before proceeding with the pod distribution test + # time.sleep(30) + + # # toggle pod anti affinity to move replica away from master node + # self.assert_distributed_pods(new_master_node, new_replica_nodes, cluster_label) + @timeout_decorator.timeout(TEST_TIMEOUT_SEC) - def test_enable_disable_connection_pooler(self): + def test_infrastructure_roles(self): ''' - For a database without connection pooler, then turns it on, scale up, - turn off and on again. Test with different ways of doing this (via - enableConnectionPooler or connectionPooler configuration section). At - the end turn connection pooler off to not interfere with other tests. + Test using external secrets for infrastructure roles ''' k8s = self.k8s - service_labels = { - 'cluster-name': 'acid-minimal-cluster', + # update infrastructure roles description + secret_name = "postgresql-infrastructure-roles-old" + roles = "secretname: postgresql-infrastructure-roles-new, userkey: user, rolekey: role, passwordkey: password" + patch_infrastructure_roles = { + "data": { + "infrastructure_roles_secret_name": secret_name, + "infrastructure_roles_secrets": roles, + }, } - pod_labels = dict({ - 'connection-pooler': 'acid-minimal-cluster-pooler', + k8s.update_config(patch_infrastructure_roles) + + # wait a little before proceeding + time.sleep(30) + + # check that new roles are represented in the config by requesting the + # operator configuration via API + operator_pod = k8s.get_operator_pod() + get_config_cmd = "wget --quiet -O - localhost:8080/config" + result = k8s.exec_with_kubectl(operator_pod.metadata.name, get_config_cmd) + roles_dict = (json.loads(result.stdout) + .get("controller", {}) + .get("InfrastructureRoles")) + + self.assertTrue("robot_zmon_acid_monitoring_new" in roles_dict) + role = roles_dict["robot_zmon_acid_monitoring_new"] + role.pop("Password", None) + self.assertDictEqual(role, { + "Name": "robot_zmon_acid_monitoring_new", + "Flags": None, + "MemberOf": ["robot_zmon_new"], + "Parameters": None, + "AdminRole": "", + "Origin": 2, }) - pod_selector = to_selector(pod_labels) - service_selector = to_selector(service_labels) - - try: - # enable connection pooler - k8s.api.custom_objects_api.patch_namespaced_custom_object( - 'acid.zalan.do', 'v1', 'default', - 'postgresqls', 'acid-minimal-cluster', - { - 'spec': { - 'enableConnectionPooler': True, - } - }) - k8s.wait_for_pod_start(pod_selector) - - pods = k8s.api.core_v1.list_namespaced_pod( - 'default', label_selector=pod_selector - ).items - - self.assertTrue(pods, 'No connection pooler pods') - - k8s.wait_for_service(service_selector) - services = k8s.api.core_v1.list_namespaced_service( - 'default', label_selector=service_selector - ).items - services = [ - s for s in services - if s.metadata.name.endswith('pooler') - ] - - self.assertTrue(services, 'No connection pooler service') - - # scale up connection pooler deployment - k8s.api.custom_objects_api.patch_namespaced_custom_object( - 'acid.zalan.do', 'v1', 'default', - 'postgresqls', 'acid-minimal-cluster', - { - 'spec': { - 'connectionPooler': { - 'numberOfInstances': 2, - }, - } - }) - - k8s.wait_for_running_pods(pod_selector, 2) - - # turn it off, keeping configuration section - k8s.api.custom_objects_api.patch_namespaced_custom_object( - 'acid.zalan.do', 'v1', 'default', - 'postgresqls', 'acid-minimal-cluster', - { - 'spec': { - 'enableConnectionPooler': False, - } - }) - k8s.wait_for_pods_to_stop(pod_selector) - - except timeout_decorator.TimeoutError: - print('Operator log: {}'.format(k8s.get_operator_log())) - raise - - @timeout_decorator.timeout(TEST_TIMEOUT_SEC) - def test_enable_load_balancer(self): - ''' - Test if services are updated when enabling/disabling load balancers - ''' - - k8s = self.k8s - cluster_label = 'application=spilo,cluster-name=acid-minimal-cluster' - - # enable load balancer services - pg_patch_enable_lbs = { - "spec": { - "enableMasterLoadBalancer": True, - "enableReplicaLoadBalancer": True - } - } - k8s.api.custom_objects_api.patch_namespaced_custom_object( - "acid.zalan.do", "v1", "default", "postgresqls", "acid-minimal-cluster", pg_patch_enable_lbs) - # wait for service recreation - time.sleep(60) - - master_svc_type = k8s.get_service_type(cluster_label + ',spilo-role=master') - self.assertEqual(master_svc_type, 'LoadBalancer', - "Expected LoadBalancer service type for master, found {}".format(master_svc_type)) - - repl_svc_type = k8s.get_service_type(cluster_label + ',spilo-role=replica') - self.assertEqual(repl_svc_type, 'LoadBalancer', - "Expected LoadBalancer service type for replica, found {}".format(repl_svc_type)) - - # disable load balancer services again - pg_patch_disable_lbs = { - "spec": { - "enableMasterLoadBalancer": False, - "enableReplicaLoadBalancer": False - } - } - k8s.api.custom_objects_api.patch_namespaced_custom_object( - "acid.zalan.do", "v1", "default", "postgresqls", "acid-minimal-cluster", pg_patch_disable_lbs) - # wait for service recreation - time.sleep(60) - - master_svc_type = k8s.get_service_type(cluster_label + ',spilo-role=master') - self.assertEqual(master_svc_type, 'ClusterIP', - "Expected ClusterIP service type for master, found {}".format(master_svc_type)) - - repl_svc_type = k8s.get_service_type(cluster_label + ',spilo-role=replica') - self.assertEqual(repl_svc_type, 'ClusterIP', - "Expected ClusterIP service type for replica, found {}".format(repl_svc_type)) - - @timeout_decorator.timeout(TEST_TIMEOUT_SEC) - def test_lazy_spilo_upgrade(self): - ''' - Test lazy upgrade for the Spilo image: operator changes a stateful set but lets pods run with the old image - until they are recreated for reasons other than operator's activity. That works because the operator configures - stateful sets to use "onDelete" pod update policy. - - The test covers: - 1) enabling lazy upgrade in existing operator deployment - 2) forcing the normal rolling upgrade by changing the operator configmap and restarting its pod - ''' - - k8s = self.k8s - - # update docker image in config and enable the lazy upgrade - conf_image = "registry.opensource.zalan.do/acid/spilo-cdp-12:1.6-p114" - patch_lazy_spilo_upgrade = { - "data": { - "docker_image": conf_image, - "enable_lazy_spilo_upgrade": "true" - } - } - k8s.update_config(patch_lazy_spilo_upgrade) - - pod0 = 'acid-minimal-cluster-0' - pod1 = 'acid-minimal-cluster-1' - - # restart the pod to get a container with the new image - k8s.api.core_v1.delete_namespaced_pod(pod0, 'default') - time.sleep(60) - - # lazy update works if the restarted pod and older pods run different Spilo versions - new_image = k8s.get_effective_pod_image(pod0) - old_image = k8s.get_effective_pod_image(pod1) - self.assertNotEqual(new_image, old_image, "Lazy updated failed: pods have the same image {}".format(new_image)) - - # sanity check - assert_msg = "Image {} of a new pod differs from {} in operator conf".format(new_image, conf_image) - self.assertEqual(new_image, conf_image, assert_msg) - - # clean up - unpatch_lazy_spilo_upgrade = { - "data": { - "enable_lazy_spilo_upgrade": "false", - } - } - k8s.update_config(unpatch_lazy_spilo_upgrade) - - # at this point operator will complete the normal rolling upgrade - # so we additonally test if disabling the lazy upgrade - forcing the normal rolling upgrade - works - - # XXX there is no easy way to wait until the end of Sync() - time.sleep(60) - - image0 = k8s.get_effective_pod_image(pod0) - image1 = k8s.get_effective_pod_image(pod1) - - assert_msg = "Disabling lazy upgrade failed: pods still have different images {} and {}".format(image0, image1) - self.assertEqual(image0, image1, assert_msg) - - @timeout_decorator.timeout(TEST_TIMEOUT_SEC) - def test_logical_backup_cron_job(self): - ''' - Ensure we can (a) create the cron job at user request for a specific PG cluster - (b) update the cluster-wide image for the logical backup pod - (c) delete the job at user request - - Limitations: - (a) Does not run the actual batch job because there is no S3 mock to upload backups to - (b) Assumes 'acid-minimal-cluster' exists as defined in setUp - ''' - - k8s = self.k8s - - # create the cron job - schedule = "7 7 7 7 *" - pg_patch_enable_backup = { - "spec": { - "enableLogicalBackup": True, - "logicalBackupSchedule": schedule - } - } - k8s.api.custom_objects_api.patch_namespaced_custom_object( - "acid.zalan.do", "v1", "default", "postgresqls", "acid-minimal-cluster", pg_patch_enable_backup) - k8s.wait_for_logical_backup_job_creation() - - jobs = k8s.get_logical_backup_job().items - self.assertEqual(1, len(jobs), "Expected 1 logical backup job, found {}".format(len(jobs))) - - job = jobs[0] - self.assertEqual(job.metadata.name, "logical-backup-acid-minimal-cluster", - "Expected job name {}, found {}" - .format("logical-backup-acid-minimal-cluster", job.metadata.name)) - self.assertEqual(job.spec.schedule, schedule, - "Expected {} schedule, found {}" - .format(schedule, job.spec.schedule)) - - # update the cluster-wide image of the logical backup pod - image = "test-image-name" - patch_logical_backup_image = { - "data": { - "logical_backup_docker_image": image, - } - } - k8s.update_config(patch_logical_backup_image) - - jobs = k8s.get_logical_backup_job().items - actual_image = jobs[0].spec.job_template.spec.template.spec.containers[0].image - self.assertEqual(actual_image, image, - "Expected job image {}, found {}".format(image, actual_image)) - - # delete the logical backup cron job - pg_patch_disable_backup = { - "spec": { - "enableLogicalBackup": False, - } - } - k8s.api.custom_objects_api.patch_namespaced_custom_object( - "acid.zalan.do", "v1", "default", "postgresqls", "acid-minimal-cluster", pg_patch_disable_backup) - k8s.wait_for_logical_backup_job_deletion() - jobs = k8s.get_logical_backup_job().items - self.assertEqual(0, len(jobs), - "Expected 0 logical backup jobs, found {}".format(len(jobs))) - - @timeout_decorator.timeout(TEST_TIMEOUT_SEC) - def test_min_resource_limits(self): - ''' - Lower resource limits below configured minimum and let operator fix it - ''' - k8s = self.k8s - cluster_label = 'application=spilo,cluster-name=acid-minimal-cluster' - labels = 'spilo-role=master,' + cluster_label - _, failover_targets = k8s.get_pg_nodes(cluster_label) - - # configure minimum boundaries for CPU and memory limits - minCPULimit = '500m' - minMemoryLimit = '500Mi' - patch_min_resource_limits = { - "data": { - "min_cpu_limit": minCPULimit, - "min_memory_limit": minMemoryLimit - } - } - k8s.update_config(patch_min_resource_limits) - - # lower resource limits below minimum - pg_patch_resources = { - "spec": { - "resources": { - "requests": { - "cpu": "10m", - "memory": "50Mi" - }, - "limits": { - "cpu": "200m", - "memory": "200Mi" - } - } - } - } - k8s.api.custom_objects_api.patch_namespaced_custom_object( - "acid.zalan.do", "v1", "default", "postgresqls", "acid-minimal-cluster", pg_patch_resources) - k8s.wait_for_pod_failover(failover_targets, labels) - k8s.wait_for_pod_start('spilo-role=replica') - - pods = k8s.api.core_v1.list_namespaced_pod( - 'default', label_selector=labels).items - self.assert_master_is_unique() - masterPod = pods[0] - - self.assertEqual(masterPod.spec.containers[0].resources.limits['cpu'], minCPULimit, - "Expected CPU limit {}, found {}" - .format(minCPULimit, masterPod.spec.containers[0].resources.limits['cpu'])) - self.assertEqual(masterPod.spec.containers[0].resources.limits['memory'], minMemoryLimit, - "Expected memory limit {}, found {}" - .format(minMemoryLimit, masterPod.spec.containers[0].resources.limits['memory'])) - - @timeout_decorator.timeout(TEST_TIMEOUT_SEC) - def test_multi_namespace_support(self): - ''' - Create a customized Postgres cluster in a non-default namespace. - ''' - k8s = self.k8s - - with open("manifests/complete-postgres-manifest.yaml", 'r+') as f: - pg_manifest = yaml.safe_load(f) - pg_manifest["metadata"]["namespace"] = self.namespace - yaml.dump(pg_manifest, f, Dumper=yaml.Dumper) - - k8s.create_with_kubectl("manifests/complete-postgres-manifest.yaml") - k8s.wait_for_pod_start("spilo-role=master", self.namespace) - self.assert_master_is_unique(self.namespace, "acid-test-cluster") - - @timeout_decorator.timeout(TEST_TIMEOUT_SEC) - def test_node_readiness_label(self): - ''' - Remove node readiness label from master node. This must cause a failover. - ''' - k8s = self.k8s - cluster_label = 'application=spilo,cluster-name=acid-minimal-cluster' - readiness_label = 'lifecycle-status' - readiness_value = 'ready' - - # get nodes of master and replica(s) (expected target of new master) - current_master_node, current_replica_nodes = k8s.get_pg_nodes(cluster_label) - num_replicas = len(current_replica_nodes) - failover_targets = self.get_failover_targets(current_master_node, current_replica_nodes) - - # add node_readiness_label to potential failover nodes - patch_readiness_label = { - "metadata": { - "labels": { - readiness_label: readiness_value - } - } - } - for failover_target in failover_targets: - k8s.api.core_v1.patch_node(failover_target, patch_readiness_label) - - # define node_readiness_label in config map which should trigger a failover of the master - patch_readiness_label_config = { - "data": { - "node_readiness_label": readiness_label + ':' + readiness_value, - } - } - k8s.update_config(patch_readiness_label_config) - new_master_node, new_replica_nodes = self.assert_failover( - current_master_node, num_replicas, failover_targets, cluster_label) - - # patch also node where master ran before - k8s.api.core_v1.patch_node(current_master_node, patch_readiness_label) - - # wait a little before proceeding with the pod distribution test - time.sleep(30) - - # toggle pod anti affinity to move replica away from master node - self.assert_distributed_pods(new_master_node, new_replica_nodes, cluster_label) - - @timeout_decorator.timeout(TEST_TIMEOUT_SEC) - def test_scaling(self): - ''' - Scale up from 2 to 3 and back to 2 pods by updating the Postgres manifest at runtime. - ''' - k8s = self.k8s - labels = "application=spilo,cluster-name=acid-minimal-cluster" - - k8s.wait_for_pg_to_scale(3) - self.assertEqual(3, k8s.count_pods_with_label(labels)) - self.assert_master_is_unique() - - k8s.wait_for_pg_to_scale(2) - self.assertEqual(2, k8s.count_pods_with_label(labels)) - self.assert_master_is_unique() - - @timeout_decorator.timeout(TEST_TIMEOUT_SEC) - def test_service_annotations(self): - ''' - Create a Postgres cluster with service annotations and check them. - ''' - k8s = self.k8s - patch_custom_service_annotations = { - "data": { - "custom_service_annotations": "foo:bar", - } - } - k8s.update_config(patch_custom_service_annotations) - - pg_patch_custom_annotations = { - "spec": { - "serviceAnnotations": { - "annotation.key": "value", - "foo": "bar", - } - } - } - k8s.api.custom_objects_api.patch_namespaced_custom_object( - "acid.zalan.do", "v1", "default", "postgresqls", "acid-minimal-cluster", pg_patch_custom_annotations) - - # wait a little before proceeding - time.sleep(30) - annotations = { - "annotation.key": "value", - "foo": "bar", - } - self.assertTrue(k8s.check_service_annotations( - "cluster-name=acid-minimal-cluster,spilo-role=master", annotations)) - self.assertTrue(k8s.check_service_annotations( - "cluster-name=acid-minimal-cluster,spilo-role=replica", annotations)) - - # clean up - unpatch_custom_service_annotations = { - "data": { - "custom_service_annotations": "", - } - } - k8s.update_config(unpatch_custom_service_annotations) - - @timeout_decorator.timeout(TEST_TIMEOUT_SEC) - def test_statefulset_annotation_propagation(self): - ''' - Inject annotation to Postgresql CRD and check it's propagation to stateful set - ''' - k8s = self.k8s - cluster_label = 'application=spilo,cluster-name=acid-minimal-cluster' - - patch_sset_propagate_annotations = { - "data": { - "downscaler_annotations": "deployment-time,downscaler/*", - } - } - k8s.update_config(patch_sset_propagate_annotations) - - pg_crd_annotations = { - "metadata": { - "annotations": { - "deployment-time": "2020-04-30 12:00:00", - "downscaler/downtime_replicas": "0", - }, - } - } - k8s.api.custom_objects_api.patch_namespaced_custom_object( - "acid.zalan.do", "v1", "default", "postgresqls", "acid-minimal-cluster", pg_crd_annotations) - - # wait a little before proceeding - time.sleep(60) - annotations = { - "deployment-time": "2020-04-30 12:00:00", - "downscaler/downtime_replicas": "0", - } - self.assertTrue(k8s.check_statefulset_annotations(cluster_label, annotations)) - - @timeout_decorator.timeout(TEST_TIMEOUT_SEC) - def test_taint_based_eviction(self): - ''' - Add taint "postgres=:NoExecute" to node with master. This must cause a failover. - ''' - k8s = self.k8s - cluster_label = 'application=spilo,cluster-name=acid-minimal-cluster' - - # get nodes of master and replica(s) (expected target of new master) - current_master_node, current_replica_nodes = k8s.get_pg_nodes(cluster_label) - num_replicas = len(current_replica_nodes) - failover_targets = self.get_failover_targets(current_master_node, current_replica_nodes) - - # taint node with postgres=:NoExecute to force failover - body = { - "spec": { - "taints": [ - { - "effect": "NoExecute", - "key": "postgres" - } - ] - } - } - - # patch node and test if master is failing over to one of the expected nodes - k8s.api.core_v1.patch_node(current_master_node, body) - new_master_node, new_replica_nodes = self.assert_failover( - current_master_node, num_replicas, failover_targets, cluster_label) - - # add toleration to pods - patch_toleration_config = { - "data": { - "toleration": "key:postgres,operator:Exists,effect:NoExecute" - } - } - k8s.update_config(patch_toleration_config) - - # wait a little before proceeding with the pod distribution test - time.sleep(30) - - # toggle pod anti affinity to move replica away from master node - self.assert_distributed_pods(new_master_node, new_replica_nodes, cluster_label) - def get_failover_targets(self, master_node, replica_nodes): ''' If all pods live on the same node, failover will happen to other worker(s) @@ -820,6 +863,11 @@ class K8s: stdout=subprocess.PIPE, stderr=subprocess.PIPE) + def exec_with_kubectl(self, pod, cmd): + return subprocess.run(["./exec.sh", pod, cmd], + stdout=subprocess.PIPE, + stderr=subprocess.PIPE) + def get_effective_pod_image(self, pod_name, namespace='default'): ''' Get the Spilo image pod currently uses. In case of lazy rolling updates diff --git a/manifests/configmap.yaml b/manifests/configmap.yaml index d1c1b3d17..1210d5015 100644 --- a/manifests/configmap.yaml +++ b/manifests/configmap.yaml @@ -47,7 +47,8 @@ data: # etcd_host: "" # gcp_credentials: "" # kubernetes_use_configmaps: "false" - # infrastructure_roles_secret_name: postgresql-infrastructure-roles + # infrastructure_roles_secret_name: "postgresql-infrastructure-roles" + # infrastructure_roles_secrets: "secretname:monitoring-roles,userkey:user,passwordkey:password,rolekey:inrole" # inherited_labels: application,environment # kube_iam_role: "" # log_s3_bucket: "" diff --git a/manifests/infrastructure-roles-new.yaml b/manifests/infrastructure-roles-new.yaml new file mode 100644 index 000000000..e4f378396 --- /dev/null +++ b/manifests/infrastructure-roles-new.yaml @@ -0,0 +1,14 @@ +apiVersion: v1 +data: + # infrastructure role definition in the new format + # robot_zmon_acid_monitoring_new + user: cm9ib3Rfem1vbl9hY2lkX21vbml0b3JpbmdfbmV3 + # robot_zmon_new + role: cm9ib3Rfem1vbl9uZXc= + # foobar_new + password: Zm9vYmFyX25ldw== +kind: Secret +metadata: + name: postgresql-infrastructure-roles-new + namespace: default +type: Opaque diff --git a/manifests/operatorconfiguration.crd.yaml b/manifests/operatorconfiguration.crd.yaml index 2b6e8ae67..55b7653ef 100644 --- a/manifests/operatorconfiguration.crd.yaml +++ b/manifests/operatorconfiguration.crd.yaml @@ -127,6 +127,28 @@ spec: type: boolean infrastructure_roles_secret_name: type: string + infrastructure_roles_secrets: + type: array + nullable: true + items: + type: object + required: + - secretname + - userkey + - passwordkey + properties: + secretname: + type: string + userkey: + type: string + passwordkey: + type: string + rolekey: + type: string + details: + type: string + template: + type: boolean inherited_labels: type: array items: diff --git a/manifests/postgresql-operator-default-configuration.yaml b/manifests/postgresql-operator-default-configuration.yaml index f7eba1f6c..c0dce42ee 100644 --- a/manifests/postgresql-operator-default-configuration.yaml +++ b/manifests/postgresql-operator-default-configuration.yaml @@ -39,6 +39,14 @@ configuration: enable_pod_disruption_budget: true enable_sidecars: true # infrastructure_roles_secret_name: "postgresql-infrastructure-roles" + # infrastructure_roles_secrets: + # - secretname: "monitoring-roles" + # userkey: "user" + # passwordkey: "password" + # rolekey: "inrole" + # - secretname: "other-infrastructure-role" + # userkey: "other-user-key" + # passwordkey: "other-password-key" # inherited_labels: # - application # - environment diff --git a/pkg/apis/acid.zalan.do/v1/crds.go b/pkg/apis/acid.zalan.do/v1/crds.go index 6f907e266..c22ed25c0 100644 --- a/pkg/apis/acid.zalan.do/v1/crds.go +++ b/pkg/apis/acid.zalan.do/v1/crds.go @@ -911,6 +911,35 @@ var OperatorConfigCRDResourceValidation = apiextv1beta1.CustomResourceValidation "infrastructure_roles_secret_name": { Type: "string", }, + "infrastructure_roles_secrets": { + Type: "array", + Items: &apiextv1beta1.JSONSchemaPropsOrArray{ + Schema: &apiextv1beta1.JSONSchemaProps{ + Type: "object", + Required: []string{"secretname", "userkey", "passwordkey"}, + Properties: map[string]apiextv1beta1.JSONSchemaProps{ + "secretname": { + Type: "string", + }, + "userkey": { + Type: "string", + }, + "passwordkey": { + Type: "string", + }, + "rolekey": { + Type: "string", + }, + "details": { + Type: "string", + }, + "template": { + Type: "boolean", + }, + }, + }, + }, + }, "inherited_labels": { Type: "array", Items: &apiextv1beta1.JSONSchemaPropsOrArray{ @@ -983,7 +1012,7 @@ var OperatorConfigCRDResourceValidation = apiextv1beta1.CustomResourceValidation "spilo_privileged": { Type: "boolean", }, - "storage_resize_mode": { + "storage_resize_mode": { Type: "string", Enum: []apiextv1beta1.JSON{ { 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 e6e13cbd3..ea08f2ff3 100644 --- a/pkg/apis/acid.zalan.do/v1/operator_configuration_type.go +++ b/pkg/apis/acid.zalan.do/v1/operator_configuration_type.go @@ -45,28 +45,29 @@ type PostgresUsersConfiguration struct { type KubernetesMetaConfiguration struct { PodServiceAccountName string `json:"pod_service_account_name,omitempty"` // TODO: change it to the proper json - PodServiceAccountDefinition string `json:"pod_service_account_definition,omitempty"` - PodServiceAccountRoleBindingDefinition string `json:"pod_service_account_role_binding_definition,omitempty"` - PodTerminateGracePeriod Duration `json:"pod_terminate_grace_period,omitempty"` - SpiloPrivileged bool `json:"spilo_privileged,omitempty"` - SpiloFSGroup *int64 `json:"spilo_fsgroup,omitempty"` - WatchedNamespace string `json:"watched_namespace,omitempty"` - PDBNameFormat config.StringTemplate `json:"pdb_name_format,omitempty"` - EnablePodDisruptionBudget *bool `json:"enable_pod_disruption_budget,omitempty"` - StorageResizeMode string `json:"storage_resize_mode,omitempty"` - 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,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"` - ClusterLabels map[string]string `json:"cluster_labels,omitempty"` - InheritedLabels []string `json:"inherited_labels,omitempty"` - DownscalerAnnotations []string `json:"downscaler_annotations,omitempty"` - ClusterNameLabel string `json:"cluster_name_label,omitempty"` - NodeReadinessLabel map[string]string `json:"node_readiness_label,omitempty"` - CustomPodAnnotations map[string]string `json:"custom_pod_annotations,omitempty"` + PodServiceAccountDefinition string `json:"pod_service_account_definition,omitempty"` + PodServiceAccountRoleBindingDefinition string `json:"pod_service_account_role_binding_definition,omitempty"` + PodTerminateGracePeriod Duration `json:"pod_terminate_grace_period,omitempty"` + SpiloPrivileged bool `json:"spilo_privileged,omitempty"` + SpiloFSGroup *int64 `json:"spilo_fsgroup,omitempty"` + WatchedNamespace string `json:"watched_namespace,omitempty"` + PDBNameFormat config.StringTemplate `json:"pdb_name_format,omitempty"` + EnablePodDisruptionBudget *bool `json:"enable_pod_disruption_budget,omitempty"` + StorageResizeMode string `json:"storage_resize_mode,omitempty"` + 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,omitempty"` + OAuthTokenSecretName spec.NamespacedName `json:"oauth_token_secret_name,omitempty"` + InfrastructureRolesSecretName spec.NamespacedName `json:"infrastructure_roles_secret_name,omitempty"` + InfrastructureRolesDefs []*config.InfrastructureRole `json:"infrastructure_roles_secrets,omitempty"` + PodRoleLabel string `json:"pod_role_label,omitempty"` + ClusterLabels map[string]string `json:"cluster_labels,omitempty"` + InheritedLabels []string `json:"inherited_labels,omitempty"` + DownscalerAnnotations []string `json:"downscaler_annotations,omitempty"` + ClusterNameLabel string `json:"cluster_name_label,omitempty"` + NodeReadinessLabel map[string]string `json:"node_readiness_label,omitempty"` + CustomPodAnnotations map[string]string `json:"custom_pod_annotations,omitempty"` // TODO: use a proper toleration structure? PodToleration map[string]string `json:"toleration,omitempty"` PodEnvironmentConfigMap spec.NamespacedName `json:"pod_environment_configmap,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 064ced184..efc31d6b6 100644 --- a/pkg/apis/acid.zalan.do/v1/zz_generated.deepcopy.go +++ b/pkg/apis/acid.zalan.do/v1/zz_generated.deepcopy.go @@ -27,6 +27,7 @@ SOFTWARE. package v1 import ( + config "github.com/zalando/postgres-operator/pkg/util/config" corev1 "k8s.io/api/core/v1" runtime "k8s.io/apimachinery/pkg/runtime" ) @@ -168,6 +169,17 @@ func (in *KubernetesMetaConfiguration) DeepCopyInto(out *KubernetesMetaConfigura } out.OAuthTokenSecretName = in.OAuthTokenSecretName out.InfrastructureRolesSecretName = in.InfrastructureRolesSecretName + if in.InfrastructureRolesDefs != nil { + in, out := &in.InfrastructureRolesDefs, &out.InfrastructureRolesDefs + *out = make([]*config.InfrastructureRole, len(*in)) + for i := range *in { + if (*in)[i] != nil { + in, out := &(*in)[i], &(*out)[i] + *out = new(config.InfrastructureRole) + **out = **in + } + } + } if in.ClusterLabels != nil { in, out := &in.ClusterLabels, &out.ClusterLabels *out = make(map[string]string, len(*in)) diff --git a/pkg/controller/controller.go b/pkg/controller/controller.go index 6011d3863..10c817016 100644 --- a/pkg/controller/controller.go +++ b/pkg/controller/controller.go @@ -300,7 +300,8 @@ func (c *Controller) initController() { c.logger.Infof("config: %s", c.opConfig.MustMarshal()) - if infraRoles, err := c.getInfrastructureRoles(&c.opConfig.InfrastructureRolesSecretName); err != nil { + roleDefs := c.getInfrastructureRoleDefinitions() + if infraRoles, err := c.getInfrastructureRoles(roleDefs); err != nil { c.logger.Warningf("could not get infrastructure roles: %v", err) } else { c.config.InfrastructureRoles = infraRoles diff --git a/pkg/controller/operator_config.go b/pkg/controller/operator_config.go index e2d8636a1..d115aa118 100644 --- a/pkg/controller/operator_config.go +++ b/pkg/controller/operator_config.go @@ -71,7 +71,22 @@ func (c *Controller) importConfigurationFromCRD(fromCRD *acidv1.OperatorConfigur result.EnableSidecars = util.CoalesceBool(fromCRD.Kubernetes.EnableSidecars, util.True()) result.SecretNameTemplate = fromCRD.Kubernetes.SecretNameTemplate result.OAuthTokenSecretName = fromCRD.Kubernetes.OAuthTokenSecretName + result.InfrastructureRolesSecretName = fromCRD.Kubernetes.InfrastructureRolesSecretName + if fromCRD.Kubernetes.InfrastructureRolesDefs != nil { + result.InfrastructureRoles = []*config.InfrastructureRole{} + for _, secret := range fromCRD.Kubernetes.InfrastructureRolesDefs { + result.InfrastructureRoles = append( + result.InfrastructureRoles, + &config.InfrastructureRole{ + SecretName: secret.SecretName, + UserKey: secret.UserKey, + RoleKey: secret.RoleKey, + PasswordKey: secret.PasswordKey, + }) + } + } + result.PodRoleLabel = util.Coalesce(fromCRD.Kubernetes.PodRoleLabel, "spilo-role") result.ClusterLabels = util.CoalesceStrMap(fromCRD.Kubernetes.ClusterLabels, map[string]string{"application": "spilo"}) result.InheritedLabels = fromCRD.Kubernetes.InheritedLabels diff --git a/pkg/controller/util.go b/pkg/controller/util.go index 511f02823..6035903dd 100644 --- a/pkg/controller/util.go +++ b/pkg/controller/util.go @@ -4,6 +4,7 @@ import ( "context" "encoding/json" "fmt" + "strings" v1 "k8s.io/api/core/v1" apiextv1beta1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1" @@ -109,8 +110,161 @@ func readDecodedRole(s string) (*spec.PgUser, error) { return &result, nil } -func (c *Controller) getInfrastructureRoles(rolesSecret *spec.NamespacedName) (map[string]spec.PgUser, error) { - if *rolesSecret == (spec.NamespacedName{}) { +var emptyName = (spec.NamespacedName{}) + +// Return information about what secrets we need to use to create +// infrastructure roles and in which format are they. This is done in +// compatible way, so that the previous logic is not changed, and handles both +// configuration in ConfigMap & CRD. +func (c *Controller) getInfrastructureRoleDefinitions() []*config.InfrastructureRole { + var roleDef config.InfrastructureRole + rolesDefs := c.opConfig.InfrastructureRoles + + if c.opConfig.InfrastructureRolesSecretName == emptyName { + // All the other possibilities require secret name to be present, so if + // it is not, then nothing else to be done here. + return rolesDefs + } + + // check if we can extract something from the configmap config option + if c.opConfig.InfrastructureRolesDefs != "" { + // The configmap option could contain either a role description (in the + // form key1: value1, key2: value2), which has to be used together with + // an old secret name. + + var secretName spec.NamespacedName + var err error + propertySep := "," + valueSep := ":" + + // The field contains the format in which secret is written, let's + // convert it to a proper definition + properties := strings.Split(c.opConfig.InfrastructureRolesDefs, propertySep) + roleDef = config.InfrastructureRole{Template: false} + + for _, property := range properties { + values := strings.Split(property, valueSep) + if len(values) < 2 { + continue + } + name := strings.TrimSpace(values[0]) + value := strings.TrimSpace(values[1]) + + switch name { + case "secretname": + if err = secretName.DecodeWorker(value, "default"); err != nil { + c.logger.Warningf("Could not marshal secret name %s: %v", value, err) + } else { + roleDef.SecretName = secretName + } + case "userkey": + roleDef.UserKey = value + case "passwordkey": + roleDef.PasswordKey = value + case "rolekey": + roleDef.RoleKey = value + default: + c.logger.Warningf("Role description is not known: %s", properties) + } + } + } else { + // At this point we deal with the old format, let's replicate it + // via existing definition structure and remember that it's just a + // template, the real values are in user1,password1,inrole1 etc. + roleDef = config.InfrastructureRole{ + SecretName: c.opConfig.InfrastructureRolesSecretName, + UserKey: "user", + PasswordKey: "password", + RoleKey: "inrole", + Template: true, + } + } + + if roleDef.UserKey != "" && + roleDef.PasswordKey != "" && + roleDef.RoleKey != "" { + rolesDefs = append(rolesDefs, &roleDef) + } + + return rolesDefs +} + +func (c *Controller) getInfrastructureRoles( + rolesSecrets []*config.InfrastructureRole) ( + map[string]spec.PgUser, []error) { + + var errors []error + var noRolesProvided = true + + roles := []spec.PgUser{} + uniqRoles := map[string]spec.PgUser{} + + // To be compatible with the legacy implementation we need to return nil if + // the provided secret name is empty. The equivalent situation in the + // current implementation is an empty rolesSecrets slice or all its items + // are empty. + for _, role := range rolesSecrets { + if role.SecretName != emptyName { + noRolesProvided = false + } + } + + if noRolesProvided { + return nil, nil + } + + for _, secret := range rolesSecrets { + infraRoles, err := c.getInfrastructureRole(secret) + + if err != nil || infraRoles == nil { + c.logger.Debugf("Cannot get infrastructure role: %+v", *secret) + + if err != nil { + errors = append(errors, err) + } + + continue + } + + for _, r := range infraRoles { + roles = append(roles, r) + } + } + + for _, r := range roles { + if _, exists := uniqRoles[r.Name]; exists { + msg := "Conflicting infrastructure roles: roles[%s] = (%q, %q)" + c.logger.Debugf(msg, r.Name, uniqRoles[r.Name], r) + } + + uniqRoles[r.Name] = r + } + + return uniqRoles, errors +} + +// Generate list of users representing one infrastructure role based on its +// description in various K8S objects. An infrastructure role could be +// described by a secret and optionally a config map. The former should contain +// the secret information, i.e. username, password, role. The latter could +// contain an extensive description of the role and even override an +// information obtained from the secret (except a password). +// +// This function returns a list of users to be compatible with the previous +// behaviour, since we don't know how many users are actually encoded in the +// secret if it's a "template" role. If the provided role is not a template +// one, the result would be a list with just one user in it. +// +// FIXME: This dependency on two different objects is rather unnecessary +// complicated, so let's get rid of it via deprecation process. +func (c *Controller) getInfrastructureRole( + infraRole *config.InfrastructureRole) ( + []spec.PgUser, error) { + + rolesSecret := infraRole.SecretName + roles := []spec.PgUser{} + + if rolesSecret == emptyName { // we don't have infrastructure roles defined, bail out return nil, nil } @@ -119,52 +273,98 @@ func (c *Controller) getInfrastructureRoles(rolesSecret *spec.NamespacedName) (m Secrets(rolesSecret.Namespace). Get(context.TODO(), rolesSecret.Name, metav1.GetOptions{}) if err != nil { - c.logger.Debugf("infrastructure roles secret name: %q", *rolesSecret) - return nil, fmt.Errorf("could not get infrastructure roles secret: %v", err) + msg := "could not get infrastructure roles secret %s/%s: %v" + return nil, fmt.Errorf(msg, rolesSecret.Namespace, rolesSecret.Name, err) } secretData := infraRolesSecret.Data - result := make(map[string]spec.PgUser) -Users: - // in worst case we would have one line per user - for i := 1; i <= len(secretData); i++ { - properties := []string{"user", "password", "inrole"} - t := spec.PgUser{Origin: spec.RoleOriginInfrastructure} - for _, p := range properties { - key := fmt.Sprintf("%s%d", p, i) - if val, present := secretData[key]; !present { - if p == "user" { - // exit when the user name with the next sequence id is absent - break Users - } - } else { - s := string(val) - switch p { - case "user": - t.Name = s - case "password": - t.Password = s - case "inrole": - t.MemberOf = append(t.MemberOf, s) - default: - c.logger.Warningf("unknown key %q", p) - } + + if infraRole.Template { + Users: + for i := 1; i <= len(secretData); i++ { + properties := []string{ + infraRole.UserKey, + infraRole.PasswordKey, + infraRole.RoleKey, } - delete(secretData, key) + t := spec.PgUser{Origin: spec.RoleOriginInfrastructure} + for _, p := range properties { + key := fmt.Sprintf("%s%d", p, i) + if val, present := secretData[key]; !present { + if p == "user" { + // exit when the user name with the next sequence id is + // absent + break Users + } + } else { + s := string(val) + switch p { + case "user": + t.Name = s + case "password": + t.Password = s + case "inrole": + t.MemberOf = append(t.MemberOf, s) + default: + c.logger.Warningf("unknown key %q", p) + } + } + // XXX: This is a part of the original implementation, which is + // rather obscure. Why do we delete this key? Wouldn't it be + // used later in comparison for configmap? + delete(secretData, key) + } + + if t.Valid() { + roles = append(roles, t) + } else { + msg := "infrastructure role %q is not complete and ignored" + c.logger.Warningf(msg, t) + } + } + } else { + roleDescr := &spec.PgUser{Origin: spec.RoleOriginInfrastructure} + + if details, exists := secretData[infraRole.Details]; exists { + if err := yaml.Unmarshal(details, &roleDescr); err != nil { + return nil, fmt.Errorf("could not decode yaml role: %v", err) + } + } else { + roleDescr.Name = string(secretData[infraRole.UserKey]) + roleDescr.Password = string(secretData[infraRole.PasswordKey]) + roleDescr.MemberOf = append(roleDescr.MemberOf, string(secretData[infraRole.RoleKey])) } - if t.Name != "" { - if t.Password == "" { - c.logger.Warningf("infrastructure role %q has no password defined and is ignored", t.Name) - continue - } - result[t.Name] = t + if roleDescr.Valid() { + roles = append(roles, *roleDescr) + } else { + msg := "infrastructure role %q is not complete and ignored" + c.logger.Warningf(msg, roleDescr) + + return nil, nil } + + if roleDescr.Name == "" { + msg := "infrastructure role %q has no name defined and is ignored" + c.logger.Warningf(msg, roleDescr.Name) + return nil, nil + } + + if roleDescr.Password == "" { + msg := "infrastructure role %q has no password defined and is ignored" + c.logger.Warningf(msg, roleDescr.Name) + return nil, nil + } + + roles = append(roles, *roleDescr) } - // perhaps we have some map entries with usernames, passwords, let's check if we have those users in the configmap - if infraRolesMap, err := c.KubeClient.ConfigMaps(rolesSecret.Namespace).Get( - context.TODO(), rolesSecret.Name, metav1.GetOptions{}); err == nil { + // Now plot twist. We need to check if there is a configmap with the same + // name and extract a role description if it exists. + infraRolesMap, err := c.KubeClient. + ConfigMaps(rolesSecret.Namespace). + Get(context.TODO(), rolesSecret.Name, metav1.GetOptions{}) + if err == nil { // we have a configmap with username - json description, let's read and decode it for role, s := range infraRolesMap.Data { roleDescr, err := readDecodedRole(s) @@ -182,20 +382,12 @@ Users: } roleDescr.Name = role roleDescr.Origin = spec.RoleOriginInfrastructure - result[role] = *roleDescr + roles = append(roles, *roleDescr) } } - if len(secretData) > 0 { - c.logger.Warningf("%d unprocessed entries in the infrastructure roles secret,"+ - " checking configmap %v", len(secretData), rolesSecret.Name) - c.logger.Info(`infrastructure role entries should be in the {key}{id} format,` + - ` where {key} can be either of "user", "password", "inrole" and the {id}` + - ` a monotonically increasing integer starting with 1`) - c.logger.Debugf("unprocessed entries: %#v", secretData) - } - - return result, nil + // TODO: check for role collisions + return roles, nil } func (c *Controller) podClusterName(pod *v1.Pod) spec.NamespacedName { diff --git a/pkg/controller/util_test.go b/pkg/controller/util_test.go index ef182248e..fd756a0c7 100644 --- a/pkg/controller/util_test.go +++ b/pkg/controller/util_test.go @@ -8,20 +8,25 @@ import ( b64 "encoding/base64" "github.com/zalando/postgres-operator/pkg/spec" + "github.com/zalando/postgres-operator/pkg/util/config" "github.com/zalando/postgres-operator/pkg/util/k8sutil" v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) const ( - testInfrastructureRolesSecretName = "infrastructureroles-test" + testInfrastructureRolesOldSecretName = "infrastructureroles-old-test" + testInfrastructureRolesNewSecretName = "infrastructureroles-new-test" ) func newUtilTestController() *Controller { controller := NewController(&spec.ControllerConfig{}, "util-test") controller.opConfig.ClusterNameLabel = "cluster-name" controller.opConfig.InfrastructureRolesSecretName = - spec.NamespacedName{Namespace: v1.NamespaceDefault, Name: testInfrastructureRolesSecretName} + spec.NamespacedName{ + Namespace: v1.NamespaceDefault, + Name: testInfrastructureRolesOldSecretName, + } controller.opConfig.Workers = 4 controller.KubeClient = k8sutil.NewMockKubernetesClient() return controller @@ -80,24 +85,32 @@ func TestClusterWorkerID(t *testing.T) { } } -func TestGetInfrastructureRoles(t *testing.T) { +// Test functionality of getting infrastructure roles from their description in +// corresponding secrets. Here we test only common stuff (e.g. when a secret do +// not exist, or empty) and the old format. +func TestOldInfrastructureRoleFormat(t *testing.T) { var testTable = []struct { - secretName spec.NamespacedName - expectedRoles map[string]spec.PgUser - expectedError error + secretName spec.NamespacedName + expectedRoles map[string]spec.PgUser + expectedErrors []error }{ { + // empty secret name spec.NamespacedName{}, nil, nil, }, { + // secret does not exist spec.NamespacedName{Namespace: v1.NamespaceDefault, Name: "null"}, - nil, - fmt.Errorf(`could not get infrastructure roles secret: NotFound`), + map[string]spec.PgUser{}, + []error{fmt.Errorf(`could not get infrastructure roles secret default/null: NotFound`)}, }, { - spec.NamespacedName{Namespace: v1.NamespaceDefault, Name: testInfrastructureRolesSecretName}, + spec.NamespacedName{ + Namespace: v1.NamespaceDefault, + Name: testInfrastructureRolesOldSecretName, + }, map[string]spec.PgUser{ "testrole": { Name: "testrole", @@ -116,15 +129,269 @@ func TestGetInfrastructureRoles(t *testing.T) { }, } for _, test := range testTable { - roles, err := utilTestController.getInfrastructureRoles(&test.secretName) - if err != test.expectedError { - if err != nil && test.expectedError != nil && err.Error() == test.expectedError.Error() { - continue - } - t.Errorf("expected error '%v' does not match the actual error '%v'", test.expectedError, err) + roles, errors := utilTestController.getInfrastructureRoles( + []*config.InfrastructureRole{ + &config.InfrastructureRole{ + SecretName: test.secretName, + UserKey: "user", + PasswordKey: "password", + RoleKey: "inrole", + Template: true, + }, + }) + + if len(errors) != len(test.expectedErrors) { + t.Errorf("expected error '%v' does not match the actual error '%v'", + test.expectedErrors, errors) } + + for idx := range errors { + err := errors[idx] + expectedErr := test.expectedErrors[idx] + + if err != expectedErr { + if err != nil && expectedErr != nil && err.Error() == expectedErr.Error() { + continue + } + t.Errorf("expected error '%v' does not match the actual error '%v'", + expectedErr, err) + } + } + if !reflect.DeepEqual(roles, test.expectedRoles) { - t.Errorf("expected roles output %v does not match the actual %v", test.expectedRoles, roles) + t.Errorf("expected roles output %#v does not match the actual %#v", + test.expectedRoles, roles) + } + } +} + +// Test functionality of getting infrastructure roles from their description in +// corresponding secrets. Here we test the new format. +func TestNewInfrastructureRoleFormat(t *testing.T) { + var testTable = []struct { + secrets []spec.NamespacedName + expectedRoles map[string]spec.PgUser + expectedErrors []error + }{ + // one secret with one configmap + { + []spec.NamespacedName{ + spec.NamespacedName{ + Namespace: v1.NamespaceDefault, + Name: testInfrastructureRolesNewSecretName, + }, + }, + map[string]spec.PgUser{ + "new-test-role": { + Name: "new-test-role", + Origin: spec.RoleOriginInfrastructure, + Password: "new-test-password", + MemberOf: []string{"new-test-inrole"}, + }, + "new-foobar": { + Name: "new-foobar", + Origin: spec.RoleOriginInfrastructure, + Password: b64.StdEncoding.EncodeToString([]byte("password")), + MemberOf: nil, + Flags: []string{"createdb"}, + }, + }, + nil, + }, + // multiple standalone secrets + { + []spec.NamespacedName{ + spec.NamespacedName{ + Namespace: v1.NamespaceDefault, + Name: "infrastructureroles-new-test1", + }, + spec.NamespacedName{ + Namespace: v1.NamespaceDefault, + Name: "infrastructureroles-new-test2", + }, + }, + map[string]spec.PgUser{ + "new-test-role1": { + Name: "new-test-role1", + Origin: spec.RoleOriginInfrastructure, + Password: "new-test-password1", + MemberOf: []string{"new-test-inrole1"}, + }, + "new-test-role2": { + Name: "new-test-role2", + Origin: spec.RoleOriginInfrastructure, + Password: "new-test-password2", + MemberOf: []string{"new-test-inrole2"}, + }, + }, + nil, + }, + } + for _, test := range testTable { + definitions := []*config.InfrastructureRole{} + for _, secret := range test.secrets { + definitions = append(definitions, &config.InfrastructureRole{ + SecretName: secret, + UserKey: "user", + PasswordKey: "password", + RoleKey: "inrole", + Template: false, + }) + } + + roles, errors := utilTestController.getInfrastructureRoles(definitions) + if len(errors) != len(test.expectedErrors) { + t.Errorf("expected error does not match the actual error:\n%+v\n%+v", + test.expectedErrors, errors) + + // Stop and do not do any further checks + return + } + + for idx := range errors { + err := errors[idx] + expectedErr := test.expectedErrors[idx] + + if err != expectedErr { + if err != nil && expectedErr != nil && err.Error() == expectedErr.Error() { + continue + } + t.Errorf("expected error '%v' does not match the actual error '%v'", + expectedErr, err) + } + } + + if !reflect.DeepEqual(roles, test.expectedRoles) { + t.Errorf("expected roles output/the actual:\n%#v\n%#v", + test.expectedRoles, roles) + } + } +} + +// Tests for getting correct infrastructure roles definitions from present +// configuration. E.g. in which secrets for which roles too look. The biggest +// point here is compatibility of old and new formats of defining +// infrastructure roles. +func TestInfrastructureRoleDefinitions(t *testing.T) { + var testTable = []struct { + rolesDefs []*config.InfrastructureRole + roleSecretName spec.NamespacedName + roleSecrets string + expectedDefs []*config.InfrastructureRole + }{ + // only new format + { + []*config.InfrastructureRole{ + &config.InfrastructureRole{ + SecretName: spec.NamespacedName{ + Namespace: v1.NamespaceDefault, + Name: testInfrastructureRolesNewSecretName, + }, + UserKey: "user", + PasswordKey: "password", + RoleKey: "inrole", + Template: false, + }, + }, + spec.NamespacedName{}, + "", + []*config.InfrastructureRole{ + &config.InfrastructureRole{ + SecretName: spec.NamespacedName{ + Namespace: v1.NamespaceDefault, + Name: testInfrastructureRolesNewSecretName, + }, + UserKey: "user", + PasswordKey: "password", + RoleKey: "inrole", + Template: false, + }, + }, + }, + // only old format + { + []*config.InfrastructureRole{}, + spec.NamespacedName{ + Namespace: v1.NamespaceDefault, + Name: testInfrastructureRolesOldSecretName, + }, + "", + []*config.InfrastructureRole{ + &config.InfrastructureRole{ + SecretName: spec.NamespacedName{ + Namespace: v1.NamespaceDefault, + Name: testInfrastructureRolesOldSecretName, + }, + UserKey: "user", + PasswordKey: "password", + RoleKey: "inrole", + Template: true, + }, + }, + }, + // only configmap format + { + []*config.InfrastructureRole{}, + spec.NamespacedName{ + Namespace: v1.NamespaceDefault, + Name: testInfrastructureRolesOldSecretName, + }, + "secretname: infrastructureroles-old-test, userkey: test-user, passwordkey: test-password, rolekey: test-role, template: false", + []*config.InfrastructureRole{ + &config.InfrastructureRole{ + SecretName: spec.NamespacedName{ + Namespace: v1.NamespaceDefault, + Name: testInfrastructureRolesOldSecretName, + }, + UserKey: "test-user", + PasswordKey: "test-password", + RoleKey: "test-role", + Template: false, + }, + }, + }, + // incorrect configmap format + { + []*config.InfrastructureRole{}, + spec.NamespacedName{ + Namespace: v1.NamespaceDefault, + Name: testInfrastructureRolesOldSecretName, + }, + "wrong-format", + []*config.InfrastructureRole{}, + }, + // configmap without a secret + { + []*config.InfrastructureRole{}, + spec.NamespacedName{}, + "userkey: test-user, passwordkey: test-password, rolekey: test-role, template: false", + []*config.InfrastructureRole{}, + }, + } + + for _, test := range testTable { + t.Logf("Test: %+v", test) + utilTestController.opConfig.InfrastructureRoles = test.rolesDefs + utilTestController.opConfig.InfrastructureRolesSecretName = test.roleSecretName + utilTestController.opConfig.InfrastructureRolesDefs = test.roleSecrets + + defs := utilTestController.getInfrastructureRoleDefinitions() + if len(defs) != len(test.expectedDefs) { + t.Errorf("expected definitions does not match the actual:\n%#v\n%#v", + test.expectedDefs, defs) + + // Stop and do not do any further checks + return + } + + for idx := range defs { + def := defs[idx] + expectedDef := test.expectedDefs[idx] + + if !reflect.DeepEqual(def, expectedDef) { + t.Errorf("expected definition/the actual:\n%#v\n%#v", + expectedDef, def) + } } } } diff --git a/pkg/spec/types.go b/pkg/spec/types.go index 08008267b..7a2c0ddac 100644 --- a/pkg/spec/types.go +++ b/pkg/spec/types.go @@ -55,6 +55,10 @@ type PgUser struct { AdminRole string `yaml:"admin_role"` } +func (user *PgUser) Valid() bool { + return user.Name != "" && user.Password != "" +} + // PgUserMap maps user names to the definitions. type PgUserMap map[string]PgUser diff --git a/pkg/util/config/config.go b/pkg/util/config/config.go index 6cab8af45..5f262107f 100644 --- a/pkg/util/config/config.go +++ b/pkg/util/config/config.go @@ -52,16 +52,42 @@ type Resources struct { ShmVolume *bool `name:"enable_shm_volume" default:"true"` } +type InfrastructureRole struct { + // Name of a secret which describes the role, and optionally name of a + // configmap with an extra information + SecretName spec.NamespacedName + + UserKey string + PasswordKey string + RoleKey string + + // This field point out the detailed yaml definition of the role, if exists + Details string + + // Specify if a secret contains multiple fields in the following format: + // + // %(userkey)idx: ... + // %(passwordkey)idx: ... + // %(rolekey)idx: ... + // + // If it does, Name/Password/Role are interpreted not as unique field + // names, but as a template. + + Template bool +} + // Auth describes authentication specific configuration parameters type Auth struct { - SecretNameTemplate StringTemplate `name:"secret_name_template" default:"{username}.{cluster}.credentials.{tprkind}.{tprgroup}"` - PamRoleName string `name:"pam_role_name" default:"zalandos"` - PamConfiguration string `name:"pam_configuration" default:"https://info.example.com/oauth2/tokeninfo?access_token= uid realm=/employees"` - TeamsAPIUrl string `name:"teams_api_url" default:"https://teams.example.com/api/"` - OAuthTokenSecretName spec.NamespacedName `name:"oauth_token_secret_name" default:"postgresql-operator"` - InfrastructureRolesSecretName spec.NamespacedName `name:"infrastructure_roles_secret_name"` - SuperUsername string `name:"super_username" default:"postgres"` - ReplicationUsername string `name:"replication_username" default:"standby"` + SecretNameTemplate StringTemplate `name:"secret_name_template" default:"{username}.{cluster}.credentials.{tprkind}.{tprgroup}"` + PamRoleName string `name:"pam_role_name" default:"zalandos"` + PamConfiguration string `name:"pam_configuration" default:"https://info.example.com/oauth2/tokeninfo?access_token= uid realm=/employees"` + TeamsAPIUrl string `name:"teams_api_url" default:"https://teams.example.com/api/"` + OAuthTokenSecretName spec.NamespacedName `name:"oauth_token_secret_name" default:"postgresql-operator"` + InfrastructureRolesSecretName spec.NamespacedName `name:"infrastructure_roles_secret_name"` + InfrastructureRoles []*InfrastructureRole `name:"-"` + InfrastructureRolesDefs string `name:"infrastructure_roles_secrets"` + SuperUsername string `name:"super_username" default:"postgres"` + ReplicationUsername string `name:"replication_username" default:"standby"` } // Scalyr holds the configuration for the Scalyr Agent sidecar for log shipping: diff --git a/pkg/util/k8sutil/k8sutil.go b/pkg/util/k8sutil/k8sutil.go index 5cde1c3e8..1234ef74a 100644 --- a/pkg/util/k8sutil/k8sutil.go +++ b/pkg/util/k8sutil/k8sutil.go @@ -271,31 +271,73 @@ func SameLogicalBackupJob(cur, new *batchv1beta1.CronJob) (match bool, reason st } func (c *mockSecret) Get(ctx context.Context, name string, options metav1.GetOptions) (*v1.Secret, error) { - if name != "infrastructureroles-test" { - return nil, fmt.Errorf("NotFound") - } - secret := &v1.Secret{} - secret.Name = "testcluster" - secret.Data = map[string][]byte{ + oldFormatSecret := &v1.Secret{} + oldFormatSecret.Name = "testcluster" + oldFormatSecret.Data = map[string][]byte{ "user1": []byte("testrole"), "password1": []byte("testpassword"), "inrole1": []byte("testinrole"), "foobar": []byte(b64.StdEncoding.EncodeToString([]byte("password"))), } - return secret, nil + + newFormatSecret := &v1.Secret{} + newFormatSecret.Name = "test-secret-new-format" + newFormatSecret.Data = map[string][]byte{ + "user": []byte("new-test-role"), + "password": []byte("new-test-password"), + "inrole": []byte("new-test-inrole"), + "new-foobar": []byte(b64.StdEncoding.EncodeToString([]byte("password"))), + } + + secrets := map[string]*v1.Secret{ + "infrastructureroles-old-test": oldFormatSecret, + "infrastructureroles-new-test": newFormatSecret, + } + + for idx := 1; idx <= 2; idx++ { + newFormatStandaloneSecret := &v1.Secret{} + newFormatStandaloneSecret.Name = fmt.Sprintf("test-secret-new-format%d", idx) + newFormatStandaloneSecret.Data = map[string][]byte{ + "user": []byte(fmt.Sprintf("new-test-role%d", idx)), + "password": []byte(fmt.Sprintf("new-test-password%d", idx)), + "inrole": []byte(fmt.Sprintf("new-test-inrole%d", idx)), + } + + secrets[fmt.Sprintf("infrastructureroles-new-test%d", idx)] = + newFormatStandaloneSecret + } + + if secret, exists := secrets[name]; exists { + return secret, nil + } + + return nil, fmt.Errorf("NotFound") } func (c *mockConfigMap) Get(ctx context.Context, name string, options metav1.GetOptions) (*v1.ConfigMap, error) { - if name != "infrastructureroles-test" { - return nil, fmt.Errorf("NotFound") - } - configmap := &v1.ConfigMap{} - configmap.Name = "testcluster" - configmap.Data = map[string]string{ + oldFormatConfigmap := &v1.ConfigMap{} + oldFormatConfigmap.Name = "testcluster" + oldFormatConfigmap.Data = map[string]string{ "foobar": "{}", } - return configmap, nil + + newFormatConfigmap := &v1.ConfigMap{} + newFormatConfigmap.Name = "testcluster" + newFormatConfigmap.Data = map[string]string{ + "new-foobar": "{\"user_flags\": [\"createdb\"]}", + } + + configmaps := map[string]*v1.ConfigMap{ + "infrastructureroles-old-test": oldFormatConfigmap, + "infrastructureroles-new-test": newFormatConfigmap, + } + + if configmap, exists := configmaps[name]; exists { + return configmap, nil + } + + return nil, fmt.Errorf("NotFound") } // Secrets to be mocked