Allow underscore in identifiers, e.g.,`spec.users.name`

[kriswuollett]

Overview

Currently the underscore character, _, is not allowed in a username due to the reUsers regex, currently ^[a-z0-9]([-a-z0-9]*[a-z0-9])?$. As can be seen a dash, -, is currently not allowed. This is bothersome in postgres both because _ is allowed as well to be an identifier, as well as the use of - requires the name token to be a quoted identifier when writing scripts anyhow. The error message does not say why, nor is there any documentation attached to the regex constant for the same.

Use Case

Allow the use of underscore in postgres identifiers where appropriate.

Environment

• Kubernetes v1.22.4
• Terraform v1.0.4
• Docker Desktop 4.3.1 macOS
• Postgres 14

Additional Information

Error Encountered

Error: spec.users.name
│ 
│   with [REDACTED...].kubernetes_manifest.database,
│   on  [REDACTED...], in resource "kubernetes_manifest" "database":
│   50: resource "kubernetes_manifest" "database" {
│ 
│ Invalid value: "core_user_admin": spec.users.name in body should match '^[a-z0-9]([-a-z0-9]*[a-z0-9])?$'

Assumptions

I'm assuming the reUsers regex was created with the allowable characters for a Kubernetes metadata.name was taken in mind (RFC 1123) for generated things like the db1-pguser-db1-admin. Its unfortunate to apply restrictions of the wrapper infrastructure to the wrapped service.

But since the Secret already has labels like the following already, the human operator should be able to search for:

postgres-operator.crunchydata.com/cluster: db1
postgres-operator.crunchydata.com/pguser: db1_admin
postgres-operator.crunchydata.com/role: pguser

Currently Terraform does not have a pair of functions to convert to or from something simple like ASCII <-> hex, so giving a name to things like substr(md5(identifier), 8) like Git short commit IDs could be an okay way to generate the part of the Kubernetes identifier that represents the postgres identifier, even though that is a one-way function.

So the instead of having Terraform like:

data "kubernetes_secret_v1" "db1_admin_pguser_secret" {
  metadata {
    name      = "${var.cluster}-pguser-${var.username}"
    namespace = var.cluster_namespace
  }
}

It would be only slightly more complicated when name would be like db1-pguser-c90c0a74 for user db1_admin:

data "kubernetes_secret_v1" "db1_admin_pguser_secret" {
  metadata {
    name      = "${var.cluster}-pguser-${substr(md5(identifier), var.username, 8)}"
    namespace = var.cluster_namespace
  }
}

[kriswuollett]

Feel free to close this issue as works as intended, as it seems a reasonable tradeoff when this operator doesn't also do full life cycle management of user and role memberships which can be done externally or one time with databaseInitSQL. However, adding a bit more to the error message would be greatly informative for new users.

[jkatz]

I think another possibility is we could just map _ to - in the Secret name.

[cbandy]

I think another possibility is we could just map _ to - in the Secret name.

Then we would have to detect and reject collisions. e.g.

spec:
  users:
    - name: "two-users"
    - name: "two_users"

The spec.users field uses Kubernetes' list-map type so one can patch individual users. Kubernetes validates that spec.users.name values are unique at this time.

https://github.com/CrunchyData/postgres-operator/blob/ba6183b15486a9f0667355ae1201dd1d6fae4c77/config/crd/bases/postgres-operator.crunchydata.com_postgresclusters.yaml#L7591-L7594 https://github.com/CrunchyData/postgres-operator/blob/ba6183b15486a9f0667355ae1201dd1d6fae4c77/pkg/apis/postgres-operator.crunchydata.com/v1beta1/postgrescluster_types.go#L171-L174

[andrewlecuyer]

Hi @kriswuollett! Just quickly noting that a story has been added to the PGO backlog to consider adding support for underscores (as described in your issue) in a future PGO release.