Skip to content
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

Add a option to change the regexp for database name/schema validation #2193

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

flyingcamilo
Copy link
Contributor

@flyingcamilo flyingcamilo commented Jan 27, 2023

The previous setup adheres to the naming convention, and that's a good thing. Unfortunately, however, there are cases in which you have to break with them.

Unfortunately, that is our case. We use external software and have no influence on their naming.

@FxKu
Copy link
Member

FxKu commented Jan 27, 2023

Would rather like to see databaseNameRegexp being configurable. See #667

@flyingcamilo
Copy link
Contributor Author

@FxKu That sound reasonable. I can do it.

@flyingcamilo flyingcamilo changed the title Add flag to disable database name/schema validation. Add a option to change the regexp for database name/schema validation Jan 27, 2023
@flyingcamilo
Copy link
Contributor Author

Hey @FxKu added in your suggestion.

The previous setup adheres to the naming convention and that's a good thing. Unfortunately,
however, there are cases in which you have to break with them.

Closes: zalando#667
@flyingcamilo
Copy link
Contributor Author

@FxKu Not sure if the e2e test is failing because of me, but I guess not. Is there anything I can do ?

@FxKu FxKu added this to the 2.0 milestone Apr 19, 2023
@FxKu
Copy link
Member

FxKu commented Apr 20, 2023

@flyingcamilo thanks for adding the config options. What would now happen with the old handling? https://github.com/zalando/postgres-operator/blob/master/pkg/cluster/cluster.go#L44
https://github.com/zalando/postgres-operator/blob/master/pkg/cluster/database.go#L419

I don't see it touched in this PR.

Some more ToDos:

  • add the parameter also in go code
  • copy from opconfig to internal config

if err = c.readValidateDatabaseNameRegexp(c.OpConfig.DatabaseNameRegexp); err != nil {
return err
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should rather place the existing validation.

@@ -337,6 +341,7 @@ func (c *Cluster) Create() error {
// that feature explicitly
if !(c.databaseAccessDisabled() || c.getNumberOfInstances(&c.Spec) <= 0 || c.Spec.StandbyCluster != nil) {
c.logger.Infof("Create roles")

Copy link
Member

@FxKu FxKu Apr 20, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please do not introduce unrelated changes - even these are just blank lines

@FxKu FxKu modified the milestones: 1.11.0, 2.0 Feb 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Open Questions
Development

Successfully merging this pull request may close these issues.

None yet

2 participants