Skip to content

Commit

Permalink
Merge pull request #199 from alphagov/disable-postgres-11-tests
Browse files Browse the repository at this point in the history 
Fix integration tests
  • Loading branch information
@monotypical
monotypical committed Dec 4, 2024
2 parents 8cf35a8 + 11dc266 commit a8961f0
Show file tree
Hide file tree
Showing 6 changed files with 33 additions and 182 deletions.
9 changes: 0 additions & 9 deletions .github/workflows/run_unit_tests.yml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ on: [push, pull_request]
env:
GO_VERSION: "1.21"
jobs:

build:
name: Build
runs-on: ubuntu-22.04
Expand All @@ -20,14 +19,6 @@ jobs:
run: |
make test_unit
- name: Run postgres 10 tests
run: |
make start_postgres_10 run_postgres_sql_tests stop_postgres_10
- name: Run postgres 11 tests
run: |
make start_postgres_11 run_postgres_sql_tests stop_postgres_11
- name: Run postgres 12 tests
run: |
make start_postgres_12 run_postgres_sql_tests stop_postgres_12
Expand Down
20 changes: 1 addition & 19 deletions Makefile
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ test_unit:
.PHONY: test_all_sql
test_all_sql: test_postgres test_mysql
.PHONY: test_postgres
test_postgres: start_postgres_10 run_postgres_sql_tests stop_postgres_10 start_postgres_11 run_postgres_sql_tests stop_postgres_11 start_postgres_12 run_postgres_sql_tests stop_postgres_12 start_postgres_13 run_postgres_sql_tests stop_postgres_13
test_postgres: start_postgres_12 run_postgres_sql_tests stop_postgres_12 start_postgres_13 run_postgres_sql_tests stop_postgres_13
.PHONY: test_mysql
test_mysql: start_mysql_80 run_mysql_sql_tests stop_mysql_80

Expand All @@ -30,16 +30,6 @@ run_mysql_sql_tests:
run_postgres_sql_tests:
POSTGRESQL_PASSWORD=$(POSTGRESQL_PASSWORD) go run github.com/onsi/ginkgo/v2/ginkgo -focus=PostgresEngine.* sqlengine/

.PHONY: start_postgres_10
start_postgres_10:
docker run -p 5432:5432 --name postgres-10 -e POSTGRES_PASSWORD=$(POSTGRESQL_PASSWORD) -d postgres:10.5; \
sleep 5

.PHONY: start_postgres_11
start_postgres_11:
docker run -p 5432:5432 --name postgres-11 -e POSTGRES_PASSWORD=$(POSTGRESQL_PASSWORD) -d postgres:11.5; \
sleep 5

.PHONY: start_postgres_12
start_postgres_12:
docker run -p 5432:5432 --name postgres-12 -e POSTGRES_PASSWORD=$(POSTGRESQL_PASSWORD) -d postgres:12.5; \
Expand All @@ -50,14 +40,6 @@ start_postgres_13:
docker run -p 5432:5432 --name postgres-13 -e POSTGRES_PASSWORD=$(POSTGRESQL_PASSWORD) -d postgres:13; \
sleep 5

.PHONY: stop_postgres_10
stop_postgres_10:
docker rm -f postgres-10

.PHONY: stop_postgres_11
stop_postgres_11:
docker rm -f postgres-11

.PHONY: stop_postgres_12
stop_postgres_12:
docker rm -f postgres-12
Expand Down
94 changes: 0 additions & 94 deletions ci/blackbox/config.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,100 +17,6 @@
"name": "postgres",
"plan_updateable": true,
"plans": [
{
"description": "Micro plan - Postgres 11",
"free": false,
"id": "postgres-micro-11",
"name": "micro-11",
"rds_properties": {
"allocated_storage": 5,
"db_instance_class": "db.t3.micro",
"db_subnet_group_name": "POPULATED_BY_TEST_SUITE",
"engine": "postgres",
"engine_version": "11",
"engine_family": "postgres11",
"multi_az": false,
"copy_tags_to_snapshot":true,
"vpc_security_group_ids": [
"POPULATED_BY_TEST_SUITE"
],
"default_extensions": [
"uuid-ossp",
"postgis",
"citext"
],
"allowed_extensions": [
"uuid-ossp",
"postgis",
"citext",
"pg_stat_statements"
]
}
},
{
"description": "Micro plan without final snapshot - Postgres 11",
"free": false,
"id": "postgres-micro-without-snapshot-11",
"name": "micro-without-snapshot-11",
"rds_properties": {
"allocated_storage": 5,
"auto_minor_version_upgrade": true,
"db_instance_class": "db.t3.micro",
"db_subnet_group_name": "POPULATED_BY_TEST_SUITE",
"engine": "postgres",
"engine_version": "11",
"engine_family": "postgres11",
"multi_az": false,
"skip_final_snapshot": true,
"copy_tags_to_snapshot":true,
"vpc_security_group_ids": [
"POPULATED_BY_TEST_SUITE"
],
"default_extensions": [
"uuid-ossp",
"postgis",
"citext"
],
"allowed_extensions": [
"uuid-ossp",
"postgis",
"citext",
"pg_stat_statements"
]
}
},
{
"description": "Small plan without final snapshot - Postgres 11",
"free": false,
"id": "postgres-small-without-snapshot-11",
"name": "small-without-snapshot-11",
"rds_properties": {
"allocated_storage": 10,
"auto_minor_version_upgrade": true,
"db_instance_class": "db.t3.micro",
"db_subnet_group_name": "POPULATED_BY_TEST_SUITE",
"engine": "postgres",
"engine_version": "11",
"engine_family": "postgres11",
"multi_az": false,
"skip_final_snapshot": true,
"copy_tags_to_snapshot":true,
"vpc_security_group_ids": [
"POPULATED_BY_TEST_SUITE"
],
"default_extensions": [
"uuid-ossp",
"postgis",
"citext"
],
"allowed_extensions": [
"uuid-ossp",
"postgis",
"citext",
"pg_stat_statements"
]
}
},
{
"description": "Micro plan - Postgres 12",
"free": false,
Expand Down
50 changes: 24 additions & 26 deletions ci/blackbox/rdsbroker_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ var _ = Describe("RDS Broker Daemon", func() {
Expect(service2.Description).To(Equal("AWS RDS PostgreSQL service"))
Expect(service2.Bindable).To(BeTrue())
Expect(service2.PlanUpdatable).To(BeTrue())
Expect(service2.Plans).To(HaveLen(9))
Expect(service2.Plans).To(HaveLen(6))
})
})

Expand Down Expand Up @@ -224,8 +224,8 @@ var _ = Describe("RDS Broker Daemon", func() {
})
}

Describe("Postgres 11", func() {
TestProvisionBindDeprovision("postgres", "postgres-micro-without-snapshot-11")
Describe("Postgres 12", func() {
TestProvisionBindDeprovision("postgres", "postgres-micro-without-snapshot-12")
})

Describe("Postgres 13", func() {
Expand Down Expand Up @@ -318,8 +318,8 @@ var _ = Describe("RDS Broker Daemon", func() {
})
}

Describe("Postgres 11", func() {
TestUpdateExtensions("postgres", "postgres-micro-without-snapshot-11")
Describe("Postgres 12", func() {
TestUpdateExtensions("postgres", "postgres-micro-without-snapshot-12")
})

Describe("Postgres 13", func() {
Expand Down Expand Up @@ -364,10 +364,6 @@ var _ = Describe("RDS Broker Daemon", func() {
})
}

Describe("Postgres 11 to 12", func() {
TestUpdatePlan("postgres", "postgres-micro-without-snapshot-11", "postgres-micro-without-snapshot-12")
})

Describe("Postgres 12 to 13", func() {
TestUpdatePlan("postgres", "postgres-micro-without-snapshot-12", "postgres-micro-without-snapshot-13")
})
Expand Down Expand Up @@ -416,8 +412,8 @@ var _ = Describe("RDS Broker Daemon", func() {
})
}

Describe("Postgres 11 to 12", func() {
TestUpdatePlan("postgres", "postgres-micro-without-snapshot-11", "postgres-micro-without-snapshot-12", "12")
Describe("Postgres 12 to 13", func() {
TestUpdatePlan("postgres", "postgres-micro-without-snapshot-12", "postgres-micro-without-snapshot-13", "13")
})
})

Expand Down Expand Up @@ -559,32 +555,32 @@ var _ = Describe("RDS Broker Daemon", func() {
})
}

Describe("Postgres 11 to 12 clean failure", func() {
Describe("Postgres 12 to 13 clean failure", func() {
// postgresSabotageUpgrade-caused failure shouldn't have produced
// any lasting effects and plan id should have been rolled back
TestUpdatePlan(
"postgres",
"postgres-micro-without-snapshot-11",
"postgres-micro-without-snapshot-12",
"postgres-micro-without-snapshot-11",
"postgres-micro-without-snapshot-13",
"postgres-micro-without-snapshot-12",
"",
)
})

Describe("Postgres 11 to 12 failure resulting in over-allocated disk", func() {
// this test upgrades from postgres 11 to 12, which fails due to
Describe("Postgres 12 to 13 failure resulting in over-allocated disk", func() {
// this test upgrades from postgres 12 to 13, which fails due to
// postgresSabotageUpgrade's actions. this will leave the aws
// storage over-allocated with 15gb instead of 10gb.
//
// the test then moves to another postgres 11 plan which still
// the test then moves to another postgres 12 plan which still
// (in theory) has less disk space than we now actually have
// (13gb), but should succeed.
TestUpdatePlan(
"postgres",
"postgres-micro-without-snapshot-11",
"postgres-micro-without-snapshot-12",
"postgres-small-without-snapshot-13",
"postgres-micro-without-snapshot-12",
"postgres-small-without-snapshot-12",
"postgres-micro-without-snapshot-11",
"postgres-small-without-snapshot-11",
)
})
})
Expand Down Expand Up @@ -727,8 +723,8 @@ var _ = Describe("RDS Broker Daemon", func() {
})
}

Describe("Postgres 11", func() {
TestFinalSnapshot("postgres", "postgres-micro-11")
Describe("Postgres 12", func() {
TestFinalSnapshot("postgres", "postgres-micro-12")
})

Describe("Postgres 13", func() {
Expand Down Expand Up @@ -887,6 +883,7 @@ var _ = Describe("RDS Broker Daemon", func() {
// Binding is synchronous
waiter := func() { return }
cleaner := func() {
By("unbinding the second instance")
code, _, err := brokerAPIClient.UnbindService(
restoredInstanceID, serviceID, planID, "post-restore-binding",
)
Expand All @@ -904,8 +901,8 @@ var _ = Describe("RDS Broker Daemon", func() {
})
}

Describe("Postgres 11", func() {
TestRestoreFromSnapshot("postgres", "postgres-micro-11", true)
Describe("Postgres 12", func() {
TestRestoreFromSnapshot("postgres", "postgres-micro-12", true)
})

Describe("Postgres 13", func() {
Expand Down Expand Up @@ -1055,6 +1052,7 @@ var _ = Describe("RDS Broker Daemon", func() {
// Binding is synchronous
waiter := func() { return }
cleaner := func() {
By("unbinding the second service instance")
code, _, err := brokerAPIClient.UnbindService(
restoredInstanceID, serviceID, planID, "post-restore-binding",
)
Expand All @@ -1072,8 +1070,8 @@ var _ = Describe("RDS Broker Daemon", func() {
})
}

Describe("Postgres 11", func() {
TestRestoreFromPointInTime("postgres", "postgres-micro-11", true)
Describe("Postgres 12", func() {
TestRestoreFromPointInTime("postgres", "postgres-micro-12", true)
})

Describe("Postgres 13", func() {
Expand Down
31 changes: 0 additions & 31 deletions rdsbroker/supported_extensions.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,37 +66,6 @@ var SupportedPreloadExtensions = map[string][]DBExtension{
},
},

"postgres11": {
DBExtension{
Name: "auto_explain",
RequiresPreloadLibrary: true,
},
DBExtension{
Name: "orafce",
RequiresPreloadLibrary: true,
},
DBExtension{
Name: "pgaudit",
RequiresPreloadLibrary: true,
},
DBExtension{
Name: "pglogical",
RequiresPreloadLibrary: true,
},
DBExtension{
Name: "pg_similarity",
RequiresPreloadLibrary: true,
},
DBExtension{
Name: "pg_stat_statements",
RequiresPreloadLibrary: true,
},
DBExtension{
Name: "pg_hint_plan",
RequiresPreloadLibrary: true,
},
},

"postgres10": {
DBExtension{
Name: "auto_explain",
Expand Down
11 changes: 8 additions & 3 deletions sqlengine/mysql_engine.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -230,9 +230,14 @@ func (d *MySQLEngine) ResetState() error {
func (d *MySQLEngine) listNonSuperUsers(logger lager.Logger) ([]string, error) {
users := []string{}

rows, err := d.db.Query(
"SELECT User FROM mysql.user WHERE Super_priv != 'Y' AND Host = '%' AND User != SUBSTRING_INDEX(CURRENT_USER(), '@', 1)",
)
rows, err := d.db.Query(`
SELECT User
FROM mysql.user
WHERE Super_priv != 'Y'
AND Host = '%'
AND User != SUBSTRING_INDEX(CURRENT_USER(), '@', 1)
AND User != 'rds_superuser_role'
`)
if err != nil {
logger.Error("sql-error", err)
return nil, err
Expand Down

0 comments on commit a8961f0

Please sign in to comment.