Revert go crypto on release/0.18 - #1952
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request transitions the build configuration across multiple services to use BoringCrypto with CGO enabled (GOEXPERIMENT=boringcrypto, CGO_ENABLED=1, GOFIPS140=off), removing previous FIPS initialization checks and adding boring build tag files. Review feedback identifies a critical issue in several Dockerfiles where explicitly passing CC=${CC} when CC is unset will cause Go compiler failures; exporting CC within the conditional block is suggested instead. Additionally, the go-synthetic Dockerfile enables CGO but lacks the necessary cross-compilation tools for arm64 builds, which will lead to compilation failures.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request transitions the build configuration to use BoringCrypto with CGO enabled and FIPS140 disabled, removing explicit runtime FIPS checks in favor of build-tagged imports. The reviewer pointed out that the new Dockerfile instructions run "apt install" without a prior "apt-get update", which will fail in minimal base images, and recommended using "apt-get" instead of "apt" for stability.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request transitions the build configuration from using noboringcrypto and GOFIPS140=certified to standard boringcrypto with CGO_ENABLED=1 and GOFIPS140=off. This involves updating several Dockerfiles to support cross-compilation for arm64 (by installing cross-compilers and setting CC), switching to a libc-compatible distroless base image, adding boring.go files that import crypto/tls/fipsonly under the boring build tag, and removing runtime FIPS initialization checks from the main entry points. A high-severity issue was identified in examples/instrumentation/go-synthetic/Dockerfile where the ARG variables (TARGETOS, TARGETARCH, BUILDARCH) are not inherited in the appbase stage, which will cause cross-compilation to fail. Redeclaring these arguments in the appbase stage is recommended.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request transitions the build configuration from a CGO-disabled, noboringcrypto setup to a CGO-enabled (CGO_ENABLED=1) boringcrypto setup across multiple services, updating Dockerfiles, removing runtime FIPS checks, and introducing crypto/tls/fipsonly imports. The review feedback highlights that because CGO is now enabled, cross-compilation is required when the target architecture differs from the host. Currently, the Dockerfiles only handle cross-compilation from amd64 to arm64. To prevent build failures on arm64 hosts targeting amd64, the reviewer recommends adding support for arm64 to amd64 cross-compilation in all affected Dockerfiles.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request transitions multiple services to use BoringCrypto with CGO enabled, updates Dockerfiles to support cross-compilation with architecture-specific compiler installations, and updates base images to GKE distroless libc. It also removes explicit FIPS-mode initialization checks from the Go entry points, introducing boring.go files with crypto/tls/fipsonly imports instead. Feedback on the Dockerfiles suggests cleaning up the apt package lists (rm -rf /var/lib/apt/lists/*) within the same RUN steps to reduce image sizes and keep intermediate build layers clean.
This replicates the approach from PR #1952 onto the main branch, reverting the adoption of Go 1.26 native FIPS mode (`crypto/fips140`, `GOFIPS140=certified`) and restoring dynamic linking with `BoringCrypto` (`gke-distroless/libc`, `GOEXPERIMENT=boringcrypto`, `CGO_ENABLED=1`), without altering `golangci-lint` configuration.
0c2fe3e to
e0cacd3
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request transitions the build configuration from using noboringcrypto with GOFIPS140=certified to standard boringcrypto with CGO enabled (CGO_ENABLED=1). It updates the Dockerfiles for all components to support cross-compilation with CGO, installs the necessary cross-compilers, and switches the final runtime images to a libc-compatible distroless base. Additionally, it introduces boring.go files to import crypto/tls/fipsonly under the boringcrypto build tag, removes explicit runtime fips140.Enabled() checks from main.go files, and updates linter configurations and helper scripts accordingly. There are no review comments to address, and I have no additional feedback to provide.
This replicates the approach from PR #1952 onto the main branch, reverting the adoption of Go 1.26 native FIPS mode (`crypto/fips140`, `GOFIPS140=certified`) and restoring dynamic linking with `BoringCrypto` (`gke-distroless/libc`, `GOEXPERIMENT=boringcrypto`, `CGO_ENABLED=1`), without altering `golangci-lint` configuration.
2 participants
No description provided.