Skip to content

Commit 16e1a38

Browse files
stuggistuggi
committed
[tls] ValidateCertSecret to not return ctrlResult
ValidateCertSecrets miss information on which cert secret is missing. Lets not return a crtlResult in case of a cert secret is missing, instead return a NotFound error and let the caller decide what to do. Jira: OSPRH-9991 Signed-off-by: Martin Schuppert <mschuppert@redhat.com>
1 parent 6e6f2bd commit 16e1a38

2 files changed

Lines changed: 39 additions & 48 deletions

File tree

‎modules/common/test/functional/tls_test.go‎

Lines changed: 8 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@ import (
2222
"github.com/openstack-k8s-operators/lib-common/modules/common/service"
2323
"github.com/openstack-k8s-operators/lib-common/modules/common/tls"
2424
"k8s.io/apimachinery/pkg/types"
25-
ctrl "sigs.k8s.io/controller-runtime"
2625
)
2726

2827
var _ = Describe("tls package", func() {
@@ -48,15 +47,13 @@ var _ = Describe("tls package", func() {
4847
th.CreateEmptySecret(sname)
4948

5049
// validate bad ca cert secret
51-
_, ctrlResult, err := tls.ValidateCACertSecret(th.Ctx, cClient, sname)
50+
_, err := tls.ValidateCACertSecret(th.Ctx, cClient, sname)
5251
Expect(err).To(HaveOccurred())
53-
Expect(ctrlResult).To(BeIdenticalTo(ctrl.Result{}))
5452

5553
// update ca cert secret with good data
5654
th.UpdateSecret(sname, tls.CABundleKey, []byte("foo"))
57-
hash, ctrlResult, err := tls.ValidateCACertSecret(th.Ctx, cClient, sname)
55+
hash, err := tls.ValidateCACertSecret(th.Ctx, cClient, sname)
5856
Expect(err).ShouldNot(HaveOccurred())
59-
Expect(ctrlResult).To(BeIdenticalTo(ctrl.Result{}))
6057
Expect(hash).To(BeIdenticalTo("n56fh645hfbh687hc9h678h87h64bh598h577hch5d6h5c9h5d4h74h84h5f4hfch6dh678h547h9bhbchb6h89h5c4h68dhc9h664h557h595h5c5q"))
6158
})
6259

@@ -73,24 +70,21 @@ var _ = Describe("tls package", func() {
7370
s := &tls.Service{
7471
SecretName: sname.Name,
7572
}
76-
_, ctrlResult, err := s.ValidateCertSecret(th.Ctx, h, namespace)
73+
_, err := s.ValidateCertSecret(th.Ctx, h, namespace)
7774
Expect(err).To(HaveOccurred())
78-
Expect(ctrlResult).To(BeIdenticalTo(ctrl.Result{}))
7975

8076
// update cert secret with cert, still key missing
8177
th.UpdateSecret(sname, tls.CertKey, []byte("cert"))
82-
_, ctrlResult, err = s.ValidateCertSecret(th.Ctx, h, namespace)
78+
_, err = s.ValidateCertSecret(th.Ctx, h, namespace)
8379
Expect(err).To(HaveOccurred())
8480
Expect(err.Error()).To(ContainSubstring("field tls.key not found in Secret"))
85-
Expect(ctrlResult).To(BeIdenticalTo(ctrl.Result{}))
8681

8782
// update cert secret with key to be a good cert secret
8883
th.UpdateSecret(sname, tls.PrivateKey, []byte("key"))
8984

9085
// validate good cert secret
91-
hash, ctrlResult, err := s.ValidateCertSecret(th.Ctx, h, namespace)
86+
hash, err := s.ValidateCertSecret(th.Ctx, h, namespace)
9287
Expect(err).ShouldNot(HaveOccurred())
93-
Expect(ctrlResult).To(BeIdenticalTo(ctrl.Result{}))
9488
Expect(hash).To(BeIdenticalTo("n547h97h5cfh587h56ch594h79hd4h96h5cfh565h587h569h688h666h685h67ch7fhfbh664h5f9h694h564h9ch645h675h665h78h7h87h566hb6q"))
9589
})
9690

@@ -107,9 +101,8 @@ var _ = Describe("tls package", func() {
107101
endpointCfgs := map[service.Endpoint]tls.Service{}
108102

109103
// validate empty service map
110-
_, ctrlResult, err := tls.ValidateEndpointCerts(th.Ctx, h, namespace, endpointCfgs)
104+
_, err := tls.ValidateEndpointCerts(th.Ctx, h, namespace, endpointCfgs)
111105
Expect(err).ToNot(HaveOccurred())
112-
Expect(ctrlResult).To(BeIdenticalTo(ctrl.Result{}))
113106

114107
endpointCfgs[service.EndpointInternal] = tls.Service{
115108
SecretName: sname.Name,
@@ -119,18 +112,16 @@ var _ = Describe("tls package", func() {
119112
}
120113

121114
// validate service map with bad cert secret
122-
_, ctrlResult, err = tls.ValidateEndpointCerts(th.Ctx, h, namespace, endpointCfgs)
115+
_, err = tls.ValidateEndpointCerts(th.Ctx, h, namespace, endpointCfgs)
123116
Expect(err).To(HaveOccurred())
124117
Expect(err.Error()).To(ContainSubstring("field tls.crt not found in Secret"))
125-
Expect(ctrlResult).To(BeIdenticalTo(ctrl.Result{}))
126118

127119
// update cert secret to have missing private key
128120
th.UpdateSecret(sname, tls.CertKey, []byte("cert"))
129121

130122
// validate service map with good cert secret
131-
hash, ctrlResult, err := tls.ValidateEndpointCerts(th.Ctx, h, namespace, endpointCfgs)
123+
hash, err := tls.ValidateEndpointCerts(th.Ctx, h, namespace, endpointCfgs)
132124
Expect(err).ShouldNot(HaveOccurred())
133-
Expect(ctrlResult).To(BeIdenticalTo(ctrl.Result{}))
134125
Expect(hash).To(BeIdenticalTo("n5d7h65dh5d5h569hffh66ch568h95h686h58fhcfh586h5b8hc6hd7h65bh56bh55bh656hfh5f7h84h54bh65dh5c9h8ch64bh64bhdfh8ch589h54bq"))
135126
})
136127
})

‎modules/common/tls/tls.go‎

Lines changed: 31 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -29,12 +29,13 @@ import (
2929
"github.com/openstack-k8s-operators/lib-common/modules/common/secret"
3030
"github.com/openstack-k8s-operators/lib-common/modules/common/service"
3131
"github.com/openstack-k8s-operators/lib-common/modules/common/util"
32+
appsv1 "k8s.io/api/apps/v1"
3233
corev1 "k8s.io/api/core/v1"
34+
k8s_errors "k8s.io/apimachinery/pkg/api/errors"
3335
"k8s.io/apimachinery/pkg/types"
3436
"k8s.io/utils/ptr"
3537
ctrl "sigs.k8s.io/controller-runtime"
3638
"sigs.k8s.io/controller-runtime/pkg/client"
37-
"sigs.k8s.io/controller-runtime/pkg/reconcile"
3839
)
3940

4041
const (
@@ -167,7 +168,7 @@ func (a *APIService) ValidateCertSecrets(
167168
ctx context.Context,
168169
h *helper.Helper,
169170
namespace string,
170-
) (string, ctrl.Result, error) {
171+
) (string, error) {
171172
var svc GenericService
172173
certHashes := map[string]env.Setter{}
173174
for _, endpt := range []service.Endpoint{service.EndpointInternal, service.EndpointPublic} {
@@ -187,20 +188,18 @@ func (a *APIService) ValidateCertSecrets(
187188
svc = a.Internal
188189
}
189190

190-
hash, ctrlResult, err := svc.ValidateCertSecret(ctx, h, namespace)
191+
hash, err := svc.ValidateCertSecret(ctx, h, namespace)
191192
if err != nil {
192-
return "", ctrlResult, err
193-
} else if (ctrlResult != ctrl.Result{}) {
194-
return "", ctrlResult, nil
193+
return "", err
195194
}
196195
certHashes["cert-"+endpt.String()] = env.SetValue(hash)
197196
}
198197

199198
certsHash, err := util.HashOfInputHashes(certHashes)
200199
if err != nil {
201-
return "", ctrl.Result{}, err
200+
return "", err
202201
}
203-
return certsHash, ctrl.Result{}, nil
202+
return certsHash, nil
204203
}
205204

206205
// ToService - convert tls.APIService to tls.Service
@@ -225,51 +224,51 @@ func (s *GenericService) ValidateCertSecret(
225224
ctx context.Context,
226225
h *helper.Helper,
227226
namespace string,
228-
) (string, ctrl.Result, error) {
227+
) (string, error) {
229228
hash := ""
230229

231230
endptTLSCfg, err := s.ToService()
232231
if err != nil {
233-
return "", ctrl.Result{}, err
232+
return "", err
234233
}
235234

236235
if endptTLSCfg.SecretName != "" {
237236
// validate the cert secret has the expected keys
238-
var ctrlResult reconcile.Result
239-
hash, ctrlResult, err = endptTLSCfg.ValidateCertSecret(ctx, h, namespace)
237+
hash, err = endptTLSCfg.ValidateCertSecret(ctx, h, namespace)
240238
if err != nil {
241-
return "", ctrlResult, err
242-
} else if (ctrlResult != ctrl.Result{}) {
243-
return "", ctrlResult, nil
239+
return "", err
244240
}
245241
}
246242

247-
return hash, ctrl.Result{}, nil
243+
return hash, nil
248244
}
249245

250246
// ValidateCACertSecret - validates the content of the cert secret to make sure "tls-ca-bundle.pem" key exists
251247
func ValidateCACertSecret(
252248
ctx context.Context,
253249
c client.Client,
254250
caSecret types.NamespacedName,
255-
) (string, ctrl.Result, error) {
251+
) (string, error) {
256252
hash, ctrlResult, err := secret.VerifySecret(
257253
ctx,
258254
caSecret,
259255
[]string{CABundleKey},
260256
c,
261257
5*time.Second)
262258
if err != nil {
263-
return "", ctrlResult, err
259+
return "", err
264260
} else if (ctrlResult != ctrl.Result{}) {
265-
return "", ctrlResult, nil
261+
return "", k8s_errors.NewNotFound(
262+
appsv1.Resource("Secret"),
263+
fmt.Sprintf("%s not found in namespace %s", caSecret.Name, caSecret.Namespace),
264+
)
266265
}
267266

268-
return hash, ctrl.Result{}, nil
267+
return hash, nil
269268
}
270269

271270
// ValidateCertSecret - validates the content of the cert secret to make sure "tls.key", "tls.crt" and optional "ca.crt" keys exist
272-
func (s *Service) ValidateCertSecret(ctx context.Context, h *helper.Helper, namespace string) (string, ctrl.Result, error) {
271+
func (s *Service) ValidateCertSecret(ctx context.Context, h *helper.Helper, namespace string) (string, error) {
273272
// define keys to expect in cert secret
274273
keys := []string{PrivateKey, CertKey}
275274
if s.CaMount != nil {
@@ -283,12 +282,15 @@ func (s *Service) ValidateCertSecret(ctx context.Context, h *helper.Helper, name
283282
h.GetClient(),
284283
5*time.Second)
285284
if err != nil {
286-
return "", ctrlResult, err
285+
return "", err
287286
} else if (ctrlResult != ctrl.Result{}) {
288-
return "", ctrlResult, nil
287+
return "", k8s_errors.NewNotFound(
288+
corev1.Resource(corev1.ResourceSecrets.String()),
289+
fmt.Sprintf("%s not found in namespace %s", s.SecretName, namespace),
290+
)
289291
}
290292

291-
return hash, ctrl.Result{}, nil
293+
return hash, nil
292294
}
293295

294296
// ValidateEndpointCerts - validates all services from an endpointCfgs and
@@ -298,16 +300,14 @@ func ValidateEndpointCerts(
298300
h *helper.Helper,
299301
namespace string,
300302
endpointCfgs map[service.Endpoint]Service,
301-
) (string, ctrl.Result, error) {
303+
) (string, error) {
302304
certHashes := map[string]env.Setter{}
303305
for endpt, endpointTLSCfg := range endpointCfgs {
304306
if endpointTLSCfg.SecretName != "" {
305307
// validate the cert secret has the expected keys
306-
hash, ctrlResult, err := endpointTLSCfg.ValidateCertSecret(ctx, h, namespace)
308+
hash, err := endpointTLSCfg.ValidateCertSecret(ctx, h, namespace)
307309
if err != nil {
308-
return "", ctrlResult, err
309-
} else if (ctrlResult != ctrl.Result{}) {
310-
return "", ctrlResult, nil
310+
return "", err
311311
}
312312

313313
certHashes["cert-"+endpt.String()] = env.SetValue(hash)
@@ -316,9 +316,9 @@ func ValidateEndpointCerts(
316316

317317
certsHash, err := util.HashOfInputHashes(certHashes)
318318
if err != nil {
319-
return "", ctrl.Result{}, err
319+
return "", err
320320
}
321-
return certsHash, ctrl.Result{}, nil
321+
return certsHash, nil
322322
}
323323

324324
// getCertMountPath - return certificate mount path

0 commit comments

Comments
 (0)