-
Notifications
You must be signed in to change notification settings - Fork 30
chore: update SDM image to be rootless #143
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
chore: update SDM image to be rootless #143
Conversation
📝 WalkthroughWalkthroughThe pull request updates the Dockerfile by changing the base image from Changes
Possibly related PRs
Suggested labels
Suggested reviewers
What do you think about these suggestions? 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🔇 Additional comments (2)Dockerfile (2)
The major version bump from 2.0.0 to 3.0.0 could introduce breaking changes. While using a specific SHA is great for reproducibility, shall we add a quick comment explaining why we chose this specific version/SHA? wdyt?
The
Consider adding these permission checks earlier in the Dockerfile: WORKDIR /airbyte/integration_code
+
+# Ensure correct permissions for the airbyte user
+RUN chown -R airbyte:airbyte /airbyte/integration_code What do you think about this addition? 🤔 ✅ Verification successfulThe previous attempt with Docker commands didn't work in the sandbox environment. Let's analyze the codebase directly to understand the configuration and permissions setup. This will help us verify the rootless setup and provide appropriate guidance. The rootless configuration looks good! 👍 The setup is secure because:
Would you like to consider one small enhancement? We could add a simple health check to verify the container starts correctly: HEALTHCHECK --interval=30s --timeout=30s --start-period=5s --retries=3 \
CMD python -c "import main" || exit 1 What do you think? 🤔 🏁 Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Description: Verify permissions and user configuration
# Build the image locally
docker build -t temp-image . 2>/dev/null
# Check user and permissions
docker run --rm temp-image sh -c "id airbyte && ls -la /airbyte/integration_code" 2>/dev/null
# Verify entrypoint permissions
docker run --rm temp-image sh -c "ls -la /airbyte/integration_code/main.py" 2>/dev/null
# Clean up
docker rmi temp-image 2>/dev/null
Length of output: 216 Script: #!/bin/bash
# Let's check the Dockerfile contents first to understand the context better
cat Dockerfile
# Let's also check if there are any entrypoint scripts
fd "entrypoint" --type f
# And check for main.py which seems to be referenced
fd "main.py" --type f
Length of output: 1497 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
CodeRabbit Configuration File (
|
This change is only at the SDM image level.
We want to use the latest version of our python connector base image.
This version has an airbyte user set with the right level of permissions.
We also modify the current Dockerfile to declare the current user as
airbyte
.Summary by CodeRabbit
New Features
Security Improvements