Skip to content

Commit

Permalink
Fix failing VCAP_SERVICE credentials check (#4084)
Browse files Browse the repository at this point in the history
* Fix invalid VCAP_SERVICE credentials check
- Fixes 4081
- When Stratos is deployed via CF and bound to a db service we search VCAP_SERVICE for db credentials
- If we don't find the right properties we fall back on the uri and try to extract them from there
- The check for the right properties was failing due to the following concept
```
fmt.Sprintf("%v", nil) == "<nil>"
```
- Fix is to ensure we pass in better values
Also
- Tidied up and improved logging
- Improved handling of uri reg ex

* Fix backend unit tests

* Fall back on default port of provider
  • Loading branch information
richard-cox authored and nwmac committed Jan 9, 2020
1 parent 6624d26 commit 26a3afa
Show file tree
Hide file tree
Showing 3 changed files with 58 additions and 25 deletions.
70 changes: 52 additions & 18 deletions src/jetstream/datastore/database_cf_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package datastore

import (
"encoding/json"
"errors"
"fmt"
"regexp"
"strconv"
Expand Down Expand Up @@ -71,21 +72,21 @@ func findDatabaseConfig(vcapServices map[string][]VCAPService, db *DatabaseConfi
if len(service.Name) > 0 {
dbCredentials := service.Credentials

log.Infof("Attempting to apply Cloud Foundry database service config from credentials")
log.Infof("Attempting to apply Cloud Foundry database service config from VCAP_SERVICES credentials")

// 1) Check db config in credentials
db.Username = fmt.Sprintf("%v", dbCredentials["username"])
db.Password = fmt.Sprintf("%v", dbCredentials["password"])
db.Host = fmt.Sprintf("%v", dbCredentials["hostname"])
db.Username = getDBCredentialsValue(dbCredentials["username"])
db.Password = getDBCredentialsValue(dbCredentials["password"])
db.Host = getDBCredentialsValue(dbCredentials["hostname"])
db.SSLMode = "disable"
db.Port, _ = strconv.Atoi(fmt.Sprintf("%v", dbCredentials["port"]))
db.Port, _ = strconv.Atoi(getDBCredentialsValue(dbCredentials["port"]))
// Note - Both isPostgresService and isMySQLService look at the credentials uri & tags
if isPostgresService(service) {
db.DatabaseProvider = "pgsql"
db.Database = fmt.Sprintf("%v", dbCredentials["dbname"])
db.Database = getDBCredentialsValue(dbCredentials["dbname"])
} else if isMySQLService(service) {
db.DatabaseProvider = "mysql"
db.Database = fmt.Sprintf("%v", dbCredentials["name"])
db.Database = getDBCredentialsValue(dbCredentials["name"])
} else {
log.Infof("Cloud Foundry database service contains unsupported db type")
return false
Expand All @@ -94,18 +95,23 @@ func findDatabaseConfig(vcapServices map[string][]VCAPService, db *DatabaseConfi

if err != nil {
// 2) Check for db config in credentials uri
log.Infof("Failed to find required Cloud Foundry database service config, falling back on credential's `%v`", DB_URI)
uri := fmt.Sprintf("%v", dbCredentials[DB_URI])
log.Infof("Failed to find required Cloud Foundry database service config, falling back on credential's `%v`\n%v", DB_URI, err)
uri := getDBCredentialsValue(dbCredentials[DB_URI])
if len(uri) == 0 {
log.Warnf("Failed to find Cloud Foundry service credential's `%v`", DB_URI)
return false
}

db.Username, db.Password, db.Host, db.Port, db.Database = findDatabaseConfigurationFromUri(uri)
err := validateRequiredDatabaseParams(db.Username, db.Password, db.Database, db.Host, db.Port)
db.Username, db.Password, db.Host, db.Port, db.Database, err = findDatabaseConfigurationFromURI(uri, defaultDBProviderPort(service))

if err != nil {
log.Warnf("Failed to find Cloud Foundry service config from `%v` (failed to parse)", DB_URI)
return false
}

err := validateRequiredDatabaseParams(db.Username, db.Password, db.Database, db.Host, db.Port)
if err != nil {
log.Warnf("Failed to find Cloud Foundry service config's from `%v`", DB_URI)
log.Warnf("Failed to find Cloud Foundry service config from `%v` (missing values)\n%v", DB_URI, err)
return false
}
}
Expand All @@ -116,6 +122,14 @@ func findDatabaseConfig(vcapServices map[string][]VCAPService, db *DatabaseConfi
return false
}

func getDBCredentialsValue(val interface{}) string {
// First ensure that the value is not null, otherwise fmt print will convert to the string "<nil>"
if val == nil {
val = ""
}
return fmt.Sprintf("%v", val)
}

func findDatabaseConfigurations(vcapServices map[string][]VCAPService) map[string]VCAPService {
var configs map[string]VCAPService
configs = make(map[string]VCAPService)
Expand All @@ -133,12 +147,12 @@ func findDatabaseConfigurations(vcapServices map[string][]VCAPService) map[strin
}

func isPostgresService(service VCAPService) bool {
uri := fmt.Sprintf("%v", service.Credentials[DB_URI])
uri := getDBCredentialsValue(service.Credentials[DB_URI])
return strings.HasPrefix(uri, URI_POSTGRES) || stringInSlice(TAG_POSTGRES, service.Tags)
}

func isMySQLService(service VCAPService) bool {
uri := fmt.Sprintf("%v", service.Credentials[DB_URI])
uri := getDBCredentialsValue(service.Credentials[DB_URI])
return strings.HasPrefix(uri, URI_MYSQL) || stringInSlice(TAG_MYSQL, service.Tags)
}

Expand All @@ -151,10 +165,15 @@ func stringInSlice(a string, list []string) bool {
return false
}

func findDatabaseConfigurationFromUri(uri string) (string, string, string, int, string) {
func findDatabaseConfigurationFromURI(uri string, defaultPort int) (string, string, string, int, string, error) {
re := regexp.MustCompile(`(?P<provider>.+)://(?P<username>[^:]+)(?::(?P<password>.+))?@(?P<host>[^:]+)(?::(?P<port>.+))?\/(?P<dbname>.+)`)
n1 := re.SubexpNames()
r2 := re.FindAllStringSubmatch(uri, -1)[0]
matches := re.FindAllStringSubmatch(uri, -1)
if len(matches) < 1 {
return "", "", "", 0, "", errors.New("Failed to parse database URI")
}

r2 := matches[0]
md := map[string]string{}
for i, n := range r2 {
md[n1[i]] = n
Expand All @@ -163,9 +182,24 @@ func findDatabaseConfigurationFromUri(uri string) (string, string, string, int,
username := md["username"]
password := md["password"]
host := md["host"]
port, _ := strconv.Atoi(fmt.Sprintf("%v", md["port"]))
portStr := fmt.Sprintf("%v", md["port"])
var port int
if portStr != "<nil>" {
port, _ = strconv.Atoi(portStr)
} else {
port = defaultPort
}
dbname := md["dbname"]

return username, password, host, port, dbname
return username, password, host, port, dbname, nil

}

func defaultDBProviderPort(service VCAPService) int {
if isPostgresService(service) {
return 5432
} else if isMySQLService(service) {
return 3306
}
return 0
}
5 changes: 2 additions & 3 deletions src/jetstream/datastore/datastore.go
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,6 @@ func NewDatabaseConnectionParametersFromConfig(dc DatabaseConfig) (DatabaseConfi
}

// Database Config validation - check required values and the SSL Mode

err := validateRequiredDatabaseParams(dc.Username, dc.Password, dc.Database, dc.Host, dc.Port)
if err != nil {
return dc, err
Expand All @@ -122,8 +121,8 @@ func validateRequiredDatabaseParams(username, password, database, host string, p
err = vala.BeginValidation().Validate(
vala.IsNotNil(username, "username"),
vala.IsNotNil(password, "password"),
vala.IsNotNil(database, "database"),
vala.IsNotNil(host, "host"),
vala.IsNotNil(database, "database name"),
vala.IsNotNil(host, "host/hostname"),
vala.GreaterThan(port, 0, "port"),
vala.Not(vala.GreaterThan(port, 65535, "port")),
).Check()
Expand Down
8 changes: 4 additions & 4 deletions src/jetstream/datastore/datastore_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ func TestDatastore(t *testing.T) {
Convey("when database is missing", func() {

mockDatabaseConfigNoSSL.Database = ""
expectedErrorMessage := "parameter validation failed: Parameter was nil: database"
expectedErrorMessage := "parameter validation failed: Parameter was nil: database name"

Convey("an error should be returned", func() {
_, err := NewDatabaseConnectionParametersFromConfig(mockDatabaseConfigNoSSL)
Expand All @@ -156,7 +156,7 @@ func TestDatastore(t *testing.T) {
Convey("when host is missing", func() {

mockDatabaseConfigNoSSL.Host = ""
expectedErrorMessage := "parameter validation failed: Parameter was nil: host"
expectedErrorMessage := "parameter validation failed: Parameter was nil: host/hostname"

Convey("an error should be returned", func() {
_, err := NewDatabaseConnectionParametersFromConfig(mockDatabaseConfigNoSSL)
Expand Down Expand Up @@ -335,7 +335,7 @@ func TestDatastore(t *testing.T) {
Convey("when database is missing", func() {

mockDatabaseConfigSSL.Database = ""
expectedErrorMessage := "parameter validation failed: Parameter was nil: database"
expectedErrorMessage := "parameter validation failed: Parameter was nil: database name"

Convey("an error should be returned", func() {
_, err := NewDatabaseConnectionParametersFromConfig(mockDatabaseConfigSSL)
Expand All @@ -346,7 +346,7 @@ func TestDatastore(t *testing.T) {
Convey("when host is missing", func() {

mockDatabaseConfigSSL.Host = ""
expectedErrorMessage := "parameter validation failed: Parameter was nil: host"
expectedErrorMessage := "parameter validation failed: Parameter was nil: host/hostname"

Convey("an error should be returned", func() {
_, err := NewDatabaseConnectionParametersFromConfig(mockDatabaseConfigSSL)
Expand Down

0 comments on commit 26a3afa

Please sign in to comment.