-
Notifications
You must be signed in to change notification settings - Fork 1
Lea/lokali #374
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
base: main
Are you sure you want to change the base?
Lea/lokali #374
Changes from all commits
0219e8d
3b8375a
67c8b04
633731f
57bd790
532c0aa
52ddb9d
433792b
22699a0
ee8a69a
8c4af36
f4c0dc6
80d0fb8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -21,8 +21,16 @@ RUN apt-get update && apt-get install -y --no-install-recommends \ | |
| libxcb-xinerama0 libxcb-cursor0 libxcb-keysyms1 libxcb-render-util0 \ | ||
| libxcb-randr0 && rm -rf /var/lib/apt/lists/* | ||
|
|
||
| # # ───────── optional CA certs ───────── | ||
| RUN if [ -d certs ]; then \ | ||
| echo "Copying certs directory..."; \ | ||
| tar -cf - certs | tar -xf -; \ | ||
| else \ | ||
| echo "No certs directory, skipping copy."; \ | ||
| fi | ||
|
|
||
| # ───────── optional CA certs ───────── | ||
|
Comment on lines
+24
to
32
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. style: Duplicate certificate handling sections with different approaches - the first RUN block (lines 25-30) and the second RUN block (lines 34-38) both attempt to handle certificates but with different logic. Is the first RUN block (lines 25-30) intended to replace the second one, or should they work together? |
||
| COPY certs /app/certs | ||
|
|
||
| RUN if [ -d ./certs ] && [ "$(ls ./certs/*.crt 2>/dev/null)" ]; then \ | ||
| echo "Configuring NetFree certificates..."; \ | ||
| cp ./certs/*.crt /usr/local/share/ca-certificates/; \ | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -6,7 +6,13 @@ WORKDIR /app | |||||||||||||||||||||||||
| ARG USE_NETFREE=true | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| RUN apt-get update && apt-get install -y --no-install-recommends ca-certificates curl && rm -rf /var/lib/apt/lists/* | ||||||||||||||||||||||||||
| COPY certs /app/certs | ||||||||||||||||||||||||||
| RUN if [ -d certs ]; then \ | ||||||||||||||||||||||||||
| echo "Copying certs directory..."; \ | ||||||||||||||||||||||||||
| tar -cf - certs | tar -xf -; \ | ||||||||||||||||||||||||||
| else \ | ||||||||||||||||||||||||||
| echo "No certs directory, skipping copy."; \ | ||||||||||||||||||||||||||
| fi | ||||||||||||||||||||||||||
|
Comment on lines
+9
to
+14
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. logic: The tar command logic is flawed - it creates and extracts in the same directory without actually copying anything. The
Suggested change
|
||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| # System CA + add NetFree certs | ||||||||||||||||||||||||||
| RUN if [ "$USE_NETFREE" = "true" ] && [ -d ./certs ] && [ "$(ls ./certs/*.crt 2>/dev/null)" ]; then \ | ||||||||||||||||||||||||||
| echo "Configuring NetFree certificates..."; \ | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,14 +4,24 @@ WORKDIR /app | |
|
|
||
| COPY requirements.txt . | ||
|
|
||
| COPY certs /app/certs | ||
|
|
||
| RUN apt-get update && \ | ||
| apt-get install -y ca-certificates && \ | ||
| cp /app/certs/*.crt /usr/local/share/ca-certificates/ && \ | ||
| update-ca-certificates && \ | ||
| apt-get clean && \ | ||
| rm -rf /var/lib/apt/lists/* | ||
| RUN if [ -d certs ]; then \ | ||
| echo "Copying certs directory..."; \ | ||
| tar -cf - certs | tar -xf -; \ | ||
| else \ | ||
| echo "No certs directory, skipping copy."; \ | ||
| fi | ||
|
Comment on lines
+7
to
+12
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. logic: This tar command logic is incorrect - it creates a tar archive of |
||
|
|
||
|
|
||
| RUN apt-get update && apt-get install -y ca-certificates && \ | ||
| if [ "$USE_NETFREE" = "true" ] && [ -d ./certs ] && [ "$(ls ./certs/*.crt 2>/dev/null)" ]; then \ | ||
| echo "Configuring NetFree certificates..."; \ | ||
| cp ./certs/*.crt /usr/local/share/ca-certificates/; \ | ||
| update-ca-certificates; \ | ||
| else \ | ||
| echo "Skipping certificate configuration (USE_NETFREE=$USE_NETFREE)"; \ | ||
| fi && \ | ||
| apt-get clean && rm -rf /var/lib/apt/lists/* | ||
|
|
||
|
|
||
| ENV REQUESTS_CA_BUNDLE=/etc/ssl/certs/ca-certificates.crt | ||
| ENV SSL_CERT_FILE=/etc/ssl/certs/ca-certificates.crt | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,9 +3,16 @@ FROM flink:1.20.0-scala_2.12-java11 | |
|
|
||
| USER root | ||
|
|
||
|
|
||
| # Add local CA (place netfree-ca.crt next to this Dockerfile before building) | ||
| # COPY netfree-ca.crt /usr/local/share/ca-certificates/netfree-ca.crt | ||
| # RUN chmod 644 /usr/local/share/ca-certificates/netfree-ca.crt && update-ca-certificates | ||
| RUN if [ -f netfree-ca.crt ]; then \ | ||
| echo "Installing netfree-ca.crt..."; \ | ||
| cp netfree-ca.crt /usr/local/share/ca-certificates/netfree-ca.crt; \ | ||
| chmod 644 /usr/local/share/ca-certificates/netfree-ca.crt; \ | ||
| update-ca-certificates; \ | ||
| else \ | ||
| echo "No netfree-ca.crt found, skipping."; \ | ||
| fi | ||
|
Comment on lines
+8
to
+15
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. logic: missing COPY command - the RUN command checks for netfree-ca.crt but there's no COPY instruction to make the file available in the build context |
||
|
|
||
| ENV SSL_CERT_FILE=/etc/ssl/certs/ca-certificates.crt | ||
| ENV REQUESTS_CA_BUNDLE=/etc/ssl/certs/ca-certificates.crt | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,12 +7,17 @@ RUN apt-get update && \ | |
|
|
||
| WORKDIR /app | ||
|
|
||
| # Copy certificates | ||
| COPY certs /app/certs | ||
| # Copy and install certificates if present | ||
| RUN if [ -d certs ] && [ "$(ls certs/*.crt 2>/dev/null)" ]; then \ | ||
| echo "Copying and installing certificates..."; \ | ||
| mkdir -p /app/certs; \ | ||
| tar -cf - certs | tar -xf -; \ | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. style: Using Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time! |
||
| cp certs/*.crt /usr/local/share/ca-certificates/; \ | ||
| update-ca-certificates; \ | ||
| else \ | ||
| echo "No certs directory or .crt files found - skipping."; \ | ||
| fi | ||
|
|
||
| # Install certificates | ||
| RUN cp /app/certs/*.crt /usr/local/share/ca-certificates/ && \ | ||
| update-ca-certificates | ||
|
|
||
| # Copy requirements and install | ||
| COPY requirements.txt . | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -2,7 +2,13 @@ FROM flink:1.20.0-scala_2.12-java11 | |||||
| USER root | ||||||
|
|
||||||
| # Copy certs dir (may be empty) and trust *.crt if present | ||||||
| COPY certs/ /tmp/certs/ | ||||||
| RUN if [ -d certs ]; then \ | ||||||
| echo "Copying certs directory..."; \ | ||||||
| tar -cf - certs | tar -xf -; \ | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. logic: Critical logic error: copying to root directory instead of /tmp/certs/. This breaks certificate processing on lines 15-21 which expects files in /tmp/certs/
Suggested change
|
||||||
| else \ | ||||||
| echo "No certs directory, skipping copy."; \ | ||||||
| fi | ||||||
|
|
||||||
|
|
||||||
| RUN apt-get update && apt-get install -y --no-install-recommends ca-certificates curl && \ | ||||||
| rm -rf /var/lib/apt/lists/* && \ | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| FROM python:3.11-slim | ||
| RUN pip install --no-cache-dir confluent-kafka psycopg2-binary | ||
| WORKDIR /app | ||
| COPY fruit_defect_sink.py ./ | ||
| CMD ["python","-u","fruit_defect_sink.py"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
logic: This logic is flawed - the 'certs' directory check happens during container runtime, not build time, and the tar command copies to the current directory (.) which may not be the intended destination