Skip to content

allow users to opt out from globally enabled secret rotation #2528

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Feb 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions charts/postgres-operator/crds/postgresqls.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -612,6 +612,11 @@ spec:
- SUPERUSER
- nosuperuser
- NOSUPERUSER
usersIgnoringSecretRotation:
type: array
nullable: true
items:
type: string
usersWithInPlaceSecretRotation:
type: array
nullable: true
Expand Down
17 changes: 17 additions & 0 deletions docs/administrator.md
Original file line number Diff line number Diff line change
Expand Up @@ -355,6 +355,23 @@ This would be the recommended option to enable rotation in secrets of database
owners, but only if they are not used as application users for regular read
and write operations.

### Ignore rotation for certain users

If you wish to globally enable password rotation but need certain users to
opt out from it there are two ways. First, you can remove the user from the
manifest's `users` section. The corresponding secret to this user will no
longer be synced by the operator then.

Secondly, if you want the operator to continue syncing the secret (e.g. to
recreate if it got accidentally removed) but cannot allow it being rotated,
add the user to the following list in your manifest:

```
spec:
usersIgnoringSecretRotation:
- bar_user
```

### Turning off password rotation

When password rotation is turned off again the operator will check if the
Expand Down
8 changes: 8 additions & 0 deletions docs/reference/cluster_manifest.md
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,14 @@ These parameters are grouped directly under the `spec` key in the manifest.
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-users).

* **usersIgnoringSecretRotation**
if you have secret rotation enabled globally you can define a list of
of users that should opt out from it, for example if you store credentials
outside of K8s, too, and corresponding deployments cannot dynamically
reference secrets. Note, you can also opt out from the rotation by removing
users from the manifest's `users` section. The operator will not drop them
from the database. Optional.

* **databases**
a map of database names to database owners for the databases that should be
created by the operator. The owner users should already exist on the cluster
Expand Down
14 changes: 12 additions & 2 deletions e2e/tests/test_e2e.py
Original file line number Diff line number Diff line change
Expand Up @@ -1578,15 +1578,18 @@ def test_password_rotation(self):
today = date.today()

# enable password rotation for owner of foo database
pg_patch_inplace_rotation_for_owner = {
pg_patch_rotation_single_users = {
"spec": {
"usersIgnoringSecretRotation": [
"test.db_user"
],
"usersWithInPlaceSecretRotation": [
"zalando"
]
}
}
k8s.api.custom_objects_api.patch_namespaced_custom_object(
"acid.zalan.do", "v1", "default", "postgresqls", "acid-minimal-cluster", pg_patch_inplace_rotation_for_owner)
"acid.zalan.do", "v1", "default", "postgresqls", "acid-minimal-cluster", pg_patch_rotation_single_users)
self.eventuallyEqual(lambda: k8s.get_operator_state(), {"0": "idle"}, "Operator does not get in sync")

# check if next rotation date was set in secret
Expand Down Expand Up @@ -1675,6 +1678,13 @@ def test_password_rotation(self):
self.eventuallyEqual(lambda: len(self.query_database_with_user(leader.metadata.name, "postgres", "SELECT 1", "foo_user")), 1,
"Could not connect to the database with rotation user {}".format(rotation_user), 10, 5)

# check if rotation has been ignored for user from test_cross_namespace_secrets test
db_user_secret = k8s.get_secret(username="test.db_user", namespace="test")
secret_username = str(base64.b64decode(db_user_secret.data["username"]), 'utf-8')

self.assertEqual("test.db_user", secret_username,
"Unexpected username in secret of test.db_user: expected {}, got {}".format("test.db_user", secret_username))

# disable password rotation for all other users (foo_user)
# and pick smaller intervals to see if the third fake rotation user is dropped
enable_password_rotation = {
Expand Down
2 changes: 2 additions & 0 deletions manifests/complete-postgres-manifest.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ spec:
- createdb
foo_user: []
# flyway: []
# usersIgnoringSecretRotation:
# - bar_user
# usersWithSecretRotation:
# - foo_user
# usersWithInPlaceSecretRotation:
Expand Down
5 changes: 5 additions & 0 deletions manifests/postgresql.crd.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -610,6 +610,11 @@ spec:
- SUPERUSER
- nosuperuser
- NOSUPERUSER
usersIgnoringSecretRotation:
type: array
nullable: true
items:
type: string
usersWithInPlaceSecretRotation:
type: array
nullable: true
Expand Down
9 changes: 9 additions & 0 deletions pkg/apis/acid.zalan.do/v1/crds.go
Original file line number Diff line number Diff line change
Expand Up @@ -996,6 +996,15 @@ var PostgresCRDResourceValidation = apiextv1.CustomResourceValidation{
},
},
},
"usersIgnoringSecretRotation": {
Type: "array",
Nullable: true,
Items: &apiextv1.JSONSchemaPropsOrArray{
Schema: &apiextv1.JSONSchemaProps{
Type: "string",
},
},
},
"usersWithInPlaceSecretRotation": {
Type: "array",
Nullable: true,
Expand Down
1 change: 1 addition & 0 deletions pkg/apis/acid.zalan.do/v1/postgresql_type.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ type PostgresSpec struct {
AllowedSourceRanges []string `json:"allowedSourceRanges"`

Users map[string]UserFlags `json:"users,omitempty"`
UsersIgnoringSecretRotation []string `json:"usersIgnoringSecretRotation,omitempty"`
UsersWithSecretRotation []string `json:"usersWithSecretRotation,omitempty"`
UsersWithInPlaceSecretRotation []string `json:"usersWithInPlaceSecretRotation,omitempty"`

Expand Down
5 changes: 5 additions & 0 deletions pkg/apis/acid.zalan.do/v1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

18 changes: 11 additions & 7 deletions pkg/cluster/sync.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
"github.com/zalando/postgres-operator/pkg/util"
"github.com/zalando/postgres-operator/pkg/util/constants"
"github.com/zalando/postgres-operator/pkg/util/k8sutil"
"golang.org/x/exp/slices"
batchv1 "k8s.io/api/batch/v1"
v1 "k8s.io/api/core/v1"
policyv1 "k8s.io/api/policy/v1"
Expand Down Expand Up @@ -689,7 +690,7 @@ func (c *Cluster) checkAndSetGlobalPostgreSQLConfiguration(pod *v1.Pod, effectiv
effectiveValue := effectivePgParameters[desiredOption]
if isBootstrapOnlyParameter(desiredOption) && (effectiveValue != desiredValue) {
parametersToSet[desiredOption] = desiredValue
if util.SliceContains(requirePrimaryRestartWhenDecreased, desiredOption) {
if slices.Contains(requirePrimaryRestartWhenDecreased, desiredOption) {
effectiveValueNum, errConv := strconv.Atoi(effectiveValue)
desiredValueNum, errConv2 := strconv.Atoi(desiredValue)
if errConv != nil || errConv2 != nil {
Expand All @@ -705,7 +706,7 @@ func (c *Cluster) checkAndSetGlobalPostgreSQLConfiguration(pod *v1.Pod, effectiv
}

// check if there exist only config updates that require a restart of the primary
if len(restartPrimary) > 0 && !util.SliceContains(restartPrimary, false) && len(configToSet) == 0 {
if len(restartPrimary) > 0 && !slices.Contains(restartPrimary, false) && len(configToSet) == 0 {
requiresMasterRestart = true
}

Expand Down Expand Up @@ -873,14 +874,17 @@ func (c *Cluster) updateSecret(
// if password rotation is enabled update password and username if rotation interval has been passed
// rotation can be enabled globally or via the manifest (excluding the Postgres superuser)
rotationEnabledInManifest := secretUsername != constants.SuperuserKeyName &&
(util.SliceContains(c.Spec.UsersWithSecretRotation, secretUsername) ||
util.SliceContains(c.Spec.UsersWithInPlaceSecretRotation, secretUsername))
(slices.Contains(c.Spec.UsersWithSecretRotation, secretUsername) ||
slices.Contains(c.Spec.UsersWithInPlaceSecretRotation, secretUsername))

// globally enabled rotation is only allowed for manifest and bootstrapped roles
allowedRoleTypes := []spec.RoleOrigin{spec.RoleOriginManifest, spec.RoleOriginBootstrap}
rotationAllowed := !pwdUser.IsDbOwner && util.SliceContains(allowedRoleTypes, pwdUser.Origin) && c.Spec.StandbyCluster == nil
rotationAllowed := !pwdUser.IsDbOwner && slices.Contains(allowedRoleTypes, pwdUser.Origin) && c.Spec.StandbyCluster == nil

if (c.OpConfig.EnablePasswordRotation && rotationAllowed) || rotationEnabledInManifest {
// users can ignore any kind of rotation
isIgnoringRotation := slices.Contains(c.Spec.UsersIgnoringSecretRotation, secretUsername)

if ((c.OpConfig.EnablePasswordRotation && rotationAllowed) || rotationEnabledInManifest) && !isIgnoringRotation {
updateSecretMsg, err = c.rotatePasswordInSecret(secret, secretUsername, pwdUser.Origin, currentTime, retentionUsers)
if err != nil {
c.logger.Warnf("password rotation failed for user %s: %v", secretUsername, err)
Expand Down Expand Up @@ -961,7 +965,7 @@ func (c *Cluster) rotatePasswordInSecret(
// 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) {
if !slices.Contains(c.Spec.UsersWithInPlaceSecretRotation, secretUsername) {
rotationUsername := fmt.Sprintf("%s%s", secretUsername, currentTime.Format(constants.RotationUserDateFormat))
secret.Data["username"] = []byte(rotationUsername)
c.logger.Infof("updating username in secret %s and creating rotation user %s in the database", secretName, rotationUsername)
Expand Down
7 changes: 6 additions & 1 deletion pkg/cluster/sync_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (

"context"

"golang.org/x/exp/slices"
v1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
Expand Down Expand Up @@ -634,7 +635,8 @@ func TestUpdateSecret(t *testing.T) {
},
Spec: acidv1.PostgresSpec{
Databases: map[string]string{dbname: dbowner},
Users: map[string]acidv1.UserFlags{"foo": {}, dbowner: {}},
Users: map[string]acidv1.UserFlags{"foo": {}, "bar": {}, dbowner: {}},
UsersIgnoringSecretRotation: []string{"bar"},
UsersWithInPlaceSecretRotation: []string{dbowner},
Streams: []acidv1.Stream{
{
Expand Down Expand Up @@ -712,6 +714,9 @@ func TestUpdateSecret(t *testing.T) {
if pgUser.Origin != spec.RoleOriginManifest {
continue
}
if slices.Contains(pg.Spec.UsersIgnoringSecretRotation, username) {
continue
}
t.Errorf("%s: password unchanged in updated secret for %s", testName, username)
}

Expand Down