Skip to content

Commit

Permalink
add RDS MariaDB e2e tests (#40066)
Browse files Browse the repository at this point in the history
* add AWS RDS MariaDB e2e tests

* fix spurious teardown error logs

* if a role or user doesn't exist, that is not an error nor is it worth
  logging during auto user teardown.

* check for admin option

* add some extra secret fetching checkings

* only drop redshift user if the user exists
  • Loading branch information
GavinFrazar authored Apr 8, 2024
1 parent b234bfe commit 54d2e90
Show file tree
Hide file tree
Showing 7 changed files with 210 additions and 5 deletions.
1 change: 1 addition & 0 deletions .github/workflows/aws-e2e-tests-non-root.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ env:
RDS_DISCOVERY_ROLE: arn:aws:iam::307493967395:role/ci-database-e2e-tests-rds-discovery
RDS_POSTGRES_INSTANCE_NAME: ci-database-e2e-tests-rds-postgres-instance-us-west-2-307493967395
RDS_MYSQL_INSTANCE_NAME: ci-database-e2e-tests-rds-mysql-instance-us-west-2-307493967395
RDS_MARIADB_INSTANCE_NAME: ci-database-e2e-tests-rds-mariadb-instance-us-west-2-307493967395
DISCOVERY_MATCHER_LABELS: "*=*"
jobs:
changes:
Expand Down
4 changes: 4 additions & 0 deletions e2e/aws/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,10 @@ const (
// name of the RDS MySQL DB instance that will be created by the Teleport
// Discovery Service.
rdsMySQLInstanceNameEnv = "RDS_MYSQL_INSTANCE_NAME"
// rdsMariaDBInstanceNameEnv is the environment variable that specifies the
// name of the RDS MariaDB instance that will be created by the Teleport
// Discovery Service.
rdsMariaDBInstanceNameEnv = "RDS_MARIADB_INSTANCE_NAME"
// kubeSvcRoleARNEnv is the environment variable that specifies
// the IAM role that Teleport Kubernetes Service will assume to access the EKS cluster.
// This role needs to have the following permissions:
Expand Down
200 changes: 200 additions & 0 deletions e2e/aws/rds_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -370,6 +370,129 @@ func testRDS(t *testing.T) {
})
}
})

t.Run("mariadb", func(t *testing.T) {
t.Parallel()

// wait for the database to be discovered
mariaDBName := mustGetEnv(t, rdsMariaDBInstanceNameEnv)
waitForDatabases(t, cluster.Process, mariaDBName)
db, err := cluster.Process.GetAuthServer().GetDatabase(ctx, mariaDBName)
require.NoError(t, err)
adminUser := mustGetDBAdmin(t, db)

// connect as the RDS database admin user - not to be confused
// with Teleport's "db admin user".
conn := connectAsRDSMySQLAdmin(t, ctx, db.GetAWS().RDS.InstanceID)
provisionMariaDBAdminUser(t, ctx, conn, adminUser.Name)

// create a couple test tables to test role assignment with.
testTable1 := "teleport.test_" + randASCII(t, 4)
_, err = conn.Execute(fmt.Sprintf("CREATE TABLE %s (x int)", testTable1))
require.NoError(t, err)
t.Cleanup(func() {
_, _ = conn.Execute(fmt.Sprintf("DROP TABLE %s", testTable1))
})
testTable2 := "teleport.test_" + randASCII(t, 4)
_, err = conn.Execute(fmt.Sprintf("CREATE TABLE %s (x int)", testTable2))
require.NoError(t, err)
t.Cleanup(func() {
_, _ = conn.Execute(fmt.Sprintf("DROP TABLE %s", testTable2))
})

// create the roles that Teleport will auto assign.
// role 1 only allows SELECT on test table 1.
// role 2 only allows SELECT on test table 2.
// a user needs to have both roles to select from a join of the tables.
_, err = conn.Execute(fmt.Sprintf("CREATE ROLE %q", autoRole1))
require.NoError(t, err)
t.Cleanup(func() {
_, _ = conn.Execute(fmt.Sprintf("DROP ROLE %q", autoRole1))
})
_, err = conn.Execute(fmt.Sprintf("CREATE ROLE %q", autoRole2))
require.NoError(t, err)
t.Cleanup(func() {
_, _ = conn.Execute(fmt.Sprintf("DROP ROLE %q", autoRole2))
})
_, err = conn.Execute(fmt.Sprintf("GRANT SELECT on %s TO %q", testTable1, autoRole1))
require.NoError(t, err)
_, err = conn.Execute(fmt.Sprintf("GRANT SELECT on %s TO %q", testTable2, autoRole2))
require.NoError(t, err)

// db admin needs the admin option for a role to grant others that role.
_, err = conn.Execute(fmt.Sprintf("GRANT %q TO %q WITH ADMIN OPTION", autoRole1, adminUser.Name))
require.NoError(t, err)
_, err = conn.Execute(fmt.Sprintf("GRANT %q TO %q WITH ADMIN OPTION", autoRole2, adminUser.Name))
require.NoError(t, err)
autoRolesQuery := fmt.Sprintf("SELECT 1 FROM %s JOIN %s", testTable1, testTable2)

t.Cleanup(func() {
// best effort cleanup all the users created by the tests.
// don't cleanup the admin or test runs will interfere with
// each other.
_, _ = conn.Execute(fmt.Sprintf("DROP ROLE %q", "tp-role-"+autoUserKeep))
_, _ = conn.Execute(fmt.Sprintf("DROP ROLE %q", "tp-role-"+autoUserDrop))
_, _ = conn.Execute(fmt.Sprintf("DROP USER %q", autoUserKeep))
_, _ = conn.Execute(fmt.Sprintf("DROP USER %q", autoUserDrop))
_, _ = conn.Execute("DELETE FROM teleport.user_attributes WHERE USER=?", autoUserKeep)
_, _ = conn.Execute("DELETE FROM teleport.user_attributes WHERE USER=?", autoUserDrop)
})

for name, test := range map[string]struct {
user string
dbUser string
query string
afterConnTestFn func(t *testing.T)
}{
"existing user": {
user: hostUser,
dbUser: adminUser.Name,
query: "select 1",
},
"auto user keep": {
user: autoUserKeep,
dbUser: autoUserKeep,
query: autoRolesQuery,
afterConnTestFn: func(t *testing.T) {
waitForMariaDBAutoUserDeactivate(t, conn, autoUserKeep)
},
},
"auto user drop": {
user: autoUserDrop,
dbUser: autoUserDrop,
query: autoRolesQuery,
afterConnTestFn: func(t *testing.T) {
waitForMariaDBAutoUserDrop(t, conn, autoUserDrop)
},
},
} {
test := test
t.Run(name, func(t *testing.T) {
t.Parallel()
route := tlsca.RouteToDatabase{
ServiceName: mariaDBName,
Protocol: defaults.ProtocolMySQL,
Username: test.dbUser,
Database: "", // not needed
}
t.Run("connect", func(t *testing.T) {
// run multiple conn tests in parallel to test parallel
// auto user connections.
t.Run("via local proxy 1", func(t *testing.T) {
t.Parallel()
mysqlLocalProxyConnTest(t, cluster, test.user, route, test.query)
})
t.Run("via local proxy 2", func(t *testing.T) {
t.Parallel()
mysqlLocalProxyConnTest(t, cluster, test.user, route, test.query)
})
})
if test.afterConnTestFn != nil {
test.afterConnTestFn(t)
}
})
}
})
}

const (
Expand Down Expand Up @@ -638,6 +761,11 @@ func getRDSAdminInfo(t *testing.T, ctx context.Context, instanceID string) rdsAd
require.NoError(t, err)
require.Len(t, result.DBInstances, 1)
dbInstance := result.DBInstances[0]
require.NotNil(t, dbInstance.MasterUsername)
require.NotNil(t, dbInstance.MasterUserSecret)
require.NotNil(t, dbInstance.MasterUserSecret.SecretArn)
require.NotEmpty(t, *dbInstance.MasterUsername)
require.NotEmpty(t, *dbInstance.MasterUserSecret.SecretArn)
return rdsAdminInfo{
address: *dbInstance.Endpoint.Address,
port: int(*dbInstance.Endpoint.Port),
Expand All @@ -659,6 +787,9 @@ func getRDSMasterUserPassword(t *testing.T, ctx context.Context, secretID string
// logs.
require.FailNow(t, "error unmarshaling secret string")
}
if len(secret.Pass) == 0 {
require.FailNow(t, "empty master user secret string")
}
return secret.Pass
}

Expand Down Expand Up @@ -723,6 +854,34 @@ func provisionRDSMySQLAutoUsersAdmin(t *testing.T, ctx context.Context, conn *my
require.NoError(t, err)
}

// provisionMariaDBAdminUser provisions an admin user suitable for auto-user
// provisioning.
func provisionMariaDBAdminUser(t *testing.T, ctx context.Context, conn *mySQLConn, adminUser string) {
t.Helper()
// provision the IAM user to test with.
// ignore errors from user creation. If the user doesn't exist
// later steps will catch it. The error we might get back when
// another test runner already created the admin is
// unpredictable: all we need to know is the user exists for
// test setup.
_, _ = conn.Execute(fmt.Sprintf("CREATE USER IF NOT EXISTS %q IDENTIFIED WITH AWSAuthenticationPlugin AS 'RDS'", adminUser))

// these statements are all idempotent - they should not return
// an error even if run in parallel by many test runners.
_, err := conn.Execute(fmt.Sprintf("GRANT PROCESS, CREATE USER ON *.* TO %q", adminUser))
require.NoError(t, err)
_, err = conn.Execute(fmt.Sprintf("GRANT SELECT ON mysql.roles_mapping to %q", adminUser))
require.NoError(t, err)
_, err = conn.Execute(fmt.Sprintf("GRANT UPDATE ON mysql.* TO %q", adminUser))
require.NoError(t, err)
_, err = conn.Execute(fmt.Sprintf("GRANT SELECT ON *.* TO %q", adminUser))
require.NoError(t, err)
_, err = conn.Execute("CREATE DATABASE IF NOT EXISTS `teleport`")
require.NoError(t, err)
_, err = conn.Execute(fmt.Sprintf("GRANT ALL ON `teleport`.* TO %q WITH GRANT OPTION", adminUser))
require.NoError(t, err)
}

// randASCII is a helper func that returns a random string of ascii characters.
func randASCII(t *testing.T, length int) string {
t.Helper()
Expand Down Expand Up @@ -829,3 +988,44 @@ func waitForMySQLAutoUserDrop(t *testing.T, conn *mySQLConn, user string) {
result.Close()
}, autoUserWaitDur, autoUserWaitStep, "waiting for auto user %q to be dropped", user)
}

func waitForMariaDBAutoUserDeactivate(t *testing.T, conn *mySQLConn, user string) {
t.Helper()
require.EventuallyWithT(t, func(c *assert.CollectT) {
result, err := conn.Execute("SELECT 1 FROM mysql.user AS u WHERE u.user = ?", user)
if !assert.NoError(c, err) {
return
}
if !assert.Equal(c, 1, result.RowNumber(), "user %q should not have been dropped after disconnecting", user) {
result.Close()
return
}
result.Close()

result, err = conn.Execute("SELECT 1 FROM mysql.global_priv AS u WHERE u.user = ? AND JSON_EXTRACT(u.priv, '$.account_locked') = true", user)
if !assert.NoError(c, err) {
return
}
if !assert.Equal(c, 1, result.RowNumber(), "user %q should not be able to login after deactivating", user) {
result.Close()
return
}
result.Close()

result, err = conn.Execute("SELECT 1 FROM mysql.roles_mapping AS u WHERE u.user = ? AND u.role != 'teleport-auto-user' AND u.ADMIN_OPTION='N'", user)
if !assert.NoError(c, err) {
return
}
if !assert.Equal(c, 0, result.RowNumber(), "user %q should have lost all additional roles after deactivating", user) {
result.Close()
return
}
result.Close()
}, autoUserWaitDur, autoUserWaitStep, "waiting for auto user %q to be deactivated", user)
}

func waitForMariaDBAutoUserDrop(t *testing.T, conn *mySQLConn, user string) {
t.Helper()
// run the same tests as mysql to check if the user was dropped.
waitForMySQLAutoUserDrop(t, conn, user)
}
4 changes: 2 additions & 2 deletions lib/srv/db/mysql/sql/mariadb_delete_user.sql
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,12 @@ BEGIN
CALL teleport_deactivate_user(username);
ELSE
SET state = 'TP003';
SET @sql := CONCAT('DROP ROLE ', QUOTE(CONCAT("tp-role-", username)));
SET @sql := CONCAT('DROP ROLE IF EXISTS', QUOTE(CONCAT("tp-role-", username)));
PREPARE stmt FROM @sql;
EXECUTE stmt;
DEALLOCATE PREPARE stmt;

SET @sql := CONCAT('DROP USER ', QUOTE(username));
SET @sql := CONCAT('DROP USER IF EXISTS', QUOTE(username));
PREPARE stmt FROM @sql;
EXECUTE stmt;
DEALLOCATE PREPARE stmt;
Expand Down
2 changes: 1 addition & 1 deletion lib/srv/db/mysql/sql/mysql_delete_user.sql
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ BEGIN
-- Throw a custom error code when user is still active from other sessions.
SIGNAL SQLSTATE 'TP000' SET MESSAGE_TEXT = 'User has active connections';
ELSE
SET @sql := CONCAT('DROP USER ', QUOTE(username));
SET @sql := CONCAT('DROP USER IF EXISTS', QUOTE(username));
PREPARE stmt FROM @sql;
EXECUTE stmt;
DEALLOCATE PREPARE stmt;
Expand Down
2 changes: 1 addition & 1 deletion lib/srv/db/postgres/sql/delete-user.sql
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ BEGIN
RAISE NOTICE 'User has active connections';
ELSE
BEGIN
EXECUTE FORMAT('DROP USER %I', username);
EXECUTE FORMAT('DROP USER IF EXISTS %I', username);
EXCEPTION
WHEN SQLSTATE '2BP01' THEN
state := 'TP004';
Expand Down
2 changes: 1 addition & 1 deletion lib/srv/db/postgres/sql/redshift-delete-user.sql
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ BEGIN
RAISE EXCEPTION 'TP000: User has active connections';
ELSE
BEGIN
EXECUTE 'DROP USER ' || QUOTE_IDENT(username);
EXECUTE 'DROP USER IF EXISTS ' || QUOTE_IDENT(username);
EXCEPTION WHEN OTHERS THEN
-- Redshift only support OTHERS as exception condition, so we handle
-- any error that might happen.
Expand Down

0 comments on commit 54d2e90

Please sign in to comment.