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.

Sign up for GitHub

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: Quote profile json when passing to kando in go code #3261

Merged
merged 2 commits into from
Nov 28, 2024
Merged

fix: Quote profile json when passing to kando in go code #3261

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2ad733d
fix: Quote profile json when passing to kando in go code
hairyhum Nov 25, 2024
b914091
Merge branch 'master' into unsafe-quoting
mergify[bot] Nov 28, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions pkg/function/export_rds_snapshot_location.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
"context"
"encoding/json"
"fmt"
"strconv"
"strings"
"time"

Expand Down Expand Up @@ -435,6 +436,8 @@ func postgresBackupCommand(dbEndpoint, username, password string, dbList []strin
return nil, errkit.New("No database found to backup")
}

profileQuoted := strconv.Quote(string(profile))

command := []string{
"bash",
"-o",
Expand All @@ -452,9 +455,9 @@ func postgresBackupCommand(dbEndpoint, username, password string, dbList []strin
for db in "${dblist[@]}";
do echo "backing up $db db" && pg_dump $db -C --inserts > /backup/$db.sql;
done
tar -zc backup | kando location push --profile '%s' --path "${BACKUP_PREFIX}/${BACKUP_ID}" -
tar -zc backup | kando location push --profile %s --path "${BACKUP_PREFIX}/${BACKUP_ID}" -
kando output %s ${BACKUP_ID}`,
dbEndpoint, backupPrefix, backupID, strings.Join(dbList, " "), profile, ExportRDSSnapshotToLocBackupID),
dbEndpoint, backupPrefix, backupID, strings.Join(dbList, " "), profileQuoted, ExportRDSSnapshotToLocBackupID),
Dismissed Show dismissed Hide dismissed
}
return command, nil
}
Expand Down
28 changes: 25 additions & 3 deletions pkg/function/rds_functions_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,10 +60,32 @@ func (s *RDSFunctionsTest) TestPrepareCommand(c *check.C) {
command: []string{"bash", "-o", "errexit", "-o", "pipefail", "-c",
fmt.Sprintf(`
export PGHOST=%s
kando location pull --profile '%s' --path "%s" - | gunzip -c -f | sed 's/"LOCALE"/"LC_COLLATE"/' | psql -q -U "${PGUSER}" %s
kando location pull --profile "%s" --path "%s" - | gunzip -c -f | sed 's/"LOCALE"/"LC_COLLATE"/' | psql -q -U "${PGUSER}" %s
`, "db-endpoint", "null", fmt.Sprintf("%s/%s", "/backup/postgres-backup", "backup-id"), postgres.DefaultConnectDatabase),
},
},
{
name: "PostgreS restore command with profile",
dbEngine: PostgrSQLEngine,
action: RestoreAction,
dbEndpoint: "db-endpoint",
username: "test-user",
password: "secret-pass",
backupPrefix: "/backup/postgres-backup",
backupID: "backup-id",
dbEngineVersion: "12.7",
errChecker: check.IsNil,
dbList: []string{"template1"},
command: []string{"bash", "-o", "errexit", "-o", "pipefail", "-c",
fmt.Sprintf(`
export PGHOST=%s
kando location pull --profile "{\"Location\":{\"type\":\"\",\"bucket\":\"\",\"endpoint\":\"\",\"prefix\":\"\",\"region\":\"\"},\"Credential\":{\"Type\":\"\",\"KeyPair\":null,\"Secret\":null,\"KopiaServerSecret\":null},\"SkipSSLVerify\":false}" --path "%s" - | gunzip -c -f | sed 's/"LOCALE"/"LC_COLLATE"/' | psql -q -U "${PGUSER}" %s
`, "db-endpoint", fmt.Sprintf("%s/%s", "/backup/postgres-backup", "backup-id"), postgres.DefaultConnectDatabase),
},
tp: param.TemplateParams{
Profile: &param.Profile{},
},
},
{
name: "PostgreS restore command",
dbEngine: PostgrSQLEngine,
Expand All @@ -79,7 +101,7 @@ func (s *RDSFunctionsTest) TestPrepareCommand(c *check.C) {
command: []string{"bash", "-o", "errexit", "-o", "pipefail", "-c",
fmt.Sprintf(`
export PGHOST=%s
kando location pull --profile '%s' --path "%s" - | gunzip -c -f | psql -q -U "${PGUSER}" %s
kando location pull --profile "%s" --path "%s" - | gunzip -c -f | psql -q -U "${PGUSER}" %s
`, "db-endpoint", "null", fmt.Sprintf("%s/%s", "/backup/postgres-backup", "backup-id"), postgres.DefaultConnectDatabase),
},
},
Expand All @@ -106,7 +128,7 @@ func (s *RDSFunctionsTest) TestPrepareCommand(c *check.C) {
for db in "${dblist[@]}";
do echo "backing up $db db" && pg_dump $db -C --inserts > /backup/$db.sql;
done
tar -zc backup | kando location push --profile '%s' --path "${BACKUP_PREFIX}/${BACKUP_ID}" -
tar -zc backup | kando location push --profile "%s" --path "${BACKUP_PREFIX}/${BACKUP_ID}" -
Copy link
Contributor

Choose a reason for hiding this comment

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

I see that at some places we are providing profile in double quotes "" and at some place without. Is that intentional?

Copy link
Contributor Author

@hairyhum hairyhum Nov 28, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

In some tests we just pass "null" string to this function. We can make it more consistent by passing "\"null\"" instead, I guess.

Copy link
Contributor

Choose a reason for hiding this comment

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

I am good, even if you want to merge this with current state.

kando output %s ${BACKUP_ID}`,
"db-endpoint", "/backup/postgres-backup", "backup-id", strings.Join([]string{"template1"}, " "), "null", ExportRDSSnapshotToLocBackupID),
},
Expand Down
7 changes: 5 additions & 2 deletions pkg/function/restore_rds_snapshot.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ package function
import (
"context"
"fmt"
"strconv"
"time"

"github.com/aws/aws-sdk-go/aws"
Expand Down Expand Up @@ -335,6 +336,8 @@ func postgresRestoreCommand(pgHost, username, password string, backupArtifactPre
replaceCommand = ` sed 's/"LOCALE"/"LC_COLLATE"/' |`
}

profileQuoted := strconv.Quote(string(profile))

return []string{
"bash",
"-o",
Expand All @@ -344,8 +347,8 @@ func postgresRestoreCommand(pgHost, username, password string, backupArtifactPre
"-c",
fmt.Sprintf(`
export PGHOST=%s
kando location pull --profile '%s' --path "%s" - | gunzip -c -f |%s psql -q -U "${PGUSER}" %s
`, pgHost, profile, fmt.Sprintf("%s/%s", backupArtifactPrefix, backupID), replaceCommand, postgres.DefaultConnectDatabase),
kando location pull --profile %s --path "%s" - | gunzip -c -f |%s psql -q -U "${PGUSER}" %s
`, pgHost, profileQuoted, fmt.Sprintf("%s/%s", backupArtifactPrefix, backupID), replaceCommand, postgres.DefaultConnectDatabase),
}, nil
}

Expand Down