-
Notifications
You must be signed in to change notification settings - Fork 128
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
Added TLS configuration for acra-rollback/acra-rotate #623
Conversation
Added TLS configuration for acra-rotate/acra-rollback
Added RegisterBaseTLSFlags
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if we parse connectionString
, then maybe GetConnectionStringHost
will be more precise? Or GetDriverConnectionStringHost
to note that it is driver specific...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense!
}) | ||
|
||
t.Run("MySQL invalid connection URL", func(t *testing.T) { | ||
url := "test:test@tcp://localhost:3306/test" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
@@ -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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CLI arg we should leave as is because it is backward incompatible change
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
func TestGetDBURLHost(t *testing.T) { | |
func TestGetDriverConnectionStringHost(t *testing.T) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
Bump boto3 version
Added TLS configuration for
acra-rotate/acra-rollback
Remove usage oflib/pg
to usepgx
Checklist
with new changes