[GitHub] [flink-docker] azagrebin commented on a change in pull request #17: [FLINK-17367] Add configurable download URL

[GitBox <gi...@apache.org>]

azagrebin commented on a change in pull request #17:
URL: https://github.com/apache/flink-docker/pull/17#discussion_r415330400



##########
File path: add-version.sh
##########
@@ -95,19 +99,46 @@ fi
 
 mkdir "$flink_release"
 
+function generate() {
+    dir=$1
+    binary_download_url=$2
+    asc_download_url=$3
+    gpg_key=$4
+    skip_gpg=$5
+    source_variant=$6
+
+    mkdir "$dir"
+    cp docker-entrypoint.sh "$dir/docker-entrypoint.sh"
+
+    # '&' has special semantics in sed replacement patterns
+    escaped_binary_download_url=$(echo "$binary_download_url" | sed 's/&/\\\&/')
+
+    sed \
+        -e "s,%%BINARY_DOWNLOAD_URL%%,${escaped_binary_download_url}," \
+        -e "s,%%ASC_DOWNLOAD_URL%%,$asc_download_url," \
+        -e "s/%%GPG_KEY%%/$gpg_key/" \
+        -e "s/%%SKIP_GPG%%/${skip_gpg}/" \
+        "Dockerfile-$source_variant.template" > "$dir/Dockerfile"
+}
+
 echo -n >&2 "Generating Dockerfiles..."
 for source_variant in "${source_variants[@]}"; do
     for scala_version in "${scala_versions[@]}"; do
         dir="$flink_release/scala_${scala_version}-${source_variant}"
 
-        mkdir "$dir"
-        cp docker-entrypoint.sh "$dir/docker-entrypoint.sh"
+        flink_url_file_path=flink/flink-${flink_version}/flink-${flink_version}-bin-scala_${scala_version}.tgz
+
+        flink_tgz_url="https://www.apache.org/dyn/closer.cgi?action=download&filename=${flink_url_file_path}"
+        # Not all mirrors have the .asc files
+        flink_asc_url=https://www.apache.org/dist/${flink_url_file_path}.asc
 
-        sed \
-            -e "s/%%FLINK_VERSION%%/$flink_version/" \
-            -e "s/%%SCALA_VERSION%%/$scala_version/" \
-            -e "s/%%GPG_KEY%%/$gpg_key/" \
-            "Dockerfile-$source_variant.template" > "$dir/Dockerfile"
+        generate "${dir}" "${flink_tgz_url}" "${flink_asc_url}" ${gpg_key} false ${source_variant}
     done
+
+    if [ -n "${binary_download_url}" ]; then
+        dir="$flink_release/custom-${source_variant}"
+
+        generate "${dir}" "${binary_download_url}" "" "" true ${source_variant}

Review comment:
       a bit weird that the script has to always generate both `flink_version` and `binary_download_url`
   and not either `flink_version` or `binary_download_url`.
   if we need `binary_download_url`, do we always want to generate a normal `flink_version` as well?

##########
File path: Dockerfile-debian.template
##########
@@ -55,27 +56,24 @@ RUN groupadd --system --gid=9999 flink && \
     useradd --system --home-dir $FLINK_HOME --uid=9999 --gid=flink flink
 WORKDIR $FLINK_HOME
 
-ENV FLINK_URL_FILE_PATH=flink/flink-${FLINK_VERSION}/flink-${FLINK_VERSION}-bin-scala_${SCALA_VERSION}.tgz
-# Not all mirrors have the .asc files
-ENV FLINK_TGZ_URL=https://www.apache.org/dyn/closer.cgi?action=download&filename=${FLINK_URL_FILE_PATH} \
-    FLINK_ASC_URL=https://www.apache.org/dist/${FLINK_URL_FILE_PATH}.asc
-
 # Install Flink
 RUN set -ex; \
   wget -nv -O flink.tgz "$FLINK_TGZ_URL"; \
-  wget -nv -O flink.tgz.asc "$FLINK_ASC_URL"; \
   \
-  export GNUPGHOME="$(mktemp -d)"; \
-  for server in ha.pool.sks-keyservers.net $(shuf -e \
-                          hkp://p80.pool.sks-keyservers.net:80 \
-                          keyserver.ubuntu.com \
-                          hkp://keyserver.ubuntu.com:80 \
-                          pgp.mit.edu) ; do \
-      gpg --batch --keyserver "$server" --recv-keys "$GPG_KEY" && break || : ; \
-  done && \
-  gpg --batch --verify flink.tgz.asc flink.tgz; \
-  gpgconf --kill all; \
-  rm -rf "$GNUPGHOME" flink.tgz.asc; \
+  if [ "$SKIP_GPG" = "false" ]; then \

Review comment:
       nit: maybe a matter of personal taste to not use negated vars, I would consider a `CHECK_GPG = true` flag.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org