From d5b7c94ba3cdc70f5392dd2de9fa1901d0860918 Mon Sep 17 00:00:00 2001 From: Dmitrii Dolgov <9erthalion6@gmail.com> Date: Thu, 16 Jul 2020 11:03:09 +0200 Subject: [PATCH] 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 implicitely 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 explicitely. The idea is to introduce a new configuration option for the Operator. infrastructure_roles_secrets: - secret: k8s_secret_name name: some_encrypted_name password: some_encrypted_password role: some_encrypted_role - secret: k8s_secret_name name: some_encrypted_name password: some_encrypted_password role: 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_secret_name: | secret: k8s_secret_name, name: some_encrypted_name, password: some_encrypted_password, role: some_encrypted_role --- pkg/controller/operator_config.go | 8 +++---- pkg/controller/util.go | 40 +++++++++++++++++++++---------- pkg/controller/util_test.go | 29 +++++++++++----------- pkg/spec/types.go | 4 ++++ pkg/util/config/config.go | 2 +- pkg/util/k8sutil/k8sutil.go | 2 +- 6 files changed, 52 insertions(+), 33 deletions(-) diff --git a/pkg/controller/operator_config.go b/pkg/controller/operator_config.go index 561c161c0..bf9e257e8 100644 --- a/pkg/controller/operator_config.go +++ b/pkg/controller/operator_config.go @@ -78,10 +78,10 @@ func (c *Controller) importConfigurationFromCRD(fromCRD *acidv1.OperatorConfigur result.InfrastructureRoles = append( result.InfrastructureRoles, &config.InfrastructureRole{ - Secret: secret.Secret, - Name: secret.Name, - Role: secret.Role, - Password: secret.Password, + SecretName: secret.SecretName, + Name: secret.Name, + Role: secret.Role, + Password: secret.Password, }) } } diff --git a/pkg/controller/util.go b/pkg/controller/util.go index 673c7d3c1..50402cf5e 100644 --- a/pkg/controller/util.go +++ b/pkg/controller/util.go @@ -110,7 +110,7 @@ func readDecodedRole(s string) (*spec.PgUser, error) { return &result, nil } -var emptyNamespacedName = (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 @@ -120,7 +120,7 @@ func (c *Controller) getInfrastructureRoleDefinitions() []*config.Infrastructure var roleDef config.InfrastructureRole rolesDefs := c.opConfig.InfrastructureRoles - if c.opConfig.InfrastructureRolesSecretName == emptyNamespacedName { + 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 @@ -140,8 +140,8 @@ func (c *Controller) getInfrastructureRoleDefinitions() []*config.Infrastructure properties := strings.Split(c.opConfig.InfrastructureRolesDefs, propertySep) roleDef = config.InfrastructureRole{ - Secret: c.opConfig.InfrastructureRolesSecretName, - Template: false, + SecretName: c.opConfig.InfrastructureRolesSecretName, + Template: false, } for _, property := range properties { @@ -169,11 +169,11 @@ func (c *Controller) getInfrastructureRoleDefinitions() []*config.Infrastructure // via existing definition structure and remember that it's just a // template, the real values are in user1,password1,inrole1 etc. roleDef = config.InfrastructureRole{ - Secret: c.opConfig.InfrastructureRolesSecretName, - Name: "user", - Password: "password", - Role: "inrole", - Template: true, + SecretName: c.opConfig.InfrastructureRolesSecretName, + Name: "user", + Password: "password", + Role: "inrole", + Template: true, } } @@ -201,7 +201,7 @@ func (c *Controller) getInfrastructureRoles( // current implementation is an empty rolesSecrets slice or all its items // are empty. for _, role := range rolesSecrets { - if role.Secret != emptyNamespacedName { + if role.SecretName != emptyName { noRolesProvided = false } } @@ -258,10 +258,10 @@ func (c *Controller) getInfrastructureRole( infraRole *config.InfrastructureRole) ( []spec.PgUser, error) { - rolesSecret := infraRole.Secret + rolesSecret := infraRole.SecretName roles := []spec.PgUser{} - if rolesSecret == (spec.NamespacedName{}) { + if rolesSecret == emptyName { // we don't have infrastructure roles defined, bail out return nil, nil } @@ -312,7 +312,12 @@ func (c *Controller) getInfrastructureRole( delete(secretData, key) } - roles = append(roles, t) + 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} @@ -327,6 +332,15 @@ func (c *Controller) getInfrastructureRole( roleDescr.MemberOf = append(roleDescr.MemberOf, string(secretData[infraRole.Role])) } + 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) diff --git a/pkg/controller/util_test.go b/pkg/controller/util_test.go index 4bb6dba94..de11c18aa 100644 --- a/pkg/controller/util_test.go +++ b/pkg/controller/util_test.go @@ -132,11 +132,11 @@ func TestOldInfrastructureRoleFormat(t *testing.T) { roles, errors := utilTestController.getInfrastructureRoles( []*config.InfrastructureRole{ &config.InfrastructureRole{ - Secret: test.secretName, - Name: "user", - Password: "password", - Role: "inrole", - Template: true, + SecretName: test.secretName, + Name: "user", + Password: "password", + Role: "inrole", + Template: true, }, }) @@ -193,6 +193,7 @@ func TestNewInfrastructureRoleFormat(t *testing.T) { Origin: spec.RoleOriginInfrastructure, Password: b64.StdEncoding.EncodeToString([]byte("password")), MemberOf: nil, + Flags: []string{"createdb"}, }, }, nil, @@ -230,11 +231,11 @@ func TestNewInfrastructureRoleFormat(t *testing.T) { definitions := []*config.InfrastructureRole{} for _, secret := range test.secrets { definitions = append(definitions, &config.InfrastructureRole{ - Secret: secret, - Name: "user", - Password: "password", - Role: "inrole", - Template: false, + SecretName: secret, + Name: "user", + Password: "password", + Role: "inrole", + Template: false, }) } @@ -282,7 +283,7 @@ func TestInfrastructureRoleDefinitions(t *testing.T) { { []*config.InfrastructureRole{ &config.InfrastructureRole{ - Secret: spec.NamespacedName{ + SecretName: spec.NamespacedName{ Namespace: v1.NamespaceDefault, Name: testInfrastructureRolesNewSecretName, }, @@ -296,7 +297,7 @@ func TestInfrastructureRoleDefinitions(t *testing.T) { "", []*config.InfrastructureRole{ &config.InfrastructureRole{ - Secret: spec.NamespacedName{ + SecretName: spec.NamespacedName{ Namespace: v1.NamespaceDefault, Name: testInfrastructureRolesNewSecretName, }, @@ -317,7 +318,7 @@ func TestInfrastructureRoleDefinitions(t *testing.T) { "", []*config.InfrastructureRole{ &config.InfrastructureRole{ - Secret: spec.NamespacedName{ + SecretName: spec.NamespacedName{ Namespace: v1.NamespaceDefault, Name: testInfrastructureRolesOldSecretName, }, @@ -338,7 +339,7 @@ func TestInfrastructureRoleDefinitions(t *testing.T) { "name: test-user, password: test-password, role: test-role", []*config.InfrastructureRole{ &config.InfrastructureRole{ - Secret: spec.NamespacedName{ + SecretName: spec.NamespacedName{ Namespace: v1.NamespaceDefault, Name: testInfrastructureRolesOldSecretName, }, 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 ef790f768..28615d551 100644 --- a/pkg/util/config/config.go +++ b/pkg/util/config/config.go @@ -54,7 +54,7 @@ type Resources struct { type InfrastructureRole struct { // Name of a secret which describes the role, and optionally name of a // configmap with an extra information - Secret spec.NamespacedName + SecretName spec.NamespacedName Name string Password string diff --git a/pkg/util/k8sutil/k8sutil.go b/pkg/util/k8sutil/k8sutil.go index 2b8bb19da..1234ef74a 100644 --- a/pkg/util/k8sutil/k8sutil.go +++ b/pkg/util/k8sutil/k8sutil.go @@ -325,7 +325,7 @@ func (c *mockConfigMap) Get(ctx context.Context, name string, options metav1.Get newFormatConfigmap := &v1.ConfigMap{} newFormatConfigmap.Name = "testcluster" newFormatConfigmap.Data = map[string]string{ - "new-foobar": "{}", + "new-foobar": "{\"user_flags\": [\"createdb\"]}", } configmaps := map[string]*v1.ConfigMap{