allow using both infrastructure_roles_options (zalando#1090)

* allow using both infrastructure_roles_options

* new default values for user and role definition

* use robot_zmon as parent role

* add operator log to debug

* right name for old secret

* only extract if rolesDefs is empty

* set password1 in old infrastructure role

* fix new infra rile secret

* choose different role key for new secret

* set memberof everywhere

* reenable all tests

* reflect feedback

* remove condition for rolesDefs

charts/postgres-operator/crds/operatorconfigurations.yaml

@@ -149,6 +149,10 @@ spec:
                         type: string
                       rolekey:
                         type: string
+                      defaultuservalue:
+                        type: string
+                      defaultrolevalue:
+                        type: string
                       details:
                         type: string
                       template:

manifests/infrastructure-roles-new.yaml

@@ -3,8 +3,6 @@ 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

manifests/infrastructure-roles.yaml

@@ -7,12 +7,14 @@ data:
   # provide other options in the configmap.
   # robot_zmon_acid_monitoring
   user1: cm9ib3Rfem1vbl9hY2lkX21vbml0b3Jpbmc=
+  # foobar
+  password1: Zm9vYmFy
   # robot_zmon
   inrole1: cm9ib3Rfem1vbg==
   # testuser
   user2: dGVzdHVzZXI=
-  # foobar
-  password2: Zm9vYmFy
+  # testpassword
+  password2: dGVzdHBhc3N3b3Jk
   # user batman with the password justice
   # look for other fields in the infrastructure roles configmap
   batman: anVzdGljZQ==

manifests/operatorconfiguration.crd.yaml

@@ -145,6 +145,10 @@ spec:
                         type: string
                       rolekey:
                         type: string
+                      defaultuservalue:
+                        type: string
+                      defaultrolevalue:
+                        type: string
                       details:
                         type: string
                       template:

pkg/apis/acid.zalan.do/v1/crds.go

@@ -930,6 +930,12 @@ var OperatorConfigCRDResourceValidation = apiextv1beta1.CustomResourceValidation
 											"rolekey": {
 												Type: "string",
 											},
+											"defaultuservalue": {
+												Type: "string",
+											},
+											"defaultrolevalue": {
+												Type: "string",
+											},
 											"details": {
 												Type: "string",
 											},

pkg/cluster/resources.go

@@ -207,7 +207,7 @@ func (c *Cluster) deleteConnectionPooler() (err error) {
 		serviceName = service.Name
 	}
 
-	// set delete propagation policy to foreground, so that all the dependant
+	// set delete propagation policy to foreground, so that all the dependent
 	// will be deleted.
 	err = c.KubeClient.
 		Services(c.Namespace).

pkg/controller/util.go

@@ -15,6 +15,7 @@ import (
 	acidv1 "github.com/zalando/postgres-operator/pkg/apis/acid.zalan.do/v1"
 	"github.com/zalando/postgres-operator/pkg/cluster"
 	"github.com/zalando/postgres-operator/pkg/spec"
+	"github.com/zalando/postgres-operator/pkg/util"
 	"github.com/zalando/postgres-operator/pkg/util/config"
 	"github.com/zalando/postgres-operator/pkg/util/k8sutil"
 	"gopkg.in/yaml.v2"
@@ -118,13 +119,9 @@ var emptyName = (spec.NamespacedName{})
 // 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
-	}
+	// take from CRD configuration
+	rolesDefs := c.opConfig.InfrastructureRoles
 
 	// check if we can extract something from the configmap config option
 	if c.opConfig.InfrastructureRolesDefs != "" {
@@ -163,27 +160,33 @@ func (c *Controller) getInfrastructureRoleDefinitions() []*config.Infrastructure
 				roleDef.PasswordKey = value
 			case "rolekey":
 				roleDef.RoleKey = value
+			case "defaultuservalue":
+				roleDef.DefaultUserValue = value
+			case "defaultrolevalue":
+				roleDef.DefaultRoleValue = value
 			default:
 				c.logger.Warningf("Role description is not known: %s", properties)
 			}
 		}
-	} else {
+
+		if roleDef.SecretName != emptyName &&
+			(roleDef.UserKey != "" || roleDef.DefaultUserValue != "") &&
+			roleDef.PasswordKey != "" {
+			rolesDefs = append(rolesDefs, &roleDef)
+		}
+	}
+
+	if c.opConfig.InfrastructureRolesSecretName != emptyName {
 		// 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{
+		rolesDefs = append(rolesDefs, &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
@@ -330,9 +333,10 @@ func (c *Controller) getInfrastructureRole(
 				return nil, fmt.Errorf("could not decode yaml role: %v", err)
 			}
 		} else {
-			roleDescr.Name = string(secretData[infraRole.UserKey])
+			roleDescr.Name = util.Coalesce(string(secretData[infraRole.UserKey]), infraRole.DefaultUserValue)
 			roleDescr.Password = string(secretData[infraRole.PasswordKey])
-			roleDescr.MemberOf = append(roleDescr.MemberOf, string(secretData[infraRole.RoleKey]))
+			roleDescr.MemberOf = append(roleDescr.MemberOf,
+				util.Coalesce(string(secretData[infraRole.RoleKey]), infraRole.DefaultRoleValue))
 		}
 
 		if roleDescr.Valid() {

pkg/controller/util_test.go

@@ -279,17 +279,17 @@ func TestInfrastructureRoleDefinitions(t *testing.T) {
 		roleSecrets    string
 		expectedDefs   []*config.InfrastructureRole
 	}{
-		// only new format
+		// only new CRD format
 		{
 			[]*config.InfrastructureRole{
 				&config.InfrastructureRole{
 					SecretName: spec.NamespacedName{
 						Namespace: v1.NamespaceDefault,
 						Name:      testInfrastructureRolesNewSecretName,
 					},
-					UserKey:     "user",
-					PasswordKey: "password",
-					RoleKey:     "inrole",
+					UserKey:     "test-user",
+					PasswordKey: "test-password",
+					RoleKey:     "test-role",
 					Template:    false,
 				},
 			},
@@ -301,14 +301,50 @@ func TestInfrastructureRoleDefinitions(t *testing.T) {
 						Namespace: v1.NamespaceDefault,
 						Name:      testInfrastructureRolesNewSecretName,
 					},
-					UserKey:     "user",
-					PasswordKey: "password",
-					RoleKey:     "inrole",
+					UserKey:     "test-user",
+					PasswordKey: "test-password",
+					RoleKey:     "test-role",
 					Template:    false,
 				},
 			},
 		},
-		// only old format
+		// only new configmap format
+		{
+			[]*config.InfrastructureRole{},
+			spec.NamespacedName{},
+			"secretname: infrastructureroles-new-test, userkey: test-user, passwordkey: test-password, rolekey: test-role",
+			[]*config.InfrastructureRole{
+				&config.InfrastructureRole{
+					SecretName: spec.NamespacedName{
+						Namespace: v1.NamespaceDefault,
+						Name:      testInfrastructureRolesNewSecretName,
+					},
+					UserKey:     "test-user",
+					PasswordKey: "test-password",
+					RoleKey:     "test-role",
+					Template:    false,
+				},
+			},
+		},
+		// new configmap format with defaultRoleValue
+		{
+			[]*config.InfrastructureRole{},
+			spec.NamespacedName{},
+			"secretname: infrastructureroles-new-test, userkey: test-user, passwordkey: test-password, defaultrolevalue: test-role",
+			[]*config.InfrastructureRole{
+				&config.InfrastructureRole{
+					SecretName: spec.NamespacedName{
+						Namespace: v1.NamespaceDefault,
+						Name:      testInfrastructureRolesNewSecretName,
+					},
+					UserKey:          "test-user",
+					PasswordKey:      "test-password",
+					DefaultRoleValue: "test-role",
+					Template:         false,
+				},
+			},
+		},
+		// only old CRD and configmap format
 		{
 			[]*config.InfrastructureRole{},
 			spec.NamespacedName{
@@ -329,42 +365,91 @@ func TestInfrastructureRoleDefinitions(t *testing.T) {
 				},
 			},
 		},
-		// only configmap format
+		// both formats for CRD
 		{
-			[]*config.InfrastructureRole{},
+			[]*config.InfrastructureRole{
+				&config.InfrastructureRole{
+					SecretName: spec.NamespacedName{
+						Namespace: v1.NamespaceDefault,
+						Name:      testInfrastructureRolesNewSecretName,
+					},
+					UserKey:     "test-user",
+					PasswordKey: "test-password",
+					RoleKey:     "test-role",
+					Template:    false,
+				},
+			},
 			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,
+						Name:      testInfrastructureRolesNewSecretName,
 					},
 					UserKey:     "test-user",
 					PasswordKey: "test-password",
 					RoleKey:     "test-role",
 					Template:    false,
 				},
+				&config.InfrastructureRole{
+					SecretName: spec.NamespacedName{
+						Namespace: v1.NamespaceDefault,
+						Name:      testInfrastructureRolesOldSecretName,
+					},
+					UserKey:     "user",
+					PasswordKey: "password",
+					RoleKey:     "inrole",
+					Template:    true,
+				},
 			},
 		},
-		// incorrect configmap format
+		// both formats for configmap
 		{
 			[]*config.InfrastructureRole{},
 			spec.NamespacedName{
 				Namespace: v1.NamespaceDefault,
 				Name:      testInfrastructureRolesOldSecretName,
 			},
+			"secretname: infrastructureroles-new-test, userkey: test-user, passwordkey: test-password, rolekey: test-role",
+			[]*config.InfrastructureRole{
+				&config.InfrastructureRole{
+					SecretName: spec.NamespacedName{
+						Namespace: v1.NamespaceDefault,
+						Name:      testInfrastructureRolesNewSecretName,
+					},
+					UserKey:     "test-user",
+					PasswordKey: "test-password",
+					RoleKey:     "test-role",
+					Template:    false,
+				},
+				&config.InfrastructureRole{
+					SecretName: spec.NamespacedName{
+						Namespace: v1.NamespaceDefault,
+						Name:      testInfrastructureRolesOldSecretName,
+					},
+					UserKey:     "user",
+					PasswordKey: "password",
+					RoleKey:     "inrole",
+					Template:    true,
+				},
+			},
+		},
+		// incorrect configmap format
+		{
+			[]*config.InfrastructureRole{},
+			spec.NamespacedName{},
 			"wrong-format",
 			[]*config.InfrastructureRole{},
 		},
 		// configmap without a secret
 		{
 			[]*config.InfrastructureRole{},
 			spec.NamespacedName{},
-			"userkey: test-user, passwordkey: test-password, rolekey: test-role, template: false",
+			"userkey: test-user, passwordkey: test-password, rolekey: test-role",
 			[]*config.InfrastructureRole{},
 		},
 	}

pkg/util/config/config.go

@@ -61,6 +61,9 @@ type InfrastructureRole struct {
 	PasswordKey string
 	RoleKey     string
 
+	DefaultUserValue string
+	DefaultRoleValue string
+
 	// This field point out the detailed yaml definition of the role, if exists
 	Details string