chore: Remove unused netcat and openssh-client#37662
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughThe changes in this pull request involve significant updates to the Dockerfile for the Appsmith application. The base image has been switched to Changes
Possibly related PRs
Suggested labels
Suggested reviewers
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 using PR comments)
Other keywords and placeholders
Documentation and Community
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (1)
deploy/docker/base.dockerfile (1)
Line range hint
1-5: Consider upgrading to Ubuntu 22.04 LTSWhile the current setup works, Ubuntu 20.04 is approaching end of standard support. Consider upgrading to Ubuntu 22.04 LTS for longer support lifecycle.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
deploy/docker/base.dockerfile(1 hunks)
🔇 Additional comments (2)
deploy/docker/base.dockerfile (2)
Line range hint 82-91: LGTM! Cleanup steps are thorough
The cleanup process removes unnecessary files and properly manages the volume declaration.
Line range hint 35-41: Address the TODO comment about Java version management
The hardcoded version is a temporary solution. Consider implementing a more robust version management strategy.
Let's check for the latest available versions:
Would you like help implementing a more robust version detection mechanism?
✅ Verification successful
TODO comment can be resolved - Linux binaries are now available
The latest release (17.0.13_11) includes Linux binaries for all architectures. You can now implement the version detection using the commented-out curl command:
version="$(curl --write-out '%{redirect_url}' 'https://github.com/adoptium/temurin17-binaries/releases/latest' | sed 's,.*jdk-,,')"🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check latest Temurin releases for Linux binaries
gh api repos/adoptium/temurin17-binaries/releases | jq -r '.[].tag_name' | head -n 5
Length of output: 155
Script:
#!/bin/bash
# Check if Linux binaries exist for the latest version
gh api repos/adoptium/temurin17-binaries/releases/latest | jq -r '.assets[].name' | grep -i linux
Length of output: 10315
| && apt-get upgrade --yes \ | ||
| && DEBIAN_FRONTEND=noninteractive apt-get install --no-install-recommends --yes \ | ||
| supervisor curl nfs-common gnupg wget netcat openssh-client \ | ||
| supervisor curl nfs-common gnupg wget \ |
There was a problem hiding this comment.
💡 Codebase verification
Documentation needs to be updated to remove deprecated migrate command reference
The packages can be safely removed, but the README.md still contains a reference to the deprecated appsmithctl migrate command in the "Migrate To New Server" section.
deploy/docker/README.md: Remove or update the "Migrate To New Server" section as the command is no longer supported
🔗 Analysis chain
Verify removal of netcat and openssh-client dependencies
The removal of these packages aligns with the deprecation of the appsmithctl migrate command.
Let's verify there are no remaining references to these packages or the migrate command:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any remaining references to netcat, openssh-client, or migrate command
rg -i "netcat|openssh-client|appsmithctl migrate"
Length of output: 150
Script:
#!/bin/bash
# Check the README.md content for context around the migrate command
cat deploy/docker/README.md
Length of output: 9529
These packages were installed in daf60fa for use with the
appsmithctl migratecommand, which has never been documented, been deprecated and then deleted from code for some time now.Removing these packages now as we don't need them any longer.
Tested on EE and verified
/test allto have passed.Automation
/test sanity
🔍 Cypress test results
Warning
Tests have not run on the HEAD 5f83399 yet
Sat, 23 Nov 2024 11:41:28 UTC
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit
New Features
ubuntu:20.04for improved compatibility.caddy-ratelimitmodule.Improvements