Skip to content

api: make RateLimitRedisSettings.url a pointer #9538

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
guanchzhou wants to merge 1 commit into
base: main
Choose a base branch
from
Open

api: make RateLimitRedisSettings.url a pointer #9538

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion api/v1alpha1/envoygateway_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -690,7 +690,7 @@ type RateLimitRedisSettings struct {
// Mutually exclusive with URLRef.
//
// +optional
URL string `json:"url,omitempty"`
URL *string `json:"url,omitempty"`

// URLRef sources the Redis URL from a Kubernetes Secret key. Use this for GitOps
// flows where the Redis endpoint is provisioned by an external controller.
Expand Down
4 changes: 2 additions & 2 deletions api/v1alpha1/validation/envoygateway_validate.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ func validateEnvoyGatewayRateLimit(rateLimit *egv1a1.RateLimit) error {
return fmt.Errorf("empty ratelimit redis settings")
}

hasURL := redis.URL != ""
hasURL := redis.URL != nil

@zirain zirain Jul 21, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not use ptr.Deref?

hasURLRef := redis.URLRef != nil
if hasURL == hasURLRef {
return fmt.Errorf("exactly one of ratelimit redis url or urlRef must be set")
Expand All @@ -215,7 +215,7 @@ func validateEnvoyGatewayRateLimit(rateLimit *egv1a1.RateLimit) error {
return nil
}

return ValidateRedisURL(redis.URL)
return ValidateRedisURL(*redis.URL)
}

// ValidateRedisURL validates a ratelimit Redis URL string, which may be a single
Expand Down
12 changes: 6 additions & 6 deletions api/v1alpha1/validation/envoygateway_validate_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,7 @@ func TestValidateEnvoyGateway(t *testing.T) {
Backend: egv1a1.RateLimitDatabaseBackend{
Type: egv1a1.RedisBackendType,
Redis: &egv1a1.RateLimitRedisSettings{
URL: ":foo",
URL: new(":foo"),

@chatgpt-codex-connector chatgpt-codex-connector Bot Jul 21, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Stop calling new with string literals

When any of the modified Go test packages are compiled, new(":foo") is invalid because new takes a type, not a value, so these updated tests fail before running. The same URL: new("...") pattern appears throughout this commit; use a string pointer helper such as ptr.To(...) or assign the literal to a variable and take its address.

Useful? React with 👍 / 👎.

@guanchzhou guanchzhou Jul 21, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a false positive. Go 1.26 (this module is go 1.26.4) added the two-argument-style new(expr) builtin that allocates and initializes from a value, so new(":foo") is valid and returns *string. The sibling RateLimitRedisSettings tests already use this form (Optional: new(true) / new(false)) from #9143 and pass upstream CI. go test ./... on the modified packages compiles and passes (473 tests).

},
},
},
Expand All @@ -295,7 +295,7 @@ func TestValidateEnvoyGateway(t *testing.T) {
Backend: egv1a1.RateLimitDatabaseBackend{
Type: egv1a1.RedisBackendType,
Redis: &egv1a1.RateLimitRedisSettings{
URL: "localhost:6376",
URL: new("localhost:6376"),
},
},
},
Expand All @@ -313,7 +313,7 @@ func TestValidateEnvoyGateway(t *testing.T) {
Backend: egv1a1.RateLimitDatabaseBackend{
Type: egv1a1.RedisBackendType,
Redis: &egv1a1.RateLimitRedisSettings{
URL: "primary_.-,node-0:26379,node-1:26379",
URL: new("primary_.-,node-0:26379,node-1:26379"),
},
},
},
Expand All @@ -331,7 +331,7 @@ func TestValidateEnvoyGateway(t *testing.T) {
Backend: egv1a1.RateLimitDatabaseBackend{
Type: egv1a1.RedisBackendType,
Redis: &egv1a1.RateLimitRedisSettings{
URL: "node-0:6376,node-1:6376,node-2:6376",
URL: new("node-0:6376,node-1:6376,node-2:6376"),
},
},
},
Expand Down Expand Up @@ -1485,7 +1485,7 @@ func TestValidateEnvoyGatewayRateLimitURLRef(t *testing.T) {
},
{
name: "url only",
rateLimit: redisBackend(&egv1a1.RateLimitRedisSettings{URL: "redis.redis.svc:6379"}),
rateLimit: redisBackend(&egv1a1.RateLimitRedisSettings{URL: new("redis.redis.svc:6379")}),
expectErr: false,
},
{
Expand All @@ -1503,7 +1503,7 @@ func TestValidateEnvoyGatewayRateLimitURLRef(t *testing.T) {
{
name: "both url and urlRef set",
rateLimit: redisBackend(&egv1a1.RateLimitRedisSettings{
URL: "redis.redis.svc:6379",
URL: new("redis.redis.svc:6379"),
URLRef: &egv1a1.RedisURLSource{
SecretKeyRef: &corev1.SecretKeySelector{
LocalObjectReference: corev1.LocalObjectReference{Name: "redis-conn"},
Expand Down
5 changes: 5 additions & 0 deletions api/v1alpha1/zz_generated.deepcopy.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions internal/envoygateway/config/decoder_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ func TestDecode(t *testing.T) {
Backend: egv1a1.RateLimitDatabaseBackend{
Type: egv1a1.RedisBackendType,
Redis: &egv1a1.RateLimitRedisSettings{
URL: "localhost:6379",
URL: new("localhost:6379"),
TLS: &egv1a1.RedisTLSSettings{
CertificateRef: &gwapiv1.SecretObjectReference{
Name: "ratelimit-cert",
Expand Down Expand Up @@ -199,7 +199,7 @@ func TestDecode(t *testing.T) {
Backend: egv1a1.RateLimitDatabaseBackend{
Type: egv1a1.RedisBackendType,
Redis: &egv1a1.RateLimitRedisSettings{
URL: "localhost:6379",
URL: new("localhost:6379"),
},
},
},
Expand Down
2 changes: 1 addition & 1 deletion internal/infrastructure/kubernetes/proxy_infra_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ func newTestInfraWithClient(t *testing.T, cli client.Client) *Infra {
Backend: egv1a1.RateLimitDatabaseBackend{
Type: egv1a1.RedisBackendType,
Redis: &egv1a1.RateLimitRedisSettings{
URL: "",
URL: new(""),
TLS: &egv1a1.RedisTLSSettings{
CertificateRef: &gwapiv1.SecretObjectReference{
Name: "ratelimit-cert",
Expand Down
4 changes: 2 additions & 2 deletions internal/infrastructure/kubernetes/ratelimit/resource.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -381,8 +381,8 @@ func expectedRateLimitContainerEnv(rateLimit *egv1a1.RateLimit, rateLimitDeploym
redisURLEnv.ValueFrom = &corev1.EnvVarSource{
SecretKeyRef: rateLimit.Backend.Redis.URLRef.SecretKeyRef,
}
} else {
redisURLEnv.Value = rateLimit.Backend.Redis.URL
} else if rateLimit.Backend.Redis.URL != nil {
redisURLEnv.Value = *rateLimit.Backend.Redis.URL
}
env = append(env, []corev1.EnvVar{
{
Expand Down
42 changes: 21 additions & 21 deletions internal/infrastructure/kubernetes/ratelimit/resource_provider_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ func TestServiceAccount(t *testing.T) {
Backend: egv1a1.RateLimitDatabaseBackend{
Type: egv1a1.RedisBackendType,
Redis: &egv1a1.RateLimitRedisSettings{
URL: "redis.redis.svc:6379",
URL: new("redis.redis.svc:6379"),
},
},
}
Expand All @@ -126,7 +126,7 @@ func TestService(t *testing.T) {
Backend: egv1a1.RateLimitDatabaseBackend{
Type: egv1a1.RedisBackendType,
Redis: &egv1a1.RateLimitRedisSettings{
URL: "redis.redis.svc:6379",
URL: new("redis.redis.svc:6379"),
},
},
},
Expand All @@ -137,7 +137,7 @@ func TestService(t *testing.T) {
Backend: egv1a1.RateLimitDatabaseBackend{
Type: egv1a1.RedisBackendType,
Redis: &egv1a1.RateLimitRedisSettings{
URL: "redis.redis.svc:6379",
URL: new("redis.redis.svc:6379"),
},
},
Telemetry: &egv1a1.RateLimitTelemetry{
Expand Down Expand Up @@ -169,7 +169,7 @@ func TestConfigmap(t *testing.T) {
Backend: egv1a1.RateLimitDatabaseBackend{
Type: egv1a1.RedisBackendType,
Redis: &egv1a1.RateLimitRedisSettings{
URL: "redis.redis.svc:6379",
URL: new("redis.redis.svc:6379"),
},
},
}
Expand All @@ -193,7 +193,7 @@ func TestPDB(t *testing.T) {
Backend: egv1a1.RateLimitDatabaseBackend{
Type: egv1a1.RedisBackendType,
Redis: &egv1a1.RateLimitRedisSettings{
URL: "redis.redis.svc:6379",
URL: new("redis.redis.svc:6379"),
},
},
},
Expand All @@ -205,7 +205,7 @@ func TestPDB(t *testing.T) {
Backend: egv1a1.RateLimitDatabaseBackend{
Type: egv1a1.RedisBackendType,
Redis: &egv1a1.RateLimitRedisSettings{
URL: "redis.redis.svc:6379",
URL: new("redis.redis.svc:6379"),
},
},
},
Expand All @@ -225,7 +225,7 @@ func TestPDB(t *testing.T) {
Backend: egv1a1.RateLimitDatabaseBackend{
Type: egv1a1.RedisBackendType,
Redis: &egv1a1.RateLimitRedisSettings{
URL: "redis.redis.svc:6379",
URL: new("redis.redis.svc:6379"),
},
},
},
Expand Down Expand Up @@ -260,7 +260,7 @@ func TestDeployment(t *testing.T) {
Backend: egv1a1.RateLimitDatabaseBackend{
Type: egv1a1.RedisBackendType,
Redis: &egv1a1.RateLimitRedisSettings{
URL: "redis.redis.svc:6379",
URL: new("redis.redis.svc:6379"),
},
},
}
Expand All @@ -280,7 +280,7 @@ func TestDeployment(t *testing.T) {
Backend: egv1a1.RateLimitDatabaseBackend{
Type: egv1a1.RedisBackendType,
Redis: &egv1a1.RateLimitRedisSettings{
URL: "redis.redis.svc:6379",
URL: new("redis.redis.svc:6379"),
},
},
Telemetry: &egv1a1.RateLimitTelemetry{
Expand Down Expand Up @@ -487,7 +487,7 @@ func TestDeployment(t *testing.T) {
Backend: egv1a1.RateLimitDatabaseBackend{
Type: egv1a1.RedisBackendType,
Redis: &egv1a1.RateLimitRedisSettings{
URL: "redis.redis.svc:6379",
URL: new("redis.redis.svc:6379"),
TLS: &egv1a1.RedisTLSSettings{
CertificateRef: &gwapiv1.SecretObjectReference{
Name: "ratelimit-cert",
Expand Down Expand Up @@ -558,7 +558,7 @@ func TestDeployment(t *testing.T) {
Backend: egv1a1.RateLimitDatabaseBackend{
Type: egv1a1.RedisBackendType,
Redis: &egv1a1.RateLimitRedisSettings{
URL: "redis.redis.svc:6379",
URL: new("redis.redis.svc:6379"),
TLS: &egv1a1.RedisTLSSettings{
CertificateRef: &gwapiv1.SecretObjectReference{
Name: "ratelimit-cert",
Expand Down Expand Up @@ -620,7 +620,7 @@ func TestDeployment(t *testing.T) {
Backend: egv1a1.RateLimitDatabaseBackend{
Type: egv1a1.RedisBackendType,
Redis: &egv1a1.RateLimitRedisSettings{
URL: "redis.redis.svc:6379",
URL: new("redis.redis.svc:6379"),
TLS: &egv1a1.RedisTLSSettings{
CertificateRef: &gwapiv1.SecretObjectReference{
Name: "ratelimit-cert-origin",
Expand Down Expand Up @@ -725,7 +725,7 @@ func TestDeployment(t *testing.T) {
Backend: egv1a1.RateLimitDatabaseBackend{
Type: egv1a1.RedisBackendType,
Redis: &egv1a1.RateLimitRedisSettings{
URL: "redis.redis.svc:6379",
URL: new("redis.redis.svc:6379"),
},
},
Telemetry: &egv1a1.RateLimitTelemetry{
Expand All @@ -743,7 +743,7 @@ func TestDeployment(t *testing.T) {
Backend: egv1a1.RateLimitDatabaseBackend{
Type: egv1a1.RedisBackendType,
Redis: &egv1a1.RateLimitRedisSettings{
URL: "redis.redis.svc:6379",
URL: new("redis.redis.svc:6379"),
},
},
Telemetry: &egv1a1.RateLimitTelemetry{
Expand All @@ -762,7 +762,7 @@ func TestDeployment(t *testing.T) {
Backend: egv1a1.RateLimitDatabaseBackend{
Type: egv1a1.RedisBackendType,
Redis: &egv1a1.RateLimitRedisSettings{
URL: "redis.redis.svc:6379",
URL: new("redis.redis.svc:6379"),
},
},
},
Expand All @@ -784,7 +784,7 @@ func TestDeployment(t *testing.T) {
Backend: egv1a1.RateLimitDatabaseBackend{
Type: egv1a1.RedisBackendType,
Redis: &egv1a1.RateLimitRedisSettings{
URL: "redis.redis.svc:6379",
URL: new("redis.redis.svc:6379"),
},
},
},
Expand Down Expand Up @@ -834,7 +834,7 @@ func TestHorizontalPodAutoscaler(t *testing.T) {
Backend: egv1a1.RateLimitDatabaseBackend{
Type: egv1a1.RedisBackendType,
Redis: &egv1a1.RateLimitRedisSettings{
URL: "redis.redis.svc:6379",
URL: new("redis.redis.svc:6379"),
},
},
},
Expand All @@ -846,7 +846,7 @@ func TestHorizontalPodAutoscaler(t *testing.T) {
Backend: egv1a1.RateLimitDatabaseBackend{
Type: egv1a1.RedisBackendType,
Redis: &egv1a1.RateLimitRedisSettings{
URL: "redis.redis.svc:6379",
URL: new("redis.redis.svc:6379"),
},
},
},
Expand Down Expand Up @@ -883,7 +883,7 @@ func TestHorizontalPodAutoscaler(t *testing.T) {
Backend: egv1a1.RateLimitDatabaseBackend{
Type: egv1a1.RedisBackendType,
Redis: &egv1a1.RateLimitRedisSettings{
URL: "redis.redis.svc:6379",
URL: new("redis.redis.svc:6379"),
},
},
},
Expand All @@ -898,7 +898,7 @@ func TestHorizontalPodAutoscaler(t *testing.T) {
Backend: egv1a1.RateLimitDatabaseBackend{
Type: egv1a1.RedisBackendType,
Redis: &egv1a1.RateLimitRedisSettings{
URL: "redis.redis.svc:6379",
URL: new("redis.redis.svc:6379"),
},
},
},
Expand Down Expand Up @@ -1063,7 +1063,7 @@ func TestValidateRedisSettings(t *testing.T) {
}
c := fakeclient.NewClientBuilder().WithScheme(envoygateway.GetScheme()).WithObjects(certSecret).Build()
require.NoError(t, Validate(context.Background(), c, redisGW(&egv1a1.RateLimitRedisSettings{
URL: "redis.redis.svc:6379",
URL: new("redis.redis.svc:6379"),
TLS: &egv1a1.RedisTLSSettings{
CertificateRef: &gwapiv1.SecretObjectReference{Name: "redis-cert"},
},
Expand Down
4 changes: 2 additions & 2 deletions internal/infrastructure/kubernetes/ratelimit_deployment_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ func TestCreateOrUpdateRateLimitDeployment(t *testing.T) {
Backend: egv1a1.RateLimitDatabaseBackend{
Type: egv1a1.RedisBackendType,
Redis: &egv1a1.RateLimitRedisSettings{
URL: "redis.redis.svc:6379",
URL: new("redis.redis.svc:6379"),
},
},
}
Expand Down Expand Up @@ -106,7 +106,7 @@ func TestDeleteRateLimitDeployment(t *testing.T) {
Backend: egv1a1.RateLimitDatabaseBackend{
Type: egv1a1.RedisBackendType,
Redis: &egv1a1.RateLimitRedisSettings{
URL: "redis.redis.svc:6379",
URL: new("redis.redis.svc:6379"),
},
},
}
Expand Down
2 changes: 1 addition & 1 deletion internal/infrastructure/kubernetes/ratelimit_service_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ func TestDeleteRateLimitService(t *testing.T) {
Backend: egv1a1.RateLimitDatabaseBackend{
Type: egv1a1.RedisBackendType,
Redis: &egv1a1.RateLimitRedisSettings{
URL: "redis.redis.svc:6379",
URL: new("redis.redis.svc:6379"),
},
},
}
Expand Down
4 changes: 2 additions & 2 deletions internal/infrastructure/kubernetes/ratelimit_serviceaccount_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ func TestCreateOrUpdateRateLimitServiceAccount(t *testing.T) {
Backend: egv1a1.RateLimitDatabaseBackend{
Type: egv1a1.RedisBackendType,
Redis: &egv1a1.RateLimitRedisSettings{
URL: "redis.redis.svc:6379",
URL: new("redis.redis.svc:6379"),
},
},
}
Expand Down Expand Up @@ -149,7 +149,7 @@ func TestDeleteRateLimitServiceAccount(t *testing.T) {
Backend: egv1a1.RateLimitDatabaseBackend{
Type: egv1a1.RedisBackendType,
Redis: &egv1a1.RateLimitRedisSettings{
URL: "redis.redis.svc:6379",
URL: new("redis.redis.svc:6379"),
},
},
}
Expand Down
Loading