Skip to content

Commit 81b8b2c

Browse files
committed
✨ test: add punycode support and improve test infrastructure
- Add pre-commit config with go-fmt, go-mod-tidy, golangci-lint, go-sec - Add Makefile targets for unit/integration tests with validation - Support Punycode to Unicode conversion in solver for international domains - Add comprehensive Punycode test cases in solver_test.go - Improve test helper functions in client_test.go (mustSetEnv/mustUnsetEnv) - Add hadolint ignore for Alpine APK install in Dockerfile - Move golang.org/x/net from indirect to direct dependency - Update DEVELOPMENT.md with simplified test commands
1 parent 9f3ba19 commit 81b8b2c

9 files changed

Lines changed: 158 additions & 28 deletions

File tree

.pre-commit-config.yaml

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
# See https://pre-commit.com for more information
2+
# See https://pre-commit.com/hooks.html for more hooks
3+
exclude: |
4+
(?x)^(
5+
gen/docs/.*|
6+
.*_gen\.go|
7+
)$
8+
fail_fast: false
9+
repos:
10+
- repo: https://github.com/pre-commit/pre-commit-hooks
11+
rev: v6.0.0
12+
hooks:
13+
- id: trailing-whitespace
14+
- id: end-of-file-fixer
15+
- id: check-added-large-files
16+
- repo: https://github.com/TekWizely/pre-commit-golang
17+
rev: v1.0.0-rc.4
18+
hooks:
19+
- id: go-fmt
20+
args: [-w]
21+
- id: go-mod-tidy
22+
- id: golangci-lint-mod
23+
- id: go-sec-mod
24+
- repo: https://github.com/hadolint/hadolint
25+
rev: v2.14.0
26+
hooks:
27+
- id: hadolint
28+
- repo: https://github.com/codespell-project/codespell
29+
rev: v2.4.1
30+
hooks:
31+
- id: codespell
32+
name: Run codespell to check for common misspellings in files
33+
language: python
34+
types: [ text ]
35+
args:
36+
- --write-changes
37+
- --skip=go.sum,./gen
38+
- -L thirdparty

DEVELOPMENT.md

Lines changed: 15 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -160,8 +160,8 @@ cd cert-manager-alidns-webhook
160160
# 2. 安装依赖
161161
go mod download
162162

163-
# 3. 运行测试
164-
go test -v ./...
163+
# 3. 运行测试(默认只跑单元测试)
164+
make
165165
```
166166

167167
### 阿里云访问凭证配置
@@ -273,10 +273,10 @@ sed -i '' 's|k8s.io/.* v0.35.0|k8s.io/client-go v0.34.1|' go.mod
273273
go mod tidy
274274
```
275275

276-
4. **运行测试验证**
276+
4. **运行测试验证(默认只跑单元测试)**
277277

278278
```bash
279-
go test -v ./...
279+
make
280280
```
281281

282282
#### 本地验证
@@ -312,7 +312,7 @@ go run main.go \
312312

313313
```bash
314314
# 运行所有单元测试
315-
go test -v ./...
315+
make test-unit
316316

317317
# 运行特定包的测试
318318
go test -v ./pkg/alidns/
@@ -336,15 +336,19 @@ go test -cover ./...
336336

337337
```bash
338338
# 设置测试域名(注意末尾的点)
339-
TEST_ZONE_NAME=example.com. make test
340-
341-
# 或者直接运行 go test(需要先设置环境变量)
342-
export TEST_ZONE_NAME=example.com.
343-
go test -v ./pkg/alidns/ -tags=integration
339+
TEST_ZONE_NAME=example.com. make test-integration
344340
```
345341

342+
Makefile 会检查 `TEST_ZONE_NAME` 是否设置并校验格式(必须以点结尾)。
343+
346344
替换上面命令中 `example.com.` 为你当前托管在阿里云用于测试的域名(不要忘记域名后面的 `.`)。
347345

346+
#### 运行全部测试
347+
348+
```bash
349+
make test-all
350+
```
351+
348352
---
349353

350354
## 参考资源
@@ -370,7 +374,7 @@ go test -v ./pkg/alidns/ -tags=integration
370374

371375
在提交 PR 前,请确保:
372376

373-
1. ✅ 代码通过所有测试(`go test ./...`
377+
1. ✅ 代码通过所有测试(`make test-all`
374378
2. ✅ 添加了必要的单元测试
375379
3. ✅ 更新了相关文档(如需要)
376380

Dockerfile

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ RUN CGO_ENABLED=0 GOOS=linux go build \
2424
# Final stage
2525
FROM alpine:3.23
2626

27+
# hadolint ignore=DL3018
2728
RUN apk add --no-cache ca-certificates tzdata && \
2829
addgroup -g 1000 cert-manager && \
2930
adduser -u 1000 -G cert-manager -D -h /home/cert-manager cert-manager

Makefile

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,11 +11,30 @@ KUBEBUILDER_VERSION=1.28.0
1111

1212
HELM_FILES := $(shell find deploy/cert-manager-alidns-webhook)
1313

14-
test: _test/kubebuilder-$(KUBEBUILDER_VERSION)-$(OS)-$(ARCH)/etcd _test/kubebuilder-$(KUBEBUILDER_VERSION)-$(OS)-$(ARCH)/kube-apiserver _test/kubebuilder-$(KUBEBUILDER_VERSION)-$(OS)-$(ARCH)/kubectl
14+
.DEFAULT_GOAL := test-unit
15+
.MAIN: test-unit
16+
17+
.PHONY: test-integration
18+
test-integration: _test/kubebuilder-$(KUBEBUILDER_VERSION)-$(OS)-$(ARCH)/etcd _test/kubebuilder-$(KUBEBUILDER_VERSION)-$(OS)-$(ARCH)/kube-apiserver _test/kubebuilder-$(KUBEBUILDER_VERSION)-$(OS)-$(ARCH)/kubectl
19+
@if [ -z "$(TEST_ZONE_NAME)" ]; then \
20+
echo "ERROR: TEST_ZONE_NAME is not set. Example: TEST_ZONE_NAME=example.com. (note the trailing dot)."; \
21+
exit 1; \
22+
fi
23+
@if ! printf '%s' "$(TEST_ZONE_NAME)" | grep -Eq '^[A-Za-z0-9]([A-Za-z0-9-]*[A-Za-z0-9])?(\.[A-Za-z0-9]([A-Za-z0-9-]*[A-Za-z0-9])?)*\.$'; then \
24+
echo "ERROR: TEST_ZONE_NAME must be a FQDN ending with a dot, e.g. example.com. (trailing dot required)."; \
25+
exit 1; \
26+
fi
1527
TEST_ASSET_ETCD=_test/kubebuilder-$(KUBEBUILDER_VERSION)-$(OS)-$(ARCH)/etcd \
1628
TEST_ASSET_KUBE_APISERVER=_test/kubebuilder-$(KUBEBUILDER_VERSION)-$(OS)-$(ARCH)/kube-apiserver \
1729
TEST_ASSET_KUBECTL=_test/kubebuilder-$(KUBEBUILDER_VERSION)-$(OS)-$(ARCH)/kubectl \
18-
$(GO) test -v .
30+
$(GO) test -v -tags=integration .
31+
32+
.PHONY: test-unit
33+
test-unit:
34+
$(GO) test -v ./...
35+
36+
.PHONY: test-all
37+
test-all: test-unit test-integration
1938

2039
_test/kubebuilder-$(KUBEBUILDER_VERSION)-$(OS)-$(ARCH).tar.gz: | _test
2140
curl -fsSL https://go.kubebuilder.io/test-tools/$(KUBEBUILDER_VERSION)/$(OS)/$(ARCH) -o $@

README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -395,4 +395,4 @@ This project is based on the [cert-manager/webhook-example](https://github.com/c
395395

396396
<p align="center">
397397
<sub>Built with ❤️ by the open source community</sub>
398-
</p>
398+
</p>

go.mod

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ require (
1010
github.com/aliyun/credentials-go v1.4.10
1111
github.com/cert-manager/cert-manager v1.19.2
1212
github.com/stretchr/testify v1.11.1
13+
golang.org/x/net v0.47.0
1314
k8s.io/client-go v0.34.1
1415
)
1516

@@ -87,7 +88,6 @@ require (
8788
golang.org/x/crypto v0.45.0 // indirect
8889
golang.org/x/exp v0.0.0-20250718183923-645b1fa84792 // indirect
8990
golang.org/x/mod v0.30.0 // indirect
90-
golang.org/x/net v0.47.0 // indirect
9191
golang.org/x/oauth2 v0.31.0 // indirect
9292
golang.org/x/sync v0.18.0 // indirect
9393
golang.org/x/sys v0.38.0 // indirect

pkg/alidns/client_test.go

Lines changed: 25 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
util "github.com/alibabacloud-go/tea-utils/v2/service"
1111
"github.com/alibabacloud-go/tea/tea"
1212
"github.com/stretchr/testify/assert"
13+
"github.com/stretchr/testify/require"
1314
)
1415

1516
// MockAliDNSClient 是用于测试的 mock 客户端
@@ -54,11 +55,11 @@ func (m *MockAliDNSClient) DescribeDomainRecordsWithOptions(request *alidns.Desc
5455

5556
func TestAddTXTRecord(t *testing.T) {
5657
tests := []struct {
57-
name string
58+
name string
5859
existingRecords []*alidns.DescribeDomainRecordsResponseBodyDomainRecordsRecord
59-
addFuncCalled bool
60-
expectError bool
61-
errorMsg string
60+
addFuncCalled bool
61+
expectError bool
62+
errorMsg string
6263
}{
6364
{
6465
name: "new record - should create",
@@ -133,9 +134,10 @@ func TestAddTXTRecord(t *testing.T) {
133134
}
134135
} else {
135136
assert.NoError(t, err)
136-
if tt.name == "record already exists - should return existing" {
137+
switch tt.name {
138+
case "record already exists - should return existing":
137139
assert.Equal(t, "existing-id", recordID)
138-
} else if tt.name == "new record - should create" {
140+
case "new record - should create":
139141
assert.Equal(t, "new-record-id", recordID)
140142
}
141143
}
@@ -196,7 +198,7 @@ func TestDeleteRecordsByKey(t *testing.T) {
196198
expectError bool
197199
}{
198200
{
199-
name: "single matching record",
201+
name: "single matching record",
200202
records: []*alidns.DescribeDomainRecordsResponseBodyDomainRecordsRecord{
201203
{
202204
RecordId: tea.String("record-1"),
@@ -228,8 +230,8 @@ func TestDeleteRecordsByKey(t *testing.T) {
228230
expectError: false,
229231
},
230232
{
231-
name: "describe API error",
232-
records: nil,
233+
name: "describe API error",
234+
records: nil,
233235
expectDelete: 0,
234236
expectError: true,
235237
},
@@ -432,21 +434,31 @@ func TestGetEndpoint(t *testing.T) {
432434
originalValue := os.Getenv("ALIBABA_CLOUD_REGION_ID")
433435
defer func() {
434436
if originalValue != "" {
435-
os.Setenv("ALIBABA_CLOUD_REGION_ID", originalValue)
437+
mustSetEnv(t, "ALIBABA_CLOUD_REGION_ID", originalValue)
436438
} else {
437-
os.Unsetenv("ALIBABA_CLOUD_REGION_ID")
439+
mustUnsetEnv(t, "ALIBABA_CLOUD_REGION_ID")
438440
}
439441
}()
440442

441443
// Set test env var
442444
if tt.envRegion != "" {
443-
os.Setenv("ALIBABA_CLOUD_REGION_ID", tt.envRegion)
445+
mustSetEnv(t, "ALIBABA_CLOUD_REGION_ID", tt.envRegion)
444446
} else {
445-
os.Unsetenv("ALIBABA_CLOUD_REGION_ID")
447+
mustUnsetEnv(t, "ALIBABA_CLOUD_REGION_ID")
446448
}
447449

448450
result := getEndpoint()
449451
assert.Equal(t, tt.expectedResult, result)
450452
})
451453
}
452454
}
455+
456+
func mustSetEnv(t *testing.T, key, value string) {
457+
t.Helper()
458+
require.NoError(t, os.Setenv(key, value))
459+
}
460+
461+
func mustUnsetEnv(t *testing.T, key string) {
462+
t.Helper()
463+
require.NoError(t, os.Unsetenv(key))
464+
}

pkg/alidns/solver.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77
"log/slog"
88

99
"github.com/cert-manager/cert-manager/pkg/acme/webhook/apis/acme/v1alpha1"
10+
"golang.org/x/net/idna"
1011
"k8s.io/client-go/rest"
1112

1213
"github.com/cert-manager/cert-manager/pkg/issuer/acme/dns/util"
@@ -200,5 +201,13 @@ func (s *Solver) extractDomainAndRR(fqdn, zone string) (string, string) {
200201
// 移除 rr 可能的结尾点
201202
rr = strings.TrimSuffix(rr, ".")
202203

204+
// Convert Punycode to Unicode for both zone (domain) and rr
205+
if uZone, err := idna.ToUnicode(zone); err == nil {
206+
zone = uZone
207+
}
208+
if uRR, err := idna.ToUnicode(rr); err == nil {
209+
rr = uRR
210+
}
211+
203212
return zone, rr
204213
}

pkg/alidns/solver_test.go

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -216,3 +216,50 @@ func TestSolver_CleanUp_Uninitialized(t *testing.T) {
216216
assert.Error(t, err)
217217
assert.Contains(t, err.Error(), "not initialized")
218218
}
219+
220+
func TestExtractDomainAndRR_Punycode(t *testing.T) {
221+
solver := &Solver{}
222+
223+
tests := []struct {
224+
name string
225+
fqdn string
226+
zone string
227+
expectDomain string
228+
expectRR string
229+
}{
230+
{
231+
name: "punycode zone",
232+
fqdn: "_acme-challenge.xn--fiq228c.com.",
233+
zone: "xn--fiq228c.com.",
234+
expectDomain: "中文.com",
235+
expectRR: "_acme-challenge",
236+
},
237+
{
238+
name: "punycode subdomain",
239+
fqdn: "_acme-challenge.xn--0zwm56d.xn--fiq228c.com.",
240+
zone: "xn--fiq228c.com.",
241+
expectDomain: "中文.com",
242+
expectRR: "_acme-challenge.测试",
243+
},
244+
{
245+
name: "mixed punycode and unicode (should fail splitting if mismatch)",
246+
fqdn: "_acme-challenge.中文.com.",
247+
zone: "xn--fiq228c.com.",
248+
// Mismatch causes split failure, so RR is full FQDN.
249+
// zone "xn--fiq228c.com." -> "中文.com"
250+
// fqdn "_acme-challenge.中文.com."
251+
// rr = fqdn (since suffix doesn't match string-wise)
252+
// rr -> ToUnicode("_acme-challenge.中文.com.") -> "_acme-challenge.中文.com"
253+
expectDomain: "中文.com",
254+
expectRR: "_acme-challenge.中文.com",
255+
},
256+
}
257+
258+
for _, tt := range tests {
259+
t.Run(tt.name, func(t *testing.T) {
260+
domain, rr := solver.extractDomainAndRR(tt.fqdn, tt.zone)
261+
assert.Equal(t, tt.expectDomain, domain, "Domain mismatch")
262+
assert.Equal(t, tt.expectRR, rr, "RR mismatch")
263+
})
264+
}
265+
}

0 commit comments

Comments
 (0)