Skip to content

Commit 79fc5ce

Browse files
wasagawasaga
authored
global config: ignore if a referenced cert not found (#1193)
## Summary <!-- For example... The existing implementation has poor numerical properties for large arguments, so use the McGillicutty algorithm to improve accuracy above 1e10. The algorithm is described at https://wikipedia.org/wiki/McGillicutty_Algorithm --> ## Related issues Fixes #683 ## Checklist - [ ] reference any related issues - [ ] updated docs - [ ] updated unit tests - [ ] updated UPGRADING.md - [ ] add appropriate tag (`improvement` / `bug` / etc) - [ ] ready for review
1 parent c1e8e13 commit 79fc5ce

3 files changed

Lines changed: 219 additions & 5 deletions

File tree

‎controllers/settings/fetch.go‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
corev1 "k8s.io/api/core/v1"
99
"k8s.io/apimachinery/pkg/types"
1010
"sigs.k8s.io/controller-runtime/pkg/client"
11+
"sigs.k8s.io/controller-runtime/pkg/log"
1112

1213
"github.com/pomerium/ingress-controller/model"
1314
"github.com/pomerium/ingress-controller/util"
@@ -43,7 +44,9 @@ func fetchConfigCerts(ctx context.Context, client client.Client, cfg *model.Conf
4344
}
4445
var secret corev1.Secret
4546
if err := client.Get(ctx, *name, &secret); err != nil {
46-
return fmt.Errorf("get %s: %w", name, err)
47+
log := log.FromContext(ctx)
48+
log.Info("certificate secret not found, skipping", "secret", name, "error", err)
49+
continue
4750
}
4851
cfg.Certs[*name] = &secret
4952
}

‎controllers/settings/fetch_test.go‎

Lines changed: 215 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,11 +9,16 @@ import (
99
"go.uber.org/mock/gomock"
1010
"google.golang.org/protobuf/proto"
1111
corev1 "k8s.io/api/core/v1"
12+
"k8s.io/apimachinery/pkg/api/errors"
1213
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
14+
"k8s.io/apimachinery/pkg/runtime"
15+
"k8s.io/apimachinery/pkg/runtime/schema"
1316
"k8s.io/apimachinery/pkg/types"
17+
clientgoscheme "k8s.io/client-go/kubernetes/scheme"
1418
"sigs.k8s.io/controller-runtime/pkg/client"
1519

1620
icsv1 "github.com/pomerium/ingress-controller/apis/ingress/v1"
21+
"github.com/pomerium/ingress-controller/controllers/deps"
1722
controllers_mock "github.com/pomerium/ingress-controller/controllers/mock"
1823
"github.com/pomerium/ingress-controller/controllers/settings"
1924
"github.com/pomerium/ingress-controller/model"
@@ -145,3 +150,213 @@ func TestFetchConstraints(t *testing.T) {
145150
})
146151
}
147152
}
153+
154+
// Related: https://github.com/pomerium/ingress-controller/issues/683
155+
func TestFetchConfigCertsMissingSecrets(t *testing.T) {
156+
t.Parallel()
157+
t.Run("missing certificate secrets should not fail", func(t *testing.T) {
158+
ctx := t.Context()
159+
mc := controllers_mock.NewMockClient(gomock.NewController(t))
160+
settingsName := types.NamespacedName{Namespace: "pomerium", Name: "settings"}
161+
162+
mc.EXPECT().Get(ctx, settingsName,
163+
gomock.AssignableToTypeOf(new(icsv1.Pomerium)),
164+
).Do(func(_ context.Context, _ types.NamespacedName, dst *icsv1.Pomerium, _ ...client.GetOptions) {
165+
*dst = icsv1.Pomerium{
166+
ObjectMeta: metav1.ObjectMeta{
167+
Name: settingsName.Name,
168+
Namespace: settingsName.Namespace,
169+
},
170+
Spec: icsv1.PomeriumSpec{
171+
Authenticate: new(icsv1.Authenticate),
172+
IdentityProvider: &icsv1.IdentityProvider{Secret: "pomerium/idp-secrets"},
173+
Certificates: []string{"pomerium/missing-cert-1", "pomerium/missing-cert-2"},
174+
Secrets: "pomerium/bootstrap-secrets",
175+
},
176+
}
177+
}).Return(nil)
178+
179+
mc.EXPECT().Get(ctx, types.NamespacedName{Namespace: "pomerium", Name: "bootstrap-secrets"},
180+
gomock.AssignableToTypeOf(new(corev1.Secret)),
181+
).Do(func(_ context.Context, name types.NamespacedName, dst *corev1.Secret, _ ...client.GetOption) {
182+
*dst = corev1.Secret{
183+
ObjectMeta: metav1.ObjectMeta{Name: name.Name, Namespace: name.Namespace},
184+
Data: map[string][]byte{},
185+
Type: corev1.SecretTypeOpaque,
186+
}
187+
}).Return(nil)
188+
189+
mc.EXPECT().Get(ctx, types.NamespacedName{Namespace: "pomerium", Name: "idp-secrets"},
190+
gomock.AssignableToTypeOf(new(corev1.Secret)),
191+
).Do(func(_ context.Context, name types.NamespacedName, dst *corev1.Secret, _ ...client.GetOption) {
192+
*dst = corev1.Secret{
193+
ObjectMeta: metav1.ObjectMeta{Name: name.Name, Namespace: name.Namespace},
194+
Data: map[string][]byte{},
195+
Type: corev1.SecretTypeOpaque,
196+
}
197+
}).Return(nil)
198+
199+
mc.EXPECT().Get(ctx, types.NamespacedName{Namespace: "pomerium", Name: "missing-cert-1"},
200+
gomock.AssignableToTypeOf(new(corev1.Secret)),
201+
).Return(errors.NewNotFound(schema.GroupResource{Group: "", Resource: "secrets"}, "missing-cert-1"))
202+
203+
mc.EXPECT().Get(ctx, types.NamespacedName{Namespace: "pomerium", Name: "missing-cert-2"},
204+
gomock.AssignableToTypeOf(new(corev1.Secret)),
205+
).Return(errors.NewNotFound(schema.GroupResource{Group: "", Resource: "secrets"}, "missing-cert-2"))
206+
207+
cfg, err := settings.FetchConfig(ctx, mc, settingsName)
208+
require.NoError(t, err, "FetchConfig should not fail when certificate secrets are missing")
209+
assert.NotNil(t, cfg)
210+
211+
assert.Len(t, cfg.Certs, 0, "No certificates should be loaded when secrets are missing")
212+
213+
assert.NotNil(t, cfg.Secrets, "Bootstrap secrets should be loaded")
214+
assert.NotNil(t, cfg.IdpSecret, "IDP secrets should be loaded")
215+
})
216+
217+
t.Run("partial certificate secrets should load existing ones", func(t *testing.T) {
218+
t.Parallel()
219+
ctx := t.Context()
220+
mc := controllers_mock.NewMockClient(gomock.NewController(t))
221+
settingsName := types.NamespacedName{Namespace: "pomerium", Name: "settings"}
222+
223+
mc.EXPECT().Get(ctx, settingsName,
224+
gomock.AssignableToTypeOf(new(icsv1.Pomerium)),
225+
).Do(func(_ context.Context, _ types.NamespacedName, dst *icsv1.Pomerium, _ ...client.GetOptions) {
226+
*dst = icsv1.Pomerium{
227+
ObjectMeta: metav1.ObjectMeta{
228+
Name: settingsName.Name,
229+
Namespace: settingsName.Namespace,
230+
},
231+
Spec: icsv1.PomeriumSpec{
232+
Authenticate: new(icsv1.Authenticate),
233+
IdentityProvider: &icsv1.IdentityProvider{Secret: "pomerium/idp-secrets"},
234+
Certificates: []string{"pomerium/existing-cert", "pomerium/missing-cert"},
235+
Secrets: "pomerium/bootstrap-secrets",
236+
},
237+
}
238+
}).Return(nil)
239+
240+
mc.EXPECT().Get(ctx, types.NamespacedName{Namespace: "pomerium", Name: "bootstrap-secrets"},
241+
gomock.AssignableToTypeOf(new(corev1.Secret)),
242+
).Do(func(_ context.Context, name types.NamespacedName, dst *corev1.Secret, _ ...client.GetOption) {
243+
*dst = corev1.Secret{
244+
ObjectMeta: metav1.ObjectMeta{Name: name.Name, Namespace: name.Namespace},
245+
Data: map[string][]byte{},
246+
Type: corev1.SecretTypeOpaque,
247+
}
248+
}).Return(nil)
249+
250+
mc.EXPECT().Get(ctx, types.NamespacedName{Namespace: "pomerium", Name: "idp-secrets"},
251+
gomock.AssignableToTypeOf(new(corev1.Secret)),
252+
).Do(func(_ context.Context, name types.NamespacedName, dst *corev1.Secret, _ ...client.GetOption) {
253+
*dst = corev1.Secret{
254+
ObjectMeta: metav1.ObjectMeta{Name: name.Name, Namespace: name.Namespace},
255+
Data: map[string][]byte{},
256+
Type: corev1.SecretTypeOpaque,
257+
}
258+
}).Return(nil)
259+
260+
mc.EXPECT().Get(ctx, types.NamespacedName{Namespace: "pomerium", Name: "existing-cert"},
261+
gomock.AssignableToTypeOf(new(corev1.Secret)),
262+
).Do(func(_ context.Context, name types.NamespacedName, dst *corev1.Secret, _ ...client.GetOption) {
263+
*dst = corev1.Secret{
264+
ObjectMeta: metav1.ObjectMeta{Name: name.Name, Namespace: name.Namespace},
265+
Data: map[string][]byte{
266+
corev1.TLSCertKey: []byte("cert-data"),
267+
corev1.TLSPrivateKeyKey: []byte("key-data"),
268+
},
269+
Type: corev1.SecretTypeTLS,
270+
}
271+
}).Return(nil)
272+
273+
mc.EXPECT().Get(ctx, types.NamespacedName{Namespace: "pomerium", Name: "missing-cert"},
274+
gomock.AssignableToTypeOf(new(corev1.Secret)),
275+
).Return(errors.NewNotFound(schema.GroupResource{Group: "", Resource: "secrets"}, "missing-cert"))
276+
277+
cfg, err := settings.FetchConfig(ctx, mc, settingsName)
278+
require.NoError(t, err, "FetchConfig should not fail with partial certificate secrets")
279+
assert.NotNil(t, cfg)
280+
281+
assert.Len(t, cfg.Certs, 1, "Only existing certificate should be loaded")
282+
existingCertKey := types.NamespacedName{Namespace: "pomerium", Name: "existing-cert"}
283+
assert.Contains(t, cfg.Certs, existingCertKey, "Existing certificate should be in the map")
284+
assert.Equal(t, corev1.SecretTypeTLS, cfg.Certs[existingCertKey].Type, "Certificate should be TLS type")
285+
})
286+
287+
t.Run("missing certificates are tracked as dependencies for automatic pickup", func(t *testing.T) {
288+
t.Parallel()
289+
ctx := t.Context()
290+
mc := controllers_mock.NewMockClient(gomock.NewController(t))
291+
settingsName := types.NamespacedName{Namespace: "pomerium", Name: "settings"}
292+
293+
scheme := runtime.NewScheme()
294+
require.NoError(t, clientgoscheme.AddToScheme(scheme))
295+
require.NoError(t, icsv1.AddToScheme(scheme))
296+
mc.EXPECT().Scheme().Return(scheme).AnyTimes()
297+
298+
registry := model.NewRegistry()
299+
settingsKey := model.Key{
300+
Kind: "Pomerium",
301+
NamespacedName: settingsName,
302+
}
303+
304+
trackingClient := deps.NewClient(mc, registry, settingsKey)
305+
306+
mc.EXPECT().Get(ctx, settingsName,
307+
gomock.AssignableToTypeOf(new(icsv1.Pomerium)),
308+
).Do(func(_ context.Context, _ types.NamespacedName, dst *icsv1.Pomerium, _ ...client.GetOptions) {
309+
*dst = icsv1.Pomerium{
310+
ObjectMeta: metav1.ObjectMeta{
311+
Name: settingsName.Name,
312+
Namespace: settingsName.Namespace,
313+
},
314+
Spec: icsv1.PomeriumSpec{
315+
Authenticate: new(icsv1.Authenticate),
316+
IdentityProvider: &icsv1.IdentityProvider{Secret: "pomerium/idp-secrets"},
317+
Certificates: []string{"pomerium/future-cert"},
318+
Secrets: "pomerium/bootstrap-secrets",
319+
},
320+
}
321+
}).Return(nil)
322+
323+
mc.EXPECT().Get(ctx, types.NamespacedName{Namespace: "pomerium", Name: "bootstrap-secrets"},
324+
gomock.AssignableToTypeOf(new(corev1.Secret)),
325+
).Do(func(_ context.Context, name types.NamespacedName, dst *corev1.Secret, _ ...client.GetOption) {
326+
*dst = corev1.Secret{
327+
ObjectMeta: metav1.ObjectMeta{Name: name.Name, Namespace: name.Namespace},
328+
Data: map[string][]byte{},
329+
Type: corev1.SecretTypeOpaque,
330+
}
331+
}).Return(nil)
332+
333+
mc.EXPECT().Get(ctx, types.NamespacedName{Namespace: "pomerium", Name: "idp-secrets"},
334+
gomock.AssignableToTypeOf(new(corev1.Secret)),
335+
).Do(func(_ context.Context, name types.NamespacedName, dst *corev1.Secret, _ ...client.GetOption) {
336+
*dst = corev1.Secret{
337+
ObjectMeta: metav1.ObjectMeta{Name: name.Name, Namespace: name.Namespace},
338+
Data: map[string][]byte{},
339+
Type: corev1.SecretTypeOpaque,
340+
}
341+
}).Return(nil)
342+
343+
mc.EXPECT().Get(ctx, types.NamespacedName{Namespace: "pomerium", Name: "future-cert"},
344+
gomock.AssignableToTypeOf(new(corev1.Secret)),
345+
).Return(errors.NewNotFound(schema.GroupResource{Group: "", Resource: "secrets"}, "future-cert"))
346+
347+
_, err := settings.FetchConfig(ctx, trackingClient, settingsName)
348+
require.NoError(t, err, "FetchConfig should not fail when certificate secrets are missing")
349+
350+
deps := registry.Deps(settingsKey)
351+
352+
var foundFutureCert bool
353+
for _, dep := range deps {
354+
if dep.Kind == "Secret" && dep.NamespacedName.Name == "future-cert" && dep.NamespacedName.Namespace == "pomerium" {
355+
foundFutureCert = true
356+
break
357+
}
358+
}
359+
360+
assert.True(t, foundFutureCert, "future-cert should be tracked as a dependency so controller will reconcile when it becomes available")
361+
})
362+
}

‎pomerium/config.go‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -253,10 +253,6 @@ func applyCertificateAuthority(_ context.Context, p *pb.Config, c *model.Config)
253253
}
254254

255255
func applyCerts(_ context.Context, p *pb.Config, c *model.Config) error {
256-
if len(c.Certs) != len(c.Spec.Certificates) {
257-
return fmt.Errorf("expected %d cert secrets, only %d was fetched. this is a bug", len(c.Spec.Certificates), len(c.Certs))
258-
}
259-
260256
for _, secret := range c.Certs {
261257
if secret.Type != corev1.SecretTypeTLS {
262258
return fmt.Errorf("%s expected secret type %s, got %s", util.GetNamespacedName(secret), corev1.SecretTypeTLS, secret.Type)

0 commit comments

Comments
 (0)