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

Fix the password parsing issue #38

Closed

Conversation

ramya-bangera
Copy link

Escaping an already escaped password is causing the migrate container to error out with the following error.

 k -n discovery-service logs ds-td-snow-mig-up-8088-38-80-113830-wgqrq
Defaulted container "migration" out of: migration, migration-source (init), migration-check (init)
{"level":"info","msg":"error: pq: password authentication failed for user \"discovery_service_user\"","time":"2024-10-08T11:46:45Z"}

Added the fix to Queryescape user and password only if it is not already escaped.

Queryescape user and password only if it is not already escaped.
Comment on lines +15 to +16
input: "hello%20world",
expected: "hello%20world",
Copy link

Choose a reason for hiding this comment

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

The flaw with this approach is a set of characters in a password like %20 would not be interpreted correctly.

@@ -67,10 +67,20 @@ func printUsageAndExit() {

func dbMakeConnectionString(driver, user, password, address, name, ssl string) string {
return fmt.Sprintf("%s://%s:%s@%s/%s?sslmode=%s",
Copy link

Choose a reason for hiding this comment

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

Consider using URL.String() to assemble this string. It will apply the required escaping. url.UserPassword() will encapsulate the username and password.

@ramya-bangera
Copy link
Author

Closing this in favour of ticket: https://infoblox.atlassian.net/browse/DEVOPS-28825

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

Successfully merging this pull request may close these issues.

2 participants