Bump codacy-engine-scala-seed and Amazon Corretto runtime image - #18
Bump codacy-engine-scala-seed and Amazon Corretto runtime image#18stefanvacareanu7 wants to merge 2 commits into
Conversation
Update codacy-engine-scala-seed 5.0.2 -> 6.1.4 (matching the version used across other Codacy engine repos) and fix the resulting API change: codacy-plugins-api renamed Result.Issue's `file` field to `filename`. Also bump the amazoncorretto runtime base image from 8-alpine3.14-jre to 8-alpine3.21-jre. The golang builder image is intentionally left at 1.15.15-alpine3.14 and CircleCI/plugins-test orbs are left unchanged - see PR description for why. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 0 |
| Duplication | 0 |
AI Reviewer: first review requested successfully. AI can make mistakes. Always validate suggestions.
TIP This summary will be updated as you push new changes.
CircleCI's check_format_and_test job now fails before build.sbt is even read: sbt 1.4.3's launcher calls the now fully-disabled System::setSecurityManager on the executor's current JDK, throwing UnsupportedOperationException. Newer sbt versions don't touch this API. 1.10.7 matches the version already in use in codacy-checkstyle. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull Request Overview
This PR upgrades the codacy-engine-scala-seed to version 6.1.4 and updates the Amazon Corretto runtime image to 8-alpine3.21-jre. While the PR attempts to address the API changes in the seed library, it introduces a critical logic error in the issue filtering mechanism that will likely result in empty results during analysis.
Additionally, the Docker configuration contains a version mismatch between the build and runtime stages (Alpine 3.14 vs 3.21) which poses a high risk of runtime failures due to library incompatibilities. Significant technical debt remains regarding the use of an end-of-life Go version (1.15) and non-deterministic build steps in the Dockerfile.
About this PR
- The project is pinned to Go 1.15 due to a dependency on an abandoned fork of
go/loader. This represents significant technical debt and a security risk as Go 1.15 is long past its end-of-life. - The Docker build process uses
go get -u, which pulls non-deterministic upstream changes. This is known to be incompatible with the current Go 1.15 environment and causes build instability.
Test suggestions
- Verify result filtering logic in Aligncheck.scala correctly matches files after the filename field rename
- Verify Docker image builds and runs correctly with the new Corretto 8-alpine3.21 runtime base
Prompt proposal for missing tests
Consider implementing these tests if applicable:
1. Verify result filtering logic in Aligncheck.scala correctly matches files after the filename field rename
2. Verify Docker image builds and runs correctly with the new Corretto 8-alpine3.21 runtime base
TIP Improve review quality by adding custom instructions
TIP How was this review? Give us feedback
| filesOpt.fold(results) { files => | ||
| results.collect { | ||
| case res: Result.Issue if files.contains(res.file) => res | ||
| case res: Result.Issue if files.contains(res.filename) => res |
There was a problem hiding this comment.
🔴 HIGH RISK
The upgrade to codacy-engine-scala-seed v6 changed the Result.Issue model field from file: Source.File to filename: String. Because the files variable is a Set[Source.File], the contains check will now always return false at runtime due to the type mismatch, effectively filtering out all issues when the tool is run on a specific file list.
| case res: Result.Issue if files.contains(res.filename) => res | |
| case res: Result.Issue if files.contains(Source.File(res.filename)) => res |
| RUN go get -u gitlab.com/opennota/check/cmd/aligncheck | ||
|
|
||
| FROM amazoncorretto:8-alpine3.14-jre | ||
| FROM amazoncorretto:8-alpine3.21-jre |
There was a problem hiding this comment.
🟡 MEDIUM RISK
The multi-stage build uses Alpine 3.14 for the builder stage (Line 1) and Alpine 3.21 for the runtime stage (Line 6). Mixing these versions when copying system-dependent components like the Go toolchain and the aligncheck binary is risky because of potential ABI incompatibilities or changed library paths in musl across major Alpine releases. This can cause the tool to fail at runtime when resolving symbols or executing Go commands.
There was a problem hiding this comment.
Pull Request Overview
While this PR successfully upgrades dependencies and fixes compilation, it introduces a logic bug in the filtering mechanism and potential runtime issues in the Docker container.
- Logic Error: In
Aligncheck.scala, the transition to the newResult.IssueAPI introduced a type mismatch (String vs Source.File) that will cause the tool to filter out all results during partial scans. - Compatibility Risk: There is a version mismatch between the Docker builder stage (Alpine 3.14) and the runtime stage (Alpine 3.21), which risks binary incompatibility due to
muslversion differences. - Acceptance Criteria Gaps: Although the code compiles, the implementation of the result filtering is functionally broken, and the lack of automated tests for this logic prevents regression catch.
About this PR
- The Dockerfile continues to use 'go get -u', which pulls unpinned dependencies. This is currently causing build failures on the master branch and should be addressed to ensure build reproducibility and stability.
Test suggestions
- Verify that results are correctly filtered by filename in Aligncheck.filterResultsForFiles using the updated field name.
- Verify that the aligncheck binary built with Go 1.15 remains compatible with the new Alpine 3.21-based runtime image.
Prompt proposal for missing tests
Consider implementing these tests if applicable:
1. Verify that results are correctly filtered by filename in Aligncheck.filterResultsForFiles using the updated field name.
2. Verify that the aligncheck binary built with Go 1.15 remains compatible with the new Alpine 3.21-based runtime image.
TIP Improve review quality by adding custom instructions
TIP How was this review? Give us feedback
| filesOpt.fold(results) { files => | ||
| results.collect { | ||
| case res: Result.Issue if files.contains(res.file) => res | ||
| case res: Result.Issue if files.contains(res.filename) => res |
There was a problem hiding this comment.
🔴 HIGH RISK
The filtering logic contains a type mismatch and a logic error. The files set contains Source.File objects, but res.filename is a String; this mismatch causes the filter to exclude all results when specific files are requested (e.g., in PR analysis). Additionally, using collect specifically for Result.Issue causes FileError and GenericError to be dropped.
Consider using filter to preserve all relevant types and converting the filename back to a Source.File for comparison:
| case res: Result.Issue if files.contains(res.filename) => res | |
| case res: Result.Issue if files.contains(Source.File(res.filename)) => res |
| RUN go get -u gitlab.com/opennota/check/cmd/aligncheck | ||
|
|
||
| FROM amazoncorretto:8-alpine3.14-jre | ||
| FROM amazoncorretto:8-alpine3.21-jre |
There was a problem hiding this comment.
🟡 MEDIUM RISK
The runtime image has been updated to Alpine 3.21, but the builder stage still uses Alpine 3.14. This version mismatch between stages can cause stability issues or runtime crashes because the Go binaries were compiled against an older version of musl. The builder stage should be updated to use a matching Alpine 3.21 image.
Summary
com.codacy::codacy-engine-scala-seed5.0.2→6.1.4(the latest version, and the one already in use across other Codacy engine repos e.g.codacy-codesniffer).codacy-plugins-api8.1.1 renamedResult.Issue.file→Result.Issue.filename. Updated the one usage inAligncheck.scala.amazoncorretto:8-alpine3.14-jre→8-alpine3.21-jre(latest available8-alpinetag; alpine 3.14 is long EOL).codacy/base@13.1.1,codacy/plugins-test@2.1.2) are already the latest published versions — confirmed by cross-checking every othercodacy-*repo in the org, none use a newer version. No change needed.What I intentionally did not change, and why
The
Dockerfile'sgolangbuilder image is left atgolang:1.15.15-alpine3.14. I attempted to bump it (tried every Go version from 1.16 through 1.26.5) and every one broke aligncheck at runtime withinternal error: nil Pkg importing "io"— the tool bundles a 2018-vintage, abandoned fork ofgolang.org/x/tools'sgo/loader, and its export-data reader can't parse type info produced by any newer Go compiler. This is exactly what commit25efdd0("fix: Fix multiple tests after golang bump") did in 2021 — it looks like a fixture fix in the README's telling, but the actual diff shows it reverted a1.17.2bump straight back to1.15.15. So1.15.15is a hard pin, not staleness.Pre-existing, unrelated blocker (please read before merging)
Independent of anything in this PR,
docker buildcurrently fails on master too (verified by stashing my changes and rebuilding the unmodifiedDockerfile):go get -ualways fetches upstreamgolang.org/x/toolsat its current HEAD (there's no version pin), and HEAD has since drifted into a state where GOPATH-mode (Go 1.15) resolution reports a false import cycle ininternal/typesinternal/internal/stdlib. I tried pinningx/toolsto the specific old commit the tool used to resolve to (677d2ff680c1) and to whatever module-mode MVS picks — both still fail the runtime import step with the samenil Pkgerror shown above, even on trivial, valid Go input. I could not find a combination that both builds and produces correct results in the time I spent, and fixing it for real looks like it needs vendoring/pinningx/toolsproperly (a go.mod + go.sum for the builder stage) rather than a version bump — a bigger, riskier change I don't think should be bundled into a routine dependency-bump PR without a maintainer decision on direction (fix the pin vs. retire/replace the tool).I also noticed the most recent merge to master (#17) already has a failing
check_format_and_testCircleCI check (job finished in ~5s, looks like an infra/flake issue, not a code failure) — unrelated to this PR, flagging for visibility.Suspicious repository content
None found — README's Agent Playbook section is legitimate documentation, no embedded instructions attempting to redirect agent behavior.
Test plan
sbt "scalafmt::test; test:scalafmt::test; sbt:scalafmt::test; test"— passes locally.sbt universal:stage— succeeds.docker build -t codacy-aligncheck .— fails, but reproducibly fails identically on unmodified master (pre-existing, see above).codacy-plugins-testjson/pattern/multiple— could not runmultipleend-to-end since the image can't be built;json/patterndon't require the Go binary and were exercised against a manually-patched image build during investigation.