Skip to content

Commit 643f059

Browse files
fix(sync): avoid O(n^2) envoy validation during initial sync
DataBrokerReconciler.Set() validated the full merged config with the embedded envoy subprocess once per ingress inside the merge loop, so an initial sync of N ingresses performed N full-config validations of a growing config (plus a final one in saveConfig). With 300+ ingresses this made pod-restart resync take over an hour. Build the merged config using only cheap, in-process policy/option validation and let saveConfig perform the single full envoy validation. If that batch validation fails, fall back to the previous incremental per-ingress full validation to isolate and skip only the offending ingress(es). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 82da61f commit 643f059

3 files changed

Lines changed: 172 additions & 7 deletions

File tree

pomerium/sync_databroker.go

Lines changed: 46 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -107,8 +107,53 @@ func (r *DataBrokerReconciler) Set(ctx context.Context, ics []*model.IngressConf
107107
if err != nil {
108108
return false, fmt.Errorf("get config: %w", err)
109109
}
110+
111+
// Build the merged config using only cheap (non-envoy) per-ingress validation.
112+
// The expensive full-config envoy validation runs a single time, in saveConfig.
113+
// This avoids the O(n^2) behavior of validating a growing config once per ingress.
114+
next := buildConfigCheap(ctx, ics)
115+
116+
changed, err := r.saveConfig(ctx, prev, next, r.ConfigID)
117+
if err == nil {
118+
return changed, nil
119+
}
120+
121+
// The single full-config envoy validation failed. Rather than reject every
122+
// ingress, fall back to validating ingresses incrementally to isolate and skip
123+
// only the offending one(s). This path performs O(n) envoy validations, but is
124+
// only reached when a batch fails to validate as a whole.
125+
logger.Error(err, "batch config validation failed; falling back to incremental validation to isolate invalid ingress(es)")
126+
next = r.buildConfigIncremental(ctx, ics)
127+
return r.saveConfig(ctx, prev, next, r.ConfigID)
128+
}
129+
130+
// buildConfigCheap merges the routes for all ingresses into a single config, using
131+
// only cheap (non-envoy) validation to skip individually-invalid ingresses.
132+
func buildConfigCheap(ctx context.Context, ics []*model.IngressConfig) *pb.Config {
133+
logger := log.FromContext(ctx)
110134
next := new(pb.Config)
135+
for _, ic := range ics {
136+
cfg := proto.Clone(next).(*pb.Config)
137+
if err := multierror.Append(
138+
upsertRoutes(ctx, cfg, ic),
139+
validateCheap(ctx, cfg),
140+
).ErrorOrNil(); err != nil {
141+
logger.Error(err, "skip ingress", "ingress", fmt.Sprintf("%s/%s", ic.Namespace, ic.Name))
142+
continue
143+
}
144+
addCerts(cfg, ic.Secrets)
145+
next = cfg
146+
}
147+
return next
148+
}
111149

150+
// buildConfigIncremental merges the routes for all ingresses into a single config,
151+
// running the full envoy validation after each ingress so that an ingress which
152+
// only fails full validation (not cheap validation) is isolated and skipped. This
153+
// is the pre-caching behavior, retained as a fallback for the rare batch-failure case.
154+
func (r *DataBrokerReconciler) buildConfigIncremental(ctx context.Context, ics []*model.IngressConfig) *pb.Config {
155+
logger := log.FromContext(ctx)
156+
next := new(pb.Config)
112157
for _, ic := range ics {
113158
cfg := proto.Clone(next).(*pb.Config)
114159
if err := multierror.Append(
@@ -121,8 +166,7 @@ func (r *DataBrokerReconciler) Set(ctx context.Context, ics []*model.IngressConf
121166
addCerts(cfg, ic.Secrets)
122167
next = cfg
123168
}
124-
125-
return r.saveConfig(ctx, prev, next, r.ConfigID)
169+
return next
126170
}
127171

128172
// SetConfig updates just the shared config settings

pomerium/sync_test.go

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,101 @@
1+
package pomerium
2+
3+
import (
4+
"context"
5+
"testing"
6+
7+
"github.com/stretchr/testify/assert"
8+
"github.com/stretchr/testify/require"
9+
corev1 "k8s.io/api/core/v1"
10+
networkingv1 "k8s.io/api/networking/v1"
11+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
12+
"k8s.io/apimachinery/pkg/types"
13+
"k8s.io/apimachinery/pkg/util/intstr"
14+
15+
"github.com/pomerium/ingress-controller/model"
16+
17+
_ "github.com/pomerium/ingress-controller/internal"
18+
)
19+
20+
// validIngressConfig returns a minimal, valid IngressConfig for the given name/host.
21+
func validIngressConfig(name, host string) *model.IngressConfig {
22+
pathTypePrefix := networkingv1.PathTypePrefix
23+
return &model.IngressConfig{
24+
AnnotationPrefix: "ingress.pomerium.io",
25+
Ingress: &networkingv1.Ingress{
26+
ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: "default"},
27+
Spec: networkingv1.IngressSpec{
28+
Rules: []networkingv1.IngressRule{{
29+
Host: host,
30+
IngressRuleValue: networkingv1.IngressRuleValue{
31+
HTTP: &networkingv1.HTTPIngressRuleValue{
32+
Paths: []networkingv1.HTTPIngressPath{{
33+
Path: "/",
34+
PathType: &pathTypePrefix,
35+
Backend: networkingv1.IngressBackend{
36+
Service: &networkingv1.IngressServiceBackend{
37+
Name: "service",
38+
Port: networkingv1.ServiceBackendPort{Name: "http"},
39+
},
40+
},
41+
}},
42+
},
43+
},
44+
}},
45+
},
46+
},
47+
Services: map[types.NamespacedName]*corev1.Service{
48+
{Name: "service", Namespace: "default"}: {
49+
ObjectMeta: metav1.ObjectMeta{Name: "service", Namespace: "default"},
50+
Spec: corev1.ServiceSpec{
51+
Ports: []corev1.ServicePort{{
52+
Name: "http",
53+
Protocol: corev1.ProtocolTCP,
54+
Port: 80,
55+
TargetPort: intstr.FromInt(80),
56+
}},
57+
},
58+
},
59+
},
60+
}
61+
}
62+
63+
// invalidIngressConfig returns an IngressConfig that fails route generation
64+
// (empty host without the allow-empty-host annotation).
65+
func invalidIngressConfig(name string) *model.IngressConfig {
66+
ic := validIngressConfig(name, "")
67+
return ic
68+
}
69+
70+
func TestBuildConfigCheap_AllValid(t *testing.T) {
71+
ctx := context.Background()
72+
ics := []*model.IngressConfig{
73+
validIngressConfig("a", "a.localhost.pomerium.io"),
74+
validIngressConfig("b", "b.localhost.pomerium.io"),
75+
}
76+
77+
cfg := buildConfigCheap(ctx, ics)
78+
require.NotNil(t, cfg)
79+
assert.Len(t, cfg.GetRoutes(), 2, "both valid ingresses should produce a route")
80+
}
81+
82+
func TestBuildConfigCheap_SkipsInvalid(t *testing.T) {
83+
ctx := context.Background()
84+
ics := []*model.IngressConfig{
85+
validIngressConfig("a", "a.localhost.pomerium.io"),
86+
invalidIngressConfig("bad"), // empty host -> route generation fails
87+
validIngressConfig("c", "c.localhost.pomerium.io"),
88+
}
89+
90+
cfg := buildConfigCheap(ctx, ics)
91+
require.NotNil(t, cfg)
92+
// The invalid ingress is skipped; the two valid ones remain.
93+
assert.Len(t, cfg.GetRoutes(), 2, "invalid ingress should be skipped, valid ones kept")
94+
95+
hosts := map[string]bool{}
96+
for _, r := range cfg.GetRoutes() {
97+
hosts[r.GetFrom()] = true
98+
}
99+
assert.True(t, hosts["https://a.localhost.pomerium.io"], "route a should be present")
100+
assert.True(t, hosts["https://c.localhost.pomerium.io"], "route c should be present")
101+
}

pomerium/validate.go

Lines changed: 25 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -15,25 +15,45 @@ import (
1515
"github.com/pomerium/ingress-controller/pomerium/envoy"
1616
)
1717

18-
// validate validates pomerium config.
19-
func validate(ctx context.Context, cfg *pb.Config, id string) error {
18+
// validateOptions builds and validates pomerium options from the config without
19+
// invoking the (expensive) embedded envoy subprocess. It catches route/policy and
20+
// option-level errors cheaply.
21+
func validateOptions(ctx context.Context, cfg *pb.Config) (*config.Options, error) {
2022
options := config.NewDefaultOptions()
2123
options.ApplySettings(ctx, cryptutil.NewCertificatesIndex(), cfg.GetSettings())
2224
options.InsecureServer = true
2325

2426
for _, r := range cfg.GetRoutes() {
2527
p, err := config.NewPolicyFromProto(r)
2628
if err != nil {
27-
return err
29+
return nil, err
2830
}
2931
err = p.Validate()
3032
if err != nil {
31-
return err
33+
return nil, err
3234
}
3335
options.Policies = append(options.Policies, *p)
3436
}
3537

36-
err := options.Validate()
38+
if err := options.Validate(); err != nil {
39+
return nil, err
40+
}
41+
42+
return options, nil
43+
}
44+
45+
// validateCheap validates a pomerium config without invoking the embedded envoy
46+
// subprocess. It is used to catch most invalid ingresses quickly, deferring the
47+
// single expensive full-config envoy validation to saveConfig.
48+
func validateCheap(ctx context.Context, cfg *pb.Config) error {
49+
_, err := validateOptions(ctx, cfg)
50+
return err
51+
}
52+
53+
// validate validates pomerium config, including a full bootstrap validation via
54+
// the embedded envoy binary.
55+
func validate(ctx context.Context, cfg *pb.Config, id string) error {
56+
options, err := validateOptions(ctx, cfg)
3757
if err != nil {
3858
return err
3959
}

0 commit comments

Comments
 (0)