diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 6f21d23f144e..299158425259 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -63,6 +63,7 @@ jobs: needs-build: ${{ steps.selective-checks.outputs.needs-build }} needs-compile: ${{ steps.selective-checks.outputs.needs-compile }} needs-compose-tests: ${{ steps.selective-checks.outputs.needs-compose-tests }} + needs-infer: ${{ steps.selective-checks.outputs.needs-infer }} needs-integration-tests: ${{ steps.selective-checks.outputs.needs-integration-tests }} needs-kubernetes-tests: ${{ steps.selective-checks.outputs.needs-kubernetes-tests }} sha: ${{ steps.get-sha.outputs.sha }} @@ -222,6 +223,21 @@ jobs: - check: errorprone fail-fast: false + infer: + needs: + - build-info + if: needs.build-info.outputs.needs-infer == 'true' + uses: $/.github/workflows/check.yml + with: + java-version: ${{ needs.build-info.outputs.java-version }} + needs-maven-cache: true + ratis-args: ${{ inputs.ratis_args }} + script: infer + sha: ${{ needs.build-info.outputs.sha }} + timeout-minutes: 60 + secrets: + DEVELOCITY_ACCESS_KEY: ${{ secrets.DEVELOCITY_ACCESS_KEY }} + errorprone: needs: - build-info diff --git a/.inferconfig b/.inferconfig new file mode 100644 index 000000000000..30f8b196397e --- /dev/null +++ b/.inferconfig @@ -0,0 +1,15 @@ +{ + "force-delete-results-dir": true, + "capture-block-list": [ + { + "source_contains": "generated-sources" + }, + { + "source_contains": "target/" + } + ], + "report-block-list-files-containing": [ + "@generated", + "generated-sources" + ] +} diff --git a/dev-support/ci/infer.bats b/dev-support/ci/infer.bats new file mode 100644 index 000000000000..d1cb351864a3 --- /dev/null +++ b/dev-support/ci/infer.bats @@ -0,0 +1,259 @@ +#!/usr/bin/env bats +# 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. + +setup() { + TEST_TMPDIR=$(mktemp -d) + export TEST_TMPDIR + local checks=hadoop-ozone/dev-support/checks + mkdir -p "${TEST_TMPDIR}/${checks}/install" "${TEST_TMPDIR}/bin" + cp "${checks}/infer.sh" "${checks}/_lib.sh" "${TEST_TMPDIR}/${checks}/" + cp "${checks}/install/infer.sh" "${TEST_TMPDIR}/${checks}/install/" + cd "${TEST_TMPDIR}" + POMS=(hadoop-ozone/ozonefs-hadoop2/pom.xml hadoop-ozone/ozonefs-shaded/pom.xml) + for pom in "${POMS[@]}"; do + mkdir -p "$(dirname "${pom}")" + printf '%s\n' '' > "${pom}" + done + git init -q + git add hadoop-ozone + git -c user.name=Test -c user.email=test@example.com -c core.hooksPath=/dev/null commit -qm initial + printf '%s\n' \ + '#!/usr/bin/env bash' \ + 'if [[ "$1" == debug ]]; then' \ + ' if [[ "${INFER_EMPTY_CAPTURE:-false}" != true ]]; then echo src/main/java/Example.java; fi' \ + ' exit 0' \ + 'fi' \ + 'touch "${TEST_TMPDIR}/infer-called"' \ + 'printf "%s\n" "$@" > "${TEST_TMPDIR}/infer-args"' \ + 'for arg in "$@"; do if [[ "$arg" == clean ]]; then rm -rf target; fi; done' \ + 'if [[ "${INFER_NO_REPORT:-false}" != true ]]; then' \ + ' mkdir -p infer-out' \ + ' printf "%s\n" "${INFER_REPORT:-No issues found}" > infer-out/report.txt' \ + ' printf "%s\n" "${INFER_JSON-[]}" > infer-out/report.json' \ + 'fi' \ + 'echo "Infer output"' \ + 'printf "%s\n" modified > hadoop-ozone/ozonefs-hadoop2/pom.xml' \ + 'printf "%s\n" modified > hadoop-ozone/ozonefs-shaded/pom.xml' \ + 'if [[ -n "${INFER_SIGNAL:-}" ]]; then kill -"${INFER_SIGNAL}" "$PPID"; fi' \ + 'exit "${INFER_EXIT_CODE:-0}"' > bin/infer + chmod +x bin/infer + export PATH="${TEST_TMPDIR}/bin:${PATH}" + export OZONE_PREFER_LOCAL_TOOL=true +} + +teardown() { + cd / + rm -rf "${TEST_TMPDIR}" +} + +@test "Infer refuses unstaged POM edits without changing either file" { + printf '%s\n' 'local edits' >> "${POMS[0]}" + cp "${POMS[0]}" expected + + run bash hadoop-ozone/dev-support/checks/infer.sh + + [ "${status}" -eq 1 ] + [ ! -e infer-called ] + cmp expected "${POMS[0]}" + git diff --quiet -- "${POMS[1]}" +} + +@test "Infer refuses staged POM edits without changing the index or worktree" { + printf '%s\n' 'staged edits' >> "${POMS[1]}" + git add "${POMS[1]}" + cp "${POMS[1]}" expected + local index_before + index_before=$(git write-tree) + + run bash hadoop-ozone/dev-support/checks/infer.sh + + [ "${status}" -eq 1 ] + [ ! -e infer-called ] + cmp expected "${POMS[1]}" + [ "$(git write-tree)" = "${index_before}" ] +} + +@test "Infer refuses a deleted POM without recreating it" { + rm "${POMS[0]}" + + run bash hadoop-ozone/dev-support/checks/infer.sh + + [ "${status}" -eq 1 ] + [ ! -e infer-called ] + [ ! -e "${POMS[0]}" ] +} + +@test "Infer restores POMs after successful analysis and preserves unrelated edits" { + printf '%s\n' '# local edits' >> hadoop-ozone/dev-support/checks/_lib.sh + cp hadoop-ozone/dev-support/checks/_lib.sh expected + + run bash hadoop-ozone/dev-support/checks/infer.sh + + [ "${status}" -eq 0 ] + [ -e infer-called ] + git diff --exit-code -- "${POMS[@]}" + cmp expected hadoop-ozone/dev-support/checks/_lib.sh +} + +@test "Infer runs without Git metadata and restores both POMs" { + mv .git saved-git + cp "${POMS[0]}" expected0 + cp "${POMS[1]}" expected1 + + run bash hadoop-ozone/dev-support/checks/infer.sh + + [ "${status}" -eq 0 ] + [ -e infer-called ] + [ -f target/infer/report.txt ] + cmp expected0 "${POMS[0]}" + cmp expected1 "${POMS[1]}" +} + +@test "Infer restores POMs after an analysis failure" { + export INFER_EXIT_CODE=2 + + run bash hadoop-ozone/dev-support/checks/infer.sh + + [ "${status}" -eq 1 ] + [ -e infer-called ] + git diff --exit-code -- "${POMS[@]}" +} + +@test "Infer restores POMs when interrupted by TERM" { + export INFER_SIGNAL=TERM + + run bash hadoop-ozone/dev-support/checks/infer.sh + + [ "${status}" -eq 143 ] + [ -e infer-called ] + git diff --exit-code -- "${POMS[@]}" +} + +@test "Infer restores POMs when interrupted by INT" { + export INFER_SIGNAL=INT + + run bash hadoop-ozone/dev-support/checks/infer.sh + + [ "${status}" -eq 130 ] + [ -e infer-called ] + git diff --exit-code -- "${POMS[@]}" +} + +@test "Infer cleans the build without losing the output log" { + mkdir -p target + touch target/previous-build + + run bash hadoop-ozone/dev-support/checks/infer.sh + + [ "${status}" -eq 0 ] + [ ! -e target/previous-build ] + grep -q 'Infer output' target/infer/output.log + grep -qx -- '-Ddevelocity.cache.local.enabled=false' infer-args + grep -qx -- '-Ddevelocity.cache.remote.enabled=false' infer-args +} + +@test "Infer preserves Maven's per-module Java release settings" { + run bash hadoop-ozone/dev-support/checks/infer.sh + + [ "${status}" -eq 0 ] + grep -qx -- '--java-version-reset' infer-args +} + +@test "Infer failures are not hidden by a fresh report" { + export INFER_EXIT_CODE=5 + + run bash hadoop-ozone/dev-support/checks/infer.sh + + [ "${status}" -eq 1 ] + grep -q 'code 5' target/infer/summary.txt + [ -f target/infer/report.txt ] +} + +@test "Infer failures do not reuse previous reports" { + mkdir -p target/infer infer-out + printf '%s\n' stale > target/infer/report.txt + printf '%s\n' stale > infer-out/report.txt + export INFER_EXIT_CODE=5 INFER_NO_REPORT=true + + run bash hadoop-ozone/dev-support/checks/infer.sh + + [ "${status}" -eq 1 ] + [ ! -e target/infer/report.txt ] + grep -q 'code 5' target/infer/summary.txt +} + +@test "Infer requires a report even when the command succeeds" { + export INFER_NO_REPORT=true + + run bash hadoop-ozone/dev-support/checks/infer.sh + + [ "${status}" -eq 1 ] + grep -q 'no report' target/infer/summary.txt +} + +@test "Infer rejects empty capture instead of reporting a clean analysis" { + export INFER_EMPTY_CAPTURE=true + + run bash hadoop-ozone/dev-support/checks/infer.sh + + [ "${status}" -eq 1 ] + grep -q 'no source files' target/infer/summary.txt +} + +@test "Infer clean reports contain a single zero count" { + run bash hadoop-ozone/dev-support/checks/infer.sh + + [ "${status}" -eq 0 ] + [ "$(cat target/infer/failures)" = 0 ] + grep -qx 'Infer analysis complete. Found 0 issues.' target/infer/summary.txt +} + +@test "Infer counts findings from JSON and keeps them informational" { + export INFER_REPORT='Example.java:1: error: Null Dereference' + export INFER_JSON='[{"severity":"ERROR"},{"severity":"WARNING"},{"suppressed":true},{"censored_reason":"filtered"}]' + + run bash hadoop-ozone/dev-support/checks/infer.sh + + [ "${status}" -eq 0 ] + [ "$(cat target/infer/failures)" = 2 ] + cmp infer-out/report.json target/infer/report.json +} + +@test "Infer rejects an invalid JSON report" { + local report + for report in 'invalid JSON' '' '[] []' '{}' 'null'; do + export INFER_JSON="${report}" + run bash hadoop-ozone/dev-support/checks/infer.sh + + [ "${status}" -eq 1 ] + grep -q 'invalid JSON report' target/infer/summary.txt + done +} + +@test "Infer installation failures preserve diagnostics in the artifact" { + export OZONE_PREFER_LOCAL_TOOL=false + printf '%s\n' '#!/usr/bin/env bash' 'echo "download failed" >&2' 'exit 22' > bin/curl + chmod +x bin/curl + + run bash hadoop-ozone/dev-support/checks/infer.sh + + [ "${status}" -eq 1 ] + [ ! -e infer-called ] + grep -q 'Failed to install infer' target/infer/summary.txt + grep -q 'download failed' target/infer/output.log + ! grep -q 'unbound variable' target/infer/output.log + git diff --exit-code -- "${POMS[@]}" +} diff --git a/dev-support/ci/selective_ci_checks.bats b/dev-support/ci/selective_ci_checks.bats index c012d4e61dec..cc230236a6a5 100644 --- a/dev-support/ci/selective_ci_checks.bats +++ b/dev-support/ci/selective_ci_checks.bats @@ -33,6 +33,77 @@ load bats-support/load.bash load bats-assert/load.bash +function run_with_changed_file() { + local file="$1" + local repo + repo=$(mktemp -d) + git -C "${repo}" init -q + git -C "${repo}" config user.name "Selective CI test" + git -C "${repo}" config user.email "test@example.com" + git -C "${repo}" -c core.hooksPath=/dev/null -c commit.gpgsign=false commit -q --allow-empty -m "Base" + mkdir -p "${repo}/$(dirname "${file}")" + touch "${repo}/${file}" + git -C "${repo}" add "${file}" + git -C "${repo}" -c core.hooksPath=/dev/null -c commit.gpgsign=false commit -q -m "Change ${file}" + + run env GIT_DIR="${repo}/.git" GIT_WORK_TREE="${repo}" dev-support/ci/selective_ci_checks.sh HEAD + rm -rf "${repo}" +} + +@test "Infer configuration only" { + run_with_changed_file .inferconfig + + assert_success + assert_output -p 'basic-checks=["rat"]' + assert_output -p needs-build=false + assert_output -p needs-compile=false + assert_output -p needs-compose-tests=false + assert_output -p needs-infer=true + assert_output -p needs-integration-tests=false + assert_output -p needs-kubernetes-tests=false +} + +@test "Infer scripts" { + local file + for file in hadoop-ozone/dev-support/checks/infer.sh hadoop-ozone/dev-support/checks/install/infer.sh; do + run_with_changed_file "${file}" + + assert_success + assert_output -p needs-infer=true + assert_output -p needs-compose-tests=false + assert_output -p needs-integration-tests=false + assert_output -p needs-kubernetes-tests=false + done +} + +@test "Infer runs for test-utils POM changes" { + run_with_changed_file hadoop-hdds/test-utils/pom.xml + + assert_success + assert_output -p needs-infer=true +} + +@test "Infer skips test-utils Java in other modules" { + run_with_changed_file hadoop-ozone/test-utils/src/main/java/org/apache/ozone/test/TestHelper.java + + assert_success + assert_output -p needs-infer=false +} + +@test "push runs Infer" { + run dev-support/ci/selective_ci_checks.sh + + assert_success + assert_output -p needs-infer=true +} + +@test "full tests needed label runs Infer for test-only changes" { + PR_LABELS="full tests needed" run dev-support/ci/selective_ci_checks.sh bf59f31a7e5fcad1acc85e1228d77c22ee7f036c + + assert_success + assert_output -p needs-infer=true +} + @test "checkstyle and bats" { run dev-support/ci/selective_ci_checks.sh 11b098430 @@ -51,6 +122,7 @@ load bats-assert/load.bash assert_output -p needs-build=true assert_output -p needs-compile=false assert_output -p needs-compose-tests=true + assert_output -p needs-infer=false assert_output -p needs-integration-tests=false assert_output -p needs-kubernetes-tests=false } @@ -84,6 +156,7 @@ load bats-assert/load.bash assert_output -p needs-build=true assert_output -p needs-compile=true assert_output -p needs-compose-tests=true + assert_output -p needs-infer=true assert_output -p needs-integration-tests=false assert_output -p needs-kubernetes-tests=true } @@ -142,6 +215,7 @@ load bats-assert/load.bash assert_output -p needs-build=true assert_output -p needs-compile=true assert_output -p needs-compose-tests=false + assert_output -p needs-infer=false assert_output -p needs-integration-tests=false assert_output -p needs-kubernetes-tests=false assert_output -p test-classes=org/apache/ozone/test/Test*,org/apache/hadoop/ozone/container/TestContainerReportHandling.java,org/apache/hadoop/ozone/container/TestContainerReportHandlingWithHA.java,org/apache/hadoop/ozone/container/TestHelper.java @@ -201,6 +275,7 @@ load bats-assert/load.bash assert_output -p needs-build=true assert_output -p needs-compile=true assert_output -p needs-compose-tests=false + assert_output -p needs-infer=false assert_output -p needs-integration-tests=true assert_output -p needs-kubernetes-tests=false assert_output -p test-classes= @@ -237,6 +312,7 @@ load bats-assert/load.bash assert_output -p needs-build=true assert_output -p needs-compile=true assert_output -p needs-compose-tests=false + assert_output -p needs-infer=false assert_output -p needs-integration-tests=true assert_output -p needs-kubernetes-tests=false assert_output -p test-classes= @@ -294,6 +370,7 @@ load bats-assert/load.bash assert_output -p needs-build=true assert_output -p needs-compile=true assert_output -p needs-compose-tests=true + assert_output -p needs-infer=true assert_output -p needs-integration-tests=true assert_output -p needs-kubernetes-tests=true } @@ -338,6 +415,7 @@ load bats-assert/load.bash assert_output -p needs-build=true assert_output -p needs-compile=true assert_output -p needs-compose-tests=true + assert_output -p needs-infer=true assert_output -p needs-integration-tests=true assert_output -p needs-kubernetes-tests=true } @@ -349,6 +427,7 @@ load bats-assert/load.bash assert_output -p needs-build=true assert_output -p needs-compile=true assert_output -p needs-compose-tests=true + assert_output -p needs-infer=true assert_output -p needs-integration-tests=true assert_output -p needs-kubernetes-tests=true } diff --git a/dev-support/ci/selective_ci_checks.sh b/dev-support/ci/selective_ci_checks.sh index ae247080de95..c30b93dea3b3 100755 --- a/dev-support/ci/selective_ci_checks.sh +++ b/dev-support/ci/selective_ci_checks.sh @@ -89,6 +89,7 @@ function set_outputs_run_everything_and_exit() { | cut -f1 -d'.') compile_needed=true compose_tests_needed=true + infer_needed=true integration_tests_needed=true kubernetes_tests_needed=true @@ -100,6 +101,7 @@ function set_outputs_run_everything_and_exit() { function set_output_skip_all_tests_and_exit() { BASIC_CHECKS="" compose_tests_needed=false + infer_needed=false integration_tests_needed=false kubernetes_tests_needed=false @@ -170,6 +172,7 @@ function filter_changed_files() { SOURCES_TRIGGERING_TESTS=( "^.github" + "^\.inferconfig$" "^dev-support" "^hadoop-hdds" "^hadoop-ozone" @@ -514,6 +517,27 @@ function check_needs_pmd() { start_end::group_end } +function check_needs_infer() { + start_end::group_start "Check if infer is needed" + local pattern_array=( + "^\.inferconfig$" + "^hadoop-ozone/dev-support/checks/infer.sh$" + "^hadoop-ozone/dev-support/checks/install/infer.sh$" + "pom.xml" + "src/main/java" + ) + local ignore_array=( + ".*/test-utils/src/main/java" + ) + filter_changed_files + + if [[ ${match_count} != "0" ]]; then + infer_needed=true + fi + + start_end::group_end +} + # Counts other files which do not need to trigger any functional test # (i.e. no compose/integration/kubernetes) @@ -532,6 +556,7 @@ function get_count_misc_files() { "\.bats$" "\.txt$" "\.md$" + "^\.inferconfig$" "findbugsExcludeFile.xml" "pmd-ruleset.xml" "/NOTICE$" @@ -606,6 +631,7 @@ function set_outputs() { initialization::ga_output needs-build "${build_needed:-false}" initialization::ga_output needs-compile "${compile_needed}" initialization::ga_output needs-compose-tests "${compose_tests_needed}" + initialization::ga_output needs-infer "${infer_needed:-false}" initialization::ga_output needs-integration-tests "${integration_tests_needed}" initialization::ga_output needs-kubernetes-tests "${kubernetes_tests_needed}" @@ -660,5 +686,6 @@ check_needs_docs check_needs_errorprone check_needs_findbugs check_needs_pmd +check_needs_infer calculate_test_types_to_run set_outputs diff --git a/dev-support/rat/rat-exclusions.txt b/dev-support/rat/rat-exclusions.txt index 23d3707a65c4..4c1199760ec0 100644 --- a/dev-support/rat/rat-exclusions.txt +++ b/dev-support/rat/rat-exclusions.txt @@ -18,6 +18,7 @@ **/*.json .gitattributes .github/* +.inferconfig AGENTS.md CLAUDE.md CONTRIBUTING.md diff --git a/hadoop-hdds/container-service/pom.xml b/hadoop-hdds/container-service/pom.xml index bcb899f2128d..24ade37f2f0c 100644 --- a/hadoop-hdds/container-service/pom.xml +++ b/hadoop-hdds/container-service/pom.xml @@ -26,6 +26,21 @@ Apache Ozone Distributed Data Store Container Service + + com.facebook.infer.annotation + infer-annotation + 0.18.0 + + + com.google.code.findbugs + jsr305 + + + org.jetbrains.kotlin + kotlin-annotations-jvm + + + com.fasterxml.jackson.core jackson-annotations diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/impl/ContainerData.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/impl/ContainerData.java index 7dd8157ead69..100751fed812 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/impl/ContainerData.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/impl/ContainerData.java @@ -28,6 +28,7 @@ import static org.apache.hadoop.ozone.OzoneConsts.ORIGIN_PIPELINE_ID; import static org.apache.hadoop.ozone.OzoneConsts.STATE; +import com.facebook.infer.annotation.ThreadSafe; import com.fasterxml.jackson.annotation.JsonIgnore; import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.Lists; @@ -56,6 +57,7 @@ * ContainerData is the in-memory representation of container metadata and is * represented on disk by the .container file. */ +@ThreadSafe public abstract class ContainerData { //Type of the container. diff --git a/hadoop-hdds/server-scm/pom.xml b/hadoop-hdds/server-scm/pom.xml index c8756b11219c..f10baec621d4 100644 --- a/hadoop-hdds/server-scm/pom.xml +++ b/hadoop-hdds/server-scm/pom.xml @@ -30,6 +30,21 @@ + + com.facebook.infer.annotation + infer-annotation + 0.18.0 + + + com.google.code.findbugs + jsr305 + + + org.jetbrains.kotlin + kotlin-annotations-jvm + + + com.fasterxml.jackson.core jackson-annotations diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/SCMStateMachine.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/SCMStateMachine.java index a1e00403745d..27e5ffeba6ea 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/SCMStateMachine.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/SCMStateMachine.java @@ -19,6 +19,7 @@ import static java.util.Objects.requireNonNull; +import com.facebook.infer.annotation.ThreadSafe; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Preconditions; import com.google.common.util.concurrent.ThreadFactoryBuilder; @@ -73,6 +74,7 @@ * responsible for applying ratis committed transactions to * {@link StorageContainerManager}. */ +@ThreadSafe public class SCMStateMachine extends BaseStateMachine { private static final Logger LOG = LoggerFactory.getLogger(SCMStateMachine.class); diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/SCMNodeManager.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/SCMNodeManager.java index 3b640d6e8cc4..5a3961f65546 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/SCMNodeManager.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/SCMNodeManager.java @@ -22,6 +22,7 @@ import static org.apache.hadoop.hdds.protocol.proto.HddsProtos.NodeOperationalState.IN_SERVICE; import static org.apache.hadoop.hdds.protocol.proto.HddsProtos.NodeState.HEALTHY; +import com.facebook.infer.annotation.ThreadSafe; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Preconditions; import com.google.common.base.Strings; @@ -118,6 +119,7 @@ * get functions in this file as a snap-shot of information that is inconsistent * as soon as you read it. */ +@ThreadSafe public class SCMNodeManager implements NodeManager, ContainerReplicaPendingOpsSubscriber { private static final Logger LOG = diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/safemode/SCMSafeModeManager.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/safemode/SCMSafeModeManager.java index fff90db4a1d5..2d1946f74b04 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/safemode/SCMSafeModeManager.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/safemode/SCMSafeModeManager.java @@ -22,6 +22,7 @@ import static org.apache.hadoop.hdds.HddsConfigKeys.HDDS_SCM_SAFEMODE_RULE_REFRESH_INTERVAL; import static org.apache.hadoop.hdds.HddsConfigKeys.HDDS_SCM_SAFEMODE_RULE_REFRESH_INTERVAL_DEFAULT; +import com.facebook.infer.annotation.ThreadSafe; import com.google.common.util.concurrent.ThreadFactoryBuilder; import java.util.ArrayList; import java.util.HashMap; @@ -76,6 +77,7 @@ * @see RatisContainerSafeModeRule * @see ECContainerSafeModeRule */ +@ThreadSafe public class SCMSafeModeManager implements SafeModeManager { private static final Logger LOG = LoggerFactory.getLogger(SCMSafeModeManager.class); diff --git a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java index 6ce6966934ce..a6d94e41f23a 100644 --- a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java +++ b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java @@ -2766,9 +2766,14 @@ static GDPRSymmetricKey getGDPRSymmetricKey(Map metadata, return null; } + String secret = metadata.get(OzoneConsts.GDPR_SECRET); + String algorithm = metadata.get(OzoneConsts.GDPR_ALGORITHM); + if (secret == null || algorithm == null) { + throw new IOException("GDPR metadata is missing secret or algorithm"); + } + final GDPRSymmetricKey gk = new GDPRSymmetricKey( - metadata.get(OzoneConsts.GDPR_SECRET), - metadata.get(OzoneConsts.GDPR_ALGORITHM)); + secret, algorithm); try { gk.getCipher().init(mode, gk.getSecretKey()); } catch (InvalidKeyException e) { diff --git a/hadoop-ozone/client/src/test/java/org/apache/hadoop/ozone/client/rpc/TestRpcClient.java b/hadoop-ozone/client/src/test/java/org/apache/hadoop/ozone/client/rpc/TestRpcClient.java index 431da719f887..ca1e2e2ad717 100644 --- a/hadoop-ozone/client/src/test/java/org/apache/hadoop/ozone/client/rpc/TestRpcClient.java +++ b/hadoop-ozone/client/src/test/java/org/apache/hadoop/ozone/client/rpc/TestRpcClient.java @@ -38,13 +38,16 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; +import java.util.HashMap; import java.util.LinkedList; import java.util.List; +import java.util.Map; import java.util.concurrent.ExecutorService; import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicReference; import java.util.function.Supplier; import java.util.stream.Stream; +import javax.crypto.Cipher; import org.apache.commons.lang3.reflect.FieldUtils; import org.apache.hadoop.hdds.client.RatisReplicationConfig; import org.apache.hadoop.hdds.client.ReplicationConfig; @@ -362,6 +365,16 @@ public void testGetOmVersionWithFutureOM() { om(OzoneManagerVersion.UNKNOWN_VERSION)))); } + @Test + public void testGetGDPRSymmetricKeyRejectsIncompleteMetadata() { + Map metadata = new HashMap<>(); + metadata.put(OzoneConsts.GDPR_FLAG, Boolean.TRUE.toString()); + + IOException ex = assertThrows(IOException.class, + () -> RpcClient.getGDPRSymmetricKey(metadata, Cipher.DECRYPT_MODE)); + assertThat(ex).hasMessage("GDPR metadata is missing secret or algorithm"); + } + @Test public void testCloseTwiceDoesNotWarn() throws IOException { RpcClient rpcClient = createRpcClient(); diff --git a/hadoop-ozone/datanode/pom.xml b/hadoop-ozone/datanode/pom.xml index 72916b644d90..b2802cd7955d 100644 --- a/hadoop-ozone/datanode/pom.xml +++ b/hadoop-ozone/datanode/pom.xml @@ -72,6 +72,7 @@ $HDDS_LIB_JARS_DIR true runtime + infer-annotation diff --git a/hadoop-ozone/dev-support/checks/bats.sh b/hadoop-ozone/dev-support/checks/bats.sh index 839b4dd11e69..5e784332d48a 100755 --- a/hadoop-ozone/dev-support/checks/bats.sh +++ b/hadoop-ozone/dev-support/checks/bats.sh @@ -33,6 +33,7 @@ rm -f "${REPORT_DIR}/output.log" find * \( \ -path '*/src/test/shell/*' -name '*.bats' \ -or -path dev-support/ci/errorprone.bats \ + -or -path dev-support/ci/infer.bats \ -or -path dev-support/ci/selective_ci_checks.bats \ -or -path dev-support/ci/pr_title_check.bats \ -or -path dev-support/ci/find_test_class_project.bats \ diff --git a/hadoop-ozone/dev-support/checks/infer.sh b/hadoop-ozone/dev-support/checks/infer.sh new file mode 100755 index 000000000000..86b598e18bea --- /dev/null +++ b/hadoop-ozone/dev-support/checks/infer.sh @@ -0,0 +1,129 @@ +#!/usr/bin/env bash +# 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. + +set -u -o pipefail + +DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" >/dev/null 2>&1 && pwd )" +cd "$DIR/../../.." || exit 1 + +POM_FILES=(hadoop-ozone/ozonefs-hadoop2/pom.xml hadoop-ozone/ozonefs-shaded/pom.xml) +# CI removes .git before running checks; only check local edits in Git checkouts. +if [[ -e .git ]] && { + ! git diff --quiet -- "${POM_FILES[@]}" || ! git diff --cached --quiet -- "${POM_FILES[@]}" +}; then + echo "Infer requires clean ozonefs POMs. Commit or stash changes to these files first:" >&2 + printf ' %s\n' "${POM_FILES[@]}" >&2 + exit 1 +fi + +POM_BACKUP_DIR=$(mktemp -d) || exit 1 +REPORT_DIR=${OUTPUT_DIR:-"$DIR/../../../target/infer"} +REPORT_FILE="$REPORT_DIR/summary.txt" +OUTPUT_LOG="${POM_BACKUP_DIR}/output.log" + +restore_poms() { + local rc=$? + local restore_failed=false + local i + if ! mkdir -p "$REPORT_DIR" || ! cp "$OUTPUT_LOG" "${REPORT_DIR}/output.log"; then + rc=1 + fi + for i in "${!POM_FILES[@]}"; do + if [[ -f "${POM_BACKUP_DIR}/${i}" ]] && ! cp "${POM_BACKUP_DIR}/${i}" "${POM_FILES[$i]}"; then + restore_failed=true + fi + done + if [[ "${restore_failed}" == "true" ]]; then + echo "Failed to restore ozonefs POMs. Original files are saved in ${POM_BACKUP_DIR}" >&2 + rc=1 + else + rm -rf "${POM_BACKUP_DIR}" + fi + if [[ ${rc} -ne 0 ]]; then + echo "Infer check failed with exit code ${rc}." >> "$REPORT_FILE" + echo "1" > "${REPORT_DIR}/failures" + fi + exit "${rc}" +} + +trap restore_poms EXIT +trap 'exit 130' INT +trap 'exit 143' TERM +mkdir -p "$REPORT_DIR" || exit 1 +rm -f "${REPORT_DIR}/report.txt" "${REPORT_DIR}/report.json" "${REPORT_DIR}/captured-sources.txt" \ + "${REPORT_DIR}/logs" infer-out/report.txt infer-out/report.json || exit 1 +: > "$REPORT_FILE" +: > "$OUTPUT_LOG" +echo "0" > "${REPORT_DIR}/failures" +for i in "${!POM_FILES[@]}"; do + cp "${POM_FILES[$i]}" "${POM_BACKUP_DIR}/${i}" || exit 1 +done + +source "${DIR}/_lib.sh" +source "${DIR}/install/infer.sh" >> "$OUTPUT_LOG" 2>&1 + +MAVEN_OPTIONS='-B -DskipTests -DskipDocs -DskipRecon -DskipShade -Dsort.skip=true --no-transfer-progress' +MAVEN_OPTIONS+=' -Ddevelocity.cache.local.enabled=false -Ddevelocity.cache.remote.enabled=false' + +# Keep the log outside target because Maven clean removes it. +# Do not let Infer's Maven profile override each module's maven.compiler.release. +infer run --java-version-reset --keep-going --report-console-limit 0 \ + --skip-analysis-in-path '.*/src/test/.*' \ + --skip-analysis-in-path '.*/target/generated-test-sources/.*' \ + --skip-analysis-in-path '.*/target/generated-sources/.*' \ + --skip-analysis-in-path '.*/test-utils/.*' \ + -- mvn ${MAVEN_OPTIONS} clean install "$@" 2>&1 | tee -a "$OUTPUT_LOG" +infer_rc=$? + +mkdir -p "$REPORT_DIR" + +# Copy reports and diagnostics, not the multi-GB capture/results databases. +for report in report.txt report.json; do + if [[ -f "infer-out/${report}" ]]; then + cp "infer-out/${report}" "${REPORT_DIR}/" || exit 1 + fi +done +cp infer-out/logs "${REPORT_DIR}/" 2>/dev/null || true +cp -R infer-out/stats "${REPORT_DIR}/" 2>/dev/null || true + +if [[ ${infer_rc} -ne 0 ]]; then + echo "Infer exited with code ${infer_rc}." >> "$REPORT_FILE" + exit 1 +fi +if [[ ! -f "${REPORT_DIR}/report.txt" || ! -f "${REPORT_DIR}/report.json" ]]; then + echo "Infer analysis produced no report." >> "$REPORT_FILE" + exit 1 +fi + +if ! infer debug --source-files > "${REPORT_DIR}/captured-sources.txt" 2>> "$OUTPUT_LOG" \ + || ! grep -q '[^[:space:]]' "${REPORT_DIR}/captured-sources.txt"; then + echo "Infer captured no source files or could not read the capture." >> "$REPORT_FILE" + exit 1 +fi + +issue_count=$(jq -es ' + if length == 1 and (.[0] | type) == "array" then + [.[0][] | select(.suppressed != true and .censored_reason == null)] | length + else error("Expected a single JSON report array") end +' "${REPORT_DIR}/report.json" 2>> "$OUTPUT_LOG") || { + echo "Infer produced an invalid JSON report." >> "$REPORT_FILE" + exit 1 +} +echo "Infer analysis complete. Found ${issue_count} issues." >> "$REPORT_FILE" +echo "${issue_count}" > "${REPORT_DIR}/failures" + +# Findings are informational; execution and reporting failures are not. +exit 0 diff --git a/hadoop-ozone/dev-support/checks/install/infer.sh b/hadoop-ozone/dev-support/checks/install/infer.sh new file mode 100755 index 000000000000..a573c8252e4d --- /dev/null +++ b/hadoop-ozone/dev-support/checks/install/infer.sh @@ -0,0 +1,42 @@ +#!/usr/bin/env bash +# 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. + +# This script installs Infer. +# Requires _install_tool from _lib.sh. Use `source` for both scripts, because it modifies $PATH. + +: ${INFER_VERSION:=1.3.0} + +_install_infer() { + local os + os=$(uname -s) + local arch + arch=$(uname -m) + + if [[ "${os}" == "Linux" ]]; then + local infer_archive="infer-linux-x86_64-v${INFER_VERSION}.tar.xz" + elif [[ "${os}" == "Darwin" ]] && [[ "${arch}" == "arm64" ]]; then + local infer_archive="infer-osx-arm64-v${INFER_VERSION}.tar.xz" + else + echo "Unsupported platform: ${os} ${arch}" >&2 + exit 1 + fi + + local url="https://github.com/facebook/infer/releases/download/v${INFER_VERSION}/${infer_archive}" + mkdir -p "infer-v${INFER_VERSION}" + curl -LSs "${url}" | tar -xJ -C "infer-v${INFER_VERSION}" --strip-components=1 -f - +} + +_install_tool infer "infer-v${INFER_VERSION}/bin" "infer" "infer" diff --git a/hadoop-ozone/dist/pom.xml b/hadoop-ozone/dist/pom.xml index 7cdf560c2737..aef10ea4de7d 100644 --- a/hadoop-ozone/dist/pom.xml +++ b/hadoop-ozone/dist/pom.xml @@ -218,6 +218,7 @@ target/ozone-${ozone.version}/share/ozone/lib runtime + infer-annotation diff --git a/hadoop-ozone/ozone-manager/pom.xml b/hadoop-ozone/ozone-manager/pom.xml index b4022b21c91e..2a0f98e1aa8d 100644 --- a/hadoop-ozone/ozone-manager/pom.xml +++ b/hadoop-ozone/ozone-manager/pom.xml @@ -30,6 +30,21 @@ + + com.facebook.infer.annotation + infer-annotation + 0.18.0 + + + com.google.code.findbugs + jsr305 + + + org.jetbrains.kotlin + kotlin-annotations-jvm + + + com.fasterxml.jackson.core jackson-annotations diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java index dc0bc020c827..d4d8b0fd7c27 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java @@ -119,6 +119,7 @@ import static org.apache.hadoop.util.ExitUtil.terminate; import static org.apache.hadoop.util.Time.monotonicNow; +import com.facebook.infer.annotation.ThreadSafe; import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.databind.ObjectReader; import com.fasterxml.jackson.databind.ObjectWriter; @@ -392,6 +393,7 @@ * Ozone Manager is the metadata manager of ozone. */ @InterfaceAudience.LimitedPrivate({"HDFS", "CBLOCK", "OZONE", "HBASE"}) +@ThreadSafe public final class OzoneManager extends ServiceRuntimeInfoImpl implements OzoneManagerProtocol, OMInterServiceProtocol, OMMXBean, Auditor { public static final Logger LOG = diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/SnapshotChainManager.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/SnapshotChainManager.java index 5dc6d1dbfb27..674b5530799e 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/SnapshotChainManager.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/SnapshotChainManager.java @@ -17,6 +17,7 @@ package org.apache.hadoop.ozone.om; +import com.facebook.infer.annotation.ThreadSafe; import com.google.common.annotations.VisibleForTesting; import java.io.IOException; import java.io.UncheckedIOException; @@ -47,6 +48,7 @@ * On start, the snapshot chains are initialized from the on disk * SnapshotInfoTable from the OM RocksDB. */ +@ThreadSafe public class SnapshotChainManager { private static final Logger LOG = LoggerFactory.getLogger(SnapshotChainManager.class); diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/lock/OzoneManagerLock.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/lock/OzoneManagerLock.java index 3f875d586af1..cec94d9cdfcd 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/lock/OzoneManagerLock.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/lock/OzoneManagerLock.java @@ -22,6 +22,7 @@ import static org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_MANAGER_STRIPED_LOCK_SIZE_DEFAULT; import static org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_MANAGER_STRIPED_LOCK_SIZE_PREFIX; +import com.facebook.infer.annotation.ThreadSafe; import com.google.common.annotations.VisibleForTesting; import com.google.common.util.concurrent.Striped; import java.util.Arrays; @@ -91,6 +92,7 @@ *
*/ +@ThreadSafe public class OzoneManagerLock implements IOzoneManagerLock { private static final Logger LOG = LoggerFactory.getLogger(OzoneManagerLock.class); diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/security/OzoneDelegationTokenSecretManager.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/security/OzoneDelegationTokenSecretManager.java index 1e1243bf6f99..0046f28a6999 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/security/OzoneDelegationTokenSecretManager.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/security/OzoneDelegationTokenSecretManager.java @@ -21,6 +21,7 @@ import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.TOKEN_EXPIRED; import static org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMTokenProto.Type.S3AUTHINFO; +import com.facebook.infer.annotation.ThreadSafe; import com.google.common.annotations.VisibleForTesting; import java.io.ByteArrayInputStream; import java.io.DataInputStream; @@ -68,6 +69,7 @@ */ @InterfaceAudience.Private @InterfaceStability.Unstable +@ThreadSafe public class OzoneDelegationTokenSecretManager extends OzoneSecretManager { diff --git a/pom.xml b/pom.xml index 4365d353e6ca..6390d1897c70 100644 --- a/pom.xml +++ b/pom.xml @@ -2419,6 +2419,7 @@ $HDDS_LIB_JARS_DIR true runtime + infer-annotation ${classpath.skip} @@ -2432,6 +2433,7 @@ ${project.build.directory}/share/ozone/lib runtime ${classpath.skip} + infer-annotation