Do not make creating a secret for PostgreSQL a prerequisite

[jianrongzhang89]

Description

Changes:

1. Check the backstage deployment and statefulset configuration, if the main container has an envFrom entry
   pointing to the secret reference with special name '{POSTGRESQL_SECRET}', a secret is auto generated, and the envFrom entry is updated to backstage-psql-secret-. Otherwise, we assume that the user has an existing secret provided and it will not be auto generated.
2. Add a new field existingDbSecret to the backstage CRD to explicitly indicate an existing database secret is used (this is the same way as the helm chart does).

Which issue(s) does this PR fix or relate to

• Fixes Do not make creating a secret for PostgreSQL a prerequisite #63

PR acceptance criteria

• [X ] Tests
• Documentation

How to test changes / Special notes to the reviewer

a) Deploy the operator and the dummy backstage CR (examples/bs1.yaml).
b) Check that the psql db and backstage are running.
c) Check that a secret backstage-psql-secret- has been created.
d) Delete the backstage CR and check and confirm that the secret gets deleted.
e)deploy examples/bs-existing-secret.yaml and check the backstage and plsql db are using the existing secret used without generating a new one.

[rm3l]

The generated secret does not have own-runtime set, ie, when the backstage CR is deleted, the secret will NOT be removed. This prevents the secret from being re-created should the CR is deleted and then redeployed.

About this, I'm wondering if we should not delete the generated secret as well. Otherwise, we might end up with a lot of secrets, especially if I delete my CR, then rename it before applying it. The data in the secret is random, so I guess it should not be a problem to recreate it, no?

[rm3l]

I'm curious to know why we lost the previous formatting. It was much more legible.

[jianrongzhang89]

Not sure what happened to the code generator, seems like kustomize issue.

[gazarenkov]

A couple notes:

I think, for the sake of consistency we need to take DB secret as a first class citizen object i.e:

• it has to have own (db-secret.yaml) default configuration and accordingly to be able to be overlayed with raw configuration
• (optionally if we decide so) it has to have dedicated Backstage CRD config (just secret name)

I do not think there are a lot of reasons to generate it, static initialization should be fine IMO (note it is a property of "local", namespace scoped Database, production is a different story)

[jianrongzhang89]

The generated secret does not have own-runtime set, ie, when the backstage CR is deleted, the secret will NOT be removed. This prevents the secret from being re-created should the CR is deleted and then redeployed.

About this, I'm wondering if we should not delete the generated secret as well. Otherwise, we might end up with a lot of secrets, especially if I delete my CR, then rename it before applying it. The data in the secret is random, so I guess it should not be a problem to recreate it, no?

Fair enough. Accepted.

[jianrongzhang89]

A couple notes:

I think, for the sake of consistency we need to take DB secret as a first class citizen object i.e:

• it has to have own (db-secret.yaml) default configuration and accordingly to be able to be overlayed with raw configuration
• (optionally if we decide so) it has to have dedicated Backstage CRD config (just secret name)

I do not think there are a lot of reasons to generate it, static initialization should be fine IMO (note it is a property of "local", namespace scoped Database, production is a different story)

Agreed to use the same approach for the psql secret as other objects. Meanwhile, I think it is worthwhile to generate a random password instead of a default one.

[gazarenkov]

@jianrongzhang89 well, I am not against generating per se, it just looks like it may cost too much.
Other methods of Backstage installation can live w/o it so let's start simple .

[rm3l]

@jianrongzhang89 well, I am not against generating per se, it just looks like it may cost too much. Other methods of Backstage installation can live w/o it so let's start simple .

IMO, it would make sense for the DB to have a random password value for each Backstage instance (for the reasons depicted in #63 (comment)).
Not sure I understand why it may cost too much. Could you please elaborate?
Also, I think other installation methods generate a random password by default if not set explicitly. That's what the Bitnami PostgreSQL Chart (which the Backstage Helm Chart depends on) seems to do in my understanding: https://github.com/bitnami/charts/blob/main/bitnami/postgresql/templates/secrets.yaml#L11

[jianrongzhang89]

@jianrongzhang89 well, I am not against generating per se, it just looks like it may cost too much. Other methods of Backstage installation can live w/o it so let's start simple .

IMO, it would make sense for the DB to have a random password value for each Backstage instance (for the reasons depicted in #63 (comment)). Not sure I understand why it may cost too much. Could you please elaborate? Also, I think other installation methods generate a random password by default if not set explicitly. That's what the Bitnami PostgreSQL Chart (which the Backstage Helm Chart depends on) seems to do in my understanding: https://github.com/bitnami/charts/blob/main/bitnami/postgresql/templates/secrets.yaml#L11

Also, if we do not generate the db password, users will be unable to use local psql db for production. Although they may use external db installations, I can image some smaller sized customers may simply want to use the local psql db instead.

[gazarenkov]

@jianrongzhang89 well, I am not against generating per se, it just looks like it may cost too much. Other methods of Backstage installation can live w/o it so let's start simple .

IMO, it would make sense for the DB to have a random password value for each Backstage instance (for the reasons depicted in #63 (comment)). Not sure I understand why it may cost too much. Could you please elaborate? Also, I think other installation methods generate a random password by default if not set explicitly. That's what the Bitnami PostgreSQL Chart (which the Backstage Helm Chart depends on) seems to do in my understanding: https://github.com/bitnami/charts/blob/main/bitnami/postgresql/templates/secrets.yaml#L11

Also, if we do not generate the db password, users will be unable to use local psql db for production. Although they may use external db installations, I can image some smaller sized customers may simply want to use the local psql db instead.

About using it for small group, I do not think it is directly related to the credential generation topic
It is still possible to install database with random credentials, the same as other configuration, just provide db-secret.yaml (for example) Secret, or specify this Secret in the Backstage.spec with generated credentials (as proposed), right?

@rm3l

Not sure I understand why it may cost too much. Could you please elaborate?

What we propose for the case when one want to change the generated credentials? Like, ok, I go to sql server and change it (as you mentioned) but how I change the Secret if after first generation we consider it as not changeable and => ignore its changing? Or I missed something?

[jianrongzhang89]

What we propose for the case when one want to change the generated credentials? Like, ok, I go to sql server and change it (as you mentioned) but how I change the Secret if after first generation we consider it as not changeable and => ignore its changing? Or I missed something?

@gazarenkov This is a good point. After the secret is generated, the user shall be able to change the passwords in the server, and then update the secret accordingly. Current implementation does not prevent the user from doing this as the secret gets generated only if it does not exist yet.

[gazarenkov]

The generated secret does not have own-runtime set, ie, when the backstage CR is deleted, the secret will NOT be removed. This prevents the secret from being re-created should the CR is deleted and then redeployed.

About this, I'm wondering if we should not delete the generated secret as well. Otherwise, we might end up with a lot of secrets, especially if I delete my CR, then rename it before applying it. The data in the secret is random, so I guess it should not be a problem to recreate it, no?

We are talking about the case when Operator generates the secret right?
In this case the generated secret is a part of operator.
So, when own-runtime = true (normally) it has to be fully synchronized with CR, i.e. it should be deleted along with CR.
Unlike the case when the secret is external - i.e. you point it as a part of CR.spec (including raw config), in this case operator does not own it and it will not be touched

[gazarenkov]

What we propose for the case when one want to change the generated credentials? Like, ok, I go to sql server and change it (as you mentioned) but how I change the Secret if after first generation we consider it as not changeable and => ignore its changing? Or I missed something?

@gazarenkov This is a good point. After the secret is generated, the user shall be able to change the passwords in the server, and then update the secret accordingly. Current implementation does not prevent the user from doing this as the secret gets generated only if it does not exist yet.

In a case when secret is generated with default configuration and synchronized with operator, I do not think existence of secret is a good criteria to decide (indeed when you create CR with defaults, secret does not exist always).
I would say (and that's how it is implemented in https://github.com/gazarenkov/janus-idp-operator/tree/next) empty password data field is better criteria.

[rm3l]

The generated secret does not have own-runtime set, ie, when the backstage CR is deleted, the secret will NOT be removed. This prevents the secret from being re-created should the CR is deleted and then redeployed.

About this, I'm wondering if we should not delete the generated secret as well. Otherwise, we might end up with a lot of secrets, especially if I delete my CR, then rename it before applying it. The data in the secret is random, so I guess it should not be a problem to recreate it, no?

We are talking about the case when Operator generates the secret right? In this case the generated secret is a part of operator. So, when own-runtime = true (normally) it has to be fully synchronized with CR, i.e. it should be deleted along with CR. Unlike the case when the secret is external - i.e. you point it as a part of CR.spec (including raw config), in this case operator does not own it and it will not be touched

Yes, this was about the case when the secret was generated. I think deletion is now handled accordingly in this PR. External secrets are not deleted when the CR is deleted.

[jianrongzhang89]

What we propose for the case when one want to change the generated credentials? Like, ok, I go to sql server and change it (as you mentioned) but how I change the Secret if after first generation we consider it as not changeable and => ignore its changing? Or I missed something?

@gazarenkov This is a good point. After the secret is generated, the user shall be able to change the passwords in the server, and then update the secret accordingly. Current implementation does not prevent the user from doing this as the secret gets generated only if it does not exist yet.

In a case when secret is generated with default configuration and synchronized with operator, I do not think existence of secret is a good criteria to decide (indeed when you create CR with defaults, secret does not exist always). I would say (and that's how it is implemented in https://github.com/gazarenkov/janus-idp-operator/tree/next) empty password data field is better criteria.

In such case, the user should be responsible to make sure the secret is correct. Also note that the secret may not already exist when the CR is created and the user may create it after the CR is already created.