[AIRFLOW-3918] Add ssh private-key support to git-sync for KubernetesExecutor #4777

dmateusp · 2019-02-25T22:26:16Z

Make sure you have checked all steps below.

Jira

My PR addresses the following Airflow Jira issues and references them in the PR title. For example, "[AIRFLOW-XXX] My Airflow PR"

https://issues.apache.org/jira/browse/AIRFLOW-XXX

In case you are fixing a typo in the documentation you can prepend your commit with [AIRFLOW-XXX], code changes always need a Jira issue.

https://issues.apache.org/jira/browse/AIRFLOW-3918

Description

Here are some details about my PR, including screenshots of any UI changes:

This PR adds support for Git Sync authentication through SSH key (e.g. a GitHub deployment read-only key)

Tests

My PR adds the following unit tests OR does not need testing for this extremely good reason:

test_worker_configuration_auth_both_ssh_and_user: tests that configuration validation throws an exception when both user authentication and SSH authentication are configured
test_init_environment_using_git_sync_ssh: tests that the environment is correctly set up for the init pod when using SSH auth
test_make_pod_git_sync_ssh: tests that the make_pod function correctly generates the config for the pod, covers security_context test

Additionally tested at HBC on our cluster

Commits

My commits all reference Jira issues in their subject lines, and I have squashed multiple commits if they address the same issue. In addition, my commits follow the guidelines from "How to write a good git commit message":

Subject is separated from body by a blank line

Subject is limited to 50 characters (not including Jira issue reference)

Subject does not end with a period

Subject uses the imperative mood ("add", not "adding")

Body wraps at 72 characters

Body explains "what" and "why", not "how"

Documentation

In case of new functionality, my PR adds documentation that describes how to use it.

When adding new operators/hooks/sensors, the autoclass documentation generation needs to be added.

All the public functions and the classes in the PR contain docstrings that explain what it does

Code Quality

Passes flake8

template Change default airflow config to have keys for SSH authentication (mutually exclusive of user authentication) Update git-sync version to the latest, current version did not support SSH authentication environment variables

Tests that the resulting configs are valid and that security_context is correctly added when using make_pod

Security context was required to read the mounted SSH key for git-sync SSH authentication

airflow/config_templates/default_airflow.cfg

airflow/contrib/kubernetes/worker_configuration.py

codecov-io · 2019-02-26T10:08:55Z

Codecov Report

Merging #4777 into master will decrease coverage by 0.06%.
The diff coverage is 95.45%.

@@            Coverage Diff             @@
##           master    #4777      +/-   ##
==========================================
- Coverage   74.44%   74.38%   -0.07%     
==========================================
  Files         450      450              
  Lines       28974    29021      +47     
==========================================
+ Hits        21571    21587      +16     
- Misses       7403     7434      +31

Impacted Files	Coverage Δ
.../kubernetes_request_factory/pod_request_factory.py	`100% <100%> (ø)`	⬆️
airflow/contrib/kubernetes/pod.py	`100% <100%> (ø)`	⬆️
airflow/contrib/kubernetes/worker_configuration.py	`92.85% <100%> (+1.45%)`	⬆️
airflow/contrib/executors/kubernetes_executor.py	`65.01% <100%> (+1.06%)`	⬆️
...etes_request_factory/kubernetes_request_factory.py	`72.63% <66.66%> (-0.2%)`	⬇️
airflow/www/security.py	`79.65% <0%> (-13.28%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bb91246...bd2f503. Read the comment docs.

codecov-io · 2019-02-26T10:08:55Z

Codecov Report

Merging #4777 into master will increase coverage by 0.03%.
The diff coverage is 97.22%.

@@            Coverage Diff             @@
##           master    #4777      +/-   ##
==========================================
+ Coverage   74.44%   74.47%   +0.03%     
==========================================
  Files         450      450              
  Lines       28970    28994      +24     
==========================================
+ Hits        21567    21594      +27     
+ Misses       7403     7400       -3

Impacted Files	Coverage Δ
.../kubernetes_request_factory/pod_request_factory.py	`100% <100%> (ø)`	⬆️
airflow/contrib/kubernetes/pod.py	`100% <100%> (ø)`	⬆️
airflow/contrib/kubernetes/worker_configuration.py	`93.45% <100%> (+2.06%)`	⬆️
airflow/contrib/executors/kubernetes_executor.py	`65.12% <100%> (+1.17%)`	⬆️
...etes_request_factory/kubernetes_request_factory.py	`72.63% <66.66%> (-0.2%)`	⬇️
airflow/operators/python_operator.py	`95.83% <0%> (ø)`	⬆️
...irflow/contrib/example_dags/example_gcp_spanner.py	`0% <0%> (ø)`	⬆️
airflow/contrib/utils/gcp_field_validator.py	`91.52% <0%> (ø)`	⬆️
airflow/ti_deps/dep_context.py	`100% <0%> (ø)`	⬆️
... and 8 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0044260...9cf874f. Read the comment docs.

Hardcode Kubernetes secret key name and known hosts configmap key name Add example of configmap + Kubernetes secret snippet in config template

airflow/contrib/kubernetes/worker_configuration.py

ashb

Few minor changes - some of the "suggestions" need more than just the one line requested, but github doesn't allow multi line suggestions.

Looking good. Please ping me again once you've done the changes (in Slack is better, my inbox is a mess right now)

tests/contrib/executors/test_kubernetes_executor.py

airflow/contrib/kubernetes/worker_configuration.py

Move hardcoded Class instance attributes to Class properties Use octal to represent file permissions Move known hosts configuration to allow usage with username/password Make ConfigurationException test more specific Remove print and extra parenthesis Rename init_volumes_and_mounts to _get_volumes_and_mounts for clarity Add tests for username/password and known_hosts configuration

ashb · 2019-03-01T14:59:56Z

I'll mark this for 10.3 - and we'll attempt to cherry-pick it in to the release branch.

…Executor (#4777) Add configuration for git SSH auth and update git-sync version in template (mutually exclusive of user authentication). Update git-sync version to the latest, current version did not support SSH authentication environment variables Security context was required to read the mounted SSH key for git-sync SSH authentication Add example of configmap + Kubernetes secret snippet in config template

…Executor (apache#4777) Add configuration for git SSH auth and update git-sync version in template (mutually exclusive of user authentication). Update git-sync version to the latest, current version did not support SSH authentication environment variables Security context was required to read the mounted SSH key for git-sync SSH authentication Add example of configmap + Kubernetes secret snippet in config template

dmateusp added 4 commits February 25, 2019 22:17

Add tests for git-sync SSH authentication

ee2aa22

Tests that the resulting configs are valid and that security_context is correctly added when using make_pod

Add security_context support to make_pod

4961523

Security context was required to read the mounted SSH key for git-sync SSH authentication

Parse SSH git-sync authentication keys

de1ab65

dmateusp mentioned this pull request Feb 25, 2019

git-sync on airflow does not support SSH authentication GoogleCloudPlatform/airflow-operator#18

Open

ashb reviewed Feb 26, 2019

View reviewed changes

airflow/config_templates/default_airflow.cfg Outdated Show resolved Hide resolved

ashb reviewed Feb 26, 2019

View reviewed changes

airflow/contrib/kubernetes/worker_configuration.py Outdated Show resolved Hide resolved

ashb reviewed Feb 26, 2019

View reviewed changes

airflow/contrib/kubernetes/worker_configuration.py Show resolved Hide resolved

dmateusp force-pushed the AIRFLOW-3918_add_git_sync_ssh_auth branch from 0009d23 to 1e96986 Compare February 26, 2019 12:32

dmateusp added 2 commits February 26, 2019 18:48

Reduce git SSH authentication config parameters to 2

fbdb013

Hardcode Kubernetes secret key name and known hosts configmap key name Add example of configmap + Kubernetes secret snippet in config template

Rename git_ssh_secret_name to git_ssh_key_secret_name

d9de91a

dmateusp force-pushed the AIRFLOW-3918_add_git_sync_ssh_auth branch from 1e96986 to d9de91a Compare February 26, 2019 22:08

ashb reviewed Mar 1, 2019

View reviewed changes

airflow/contrib/kubernetes/worker_configuration.py Outdated Show resolved Hide resolved

ashb requested changes Mar 1, 2019

View reviewed changes

ashb changed the title ~~Airflow 3918 add git sync ssh auth~~ [AIRFLOW-3918] Add ssh private-key support to git-sync for KubernetesExecutor Mar 1, 2019

ashb reviewed Mar 1, 2019

View reviewed changes

airflow/contrib/kubernetes/worker_configuration.py Outdated Show resolved Hide resolved

ashb approved these changes Mar 1, 2019

View reviewed changes

ashb merged commit f4253a2 into apache:master Mar 1, 2019

dmateusp deleted the AIRFLOW-3918_add_git_sync_ssh_auth branch March 2, 2019 08:48

pgagnon mentioned this pull request Mar 5, 2019

[AIRFLOW-3274] Add run_as_user and fs_group options for Kubernetes #4648

Merged

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[AIRFLOW-3918] Add ssh private-key support to git-sync for KubernetesExecutor #4777

[AIRFLOW-3918] Add ssh private-key support to git-sync for KubernetesExecutor #4777

dmateusp commented Feb 25, 2019

codecov-io commented Feb 26, 2019

codecov-io commented Feb 26, 2019 •

edited

Loading

ashb left a comment

ashb commented Mar 1, 2019

[AIRFLOW-3918] Add ssh private-key support to git-sync for KubernetesExecutor #4777

[AIRFLOW-3918] Add ssh private-key support to git-sync for KubernetesExecutor #4777

Conversation

dmateusp commented Feb 25, 2019

Jira

Description

Tests

Commits

Documentation

Code Quality

codecov-io commented Feb 26, 2019

Codecov Report

codecov-io commented Feb 26, 2019 • edited Loading

Codecov Report

ashb left a comment

Choose a reason for hiding this comment

ashb commented Mar 1, 2019

codecov-io commented Feb 26, 2019 •

edited

Loading