diff --git a/.github/workflows/architecture-guard.yml b/.github/workflows/architecture-guard.yml new file mode 100644 index 0000000000..62af103546 --- /dev/null +++ b/.github/workflows/architecture-guard.yml @@ -0,0 +1,70 @@ +# +# 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: 'Architecture Guard' + +# Runs the ArchUnit rules as their own check so a violation shows up as +# "Architecture Guard" on the PR instead of hiding inside the 30-minute +# Build job. Only the modules that feed the analysis trigger it. +on: + push: + branches: [ '**' ] + paths: + - 'eventmesh-architecture-guard/**' + - 'eventmesh-common/**' + - 'eventmesh-runtime/**' + - 'eventmesh-spi/**' + - 'eventmesh-protocol-plugin/**' + - 'eventmesh-storage-plugin/**' + - '.github/workflows/architecture-guard.yml' + pull_request: + branches: [ '**' ] + paths: + - 'eventmesh-architecture-guard/**' + - 'eventmesh-common/**' + - 'eventmesh-runtime/**' + - 'eventmesh-spi/**' + - 'eventmesh-protocol-plugin/**' + - 'eventmesh-storage-plugin/**' + - '.github/workflows/architecture-guard.yml' + +permissions: + contents: read + +jobs: + arch-guard: + name: Architecture Guard + runs-on: ubuntu-latest + steps: + - name: Checkout repository + uses: actions/checkout@v6 + with: + submodules: true + + - name: Set up JDK 21 + uses: actions/setup-java@v5 + with: + distribution: 'zulu' + java-version: 21 + + - name: Setup Gradle + uses: gradle/actions/setup-gradle@50e97c2cd7a37755bbfafc9c5b7cafaece252f6e #v6.1.0 + + - name: Run architecture rules + run: ./gradlew :eventmesh-architecture-guard:architectureCheck --no-daemon + env: + DEVELOCITY_ACCESS_KEY: ${{ secrets.DEVELOCITY_ACCESS_KEY }} diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 2f0837464c..7f991b595e 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -67,10 +67,15 @@ jobs: java-version: ${{ matrix.java }} # https://docs.gradle.org/current/userguide/performance.html + # :eventmesh-architecture-guard:test is excluded here because the + # dedicated Architecture Guard workflow already runs those rules + # (via architectureCheck) on every relevant change; running them + # twice just lengthens the matrix build. - name: Build run: > ./gradlew clean build dist jacocoTestReport --parallel --daemon --scan -x spotlessJava -x generateGrammarSource -x generateDistLicense -x checkDeniedLicense + -x :eventmesh-architecture-guard:test env: DEVELOCITY_ACCESS_KEY: ${{ secrets.DEVELOCITY_ACCESS_KEY }} diff --git a/eventmesh-architecture-guard/README.md b/eventmesh-architecture-guard/README.md index 753ab91fd4..ededcf19ff 100644 --- a/eventmesh-architecture-guard/README.md +++ b/eventmesh-architecture-guard/README.md @@ -1,31 +1,54 @@ # :eventmesh-architecture-guard -Minimum-viable architecture guard for issue #5305. Ships with the #5298 -PR (sub-package split of eventmesh-common). +Architecture guard for issue #5305. Enforces the package boundaries +introduced by #5298 (`eventmesh-common` sub-packages) and #5297 +(`eventmesh-runtime` sub-packages). ## What it does -Five ArchUnit rules that enforce the new org.apache.eventmesh.common -sub-package boundaries declared in #5298: +Nine ArchUnit rules, asserted in FAIL mode: any violation breaks the +build. -| Rule | Forbids | -|----------------------------|------------------------------------------------------------------------------------------| -| ruleInternalHidden | org.apache.eventmesh.common.internal.. reached from outside org.apache.eventmesh.common.. | -| ruleHttpProtocolHidden | org.apache.eventmesh.common.protocol.http.. from modules other than meshmessage / sdks | -| ruleGrpcProtocolHidden | org.apache.eventmesh.common.protocol.grpc.. from modules other than meshmessage / sdks | -| ruleTcpProtocolHidden | org.apache.eventmesh.common.protocol.tcp.. from modules other than meshmessage / sdks / runtime | -| ruleOldUtilsRenamed | org.apache.eventmesh.common.utils.. (renamed to .util.. in #5298) | +`eventmesh-common` boundaries (from #5298): -## Severity: WARN in 1.13.0, FAIL in 1.14.0 +| Rule | Forbids | +|------------------------|-------------------------------------------------------------------------------------------| +| ruleInternalHidden | org.apache.eventmesh.common.internal.. reached from outside org.apache.eventmesh.common.. | +| ruleHttpProtocolHidden | org.apache.eventmesh.common.protocol.http.. from modules other than meshmessage / sdks | +| ruleGrpcProtocolHidden | org.apache.eventmesh.common.protocol.grpc.. from modules other than meshmessage / sdks | +| ruleTcpProtocolHidden | org.apache.eventmesh.common.protocol.tcp.. from modules other than meshmessage / sdks / runtime | +| ruleOldUtilsRenamed | org.apache.eventmesh.common.utils.. (renamed to .util.. in #5298) | -The 1.13.0 release ships the rules in WARN mode (rule evaluations are -logged via System.out.println in ArchitectureRulesTest, but violations -do NOT fail `gradle check`). From 1.14.0 the *_warn test methods will -be replaced with hard rule.check(classes) assertions, at which point -new violations WILL fail `gradle architectureCheck` and thus the build. +`eventmesh-runtime` boundaries (from #5297): + +| Rule | Forbids | +|-------------------------------------|--------------------------------------------------------------------------| +| ruleRuntimeTcpInternalHidden | runtime.tcp.internal.. reached from outside runtime.tcp.. | +| ruleRuntimeEngineIsolatedFromInfra | runtime.boot / ingress / delivery depending on runtime.tcp.internal.. | +| ruleRuntimePushDoesNotImportCodec | runtime.push depending on runtime.tcp.internal.. | +| ruleRuntimeSubscriptionStateIsolated | runtime.ingress depending on runtime.state.internal.. | + +## Severity: FAIL + +`ArchitectureRulesTest` calls `rule.check(classes)` for every rule, so +a violation throws and fails the task. + +This replaced the original WARN mode, which turned out to be silent: +the module has no SLF4J binding on its test runtime classpath, so +`LoggerFactory` hands back the NOP logger and every +`LOG.warn(report)` call was discarded. Violations were reported +nowhere. FAIL mode does not depend on logging at all. ## Running locally ```bash ./gradlew :eventmesh-architecture-guard:architectureCheck ``` + +## Continuous integration + +`.github/workflows/architecture-guard.yml` runs the same task as its +own check on pushes and pull requests that touch the analysed modules, +so a violation appears as "Architecture Guard" on the PR rather than +buried in the Build job log. The matrix build in `ci.yml` excludes +`:eventmesh-architecture-guard:test` to avoid running the rules twice. diff --git a/eventmesh-architecture-guard/src/main/java/org/apache/eventmesh/architecture/guard/ArchitectureRules.java b/eventmesh-architecture-guard/src/main/java/org/apache/eventmesh/architecture/guard/ArchitectureRules.java index c9bd406439..ffe25736d5 100644 --- a/eventmesh-architecture-guard/src/main/java/org/apache/eventmesh/architecture/guard/ArchitectureRules.java +++ b/eventmesh-architecture-guard/src/main/java/org/apache/eventmesh/architecture/guard/ArchitectureRules.java @@ -56,21 +56,21 @@ public static JavaClasses loadProductionClasses() { public static ArchRule ruleHttpProtocolHidden = noClasses() .that().resideOutsideOfPackage("org.apache.eventmesh.common.protocol.http..") .and().resideOutsideOfPackage("org.apache.eventmesh.common..") - .and().resideOutsideOfPackage("org.apache.eventmesh.protocol.plugin.meshmessage..") + .and().resideOutsideOfPackage("org.apache.eventmesh.protocol.meshmessage..") .and().resideOutsideOfPackage("org.apache.eventmesh.client..") .should().dependOnClassesThat().resideInAPackage("org.apache.eventmesh.common.protocol.http.."); public static ArchRule ruleGrpcProtocolHidden = noClasses() .that().resideOutsideOfPackage("org.apache.eventmesh.common.protocol.grpc..") .and().resideOutsideOfPackage("org.apache.eventmesh.common..") - .and().resideOutsideOfPackage("org.apache.eventmesh.protocol.plugin.meshmessage..") + .and().resideOutsideOfPackage("org.apache.eventmesh.protocol.meshmessage..") .and().resideOutsideOfPackage("org.apache.eventmesh.client..") .should().dependOnClassesThat().resideInAPackage("org.apache.eventmesh.common.protocol.grpc.."); public static ArchRule ruleTcpProtocolHidden = noClasses() .that().resideOutsideOfPackage("org.apache.eventmesh.common.protocol.tcp..") .and().resideOutsideOfPackage("org.apache.eventmesh.common..") - .and().resideOutsideOfPackage("org.apache.eventmesh.protocol.plugin.meshmessage..") + .and().resideOutsideOfPackage("org.apache.eventmesh.protocol.meshmessage..") .and().resideOutsideOfPackage("org.apache.eventmesh.client..") .and().resideOutsideOfPackage("org.apache.eventmesh.runtime..") .should().dependOnClassesThat().resideInAPackage("org.apache.eventmesh.common.protocol.tcp.."); @@ -82,10 +82,6 @@ public static JavaClasses loadProductionClasses() { .that().resideOutsideOfPackage("org.apache.eventmesh.runtime.tcp..") .should().dependOnClassesThat().resideInAPackage("org.apache.eventmesh.runtime.tcp.internal.."); - public static ArchRule ruleRuntimeTcpInternalNoReverse = noClasses() - .that().resideInAPackage("org.apache.eventmesh.runtime.tcp.internal..") - .should().dependOnClassesThat().resideInAPackage("org.apache.eventmesh.runtime.tcp.."); - public static ArchRule ruleRuntimeEngineIsolatedFromInfra = noClasses() .that().resideInAPackage("org.apache.eventmesh.runtime.boot..") .or().resideInAPackage("org.apache.eventmesh.runtime.ingress..") diff --git a/eventmesh-architecture-guard/src/test/java/org/apache/eventmesh/architecture/guard/ArchitectureRulesTest.java b/eventmesh-architecture-guard/src/test/java/org/apache/eventmesh/architecture/guard/ArchitectureRulesTest.java index 1b8803051f..423d93f88d 100644 --- a/eventmesh-architecture-guard/src/test/java/org/apache/eventmesh/architecture/guard/ArchitectureRulesTest.java +++ b/eventmesh-architecture-guard/src/test/java/org/apache/eventmesh/architecture/guard/ArchitectureRulesTest.java @@ -17,85 +17,67 @@ package org.apache.eventmesh.architecture.guard; - import org.junit.jupiter.api.Test; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; import com.tngtech.archunit.core.domain.JavaClasses; -import com.tngtech.archunit.lang.EvaluationResult; /** * Test class for {@link ArchitectureRules}. * - *
Each rule is asserted in WARN mode: violations are logged at - * {@code WARN} level but the test passes. From 1.14.0 the - * {@code *_warn} test methods will be replaced with hard - * {@code rule.check(classes)} assertions that fail on violation. + *
Each rule is asserted in FAIL mode: {@code rule.check(classes)} + * throws {@code AssertionError} listing every violating location, so + * any architecture violation breaks the build. + * + *
This module intentionally carries no SLF4J binding, so the + * earlier WARN mode discarded every violation report (SLF4J falls back + * to the NOP logger). FAIL mode does not depend on logging at all. */ class ArchitectureRulesTest { - private static final Logger LOG = LoggerFactory.getLogger(ArchitectureRulesTest.class); - private final JavaClasses classes = ArchitectureRules.loadProductionClasses(); @Test - void ruleInternalHidden_warn() { - EvaluationResult r = ArchitectureRules.ruleInternalHidden.evaluate(classes); - LOG.warn("ArchitectureRules.ruleInternalHidden violations:\n{}", r.getFailureReport()); - } - - @Test - void ruleHttpProtocolHidden_warn() { - EvaluationResult r = ArchitectureRules.ruleHttpProtocolHidden.evaluate(classes); - LOG.warn("ArchitectureRules.ruleHttpProtocolHidden violations:\n{}", r.getFailureReport()); + void ruleInternalHidden() { + ArchitectureRules.ruleInternalHidden.check(classes); } @Test - void ruleGrpcProtocolHidden_warn() { - EvaluationResult r = ArchitectureRules.ruleGrpcProtocolHidden.evaluate(classes); - LOG.warn("ArchitectureRules.ruleGrpcProtocolHidden violations:\n{}", r.getFailureReport()); + void ruleHttpProtocolHidden() { + ArchitectureRules.ruleHttpProtocolHidden.check(classes); } @Test - void ruleTcpProtocolHidden_warn() { - EvaluationResult r = ArchitectureRules.ruleTcpProtocolHidden.evaluate(classes); - LOG.warn("ArchitectureRules.ruleTcpProtocolHidden violations:\n{}", r.getFailureReport()); + void ruleGrpcProtocolHidden() { + ArchitectureRules.ruleGrpcProtocolHidden.check(classes); } @Test - void ruleOldUtilsRenamed_warn() { - EvaluationResult r = ArchitectureRules.ruleOldUtilsRenamed.evaluate(classes); - LOG.warn("ArchitectureRules.ruleOldUtilsRenamed violations:\n{}", r.getFailureReport()); + void ruleTcpProtocolHidden() { + ArchitectureRules.ruleTcpProtocolHidden.check(classes); } @Test - void ruleRuntimeTcpInternalHidden_warn() { - EvaluationResult r = ArchitectureRules.ruleRuntimeTcpInternalHidden.evaluate(classes); - LOG.warn("ArchitectureRules.ruleRuntimeTcpInternalHidden violations:\n{}", r.getFailureReport()); + void ruleOldUtilsRenamed() { + ArchitectureRules.ruleOldUtilsRenamed.check(classes); } @Test - void ruleRuntimeTcpInternalNoReverse_warn() { - EvaluationResult r = ArchitectureRules.ruleRuntimeTcpInternalNoReverse.evaluate(classes); - LOG.warn("ArchitectureRules.ruleRuntimeTcpInternalNoReverse violations:\n{}", r.getFailureReport()); + void ruleRuntimeTcpInternalHidden() { + ArchitectureRules.ruleRuntimeTcpInternalHidden.check(classes); } @Test - void ruleRuntimeEngineIsolatedFromInfra_warn() { - EvaluationResult r = ArchitectureRules.ruleRuntimeEngineIsolatedFromInfra.evaluate(classes); - LOG.warn("ArchitectureRules.ruleRuntimeEngineIsolatedFromInfra violations:\n{}", r.getFailureReport()); + void ruleRuntimeEngineIsolatedFromInfra() { + ArchitectureRules.ruleRuntimeEngineIsolatedFromInfra.check(classes); } @Test - void ruleRuntimePushDoesNotImportCodec_warn() { - EvaluationResult r = ArchitectureRules.ruleRuntimePushDoesNotImportCodec.evaluate(classes); - LOG.warn("ArchitectureRules.ruleRuntimePushDoesNotImportCodec violations:\n{}", r.getFailureReport()); + void ruleRuntimePushDoesNotImportCodec() { + ArchitectureRules.ruleRuntimePushDoesNotImportCodec.check(classes); } @Test - void ruleRuntimeSubscriptionStateIsolated_warn() { - EvaluationResult r = ArchitectureRules.ruleRuntimeSubscriptionStateIsolated.evaluate(classes); - LOG.warn("ArchitectureRules.ruleRuntimeSubscriptionStateIsolated violations:\n{}", r.getFailureReport()); + void ruleRuntimeSubscriptionStateIsolated() { + ArchitectureRules.ruleRuntimeSubscriptionStateIsolated.check(classes); } }