Skip to content

Commit 691242e

Browse files
authored
rework settings sync to apply Core defaults (#1391)
When syncing settings via the new API we should apply any default values explicitly, as they may not be inferred by the API implementation. Also remove the hard-coded default authenticate service URL, as this should no longer be necessary.
1 parent fb0d004 commit 691242e

3 files changed

Lines changed: 43 additions & 30 deletions

File tree

pomerium/config.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -273,7 +273,6 @@ func applyCerts(_ context.Context, p *pb.Config, c *model.Config) error {
273273

274274
func applyAuthenticate(_ context.Context, p *pb.Config, c *model.Config) error {
275275
if c.Spec.Authenticate == nil {
276-
p.Settings.AuthenticateServiceUrl = proto.String("https://authenticate.pomerium.app")
277276
return nil
278277
}
279278

pomerium/sync_api.go

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import (
2323
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
2424
"sigs.k8s.io/controller-runtime/pkg/log"
2525

26+
"github.com/pomerium/pomerium/config"
2627
pb "github.com/pomerium/pomerium/pkg/grpc/config"
2728
"github.com/pomerium/sdk-go"
2829
"github.com/pomerium/sdk-go/proto/pomerium"
@@ -341,7 +342,11 @@ func (r *APIReconciler) SetConfig(ctx context.Context, cfg *model.Config) (chang
341342
pbConfig.Settings.Certificates = nil
342343
pbConfig.Settings.CertificateAuthority = nil
343344

344-
settings, err := convertProto[*pomerium.Settings](pbConfig.Settings)
345+
// Apply all Core defaults.
346+
mergedSettings := config.NewDefaultOptions()
347+
mergedSettings.ApplySettings(ctx, nil, pbConfig.Settings)
348+
349+
settings, err := convertProto[*pomerium.Settings](mergedSettings.ToProto().GetSettings())
345350
if err != nil {
346351
return false, err
347352
}
@@ -350,18 +355,13 @@ func (r *APIReconciler) SetConfig(ctx context.Context, cfg *model.Config) (chang
350355
if err != nil {
351356
return false, err
352357
}
353-
354-
// Mask any settings that cannot be set via the Pomerium CRD
355358
existing := resp.Msg.Settings
356-
existing.Address = nil
357-
existing.Autocert = nil
358-
existing.ClusterId = nil
359-
existing.GrpcAddress = nil
360-
existing.GrpcInsecure = nil
361-
existing.Id = nil
362-
existing.InsecureServer = nil
363-
existing.NamespaceId = nil
364-
existing.SharedSecret = nil
359+
360+
// Preserve any settings that cannot be set via the Pomerium CRD.
361+
settings.Id = existing.Id
362+
settings.AutoApplyChangesets = existing.AutoApplyChangesets
363+
settings.AutocertDir = existing.AutocertDir
364+
settings.RuntimeFlags = existing.RuntimeFlags
365365

366366
// Mask timestamp metadata.
367367
existing.CreatedAt = nil

pomerium/sync_api_test.go

Lines changed: 31 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import (
2121
"sigs.k8s.io/controller-runtime/pkg/client"
2222
gateway_v1 "sigs.k8s.io/gateway-api/apis/v1"
2323

24+
"github.com/pomerium/pomerium/config"
2425
"github.com/pomerium/sdk-go/proto/pomerium"
2526

2627
icgv1alpha1 "github.com/pomerium/ingress-controller/apis/gateway/v1alpha1"
@@ -1028,6 +1029,9 @@ func TestAPIReconciler_SetConfig(t *testing.T) {
10281029
},
10291030
}
10301031

1032+
defaultSettings, err := convertProto[*pomerium.Settings](config.NewDefaultOptions().ToProto().GetSettings())
1033+
require.NoError(t, err)
1034+
10311035
t.Run("settings changed", func(t *testing.T) {
10321036
apiClient, _, r := setupReconciler(t)
10331037
ctx := t.Context()
@@ -1042,16 +1046,20 @@ func TestAPIReconciler_SetConfig(t *testing.T) {
10421046
},
10431047
}, nil)
10441048

1049+
expectedSettings := proto.CloneOf(defaultSettings)
1050+
expectedSettings.Id = new("settings-id-123")
1051+
expectedSettings.AuthenticateServiceUrl = new("https://authenticate.localhost.pomerium.io")
1052+
expectedSettings.AutocertDir = nil
1053+
expectedSettings.IdpClientId = new("CLIENT_ID")
1054+
expectedSettings.IdpClientSecret = new("CLIENT_SECRET")
1055+
expectedSettings.IdpProvider = new("oidc")
1056+
expectedSettings.IdpProviderUrl = new("https://idp.example.com")
1057+
expectedSettings.PassIdentityHeaders = new(true)
1058+
expectedSettings.RuntimeFlags = nil
1059+
10451060
// ...and then call UpdateSettings() once it knows there are changes to sync.
10461061
apiClient.EXPECT().UpdateSettings(ctx, RequestEq(&pomerium.UpdateSettingsRequest{
1047-
Settings: &pomerium.Settings{
1048-
AuthenticateServiceUrl: new("https://authenticate.localhost.pomerium.io"),
1049-
IdpClientId: new("CLIENT_ID"),
1050-
IdpClientSecret: new("CLIENT_SECRET"),
1051-
IdpProvider: new("oidc"),
1052-
IdpProviderUrl: new("https://idp.example.com"),
1053-
PassIdentityHeaders: new(true),
1054-
},
1062+
Settings: expectedSettings,
10551063
})).Return(&connect.Response[pomerium.UpdateSettingsResponse]{
10561064
Msg: &pomerium.UpdateSettingsResponse{},
10571065
}, nil)
@@ -1065,19 +1073,25 @@ func TestAPIReconciler_SetConfig(t *testing.T) {
10651073
apiClient, _, r := setupReconciler(t)
10661074
ctx := t.Context()
10671075

1076+
existingSettings := proto.CloneOf(defaultSettings)
1077+
proto.Merge(existingSettings, &pomerium.Settings{
1078+
Id: new("settings-id-123"),
1079+
1080+
AuthenticateServiceUrl: new("https://authenticate.localhost.pomerium.io"),
1081+
IdpClientId: new("CLIENT_ID"),
1082+
IdpClientSecret: new("CLIENT_SECRET"),
1083+
IdpProvider: new("oidc"),
1084+
IdpProviderUrl: new("https://idp.example.com"),
1085+
PassIdentityHeaders: new(true),
1086+
1087+
AutoApplyChangesets: new(true), // this setting should be ignored
1088+
})
1089+
10681090
// If the settings already match, there should be no UpdateSettings() call.
10691091
apiClient.EXPECT().GetSettings(ctx, connect.NewRequest(&pomerium.GetSettingsRequest{})).
10701092
Return(&connect.Response[pomerium.GetSettingsResponse]{
10711093
Msg: &pomerium.GetSettingsResponse{
1072-
Settings: &pomerium.Settings{
1073-
Id: new("settings-id-123"),
1074-
1075-
AuthenticateServiceUrl: new("https://authenticate.localhost.pomerium.io"),
1076-
IdpClientId: new("CLIENT_ID"),
1077-
IdpClientSecret: new("CLIENT_SECRET"),
1078-
IdpProvider: new("oidc"),
1079-
IdpProviderUrl: new("https://idp.example.com"),
1080-
PassIdentityHeaders: new(true)},
1094+
Settings: existingSettings,
10811095
},
10821096
}, nil)
10831097

0 commit comments

Comments
 (0)