You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "moiseenkov (via GitHub)" <gi...@apache.org> on 2024/04/10 13:32:07 UTC

[PR] Enhancement for SSL-support in CloudSQLExecuteQueryOperator [airflow]

moiseenkov opened a new pull request, #38894:
URL: https://github.com/apache/airflow/pull/38894

   The PR provides enhancement for SSL support in `CloudSQLExecuteQueryOperator`.
   1. The previous implementation used to work with SSL certificates provided by users. If those files have excessive permissions, then `psql` raises an exception:
   ```python
   psql: error: connection to server at "10.40.112.2", port 5432 failed: private key file "client-key.pem" has group or world access; file must have permissions u=rw (0600) or less if owned by the current user, or permissions u=rw,g=r (0640) or less if owned by root
   ```
   In order to fix this problem, the `CloudSQLDatabaseHook` copies those files into a `/tmp/certs/` with minimal required permissions, and uses those copies instead.
   
   2. Added integration with Google Cloud Secret Manager, so users now can store their certificates as secrets and specify a secret id in the operator.
   3. Refactored system tests and removed the deprecated example DAG.


-- 
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: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Enhancement for SSL-support in CloudSQLExecuteQueryOperator [airflow]

Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on code in PR #38894:
URL: https://github.com/apache/airflow/pull/38894#discussion_r1560673265


##########
airflow/providers/google/cloud/hooks/cloud_sql.py:
##########
@@ -805,6 +858,84 @@ def __init__(
         self.db_conn_id = str(uuid.uuid1())
         self._validate_inputs()
 
+    @property
+    def sslcert(self) -> str | None:
+        return self._get_ssl_temporary_file_path(cert_name="sslcert", cert_path=self.ssl_cert)
+
+    @property
+    def sslkey(self) -> str | None:
+        return self._get_ssl_temporary_file_path(cert_name="sslkey", cert_path=self.ssl_key)
+
+    @property
+    def sslrootcert(self) -> str | None:
+        return self._get_ssl_temporary_file_path(cert_name="sslrootcert", cert_path=self.ssl_root_cert)
+
+    def _get_ssl_temporary_file_path(self, cert_name: str, cert_path: str | None) -> str | None:
+        cert_value = self._get_cert_from_secret(cert_name)
+        original_cert_path = cert_path or self.extras.get(cert_name)
+        if cert_value or original_cert_path:
+            if cert_name not in self._ssl_cert_temp_files:
+                return self._set_temporary_ssl_file(
+                    cert_name=cert_name, cert_path=original_cert_path, cert_value=cert_value
+                )
+            return self._ssl_cert_temp_files[cert_name].name
+        return None
+
+    def _get_cert_from_secret(self, cert_name: str) -> str | None:
+        if not self.ssl_secret_id:
+            return None
+
+        secret_hook = GoogleCloudSecretManagerHook(
+            gcp_conn_id=self.gcp_conn_id, impersonation_chain=self.impersonation_chain
+        )
+        secret: AccessSecretVersionResponse = secret_hook.access_secret(
+            project_id=self.project_id,
+            secret_id=self.ssl_secret_id,
+        )
+        secret_data = json.loads(base64.b64decode(secret.payload.data))
+        if cert_name in secret_data:
+            return secret_data[cert_name]
+        else:
+            raise AirflowException(
+                "Invalid secret format. Expected dictionary with keys: `sslcert`, `sslkey`, `sslrootcert`"
+            )
+
+    def _set_temporary_ssl_file(
+        self, cert_name: str, cert_path: str | None = None, cert_value: str | None = None
+    ) -> str | None:
+        """Save the certificate as a temporary file.
+
+        This method was implemented in order to overcome psql connection error caused by excessive file

Review Comment:
   +1. Glad you added the comment.



-- 
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: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Enhancement for SSL-support in CloudSQLExecuteQueryOperator [airflow]

Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk merged PR #38894:
URL: https://github.com/apache/airflow/pull/38894


-- 
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: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org