Vamsi-klu commented on code in PR #61936:
URL: https://github.com/apache/airflow/pull/61936#discussion_r2810482924
##########
providers/cncf/kubernetes/src/airflow/providers/cncf/kubernetes/hooks/kubernetes.py:
##########
@@ -56,6 +60,11 @@
from kubernetes.client.models import CoreV1Event, CoreV1EventList, V1Job,
V1Pod
LOADING_KUBE_CONFIG_FILE_RESOURCE = "Loading Kubernetes configuration file
kube_config from {}..."
+_AWS_EXEC_AUTH_VERSION_CHECK_MODE_FIELD =
"exec_auth_aws_cli_version_check_mode"
+_AWS_EXEC_AUTH_VERSION_CHECK_MODES = {"warn", "fail", "ignore"}
+_AWS_EXEC_AUTH_AWS_BINARY_NAMES = {"aws", "aws.exe", "aws2", "aws2.exe"}
+_AWS_EXEC_AUTH_FIXED_BOTOCORE_VERSION = (1, 40, 2)
+_BOTOCORE_VERSION_PATTERN =
re.compile(r"botocore/(?P<version>\d+(?:\.\d+){1,2})")
Review Comment:
Thanks for the feedback @jscheffl, that's a fair point about keeping the K8s
provider cloud agnostic.
The reason this was placed in KubernetesHook is that the vulnerability
affects any kubeconfig using aws eks get-token exec auth, not just users going
through EksHook or EksPodOperator. Many users configure a generic Kubernetes
connection with a kubeconfig file that happens to use AWS exec auth and they
never interact with the AWS provider at all.
That said, I agree AWS specific constants and detection logic shouldn't live
here. How about this approach:
In the K8s provider, add a minimal, generic exec auth validation hook point
in KubernetesHook.get_conn() (for example a discoverable entry point or
registry pattern like _validate_exec_auth(kubeconfig, context)). No
AWS/GCP/Azure specific code, just a generic extension mechanism.
In the AWS provider, register a validator that handles the AWS specific
detection (aws eks get-token binary matching, botocore version parsing,
subprocess call) and the connection extra field
(exec_auth_aws_cli_version_check_mode). All AWS constants, helpers, tests, and
docs move there.
This way the K8s hook stays cloud agnostic while still protecting users who
use generic KubernetesHook with an EKS kubeconfig. It also makes the pattern
extensible so any other provider could register their own exec auth validators
in the future.
Would this approach work for you, or would you prefer everything moved
entirely into the AWS provider package?
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
