test #128

hitanshu · 2026-07-05T11:00:54Z

hitanshu commented

2026-07-05 11:00:54 +00:00

No description provided.

hitanshu added 9 commits 2026-07-05 11:00:54 +00:00

Hithomelabs/HomeLabDocker#86 : chore: scaffold multi-module Gradle structure 2a6e7c176c

Hithomelabs/HomeLabDocker#86 : refactor: move cftunnels source into :cftunnels-service module 0c2e186ba7

Hithomelabs/HomeLabDocker#86 : feat: add :common module with RestTemplateConfig ee1762711d

Hithomelabs/HomeLabDocker#86 : chore: add :portainer-automation scaffold 0c6944685f

Hithomelabs/HomeLabDocker#86 : ci: update workflows for multi-module paths 4abec13ec4

Hithomelabs/HomeLabDocker#86 : fix: add BOM import and update RestTemplateConfig imports

sample gradle build and test / build (pull_request) Successful in 2m55s

Details

sample gradle build and test / tag (push) Successful in 6s

Details

sample gradle build and test / build_tag_push (push) Failing after 55s

Details

4a3e2777db

Hithomelabs/CFTunnels#86 : ci: replace Gradle wrapper validation with offline checksum

sample gradle build and test / build (pull_request) Successful in 1m56s

Details

sample gradle build and test / tag (push) Successful in 6s

Details

sample gradle build and test / build_tag_push (push) Failing after 1m53s

Details

d500a8b276

ci: fix Gradle wrapper validation in test_image_build_push.yml

sample gradle build and test / build (pull_request) Successful in 2m5s

Details

040c0e9b09

Merge pull request 'ci: fix Gradle wrapper validation in test_image_build_push.yml (missed in PR #126 )' (#127 ) from Dave/CFTunnels:ISSUE-86-fix into test

sample gradle build and test / tag (push) Successful in 7s

Details

sample gradle build and test / build_tag_push (push) Successful in 3m33s

Details

Daily cloudflare API integration test / cloudflare-api-test (push) Successful in 1m49s

Details

Promote image with tag test to prod / tag (push) Successful in 7s

Details

Promote image with tag test to prod / build_tag_push (push) Successful in 14s

Details

9fb4c4fb14

Reviewed-on: #127
Reviewed-by: hitanshu <hitanshu98@gmail.com>

hitanshu requested review from Linus 2026-07-05 11:01:04 +00:00

Linus approved these changes 2026-07-05 11:02:48 +00:00

Linus left a comment

PR Review: test → main

Summary

This PR restructures the CFTunnels project from a single-module Gradle project into a multi-module architecture (:common, :cftunnels-service, :portainer-automation), relocates source code, extracts shared configuration into a :common module, updates CI workflows to use offline Gradle wrapper validation, and updates the Dockerfile for multi-module builds.

What Changed

Multi-module Gradle: Root settings.gradle now includes 3 submodules; build.gradle uses Spring BOM with apply false
Code moved: Source moved from src/main/java/.../CFTunnels/ to cftunnels-service/src/main/java/.../cftunnels/ (also package rename)
common module: Extracts RestTemplateConfig as a shared bean for reuse across submodules
portainer-automation module: Scaffold submodule (minimal, referencing :common)
CI/CD: Replaced gradle/actions/wrapper-validation@v3 with offline sha256sum --check; updated test_image_build_push.yml to use :cftunnels-service:bootBuildImage
Dockerfile: Updated to copy all submodule sources before build; moved to cftunnels-service/Dockerfile

PR Review Checklist

Code Quality

✅ Multi-module structure follows standard Gradle conventions
✅ scanBasePackages = "com.hithomelabs" correctly picks up beans from both :common and :cftunnels-service
✅ BOM import via dependencyManagement properly centralizes dependency versions
⚠️ Package renamed from CFTunnels → cftunnels (lowercase) - ensure no external consumers reference the old package
⚠️ Old Dockerfile at root and old src/ directory will be removed on merge - confirm no references remain

Security

✅ Offline checksum validation for Gradle wrapper is more secure for air-gapped CI runners
✅ No exposed secrets in changes
✅ Input validation patterns unchanged (existing)

Testing

✅ Existing tests preserved and relocated to cftunnels-service/src/test/
⚠️ Missing dedicated unit tests for the new common module's RestTemplateConfig

CI/CD

✅ test_build.yml triggers on PRs to test branch
✅ test_image_build_push.yml triggers on push to test branch
⚠️ These workflows only trigger on the test branch - ensure main branch CI workflows are added separately if needed

Recommendations

PR title: The title "test" is not descriptive. Consider renaming to something like "refactor: multi-module Gradle restructuring with common and portainer-automation modules"
portainer-automation module: This is currently just a scaffold with an empty @SpringBootApplication class and no actual logic. Consider either completing it before merging into main, or deferring its addition until it has meaningful content. Including an empty module in main may cause confusion.
Dockerfile copies portainer-automation: If the module is kept, this is fine. But it does add build context bloat for an empty module.
Unit tests for common module: RestTemplateConfig in common should have its own unit test confirming the bean is created with the expected configuration (custom ObjectMapper with WRAP_ROOT_VALUE and NON_NULL serialization inclusion).
Gradle wrapper checksum: Verify that gradle-wrapper.jar.sha256 matches the actual gradle-wrapper.jar in the repo (it was not verifiable via my review, but the CI will catch mismatches).
Old source directory: Ensure the old src/ directory is fully deleted on main branch after merge to avoid stale reference confusion.

Decision: APPROVED ⚠️ with minor suggestions (items 1-6 above are recommendations, not blocking issues)

The restructuring is well-executed and the changes are sound. The multi-module approach improves separation of concerns and sets a good foundation for future development. The CI improvements are pragmatic for the Gitea CI runner environment.

## PR Review: test → main ### Summary This PR restructures the CFTunnels project from a single-module Gradle project into a multi-module architecture (`:common`, `:cftunnels-service`, `:portainer-automation`), relocates source code, extracts shared configuration into a `:common` module, updates CI workflows to use offline Gradle wrapper validation, and updates the Dockerfile for multi-module builds. ### What Changed - **Multi-module Gradle**: Root `settings.gradle` now includes 3 submodules; `build.gradle` uses Spring BOM with `apply false` - **Code moved**: Source moved from `src/main/java/.../CFTunnels/` to `cftunnels-service/src/main/java/.../cftunnels/` (also package rename) - **`common` module**: Extracts `RestTemplateConfig` as a shared bean for reuse across submodules - **`portainer-automation` module**: Scaffold submodule (minimal, referencing `:common`) - **CI/CD**: Replaced `gradle/actions/wrapper-validation@v3` with offline `sha256sum --check`; updated `test_image_build_push.yml` to use `:cftunnels-service:bootBuildImage` - **Dockerfile**: Updated to copy all submodule sources before build; moved to `cftunnels-service/Dockerfile` ### PR Review Checklist **Code Quality** - ✅ Multi-module structure follows standard Gradle conventions - ✅ `scanBasePackages = "com.hithomelabs"` correctly picks up beans from both `:common` and `:cftunnels-service` - ✅ BOM import via `dependencyManagement` properly centralizes dependency versions - ⚠️ Package renamed from `CFTunnels` → `cftunnels` (lowercase) - ensure no external consumers reference the old package - ⚠️ Old `Dockerfile` at root and old `src/` directory will be removed on merge - confirm no references remain **Security** - ✅ Offline checksum validation for Gradle wrapper is more secure for air-gapped CI runners - ✅ No exposed secrets in changes - ✅ Input validation patterns unchanged (existing) **Testing** - ✅ Existing tests preserved and relocated to `cftunnels-service/src/test/` - ⚠️ Missing dedicated unit tests for the new `common` module's `RestTemplateConfig` **CI/CD** - ✅ `test_build.yml` triggers on PRs to `test` branch - ✅ `test_image_build_push.yml` triggers on push to `test` branch - ⚠️ These workflows only trigger on the `test` branch - ensure `main` branch CI workflows are added separately if needed ### Recommendations 1. **PR title**: The title "test" is not descriptive. Consider renaming to something like "refactor: multi-module Gradle restructuring with common and portainer-automation modules" 2. **`portainer-automation` module**: This is currently just a scaffold with an empty `@SpringBootApplication` class and no actual logic. Consider either completing it before merging into `main`, or deferring its addition until it has meaningful content. Including an empty module in `main` may cause confusion. 3. **Dockerfile copies `portainer-automation`**: If the module is kept, this is fine. But it does add build context bloat for an empty module. 4. **Unit tests for `common` module**: `RestTemplateConfig` in `common` should have its own unit test confirming the bean is created with the expected configuration (custom `ObjectMapper` with `WRAP_ROOT_VALUE` and `NON_NULL` serialization inclusion). 5. **Gradle wrapper checksum**: Verify that `gradle-wrapper.jar.sha256` matches the actual `gradle-wrapper.jar` in the repo (it was not verifiable via my review, but the CI will catch mismatches). 6. **Old source directory**: Ensure the old `src/` directory is fully deleted on main branch after merge to avoid stale reference confusion. **Decision: APPROVED** ⚠️ with minor suggestions (items 1-6 above are recommendations, not blocking issues) The restructuring is well-executed and the changes are sound. The multi-module approach improves separation of concerns and sets a good foundation for future development. The CI improvements are pragmatic for the Gitea CI runner environment.

Linus approved these changes 2026-07-05 11:02:52 +00:00

hitanshu merged commit 9fb4c4fb14 into main

2026-07-05 11:04:09 +00:00

Sign in to join this conversation.

No reviewers