Move test database properties to test resources#3628
Draft
duanemay wants to merge 5 commits into
Draft
Conversation
cf-foundation-community-automation
Bot
moved this to Inbox
in Foundational Infrastructure Working Group
duanemay
force-pushed
the
fix/test_props
branch
from
3819a7c to
5e1f3a0
Compare
fhanik
force-pushed
the
fix/test_props
branch
from
5e1f3a0 to
0d52aea
Compare
Contributor
|
rebased on develop branch |
…tion tests application-mysql.properties/application-postgresql.properties no longer ship database.username/password/url, since those are packaged into the WAR. bootRun, the embedded integrationTest auto-start, and the CI external-Tomcat integration tests (scripts/tomcat/integration_tests_tomcat.sh) all relied on those defaults to reach the root/changeme database that lib_db_helper.sh boots locally/in CI. Supply the same values via -D system properties instead, so those flows keep working without reintroducing the credentials into the shipped artifact.
Gradle's gradle.taskGraph.whenReady block only forwarded spring.profiles.active, testId, and zones.paths.enabled into forked Test-task JVMs; arbitrary -D flags passed on the command line (e.g. -Ddatabase.username) are never propagated to a forked test JVM automatically. Once application-mysql.properties/application- postgresql.properties stopped shipping database.username/password/url, any test that builds a DatabaseConfiguration context (e.g. ClientAdminBootstrapProdEncoderTest) under the mysql/postgresql profile got a null database URL and failed to create a DataSource. Forward the same root/changeme local-dev credentials used for bootRun/integrationTest/CI Tomcat into every Test task's active profile. Also apply the same fix to bootWarRun for consistency. Verified locally against a real Postgres: ClientAdminBootstrapProdEncoderTest now passes under -Dspring.profiles.active=postgresql, reproducing and fixing the CI db-integration-test/tomcat-integration-test failures on PR #3628.
The previous commit forwarded database.username/password/url into every Test task, but the plain `test` task includes tests (DatabasePropertiesTest, YamlServletProfileInitializerTest) that deliberately exercise their own per-profile/custom database property fixtures (e.g. hsqldb's "sa" default, a mysql URL pattern, a custom "donkey" YAML value). Since JVM system properties take precedence over everything else in Spring's Environment, the blanket override clobbered those fixtures and broke db-unit-test in CI. Restrict the override to the integrationTest task, which is the one that actually needs it (it connects to a real external mysql/postgresql with no per-test property source of its own). Verified locally: DatabasePropertiesTest and YamlServletProfileInitializerTest pass again under -Dspring.profiles.active=postgresql, the full :cloudfoundry-identity-server:test suite passes (4819/4820, the one failure is an unrelated collation/locale artifact of the local Postgres install), and ClientAdminBootstrapProdEncoderTest still passes under integrationTest.
…ncher The mysql JDBC URL contains an unescaped & (...useSSL=true&trustServerCertificate=true). The doFirst block builds a raw command string passed straight to `bash -c`, which parses it fresh (unlike the CATALINA_OPTS case in integration_tests_tomcat.sh, where the same value only ever appears via variable expansion, so control-operator lexing doesn't apply). An unquoted & there is lexed as a background-job operator, splitting the command mid-argument and silently truncating the java -jar invocation, which is why db-integration-test (mysql-8.0/8.4/9) kept failing to start UAA within 300s while the postgresql matrix (no & in its URL) passed. Single-quote each -D value.
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
What
application-mysql.properties/application-postgresql.propertiesinserver/src/main/resourcesshipped hardcodeddatabase.username=root/database.password=changeme/a127.0.0.1/local JDBC URL — these look like local test/dev fixtures, not production defaults, and end up packaged into the WAR. This moves those values intosrc/test/resources(kept for the test suite, which needs them), leaving onlydatabase.driverClassNamein the packagedapplication-mysql.properties/application-postgresql.properties.This is not a fix for an exploitable default: UAA falls back to the
hsqldbprofile with zero configuration (YamlServletProfileInitializer.applySpringProfiles), so these values are only ever read if an operator explicitly activates themysql/postgresqlprofile. It's hygiene — don't ship plausible-looking credentials in the production artifact.Follow-up fix included
Removing these from
src/main/resourcesbroke three local/CI flows that relied on them being present in the packaged artifact when themysql/postgresqlprofile is active, with no other credentials supplied:./gradlew run/bootRunwith-Dspring.profiles.active=postgresqlormysql(documented inREADME.md)integrationTestauto-start (java -jarlaunch inuaa/build.gradle.kts)scripts/tomcat/integration_tests_tomcat.sh,.github/workflows/integration-tests.yml, mysql/postgresql matrix)All three now supply the same
root/changemelocal-dev credentials via explicit-Dsystem properties (still overridable) instead of relying on the packaged properties files. Verified by runningbootRun -Dspring.profiles.active=postgresqlagainst a real local Postgres — UAA started and Flyway created all tables as expected.Test plan
./gradlew :cloudfoundry-identity-uaa:bootRun -Dspring.profiles.active=postgresql -Ddatabase.url=jdbc:postgresql://localhost:5433/uaaagainst a local Postgres with aroot/changemesuperuser — UAA started, Flyway created all 16 tablesshellcheck scripts/tomcat/integration_tests_tomcat.sh— no new findings