diff --git a/docs/administrator.md b/docs/administrator.md index f9ca7f15d..5a6af0b95 100644 --- a/docs/administrator.md +++ b/docs/administrator.md @@ -297,13 +297,14 @@ For Helm deployments setting `rbac.createAggregateClusterRoles: true` adds these The operator regularly updates credentials in the K8s secrets if the `enable_password_rotation` option is set to `true` in the configuration. -It happens only for LOGIN roles with an associated secret (manifest roles, -default users from `preparedDatabases`, system users). Furthermore, there -are the following exceptions: +It happens only for `LOGIN` roles with an associated secret (manifest roles, +default users from `preparedDatabases`). Furthermore, there are the following +exceptions: 1. Infrastructure role secrets since rotation should happen by the infrastructure. -2. Team API roles that connect via OAuth2 and JWT token. Rotation should be provided by the infrastructure + there is even no secret for these roles -3. Database owners and direct members of owners, since ownership on database objects can not be inherited. +2. Team API roles that connect via OAuth2 and JWT token (no secrets to these roles anyway). +3. Database owners since ownership on database objects can not be inherited. +4. System users such as `postgres`, `standby` and `pooler` user. The interval of days can be set with `password_rotation_interval` (default `90` = 90 days, minimum 1). On each rotation the user name and password values @@ -317,7 +318,7 @@ length of the interval. Pods still using the previous secret values which they keep in memory continue to connect to the database since the password of the corresponding user is not -replaced. However, a retention policy can be configured for roles created by +replaced. However, a retention policy can be configured for users created by the password rotation feature with `password_rotation_user_retention`. The operator will ensure that this period is at least twice as long as the configured rotation interval, hence the default of `180` = 180 days. When @@ -326,7 +327,7 @@ might not get removed immediately. Only on the next user rotation it is checked if users can get removed. Therefore, you might want to configure the retention to be a multiple of the rotation interval. -### Password rotation for single roles +### Password rotation for single users From the configuration, password rotation is enabled for all secrets with the mentioned exceptions. If you wish to first test rotation for a single user (or @@ -340,11 +341,11 @@ spec: - bar_reader_user ``` -### Password replacement without extra roles +### Password replacement without extra users For some use cases where the secret is only used rarely - think of a `flyway` user running a migration script on pod start - we do not need to create extra -database roles but can replace only the password in the K8s secret. This type +database users but can replace only the password in the K8s secret. This type of rotation cannot be configured globally but specified in the cluster manifest: @@ -367,7 +368,7 @@ with the latter. A new password is assigned and the `nextRotation` field is cleared. A final lookup for child (rotation) users to be removed is done but they will only be dropped if the retention policy allows for it. This is to avoid sudden connection issues in pods which still use credentials of these -users in memory. You have to remove these child roles manually or re-enable +users in memory. You have to remove these child users manually or re-enable password rotation with smaller interval so they get cleaned up. ## Use taints and tolerations for dedicated PostgreSQL nodes diff --git a/docs/reference/cluster_manifest.md b/docs/reference/cluster_manifest.md index 6bb4e4a82..9d65062c0 100644 --- a/docs/reference/cluster_manifest.md +++ b/docs/reference/cluster_manifest.md @@ -129,7 +129,7 @@ These parameters are grouped directly under the `spec` key in the manifest. password value will be replaced in the secrets which the operator reflects in the database, too. List only users here that rarely connect to the database, like a flyway user running a migration on Pod start. See more - details in the [administrator docs](https://github.com/zalando/postgres-operator/blob/master/docs/administrator.md#password-replacement-without-extra-roles). + details in the [administrator docs](https://github.com/zalando/postgres-operator/blob/master/docs/administrator.md#password-replacement-without-extra-users). * **databases** a map of database names to database owners for the databases that should be diff --git a/docs/reference/operator_parameters.md b/docs/reference/operator_parameters.md index 4a67ace09..4df9ad94d 100644 --- a/docs/reference/operator_parameters.md +++ b/docs/reference/operator_parameters.md @@ -175,12 +175,12 @@ under the `users` key. `standby`. * **enable_password_rotation** - For all LOGIN roles that are not database owners the Operator can rotate + For all `LOGIN` roles that are not database owners the operator can rotate credentials in the corresponding K8s secrets by replacing the username and password. This means, new users will be added on each rotation inheriting - all priviliges from the original roles. The rotation date in the YYMMDD is - appended to the new user names. The timestamp of the next rotation is - written to the secret. The default is `false`. + all priviliges from the original roles. The rotation date (in YYMMDD format) + is appended to the names of the new user. The timestamp of the next rotation + is written to the secret. The default is `false`. * **password_rotation_interval** If password rotation is enabled (either from config or cluster manifest) the diff --git a/pkg/cluster/cluster.go b/pkg/cluster/cluster.go index 3a57f2b0e..0059ca134 100644 --- a/pkg/cluster/cluster.go +++ b/pkg/cluster/cluster.go @@ -1000,7 +1000,6 @@ func (c *Cluster) initSystemUsers() { Name: c.OpConfig.SuperUsername, Namespace: c.Namespace, Password: util.RandomPassword(constants.PasswordLength), - IsDbOwner: true, } c.systemUsers[constants.ReplicationUserKeyName] = spec.PgUser{ Origin: spec.RoleOriginSystem, diff --git a/pkg/cluster/sync.go b/pkg/cluster/sync.go index 1f75f8494..bcd26a1ca 100644 --- a/pkg/cluster/sync.go +++ b/pkg/cluster/sync.go @@ -617,131 +617,37 @@ func (c *Cluster) getNextRotationDate(currentDate time.Time) (time.Time, string) } func (c *Cluster) syncSecrets() error { - var ( - err error - secret *v1.Secret - updateSecret bool - updateSecretMsg string - nextRotationDate time.Time - nextRotationDateStr string - ) + c.logger.Info("syncing secrets") c.setProcessName("syncing secrets") - secrets := c.generateUserSecrets() + generatedSecrets := c.generateUserSecrets() rotationUsers := make(spec.PgUserMap) retentionUsers := make([]string, 0) + currentTime := time.Now() - for secretUsername, secretSpec := range secrets { - if secret, err = c.KubeClient.Secrets(secretSpec.Namespace).Create(context.TODO(), secretSpec, metav1.CreateOptions{}); err == nil { + for secretUsername, generatedSecretSpec := range generatedSecrets { + secret, err := c.KubeClient.Secrets(generatedSecretSpec.Namespace).Create(context.TODO(), generatedSecretSpec, metav1.CreateOptions{}) + if err == nil { c.Secrets[secret.UID] = secret - c.logger.Debugf("created new secret %s, namespace: %s, uid: %s", util.NameFromMeta(secret.ObjectMeta), secretSpec.Namespace, secret.UID) + c.logger.Debugf("created new secret %s, namespace: %s, uid: %s", util.NameFromMeta(secret.ObjectMeta), generatedSecretSpec.Namespace, secret.UID) continue } if k8sutil.ResourceAlreadyExists(err) { - if secret, err = c.KubeClient.Secrets(secretSpec.Namespace).Get(context.TODO(), secretSpec.Name, metav1.GetOptions{}); err != nil { + if secret, err = c.KubeClient.Secrets(generatedSecretSpec.Namespace).Get(context.TODO(), generatedSecretSpec.Name, metav1.GetOptions{}); err != nil { return fmt.Errorf("could not get current secret: %v", err) } c.Secrets[secret.UID] = secret - c.logger.Debugf("secret %s already exists, fetching its password", util.NameFromMeta(secret.ObjectMeta)) - - // fetch user map to update later - var userMap map[string]spec.PgUser - var userKey string - if secretUsername == c.systemUsers[constants.SuperuserKeyName].Name { - userKey = constants.SuperuserKeyName - userMap = c.systemUsers - } else if secretUsername == c.systemUsers[constants.ReplicationUserKeyName].Name { - userKey = constants.ReplicationUserKeyName - userMap = c.systemUsers - } else { - userKey = secretUsername - userMap = c.pgUsers + if err = c.updateSecret(secretUsername, generatedSecretSpec, secret, &rotationUsers, &retentionUsers, currentTime); err != nil { + c.logger.Warningf("syncing secret %s failed: %v", util.NameFromMeta(secret.ObjectMeta), err) } - pwdUser := userMap[userKey] - - // if password rotation is enabled update password and username if rotation interval has been passed - if (c.OpConfig.EnablePasswordRotation && pwdUser.Origin != spec.RoleOriginInfrastructure && !pwdUser.IsDbOwner) || - util.SliceContains(c.Spec.UsersWithSecretRotation, secretUsername) || util.SliceContains(c.Spec.UsersWithInPlaceSecretRotation, secretUsername) { - currentTime := time.Now() - - // initialize password rotation setting first rotation date - nextRotationDateStr = string(secret.Data["nextRotation"]) - if nextRotationDate, err = time.Parse("2006-01-02 15:04:05", nextRotationDateStr); err != nil { - nextRotationDate, nextRotationDateStr = c.getNextRotationDate(currentTime) - secret.Data["nextRotation"] = []byte(nextRotationDateStr) - updateSecret = true - updateSecretMsg = fmt.Sprintf("rotation date not found in secret %q. Setting it to %s", secretSpec.Name, nextRotationDateStr) - } - - // check if next rotation can happen sooner - // if rotation interval has been decreased - currentRotationDate, _ := c.getNextRotationDate(currentTime) - if nextRotationDate.After(currentRotationDate) { - nextRotationDate = currentRotationDate - } - - // update password and next rotation date if configured interval has passed - if currentTime.After(nextRotationDate) { - // create rotation user if role is not listed for in-place password update - if !util.SliceContains(c.Spec.UsersWithInPlaceSecretRotation, secretUsername) { - rotationUser := pwdUser - newRotationUsername := secretUsername + currentTime.Format("060102") - rotationUser.Name = newRotationUsername - rotationUser.MemberOf = []string{secretUsername} - rotationUsers[newRotationUsername] = rotationUser - secret.Data["username"] = []byte(newRotationUsername) - - // whenever there is a rotation, check if old rotation users can be deleted - retentionUsers = append(retentionUsers, secretUsername) - } - secret.Data["password"] = []byte(util.RandomPassword(constants.PasswordLength)) - - _, nextRotationDateStr = c.getNextRotationDate(nextRotationDate) - secret.Data["nextRotation"] = []byte(nextRotationDateStr) - - updateSecret = true - updateSecretMsg = fmt.Sprintf("updating secret %q due to password rotation - next rotation date: %s", secretSpec.Name, nextRotationDateStr) - } - } else { - // username might not match if password rotation has been disabled again - if secretUsername != string(secret.Data["username"]) { - retentionUsers = append(retentionUsers, secretUsername) - secret.Data["username"] = []byte(secretUsername) - secret.Data["password"] = []byte(util.RandomPassword(constants.PasswordLength)) - secret.Data["nextRotation"] = []byte{} - updateSecret = true - updateSecretMsg = fmt.Sprintf("secret %s does not contain the role %s - updating username and resetting password", secretSpec.Name, secretUsername) - } - } - - // if this secret belongs to the infrastructure role and the password has changed - replace it in the secret - if pwdUser.Password != string(secret.Data["password"]) && pwdUser.Origin == spec.RoleOriginInfrastructure { - secret = secretSpec - updateSecret = true - updateSecretMsg = fmt.Sprintf("updating the secret %s from the infrastructure roles", secretSpec.Name) - } else { - // for non-infrastructure role - update the role with the password from the secret - pwdUser.Password = string(secret.Data["password"]) - userMap[userKey] = pwdUser - } - - if updateSecret { - c.logger.Debugln(updateSecretMsg) - if _, err = c.KubeClient.Secrets(secretSpec.Namespace).Update(context.TODO(), secret, metav1.UpdateOptions{}); err != nil { - c.logger.Warningf("could not update secret %q: %v", secretSpec.Name, err) - continue - } - c.Secrets[secret.UID] = secret - } - } else { - return fmt.Errorf("could not create secret for user %s: in namespace %s: %v", secretUsername, secretSpec.Namespace, err) + return fmt.Errorf("could not create secret for user %s: in namespace %s: %v", secretUsername, generatedSecretSpec.Namespace, err) } } // add new user with date suffix and use it in the secret of the original user if len(rotationUsers) > 0 { - err = c.initDbConn() + err := c.initDbConn() if err != nil { return fmt.Errorf("could not init db connection: %v", err) } @@ -756,7 +662,7 @@ func (c *Cluster) syncSecrets() error { // remove rotation users that exceed the retention interval if len(retentionUsers) > 0 { - err = c.initDbConn() + err := c.initDbConn() if err != nil { return fmt.Errorf("could not init db connection: %v", err) } @@ -771,6 +677,115 @@ func (c *Cluster) syncSecrets() error { return nil } +func (c *Cluster) updateSecret( + secretUsername string, + generatedSecret *v1.Secret, + secret *v1.Secret, + rotationUsers *spec.PgUserMap, + retentionUsers *[]string, + currentTime time.Time) error { + var ( + err error + updateSecret bool + updateSecretMsg string + nextRotationDate time.Time + nextRotationDateStr string + ) + + // fetch user map to update later + var userMap map[string]spec.PgUser + var userKey string + if secretUsername == c.systemUsers[constants.SuperuserKeyName].Name { + userKey = constants.SuperuserKeyName + userMap = c.systemUsers + } else if secretUsername == c.systemUsers[constants.ReplicationUserKeyName].Name { + userKey = constants.ReplicationUserKeyName + userMap = c.systemUsers + } else { + userKey = secretUsername + userMap = c.pgUsers + } + pwdUser := userMap[userKey] + secretName := util.NameFromMeta(secret.ObjectMeta) + + // if password rotation is enabled update password and username if rotation interval has been passed + if (c.OpConfig.EnablePasswordRotation && !pwdUser.IsDbOwner && + pwdUser.Origin != spec.RoleOriginInfrastructure && pwdUser.Origin != spec.RoleOriginSystem) || + util.SliceContains(c.Spec.UsersWithSecretRotation, secretUsername) || + util.SliceContains(c.Spec.UsersWithInPlaceSecretRotation, secretUsername) { + + // initialize password rotation setting first rotation date + nextRotationDateStr = string(secret.Data["nextRotation"]) + if nextRotationDate, err = time.ParseInLocation("2006-01-02 15:04:05", nextRotationDateStr, time.Local); err != nil { + nextRotationDate, nextRotationDateStr = c.getNextRotationDate(currentTime) + secret.Data["nextRotation"] = []byte(nextRotationDateStr) + updateSecret = true + updateSecretMsg = fmt.Sprintf("rotation date not found in secret %q. Setting it to %s", secretName, nextRotationDateStr) + } + + // check if next rotation can happen sooner + // if rotation interval has been decreased + currentRotationDate, _ := c.getNextRotationDate(currentTime) + if nextRotationDate.After(currentRotationDate) { + nextRotationDate = currentRotationDate + } + + // update password and next rotation date if configured interval has passed + if currentTime.After(nextRotationDate) { + // create rotation user if role is not listed for in-place password update + if !util.SliceContains(c.Spec.UsersWithInPlaceSecretRotation, secretUsername) { + rotationUser := pwdUser + newRotationUsername := secretUsername + currentTime.Format("060102") + rotationUser.Name = newRotationUsername + rotationUser.MemberOf = []string{secretUsername} + (*rotationUsers)[newRotationUsername] = rotationUser + secret.Data["username"] = []byte(newRotationUsername) + + // whenever there is a rotation, check if old rotation users can be deleted + *retentionUsers = append(*retentionUsers, secretUsername) + } + secret.Data["password"] = []byte(util.RandomPassword(constants.PasswordLength)) + + _, nextRotationDateStr = c.getNextRotationDate(nextRotationDate) + secret.Data["nextRotation"] = []byte(nextRotationDateStr) + + updateSecret = true + updateSecretMsg = fmt.Sprintf("updating secret %q due to password rotation - next rotation date: %s", secretName, nextRotationDateStr) + } + } else { + // username might not match if password rotation has been disabled again + if secretUsername != string(secret.Data["username"]) { + *retentionUsers = append(*retentionUsers, secretUsername) + secret.Data["username"] = []byte(secretUsername) + secret.Data["password"] = []byte(util.RandomPassword(constants.PasswordLength)) + secret.Data["nextRotation"] = []byte{} + updateSecret = true + updateSecretMsg = fmt.Sprintf("secret %s does not contain the role %s - updating username and resetting password", secretName, secretUsername) + } + } + + // if this secret belongs to the infrastructure role and the password has changed - replace it in the secret + if pwdUser.Password != string(secret.Data["password"]) && pwdUser.Origin == spec.RoleOriginInfrastructure { + secret = generatedSecret + updateSecret = true + updateSecretMsg = fmt.Sprintf("updating the secret %s from the infrastructure roles", secretName) + } else { + // for non-infrastructure role - update the role with the password from the secret + pwdUser.Password = string(secret.Data["password"]) + userMap[userKey] = pwdUser + } + + if updateSecret { + c.logger.Debugln(updateSecretMsg) + if _, err = c.KubeClient.Secrets(secret.Namespace).Update(context.TODO(), secret, metav1.UpdateOptions{}); err != nil { + return fmt.Errorf("could not update secret %q: %v", secretName, err) + } + c.Secrets[secret.UID] = secret + } + + return nil +} + func (c *Cluster) syncRoles() (err error) { c.setProcessName("syncing roles") diff --git a/pkg/cluster/sync_test.go b/pkg/cluster/sync_test.go index 3e3fc9318..65583298f 100644 --- a/pkg/cluster/sync_test.go +++ b/pkg/cluster/sync_test.go @@ -19,6 +19,7 @@ import ( "github.com/zalando/postgres-operator/mocks" acidv1 "github.com/zalando/postgres-operator/pkg/apis/acid.zalan.do/v1" fakeacidv1 "github.com/zalando/postgres-operator/pkg/generated/clientset/versioned/fake" + "github.com/zalando/postgres-operator/pkg/spec" "github.com/zalando/postgres-operator/pkg/util/config" "github.com/zalando/postgres-operator/pkg/util/k8sutil" "github.com/zalando/postgres-operator/pkg/util/patroni" @@ -26,6 +27,8 @@ import ( ) var patroniLogger = logrus.New().WithField("test", "patroni") +var acidClientSet = fakeacidv1.NewSimpleClientset() +var clientSet = fake.NewSimpleClientset() func newMockPod(ip string) *v1.Pod { return &v1.Pod{ @@ -36,9 +39,6 @@ func newMockPod(ip string) *v1.Pod { } func newFakeK8sSyncClient() (k8sutil.KubernetesClient, *fake.Clientset) { - acidClientSet := fakeacidv1.NewSimpleClientset() - clientSet := fake.NewSimpleClientset() - return k8sutil.KubernetesClient{ PodsGetter: clientSet.CoreV1(), PostgresqlsGetter: acidClientSet.AcidV1(), @@ -46,6 +46,12 @@ func newFakeK8sSyncClient() (k8sutil.KubernetesClient, *fake.Clientset) { }, clientSet } +func newFakeK8sSyncSecretsClient() (k8sutil.KubernetesClient, *fake.Clientset) { + return k8sutil.KubernetesClient{ + SecretsGetter: clientSet.CoreV1(), + }, clientSet +} + func TestSyncStatefulSetsAnnotations(t *testing.T) { testName := "test syncing statefulsets annotations" client, _ := newFakeK8sSyncClient() @@ -257,3 +263,72 @@ func TestCheckAndSetGlobalPostgreSQLConfiguration(t *testing.T) { } } } + +func TestUpdateSecret(t *testing.T) { + testName := "test syncing secrets" + client, _ := newFakeK8sSyncSecretsClient() + + clusterName := "acid-test-cluster" + namespace := "default" + username := "foo" + secretTemplate := config.StringTemplate("{username}.{cluster}.credentials") + rotationUsers := make(spec.PgUserMap) + retentionUsers := make([]string, 0) + yesterday := time.Now().AddDate(0, 0, -1) + + // new cluster with pvc storage resize mode and configured labels + var cluster = New( + Config{ + OpConfig: config.Config{ + Auth: config.Auth{ + SecretNameTemplate: secretTemplate, + EnablePasswordRotation: true, + PasswordRotationInterval: 1, + PasswordRotationUserRetention: 3, + }, + Resources: config.Resources{ + ClusterLabels: map[string]string{"application": "spilo"}, + ClusterNameLabel: "cluster-name", + }, + }, + }, client, acidv1.Postgresql{}, logger, eventRecorder) + + cluster.Name = clusterName + cluster.Namespace = namespace + cluster.pgUsers = map[string]spec.PgUser{} + cluster.Spec.Users = map[string]acidv1.UserFlags{username: {}} + cluster.initRobotUsers() + + // create a secret for user foo + cluster.syncSecrets() + + secret, err := cluster.KubeClient.Secrets(namespace).Get(context.TODO(), secretTemplate.Format("username", username, "cluster", clusterName), metav1.GetOptions{}) + assert.NoError(t, err) + generatedSecret := cluster.Secrets[secret.UID] + + // now update the secret setting next rotation date (yesterday + interval) + cluster.updateSecret(username, generatedSecret, secret, &rotationUsers, &retentionUsers, yesterday) + updatedSecret, err := cluster.KubeClient.Secrets(namespace).Get(context.TODO(), secretTemplate.Format("username", username, "cluster", clusterName), metav1.GetOptions{}) + assert.NoError(t, err) + + nextRotation := string(updatedSecret.Data["nextRotation"]) + _, nextRotationDate := cluster.getNextRotationDate(yesterday) + if nextRotation != nextRotationDate { + t.Errorf("%s: updated secret does not contain correct rotation date: expected %s, got %s", testName, nextRotationDate, nextRotation) + } + + // update secret again but use current time to trigger rotation + cluster.updateSecret(username, generatedSecret, updatedSecret, &rotationUsers, &retentionUsers, time.Now()) + updatedSecret, err = cluster.KubeClient.Secrets(namespace).Get(context.TODO(), secretTemplate.Format("username", username, "cluster", clusterName), metav1.GetOptions{}) + assert.NoError(t, err) + + if len(rotationUsers) != 1 && len(retentionUsers) != 1 { + t.Errorf("%s: unexpected number of users to rotate - expected only foo, found %d", testName, len(rotationUsers)) + } + + secretUsername := string(updatedSecret.Data["username"]) + rotatedUsername := username + time.Now().Format("060102") + if secretUsername != rotatedUsername { + t.Errorf("%s: updated secret does not contain correct username: expected %s, got %s", testName, rotatedUsername, secretUsername) + } +}