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

database: avoid regenerating password (PROJQUAY-2319) #501

Merged
merged 1 commit into from
Aug 25, 2021
Merged

database: avoid regenerating password (PROJQUAY-2319) #501

merged 1 commit into from
Aug 25, 2021

Conversation

ricardomaraschini
Copy link
Contributor

Proceed to store database root password the same way we do for the
DB_URI. By doing that we avoid postgres redeployment during every
reconcile cycle.

Proceed to store database root password the same way we do for the
DB_URI. By doing that we avoid postgres redeployment during every
reconcile cycle.
Copy link

@crozzy crozzy left a comment

Choose a reason for hiding this comment

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

LGTM, from a code POV, just a couple nits

@@ -324,7 +333,7 @@ func KustomizationFor(ctx *quaycontext.QuayRegistryContext, quay *v1.QuayRegistr
componentPaths = append(componentPaths, filepath.Join("..", "components", string(component.Kind)))
managedFieldGroups = append(managedFieldGroups, fieldGroupNameFor(component.Kind))

componentConfigFiles, err := componentConfigFilesFor(component.Kind, quay, quayConfigFiles)
componentConfigFiles, err := componentConfigFilesFor(ctx, component.Kind, quay, quayConfigFiles)
Copy link

Choose a reason for hiding this comment

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

Nit: Maybe a different name here would be a good idea, ctx is traditionally always context.Context. qCtx or quayCtx might distinguish it more.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Following pre established standards here. I fully agree with you.

@@ -264,6 +264,14 @@ func KustomizationFor(ctx *quaycontext.QuayRegistryContext, quay *v1.QuayRegistr
return nil, err
}

if ctx.DbRootPw == "" {
rootpw, err := generateRandomString(32)
Copy link

Choose a reason for hiding this comment

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

Nit: maybe rootPw to be consistent with the rest of the PR

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, you are right. This was a copy and paste from a branch where I am working in refactoring this. I guess we can live with this until we land that other PR of mine.

Copy link
Collaborator

@jonathankingfc jonathankingfc left a comment

Choose a reason for hiding this comment

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

Approved

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants