Skip to content

Commit 3fbca0c

Browse files
kabicinkabicin
committed
Add secret cleanup after reconcile context cancel
1 parent fac1f4f commit 3fbca0c

7 files changed

Lines changed: 106 additions & 60 deletions

File tree

‎common/secret.go‎

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
package common
2+
3+
import (
4+
"context"
5+
"sync"
6+
7+
corev1 "k8s.io/api/core/v1"
8+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
9+
)
10+
11+
type SecretResource struct {
12+
secret *corev1.Secret
13+
once sync.Once
14+
}
15+
16+
// Creates a Secret that clears it's data when recCtx is canceled
17+
func NewSecret(recCtx context.Context, name, namespace string) *corev1.Secret {
18+
secretResource := NewSecretResource(name, namespace)
19+
go func() {
20+
<-recCtx.Done()
21+
secretResource.Clear()
22+
}()
23+
return secretResource.GetSecret()
24+
}
25+
26+
func NewSecretResource(name, namespace string) *SecretResource {
27+
resource := &SecretResource{
28+
secret: &corev1.Secret{
29+
ObjectMeta: metav1.ObjectMeta{
30+
Name: name,
31+
Namespace: namespace,
32+
},
33+
},
34+
}
35+
return resource
36+
}
37+
38+
func (r *SecretResource) GetSecret() *corev1.Secret {
39+
return r.secret
40+
}
41+
42+
func (r *SecretResource) Clear() {
43+
if r.secret == nil {
44+
return
45+
}
46+
47+
r.once.Do(func() {
48+
for secretKey, secretValue := range r.secret.Data {
49+
clear(secretValue)
50+
delete(r.secret.Data, secretKey)
51+
}
52+
})
53+
}

‎internal/controller/runtimecomponent_controller.go‎

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,11 @@ func (r *RuntimeComponentReconciler) Reconcile(ctx context.Context, req ctrl.Req
101101
ns = r.watchNamespaces[0]
102102
}
103103

104+
// This reconcile context is cancelled right before the Reconcile function terminates or when the parent context ctx is cancelled (such as in operator leader election), whichever happens first.
105+
// Resources that require cleanup may use this context to hook into program execution before a function return.
106+
recCtx, cancel := context.WithCancel(ctx)
107+
defer cancel()
108+
104109
configMap, err := r.GetOpConfigMap(OperatorName, ns)
105110
if err != nil {
106111
reqLogger.Info("Failed to find runtime-component-operator config map")
@@ -238,7 +243,7 @@ func (r *RuntimeComponentReconciler) Reconcile(ctx context.Context, req ctrl.Req
238243
if serviceAccountName == "" {
239244
serviceAccount := &corev1.ServiceAccount{ObjectMeta: defaultMeta}
240245
err = r.CreateOrUpdate(serviceAccount, instance, func() error {
241-
return appstacksutils.CustomizeServiceAccount(serviceAccount, instance, r.GetClient())
246+
return appstacksutils.CustomizeServiceAccount(recCtx, serviceAccount, instance, r.GetClient())
242247
})
243248
if err != nil {
244249
reqLogger.Error(err, "Failed to reconcile ServiceAccount")
@@ -257,7 +262,7 @@ func (r *RuntimeComponentReconciler) Reconcile(ctx context.Context, req ctrl.Req
257262

258263
// Check if the ServiceAccount has a valid pull secret before creating the deployment/statefulset
259264
// or setting up knative. Otherwise the pods can go into an ImagePullBackOff loop
260-
saErr := appstacksutils.ServiceAccountPullSecretExists(instance, r.GetClient())
265+
saErr := appstacksutils.ServiceAccountPullSecretExists(recCtx, instance, r.GetClient())
261266
if saErr != nil {
262267
return r.ManageError(saErr, common.StatusConditionTypeReconciled, instance)
263268
}
@@ -319,7 +324,7 @@ func (r *RuntimeComponentReconciler) Reconcile(ctx context.Context, req ctrl.Req
319324
}
320325
}
321326

322-
useCertmanager, err := r.GenerateSvcCertSecret(ba, "rco", "Runtime Component Operator", "runtime-component-operator")
327+
useCertmanager, err := r.GenerateSvcCertSecret(recCtx, ba, "rco", "Runtime Component Operator", "runtime-component-operator")
323328
if err != nil {
324329
reqLogger.Error(err, "Failed to reconcile CertManager Certificate")
325330
return r.ManageError(err, common.StatusConditionTypeReconciled, instance)
@@ -365,7 +370,7 @@ func (r *RuntimeComponentReconciler) Reconcile(ctx context.Context, req ctrl.Req
365370
}
366371
}
367372

368-
err = r.ReconcileBindings(instance)
373+
err = r.ReconcileBindings(recCtx, instance)
369374
if err != nil {
370375
return r.ManageError(err, common.StatusConditionTypeReconciled, ba)
371376
}
@@ -395,7 +400,7 @@ func (r *RuntimeComponentReconciler) Reconcile(ctx context.Context, req ctrl.Req
395400
err = r.CreateOrUpdate(statefulSet, instance, func() error {
396401
appstacksutils.CustomizeStatefulSet(statefulSet, instance)
397402
appstacksutils.CustomizePodSpec(&statefulSet.Spec.Template, instance)
398-
if err := appstacksutils.CustomizePodWithSVCCertificate(&statefulSet.Spec.Template, instance, r.GetClient()); err != nil {
403+
if err := appstacksutils.CustomizePodWithSVCCertificate(recCtx, &statefulSet.Spec.Template, instance, r.GetClient()); err != nil {
399404
return err
400405
}
401406
appstacksutils.CustomizePersistence(statefulSet, instance)
@@ -427,7 +432,7 @@ func (r *RuntimeComponentReconciler) Reconcile(ctx context.Context, req ctrl.Req
427432
err = r.CreateOrUpdate(deploy, instance, func() error {
428433
appstacksutils.CustomizeDeployment(deploy, instance)
429434
appstacksutils.CustomizePodSpec(&deploy.Spec.Template, instance)
430-
if err := appstacksutils.CustomizePodWithSVCCertificate(&deploy.Spec.Template, instance, r.GetClient()); err != nil {
435+
if err := appstacksutils.CustomizePodWithSVCCertificate(recCtx, &deploy.Spec.Template, instance, r.GetClient()); err != nil {
431436
return err
432437
}
433438
return nil
@@ -529,7 +534,7 @@ func (r *RuntimeComponentReconciler) Reconcile(ctx context.Context, req ctrl.Req
529534
} else if ok {
530535
if instance.Spec.Monitoring != nil && (instance.Spec.CreateKnativeService == nil || !*instance.Spec.CreateKnativeService) {
531536
// Validate the monitoring endpoints' configuration before creating/updating the ServiceMonitor
532-
if err := appstacksutils.ValidatePrometheusMonitoringEndpoints(instance, r.GetClient(), instance.GetNamespace()); err != nil {
537+
if err := appstacksutils.ValidatePrometheusMonitoringEndpoints(recCtx, instance, r.GetClient(), instance.GetNamespace()); err != nil {
533538
return r.ManageError(err, common.StatusConditionTypeReconciled, instance)
534539
}
535540
sm := &prometheusv1.ServiceMonitor{ObjectMeta: defaultMeta}

‎utils/reconciler.go‎

Lines changed: 11 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -621,14 +621,12 @@ func (r *ReconcilerBase) checkIssuerReady(issuer *certmanagerv1.Issuer) error {
621621
return nil
622622
}
623623

624-
func (r *ReconcilerBase) checkSecretExists(secretName, secretNamespace string) error {
625-
secret := &corev1.Secret{}
626-
secret.Name = secretName
627-
secret.Namespace = secretNamespace
628-
return r.GetClient().Get(context.TODO(), types.NamespacedName{Name: secretName, Namespace: secretNamespace}, secret)
624+
func (r *ReconcilerBase) checkSecretExists(recCtx context.Context, secretName, secretNamespace string) error {
625+
secret := common.NewSecret(recCtx, secretName, secretNamespace)
626+
return r.GetClient().Get(context.TODO(), types.NamespacedName{Name: secret.Name, Namespace: secret.Namespace}, secret)
629627
}
630628

631-
func (r *ReconcilerBase) GenerateCMIssuer(namespace string, prefix string, CACommonName string, operatorName string) error {
629+
func (r *ReconcilerBase) GenerateCMIssuer(recCtx context.Context, namespace string, prefix string, CACommonName string, operatorName string) error {
632630
if ok, err := r.IsGroupVersionSupported(certmanagerv1.SchemeGroupVersion.String(), "Issuer"); err != nil {
633631
return err
634632
} else if !ok {
@@ -679,21 +677,17 @@ func (r *ReconcilerBase) GenerateCMIssuer(namespace string, prefix string, CACom
679677
return err
680678
}
681679

682-
CustomCACert := &corev1.Secret{ObjectMeta: metav1.ObjectMeta{
683-
Name: prefix + "-custom-ca-tls",
684-
Namespace: namespace,
685-
}}
680+
customCACert := common.NewSecret(recCtx, prefix+"-custom-ca-tls", namespace)
686681
customCACertFound := false
687-
err = r.GetClient().Get(context.Background(), types.NamespacedName{Name: CustomCACert.GetName(),
688-
Namespace: CustomCACert.GetNamespace()}, CustomCACert)
682+
err = r.GetClient().Get(context.Background(), types.NamespacedName{Name: customCACert.Name, Namespace: customCACert.Namespace}, customCACert)
689683
if err == nil {
690684
customCACertFound = true
691685
} else {
692686
// check CA Certificate and it's Secret exist before CA Issuer init
693687
if err := r.checkCertificateReady(caCert); err != nil {
694688
return err
695689
}
696-
if err := r.checkSecretExists(caCertSecretName, namespace); err != nil {
690+
if err := r.checkSecretExists(recCtx, caCertSecretName, namespace); err != nil {
697691
return err
698692
}
699693
}
@@ -710,8 +704,7 @@ func (r *ReconcilerBase) GenerateCMIssuer(namespace string, prefix string, CACom
710704
issuer.Annotations = map[string]string{}
711705
}
712706
if customCACertFound {
713-
issuer.Spec.CA.SecretName = CustomCACert.Name
714-
707+
issuer.Spec.CA.SecretName = customCACert.Name
715708
}
716709
return nil
717710
})
@@ -730,7 +723,7 @@ func (r *ReconcilerBase) GenerateCMIssuer(namespace string, prefix string, CACom
730723
return nil
731724
}
732725

733-
func (r *ReconcilerBase) GenerateSvcCertSecret(ba common.BaseComponent, prefix string, CACommonName string, operatorName string) (bool, error) {
726+
func (r *ReconcilerBase) GenerateSvcCertSecret(recCtx context.Context, ba common.BaseComponent, prefix string, CACommonName string, operatorName string) (bool, error) {
734727
delete(ba.GetStatus().GetReferences(), common.StatusReferenceCertSecretName)
735728
cleanup := func() {
736729
if ok, err := r.IsGroupVersionSupported(certmanagerv1.SchemeGroupVersion.String(), "Certificate"); err != nil {
@@ -771,7 +764,7 @@ func (r *ReconcilerBase) GenerateSvcCertSecret(ba common.BaseComponent, prefix s
771764
} else if ok {
772765
bao := ba.(metav1.Object)
773766

774-
err = r.GenerateCMIssuer(bao.GetNamespace(), prefix, CACommonName, operatorName)
767+
err = r.GenerateCMIssuer(recCtx, bao.GetNamespace(), prefix, CACommonName, operatorName)
775768
if err != nil {
776769
if errors.Is(err, APIVersionNotFoundError) {
777770
return false, nil
@@ -834,7 +827,7 @@ func (r *ReconcilerBase) GenerateSvcCertSecret(ba common.BaseComponent, prefix s
834827
svcCert.Spec.IssuerRef.Name = customIssuer.Name
835828
}
836829

837-
rVersion, _ := GetIssuerResourceVersion(r.client, svcCert)
830+
rVersion, _ := GetIssuerResourceVersion(recCtx, r.client, svcCert)
838831
if svcCert.Spec.SecretTemplate == nil {
839832
svcCert.Spec.SecretTemplate = &certmanagerv1.CertificateSecretTemplate{
840833
Annotations: map[string]string{},

‎utils/reconciler_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -196,7 +196,7 @@ func TestCreateOrUpdate(t *testing.T) {
196196
r := NewReconcilerBase(rcl, cl, s, &rest.Config{}, record.NewFakeRecorder(10))
197197

198198
err := r.CreateOrUpdate(serviceAccount, runtimecomponent, func() error {
199-
CustomizeServiceAccount(serviceAccount, runtimecomponent, r.GetClient())
199+
CustomizeServiceAccount(context.TODO(), serviceAccount, runtimecomponent, r.GetClient())
200200
return nil
201201
})
202202

‎utils/service_binding_reconciler.go‎

Lines changed: 16 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -20,21 +20,16 @@ const (
2020
)
2121

2222
// ReconcileBindings goes through the reconcile logic for service binding
23-
func (r *ReconcilerBase) ReconcileBindings(ba common.BaseComponent) error {
24-
if err := r.reconcileExpose(ba); err != nil {
23+
func (r *ReconcilerBase) ReconcileBindings(recCtx context.Context, ba common.BaseComponent) error {
24+
if err := r.reconcileExpose(recCtx, ba); err != nil {
2525
return err
2626
}
2727
return nil
2828
}
2929

30-
func (r *ReconcilerBase) reconcileExpose(ba common.BaseComponent) error {
30+
func (r *ReconcilerBase) reconcileExpose(recCtx context.Context, ba common.BaseComponent) error {
3131
mObj := ba.(metav1.Object)
32-
bindingSecret := &corev1.Secret{
33-
ObjectMeta: metav1.ObjectMeta{
34-
Name: getExposeBindingSecretName(ba),
35-
Namespace: mObj.GetNamespace(),
36-
},
37-
}
32+
bindingSecret := common.NewSecret(recCtx, getExposeBindingSecretName(ba), mObj.GetNamespace())
3833

3934
if ba.GetService() != nil && ba.GetService().GetBindable() != nil && *ba.GetService().GetBindable() {
4035
err := r.CreateOrUpdate(bindingSecret, mObj, func() error {
@@ -46,7 +41,7 @@ func (r *ReconcilerBase) reconcileExpose(ba common.BaseComponent) error {
4641
// Use content of the 'override' secret as the base secret content
4742
bindingSecret.Data = customSecret.Data
4843
// Apply default values to the override secret if certain values are not set
49-
r.applyDefaultValuesToExpose(bindingSecret, ba)
44+
r.applyDefaultValuesToExpose(recCtx, bindingSecret, ba)
5045
return nil
5146
})
5247
if err != nil {
@@ -77,15 +72,15 @@ func (r *ReconcilerBase) getCustomValuesToExpose(secret *corev1.Secret, ba commo
7772
return nil
7873
}
7974

80-
func (r *ReconcilerBase) applyDefaultValuesToExpose(secret *corev1.Secret, ba common.BaseComponent) {
75+
func (r *ReconcilerBase) applyDefaultValuesToExpose(recCtx context.Context, secret *corev1.Secret, ba common.BaseComponent) {
8176
mObj := ba.(metav1.Object)
8277
secret.Labels = ba.GetLabels()
8378
secret.Annotations = MergeMaps(secret.Annotations, ba.GetAnnotations())
8479

85-
secretData := secret.Data
86-
if secretData == nil {
87-
secretData = map[string][]byte{}
80+
if secret.Data == nil {
81+
secret.Data = map[string][]byte{}
8882
}
83+
secretData := secret.Data
8984
var host, protocol, basePath, port []byte
9085
var found bool
9186
if host, found = secretData["host"]; !found {
@@ -126,26 +121,24 @@ func (r *ReconcilerBase) applyDefaultValuesToExpose(secret *corev1.Secret, ba co
126121
}
127122

128123
if _, found = secretData["certificates"]; !found && ba.GetStatus().GetReferences()[common.StatusReferenceCertSecretName] != "" {
129-
130-
certSecret := &corev1.Secret{}
131-
err := r.GetClient().Get(context.TODO(), types.NamespacedName{Name: ba.GetStatus().GetReferences()[common.StatusReferenceCertSecretName], Namespace: mObj.GetNamespace()}, certSecret)
124+
certSecret := common.NewSecret(recCtx, ba.GetStatus().GetReferences()[common.StatusReferenceCertSecretName], mObj.GetNamespace())
125+
err := r.GetClient().Get(context.TODO(), types.NamespacedName{Name: certSecret.Name, Namespace: certSecret.Namespace}, certSecret)
132126
if err == nil {
133127
caCert := certSecret.Data["ca.crt"]
134128
tlsCrt := certSecret.Data["tls.crt"]
135-
chain := string(tlsCrt) + string(caCert)
136-
if chain != "" {
137-
secretData["certificates"] = []byte(chain)
129+
chainedCerts := make([]byte, len(caCert)+len(tlsCrt))
130+
nCount := copy(chainedCerts, tlsCrt)
131+
nCount += copy(chainedCerts[len(tlsCrt):], caCert)
132+
if nCount > 0 {
133+
secretData["certificates"] = chainedCerts
138134
}
139135
}
140136
}
141137

142138
if _, found = secretData["ingress-uri"]; !found && ba.GetExpose() != nil && *ba.GetExpose() {
143-
144139
host, path, protocol := r.GetIngressInfo(ba)
145140
secretData["ingress-uri"] = []byte(fmt.Sprintf("%s://%s%s%s", protocol, host, path, string(basePath)))
146-
147141
}
148-
secret.Data = secretData
149142
}
150143

151144
func (r *ReconcilerBase) updateBindingStatus(bindingSecretName string, ba common.BaseComponent) {

‎utils/utils.go‎

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -846,7 +846,7 @@ func CustomizePersistence(statefulSet *appsv1.StatefulSet, ba common.BaseCompone
846846
}
847847

848848
// CustomizeServiceAccount ...
849-
func CustomizeServiceAccount(sa *corev1.ServiceAccount, ba common.BaseComponent, client client.Client) error {
849+
func CustomizeServiceAccount(recCtx context.Context, sa *corev1.ServiceAccount, ba common.BaseComponent, client client.Client) error {
850850
sa.Labels = ba.GetLabels()
851851
sa.Annotations = MergeMaps(sa.Annotations, ba.GetAnnotations())
852852

@@ -1111,7 +1111,7 @@ func secretShouldExist(secretKeySelector corev1.SecretKeySelector) bool {
11111111
}
11121112

11131113
// Returns an error if any user specified non-optional Secret or ConfigMap for Prometheus monitoring does not exist, otherwise return nil.
1114-
func ValidatePrometheusMonitoringEndpoints(ba common.BaseComponent, client client.Client, namespace string) error {
1114+
func ValidatePrometheusMonitoringEndpoints(recCtx context.Context, ba common.BaseComponent, client client.Client, namespace string) error {
11151115
var basicAuth *prometheusv1.BasicAuth
11161116
var oauth2 *prometheusv1.OAuth2
11171117
var bearerTokenSecret corev1.SecretKeySelector
@@ -1158,7 +1158,8 @@ func ValidatePrometheusMonitoringEndpoints(ba common.BaseComponent, client clien
11581158
}
11591159
// Error if any Secret is specified but does not exist
11601160
for _, secretName := range secretNames {
1161-
if err := client.Get(context.TODO(), types.NamespacedName{Name: secretName, Namespace: namespace}, &corev1.Secret{}); err != nil {
1161+
secret := common.NewSecret(recCtx, secretName, namespace)
1162+
if err := client.Get(context.TODO(), types.NamespacedName{Name: secretName, Namespace: namespace}, secret); err != nil {
11621163
errorMessage := fmt.Sprintf("Could not find Secret '%s' in this namespace.", secretName)
11631164
return errors.New(errorMessage)
11641165
}
@@ -1619,7 +1620,7 @@ func (r *ReconcilerBase) toJSONFromRaw(content *runtime.RawExtension) (map[strin
16191620
// Looks for a pull secret in the service account retrieved from the component
16201621
// Returns nil if there is at least one image pull secret, otherwise an error
16211622
// Will always return nil if 'skipPullSecretValidation' is specified in the CR
1622-
func ServiceAccountPullSecretExists(ba common.BaseComponent, client client.Client) error {
1623+
func ServiceAccountPullSecretExists(recCtx context.Context, ba common.BaseComponent, client client.Client) error {
16231624
obj := ba.(metav1.Object)
16241625
ns := obj.GetNamespace()
16251626
saName := obj.GetName()
@@ -1654,7 +1655,8 @@ func ServiceAccountPullSecretExists(ba common.BaseComponent, client client.Clien
16541655
// if this is our service account there will be one image pull secret
16551656
// For others there could be more. either way, just use the first?
16561657
sName := secrets[0].Name
1657-
err := client.Get(context.TODO(), types.NamespacedName{Name: sName, Namespace: ns}, &corev1.Secret{})
1658+
pullSecret := common.NewSecret(recCtx, sName, ns)
1659+
err := client.Get(context.TODO(), types.NamespacedName{Name: pullSecret.Name, Namespace: pullSecret.Namespace}, pullSecret)
16581660
if err != nil {
16591661
saErr := errors.New("Service account " + saName + " isn't ready. Reason: " + err.Error())
16601662
return saErr
@@ -1844,17 +1846,16 @@ func UpdateConfigMap(configMap *corev1.ConfigMap, key string) {
18441846
data[key] = common.LoadFromConfig(common.Config, key)
18451847
}
18461848

1847-
func GetIssuerResourceVersion(client client.Client, certificate *certmanagerv1.Certificate) (string, error) {
1849+
func GetIssuerResourceVersion(recCtx context.Context, client client.Client, certificate *certmanagerv1.Certificate) (string, error) {
18481850
issuer := &certmanagerv1.Issuer{}
18491851
err := client.Get(context.Background(), types.NamespacedName{Name: certificate.Spec.IssuerRef.Name,
18501852
Namespace: certificate.Namespace}, issuer)
18511853
if err != nil {
18521854
return "", err
18531855
}
18541856
if issuer.Spec.CA != nil {
1855-
caSecret := &corev1.Secret{}
1856-
err = client.Get(context.Background(), types.NamespacedName{Name: issuer.Spec.CA.SecretName,
1857-
Namespace: certificate.Namespace}, caSecret)
1857+
caSecret := common.NewSecret(recCtx, issuer.Spec.CA.SecretName, certificate.Namespace)
1858+
err = client.Get(context.Background(), types.NamespacedName{Name: caSecret.Name, Namespace: caSecret.Namespace}, caSecret)
18581859
if err != nil {
18591860
return "", err
18601861
} else {

‎utils/utils_test.go‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package utils
22

33
import (
4+
"context"
45
"os"
56
"reflect"
67
"strconv"
@@ -535,13 +536,13 @@ func TestCustomizeServiceAccount(t *testing.T) {
535536

536537
spec := appstacksv1.RuntimeComponentSpec{PullSecret: &pullSecret}
537538
sa, runtime := &corev1.ServiceAccount{}, createRuntimeComponent(name, namespace, spec)
538-
CustomizeServiceAccount(sa, runtime, fcl)
539+
CustomizeServiceAccount(context.TODO(), sa, runtime, fcl)
539540
emptySAIPS := sa.ImagePullSecrets[0].Name
540541

541542
newSecret := "my-new-secret"
542543
spec = appstacksv1.RuntimeComponentSpec{PullSecret: &newSecret}
543544
runtime = createRuntimeComponent(name, namespace, spec)
544-
CustomizeServiceAccount(sa, runtime, fcl)
545+
CustomizeServiceAccount(context.TODO(), sa, runtime, fcl)
545546

546547
testCSA := []Test{
547548
{"ServiceAccount image pull secrets is empty", pullSecret, emptySAIPS},

0 commit comments

Comments
 (0)