chore: upgrade to go1.22.4 and rudder-go-kit #4758
Conversation
|
Note Currently processing new changes in this PR. This may take a few minutes, please wait... Files selected for processing (1)
WalkthroughThe updates include a minor version bump for Go in the Dockerfile, enhancements to the Makefile for formatting, and new functionality in a script for Docker Go version checking. Additionally, there are corrections and improvements in the Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
lvrach
changed the title
lvrach
force-pushed
the
chore.go-kit-upgrade
branch
from
6dcc0a8
to
d49d045
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range and nitpick comments (4)
Dockerfile (4)
Line range hint
14-14: Pin package versions inapk addto ensure reproducible builds.- RUN apk add --update make tzdata ca-certificates + RUN apk add --update make=4.3-r0 tzdata=2021a-r0 ca-certificates=20191127-r0
Line range hint
14-14: Use--no-cachewithapk addto avoid the need for--updateand to prevent caching.- RUN apk add --update make tzdata ca-certificates + RUN apk add --no-cache make tzdata ca-certificates
Line range hint
30-30: Consolidate consecutiveRUNinstructions to reduce layers and optimize the Docker image size.- RUN go build -o devtool ./cmd/devtool/ - RUN go build -o rudder-cli ./cmd/rudder-cli/ + RUN go build -o devtool ./cmd/devtool/ && \ + go build -o rudder-cli ./cmd/rudder-cli/
Line range hint
34-34: Pin versions inapk addand setWORKDIRbeforeCOPYto ensure reproducibility and correct file paths.+ WORKDIR /rudder-server - RUN apk --no-cache upgrade && \ - apk --no-cache add tzdata ca-certificates postgresql-client curl bash + RUN apk --no-cache upgrade && \ + apk --no-cache add tzdata=2021a-r0 ca-certificates=20191127-r0 postgresql-client=13.3-r0 curl=7.76.1-r0 bash=5.1.4-r0 - COPY --from=builder rudder-server/rudder-server . - COPY --from=builder rudder-server/build/wait-for-go/wait-for-go . - COPY --from=builder rudder-server/build/regulation-worker . - COPY --from=builder rudder-server/devtool . - COPY --from=builder rudder-server/rudder-cli /usr/bin/rudder-cli + COPY --from=builder /rudder-server/rudder-server . + COPY --from=builder /rudder-server/build/wait-for-go/wait-for-go . + COPY --from=builder /rudder-server/build/regulation-worker . + COPY --from=builder /rudder-server/devtool . + COPY --from=builder /rudder-server/rudder-cli /usr/bin/rudder-cliAlso applies to: 37-40
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (2)
go.modis excluded by!**/*.modgo.sumis excluded by!**/*.sum
Files selected for processing (1)
- Dockerfile (1 hunks)
Additional context used
Hadolint
Dockerfile
[warning] 14-14: Pin versions in apk add. Instead of
apk add <package>useapk add <package>=<version>(DL3018)
[info] 14-14: Use the
--no-cacheswitch to avoid the need to use--updateand remove/var/cache/apk/*when done installing packages (DL3019)
[warning] 25-25: This expansion will not see the mentioned assignment. (SC2098)
[info] 30-30: Multiple consecutive
RUNinstructions. Consider consolidation. (DL3059)
[warning] 34-34: Pin versions in apk add. Instead of
apk add <package>useapk add <package>=<version>(DL3018)
[warning] 37-37:
COPYto a relative destination withoutWORKDIRset. (DL3045)
[warning] 38-38:
COPYto a relative destination withoutWORKDIRset. (DL3045)
[warning] 39-39:
COPYto a relative destination withoutWORKDIRset. (DL3045)
[warning] 40-40:
COPYto a relative destination withoutWORKDIRset. (DL3045)
Additional comments not posted (2)
Dockerfile (2)
3-3: Update to Go version 1.22.4 aligns with the PR objectives to address vulnerabilities and maintain compatibility.
Line range hint
25-25: Ensure that theBUILD_DATEvariable is correctly expanded in theRUNcommand.Verification successful
The
BUILD_DATEvariable is correctly expanded in theRUNcommand within the Dockerfile.
Dockerfile:RUN BUILD_DATE=$(date "+%F,%T") \Dockerfile: LDFLAGS="-s -w -X main.version=${VERSION} -X main.commit=${COMMIT_HASH} -X main.buildDate=$BUILD_DATE -X main.builtBy=${REVISION} -X main.enterpriseToken=${ENTERPRISE_TOKEN} " \Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that BUILD_DATE is correctly expanded in the Dockerfile. # Test: Search for the BUILD_DATE usage. Expect: Correct expansion. rg --type dockerfile $'BUILD_DATE'Length of output: 68
Script:
#!/bin/bash # Description: Verify that BUILD_DATE is correctly expanded in the Dockerfile. # Test: Search for the BUILD_DATE usage in Dockerfiles. Expect: Correct expansion. rg 'BUILD_DATE' --glob '*Dockerfile*'Length of output: 275
lvrach
force-pushed
the
chore.go-kit-upgrade
branch
from
b1a3179
to
1e0e3e0
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- Dockerfile (1 hunks)
- Makefile (1 hunks)
- build/docker-go-version.sh (1 hunks)
Files skipped from review due to trivial changes (1)
- build/docker-go-version.sh
Files skipped from review as they are similar to previous changes (1)
- Dockerfile
Additional comments not posted (1)
Makefile (1)
110-110: Ensure the new scriptbuild/docker-go-version.shfunctions as expected.#!/bin/bash # Description: Verify the functionality of the script `build/docker-go-version.sh`. # Test: Execute the script and check for correct Go version output. ./build/docker-go-version.sh
lvrach
force-pushed
the
chore.go-kit-upgrade
branch
2 times, most recently
from
54ae53c
to
53837eb
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
Outside diff range and nitpick comments (4)
Dockerfile (4)
Line range hint
16-16: Pin package versions to ensure reproducible builds.- RUN apk add --update make tzdata ca-certificates + RUN apk add --no-cache make=4.3-r0 tzdata=2021a-r0 ca-certificates=20191127-r4This change pins the versions of the packages to ensure that the build is reproducible and consistent across environments. It also uses
--no-cacheto avoid the need to use--updateand to remove/var/cache/apk/*when done installing packages.
Line range hint
27-27: Correct the environment variable expansion in theRUNcommand.- RUN BUILD_DATE=$(date "+%F,%T") \ + RUN export BUILD_DATE=$(date "+%F,%T") \This change ensures that the
BUILD_DATEvariable is correctly exported and available in the subsequent commands in the sameRUNinstruction.
Line range hint
32-32: Consolidate consecutiveRUNinstructions to reduce layers.- RUN go build -o devtool ./cmd/devtool/ - RUN go build -o rudder-cli ./cmd/rudder-cli/ + RUN go build -o devtool ./cmd/devtool/ && \ + go build -o rudder-cli ./cmd/rudder-cli/This change consolidates the
RUNinstructions into a single layer, reducing the total number of layers in the image, which can improve the build time and the size of the final image.
Line range hint
39-42: Set aWORKDIRbefore copying files to ensure the destination is clear.+ WORKDIR /rudder-server COPY --from=builder rudder-server/rudder-server . COPY --from=builder rudder-server/build/wait-for-go/wait-for-go . COPY --from=builder rudder-server/build/regulation-worker . COPY --from=builder rudder-server/devtool . COPY --from=builder rudder-server/rudder-cli /usr/bin/rudder-cliAdding a
WORKDIRdirective ensures that allCOPYcommands are relative to a known directory, improving the clarity and maintainability of the Dockerfile.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (4)
- Dockerfile (1 hunks)
- Makefile (1 hunks)
- build/docker-go-version.sh (1 hunks)
- suppression-backup-service/Dockerfile (1 hunks)
Files skipped from review due to trivial changes (1)
- suppression-backup-service/Dockerfile
Files skipped from review as they are similar to previous changes (1)
- Makefile
Additional context used
Shellcheck
build/docker-go-version.sh
[style] 11-11: Use $(...) notation instead of legacy backticks
.... (SC2006)
[info] 11-11: read without -r will mangle backslashes. (SC2162)
[info] 11-11: Double quote to prevent globbing and word splitting. (SC2086)
[info] 17-17: Double quote to prevent globbing and word splitting. (SC2086)
[info] 17-17: Double quote to prevent globbing and word splitting. (SC2086)
Hadolint
Dockerfile
[warning] 16-16: Pin versions in apk add. Instead of
apk add <package>useapk add <package>=<version>(DL3018)
[info] 16-16: Use the
--no-cacheswitch to avoid the need to use--updateand remove/var/cache/apk/*when done installing packages (DL3019)
[warning] 27-27: This expansion will not see the mentioned assignment. (SC2098)
[info] 32-32: Multiple consecutive
RUNinstructions. Consider consolidation. (DL3059)
[warning] 36-36: Pin versions in apk add. Instead of
apk add <package>useapk add <package>=<version>(DL3018)
[warning] 39-39:
COPYto a relative destination withoutWORKDIRset. (DL3045)
[warning] 40-40:
COPYto a relative destination withoutWORKDIRset. (DL3045)
[warning] 41-41:
COPYto a relative destination withoutWORKDIRset. (DL3045)
[warning] 42-42:
COPYto a relative destination withoutWORKDIRset. (DL3045)
build/docker-go-version.sh
Outdated
|
|
||
| for DOCKERFILE_PATH in "$@" | ||
| do | ||
| sed -i $BAK_SUFFIX "s/ARG GO_VERSION=[0-9].*/ARG GO_VERSION=$v/" $DOCKERFILE_PATH && rm $DOCKERFILE_PATH$BAK_SUFFIX |
There was a problem hiding this comment.
Ensure variables are properly quoted to prevent word splitting and globbing.
- sed -i $BAK_SUFFIX "s/ARG GO_VERSION=[0-9].*/ARG GO_VERSION=$v/" $DOCKERFILE_PATH && rm $DOCKERFILE_PATH$BAK_SUFFIX
+ sed -i "$BAK_SUFFIX" "s/ARG GO_VERSION=[0-9].*/ARG GO_VERSION=$v/" "$DOCKERFILE_PATH" && rm "$DOCKERFILE_PATH$BAK_SUFFIX"This change wraps the variables in double quotes to ensure that filenames with spaces or special characters are handled correctly.
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| sed -i $BAK_SUFFIX "s/ARG GO_VERSION=[0-9].*/ARG GO_VERSION=$v/" $DOCKERFILE_PATH && rm $DOCKERFILE_PATH$BAK_SUFFIX | |
| sed -i "$BAK_SUFFIX" "s/ARG GO_VERSION=[0-9].*/ARG GO_VERSION=$v/" "$DOCKERFILE_PATH" && rm "$DOCKERFILE_PATH$BAK_SUFFIX" |
Tools
Shellcheck
[info] 17-17: Double quote to prevent globbing and word splitting. (SC2086)
[info] 17-17: Double quote to prevent globbing and word splitting. (SC2086)
lvrach
force-pushed
the
chore.go-kit-upgrade
branch
2 times, most recently
from
872d710
to
12ac3a1
Compare
lvrach
force-pushed
the
chore.go-kit-upgrade
branch
from
12ac3a1
to
3329b70
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- build/docker-go-version.sh (1 hunks)
Files not reviewed due to errors (1)
- build/docker-go-version.sh (no review received)
Additional context used
Shellcheck
build/docker-go-version.sh
[style] 11-11: Use $(...) notation instead of legacy backticks
.... (SC2006)
[info] 11-11: read without -r will mangle backslashes. (SC2162)
[info] 11-11: Double quote to prevent globbing and word splitting. (SC2086)
lvrach
force-pushed
the
chore.go-kit-upgrade
branch
from
35a9481
to
456a535
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- warehouse/router/state_export_data.go (2 hunks)
- warehouse/router/upload.go (1 hunks)
- warehouse/router/upload_stats.go (1 hunks)
Files skipped from review due to trivial changes (2)
- warehouse/router/state_export_data.go
- warehouse/router/upload_stats.go
Additional comments not posted (1)
warehouse/router/upload.go (1)
215-218: The addition of thesourceCategorytag in thegaugeStatmethod enhances tracking capabilities. Ensure that this new tag is properly integrated and utilized in relevant parts of the system.Verification successful
The
sourceCategorytag is consistently used across various parts of the system, including test files, state export data, integrations, and reporting. This indicates that the integration of the new tag is thorough and well-implemented.
warehouse/router/upload_test.gowarehouse/router/state_export_data.gowarehouse/integrations/snowflake/snowflake.gowarehouse/integrations/deltalake/deltalake.goutils/types/reporting_types.gorouter/worker.gorouter/batchrouter/handle_observability.gorouter/types/types.goprocessor/processor.goprocessor/transformer/transformer.gointegration_test/warehouse/integration_test.goenterprise/reporting/event_stats_test.goenterprise/reporting/event_stats.goScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the integration of the `sourceCategory` tag across the system. # Test: Search for the usage of `sourceCategory` tag. Expect: Consistent usage across relevant parts. rg --type go $'sourceCategory'Length of output: 3810
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #4758 +/- ##
==========================================
+ Coverage 74.60% 74.68% +0.08%
==========================================
Files 414 414
Lines 48595 48597 +2
==========================================
+ Hits 36254 36297 +43
+ Misses 9961 9937 -24
+ Partials 2380 2363 -17 ☔ View full report in Codecov by Sentry. |
Description
Linear Ticket
resolves PIPE-1166
Security
Summary by CodeRabbit
Bug Fixes
Chores
1.22.3to1.22.4in Dockerfile for improved compatibility.Refactor
guageStattogaugeStatin multiple files to correct a typo and ensure consistency.