Skip to content

Commit 628aa42

Browse files
authored
config: sort certs to prevent churn (#1246)
## Summary sort certs in the config, to prevent configuration churn. ## Related issues Ref: #1244 ## Checklist - [x] reference any related issues - [ ] updated docs - [x] updated unit tests - [ ] updated UPGRADING.md - [x] add appropriate tag (`improvement` / `bug` / etc) - [x] ready for review
1 parent 2b63483 commit 628aa42

3 files changed

Lines changed: 148 additions & 4 deletions

File tree

pomerium/deterministic.go

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
package pomerium
2+
3+
import (
4+
"cmp"
5+
"slices"
6+
"sort"
7+
8+
pb "github.com/pomerium/pomerium/pkg/grpc/config"
9+
)
10+
11+
func ensureDeterministicConfigOrder(cfg *pb.Config) {
12+
if cfg == nil {
13+
return
14+
}
15+
// https://kubernetes.io/docs/concepts/services-networking/ingress/#multiple-matches
16+
// envoy matches according to the order routes are present in the configuration
17+
sort.Sort(routeList(cfg.Routes))
18+
19+
if len(cfg.GetSettings().GetCertificates()) > 0 {
20+
slices.SortFunc(cfg.Settings.Certificates, func(a, b *pb.Settings_Certificate) int {
21+
return cmp.Compare(a.Id, b.Id)
22+
})
23+
}
24+
}

pomerium/deterministic_test.go

Lines changed: 123 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,123 @@
1+
package pomerium
2+
3+
import (
4+
"testing"
5+
6+
"github.com/google/go-cmp/cmp"
7+
"google.golang.org/protobuf/proto"
8+
"google.golang.org/protobuf/testing/protocmp"
9+
10+
configpb "github.com/pomerium/pomerium/pkg/grpc/config"
11+
)
12+
13+
func TestEnsureDeterministicConfigOrder(t *testing.T) {
14+
t.Parallel()
15+
16+
testCases := []struct {
17+
name string
18+
cfg *configpb.Config
19+
want *configpb.Config
20+
}{
21+
{
22+
name: "sorts routes and certificates",
23+
cfg: &configpb.Config{
24+
Routes: []*configpb.Route{
25+
{Name: "route-b", From: "https://b.example.com"},
26+
{Name: "route-a", From: "https://a.example.com"},
27+
},
28+
Settings: &configpb.Settings{
29+
Certificates: []*configpb.Settings_Certificate{
30+
{Id: "cert-b"},
31+
{Id: "cert-a"},
32+
},
33+
},
34+
},
35+
want: &configpb.Config{
36+
Routes: []*configpb.Route{
37+
{Name: "route-a", From: "https://a.example.com"},
38+
{Name: "route-b", From: "https://b.example.com"},
39+
},
40+
Settings: &configpb.Settings{
41+
Certificates: []*configpb.Settings_Certificate{
42+
{Id: "cert-a"},
43+
{Id: "cert-b"},
44+
},
45+
},
46+
},
47+
},
48+
{
49+
name: "sorts routes with identical hosts",
50+
cfg: &configpb.Config{
51+
Routes: []*configpb.Route{
52+
{
53+
Name: "route-root",
54+
From: "https://a.example.com",
55+
Path: "/",
56+
},
57+
{
58+
Name: "route-deep",
59+
From: "https://a.example.com",
60+
Path: "/nested",
61+
},
62+
},
63+
},
64+
want: &configpb.Config{
65+
Routes: []*configpb.Route{
66+
{
67+
Name: "route-deep",
68+
From: "https://a.example.com",
69+
Path: "/nested",
70+
},
71+
{
72+
Name: "route-root",
73+
From: "https://a.example.com",
74+
Path: "/",
75+
},
76+
},
77+
},
78+
},
79+
{
80+
name: "leaves sorted config untouched",
81+
cfg: &configpb.Config{
82+
Routes: []*configpb.Route{
83+
{Name: "route-a", From: "https://a.example.com"},
84+
},
85+
Settings: &configpb.Settings{
86+
Certificates: []*configpb.Settings_Certificate{
87+
{Id: "cert-a"},
88+
},
89+
},
90+
},
91+
want: &configpb.Config{
92+
Routes: []*configpb.Route{
93+
{Name: "route-a", From: "https://a.example.com"},
94+
},
95+
Settings: &configpb.Settings{
96+
Certificates: []*configpb.Settings_Certificate{
97+
{Id: "cert-a"},
98+
},
99+
},
100+
},
101+
},
102+
}
103+
104+
for _, tc := range testCases {
105+
t.Run(tc.name, func(t *testing.T) {
106+
t.Parallel()
107+
108+
cfg := proto.Clone(tc.cfg).(*configpb.Config)
109+
ensureDeterministicConfigOrder(cfg)
110+
111+
if diff := cmp.Diff(tc.want, cfg, protocmp.Transform()); diff != "" {
112+
t.Fatalf("unexpected config (-want +got):\n%s", diff)
113+
}
114+
115+
again := proto.Clone(cfg).(*configpb.Config)
116+
ensureDeterministicConfigOrder(again)
117+
118+
if diff := cmp.Diff(cfg, again, protocmp.Transform()); diff != "" {
119+
t.Fatalf("ensureDeterministicConfigOrder not idempotent (-first +second):\n%s", diff)
120+
}
121+
})
122+
}
123+
}

pomerium/sync.go

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ package pomerium
44
import (
55
"context"
66
"fmt"
7-
"sort"
87

98
"github.com/hashicorp/go-multierror"
109
"github.com/sergi/go-diff/diffmatchpatch"
@@ -194,9 +193,7 @@ func (r *DataBrokerReconciler) saveConfig(ctx context.Context, prev, next *pb.Co
194193
}
195194
}
196195

197-
// https://kubernetes.io/docs/concepts/services-networking/ingress/#multiple-matches
198-
// envoy matches according to the order routes are present in the configuration
199-
sort.Sort(routeList(next.Routes))
196+
ensureDeterministicConfigOrder(next)
200197

201198
if err := validate(ctx, next, id); err != nil {
202199
return false, fmt.Errorf("config validation: %w", err)

0 commit comments

Comments
 (0)