Added TLS configuration for acra-rollback/acra-rotate #623
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Added TLS configuration for acra-rotate/acra-rollback
Added RegisterBaseTLSFlags
Lagovas
reviewed
Jan 19, 2023
cmd/acra-rollback/acra-rollback.go
Outdated
| "os" | ||
| "path/filepath" | ||
| "strings" | ||
|
|
||
| "github.com/go-sql-driver/mysql" | ||
| _ "github.com/go-sql-driver/mysql" |
There was a problem hiding this comment.
we can remove it if already imported it
cmd/acra-rollback/acra-rollback.go
Outdated
| } else { | ||
| config, err := pgx.ParseConfig(*connectionString) | ||
| if err != nil { | ||
| log.WithError(err).Errorln("Can't connect to db") |
There was a problem hiding this comment.
maybe we can't parse config here?
network/utils.go
Outdated
| @@ -242,3 +242,33 @@ func isFlagSet(name string, flagset *flag.FlagSet) bool { | |||
| }) | |||
| return set | |||
| } | |||
|
|
|||
| // GetDBURLHost return DB host from MySQL/PostgreSQL connection string to use as SNI | |||
There was a problem hiding this comment.
lets note that it parses MySQL/PostgreSQL driver specific connection strings. Because it can be changed in the future.
network/utils.go
Outdated
| // GetDBURLHost return DB host from MySQL/PostgreSQL connection string to use as SNI | ||
| // PostgreSQL - postgresql://{user}:{password}@{host}:{port}/{dbname} | ||
| // MySQL - ({user}:{password}@tcp({host}:{port})/{dbname} | ||
| func GetDBURLHost(connectionString string, useMySQL bool) (string, error) { |
There was a problem hiding this comment.
if we parse connectionString, then maybe GetConnectionStringHost will be more precise? Or GetDriverConnectionStringHost to note that it is driver specific...
| }) | ||
|
|
||
| t.Run("MySQL invalid connection URL", func(t *testing.T) { | ||
| url := "test:test@tcp://localhost:3306/test" |
There was a problem hiding this comment.
lets test test:test@tcp:localhost:3306/test and test:test@localhost:3306/test values with useMysql=True. And some PostgreSQL specific URLs.
And MySQL specific strings with useMysql=false to test reverse conditions.
Fixed after review
Fixed failing tests
Downgrade pgx version
Lagovas
reviewed
Jan 19, 2023
| @@ -56,7 +56,7 @@ func main() { | |||
| fileMapConfig := flag.String("file_map_config", "", "Path to file with map of <ClientId>: <FilePaths> in json format {\"client_id1\": [\"filepath1\", \"filepath2\"], \"client_id2\": [\"filepath1\", \"filepath2\"]}") | |||
| sqlSelect := flag.String("sql_select", "", "Select query with ? as placeholders where last columns in result must be ClientId and AcraStruct. Other columns will be passed into insert/update query into placeholders") | |||
| sqlUpdate := flag.String("sql_update", "", "Insert/Update query with ? as placeholder where into first will be placed rotated AcraStruct") | |||
| connectionString := flag.String("connection_string", "", "Connection string for DB PostgreSQL(postgresql://{user}:{password}@{host}:{port}/{dbname}?sslmode={sslmode}), MySQL ({user}:{password}@tcp({host}:{port})/{dbname})") | |||
| connectionString := flag.String("db_connection_string", "", "Connection string for DB PostgreSQL(postgresql://{user}:{password}@{host}:{port}/{dbname}?sslmode={sslmode}), MySQL ({user}:{password}@tcp({host}:{port})/{dbname})") | |||
There was a problem hiding this comment.
CLI arg we should leave as is because it is backward incompatible change
Lagovas
reviewed
Jan 20, 2023
network/utils_test.go
Outdated
| @@ -79,3 +81,50 @@ func TestSafeCloseConnection(t *testing.T) { | |||
| t.Fatal("Close method return incorrect error") | |||
| } | |||
| } | |||
|
|
|||
| func TestGetDBURLHost(t *testing.T) { | |||
There was a problem hiding this comment.
Suggested change
| func TestGetDBURLHost(t *testing.T) { | |
| func TestGetDriverConnectionStringHost(t *testing.T) { |
Bump boto3 version
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Added TLS configuration for
acra-rotate/acra-rollbackRemove usage oflib/pgto usepgxChecklist
with new changes