Skip to content

Commit 1cf1af6

Browse files
potiukpotiuk
authored
Do not override in_container scripts when building the image (apache#10442)
After apache#10368, we've changed the way we build the images on CI. We are overriding the ci scripts that we use to build the image with the scripts taken from master to not give roque PR authors the possibiility to run something with the write credentials. We should not override the in_container scripts, however because they become part of the image, so we should use those that came with the PR. That's why we have to move the "in_container" scripts out of the "ci" folder and only override the "ci" folder with the one from master. We've made sure that those scripts in ci are self-contained and they do not need reach outside of that folder. Also the static checks are done with local files mounted on CI because we want to check all the files - not only those that are embedded in the container.
1 parent fdd68ec commit 1cf1af6

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

51 files changed

+77
-67
lines changed

‎.dockerignore‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -36,11 +36,7 @@
3636
!metastore_browser
3737

3838
# Add those folders to the context so that they are available in the CI container
39-
!scripts/ci/in_container
40-
!scripts/ci/pre_commit
41-
!scripts/prod
42-
!scripts/perf
43-
!scripts/tools
39+
!scripts/in_container
4440

4541
# Add backport packages to the context
4642
!backport_packages

‎.github/workflows/build-images-workflow-run.yml‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -201,10 +201,13 @@ jobs:
201201
uses: actions/setup-python@v2
202202
with:
203203
python-version: 3.6
204-
- name: "Override 'scripts' with the ${{ github.ref }} version so that the PR cannot override it."
204+
- name: "Override 'scripts/ci' with the ${{ github.ref }} version so that the PR cannot override it."
205+
# We should not override those scripts which become part of the image as they will not be
206+
# changed in the image built - we should only override those that are executed to build
207+
# the image.
205208
run: |
206-
rm -rf "scripts"
207-
mv "main-airflow/scripts" .
209+
rm -rf "scripts/ci"
210+
mv "main-airflow/scripts/ci" "scripts"
208211
- name: "Free space"
209212
run: ./scripts/ci/tools/ci_free_space_on_ci.sh
210213
- name: "Build CI images ${{ matrix.python-version }}:${{ github.event.workflow_run.id }}"

‎.github/workflows/ci.yml‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ on:
2727

2828
env:
2929

30-
MOUNT_LOCAL_SOURCES: "false"
30+
MOUNT_LOCAL_SOURCES: "true"
3131
MOUNT_FILES: "true"
3232
FORCE_ANSWER_TO_QUESTIONS: "yes"
3333
FORCE_PULL_IMAGES: "true"
@@ -160,7 +160,10 @@ jobs:
160160
needs: [ci-images]
161161
env:
162162
SKIP: "pylint"
163+
MOUNT_LOCAL_SOURCES: "true"
163164
if: github.repository == 'apache/airflow' || github.event_name != 'schedule'
165+
# We want to make sure we have latest sources as only in_container scripts are added
166+
# to the image but we want to static-check all of them
164167
steps:
165168
- name: "Checkout ${{ github.ref }} ( ${{ github.sha }} )"
166169
uses: actions/checkout@v2
@@ -190,6 +193,10 @@ jobs:
190193
runs-on: ubuntu-latest
191194
needs: [ci-images]
192195
if: github.repository == 'apache/airflow' || github.event_name != 'schedule'
196+
env:
197+
# We want to make sure we have latest sources as only in_container scripts are added
198+
# to the image but we want to static-check all of them
199+
MOUNT_LOCAL_SOURCES: "true"
193200
steps:
194201
- name: "Checkout ${{ github.ref }} ( ${{ github.sha }} )"
195202
uses: actions/checkout@v2

‎Dockerfile‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -359,8 +359,8 @@ RUN mkdir -pv "${AIRFLOW_HOME}"; \
359359

360360
COPY --chown=airflow:root --from=airflow-build-image /root/.local "${AIRFLOW_USER_HOME_DIR}/.local"
361361

362-
COPY scripts/prod/entrypoint_prod.sh /entrypoint
363-
COPY scripts/prod/clean-logs.sh /clean-logs
362+
COPY scripts/in_container/prod/entrypoint_prod.sh /entrypoint
363+
COPY scripts/in_container/prod/clean-logs.sh /clean-logs
364364
RUN chmod a+x /entrypoint /clean-logs
365365

366366
# Make /etc/passwd root-group-writeable so that user can be dynamically added by OpenShift

‎Dockerfile.ci‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -278,7 +278,7 @@ COPY airflow/www/static ${AIRFLOW_SOURCES}/airflow/www/static/
278278
# Package JS/css for production
279279
RUN yarn --cwd airflow/www run prod
280280

281-
COPY scripts/ci/in_container/entrypoint_ci.sh /entrypoint
281+
COPY scripts/in_container/entrypoint_ci.sh /entrypoint
282282
RUN chmod a+x /entrypoint
283283

284284
# We can copy everything here. The Context is filtered by dockerignore. This makes sure we are not

‎IMAGES.rst‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -603,7 +603,7 @@ Running the CI image
603603
====================
604604

605605
The entrypoint in the CI image contains all the initialisation needed for tests to be immediately executed.
606-
It is copied from ``scripts/ci/in_container/entrypoint_ci.sh``.
606+
It is copied from ``scripts/in_container/entrypoint_ci.sh``.
607607

608608
The default behaviour is that you are dropped into bash shell. However if RUN_TESTS variable is
609609
set to "true", then tests passed as arguments are executed
@@ -639,7 +639,7 @@ Using the PROD image
639639
====================
640640

641641
The entrypoint in the PROD image contains all the initialisation needed for tests to be immediately executed.
642-
It is copied from ``scripts/ci/in_container/entrypoint_prod.sh``.
642+
It is copied from ``scripts/in_container/entrypoint_prod.sh``.
643643

644644
The PROD image entrypoint works as follows:
645645

‎STATIC_CODE_CHECKS.rst‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -387,11 +387,11 @@ Running Static Code Checks in the Docker Container
387387
If you are already in the Breeze Docker environment (by running the ``./breeze`` command),
388388
you can also run the same static checks via run_scripts:
389389

390-
* Mypy: ``./scripts/ci/in_container/run_mypy.sh airflow tests``
391-
* Pylint: ``./scripts/ci/in_container/run_pylint.sh``
392-
* Flake8: ``./scripts/ci/in_container/run_flake8.sh``
393-
* License check: ``./scripts/ci/in_container/run_check_licence.sh``
394-
* Documentation: ``./scripts/ci/in_container/run_docs_build.sh``
390+
* Mypy: ``./scripts/in_container/run_mypy.sh airflow tests``
391+
* Pylint: ``./scripts/in_container/run_pylint.sh``
392+
* Flake8: ``./scripts/in_container/run_flake8.sh``
393+
* License check: ``./scripts/in_container/run_check_licence.sh``
394+
* Documentation: ``./scripts/in_container/run_docs_build.sh``
395395

396396
Running Static Code Checks for Selected Files
397397
.............................................
@@ -403,13 +403,13 @@ In the Docker container:
403403

404404
.. code-block::
405405
406-
./scripts/ci/in_container/run_pylint.sh ./airflow/example_dags/
406+
./scripts/in_container/run_pylint.sh ./airflow/example_dags/
407407
408408
or
409409

410410
.. code-block::
411411
412-
./scripts/ci/in_container/run_pylint.sh ./airflow/example_dags/test_utils.py
412+
./scripts/in_container/run_pylint.sh ./airflow/example_dags/test_utils.py
413413
414414
On the host:
415415

‎breeze‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2224,7 +2224,7 @@ function run_breeze_command {
22242224
: "${AIRFLOW_TESTING_CONTAINER:?"ERROR! Breeze must be running in order to exec into running container"}"
22252225
set -e
22262226
docker exec -it "${AIRFLOW_TESTING_CONTAINER}" \
2227-
"/opt/airflow/scripts/ci/in_container/entrypoint_exec.sh" "${@}"
2227+
"/opt/airflow/scripts/in_container/entrypoint_exec.sh" "${@}"
22282228
;;
22292229
run_tests)
22302230
export RUN_TESTS="true"

‎docs/executor/kubernetes.rst‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ Kubernetes Executor
2323

2424
The kubernetes executor is introduced in Apache Airflow 1.10.0. The Kubernetes executor will create a new pod for every task instance.
2525

26-
Example kubernetes files are available at ``scripts/ci/in_container/kubernetes/app/{secrets,volumes,postgres}.yaml`` in the source distribution (please note that these examples are not ideal for production environments).
26+
Example kubernetes files are available at ``scripts/in_container/kubernetes/app/{secrets,volumes,postgres}.yaml`` in the source distribution (please note that these examples are not ideal for production environments).
2727
The volumes are optional and depend on your configuration. There are two volumes available:
2828

2929
- **Dags**:

‎scripts/ci/backport_packages/ci_test_backport_packages_import_all_classes.sh‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ function run_test_package_import_all_classes() {
3030
-v "${AIRFLOW_SOURCES}/airflow/version.py:/airflow_sources/airflow/version.py:cached" \
3131
-v "${AIRFLOW_SOURCES}/backport_packages/import_all_provider_classes.py:/import_all_provider_classes.py:cached" \
3232
"${AIRFLOW_CI_IMAGE}" \
33-
"--" "/opt/airflow/scripts/ci/in_container/run_test_package_import_all_classes.sh"
33+
"--" "/opt/airflow/scripts/in_container/run_test_package_import_all_classes.sh"
3434
}
3535

3636
get_environment_for_builds_on_ci

0 commit comments

Comments
 (0)