Răsfoiți Sursa

fix(cloudflare): DNS record query conditions - Boolean encoding and extra filter fallback (#571)

* Initial plan

* Fix Boolean values in URL parameters and add fallback logic for extra filters

Co-authored-by: NewFuture <[email protected]>

* Add documentation for extra filter and fallback behavior

Co-authored-by: NewFuture <[email protected]>

* docs: improve formatting consistency in documentation

Co-authored-by: NewFuture <[email protected]>

* refactor: move boolean conversion to cloudflare.py and simplify _query_record

Co-authored-by: NewFuture <[email protected]>

* chore: fix ruff linting issues and add format commands to copilot instructions

Co-authored-by: NewFuture <[email protected]>

---------

Co-authored-by: copilot-swe-agent[bot] <[email protected]>
Co-authored-by: NewFuture <[email protected]>
Copilot 3 săptămâni în urmă
părinte
comite
aba52bc517

+ 7 - 2
.github/copilot-instructions.md

@@ -7,11 +7,16 @@ This is a Python-based Dynamic DNS (DDNS) client that automatically updates DNS
 - Use only Python standard library modules (no external dependencies)
 - Ensure Python 2.7 and 3.x compatibility
 - Run tests before committing to ensure all functionality works correctly
-- Check the linting and formatting using `ruff`
+- Format and lint code using `ruff` before each commit:
+  ```bash
+  ruff check --fix --unsafe-fixes .
+  ruff format .
+  ```
 
 ### Development Flow
 - Test: `python -m unittest discover tests` or `python -m pytest tests/`
-- Format: Use ruff for code formatting and linting
+- Lint: `ruff check --fix --unsafe-fixes .`
+- Format: `ruff format .`
 
 ### Add a New DNS Provider
 

+ 22 - 3
ddns/provider/cloudflare.py

@@ -49,14 +49,33 @@ class CloudflareProvider(BaseProvider):
 
     def _query_record(self, zone_id, subdomain, main_domain, record_type, line, extra):
         # type: (str, str, str, str, str | None, dict) -> dict | None
-        """https://developers.cloudflare.com/api/resources/dns/subresources/records/methods/list/"""
+        """
+        查询DNS记录,优先使用extra filter匹配,匹配不到则fallback到不带extra的结果
+
+        Query DNS records, prioritize extra filters, fallback to query without extra if no match found.
+        https://developers.cloudflare.com/api/resources/dns/subresources/records/methods/list/
+        """
         # cloudflare的域名查询需要完整域名
         name = join_domain(subdomain, main_domain)
         query = {"name.exact": name}  # type: dict[str, str|None]
-        if extra:
-            query["proxied"] = extra.get("proxied", None)  # 代理状态
+
+        # 添加extra filter到查询参数,将布尔值转换为小写字符串
+        proxied = extra.get("proxied") if extra else None
+        if proxied is not None:
+            query["proxied"] = str(proxied).lower()  # True -> "true", False -> "false"
+
+        # 先使用extra filter查询
         data = self._request("GET", "/{}/dns_records".format(zone_id), type=record_type, per_page=10000, **query)
         record = next((r for r in data if r.get("name") == name and r.get("type") == record_type), None)
+
+        # 如果使用了extra filter但没找到记录,尝试不带extra filter查询
+        if not record and proxied is not None:
+            self.logger.debug("No record found with extra filters, retrying without extra filters")
+            data = self._request(
+                "GET", "/{}/dns_records".format(zone_id), type=record_type, per_page=10000, **{"name.exact": name}
+            )
+            record = next((r for r in data if r.get("name") == name and r.get("type") == record_type), None)
+
         self.logger.debug("Record queried: %s", record)
         if record:
             return record

+ 25 - 0
doc/providers/cloudflare.en.md

@@ -94,6 +94,7 @@ You can view and configure permissions in [Cloudflare API Token Management](http
 | ipv4 | IPv4 domains | Array | Domain list | None | Common Config |
 | ipv6 | IPv6 domains | Array | Domain list | None | Common Config |
 | ttl | TTL time | Integer (seconds) | [Reference below](#ttl) | `300/auto` | Provider Parameter |
+| proxied | Proxy status | Boolean | `true`, `false` | None | Provider Parameter |
 | proxy | Proxy settings | Array | [Reference](../config/json.en.md#proxy) | None | Common Network |
 | ssl | SSL verification | Boolean/String | `"auto"`, `true`, `false` | `auto` | Common Network |
 | cache | Cache settings | Boolean/String | `true`, `false`, `filepath` | `true` | Common Config |
@@ -105,6 +106,30 @@ You can view and configure permissions in [Cloudflare API Token Management](http
 > - **Common Network**: Network setting parameters applicable to all supported DNS providers
 > - **Provider Parameter**: Supported by current provider, values related to current provider
 
+### proxied
+
+The `proxied` parameter controls whether DNS records are proxied through Cloudflare. This is a Cloudflare-specific feature used to enable or disable its CDN and security protection features.
+
+#### Record Query Matching Logic
+
+When the `proxied` parameter is configured, DDNS queries existing DNS records with the following priority:
+
+1. **Priority: Use `proxied` filter**: First attempt to query records matching the specified `proxied` status
+2. **Fallback: Query without filter**: If the query with `proxied` filter finds no records, automatically retry the query without the `proxied` filter
+
+This fallback mechanism ensures correct handling of the following scenarios:
+
+- **Scenario 1**: Configuration changed from `"proxied": true` to `"proxied": false`
+  - Even if the original record has `proxied=true`, the system can still find and update it
+  
+- **Scenario 2**: Configuration changed from `"proxied": false` to `"proxied": true`
+  - Even if the original record has `proxied=false`, the system can still find and update it
+
+- **Scenario 3**: Configuration newly adds `"proxied": true/false` parameter
+  - Can find records created without `proxied` filter and update them
+
+> **Note**: If a record is found with the `proxied` filter during the query, the fallback query will not be executed and the matched record will be used directly.
+
 ### ttl
 
 The `ttl` parameter specifies the Time To Live (TTL) of DNS records in seconds. Cloudflare's TTL settings vary depending on whether the record has proxy enabled.

+ 25 - 0
doc/providers/cloudflare.md

@@ -94,6 +94,7 @@ API Token 方式更安全,支持精细化权限控制,是 Cloudflare 推荐
 | ipv4    | IPv4 域名     | 数组           | 域名列表                           | 无        | 公用配置   |
 | ipv6    | IPv6 域名     | 数组           | 域名列表                           | 无        | 公用配置   |
 | ttl     | TTL 时间      | 整数(秒)     | [参考下方](#ttl)                    | 300/auto | 服务商参数 |
+| proxied | 代理状态      | 布尔           | `true`、`false`                    | 无        | 服务商参数 |
 | proxy   | 代理设置      | 数组           | [参考配置](../config/json.md#proxy)        | 无        | 公用网络   |
 | ssl     | SSL 验证方式  | 布尔/字符串    | `"auto"`、`true`、`false`            | `auto`    | 公用网络   |
 | cache   | 缓存设置      | 布尔/字符串    | `true`、`false`、`filepath`        | `true`    | 公用配置   |
@@ -105,6 +106,30 @@ API Token 方式更安全,支持精细化权限控制,是 Cloudflare 推荐
 > - **公用网络**:所有支持的DNS服务商均适用的网络设置参数  
 > - **服务商参数**:当前服务商支持, 值与当前服务商相关
 
+### proxied
+
+`proxied` 参数控制 DNS 记录是否通过 Cloudflare 代理。这是 Cloudflare 特有的功能,用于启用或禁用其 CDN 和安全保护功能。
+
+#### 查询记录匹配逻辑
+
+当配置了 `proxied` 参数时,DDNS 会按照以下优先级查询现有的 DNS 记录:
+
+1. **优先使用 `proxied` 过滤**:首先尝试查询匹配指定 `proxied` 状态的记录
+2. **回退到无过滤查询**:如果带有 `proxied` 过滤的查询没有找到记录,将自动重试不带 `proxied` 过滤的查询
+
+这种回退机制确保了以下场景的正确处理:
+
+- **场景1**: 配置文件从 `"proxied": true` 改为 `"proxied": false`
+  - 即使原记录是 `proxied=true`,系统也能找到并更新它
+  
+- **场景2**: 配置文件从 `"proxied": false` 改为 `"proxied": true`
+  - 即使原记录是 `proxied=false`,系统也能找到并更新它
+
+- **场景3**: 配置文件新增 `"proxied": true/false` 参数
+  - 能够找到不带 `proxied` 过滤创建的记录并进行更新
+
+> **注意**:如果查询时带 `proxied` 过滤找到了记录,则不会执行回退查询,直接使用匹配的记录。
+
 ### ttl
 
 `ttl` 参数指定 DNS 记录的生存时间(TTL),单位为秒。Cloudflare 的 TTL 设置根据记录是否启用代理而有所不同。

+ 121 - 6
tests/test_provider_cloudflare.py

@@ -200,18 +200,123 @@ class TestCloudflareProvider(BaseProviderTestCase):
             provider, "_request"
         ) as mock_request:
             mock_join.return_value = "www.example.com"
-            mock_request.return_value = []
+            # When record is found with extra filter, should not fallback
+            mock_request.return_value = [
+                {"id": "rec123", "name": "www.example.com", "type": "A", "content": "1.2.3.4", "proxied": True}
+            ]
 
             extra = {"proxied": True}
-            provider._query_record("zone123", "www", "example.com", "A", None, extra)
+            result = provider._query_record("zone123", "www", "example.com", "A", None, extra)
 
+            # Should call only once since record is found with extra filter
+            # Note: proxied is converted to lowercase string "true"
             mock_request.assert_called_once_with(
                 "GET",
                 "/zone123/dns_records",
                 type="A",
                 per_page=10000,
-                **{"name.exact": "www.example.com", "proxied": True}
+                **{"name.exact": "www.example.com", "proxied": "true"}
             )  # fmt: skip
+            self.assertIsNotNone(result)
+
+    def test_query_record_with_proxy_false_fallback(self):
+        """Test _query_record fallback logic when proxied=False filter returns no results"""
+        provider = CloudflareProvider(self.authid, self.token)
+
+        with patch("ddns.provider.cloudflare.join_domain") as mock_join, patch.object(
+            provider, "_request"
+        ) as mock_request:
+            mock_join.return_value = "test.example.net"
+            # First call with extra filter returns empty, second call without filter returns record
+            mock_request.side_effect = [
+                [],  # No results with proxied=False
+                [{"id": "rec123", "name": "test.example.net", "type": "A", "content": "1.2.3.4", "proxied": True}],
+            ]
+
+            extra = {"proxied": False}
+            result = provider._query_record("zone123", "test", "example.net", "A", None, extra)
+
+            # Should call twice - first with extra filter, then without
+            self.assertEqual(mock_request.call_count, 2)
+            # Note: proxied is converted to lowercase string "false"
+            mock_request.assert_any_call(
+                "GET",
+                "/zone123/dns_records",
+                type="A",
+                per_page=10000,
+                **{"name.exact": "test.example.net", "proxied": "false"}
+            )  # fmt: skip
+            mock_request.assert_any_call(
+                "GET",
+                "/zone123/dns_records",
+                type="A",
+                per_page=10000,
+                **{"name.exact": "test.example.net"}
+            )  # fmt: skip
+            # Should return the record found without extra filter
+            self.assertIsNotNone(result)
+            self.assertEqual(result["id"], "rec123")
+
+    def test_query_record_with_proxy_true_fallback(self):
+        """Test _query_record fallback logic when proxied=True filter returns no results"""
+        provider = CloudflareProvider(self.authid, self.token)
+
+        with patch("ddns.provider.cloudflare.join_domain") as mock_join, patch.object(
+            provider, "_request"
+        ) as mock_request:
+            mock_join.return_value = "test.example.net"
+            # First call with extra filter returns empty, second call without filter returns record
+            mock_request.side_effect = [
+                [],  # No results with proxied=True
+                [{"id": "rec456", "name": "test.example.net", "type": "A", "content": "1.2.3.4", "proxied": False}],
+            ]
+
+            extra = {"proxied": True}
+            result = provider._query_record("zone123", "test", "example.net", "A", None, extra)
+
+            # Should call twice - first with extra filter, then without
+            self.assertEqual(mock_request.call_count, 2)
+            # Should return the record found without extra filter
+            self.assertIsNotNone(result)
+            self.assertEqual(result["id"], "rec456")
+
+    def test_query_record_with_proxy_found_with_filter(self):
+        """Test _query_record does not fallback when record is found with extra filter"""
+        provider = CloudflareProvider(self.authid, self.token)
+
+        with patch("ddns.provider.cloudflare.join_domain") as mock_join, patch.object(
+            provider, "_request"
+        ) as mock_request:
+            mock_join.return_value = "test.example.net"
+            # Returns record on first call with extra filter
+            mock_request.return_value = [
+                {"id": "rec789", "name": "test.example.net", "type": "A", "content": "1.2.3.4", "proxied": True}
+            ]
+
+            extra = {"proxied": True}
+            result = provider._query_record("zone123", "test", "example.net", "A", None, extra)
+
+            # Should call only once since record found with extra filter
+            self.assertEqual(mock_request.call_count, 1)
+            self.assertIsNotNone(result)
+            self.assertEqual(result["id"], "rec789")
+
+    def test_query_record_no_extra_filter(self):
+        """Test _query_record without extra filters does not perform fallback"""
+        provider = CloudflareProvider(self.authid, self.token)
+
+        with patch("ddns.provider.cloudflare.join_domain") as mock_join, patch.object(
+            provider, "_request"
+        ) as mock_request:
+            mock_join.return_value = "www.example.com"
+            mock_request.return_value = []
+
+            # No extra filters
+            result = provider._query_record("zone123", "www", "example.com", "A", None, {})
+
+            # Should call only once since no extra filters
+            self.assertEqual(mock_request.call_count, 1)
+            self.assertIsNone(result)
 
     def test_create_record_success(self):
         """Test _create_record method with successful creation"""
@@ -326,7 +431,14 @@ class TestCloudflareProvider(BaseProviderTestCase):
         """Test _update_record method with extra parameters overriding old_record values"""
         provider = CloudflareProvider(self.authid, self.token)
 
-        old_record = {"id": "rec123", "name": "www.example.com", "comment": "Old comment", "proxied": False, "tags": ["old_tag"], "settings": {"old": "setting"}}
+        old_record = {
+            "id": "rec123",
+            "name": "www.example.com",
+            "comment": "Old comment",
+            "proxied": False,
+            "tags": ["old_tag"],
+            "settings": {"old": "setting"},
+        }
 
         with patch.object(provider, "_request") as mock_request:
             mock_request.return_value = {"id": "rec123"}
@@ -345,7 +457,9 @@ class TestCloudflareProvider(BaseProviderTestCase):
                 proxied=True,  # extra.get("proxied", old_record.get("proxied")) - extra takes priority
                 priority=20,  # From extra
                 tags=["new_tag"],  # extra.get("tags", old_record.get("tags")) - extra takes priority
-                settings={"old": "setting"},  # extra.get("settings", old_record.get("settings")) - falls back to old_record
+                settings={
+                    "old": "setting"
+                },  # extra.get("settings", old_record.get("settings")) - falls back to old_record
             )
             self.assertTrue(result)
 
@@ -508,7 +622,8 @@ class TestCloudflareProviderIntegration(BaseProviderTestCase):
             # Simulate successful creation with custom options
             mock_request.side_effect = [
                 [{"id": "zone123", "name": "example.com"}],  # _query_zone_id response
-                [],  # _query_record response (no existing record)
+                [],  # _query_record response with extra filter (no existing record)
+                [],  # _query_record fallback without extra filter (no existing record)
                 {"id": "rec123", "name": "www.example.com"},  # _create_record response
             ]