[pull] master from DataDog:master

harbor/tests/conftest.py

@@ -50,8 +50,12 @@ def dd_environment(e2e_instance):
         CheckDockerLogs(compose_file, expected_log, wait=3, service='core'),
         WaitFor(create_simple_user, wait=5),
     ]
+    e2e_metadata = {}
+    if os.environ.get('HARBOR_USE_SSL'):
+        cert_dir = os.path.join(HERE, 'compose', 'common', 'cert')
+        e2e_metadata['docker_volumes'] = ['{}:/home/harbor/tests/compose/common/cert'.format(cert_dir)]
     with docker_run(compose_file, conditions=conditions, attempts=5, waith_for_health=True):
-        yield e2e_instance
+        yield e2e_instance, e2e_metadata
 
 
 def create_simple_user():

postgres/changelog.d/23602.fixed

@@ -0,0 +1 @@
+Fix connection leak and improve error handling during Postgres database connectivity diagnostics

postgres/datadog_checks/postgres/diagnose.py

@@ -198,12 +198,9 @@ def _open_probe_connection(self, dbname):
         """
         host_desc = self._host_desc()
         username = self._check._config.username
-        conn = None
         try:
             conn = self._check._new_connection(dbname)
-        except psycopg.Error as e:
-            if conn is not None:
-                _safe_close(conn)
+        except Exception as e:
             code = DatabaseConfigurationError.connection_failure
             self._fail(
                 code,
@@ -226,12 +223,31 @@ def _open_probe_connection(self, dbname):
     def _diagnose_version(self, conn):
         code = DatabaseConfigurationError.postgres_version_unsupported
         try:
-            raw_version = _fetchone(conn, "SHOW SERVER_VERSION")[0]
+            with conn.cursor() as cursor:
+                cursor.execute("SHOW SERVER_VERSION")
+                row = cursor.fetchone()
+        except psycopg.Error as e:
+            self._fail(
+                code,
+                diagnosis="Unable to determine Postgres version: {}".format(e),
+                category=CATEGORY_POSTGRES,
+                rawerror=str(e),
+            )
+            return
+        if not row or row[0] is None:
+            self._fail(
+                code,
+                diagnosis="Unable to determine Postgres version: SHOW SERVER_VERSION returned no rows.",
+                category=CATEGORY_POSTGRES,
+            )
+            return
+        raw_version = row[0]
+        try:
             version = VersionUtils.parse_version(raw_version)
         except Exception as e:
             self._fail(
                 code,
-                diagnosis="Unable to determine Postgres version: {}".format(e),
+                diagnosis="Unable to parse Postgres version {!r}: {}".format(raw_version, e),
                 category=CATEGORY_POSTGRES,
                 rawerror=str(e),
             )
@@ -367,17 +383,37 @@ def _diagnose_pg_stat_statements_max(self, conn):
     def _diagnose_pg_monitor_role(self, conn):
         # pg_monitor only exists on PG >= 10.
         code = DatabaseConfigurationError.missing_pg_monitor_role
-        row = _fetchone(
-            conn,
-            "SELECT 1 FROM pg_roles WHERE rolname = 'pg_monitor'",
-        )
+        try:
+            with conn.cursor() as cursor:
+                cursor.execute("SELECT 1 FROM pg_roles WHERE rolname = 'pg_monitor'")
+                row = cursor.fetchone()
+        except psycopg.Error as e:
+            self._fail(
+                code,
+                diagnosis="Unable to check pg_monitor role membership: {}".format(e),
+                category=CATEGORY_POSTGRES,
+                description=DIAGNOSTIC_METADATA[code]["description"],
+                remediation=build_remediation(code),
+                rawerror=str(e),
+            )
+            return
         if row is None:
             # PG < 10 has no pg_monitor role — the version diagnostic handles unsupported versions.
             return
-        has_role = _fetchone(
-            conn,
-            "SELECT pg_has_role(current_user, 'pg_monitor', 'MEMBER')",
-        )
+        try:
+            with conn.cursor() as cursor:
+                cursor.execute("SELECT pg_has_role(current_user, 'pg_monitor', 'MEMBER')")
+                has_role = cursor.fetchone()
+        except psycopg.Error as e:
+            self._fail(
+                code,
+                diagnosis="Unable to check pg_monitor role membership: {}".format(e),
+                category=CATEGORY_POSTGRES,
+                description=DIAGNOSTIC_METADATA[code]["description"],
+                remediation=build_remediation(code),
+                rawerror=str(e),
+            )
+            return
         if has_role and has_role[0]:
             self._check.diagnosis.success(
                 name=code.value,

postgres/datadog_checks/postgres/postgres.py

@@ -1018,7 +1018,11 @@ def _new_connection(self, dbname):
             kwargs["token_provider"] = self.db_pool.token_provider
 
         conn = TokenAwareConnection.connect(**kwargs)
-        self.db_pool._configure_connection(conn)
+        try:
+            self.db_pool._configure_connection(conn)
+        except Exception:
+            conn.close()
+            raise
         return conn
 
     def _connect(self):

postgres/tests/test_diagnose.py

@@ -283,6 +283,46 @@ def test_probe_connection_uses_pool_configuration(integration_check, pg_instance
     configure.assert_called_once_with(conn)
 
 
+@pytest.mark.parametrize(
+    'err',
+    [
+        pytest.param(
+            __import__('botocore.exceptions', fromlist=['NoCredentialsError']).NoCredentialsError(),
+            id='aws-no-credentials',
+        ),
+        pytest.param(
+            __import__('azure.core.exceptions', fromlist=['ClientAuthenticationError']).ClientAuthenticationError(
+                message='managed identity unavailable'
+            ),
+            id='azure-auth-error',
+        ),
+    ],
+)
+def test_non_psycopg_connection_error_surfaces_fail_diagnostic(integration_check, pg_instance, err):
+    """Non-psycopg exceptions from AWS/Azure token providers must produce a
+    connection-failure FAIL row, not an unhandled traceback."""
+    check = integration_check(pg_instance)
+    with mock.patch('datadog_checks.postgres.postgres.TokenAwareConnection.connect', side_effect=err):
+        diagnoses = _get_diagnoses(check)
+    rows = _by_name(diagnoses, DatabaseConfigurationError.connection_failure.value)
+    assert len(rows) == 1
+    assert rows[0]['result'] == Diagnosis.DIAGNOSIS_FAIL
+
+
+def test_configure_connection_failure_emits_connection_fail_diagnostic(integration_check, pg_instance):
+    """When _configure_connection raises on the probe connection, diagnose must close the
+    leaked connection and emit a connection-failure FAIL row rather than crashing."""
+    check = integration_check(pg_instance)
+    conn = mock.MagicMock()
+    with mock.patch('datadog_checks.postgres.postgres.TokenAwareConnection.connect', return_value=conn):
+        with mock.patch.object(check.db_pool, '_configure_connection', side_effect=psycopg.Error('SET failed')):
+            diagnoses = _get_diagnoses(check)
+    conn.close.assert_called()
+    rows = _by_name(diagnoses, DatabaseConfigurationError.connection_failure.value)
+    assert len(rows) == 1
+    assert rows[0]['result'] == Diagnosis.DIAGNOSIS_FAIL
+
+
 # -- Server config diagnostics ------------------------------------------------
 
 
@@ -367,6 +407,34 @@ def test_unsupported_postgres_version_fails(integration_check, pg_instance):
     assert '9.5' in entry['diagnosis']
 
 
+def test_version_probe_psycopg_error_surfaces_underlying_message(integration_check, pg_instance):
+    """A transient connection error during the version probe should surface the underlying
+    psycopg error, not 'NoneType' object is not subscriptable from indexing a swallowed None."""
+    check = integration_check(pg_instance)
+    responses = _override(
+        _happy_server_responses(),
+        'SHOW SERVER_VERSION',
+        psycopg.OperationalError('server closed the connection unexpectedly'),
+    )
+    diagnoses = _run(check, responses)
+    entry = _by_name(diagnoses, DatabaseConfigurationError.postgres_version_unsupported.value)[0]
+    assert entry['result'] == Diagnosis.DIAGNOSIS_FAIL
+    assert 'server closed the connection unexpectedly' in entry['diagnosis']
+    assert 'NoneType' not in entry['diagnosis']
+
+
+def test_version_probe_no_rows_fails_clearly(integration_check, pg_instance):
+    """An empty result from SHOW SERVER_VERSION should fail with a clear message rather than
+    crash on None subscripting."""
+    check = integration_check(pg_instance)
+    responses = _override(_happy_server_responses(), 'SHOW SERVER_VERSION', [])
+    diagnoses = _run(check, responses)
+    entry = _by_name(diagnoses, DatabaseConfigurationError.postgres_version_unsupported.value)[0]
+    assert entry['result'] == Diagnosis.DIAGNOSIS_FAIL
+    assert 'NoneType' not in entry['diagnosis']
+    assert 'SHOW SERVER_VERSION' in entry['diagnosis']
+
+
 # -- Privilege diagnostics ----------------------------------------------------
 
 
@@ -379,6 +447,35 @@ def test_missing_pg_monitor_role_fails(integration_check, pg_instance):
     assert 'pg_monitor' in entry['remediation']
 
 
+def test_pg_monitor_role_existence_probe_psycopg_error_surfaces_underlying_message(integration_check, pg_instance):
+    """A transient error on the pg_roles probe must not be mistaken for PG < 10 (silent skip)."""
+    check = integration_check(pg_instance)
+    responses = _override(
+        _happy_server_responses(),
+        "rolname = 'pg_monitor'",
+        psycopg.OperationalError('server closed the connection unexpectedly'),
+    )
+    diagnoses = _run(check, responses)
+    entry = _by_name(diagnoses, DatabaseConfigurationError.missing_pg_monitor_role.value)[0]
+    assert entry['result'] == Diagnosis.DIAGNOSIS_FAIL
+    assert 'server closed the connection unexpectedly' in entry['diagnosis']
+
+
+def test_pg_monitor_has_role_probe_psycopg_error_surfaces_underlying_message(integration_check, pg_instance):
+    """A transient error on the pg_has_role probe must not be misclassified as 'not a member'."""
+    check = integration_check(pg_instance)
+    responses = _override(
+        _happy_server_responses(),
+        'pg_has_role',
+        psycopg.OperationalError('server closed the connection unexpectedly'),
+    )
+    diagnoses = _run(check, responses)
+    entry = _by_name(diagnoses, DatabaseConfigurationError.missing_pg_monitor_role.value)[0]
+    assert entry['result'] == Diagnosis.DIAGNOSIS_FAIL
+    assert 'server closed the connection unexpectedly' in entry['diagnosis']
+    assert 'not a member' not in entry['diagnosis']
+
+
 def test_pg_stat_database_access_fails_on_permission_error(integration_check, pg_instance):
     check = integration_check(pg_instance)
     responses = _override(

postgres/tests/test_unit.py

@@ -325,3 +325,14 @@ def test_run_explain_uses_parameterized_statement(pg_instance, integration_check
     assert '$stmt$' not in query, "Dollar-quote tag must not appear in the SQL template"
     assert statement not in query, "Statement must not be interpolated into the SQL template"
     assert params == (statement,), "Statement must be passed as a bound parameter"
+
+
+def test_new_connection_closes_conn_when_configure_raises(integration_check, pg_instance):
+    """If _configure_connection raises after connect() succeeds, the connection must be closed."""
+    check = integration_check(pg_instance)
+    conn = mock.MagicMock()
+    with mock.patch('datadog_checks.postgres.postgres.TokenAwareConnection.connect', return_value=conn):
+        with mock.patch.object(check.db_pool, '_configure_connection', side_effect=psycopg.Error('SET failed')):
+            with pytest.raises(psycopg.Error):
+                check._new_connection(check._config.dbname)
+    conn.close.assert_called_once()

zscaler_private_access/README.md

@@ -95,7 +95,7 @@ For Zscaler Private Access integration, specific custom log formats must be conf
 
    1. **User Activity Log**
    ```
-   {"LogTimestamp": %j{LogTimestamp:time},"Customer": %j{Customer},"SessionID": %j{SessionID},"ConnectionID": %j{ConnectionID},"InternalReason": %j{InternalReason},"ConnectionStatus": %j{ConnectionStatus},"IPProtocol": %d{IPProtocol},"DoubleEncryption": %d{DoubleEncryption},"Username": %j{Username},"ServicePort": %d{ServicePort},"ClientPublicIP": %j{ClientPublicIP},"ClientPrivateIP": %j{ClientPrivateIP},"ClientLatitude": %f{ClientLatitude},"ClientLongitude": %f{ClientLongitude},"ClientCountryCode": %j{ClientCountryCode},"ClientZEN": %j{ClientZEN},"Policy": %j{Policy},"Connector": %j{Connector},"ConnectorZEN": %j{ConnectorZEN},"ConnectorIP": %j{ConnectorIP},"ConnectorPort": %d{ConnectorPort},"Host": %j{Host},"Application": %j{Application},"AppGroup": %j{AppGroup},"Server": %j{Server},"ServerIP": %j{ServerIP},"ServerPort": %d{ServerPort},"PolicyProcessingTime": %d{PolicyProcessingTime},"ServerSetupTime": %d{ServerSetupTime},"TimestampConnectionStart": %j{TimestampConnectionStart:iso8601},"TimestampConnectionEnd": %j{TimestampConnectionEnd:iso8601},"TimestampCATx": %j{TimestampCATx:iso8601},"TimestampCARx": %j{TimestampCARx:iso8601},"TimestampAppLearnStart": %j{TimestampAppLearnStart:iso8601},"TimestampZENFirstRxClient": %j{TimestampZENFirstRxClient:iso8601},"TimestampZENFirstTxClient": %j{TimestampZENFirstTxClient:iso8601},"TimestampZENLastRxClient": %j{TimestampZENLastRxClient:iso8601},"TimestampZENLastTxClient": %j{TimestampZENLastTxClient:iso8601},"TimestampConnectorZENSetupComplete": %j{TimestampConnectorZENSetupComplete:iso8601},"TimestampZENFirstRxConnector": %j{TimestampZENFirstRxConnector:iso8601},"TimestampZENFirstTxConnector": %j{TimestampZENFirstTxConnector:iso8601},"TimestampZENLastRxConnector": %j{TimestampZENLastRxConnector:iso8601},"TimestampZENLastTxConnector": %j{TimestampZENLastTxConnector:iso8601},"ZENTotalBytesRxClient": %d{ZENTotalBytesRxClient},"ZENBytesRxClient": %d{ZENBytesRxClient},"ZENTotalBytesTxClient": %d{ZENTotalBytesTxClient},"ZENBytesTxClient": %d{ZENBytesTxClient},"ZENTotalBytesRxConnector": %d{ZENTotalBytesRxConnector},"ZENBytesRxConnector": %d{ZENBytesRxConnector},"ZENTotalBytesTxConnector": %d{ZENTotalBytesTxConnector},"ZENBytesTxConnector": %d{ZENBytesTxConnector},"Idp": %j{Idp},"ClientToClient": %j{c2c},"ClientCity": %j{ClientCity},"MicroTenantID": %j{MicroTenantID},"AppMicroTenantID": %j{AppMicroTenantID},"Platform": %j{Platform},"Hostname": %j{Hostname},"AppLearnTime": %d{AppLearnTime},"CAProcessingTime": %d{CAProcessingTime},"ConnectionSetupTime": %d{ConnectionSetupTime},"ConnectorZENSetupTime": %d{ConnectorZENSetupTime},"PRAApprovalID": %j{PRAApprovalID},"PRACapabilityPolicyID": %j{PRACapabilityPolicyID},"PRAConnectionID": %j{PRAConnectionID},"PRAConsoleType": %j{PRAConsoleType},"PRACredentialLoginType": %j{PRACredentialLoginType},"PRACredentialPolicyID": %j{PRACredentialPolicyID},"PRACredentialUserName": %j{PRACredentialUserName},"PRAErrorStatus": %j{PRAErrorStatus},"PRAFileTransferList": %j{PRAFileTransferList},"PRARecordingStatus": %j{PRARecordingStatus},"PRASessionType": %j{PRASessionType},"PRASharedMode": %j{PRASharedMode},"PRASharedUserList": %j{PRASharedUserList},"EventType": "user-activity"}\n
+   {"LogTimestamp": %j{LogTimestamp:time},"Customer": %j{Customer},"SessionID": %j{SessionID},"ConnectionID": %j{ConnectionID},"InternalReason": %j{InternalReason},"ConnectionStatus": %j{ConnectionStatus},"IPProtocol": %d{IPProtocol},"DoubleEncryption": %d{DoubleEncryption},"Username": %j{Username},"ServicePort": %d{ServicePort},"ClientPublicIP": %j{ClientPublicIP},"ClientPrivateIP": %j{ClientPrivateIP},"ClientLatitude": %f{ClientLatitude},"ClientLongitude": %f{ClientLongitude},"ClientCountryCode": %j{ClientCountryCode},"ClientZEN": %j{ClientZEN},"Policy": %j{Policy},"Connector": %j{Connector},"ConnectorZEN": %j{ConnectorZEN},"ConnectorIP": %j{ConnectorIP},"ConnectorPort": %d{ConnectorPort},"Host": %j{Host},"Application": %j{Application},"AppGroup": %j{AppGroup},"Server": %j{Server},"ServerIP": %j{ServerIP},"ServerPort": %d{ServerPort},"PolicyProcessingTime": %d{PolicyProcessingTime},"ServerSetupTime": %d{ServerSetupTime},"TimestampConnectionStart": %j{TimestampConnectionStart:iso8601},"TimestampConnectionEnd": %j{TimestampConnectionEnd:iso8601},"TimestampCATx": %j{TimestampCATx:iso8601},"TimestampCARx": %j{TimestampCARx:iso8601},"TimestampAppLearnStart": %j{TimestampAppLearnStart:iso8601},"TimestampZENFirstRxClient": %j{TimestampZENFirstRxClient:iso8601},"TimestampZENFirstTxClient": %j{TimestampZENFirstTxClient:iso8601},"TimestampZENLastRxClient": %j{TimestampZENLastRxClient:iso8601},"TimestampZENLastTxClient": %j{TimestampZENLastTxClient:iso8601},"TimestampConnectorZENSetupComplete": %j{TimestampConnectorZENSetupComplete:iso8601},"TimestampZENFirstRxConnector": %j{TimestampZENFirstRxConnector:iso8601},"TimestampZENFirstTxConnector": %j{TimestampZENFirstTxConnector:iso8601},"TimestampZENLastRxConnector": %j{TimestampZENLastRxConnector:iso8601},"TimestampZENLastTxConnector": %j{TimestampZENLastTxConnector:iso8601},"ZENTotalBytesRxClient": %d{ZENTotalBytesRxClient},"ZENBytesRxClient": %d{ZENBytesRxClient},"ZENTotalBytesTxClient": %d{ZENTotalBytesTxClient},"ZENBytesTxClient": %d{ZENBytesTxClient},"ZENTotalBytesRxConnector": %d{ZENTotalBytesRxConnector},"ZENBytesRxConnector": %d{ZENBytesRxConnector},"ZENTotalBytesTxConnector": %d{ZENTotalBytesTxConnector},"ZENBytesTxConnector": %d{ZENBytesTxConnector},"Idp": %j{Idp},"ClientToClient": %j{c2c},"ClientCity": %j{ClientCity},"MicroTenantID": %j{MicroTenantID},"AppMicroTenantID": %j{AppMicroTenantID},"Platform": %j{Platform},"Hostname": %j{Hostname},"AppLearnTime": %d{AppLearnTime},"CAProcessingTime": %d{CAProcessingTime},"ConnectionSetupTime": %d{ConnectionSetupTime},"ConnectorZENSetupTime": %d{ConnectorZENSetupTime},"PRAApprovalID": %j{PRAApprovalID},"PRACapabilityPolicyID": %j{PRACapabilityPolicyID},"PRAConnectionID": %j{PRAConnectionID},"PRAConsoleType": %j{PRAConsoleType},"PRACredentialLoginType": %j{PRACredentialLoginType},"PRACredentialPolicyID": %j{PRACredentialPolicyID},"PRACredentialUserName": %j{PRACredentialUserName},"PRAErrorStatus": %j{PRAErrorStatus},"PRAFileTransferList": %j{PRAFileTransferList},"PRARecordingStatus": %j{PRARecordingStatus},"PRASessionType": %j{PRASessionType},"PRASharedMode": %j{PRASharedMode},"EventType": "user-activity"}\n
    ```
 
    2. **User Status Log**

zscaler_private_access/assets/logs/zscaler-private-access_tests.yaml

@@ -664,7 +664,6 @@ tests:
         "Idp" : "Okta",
         "PRAConnectionID" : "PRA-SESSION-1",
         "PRAErrorStatus" : "",
-        "PRASharedUserList" : "{\"shared_user_list\":[{\"name\":\"secops@global.com\"}]}",
         "PRASharedMode" : "control",
         "ConnectionStatus" : "active",
         "PRAApprovalID" : 11111,
@@ -710,7 +709,6 @@ tests:
         PRAErrorStatus: ""
         PRARecordingStatus: "Available"
         PRASharedMode: "control"
-        PRASharedUserList: "{\"shared_user_list\":[{\"name\":\"secops@global.com\"}]}"
         Platform: "linux"
         Policy: "PRA SSH Policy"
         ServicePort: 22
@@ -738,7 +736,6 @@ tests:
           "Idp" : "Okta",
           "PRAConnectionID" : "PRA-SESSION-1",
           "PRAErrorStatus" : "",
-          "PRASharedUserList" : "{\"shared_user_list\":[{\"name\":\"secops@global.com\"}]}",
           "PRASharedMode" : "control",
           "ConnectionStatus" : "active",
           "PRAApprovalID" : 11111,