Skip to content

Commit cfefead

Browse files
Add tests for buildCustomHostnameNewParams and improve error handling comments
- Add comprehensive test coverage for buildCustomHostnameNewParams function - Improve comment explaining why string-based rate limit detection is needed with v5 SDK (SDK's retry logic can wrap errors hiding structured types) - Addresses reviewer feedback about test coverage and error handling rationale
1 parent 47f7fbc commit cfefead

File tree

2 files changed

+126
-2
lines changed

2 files changed

+126
-2
lines changed

provider/cloudflare/cloudflare.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -353,8 +353,10 @@ func convertCloudflareError(err error) error {
353353
}
354354
}
355355

356-
// Also check for rate limit indicators in error message strings as a fallback
357-
// This handles cases where the SDK or retry logic wraps the error
356+
// Also check for rate limit indicators in error message strings as a fallback.
357+
// The v5 SDK's retry logic and error wrapping can hide the structured error type,
358+
// so we need string matching to catch rate limits in wrapped errors like:
359+
// "exceeded available rate limit retries" from the SDK's auto-retry mechanism.
358360
errMsg := strings.ToLower(err.Error())
359361
if strings.Contains(errMsg, "rate limit") ||
360362
strings.Contains(errMsg, "429") ||

provider/cloudflare/cloudflare_test.go

Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3004,6 +3004,128 @@ func TestGetUpdateDNSRecordParam(t *testing.T) {
30043004
assert.Equal(t, "test-comment", body.Comment.Value)
30053005
}
30063006

3007+
func TestBuildCustomHostnameNewParams(t *testing.T) {
3008+
t.Run("Minimal custom hostname without SSL", func(t *testing.T) {
3009+
ch := CustomHostname{
3010+
Hostname: "test.example.com",
3011+
CustomOriginServer: "origin.example.com",
3012+
}
3013+
3014+
params := buildCustomHostnameNewParams("zone-123", ch)
3015+
3016+
assert.Equal(t, "zone-123", params.ZoneID.Value)
3017+
assert.Equal(t, "test.example.com", params.Hostname.Value)
3018+
assert.False(t, params.SSL.Present)
3019+
})
3020+
3021+
t.Run("Custom hostname with full SSL configuration", func(t *testing.T) {
3022+
ch := CustomHostname{
3023+
Hostname: "test.example.com",
3024+
CustomOriginServer: "origin.example.com",
3025+
SSL: &CustomHostnameSSL{
3026+
Type: "dv",
3027+
Method: "http",
3028+
BundleMethod: "ubiquitous",
3029+
CertificateAuthority: "digicert",
3030+
Settings: CustomHostnameSSLSettings{
3031+
MinTLSVersion: "1.2",
3032+
},
3033+
},
3034+
}
3035+
3036+
params := buildCustomHostnameNewParams("zone-123", ch)
3037+
3038+
assert.Equal(t, "zone-123", params.ZoneID.Value)
3039+
assert.Equal(t, "test.example.com", params.Hostname.Value)
3040+
assert.True(t, params.SSL.Present)
3041+
3042+
ssl := params.SSL.Value
3043+
assert.Equal(t, "dv", string(ssl.Type.Value))
3044+
assert.Equal(t, "http", string(ssl.Method.Value))
3045+
assert.Equal(t, "ubiquitous", string(ssl.BundleMethod.Value))
3046+
assert.Equal(t, "digicert", string(ssl.CertificateAuthority.Value))
3047+
assert.Equal(t, "1.2", string(ssl.Settings.Value.MinTLSVersion.Value))
3048+
})
3049+
3050+
t.Run("Custom hostname with partial SSL configuration", func(t *testing.T) {
3051+
ch := CustomHostname{
3052+
Hostname: "test.example.com",
3053+
CustomOriginServer: "origin.example.com",
3054+
SSL: &CustomHostnameSSL{
3055+
Type: "dv",
3056+
Method: "http",
3057+
},
3058+
}
3059+
3060+
params := buildCustomHostnameNewParams("zone-123", ch)
3061+
3062+
assert.True(t, params.SSL.Present)
3063+
ssl := params.SSL.Value
3064+
assert.Equal(t, "dv", string(ssl.Type.Value))
3065+
assert.Equal(t, "http", string(ssl.Method.Value))
3066+
assert.False(t, ssl.BundleMethod.Present)
3067+
assert.False(t, ssl.CertificateAuthority.Present)
3068+
assert.False(t, ssl.Settings.Present)
3069+
})
3070+
3071+
t.Run("Custom hostname with 'none' certificate authority", func(t *testing.T) {
3072+
ch := CustomHostname{
3073+
Hostname: "test.example.com",
3074+
CustomOriginServer: "origin.example.com",
3075+
SSL: &CustomHostnameSSL{
3076+
Type: "dv",
3077+
Method: "http",
3078+
CertificateAuthority: "none",
3079+
},
3080+
}
3081+
3082+
params := buildCustomHostnameNewParams("zone-123", ch)
3083+
3084+
assert.True(t, params.SSL.Present)
3085+
ssl := params.SSL.Value
3086+
// "none" should not be set as certificate authority
3087+
assert.False(t, ssl.CertificateAuthority.Present)
3088+
})
3089+
3090+
t.Run("Custom hostname with empty certificate authority", func(t *testing.T) {
3091+
ch := CustomHostname{
3092+
Hostname: "test.example.com",
3093+
CustomOriginServer: "origin.example.com",
3094+
SSL: &CustomHostnameSSL{
3095+
Type: "dv",
3096+
Method: "http",
3097+
CertificateAuthority: "",
3098+
},
3099+
}
3100+
3101+
params := buildCustomHostnameNewParams("zone-123", ch)
3102+
3103+
assert.True(t, params.SSL.Present)
3104+
ssl := params.SSL.Value
3105+
// Empty string should not be set
3106+
assert.False(t, ssl.CertificateAuthority.Present)
3107+
})
3108+
3109+
t.Run("Custom hostname with only MinTLSVersion", func(t *testing.T) {
3110+
ch := CustomHostname{
3111+
Hostname: "test.example.com",
3112+
CustomOriginServer: "origin.example.com",
3113+
SSL: &CustomHostnameSSL{
3114+
Settings: CustomHostnameSSLSettings{
3115+
MinTLSVersion: "1.3",
3116+
},
3117+
},
3118+
}
3119+
3120+
params := buildCustomHostnameNewParams("zone-123", ch)
3121+
3122+
assert.True(t, params.SSL.Present)
3123+
ssl := params.SSL.Value
3124+
assert.True(t, ssl.Settings.Present)
3125+
assert.Equal(t, "1.3", string(ssl.Settings.Value.MinTLSVersion.Value))
3126+
})
3127+
}
3128+
30073129
func TestZoneService(t *testing.T) {
30083130
t.Parallel()
30093131

0 commit comments

Comments
 (0)