Skip to content

Conversation

taik0
Copy link
Contributor

@taik0 taik0 commented Nov 25, 2022

Signed-off-by: Daniel Ortiz [email protected]

KrakenD is a stateless Open Source API Gateway built on top of the Lura Framework (formerly KrakenD Framework).
To know more about KrakenD, please read our documentation

https://github.com/krakendio/krakend-ce

Checklist for Review

NOTE: This checklist is intended for the use of the Official Images maintainers both to track the status of your PR and to help inform you and others of where we're at. As such, please leave the "checking" of items to the repository maintainers. If there is a point below for which you would like to provide additional information or note completion, please do so by commenting on the PR. Thanks! (and thanks for staying patient with us ❤️)

@taik0
Copy link
Contributor Author

taik0 commented Nov 25, 2022

Documentation PR: docker-library/docs#2082

@taik0
Copy link
Contributor Author

taik0 commented Nov 25, 2022

I didn't realize that that repository used the main branch instead of master. I pushed a master branch, so we don't have to change anything on this PR.
A re-run should work now.

@taik0
Copy link
Contributor Author

taik0 commented Dec 13, 2022

Hi, there's something in my hand that can be done to unblock this PR?

Thank you!

@yosifkit
Copy link
Member

Hello! ✨

Thanks for your interest in contributing to the official images program. 💭

As you may have noticed, we've usually got a pretty decently sized queue of new images (not to mention image updates and maintenance of images under @docker-library which are maintained by the core official images team). As such, it may be some time before we get to reviewing this image (image updates get priority both because users expect them and because reviewing new images is a more involved process than reviewing updates), so we apologize in advance! Please be patient with us -- rest assured, we've seen your PR and it's in the queue. ❤️

We do try to proactively add and update the "new image checklist" on each PR, so if you haven't looked at it yet, that's a good use of time while you wait. ☔

Thanks! 💖 💙 💚 ❤️

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@JAORMX
Copy link

JAORMX commented Mar 14, 2023

Hey folks! As a krakend user we'd really benefit from getting this PR accepted. The project is growing fast and this would be very beneficial not only for the project itself but for the community that's growing around it.

@github-actions

This comment has been minimized.

This comment has been minimized.

@taik0
Copy link
Contributor Author

taik0 commented Dec 14, 2023

Hi team,

Are new images still blocked?

@taik0 taik0 requested a review from a team as a code owner April 23, 2024 12:26

This comment has been minimized.

@taik0
Copy link
Contributor Author

taik0 commented Apr 23, 2024

Hi @yosifkit
Is something wrong on our side? There hasn't been any response in almost 1.5 years, are new docker images still being accepted?

@LaurentGoderre
Copy link
Member

I opened a PR to optimize the image: krakend/docker-library#4

Could you also open a docs PR as explained here: https://github.com/docker-library/docs/blob/master/README.md

This comment has been minimized.

@taik0
Copy link
Contributor Author

taik0 commented May 23, 2024

Hi @LaurentGoderre, thank you for the PR, it's already merged.

The doc PR is here docker-library/docs#2082

LaurentGoderre
LaurentGoderre previously approved these changes May 23, 2024
@LaurentGoderre
Copy link
Member

Small comments on the docs

@alombarte
Copy link
Contributor

Comments on the docs are addressed. Thank you very much for your review!

This comment has been minimized.

@alombarte
Copy link
Contributor

Hello @yosifkit. I see that @LaurentGoderre approved the PR almost one year ago. What are the chances of the image being added as an official image? Shall we continue updating this PR after two years?

@yosifkit
Copy link
Member

yosifkit commented Mar 7, 2025

Apologies for the long delay. 🙇

I had a few notes typed up but forgot to flesh them out and post them. 🤦 So here they are!

Blockers:

  • gpgconf --kill all and rm -rf "$GNUPGHOME" needs to be added somewhere after the gpg --verify (but within the RUN line)
  • these two images are FROM alpine:3.18 which is end of life in just two months (https://alpinelinux.org/releases/); but it looks like the Dockerfiles for newer krackend versions in https://github.com/krakend/docker-library are already updated. So the ask, do you want to stick with these older krackend releases and replace them in a follow up PR or just update this PR with the latest supported release(s)?
  • the user conditional should be swapped so that any non-root user can work (e.g., docker run --user 1234:5678 ...) and so that the su-exec is only run as root since that is the only time it can work; it could also fall through to the exec "$@" at the end:
    -    if [ "$(id -u)" = 1000 ]; then
    -        exec "$@"    
    -    else
    +    if [ "$(id -u)" = 0 ]; then
             # use su-exec to drop to a non-root user
             exec su-exec krakend "$@"
    +    else
    +        exec "$@"
         fi
    
    # alternative
    
    -    if [ "$(id -u)" = 1000 ]; then
    -        exec "$@"    
    -    else
    -        # use su-exec to drop to a non-root user
    -        exec su-exec krakend "$@"
    +    if [ "$(id -u)" = 0 ]; then
    +        set -- su-exec krakend "$@"
         fi
  • the sha256sum should be done before the tar extraction

Suggestions:

  • the tar xvf should avoid "traditional" flags and just use Unix and GNU style flags: https://manpages.debian.org/bookworm/tar/tar.1.en.html
    -    tar xzf krakend.tar.gz -C / --strip-components 1; \
    +    tar -xzf krakend.tar.gz -C / --strip-components 1; \
  • perhaps the entrypoint could be added to the PATH (like /usr/local/bin/)?
    COPY docker-entrypoint.sh /usr/local/bin/
    ENTRYPOINT [ "docker-entrypoint.sh" ]
  • 🤔 is the -c /etc/krakend/krakend.json in the default CMD important for users adding their own run flags or perhaps already the default config file location such that the flag is unnecessary?

This comment has been minimized.

@taik0
Copy link
Contributor Author

taik0 commented Mar 10, 2025

Hi @yosifkit

Thank you for the review! I've addressed all the required changes and updated the KrakenD versions to the latest (which include Alpine 3.21).
Regarding the CMD, this has been the default in all KrakenD history, and users expect that running the container without arguments will start KrakenD with those settings (the software does not enforce a default config file, but that parameters are widely used as a default).

Thank you!

yosifkit
yosifkit previously approved these changes Mar 11, 2025
@yosifkit yosifkit requested a review from tianon March 11, 2025 21:57
@tianon
Copy link
Member

tianon commented Mar 12, 2025

To be explicit/clear, I don't think anything below is an outright blocker; mostly food-for-thought and suggestion-for-improvement. 👍


+Tags: 2.9.2
+Architectures: amd64, arm64v8
+GitCommit: a84f9f39fb6dc30354dee6a3ecd35666791aeb9e
+Directory: 2.9.2

This is only here to backfill the version, and it'll be removed after it's built/pushed, right?
(It's not "supported" in the sense that there might be a "2.9.2.1" release if there were a bug found in it, for example.)

Removing tags here will remove them from the "Supported" section on the Hub readme (and will prevent us from spending cycles rebuilding them on the official build servers), but the tags will still be available to users who want them. (See https://github.com/docker-library/official-images#library-definition-files for more detail on this.)


It doesn't affect your usage of su-exec, but you probably want to be aware of ncopa/su-exec#26 (and if you end up needing more su-exec in the future, I would strongly suggest finding alternatives).


+commandRun="run"
+commandVersion="version"
+commandCheck="check"
+commandPlugin="check-plugin"
+commandHelp="help"
+commandValidate="validate"
+commandAudit="audit"
+
+# this if will check if the first argument is a flag
+# but only works if all arguments require a hyphenated flag
+# -v; -SL; -f arg; Also check if the first argument is any
+# of the KrakenD commands
+if [ "$#" -eq 0 ] || [ "${1#-}" != "$1" ] ||
+   [ "$1" = "${commandRun}" ] || 
+   [ "$1" = "${commandVersion}" ] || 
+   [ "$1" = "${commandCheck}" ] || 
+   [ "$1" = "${commandPlugin}" ] ||
+   [ "$1" = "${commandValidate}" ] ||
+   [ "$1" = "${commandAudit}" ] ||
+   [ "$1" = "${commandHelp}" ]; then
+    set -- krakend "$@"
+fi

Just as a suggestion, this might be easier to maintain/understand as a case statement, something like this:

#!/bin/sh
set -eu

# this if will check if the first argument is nonexistent or a flag
# but only works if all arguments require a hyphenated flag
# -v; -SL; -f arg; Also check if the first argument is any
# of the KrakenD commands
case "${1:-}" in
	''       | \
	-*       | \
	run      | \
	version  | \
	check    | \
	plugin   | \
	validate | \
	audit    | \
	help     )
		set -- krakend "$@"
		;;
esac

# ...

+CMD [ "krakend", "run", "-c", "/etc/krakend/krakend.json" ]

When I read this line in the proposed docs:

The configuration files are taken from the current directory ($PWD). Therefore, all examples expect to find at least the file krakend.json.

I get the sense that if I supply --workdir/-w to my container, that it will read krakend.json from the new directory, not from /etc/krakend anymore, but because this default command uses a full path, that's not actually true (and if I keep reading, it's clear that $PWD there was referring to the host, not the container).

I'd suggest either updating the documentation to clarify this, or perhaps better (and the solution I'd suggest) is removing the full path from CMD (and perhaps from all those examples, if you are willing to commit to /etc/krakend being the WORKDIR long-term, which doesn't seem like a stretch given your other replies here 😄):

CMD [ "krakend", "run", "-c", "krakend.json" ]

(this then also makes it easier to reproduce for users who want to run the original command but with some extra flags/arguments)

@taik0
Copy link
Contributor Author

taik0 commented Mar 17, 2025

Let me address those recommendations before merging since I find them pretty good improvements. Thank you!

….2 since it has vulnerabilities already fixed in 2.9.3.

Signed-off-by: Daniel Ortiz <[email protected]>
Copy link

Diff for c4687b4:
diff --git a/_bashbrew-arches b/_bashbrew-arches
index 8b13789..e85a97f 100644
--- a/_bashbrew-arches
+++ b/_bashbrew-arches
@@ -1 +1,2 @@
-
+amd64
+arm64v8
diff --git a/_bashbrew-cat b/_bashbrew-cat
index bdfae4a..90716be 100644
--- a/_bashbrew-cat
+++ b/_bashbrew-cat
@@ -1 +1,7 @@
-Maintainers: New Image! :D (@docker-library-bot)
+Maintainers: Daniel Ortiz <[email protected]> (@taik0), Daniel López <[email protected]> (@kpacha)
+GitRepo: https://github.com/krakendio/docker-library.git
+
+Tags: 2.9.3, 2.9, 2, latest
+Architectures: amd64, arm64v8
+GitCommit: bac1c35e5818831f0669176ab140e34705185b55
+Directory: 2.9.3
diff --git a/_bashbrew-list b/_bashbrew-list
index e69de29..0635bb1 100644
--- a/_bashbrew-list
+++ b/_bashbrew-list
@@ -0,0 +1,4 @@
+krakend:2
+krakend:2.9
+krakend:2.9.3
+krakend:latest
diff --git a/_bashbrew-list-build-order b/_bashbrew-list-build-order
index e69de29..c11b934 100644
--- a/_bashbrew-list-build-order
+++ b/_bashbrew-list-build-order
@@ -0,0 +1 @@
+krakend:latest
diff --git a/krakend_latest/Dockerfile b/krakend_latest/Dockerfile
new file mode 100644
index 0000000..0aeb3c1
--- /dev/null
+++ b/krakend_latest/Dockerfile
@@ -0,0 +1,47 @@
+#
+# NOTE: THIS DOCKERFILE IS GENERATED VIA "apply-templates.sh"
+#
+# PLEASE DO NOT EDIT IT DIRECTLY.
+#
+FROM alpine:3.21
+
+LABEL org.opencontainers.image.authors="[email protected]"
+
+RUN set -eux; \
+	apk add --no-cache --virtual .run-deps ca-certificates su-exec tzdata; \
+	adduser -u 1000 -S -D -H krakend;
+
+RUN set -eux; \
+    apk add --no-cache --virtual .build-deps gnupg; \
+    arch="$(apk --print-arch)"; \
+	case "$arch" in \
+		'x86_64') \
+			export GOARCH='amd64' GOOS='linux'; \
+			export KRAKEND_DOWNLOAD_SHA512=1a6adae8ad6c3121e55424b0a7dd1cd0cc632848a3f3bb108dbfcb9e2da4ffc4160c3ca4179d32de410cd9a92afdd6524456b0a363e41e7e420dfc040dadf40d; \
+			;; \
+		'aarch64') \
+			export GOARCH='arm64' GOOS='linux'; \
+			export KRAKEND_DOWNLOAD_SHA512=02fb8b6f9bdade76bc51fbf7f9ef08094027a92e51c6f72298b1e96ed1744fe24c6746a4a4854427d5a51da5a6a66a8aa709ede9993cea20f3661ddc05dcda95; \
+			;; \
+		*) echo >&2 "error: unsupported architecture '$TARGETARCH' (likely packaging update needed)"; exit 1 ;; \
+	esac; \
+    wget -O krakend.tar.gz "https://github.com/krakendio/krakend-ce/releases/download/v2.9.3/krakend_2.9.3_${GOARCH}_alpine.tar.gz"; \
+    wget -O krakend.tar.gz.asc "https://github.com/krakendio/krakend-ce/releases/download/v2.9.3/krakend_2.9.3_${GOARCH}_alpine.tar.gz.asc"; \
+    export GNUPGHOME="$(mktemp -d)"; \
+    gpg --batch --keyserver keyserver.ubuntu.com --recv-keys 5B270F2E01E375FD9D5635E25DE6FD698AD6FDD2; \
+    gpg --batch --verify krakend.tar.gz.asc krakend.tar.gz; \
+    gpgconf --kill all; \
+    rm -rf "$GNUPGHOME"; \
+    echo "$KRAKEND_DOWNLOAD_SHA512 *krakend.tar.gz" | sha512sum -c; \
+    tar -xzf krakend.tar.gz -C / --strip-components 1; \
+    rm -f krakend.tar.gz krakend.tar.gz.asc; \
+    apk del --no-network .build-deps; \
+    echo '{ "version": 3 }' > /etc/krakend/krakend.json
+
+WORKDIR /etc/krakend
+
+COPY docker-entrypoint.sh /usr/local/bin/
+ENTRYPOINT [ "docker-entrypoint.sh" ]
+
+EXPOSE 8080 8090
+CMD [ "krakend", "run", "-c", "krakend.json" ]
diff --git a/krakend_latest/docker-entrypoint.sh b/krakend_latest/docker-entrypoint.sh
new file mode 100755
index 0000000..4116fc1
--- /dev/null
+++ b/krakend_latest/docker-entrypoint.sh
@@ -0,0 +1,37 @@
+#!/bin/sh
+set -eu
+
+# this if will check if the first argument is nonexistent or a flag
+# but only works if all arguments require a hyphenated flag
+# -v; -SL; -f arg; Also check if the first argument is any
+# of the KrakenD commands
+case "${1:-}" in
+	''            | \
+	-*            | \
+	run           | \
+	version       | \
+	check         | \
+	check-plugin  | \
+	test-plugin   | \
+	validate      | \
+	audit         | \
+	help          )
+		set -- krakend "$@"
+		;;
+esac
+
+# check for the expected command
+if [ "$1" = 'krakend' ]; then
+    # krakend user has uid 1000
+    # https://kubernetes.io/docs/tasks/configure-pod-container/security-context/
+    # runAsUser: 1000
+    if [ "$(id -u)" = 0 ]; then
+        # use su-exec to drop to a non-root user
+        exec su-exec krakend "$@"
+    else
+        exec "$@"
+    fi
+fi
+
+# else default to run whatever the user wanted like "bash" or "sh"
+exec "$@"

@taik0
Copy link
Contributor Author

taik0 commented Mar 26, 2025

Hi,

I've changed the entrypoint to use the case you recommended (really helps adding new commands).
I also changed the CMD to remove the /etc/krakend from the config file.

Regarding having 2.9.2, I removed it. We always recommend updating to the latest version (we never remove functionality), and since the latest version (2.9.3) has security fixes, it wouldn't be right to publish an insecure version.

In future updates, we will try to address the change of su-exec for an alternative.

Thank you!

@tianon tianon merged commit 7f52e3f into docker-library:master Mar 26, 2025
6 checks passed
@alombarte alombarte deleted the krakend branch March 28, 2025 08:46
gquintard pushed a commit to gquintard/official-images that referenced this pull request Jun 11, 2025
tglman pushed a commit to tglman/official-images that referenced this pull request Aug 11, 2025
gquintard pushed a commit to gquintard/official-images that referenced this pull request Aug 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants