From 720783ff694c72adbf27772928659628035894ea Mon Sep 17 00:00:00 2001 From: steventrouble Date: Mon, 23 Oct 2023 16:00:18 -0700 Subject: [PATCH 1/5] Add spring-velocity dependency See also https://github.com/UWIT-IAM/cert-service/pull/58 --- .gitmodules | 3 +++ deps/velocity-spring | 1 + 2 files changed, 4 insertions(+) create mode 100644 .gitmodules create mode 160000 deps/velocity-spring diff --git a/.gitmodules b/.gitmodules new file mode 100644 index 0000000..41d3186 --- /dev/null +++ b/.gitmodules @@ -0,0 +1,3 @@ +[submodule "deps/velocity-spring"] + path = deps/velocity-spring + url = https://github.com/UWIT-IAM/velocity-spring.git diff --git a/deps/velocity-spring b/deps/velocity-spring new file mode 160000 index 0000000..b8b1c31 --- /dev/null +++ b/deps/velocity-spring @@ -0,0 +1 @@ +Subproject commit b8b1c3169bac0461e5dea5dc6823a2da8d2ea0ad From c1e5a4399946d2edec6141e2bd7e09aabd4db261 Mon Sep 17 00:00:00 2001 From: steventrouble Date: Mon, 23 Oct 2023 16:01:41 -0700 Subject: [PATCH 2/5] Add github autobuild action. See https://github.com/UWIT-IAM/cert-service/pull/56 --- .github/workflows/maven.yml | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) create mode 100644 .github/workflows/maven.yml diff --git a/.github/workflows/maven.yml b/.github/workflows/maven.yml new file mode 100644 index 0000000..5c1b33a --- /dev/null +++ b/.github/workflows/maven.yml @@ -0,0 +1,34 @@ +# This workflow will build a Java project with Maven, and cache/restore any dependencies to improvethe workflow execution time +# For more information see: https://docs.github.com/en/actions/automating-builds-and-tests/building-and-testing-java-with-maven + +# This workflow uses actions that are not certified by GitHub. +# They are provided by a third-party and are governed by +# separate terms of service, privacy policy, and support +# documentation. + +name: Java CI with Maven + +on: + push: + branches: [ "main" ] + pull_request: + branches: [ "main" ] + +jobs: + build: + + runs-on: ubuntu-latest + + steps: + - uses: actions/checkout@v3 + with: + submodules: recursive + token: ${{ secrets.ACTIONS_PAT }} + - name: Set up JDK 8 + uses: actions/setup-java@v3 + with: + java-version: '8' + distribution: 'temurin' + cache: maven + - name: Build with Maven + run: mvn --no-transfer-progress clean compile package From 261a850695ea9d5347d2250e2f59ede13832b93e Mon Sep 17 00:00:00 2001 From: steventrouble Date: Mon, 23 Oct 2023 16:01:59 -0700 Subject: [PATCH 3/5] Add pre-commit checks --- .pre-commit-config.yaml | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) create mode 100644 .pre-commit-config.yaml diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml new file mode 100644 index 0000000..7f038b4 --- /dev/null +++ b/.pre-commit-config.yaml @@ -0,0 +1,26 @@ +# See https://pre-commit.com/ for file specification +# +# These checks are not performed by default. If you'd like to use these checks, +# install pre-commit and then run `pre-commit install` in the root of your +# repository. + +repos: +- repo: https://github.com/pre-commit/pre-commit-hooks + rev: v2.1.0 + hooks: + # Avoid trailing whitespace, see Java style guide + - id: trailing-whitespace + + # Avoid files without newline ending, see Java style guide + - id: end-of-file-fixer + + # Verify modified data files + - id: check-yaml + - id: check-xml + +- repo: https://github.com/ejba/pre-commit-maven + rev: v0.3.3 + hooks: + # Project compiles and passes unit tests + - id: maven-compile + - id: maven-test From ba4e19f7a9699b7e66f82eb2ae4e9bcdfd5d8f20 Mon Sep 17 00:00:00 2001 From: steventrouble Date: Mon, 23 Oct 2023 17:12:17 -0700 Subject: [PATCH 4/5] Update pom.xml to use the velocity spring dep I had to fix a few broken tests first. In particular, the schema file was written for a different DB type, and had to be updated to use the PostgreSQL `SERIAL` type instead of `AUTO_INCREMENT`. --- README.dependencies | 38 ------------ README.md | 12 ++-- pom.xml | 61 +++++++++++++++---- .../filter/DBFilterPolicyDAOTest.java | 4 ++ .../registry/proxy/ProxyManagerDBTest.java | 2 +- src/test/resources/logback.xml | 30 +++++++++ src/test/resources/test-schema.sql | 10 ++- 7 files changed, 91 insertions(+), 66 deletions(-) delete mode 100644 README.dependencies create mode 100644 src/test/resources/logback.xml diff --git a/README.dependencies b/README.dependencies deleted file mode 100644 index f15bab5..0000000 --- a/README.dependencies +++ /dev/null @@ -1,38 +0,0 @@ -# SPReg dependencies - -A special step is needed to prepare a dependency for the SPReg build. - -Spregistry is built using the Spring and Velocity frameworks. However, Spring 5.1 and later versions dropped support for Velocity. -In particular, they removed two packages from release versions of the spring-webmvc artifact: -org.springframework.ui.velocity -org.springframework.web.servlet.view.velocity - -Security vulnerabilities in older Spring versions required an upgrade to a Spring version later than 5.1, -but we wanted to avoid major effort to replace Velocity (for example, with the Freemarker package formally -supported in Spring 5.1+). Fortunately, we were able to craft a workaround as described below. - -The Apache Velocity team created a drop-in replacement for the org.springframework.ui.velocity package, -available as the artifact org.apache.velocity:spring-velocity-support available online at Maven Central. - -For the second package, we extracted the classes from an older version of Spring, specifically 4.3.20.RELEASE -which was the version previously used, and repackaged these classes as the local artifact -edu.washington.iam:uw-spring-velocity-web:4.3.20.RELEASE -The version number was set identical to the Spring version from which they were extracted. -Testing confirmed that this package, along with a new configuration class called IamVelocityConfig, successfully -implemented the Spring-Velocity integration. Furthermore, conversation with the Spring team indicated that this -was a reasonable approach for all Spring 5.X versions. Further investigation will be needed if and when this -application is migrated to Spring 6+. - -We made the uw-spring-velocity-web package available by manually installing this into the local Maven cache for -the build user (that is, ~/.m2/repository/edu/washington/iam/uw-spring-velocity-web/4.3.20.RELEASE/). -To build this package in your own environment, you will need to install this package into your own Maven cache, -either by copying this from the build server or rebuilding the package as described above and following Maven -documentation for installing a package into your local cache. - -The formally correct way to deploy a local package such as this would be to install it into a local package repository -and configure our Maven build environments to read from that repository. However, this is the first case where we have -ever needed a local package included in our builds and we do not have a local repository solution in place. The use -of the local cache for this is a reasonable approach for now given that this application is only built locally by -a few users who have access to the build server. - - diff --git a/README.md b/README.md index e4130af..eac31dd 100644 --- a/README.md +++ b/README.md @@ -11,35 +11,31 @@ The application is quite specific to resources at the University, such as a DNS Create spreg.properties.dev and spreg.properties.prod from spreg.properties.tmpl -``` - -``` ## Dependencies See README.dependencies for information on a special step needed to prepare dependencies for the build. -``` ## Build +``` $> mvn clean compile package ``` -``` ## Install +``` $> cd ansible -see the README for configuration steps +# see the README for configuration steps $> ./install.sh -h (target) ``` -``` ## IdP install +``` $> cd idp-tools see the README for configuration steps $> ./install.sh -h (target) ``` - diff --git a/pom.xml b/pom.xml index 881c1fd..3aa0985 100644 --- a/pom.xml +++ b/pom.xml @@ -1,5 +1,5 @@ - + 4.0.0 edu.washington.iam spreg @@ -11,7 +11,7 @@ apache apache - http://maven.apache.org + https://maven.apache.org @@ -21,6 +21,11 @@ 4.13.2 test + + javax.annotation + javax.annotation-api + 1.3.2 + org.apache.tomcat servlet-api @@ -39,13 +44,13 @@ javers-core 6.6.5 - + org.slf4j slf4j-api 1.7.36 - + + org.apache.maven.plugins + maven-war-plugin + 3.3.1 + + + + + org.codehaus.mojo + build-helper-maven-plugin + 3.4.0 + + + add-source + generate-sources + + add-source + + + + ./deps/velocity-spring/src/main/java + + + + + spreg diff --git a/src/test/java/edu/washington/iam/registry/filter/DBFilterPolicyDAOTest.java b/src/test/java/edu/washington/iam/registry/filter/DBFilterPolicyDAOTest.java index 493a02e..0bdafe4 100644 --- a/src/test/java/edu/washington/iam/registry/filter/DBFilterPolicyDAOTest.java +++ b/src/test/java/edu/washington/iam/registry/filter/DBFilterPolicyDAOTest.java @@ -96,6 +96,7 @@ public void testGetFilterPoliciesCaching() throws Exception { // get it again to test that update check works afps = dao.getFilterPolicies(filterPolicyGroup); Assert.assertEquals(fakeEntityIds.size(), afps.size()); + NamedParameterJdbcTemplate namedTemplate = new NamedParameterJdbcTemplate(template); namedTemplate.update("delete from filter where entity_id in (:entityIds)", new MapSqlParameterSource().addValue("entityIds", fakeEntityIds)); @@ -359,6 +360,9 @@ private void setupWithRPs(List entityIds){ "values (?, ?, ?, ?, ?, now())", genUUID(), groupId, entityId, fakeRelyingPartyXml(entityId), null); } + + // All tests expect filter to be empty at start. + template.update("delete from filter"); } private void teardownRPs(List entityIds){ diff --git a/src/test/java/edu/washington/iam/registry/proxy/ProxyManagerDBTest.java b/src/test/java/edu/washington/iam/registry/proxy/ProxyManagerDBTest.java index 2e306a1..77b1033 100644 --- a/src/test/java/edu/washington/iam/registry/proxy/ProxyManagerDBTest.java +++ b/src/test/java/edu/washington/iam/registry/proxy/ProxyManagerDBTest.java @@ -10,7 +10,7 @@ @RunWith(SpringJUnit4ClassRunner.class) -@ContextConfiguration("/test-db-context.xml") +@ContextConfiguration("classpath:test-db-context.xml") public class ProxyManagerDBTest { /* Integration tests commented out to placate CI */ @Autowired diff --git a/src/test/resources/logback.xml b/src/test/resources/logback.xml new file mode 100644 index 0000000..0748559 --- /dev/null +++ b/src/test/resources/logback.xml @@ -0,0 +1,30 @@ + + + + + target/test.log + + + + target/test-%d{yyyy-MM-dd}.log + + + + + %date{HH:mm:ss.SSS} %level [%logger:%line] - %msg%n %ex{1} + + + + + + + + + + + + + + + + diff --git a/src/test/resources/test-schema.sql b/src/test/resources/test-schema.sql index 6944134..db1b634 100644 --- a/src/test/resources/test-schema.sql +++ b/src/test/resources/test-schema.sql @@ -1,7 +1,7 @@ drop table if exists proxy; CREATE TABLE proxy ( - id integer NOT NULL AUTO_INCREMENT, + id SERIAL NOT NULL, uuid uuid NOT NULL, entity_id text, end_time timestamp NULL, @@ -48,7 +48,7 @@ CREATE TABLE filter ( start_time timestamp DEFAULT now(), end_time timestamp NULL, updated_by text, - id integer NOT NULL AUTO_INCREMENT, + id SERIAL NOT NULL, status INTEGER ); @@ -80,7 +80,7 @@ insert into metadata_group (id, header_xml, footer_xml, status, update_time, edi drop table if exists metadata; CREATE TABLE metadata ( - id integer NOT NULL AUTO_INCREMENT, + id SERIAL NOT NULL, uuid uuid NOT NULL, entity_id character varying(128) NOT NULL, xml text, @@ -101,7 +101,7 @@ ALTER TABLE metadata drop table if exists access_control; CREATE TABLE access_control ( -id integer NOT NULL AUTO_INCREMENT, +id SERIAL NOT NULL, uuid uuid NOT NULL, entity_id text NOT NULL, end_time timestamp NULL, @@ -116,5 +116,3 @@ conditional_link varchar alter table access_control ADD PRIMARY KEY (id); - - From 8250ce4c6d0aa87396d2523116e0cf7d38b82aea Mon Sep 17 00:00:00 2001 From: steventrouble Date: Thu, 26 Oct 2023 10:24:24 -0700 Subject: [PATCH 5/5] Automate testing too, and fix flaky and slow tests --- .github/workflows/maven.yml | 2 +- .../filter/DBFilterPolicyDAOTest.java | 29 ++++++++++--------- 2 files changed, 17 insertions(+), 14 deletions(-) diff --git a/.github/workflows/maven.yml b/.github/workflows/maven.yml index 5c1b33a..18c880c 100644 --- a/.github/workflows/maven.yml +++ b/.github/workflows/maven.yml @@ -31,4 +31,4 @@ jobs: distribution: 'temurin' cache: maven - name: Build with Maven - run: mvn --no-transfer-progress clean compile package + run: mvn --no-transfer-progress clean compile test package diff --git a/src/test/java/edu/washington/iam/registry/filter/DBFilterPolicyDAOTest.java b/src/test/java/edu/washington/iam/registry/filter/DBFilterPolicyDAOTest.java index 0bdafe4..ae91172 100644 --- a/src/test/java/edu/washington/iam/registry/filter/DBFilterPolicyDAOTest.java +++ b/src/test/java/edu/washington/iam/registry/filter/DBFilterPolicyDAOTest.java @@ -24,6 +24,13 @@ @RunWith(SpringJUnit4ClassRunner.class) @ContextConfiguration("classpath:test-db-context.xml") public class DBFilterPolicyDAOTest { + /** + * Amount of time for tests to sleep waiting for DB operations. + * + * Set as low as possible. + */ + private static int SLEEP_DELAY_MS = 10; + private final Logger log = LoggerFactory.getLogger(getClass()); @Autowired @@ -201,7 +208,7 @@ public void testCreateFilterPolicy() throws Exception { // check that there's nothing in there already Assert.assertEquals(0, qResults.size()); Timestamp preUpdateTime = new Timestamp(new Date().getTime()); - Thread.sleep(500); + Thread.sleep(SLEEP_DELAY_MS); attributeFilterPolicy.setUuid(genUUID()); dao.createFilterPolicy(filterPolicyGroup, attributeFilterPolicy, remoteUser); @@ -229,7 +236,7 @@ public void testCreateFilterPolicyPreviouslyDeleted() throws Exception { , Timestamp.class); Assert.assertEquals(0, qResults.size()); Timestamp preUpdateTime = new Timestamp(new Date().getTime()); - Thread.sleep(500); + Thread.sleep(SLEEP_DELAY_MS); dao.updateFilterPolicies(filterPolicyGroup, Arrays.asList(attributeFilterPolicy), remoteUser); qResults = template.queryForList("select start_time from filter where entity_id = ? and end_time is null" @@ -256,7 +263,7 @@ public void testUpdateFilterPolicy() throws Exception { Assert.assertEquals(1, qResults.size()); Timestamp preUpdateTime = qResults.get(0); //it's too fast! - Thread.sleep(500); + Thread.sleep(SLEEP_DELAY_MS); dao.updateFilterPolicy(filterPolicyGroup, attributeFilterPolicy, remoteUser); qResults = template.queryForList("select start_time from filter where entity_id = ? and end_time is null" @@ -284,8 +291,7 @@ public void testUpdateFilterPolicies() throws Exception { , new MapSqlParameterSource().addValue("ids", entityIds) , Timestamp.class); Assert.assertEquals(2, qResults.size()); - Timestamp preUpdateTime = new Timestamp(new Date().getTime()); - Thread.sleep(500); + Thread.sleep(SLEEP_DELAY_MS); List updatePolicies = new ArrayList<>(); updatePolicies.add(fakeAttributeFilterPolicy(filterPolicyGroup, updateEntityId)); @@ -293,16 +299,13 @@ public void testUpdateFilterPolicies() throws Exception { dao.updateFilterPolicies(filterPolicyGroup, updatePolicies, remoteUser); - qResults = namedTemplate.queryForList("select start_time from filter where end_time is null and entity_id in (:ids)" + // The two updated policies should have later start_times than other one + qResults = namedTemplate.queryForList( + "select start_time from filter where end_time is null and entity_id in (:ids)" + + " and start_time > (select min(start_time) from filter where end_time is null)" , new MapSqlParameterSource().addValue("ids", entityIds) , Timestamp.class); - Assert.assertEquals(3, qResults.size()); - int updateCount = 0; - for(Timestamp result : qResults){ - if(result.after(preUpdateTime)) - updateCount++; - } - Assert.assertEquals(2, updateCount); + Assert.assertEquals(2, qResults.size()); //clean up namedTemplate.update("delete from filter where entity_id in (:ids)",