potiuk commented on a change in pull request #13726:
URL: https://github.com/apache/airflow/pull/13726#discussion_r559876284



##########
File path: scripts/ci/libraries/_build_images.sh
##########
@@ -394,30 +394,129 @@ function build_images::get_docker_image_names() {
     # File that is touched when the CI image is built for the first time 
locally
     export 
BUILT_CI_IMAGE_FLAG_FILE="${BUILD_CACHE_DIR}/${BRANCH_NAME}/.built_${PYTHON_MAJOR_MINOR_VERSION}"
 
-    # GitHub Registry names must be lowercase :(
-    github_repository_lowercase="$(echo "${GITHUB_REPOSITORY}" | tr 
'[:upper:]' '[:lower:]')"
-    export 
GITHUB_REGISTRY_AIRFLOW_PROD_IMAGE="${GITHUB_REGISTRY}/${github_repository_lowercase}/${AIRFLOW_PROD_BASE_TAG}${GITHUB_REGISTRY_IMAGE_SUFFIX}"
-    export 
GITHUB_REGISTRY_AIRFLOW_PROD_BUILD_IMAGE="${GITHUB_REGISTRY}/${github_repository_lowercase}/${AIRFLOW_PROD_BASE_TAG}${GITHUB_REGISTRY_IMAGE_SUFFIX}-build"
-    export 
GITHUB_REGISTRY_PYTHON_BASE_IMAGE="${GITHUB_REGISTRY}/${github_repository_lowercase}/python${GITHUB_REGISTRY_IMAGE_SUFFIX}:${PYTHON_BASE_IMAGE_VERSION}-slim-buster"
-
-    export 
GITHUB_REGISTRY_AIRFLOW_CI_IMAGE="${GITHUB_REGISTRY}/${github_repository_lowercase}/${AIRFLOW_CI_BASE_TAG}${GITHUB_REGISTRY_IMAGE_SUFFIX}"
-    export 
GITHUB_REGISTRY_PYTHON_BASE_IMAGE="${GITHUB_REGISTRY}/${github_repository_lowercase}/python${GITHUB_REGISTRY_IMAGE_SUFFIX}:${PYTHON_BASE_IMAGE_VERSION}-slim-buster"
+    # This is 1-1 mapping of image names of Apache Airflow stored in DockerHub 
vs. the same images stored
+    # in Github Registries (either Github Container Registry or Github 
Packages)
+    #
+    # We have to apply naming conventions used by the registries and keep 
multiple RUN_ID tags. We use
+    # common suffix ('gcr-v1') to be able to switch to different set of cache 
images if needed
+    # - for example when some images gets broken (might happen with Github 
Actions Registries) or when
+    # the storage capacity per image is reached (though it is apparently 
unlimited)
+    #
+    # Some examples:
+    #
+    # In case of Github Container Registry:
+    #
+    # * Prod Image: "apache/airflow:master-python3.8" ->  
"apache/airflow-master-python3.8-gcr-v1:<RUN_ID>"
+    # * Prod build image: "apache/airflow:master-python3.8-build" ->  
"apache/airflow-master-python3.8-build-gcr-v1:<RUN_ID>"
+    # * CI build image: "apache/airflow:master-python3.8-ci" ->  
"apache/airflow-master-python3.8-ci-gcr-v1:<RUN_ID>"
+    #
+    # The python base image/tag mapping is slightly different (the base images 
are shared by all Prod/Build/CI images)
+    # And python version is part of the tag.
+    #
+    # "apache/airflow:python-3.6 ->  
"apache/airflow-python-gcr-v1:3.6-slim-buster-<RUN_ID>"
+    #
+    # In case of Github Packages image must be part of the repository:
+    #
+    # * Prod Image: "apache/airflow:master-python3.8" ->  
"apache/airflow/master-python3.8-gcr-v1:<RUN_ID>"
+    # * Prod build image: "apache/airflow:master-python3.8-build" ->  
"apache/airflow/master-python3.8-build-gcr-v1:<RUN_ID>"
+    # * CI build image: "apache/airflow:master-python3.8-ci" ->  
"apache/airflow/master-python3.8-ci-gcr-v1:<RUN_ID>"
+    #
+    # The python base image/tag mapping is slightly different (the base images 
are shared by all
+    # Prod/Build/CI images) and python version is part of the tag.
+    #
+    # "apache/airflow:python-3.6 ->  
"apache/airflow/python/gcr-v1:3.6-slim-buster-<RUN_ID>"
+
+
+    local image_name
+    
image_name="${GITHUB_REGISTRY}/$(get_github_container_registry_image_prefix)"
+    local image_separator
+    if [[ ${GITHUB_REGISTRY} == "ghcr.io" ]]; then
+        image_separator="-"
+    elif [[ ${GITHUB_REGISTRY} == "docker.pkg.github.com" ]]; then
+        image_separator="/"
+    else
+        echo
+        echo  "${COLOR_RED}ERROR: Bad value of '${GITHUB_REGISTRY}'. Should be 
either 'ghcr.io' or 'docker.pkg.github.com'!${COLOR_RESET}"
+        echo
+        exit 1
+    fi
+
+    export 
GITHUB_REGISTRY_AIRFLOW_PROD_IMAGE="${image_name}${image_separator}${AIRFLOW_PROD_BASE_TAG}${GITHUB_REGISTRY_IMAGE_SUFFIX}"
+    export 
GITHUB_REGISTRY_AIRFLOW_PROD_BUILD_IMAGE="${image_name}-${AIRFLOW_PROD_BASE_TAG}${image_separator}build${GITHUB_REGISTRY_IMAGE_SUFFIX}"
+    export 
GITHUB_REGISTRY_PYTHON_BASE_IMAGE="${image_name}${image_separator}python${GITHUB_REGISTRY_IMAGE_SUFFIX}:${PYTHON_BASE_IMAGE_VERSION}-slim-buster"
+
+    export 
GITHUB_REGISTRY_AIRFLOW_CI_IMAGE="${image_name}${image_separator}${AIRFLOW_CI_BASE_TAG}${GITHUB_REGISTRY_IMAGE_SUFFIX}"
+    export 
GITHUB_REGISTRY_PYTHON_BASE_IMAGE="${image_name}${image_separator}python${GITHUB_REGISTRY_IMAGE_SUFFIX}:${PYTHON_BASE_IMAGE_VERSION}-slim-buster"
 }
 
-# If GitHub Registry is used, login to the registry using GITHUB_USERNAME and 
GITHUB_TOKEN
-function build_image::login_to_github_registry_if_needed() {
+# If GitHub Registry is used, login to the registry using GITHUB_USERNAME and
+# either GITHUB_TOKEN or CONTAINER_REGISTRY_TOKEN depending on the registry.
+# In case Personal Access token is not set, skip logging in
+function 
build_image::login_to_github_container_registry_if_github_registry_enabled() {
     if [[ ${USE_GITHUB_REGISTRY} == "true" ]]; then
-        if [[ -n ${GITHUB_TOKEN=} ]]; then
-            start_end::group_start "Login to GitHub registry"
-            echo "${GITHUB_TOKEN}" | docker login \
+        start_end::group_start "Determine Github Registry token used"
+        local token=""
+        if [[ "${GITHUB_REGISTRY}" == "ghcr.io" ]]; then
+            # For now ghcr.io can only authenticate using Personal Access 
Token with package access scope.
+            # There are plans to implement GITHUB_TOKEN authentication but 
this is not implemented yet
+            token="${CONTAINER_REGISTRY_TOKEN=}"
+            echo
+            echo "Using CONTAINER_REGISTRY_TOKEN!"
+            echo
+        elif [[ "${GITHUB_REGISTRY}" == "docker.pkg.github.com" ]]; then
+            token="${GITHUB_TOKEN}"
+            echo
+            echo "Using GITHUB_TOKEN!"
+            echo
+        else
+            echo
+            echo  "${COLOR_RED}ERROR: Bad value of '${GITHUB_REGISTRY}'. 
Should be either 'ghcr.io' or 'docker.pkg.github.com'!${COLOR_RESET}"
+            echo
+            exit 1
+        fi
+        if [[ "${token}" == "" ]] ; then

Review comment:
       Surely can do. I sometimes prefer explicit comparision for people who 
are less familiar with some bash-isms . But yeah, why not.  It sometimes good 
to change your habits. I corrected it everywhere where I used != "" or == "" 
(there were not so many places overall.




----------------------------------------------------------------
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:
[email protected]


Reply via email to