Update UI and fix some errors

Author: pierrejeambrun

.pre-commit-config.yaml

@@ -1636,6 +1636,7 @@ repos:
           ^airflow-core/src/airflow/api/common/mark_tasks\.py$|
           ^airflow-core/src/airflow/api_fastapi/core_api/datamodels/assets\.py$|
           ^airflow-core/src/airflow/api_fastapi/core_api/datamodels/connections\.py$|
+          ^airflow-core/src/airflow/api_fastapi/core_api/services/public/connections\.py$|
           ^airflow-core/src/airflow/api_fastapi/core_api/datamodels/hitl\.py$|
           ^airflow-core/src/airflow/api_fastapi/core_api/datamodels/variables\.py$|
           ^airflow-core/src/airflow/api_fastapi/core_api/routes/ui/grid\.py$|

airflow-core/src/airflow/api_fastapi/core_api/services/public/connections.py

@@ -17,6 +17,8 @@
 
 from __future__ import annotations
 
+import json
+
 from fastapi import HTTPException, status
 from pydantic import ValidationError
 from sqlalchemy import select
@@ -32,6 +34,7 @@
 from airflow.api_fastapi.core_api.datamodels.connections import ConnectionBody
 from airflow.api_fastapi.core_api.services.public.common import BulkService
 from airflow.models.connection import Connection
+from airflow.sdk.execution_time.secrets_masker import merge
 
 
 def update_orm_from_pydantic(
@@ -56,11 +59,24 @@ def update_orm_from_pydantic(
     if (not update_mask and "password" in pydantic_conn.model_fields_set) or (
         update_mask and "password" in update_mask
     ):
-        orm_conn.set_password(pydantic_conn.password)
+        if pydantic_conn.password is None:
+            orm_conn.set_password(pydantic_conn.password)
+            return
+
+        merged_password = merge(pydantic_conn.password, orm_conn.password, "password")
+        orm_conn.set_password(merged_password)
     if (not update_mask and "extra" in pydantic_conn.model_fields_set) or (
         update_mask and "extra" in update_mask
     ):
-        orm_conn.set_extra(pydantic_conn.extra)
+        if pydantic_conn.extra is None or orm_conn.extra is None:
+            orm_conn.set_extra(pydantic_conn.extra)
+            return
+        try:
+            merged_extra = merge(json.loads(pydantic_conn.extra), json.loads(orm_conn.extra))
+            orm_conn.set_extra(json.dumps(merged_extra))
+        except json.JSONDecodeError:
+            # We can't merge fields in an unstructured `extra`
+            orm_conn.set_extra(pydantic_conn.extra)
 
 
 class BulkConnectionService(BulkService[ConnectionBody]):

airflow-core/src/airflow/ui/public/i18n/locales/en/admin.json

@@ -36,6 +36,7 @@
       "extraFields": "Extra Fields",
       "extraFieldsJson": "Extra Fields JSON",
       "helperText": "Connection type missing? Make sure you have installed the corresponding Airflow Providers Package.",
+      "helperTextForRedactedFields": "Redacted fields ('***') will remain unchanged if not modified.",
       "selectConnectionType": "Select Connection Type",
       "standardFields": "Standard Fields"
     },

airflow-core/src/airflow/ui/src/components/FlexibleForm/FlexibleForm.tsx

@@ -32,12 +32,14 @@ export type FlexibleFormProps = {
   initialParamsDict: { paramsDict: ParamsSpec };
   key?: string;
   setError: (error: boolean) => void;
+  subHeader?: string;
 };
 
 export const FlexibleForm = ({
   flexibleFormDefaultSection,
   initialParamsDict,
   setError,
+  subHeader,
 }: FlexibleFormProps) => {
   const { paramsDict: params, setInitialParamDict, setParamsDict } = useParamStore();
   const processedSections = new Map();
@@ -126,6 +128,11 @@ export const FlexibleForm = ({
 
               <Accordion.ItemContent pt={0}>
                 <Accordion.ItemBody>
+                  {Boolean(subHeader) ? (
+                    <Text color="fg.muted" fontSize="xs" mb={2}>
+                      {subHeader}
+                    </Text>
+                  ) : undefined}
                   <Stack separator={<StackSeparator />}>
                     {Object.entries(params)
                       .filter(

airflow-core/src/airflow/ui/src/pages/Connections/ConnectionForm.tsx

@@ -37,13 +37,15 @@ import type { ConnectionBody } from "./Connections";
 type AddConnectionFormProps = {
   readonly error: unknown;
   readonly initialConnection: ConnectionBody;
+  readonly isEditMode?: boolean;
   readonly isPending: boolean;
   readonly mutateConnection: (requestBody: ConnectionBody) => void;
 };
 
 const ConnectionForm = ({
   error,
   initialConnection,
+  isEditMode = false,
   isPending,
   mutateConnection,
 }: AddConnectionFormProps) => {
@@ -202,6 +204,7 @@ const ConnectionForm = ({
               initialParamsDict={paramsDic}
               key={selectedConnType}
               setError={setFormErrors}
+              subHeader={isEditMode ? translate("connections.form.helperTextForRedactedFields") : undefined}
             />
             <Accordion.Item key="extraJson" value="extraJson">
               <Accordion.ItemTrigger cursor="button">
@@ -220,6 +223,11 @@ const ConnectionForm = ({
                         }}
                       />
                       {Boolean(errors.conf) ? <Field.ErrorText>{errors.conf}</Field.ErrorText> : undefined}
+                      {isEditMode ? (
+                        <Field.HelperText>
+                          {translate("connections.form.helperTextForRedactedFields")}
+                        </Field.HelperText>
+                      ) : undefined}
                     </Field.Root>
                   )}
                 />

airflow-core/src/airflow/ui/src/pages/Connections/EditConnectionButton.tsx

@@ -81,6 +81,7 @@ const EditConnectionButton = ({ connection, disabled }: Props) => {
             <ConnectionForm
               error={error}
               initialConnection={initialConnectionValue}
+              isEditMode
               isPending={isPending}
               mutateConnection={editConnection}
             />

airflow-core/tests/unit/api_fastapi/core_api/routes/public/test_connections.py

@@ -468,6 +468,27 @@ class TestPatchConnection(TestConnectionEndpoint):
                     "schema": None,
                 },
             ),
+            (
+                # Sensitive "***" should be ignored.
+                {
+                    "connection_id": TEST_CONN_ID,
+                    "conn_type": TEST_CONN_TYPE,
+                    "port": 80,
+                    "login": "test_login_patch",
+                    "password": "***",
+                },
+                {
+                    "conn_type": TEST_CONN_TYPE,
+                    "connection_id": TEST_CONN_ID,
+                    "description": TEST_CONN_DESCRIPTION,
+                    "extra": None,
+                    "host": TEST_CONN_HOST,
+                    "login": "test_login_patch",
+                    "password": None,
+                    "port": 80,
+                    "schema": None,
+                },
+            ),
             (
                 {
                     "connection_id": TEST_CONN_ID,

task-sdk/src/airflow/sdk/execution_time/secrets_masker.py

@@ -27,7 +27,7 @@
 from enum import Enum
 from functools import cache, cached_property
 from re import Pattern
-from typing import Any, TextIO, TypeAlias, TypeVar
+from typing import Any, TextIO, TypeAlias, TypeVar, overload
 
 from airflow import settings
 
@@ -116,9 +116,17 @@ def redact(value: Redactable, name: str | None = None, max_depth: int | None = N
     return _secrets_masker().redact(value, name, max_depth)
 
 
+@overload
+def merge(new_value: str, old_value: str, name: str | None = None, max_depth: int | None = None) -> str: ...
+
+
+@overload
+def merge(new_value: dict, old_value: dict, name: str | None = None, max_depth: int | None = None) -> str: ...
+
+
 def merge(
     new_value: Redacted, old_value: Redactable, name: str | None = None, max_depth: int | None = None
-) -> Redactable:
+) -> Redacted:
     """
     Merge a redacted value with its original unredacted counterpart.
 
@@ -313,7 +321,7 @@ def _merge(
         depth: int,
         max_depth: int,
         force_sensitive: bool = False,
-    ) -> Redactable:
+    ) -> Redacted:
         """Merge a redacted item with its original unredacted counterpart."""
         if depth > max_depth:
             if isinstance(new_item, str) and new_item == "***":
@@ -394,7 +402,7 @@ def redact(self, item: Redactable, name: str | None = None, max_depth: int | Non
 
     def merge(
         self, new_item: Redacted, old_item: Redactable, name: str | None = None, max_depth: int | None = None
-    ) -> Redactable:
+    ) -> Redacted:
         """
         Merge a redacted item with its original unredacted counterpart.