adoroszlai commented on code in PR #1328: URL: https://github.com/apache/ratis/pull/1328#discussion_r2622837117
########## .github/workflows/vulnerability-check.yml: ########## @@ -0,0 +1,76 @@ +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You under the Apache License, Version 2.0 +# (the "License"); you may not use this file except in compliance with +# the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +name: vulnerability-check + +on: + schedule: + # Run at UTC 16:00 every week (CST 00:00 AM) + - cron: "0 16 * * 0" + workflow_dispatch: + +concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true + +env: + MAVEN_OPTS: -Dhttp.keepAlive=false -Dmaven.wagon.http.pool=false -Dmaven.wagon.http.retryHandler.class=standard -Dmaven.wagon.http.retryHandler.count=3 + MAVEN_ARGS: --batch-mode --no-transfer-progress + +jobs: + dependency-check: + strategy: + fail-fast: false + max-parallel: 15 + matrix: + java: [17] + os: [ubuntu-latest] + runs-on: ${{ matrix.os }} + + steps: + - uses: actions/checkout@v4 + - name: Set up JDK ${{ matrix.java }} + uses: actions/setup-java@v4 + with: + distribution: corretto + java-version: ${{ matrix.java }} + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + - name: Cache Maven packages + uses: actions/cache@v4 + with: + path: ~/.m2 + key: ${{ runner.os }}-m2-${{ hashFiles('**/pom.xml') }} + restore-keys: ${{ runner.os }}-m2- Review Comment: Plugin doc recommends including the date in `key`: ``` # Using datetime in cache key as OWASP database may change, without the pom changing key: ${{ runner.os }}-maven-${{ steps.get-date.outputs.datetime }}-${{ hashFiles('**/pom.xml') }} restore-keys: | ${{ runner.os }}-maven-${{ steps.get-date.outputs.datetime }} ${{ runner.os }}-maven- ``` For that we need to calculate and store the date before this step. Also, I think only `~/.m2/repository` should be cached, not `~/.m2`. ########## .github/workflows/vulnerability-check.yml: ########## @@ -0,0 +1,76 @@ +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You under the Apache License, Version 2.0 +# (the "License"); you may not use this file except in compliance with +# the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +name: vulnerability-check + +on: + schedule: + # Run at UTC 16:00 every week (CST 00:00 AM) + - cron: "0 16 * * 0" + workflow_dispatch: + +concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true + +env: + MAVEN_OPTS: -Dhttp.keepAlive=false -Dmaven.wagon.http.pool=false -Dmaven.wagon.http.retryHandler.class=standard -Dmaven.wagon.http.retryHandler.count=3 + MAVEN_ARGS: --batch-mode --no-transfer-progress + +jobs: + dependency-check: + strategy: + fail-fast: false + max-parallel: 15 + matrix: + java: [17] + os: [ubuntu-latest] + runs-on: ${{ matrix.os }} + + steps: + - uses: actions/checkout@v4 + - name: Set up JDK ${{ matrix.java }} + uses: actions/setup-java@v4 + with: + distribution: corretto + java-version: ${{ matrix.java }} + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + - name: Cache Maven packages + uses: actions/cache@v4 + with: + path: ~/.m2 + key: ${{ runner.os }}-m2-${{ hashFiles('**/pom.xml') }} + restore-keys: ${{ runner.os }}-m2- + - name: Do Maven install + shell: bash + run: mvn clean install -DskipTests + - name: Do the dependency-check:check + shell: bash + run: mvn org.owasp:dependency-check-maven:check -DossIndexUsername=${{ secrets.OSS_INDEX_USER }} -DossIndexPassword=${{ secrets.OSS_INDEX_TOKEN }} Review Comment: The [IoTDB run](https://github.com/apache/iotdb/actions/runs/20006782722/job/57370235598#step:6:66) logged this warning: ``` An NVD API Key was not provided - it is highly recommended to use an NVD API key as the update can take a VERY long time without an API Key ``` The plugin's [doc](https://dependency-check.github.io/DependencyCheck/dependency-check-maven/configuration.html) mentions `-DnvdApiKey`. ########## .github/workflows/vulnerability-check.yml: ########## @@ -0,0 +1,76 @@ +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You under the Apache License, Version 2.0 +# (the "License"); you may not use this file except in compliance with +# the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +name: vulnerability-check + +on: + schedule: + # Run at UTC 16:00 every week (CST 00:00 AM) + - cron: "0 16 * * 0" + workflow_dispatch: + +concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true + +env: + MAVEN_OPTS: -Dhttp.keepAlive=false -Dmaven.wagon.http.pool=false -Dmaven.wagon.http.retryHandler.class=standard -Dmaven.wagon.http.retryHandler.count=3 + MAVEN_ARGS: --batch-mode --no-transfer-progress + +jobs: + dependency-check: + strategy: + fail-fast: false + max-parallel: 15 + matrix: + java: [17] + os: [ubuntu-latest] Review Comment: Do we really need this matrix? ########## .github/workflows/vulnerability-check.yml: ########## @@ -0,0 +1,76 @@ +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You under the Apache License, Version 2.0 +# (the "License"); you may not use this file except in compliance with +# the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +name: vulnerability-check + +on: + schedule: + # Run at UTC 16:00 every week (CST 00:00 AM) + - cron: "0 16 * * 0" + workflow_dispatch: + +concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true + +env: + MAVEN_OPTS: -Dhttp.keepAlive=false -Dmaven.wagon.http.pool=false -Dmaven.wagon.http.retryHandler.class=standard -Dmaven.wagon.http.retryHandler.count=3 + MAVEN_ARGS: --batch-mode --no-transfer-progress + +jobs: + dependency-check: Review Comment: I think scheduled run should be skipped in forks. ```suggestion dependency-check: if: github.event_name == 'workflow_dispatch' || github.repository == 'apache/ratis' ``` ########## .github/workflows/vulnerability-check.yml: ########## @@ -0,0 +1,76 @@ +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You under the Apache License, Version 2.0 +# (the "License"); you may not use this file except in compliance with +# the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +name: vulnerability-check + +on: + schedule: + # Run at UTC 16:00 every week (CST 00:00 AM) + - cron: "0 16 * * 0" + workflow_dispatch: + +concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true + +env: + MAVEN_OPTS: -Dhttp.keepAlive=false -Dmaven.wagon.http.pool=false -Dmaven.wagon.http.retryHandler.class=standard -Dmaven.wagon.http.retryHandler.count=3 + MAVEN_ARGS: --batch-mode --no-transfer-progress + +jobs: + dependency-check: + strategy: + fail-fast: false + max-parallel: 15 + matrix: + java: [17] + os: [ubuntu-latest] + runs-on: ${{ matrix.os }} + + steps: + - uses: actions/checkout@v4 + - name: Set up JDK ${{ matrix.java }} + uses: actions/setup-java@v4 + with: + distribution: corretto + java-version: ${{ matrix.java }} + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + - name: Cache Maven packages + uses: actions/cache@v4 + with: + path: ~/.m2 + key: ${{ runner.os }}-m2-${{ hashFiles('**/pom.xml') }} + restore-keys: ${{ runner.os }}-m2- + - name: Do Maven install + shell: bash + run: mvn clean install -DskipTests Review Comment: With `mvn install` we should delete Ratis artifacts at the end of the workflow to prevent Ratis jars from being cached. Even in case of failures (`if: always()`). Alternatively, we can run build and check in a single command, using `package` instead of `install`: ``` mvn ... clean package org.owasp:dependency-check-maven:aggregate ``` Better yet, we can use existing CI script to build Ratis: ``` dev-support/checks/compile.sh org.owasp:dependency-check-maven:aggregate <... arguments for dependency-check> ``` `compile.sh` is basically an alias for: ``` mvn -Dmaven.javadoc.skip=true -DskipTests -Djacoco.skip clean verify ``` and accepts additional arguments for Maven. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
