From 53da321d9f0a7ed02c272975e45fd30ff45d8c9c Mon Sep 17 00:00:00 2001 From: Kirby Chin <37311900+kabicin@users.noreply.github.com> Date: Wed, 7 Jan 2026 14:45:30 -0500 Subject: [PATCH 1/7] Add secret cleanup after reconcile context cancel --- common/secret.go | 53 +++++++++++++++++++ .../controller/runtimecomponent_controller.go | 19 ++++--- utils/reconciler.go | 29 ++++------ utils/reconciler_test.go | 2 +- utils/service_binding_reconciler.go | 39 ++++++-------- utils/utils.go | 19 +++---- utils/utils_test.go | 5 +- 7 files changed, 106 insertions(+), 60 deletions(-) create mode 100644 common/secret.go diff --git a/common/secret.go b/common/secret.go new file mode 100644 index 000000000..b2e4dd070 --- /dev/null +++ b/common/secret.go @@ -0,0 +1,53 @@ +package common + +import ( + "context" + "sync" + + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +type SecretResource struct { + secret *corev1.Secret + once sync.Once +} + +// Creates a Secret that clears it's data when recCtx is canceled +func NewSecret(recCtx context.Context, name, namespace string) *corev1.Secret { + secretResource := NewSecretResource(name, namespace) + go func() { + <-recCtx.Done() + secretResource.Clear() + }() + return secretResource.GetSecret() +} + +func NewSecretResource(name, namespace string) *SecretResource { + resource := &SecretResource{ + secret: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + }, + }, + } + return resource +} + +func (r *SecretResource) GetSecret() *corev1.Secret { + return r.secret +} + +func (r *SecretResource) Clear() { + if r.secret == nil { + return + } + + r.once.Do(func() { + for secretKey, secretValue := range r.secret.Data { + clear(secretValue) + delete(r.secret.Data, secretKey) + } + }) +} diff --git a/internal/controller/runtimecomponent_controller.go b/internal/controller/runtimecomponent_controller.go index 861092b5a..c18fa0f31 100644 --- a/internal/controller/runtimecomponent_controller.go +++ b/internal/controller/runtimecomponent_controller.go @@ -101,6 +101,11 @@ func (r *RuntimeComponentReconciler) Reconcile(ctx context.Context, req ctrl.Req ns = r.watchNamespaces[0] } + // 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. + // Resources that require cleanup may use this context to hook into program execution before a function return. + recCtx, cancel := context.WithCancel(ctx) + defer cancel() + configMap, err := r.GetOpConfigMap(OperatorName, ns) if err != nil { reqLogger.Info("Failed to find runtime-component-operator config map") @@ -238,7 +243,7 @@ func (r *RuntimeComponentReconciler) Reconcile(ctx context.Context, req ctrl.Req if serviceAccountName == "" { serviceAccount := &corev1.ServiceAccount{ObjectMeta: defaultMeta} err = r.CreateOrUpdate(serviceAccount, instance, func() error { - return appstacksutils.CustomizeServiceAccount(serviceAccount, instance, r.GetClient()) + return appstacksutils.CustomizeServiceAccount(recCtx, serviceAccount, instance, r.GetClient()) }) if err != nil { reqLogger.Error(err, "Failed to reconcile ServiceAccount") @@ -257,7 +262,7 @@ func (r *RuntimeComponentReconciler) Reconcile(ctx context.Context, req ctrl.Req // Check if the ServiceAccount has a valid pull secret before creating the deployment/statefulset // or setting up knative. Otherwise the pods can go into an ImagePullBackOff loop - saErr := appstacksutils.ServiceAccountPullSecretExists(instance, r.GetClient()) + saErr := appstacksutils.ServiceAccountPullSecretExists(recCtx, instance, r.GetClient()) if saErr != nil { return r.ManageError(saErr, common.StatusConditionTypeReconciled, instance) } @@ -319,7 +324,7 @@ func (r *RuntimeComponentReconciler) Reconcile(ctx context.Context, req ctrl.Req } } - useCertmanager, err := r.GenerateSvcCertSecret(ba, "rco", "Runtime Component Operator", "runtime-component-operator") + useCertmanager, err := r.GenerateSvcCertSecret(recCtx, ba, "rco", "Runtime Component Operator", "runtime-component-operator") if err != nil { reqLogger.Error(err, "Failed to reconcile CertManager Certificate") return r.ManageError(err, common.StatusConditionTypeReconciled, instance) @@ -365,7 +370,7 @@ func (r *RuntimeComponentReconciler) Reconcile(ctx context.Context, req ctrl.Req } } - err = r.ReconcileBindings(instance) + err = r.ReconcileBindings(recCtx, instance) if err != nil { return r.ManageError(err, common.StatusConditionTypeReconciled, ba) } @@ -395,7 +400,7 @@ func (r *RuntimeComponentReconciler) Reconcile(ctx context.Context, req ctrl.Req err = r.CreateOrUpdate(statefulSet, instance, func() error { appstacksutils.CustomizeStatefulSet(statefulSet, instance) appstacksutils.CustomizePodSpec(&statefulSet.Spec.Template, instance) - if err := appstacksutils.CustomizePodWithSVCCertificate(&statefulSet.Spec.Template, instance, r.GetClient()); err != nil { + if err := appstacksutils.CustomizePodWithSVCCertificate(recCtx, &statefulSet.Spec.Template, instance, r.GetClient()); err != nil { return err } appstacksutils.CustomizePersistence(statefulSet, instance) @@ -427,7 +432,7 @@ func (r *RuntimeComponentReconciler) Reconcile(ctx context.Context, req ctrl.Req err = r.CreateOrUpdate(deploy, instance, func() error { appstacksutils.CustomizeDeployment(deploy, instance) appstacksutils.CustomizePodSpec(&deploy.Spec.Template, instance) - if err := appstacksutils.CustomizePodWithSVCCertificate(&deploy.Spec.Template, instance, r.GetClient()); err != nil { + if err := appstacksutils.CustomizePodWithSVCCertificate(recCtx, &deploy.Spec.Template, instance, r.GetClient()); err != nil { return err } return nil @@ -529,7 +534,7 @@ func (r *RuntimeComponentReconciler) Reconcile(ctx context.Context, req ctrl.Req } else if ok { if instance.Spec.Monitoring != nil && (instance.Spec.CreateKnativeService == nil || !*instance.Spec.CreateKnativeService) { // Validate the monitoring endpoints' configuration before creating/updating the ServiceMonitor - if err := appstacksutils.ValidatePrometheusMonitoringEndpoints(instance, r.GetClient(), instance.GetNamespace()); err != nil { + if err := appstacksutils.ValidatePrometheusMonitoringEndpoints(recCtx, instance, r.GetClient(), instance.GetNamespace()); err != nil { return r.ManageError(err, common.StatusConditionTypeReconciled, instance) } sm := &prometheusv1.ServiceMonitor{ObjectMeta: defaultMeta} diff --git a/utils/reconciler.go b/utils/reconciler.go index 1400ea53b..105c4a0a6 100644 --- a/utils/reconciler.go +++ b/utils/reconciler.go @@ -621,14 +621,12 @@ func (r *ReconcilerBase) checkIssuerReady(issuer *certmanagerv1.Issuer) error { return nil } -func (r *ReconcilerBase) checkSecretExists(secretName, secretNamespace string) error { - secret := &corev1.Secret{} - secret.Name = secretName - secret.Namespace = secretNamespace - return r.GetClient().Get(context.TODO(), types.NamespacedName{Name: secretName, Namespace: secretNamespace}, secret) +func (r *ReconcilerBase) checkSecretExists(recCtx context.Context, secretName, secretNamespace string) error { + secret := common.NewSecret(recCtx, secretName, secretNamespace) + return r.GetClient().Get(context.TODO(), types.NamespacedName{Name: secret.Name, Namespace: secret.Namespace}, secret) } -func (r *ReconcilerBase) GenerateCMIssuer(namespace string, prefix string, CACommonName string, operatorName string) error { +func (r *ReconcilerBase) GenerateCMIssuer(recCtx context.Context, namespace string, prefix string, CACommonName string, operatorName string) error { if ok, err := r.IsGroupVersionSupported(certmanagerv1.SchemeGroupVersion.String(), "Issuer"); err != nil { return err } else if !ok { @@ -679,13 +677,9 @@ func (r *ReconcilerBase) GenerateCMIssuer(namespace string, prefix string, CACom return err } - CustomCACert := &corev1.Secret{ObjectMeta: metav1.ObjectMeta{ - Name: prefix + "-custom-ca-tls", - Namespace: namespace, - }} + customCACert := common.NewSecret(recCtx, prefix+"-custom-ca-tls", namespace) customCACertFound := false - err = r.GetClient().Get(context.Background(), types.NamespacedName{Name: CustomCACert.GetName(), - Namespace: CustomCACert.GetNamespace()}, CustomCACert) + err = r.GetClient().Get(context.Background(), types.NamespacedName{Name: customCACert.Name, Namespace: customCACert.Namespace}, customCACert) if err == nil { customCACertFound = true } else { @@ -693,7 +687,7 @@ func (r *ReconcilerBase) GenerateCMIssuer(namespace string, prefix string, CACom if err := r.checkCertificateReady(caCert); err != nil { return err } - if err := r.checkSecretExists(caCertSecretName, namespace); err != nil { + if err := r.checkSecretExists(recCtx, caCertSecretName, namespace); err != nil { return err } } @@ -710,8 +704,7 @@ func (r *ReconcilerBase) GenerateCMIssuer(namespace string, prefix string, CACom issuer.Annotations = map[string]string{} } if customCACertFound { - issuer.Spec.CA.SecretName = CustomCACert.Name - + issuer.Spec.CA.SecretName = customCACert.Name } return nil }) @@ -730,7 +723,7 @@ func (r *ReconcilerBase) GenerateCMIssuer(namespace string, prefix string, CACom return nil } -func (r *ReconcilerBase) GenerateSvcCertSecret(ba common.BaseComponent, prefix string, CACommonName string, operatorName string) (bool, error) { +func (r *ReconcilerBase) GenerateSvcCertSecret(recCtx context.Context, ba common.BaseComponent, prefix string, CACommonName string, operatorName string) (bool, error) { delete(ba.GetStatus().GetReferences(), common.StatusReferenceCertSecretName) cleanup := func() { if ok, err := r.IsGroupVersionSupported(certmanagerv1.SchemeGroupVersion.String(), "Certificate"); err != nil { @@ -771,7 +764,7 @@ func (r *ReconcilerBase) GenerateSvcCertSecret(ba common.BaseComponent, prefix s } else if ok { bao := ba.(metav1.Object) - err = r.GenerateCMIssuer(bao.GetNamespace(), prefix, CACommonName, operatorName) + err = r.GenerateCMIssuer(recCtx, bao.GetNamespace(), prefix, CACommonName, operatorName) if err != nil { if errors.Is(err, APIVersionNotFoundError) { return false, nil @@ -834,7 +827,7 @@ func (r *ReconcilerBase) GenerateSvcCertSecret(ba common.BaseComponent, prefix s svcCert.Spec.IssuerRef.Name = customIssuer.Name } - rVersion, _ := GetIssuerResourceVersion(r.client, svcCert) + rVersion, _ := GetIssuerResourceVersion(recCtx, r.client, svcCert) if svcCert.Spec.SecretTemplate == nil { svcCert.Spec.SecretTemplate = &certmanagerv1.CertificateSecretTemplate{ Annotations: map[string]string{}, diff --git a/utils/reconciler_test.go b/utils/reconciler_test.go index 5cb68b0d6..e1d318064 100644 --- a/utils/reconciler_test.go +++ b/utils/reconciler_test.go @@ -196,7 +196,7 @@ func TestCreateOrUpdate(t *testing.T) { r := NewReconcilerBase(rcl, cl, s, &rest.Config{}, record.NewFakeRecorder(10)) err := r.CreateOrUpdate(serviceAccount, runtimecomponent, func() error { - CustomizeServiceAccount(serviceAccount, runtimecomponent, r.GetClient()) + CustomizeServiceAccount(context.TODO(), serviceAccount, runtimecomponent, r.GetClient()) return nil }) diff --git a/utils/service_binding_reconciler.go b/utils/service_binding_reconciler.go index 911d3551a..7ead503fe 100644 --- a/utils/service_binding_reconciler.go +++ b/utils/service_binding_reconciler.go @@ -20,21 +20,16 @@ const ( ) // ReconcileBindings goes through the reconcile logic for service binding -func (r *ReconcilerBase) ReconcileBindings(ba common.BaseComponent) error { - if err := r.reconcileExpose(ba); err != nil { +func (r *ReconcilerBase) ReconcileBindings(recCtx context.Context, ba common.BaseComponent) error { + if err := r.reconcileExpose(recCtx, ba); err != nil { return err } return nil } -func (r *ReconcilerBase) reconcileExpose(ba common.BaseComponent) error { +func (r *ReconcilerBase) reconcileExpose(recCtx context.Context, ba common.BaseComponent) error { mObj := ba.(metav1.Object) - bindingSecret := &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: getExposeBindingSecretName(ba), - Namespace: mObj.GetNamespace(), - }, - } + bindingSecret := common.NewSecret(recCtx, getExposeBindingSecretName(ba), mObj.GetNamespace()) if ba.GetService() != nil && ba.GetService().GetBindable() != nil && *ba.GetService().GetBindable() { err := r.CreateOrUpdate(bindingSecret, mObj, func() error { @@ -46,7 +41,7 @@ func (r *ReconcilerBase) reconcileExpose(ba common.BaseComponent) error { // Use content of the 'override' secret as the base secret content bindingSecret.Data = customSecret.Data // Apply default values to the override secret if certain values are not set - r.applyDefaultValuesToExpose(bindingSecret, ba) + r.applyDefaultValuesToExpose(recCtx, bindingSecret, ba) return nil }) if err != nil { @@ -77,15 +72,15 @@ func (r *ReconcilerBase) getCustomValuesToExpose(secret *corev1.Secret, ba commo return nil } -func (r *ReconcilerBase) applyDefaultValuesToExpose(secret *corev1.Secret, ba common.BaseComponent) { +func (r *ReconcilerBase) applyDefaultValuesToExpose(recCtx context.Context, secret *corev1.Secret, ba common.BaseComponent) { mObj := ba.(metav1.Object) secret.Labels = ba.GetLabels() secret.Annotations = MergeMaps(secret.Annotations, ba.GetAnnotations()) - secretData := secret.Data - if secretData == nil { - secretData = map[string][]byte{} + if secret.Data == nil { + secret.Data = map[string][]byte{} } + secretData := secret.Data var host, protocol, basePath, port []byte var found bool if host, found = secretData["host"]; !found { @@ -126,26 +121,24 @@ func (r *ReconcilerBase) applyDefaultValuesToExpose(secret *corev1.Secret, ba co } if _, found = secretData["certificates"]; !found && ba.GetStatus().GetReferences()[common.StatusReferenceCertSecretName] != "" { - - certSecret := &corev1.Secret{} - err := r.GetClient().Get(context.TODO(), types.NamespacedName{Name: ba.GetStatus().GetReferences()[common.StatusReferenceCertSecretName], Namespace: mObj.GetNamespace()}, certSecret) + certSecret := common.NewSecret(recCtx, ba.GetStatus().GetReferences()[common.StatusReferenceCertSecretName], mObj.GetNamespace()) + err := r.GetClient().Get(context.TODO(), types.NamespacedName{Name: certSecret.Name, Namespace: certSecret.Namespace}, certSecret) if err == nil { caCert := certSecret.Data["ca.crt"] tlsCrt := certSecret.Data["tls.crt"] - chain := string(tlsCrt) + string(caCert) - if chain != "" { - secretData["certificates"] = []byte(chain) + chainedCerts := make([]byte, len(caCert)+len(tlsCrt)) + nCount := copy(chainedCerts, tlsCrt) + nCount += copy(chainedCerts[len(tlsCrt):], caCert) + if nCount > 0 { + secretData["certificates"] = chainedCerts } } } if _, found = secretData["ingress-uri"]; !found && ba.GetExpose() != nil && *ba.GetExpose() { - host, path, protocol := r.GetIngressInfo(ba) secretData["ingress-uri"] = []byte(fmt.Sprintf("%s://%s%s%s", protocol, host, path, string(basePath))) - } - secret.Data = secretData } func (r *ReconcilerBase) updateBindingStatus(bindingSecretName string, ba common.BaseComponent) { diff --git a/utils/utils.go b/utils/utils.go index e6a0d413c..43b91608f 100644 --- a/utils/utils.go +++ b/utils/utils.go @@ -848,7 +848,7 @@ func CustomizePersistence(statefulSet *appsv1.StatefulSet, ba common.BaseCompone } // CustomizeServiceAccount ... -func CustomizeServiceAccount(sa *corev1.ServiceAccount, ba common.BaseComponent, client client.Client) error { +func CustomizeServiceAccount(recCtx context.Context, sa *corev1.ServiceAccount, ba common.BaseComponent, client client.Client) error { sa.Labels = ba.GetLabels() sa.Annotations = MergeMaps(sa.Annotations, ba.GetAnnotations()) @@ -1113,7 +1113,7 @@ func secretShouldExist(secretKeySelector corev1.SecretKeySelector) bool { } // Returns an error if any user specified non-optional Secret or ConfigMap for Prometheus monitoring does not exist, otherwise return nil. -func ValidatePrometheusMonitoringEndpoints(ba common.BaseComponent, client client.Client, namespace string) error { +func ValidatePrometheusMonitoringEndpoints(recCtx context.Context, ba common.BaseComponent, client client.Client, namespace string) error { var basicAuth *prometheusv1.BasicAuth var oauth2 *prometheusv1.OAuth2 var bearerTokenSecret corev1.SecretKeySelector @@ -1160,7 +1160,8 @@ func ValidatePrometheusMonitoringEndpoints(ba common.BaseComponent, client clien } // Error if any Secret is specified but does not exist for _, secretName := range secretNames { - if err := client.Get(context.TODO(), types.NamespacedName{Name: secretName, Namespace: namespace}, &corev1.Secret{}); err != nil { + secret := common.NewSecret(recCtx, secretName, namespace) + if err := client.Get(context.TODO(), types.NamespacedName{Name: secretName, Namespace: namespace}, secret); err != nil { errorMessage := fmt.Sprintf("Could not find Secret '%s' in this namespace.", secretName) return errors.New(errorMessage) } @@ -1621,7 +1622,7 @@ func (r *ReconcilerBase) toJSONFromRaw(content *runtime.RawExtension) (map[strin // Looks for a pull secret in the service account retrieved from the component // Returns nil if there is at least one image pull secret, otherwise an error // Will always return nil if 'skipPullSecretValidation' is specified in the CR -func ServiceAccountPullSecretExists(ba common.BaseComponent, client client.Client) error { +func ServiceAccountPullSecretExists(recCtx context.Context, ba common.BaseComponent, client client.Client) error { obj := ba.(metav1.Object) ns := obj.GetNamespace() saName := obj.GetName() @@ -1656,7 +1657,8 @@ func ServiceAccountPullSecretExists(ba common.BaseComponent, client client.Clien // if this is our service account there will be one image pull secret // For others there could be more. either way, just use the first? sName := secrets[0].Name - err := client.Get(context.TODO(), types.NamespacedName{Name: sName, Namespace: ns}, &corev1.Secret{}) + pullSecret := common.NewSecret(recCtx, sName, ns) + err := client.Get(context.TODO(), types.NamespacedName{Name: pullSecret.Name, Namespace: pullSecret.Namespace}, pullSecret) if err != nil { saErr := errors.New("Service account " + saName + " isn't ready. Reason: " + err.Error()) return saErr @@ -1846,7 +1848,7 @@ func UpdateConfigMap(configMap *corev1.ConfigMap, key string) { data[key] = common.LoadFromConfig(common.Config, key) } -func GetIssuerResourceVersion(client client.Client, certificate *certmanagerv1.Certificate) (string, error) { +func GetIssuerResourceVersion(recCtx context.Context, client client.Client, certificate *certmanagerv1.Certificate) (string, error) { issuer := &certmanagerv1.Issuer{} err := client.Get(context.Background(), types.NamespacedName{Name: certificate.Spec.IssuerRef.Name, Namespace: certificate.Namespace}, issuer) @@ -1854,9 +1856,8 @@ func GetIssuerResourceVersion(client client.Client, certificate *certmanagerv1.C return "", err } if issuer.Spec.CA != nil { - caSecret := &corev1.Secret{} - err = client.Get(context.Background(), types.NamespacedName{Name: issuer.Spec.CA.SecretName, - Namespace: certificate.Namespace}, caSecret) + caSecret := common.NewSecret(recCtx, issuer.Spec.CA.SecretName, certificate.Namespace) + err = client.Get(context.Background(), types.NamespacedName{Name: caSecret.Name, Namespace: caSecret.Namespace}, caSecret) if err != nil { return "", err } else { diff --git a/utils/utils_test.go b/utils/utils_test.go index a3d58bb82..b3a454b2d 100644 --- a/utils/utils_test.go +++ b/utils/utils_test.go @@ -1,6 +1,7 @@ package utils import ( + "context" "os" "reflect" "strconv" @@ -535,13 +536,13 @@ func TestCustomizeServiceAccount(t *testing.T) { spec := appstacksv1.RuntimeComponentSpec{PullSecret: &pullSecret} sa, runtime := &corev1.ServiceAccount{}, createRuntimeComponent(name, namespace, spec) - CustomizeServiceAccount(sa, runtime, fcl) + CustomizeServiceAccount(context.TODO(), sa, runtime, fcl) emptySAIPS := sa.ImagePullSecrets[0].Name newSecret := "my-new-secret" spec = appstacksv1.RuntimeComponentSpec{PullSecret: &newSecret} runtime = createRuntimeComponent(name, namespace, spec) - CustomizeServiceAccount(sa, runtime, fcl) + CustomizeServiceAccount(context.TODO(), sa, runtime, fcl) testCSA := []Test{ {"ServiceAccount image pull secrets is empty", pullSecret, emptySAIPS}, From 8464518fddb799d4061b71b7d9f836555ed2e791 Mon Sep 17 00:00:00 2001 From: Kirby Chin <37311900+kabicin@users.noreply.github.com> Date: Thu, 8 Jan 2026 11:38:00 -0500 Subject: [PATCH 2/7] Add TrackedCreateOrUpdate and WaitableSecretResource --- common/secret.go | 33 +++++++++++++++++++++++++++-- utils/reconciler.go | 10 +++++++++ utils/service_binding_reconciler.go | 6 +++--- 3 files changed, 44 insertions(+), 5 deletions(-) diff --git a/common/secret.go b/common/secret.go index b2e4dd070..6bc5613dc 100644 --- a/common/secret.go +++ b/common/secret.go @@ -13,16 +13,30 @@ type SecretResource struct { once sync.Once } +type WaitableSecretResource struct { + *SecretResource + wg sync.WaitGroup +} + // Creates a Secret that clears it's data when recCtx is canceled func NewSecret(recCtx context.Context, name, namespace string) *corev1.Secret { secretResource := NewSecretResource(name, namespace) go func() { <-recCtx.Done() - secretResource.Clear() + secretResource.Clear(nil) }() return secretResource.GetSecret() } +func NewWaitableSecret(recCtx context.Context, name, namespace string) (*corev1.Secret, *sync.WaitGroup) { + waitableSecretResource := NewWaitableSecretResource(name, namespace) + go func() { + <-recCtx.Done() + waitableSecretResource.Clear(waitableSecretResource.GetWaitGroup()) + }() + return waitableSecretResource.GetSecret(), waitableSecretResource.GetWaitGroup() +} + func NewSecretResource(name, namespace string) *SecretResource { resource := &SecretResource{ secret: &corev1.Secret{ @@ -35,16 +49,31 @@ func NewSecretResource(name, namespace string) *SecretResource { return resource } +func NewWaitableSecretResource(name, namespace string) *WaitableSecretResource { + resource := &WaitableSecretResource{ + SecretResource: NewSecretResource(name, namespace), + wg: sync.WaitGroup{}, + } + return resource +} + +func (wsr *WaitableSecretResource) GetWaitGroup() *sync.WaitGroup { + return &wsr.wg +} + func (r *SecretResource) GetSecret() *corev1.Secret { return r.secret } -func (r *SecretResource) Clear() { +func (r *SecretResource) Clear(wg *sync.WaitGroup) { if r.secret == nil { return } r.once.Do(func() { + if wg != nil { + wg.Wait() + } for secretKey, secretValue := range r.secret.Data { clear(secretValue) delete(r.secret.Data, secretKey) diff --git a/utils/reconciler.go b/utils/reconciler.go index 105c4a0a6..d5921c142 100644 --- a/utils/reconciler.go +++ b/utils/reconciler.go @@ -7,6 +7,7 @@ import ( "fmt" "math" "strconv" + "sync" "time" networkingv1 "k8s.io/api/networking/v1" @@ -172,6 +173,15 @@ func (r *ReconcilerBase) CreateOrUpdate(obj client.Object, owner metav1.Object, return err } +// Runs CreateOrUpdate tracked by a WaitGroup wg +func (r *ReconcilerBase) TrackedCreateOrUpdate(obj client.Object, owner metav1.Object, reconcile func() error, wg *sync.WaitGroup) error { + if wg != nil { + wg.Add(1) + defer wg.Done() + } + return r.CreateOrUpdate(obj, owner, reconcile) +} + // DeleteResource deletes kubernetes resource func (r *ReconcilerBase) DeleteResource(obj client.Object) error { err := r.client.Delete(context.TODO(), obj) diff --git a/utils/service_binding_reconciler.go b/utils/service_binding_reconciler.go index 7ead503fe..cbc93b359 100644 --- a/utils/service_binding_reconciler.go +++ b/utils/service_binding_reconciler.go @@ -29,10 +29,10 @@ func (r *ReconcilerBase) ReconcileBindings(recCtx context.Context, ba common.Bas func (r *ReconcilerBase) reconcileExpose(recCtx context.Context, ba common.BaseComponent) error { mObj := ba.(metav1.Object) - bindingSecret := common.NewSecret(recCtx, getExposeBindingSecretName(ba), mObj.GetNamespace()) + bindingSecret, wg := common.NewWaitableSecret(recCtx, getExposeBindingSecretName(ba), mObj.GetNamespace()) if ba.GetService() != nil && ba.GetService().GetBindable() != nil && *ba.GetService().GetBindable() { - err := r.CreateOrUpdate(bindingSecret, mObj, func() error { + err := r.TrackedCreateOrUpdate(bindingSecret, mObj, func() error { customSecret := &corev1.Secret{} // Check if custom values are provided in a secret, and apply the custom values if err := r.getCustomValuesToExpose(customSecret, ba); err != nil { @@ -43,7 +43,7 @@ func (r *ReconcilerBase) reconcileExpose(recCtx context.Context, ba common.BaseC // Apply default values to the override secret if certain values are not set r.applyDefaultValuesToExpose(recCtx, bindingSecret, ba) return nil - }) + }, wg) if err != nil { return err } From 529c4cb99a67abb8a535c79bd0be836693c5d6d5 Mon Sep 17 00:00:00 2001 From: Kirby Chin <37311900+kabicin@users.noreply.github.com> Date: Tue, 17 Mar 2026 13:37:50 -0400 Subject: [PATCH 3/7] Use memguard locked buffer for Secret operations --- cmd/main.go | 4 + common/secret.go | 119 +++++++++++++++------------- go.mod | 2 + go.sum | 4 + utils/hash.go | 52 +++++++++++- utils/reconciler.go | 79 ++++++++++-------- utils/service_binding_reconciler.go | 82 ++++++++++--------- utils/utils.go | 13 ++- 8 files changed, 222 insertions(+), 133 deletions(-) diff --git a/cmd/main.go b/cmd/main.go index fa1745a49..66239a1f7 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -36,6 +36,7 @@ import ( "github.com/application-stacks/runtime-component-operator/common" "github.com/application-stacks/runtime-component-operator/internal/controller" "github.com/application-stacks/runtime-component-operator/utils" + "github.com/awnumar/memguard" certmanagerv1 "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1" imagev1 "github.com/openshift/api/image/v1" routev1 "github.com/openshift/api/route/v1" @@ -68,6 +69,9 @@ func init() { } func main() { + memguard.CatchInterrupt() + defer memguard.Purge() + var metricsAddr string var enableLeaderElection bool var probeAddr string diff --git a/common/secret.go b/common/secret.go index 6bc5613dc..f4ac1b432 100644 --- a/common/secret.go +++ b/common/secret.go @@ -2,81 +2,92 @@ package common import ( "context" - "sync" + "fmt" + "github.com/awnumar/memguard" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" ) -type SecretResource struct { - secret *corev1.Secret - once sync.Once +type LockedBufferSecret struct { + metav1.TypeMeta `json:",inline"` + metav1.ObjectMeta `json:"metadata,omitempty"` + LockedData SecretMap `json:"lockedData,omitempty"` } -type WaitableSecretResource struct { - *SecretResource - wg sync.WaitGroup -} +type SecretMap map[string]*memguard.LockedBuffer -// Creates a Secret that clears it's data when recCtx is canceled -func NewSecret(recCtx context.Context, name, namespace string) *corev1.Secret { - secretResource := NewSecretResource(name, namespace) - go func() { - <-recCtx.Done() - secretResource.Clear(nil) - }() - return secretResource.GetSecret() +func (sm SecretMap) Destroy() { + for _, buf := range sm { + buf.Destroy() + } } -func NewWaitableSecret(recCtx context.Context, name, namespace string) (*corev1.Secret, *sync.WaitGroup) { - waitableSecretResource := NewWaitableSecretResource(name, namespace) - go func() { - <-recCtx.Done() - waitableSecretResource.Clear(waitableSecretResource.GetWaitGroup()) - }() - return waitableSecretResource.GetSecret(), waitableSecretResource.GetWaitGroup() +func (sm SecretMap) Get(key string) ([]byte, bool) { + if buf, found := sm[key]; found { + return buf.Bytes(), true + } + return []byte{}, false } -func NewSecretResource(name, namespace string) *SecretResource { - resource := &SecretResource{ - secret: &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: name, - Namespace: namespace, - }, - }, +func (lockedBufferSecret LockedBufferSecret) Destroy() { + if lockedBufferSecret.LockedData != nil { + lockedBufferSecret.LockedData.Destroy() } - return resource } -func NewWaitableSecretResource(name, namespace string) *WaitableSecretResource { - resource := &WaitableSecretResource{ - SecretResource: NewSecretResource(name, namespace), - wg: sync.WaitGroup{}, +// Gets a Secret from the k8s client loaded as a LockedBufferSecret +func GetSecret(client client.Client, name string, ns string) (*LockedBufferSecret, error) { + if client == nil { + return nil, fmt.Errorf("the reconciler client could not be found") + } + secret := &corev1.Secret{} + secret.Name = name + secret.Namespace = ns + err := client.Get(context.TODO(), types.NamespacedName{Name: name, Namespace: ns}, secret) + if err != nil { + return nil, err } - return resource -} -func (wsr *WaitableSecretResource) GetWaitGroup() *sync.WaitGroup { - return &wsr.wg + lockedBufferSecret := &LockedBufferSecret{} + lockedBufferSecret.TypeMeta = secret.TypeMeta + lockedBufferSecret.ObjectMeta = secret.ObjectMeta + for secretKey, secretValue := range secret.Data { + lockedBufferSecret.LockedData[secretKey] = memguard.NewBufferFromBytes(secretValue) + } + return lockedBufferSecret, nil } -func (r *SecretResource) GetSecret() *corev1.Secret { - return r.secret +// Copies a Locked Buffer Secret into a core Secret with a corresponding cleanup func +func CopySecret(in *LockedBufferSecret, out *corev1.Secret) func() { + out.TypeMeta = in.TypeMeta + out.ObjectMeta = in.ObjectMeta + for key, buf := range in.LockedData { + out.Data[key] = buf.Bytes() + } + return func() { + for key := range out.Data { + delete(out.Data, key) + } + out.Data = nil + } } -func (r *SecretResource) Clear(wg *sync.WaitGroup) { - if r.secret == nil { - return +// Returns the client.Get status of Secret name in namespace ns after clearing sensitive byte array content +func CheckSecret(client client.Client, name string, ns string) error { + if client == nil { + return fmt.Errorf("the reconciler client could not be found") } - r.once.Do(func() { - if wg != nil { - wg.Wait() - } - for secretKey, secretValue := range r.secret.Data { - clear(secretValue) - delete(r.secret.Data, secretKey) - } - }) + secret := &corev1.Secret{} + secret.Name = name + secret.Namespace = ns + err := client.Get(context.TODO(), types.NamespacedName{Name: name, Namespace: ns}, secret) + for key, value := range secret.Data { + clear(value) + delete(secret.Data, key) + } + return err } diff --git a/go.mod b/go.mod index 28fef5825..d4e7d0dce 100644 --- a/go.mod +++ b/go.mod @@ -3,6 +3,7 @@ module github.com/application-stacks/runtime-component-operator go 1.26 require ( + github.com/awnumar/memguard v0.23.0 github.com/cert-manager/cert-manager v1.19.4 github.com/go-logr/logr v1.4.3 github.com/openshift/api v0.0.0-20260304172252-b0658d22beea @@ -27,6 +28,7 @@ require ( ) require ( + github.com/awnumar/memcall v0.4.0 // indirect github.com/beorn7/perks v1.0.1 // indirect github.com/blang/semver/v4 v4.0.0 // indirect github.com/blendle/zapdriver v1.3.1 // indirect diff --git a/go.sum b/go.sum index a6c98428e..0e57ff946 100644 --- a/go.sum +++ b/go.sum @@ -1,5 +1,9 @@ github.com/armon/go-socks5 v0.0.0-20160902184237-e75332964ef5 h1:0CwZNZbxp69SHPdPJAN/hZIm0C4OItdklCFmMRWYpio= github.com/armon/go-socks5 v0.0.0-20160902184237-e75332964ef5/go.mod h1:wHh0iHkYZB8zMSxRWpUBQtwG5a7fFgvEO+odwuTv2gs= +github.com/awnumar/memcall v0.4.0 h1:B7hgZYdfH6Ot1Goaz8jGne/7i8xD4taZie/PNSFZ29g= +github.com/awnumar/memcall v0.4.0/go.mod h1:8xOx1YbfyuCg3Fy6TO8DK0kZUua3V42/goA5Ru47E8w= +github.com/awnumar/memguard v0.23.0 h1:sJ3a1/SWlcuKIQ7MV+R9p0Pvo9CWsMbGZvcZQtmc68A= +github.com/awnumar/memguard v0.23.0/go.mod h1:olVofBrsPdITtJ2HgxQKrEYEMyIBAIciVG4wNnZhW9M= github.com/beorn7/perks v1.0.1 h1:VlbKKnNfV8bJzeqoa4cOKqO6bYr3WgKZxO8Z16+hsOM= github.com/beorn7/perks v1.0.1/go.mod h1:G2ZrVWU2WbWT9wwq4/hrbKbnv/1ERSJQ0ibhJ6rlkpw= github.com/blang/semver/v4 v4.0.0 h1:1PFHFE6yCCTv8C1TeyNNarDzntLi7wMI5i/pzqYIsAM= diff --git a/utils/hash.go b/utils/hash.go index e8b9ed577..c8425a816 100644 --- a/utils/hash.go +++ b/utils/hash.go @@ -4,22 +4,66 @@ import ( "bytes" "encoding/hex" "io" + "runtime" "sort" + "github.com/application-stacks/runtime-component-operator/common" "lukechampine.com/blake3" ) +func HashSecretMapData(data common.SecretMap) string { + return hash(data, serializeSecretMapData) +} + func HashData(data map[string][]byte) string { + return hash(data, serializeSecretData) +} + +func hash(data any, serializer func(any) []byte) string { hasher := blake3.New(32, nil) - io.Copy(hasher, bytes.NewReader(serializeSecretData(data))) + secretBytes := serializer(data) + io.Copy(hasher, bytes.NewReader(secretBytes)) + // clear secret bytes + for i := range secretBytes { + secretBytes[i] = 0 + } hash := hasher.Sum(nil) + // force gc + hasher = nil + runtime.GC() return hex.EncodeToString(hash) } -func serializeSecretData(data map[string][]byte) []byte { +func serializeSecretMapData(data any) []byte { + if _, ok := data.(common.SecretMap); !ok { + return []byte{} + } + dataObj := data.(common.SecretMap) + // sort data keys + dataKeys := []string{} + for k := range dataObj { + dataKeys = append(dataKeys, k) + } + sort.Strings(dataKeys) + // load dataBuffer delimited by a null character for every key-value pair \0\0 + dataBuffer := []byte{} + for _, k := range dataKeys { + dataBuffer = append(dataBuffer, []byte(k)...) + dataBuffer = append(dataBuffer, '\000') + dataBuffer = append(dataBuffer, dataObj[k].Bytes()...) + dataBuffer = append(dataBuffer, '\000') + } + return dataBuffer +} + +func serializeSecretData(data any) []byte { + if _, ok := data.(map[string][]byte); !ok { + return []byte{} + } + dataObj := data.(map[string][]byte) // sort data keys dataKeys := []string{} - for k := range data { + for k := range dataObj { dataKeys = append(dataKeys, k) } sort.Strings(dataKeys) @@ -28,7 +72,7 @@ func serializeSecretData(data map[string][]byte) []byte { for _, k := range dataKeys { dataBuffer = append(dataBuffer, []byte(k)...) dataBuffer = append(dataBuffer, '\000') - dataBuffer = append(dataBuffer, data[k]...) + dataBuffer = append(dataBuffer, dataObj[k]...) dataBuffer = append(dataBuffer, '\000') } return dataBuffer diff --git a/utils/reconciler.go b/utils/reconciler.go index d5921c142..768c12e2c 100644 --- a/utils/reconciler.go +++ b/utils/reconciler.go @@ -7,7 +7,6 @@ import ( "fmt" "math" "strconv" - "sync" "time" networkingv1 "k8s.io/api/networking/v1" @@ -173,13 +172,29 @@ func (r *ReconcilerBase) CreateOrUpdate(obj client.Object, owner metav1.Object, return err } -// Runs CreateOrUpdate tracked by a WaitGroup wg -func (r *ReconcilerBase) TrackedCreateOrUpdate(obj client.Object, owner metav1.Object, reconcile func() error, wg *sync.WaitGroup) error { - if wg != nil { - wg.Add(1) - defer wg.Done() +func (r *ReconcilerBase) CreateOrUpdateSecret(secret *common.LockedBufferSecret, owner metav1.Object) error { + // populate Secret obj with the locked buffer secret contents + obj := &corev1.Secret{} + objCleanup := common.CopySecret(secret, obj) + // once completed, delete references to protected memory + defer objCleanup() + + if owner != nil { + controllerutil.SetControllerReference(owner, obj, r.scheme) + } + + result, err := controllerutil.CreateOrUpdate(context.TODO(), r.GetClient(), obj, func() error { return nil }) + if err != nil { + return err } - return r.CreateOrUpdate(obj, owner, reconcile) + + var gvk schema.GroupVersionKind + gvk, err = apiutil.GVKForObject(obj, r.scheme) + if err == nil { + logD1.Info("Reconciled", "Kind", gvk.Kind, "Namespace", obj.GetNamespace(), "Name", obj.GetName(), "Status", result) + } + + return err } // DeleteResource deletes kubernetes resource @@ -208,6 +223,13 @@ func (r *ReconcilerBase) DeleteResource(obj client.Object) error { return nil } +func (r *ReconcilerBase) DeleteSecretResource(secret *common.LockedBufferSecret) error { + obj := &corev1.Secret{} + obj.TypeMeta = secret.TypeMeta + obj.ObjectMeta = secret.ObjectMeta + return r.DeleteResource(obj) +} + // DeleteResources ... func (r *ReconcilerBase) DeleteResources(resources []client.Object) error { for i := range resources { @@ -532,35 +554,35 @@ func (r *ReconcilerBase) GetRouteTLSValues(ba common.BaseComponent) (key string, key, cert, ca, destCa = "", "", "", "" mObj := ba.(metav1.Object) if ba.GetManageTLS() == nil || *ba.GetManageTLS() || ba.GetService() != nil && ba.GetService().GetCertificateSecretRef() != nil { - tlsSecret := &corev1.Secret{} secretName := ba.GetStatus().GetReferences()[common.StatusReferenceCertSecretName] - err := r.GetClient().Get(context.TODO(), types.NamespacedName{Name: secretName, Namespace: mObj.GetNamespace()}, tlsSecret) + tlsSecret, err := common.GetSecret(r.GetClient(), secretName, mObj.GetNamespace()) + defer tlsSecret.Destroy() if err != nil { r.ManageError(err, common.StatusConditionTypeReconciled, ba) return "", "", "", "", err } - caCrt, ok := tlsSecret.Data["ca.crt"] + caCrt, ok := tlsSecret.LockedData["ca.crt"] if ok { - destCa = string(caCrt) + destCa = string(caCrt.Bytes()) } } if ba.GetRoute() != nil && ba.GetRoute().GetCertificateSecretRef() != nil { - tlsSecret := &corev1.Secret{} secretName := mObj.GetName() + "-route-tls" if ba.GetRoute().GetCertificateSecretRef() != nil { secretName = *ba.GetRoute().GetCertificateSecretRef() } - err := r.GetClient().Get(context.TODO(), types.NamespacedName{Name: secretName, Namespace: mObj.GetNamespace()}, tlsSecret) + tlsSecret, err := common.GetSecret(r.GetClient(), secretName, mObj.GetNamespace()) + defer tlsSecret.Destroy() if err != nil { r.ManageError(err, common.StatusConditionTypeReconciled, ba) return "", "", "", "", err } var extraCerts string - v, ok := tlsSecret.Data["tls.crt"] + v, ok := tlsSecret.LockedData["tls.crt"] if ok { - cert = string(v) - certBlock, nextCerts := pem.Decode(v) + cert = string(v.Bytes()) + certBlock, nextCerts := pem.Decode(v.Bytes()) if certBlock == nil { return "", "", "", "", errors.New("failed to load route certificate") } @@ -570,9 +592,9 @@ func (r *ReconcilerBase) GetRouteTLSValues(ba common.BaseComponent) (key string, extraCerts = string(nextCerts) } } - v, ok = tlsSecret.Data["ca.crt"] + v, ok = tlsSecret.LockedData["ca.crt"] if ok || extraCerts != "" { - ca = string(v) + ca = string(v.Bytes()) if extraCerts != "" { if ca != "" { ca = ca + "\n" + extraCerts @@ -581,13 +603,13 @@ func (r *ReconcilerBase) GetRouteTLSValues(ba common.BaseComponent) (key string, } } } - v, ok = tlsSecret.Data["tls.key"] + v, ok = tlsSecret.LockedData["tls.key"] if ok { - key = string(v) + key = string(v.Bytes()) } - v, ok = tlsSecret.Data["destCA.crt"] + v, ok = tlsSecret.LockedData["destCA.crt"] if ok { - destCa = string(v) + destCa = string(v.Bytes()) } } return key, cert, ca, destCa, nil @@ -631,11 +653,6 @@ func (r *ReconcilerBase) checkIssuerReady(issuer *certmanagerv1.Issuer) error { return nil } -func (r *ReconcilerBase) checkSecretExists(recCtx context.Context, secretName, secretNamespace string) error { - secret := common.NewSecret(recCtx, secretName, secretNamespace) - return r.GetClient().Get(context.TODO(), types.NamespacedName{Name: secret.Name, Namespace: secret.Namespace}, secret) -} - func (r *ReconcilerBase) GenerateCMIssuer(recCtx context.Context, namespace string, prefix string, CACommonName string, operatorName string) error { if ok, err := r.IsGroupVersionSupported(certmanagerv1.SchemeGroupVersion.String(), "Issuer"); err != nil { return err @@ -687,9 +704,9 @@ func (r *ReconcilerBase) GenerateCMIssuer(recCtx context.Context, namespace stri return err } - customCACert := common.NewSecret(recCtx, prefix+"-custom-ca-tls", namespace) + customCACertName := prefix + "-custom-ca-tls" + err = common.CheckSecret(r.GetClient(), customCACertName, namespace) customCACertFound := false - err = r.GetClient().Get(context.Background(), types.NamespacedName{Name: customCACert.Name, Namespace: customCACert.Namespace}, customCACert) if err == nil { customCACertFound = true } else { @@ -697,7 +714,7 @@ func (r *ReconcilerBase) GenerateCMIssuer(recCtx context.Context, namespace stri if err := r.checkCertificateReady(caCert); err != nil { return err } - if err := r.checkSecretExists(recCtx, caCertSecretName, namespace); err != nil { + if err := common.CheckSecret(r.GetClient(), caCertSecretName, namespace); err != nil { return err } } @@ -714,7 +731,7 @@ func (r *ReconcilerBase) GenerateCMIssuer(recCtx context.Context, namespace stri issuer.Annotations = map[string]string{} } if customCACertFound { - issuer.Spec.CA.SecretName = customCACert.Name + issuer.Spec.CA.SecretName = customCACertName } return nil }) diff --git a/utils/service_binding_reconciler.go b/utils/service_binding_reconciler.go index cbc93b359..b28fde0a7 100644 --- a/utils/service_binding_reconciler.go +++ b/utils/service_binding_reconciler.go @@ -7,9 +7,9 @@ import ( "strings" "github.com/application-stacks/runtime-component-operator/common" + "github.com/awnumar/memguard" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" ) @@ -29,24 +29,29 @@ func (r *ReconcilerBase) ReconcileBindings(recCtx context.Context, ba common.Bas func (r *ReconcilerBase) reconcileExpose(recCtx context.Context, ba common.BaseComponent) error { mObj := ba.(metav1.Object) - bindingSecret, wg := common.NewWaitableSecret(recCtx, getExposeBindingSecretName(ba), mObj.GetNamespace()) + bindingSecret, err := common.GetSecret(r.GetClient(), getExposeBindingSecretName(ba), mObj.GetNamespace()) + defer bindingSecret.Destroy() + if err != nil { + return err + } if ba.GetService() != nil && ba.GetService().GetBindable() != nil && *ba.GetService().GetBindable() { - err := r.TrackedCreateOrUpdate(bindingSecret, mObj, func() error { - customSecret := &corev1.Secret{} - // Check if custom values are provided in a secret, and apply the custom values - if err := r.getCustomValuesToExpose(customSecret, ba); err != nil { - return err - } - // Use content of the 'override' secret as the base secret content - bindingSecret.Data = customSecret.Data - // Apply default values to the override secret if certain values are not set - r.applyDefaultValuesToExpose(recCtx, bindingSecret, ba) - return nil - }, wg) + customSecret := &common.LockedBufferSecret{} + defer customSecret.Destroy() + // Check if custom values are provided in a secret, and apply the custom values + err := r.getCustomValuesToExpose(customSecret, ba) if err != nil { return err } + // Use content of the 'override' secret as the base secret content + bindingSecret.LockedData = customSecret.LockedData + customSecret.LockedData = nil + // Apply default values to the override secret if certain values are not set + r.applyDefaultValuesToExpose(recCtx, bindingSecret, ba) + + if err := r.CreateOrUpdateSecret(bindingSecret, mObj); err != nil { + return err + } // Update binding status r.updateBindingStatus(bindingSecret.Name, ba) @@ -56,55 +61,57 @@ func (r *ReconcilerBase) reconcileExpose(recCtx context.Context, ba common.BaseC // Update status r.updateBindingStatus("", ba) // Remove binding secret - if err := r.DeleteResource(bindingSecret); client.IgnoreNotFound(err) != nil { + if err := r.DeleteSecretResource(bindingSecret); client.IgnoreNotFound(err) != nil { return err } return nil } -func (r *ReconcilerBase) getCustomValuesToExpose(secret *corev1.Secret, ba common.BaseComponent) error { +func (r *ReconcilerBase) getCustomValuesToExpose(secret *common.LockedBufferSecret, ba common.BaseComponent) error { mObj := ba.(metav1.Object) - key := types.NamespacedName{Name: getOverrideExposeBindingSecretName(ba), Namespace: mObj.GetNamespace()} - err := r.GetClient().Get(context.TODO(), key, secret) + overrideExposeBindingSecret, err := common.GetSecret(r.GetClient(), getOverrideExposeBindingSecretName(ba), mObj.GetNamespace()) if client.IgnoreNotFound(err) != nil { return err } + if overrideExposeBindingSecret != nil { + secret = overrideExposeBindingSecret + } return nil } -func (r *ReconcilerBase) applyDefaultValuesToExpose(recCtx context.Context, secret *corev1.Secret, ba common.BaseComponent) { +func (r *ReconcilerBase) applyDefaultValuesToExpose(recCtx context.Context, secret *common.LockedBufferSecret, ba common.BaseComponent) { mObj := ba.(metav1.Object) secret.Labels = ba.GetLabels() secret.Annotations = MergeMaps(secret.Annotations, ba.GetAnnotations()) - if secret.Data == nil { - secret.Data = map[string][]byte{} + if secret.LockedData == nil { + secret.LockedData = common.SecretMap{} } - secretData := secret.Data + secretData := secret.LockedData var host, protocol, basePath, port []byte var found bool - if host, found = secretData["host"]; !found { + if host, found := secretData.Get("host"); !found { host = []byte(fmt.Sprintf("%s.%s.svc.cluster.local", mObj.GetName(), mObj.GetNamespace())) - secretData["host"] = host + secretData["host"] = memguard.NewBufferFromBytes(host) } - if protocol, found = secretData["protocol"]; !found { + if protocol, found = secretData.Get("protocol"); !found { if ba.GetManageTLS() == nil || *ba.GetManageTLS() { protocol = []byte("https") } else { protocol = []byte("http") } - secretData["protocol"] = protocol + secretData["protocol"] = memguard.NewBufferFromBytes(protocol) } - if basePath, found = secretData["basePath"]; !found { + if basePath, found = secretData.Get("basePath"); !found { basePath = []byte("/") - secretData["basePath"] = basePath + secretData["basePath"] = memguard.NewBufferFromBytes(basePath) } - if port, found = secretData["port"]; !found { + if port, found = secretData.Get("port"); !found { if ba.GetCreateKnativeService() == nil || !*ba.GetCreateKnativeService() { port = []byte(strconv.Itoa(int(ba.GetService().GetPort()))) } - secretData["port"] = port + secretData["port"] = memguard.NewBufferFromBytes(port) } if _, found = secretData["uri"]; !found { uri := []byte(fmt.Sprintf("%s://%s", protocol, host)) @@ -117,27 +124,28 @@ func (r *ReconcilerBase) applyDefaultValuesToExpose(recCtx context.Context, secr basePathStr = strings.TrimPrefix(basePathStr, "/") uri = []byte(fmt.Sprintf("%s/%s", uri, basePathStr)) } - secretData["uri"] = uri + secretData["uri"] = memguard.NewBufferFromBytes(uri) } if _, found = secretData["certificates"]; !found && ba.GetStatus().GetReferences()[common.StatusReferenceCertSecretName] != "" { - certSecret := common.NewSecret(recCtx, ba.GetStatus().GetReferences()[common.StatusReferenceCertSecretName], mObj.GetNamespace()) - err := r.GetClient().Get(context.TODO(), types.NamespacedName{Name: certSecret.Name, Namespace: certSecret.Namespace}, certSecret) + certSecret, err := common.GetSecret(r.GetClient(), ba.GetStatus().GetReferences()[common.StatusReferenceCertSecretName], mObj.GetNamespace()) + defer certSecret.LockedData.Destroy() if err == nil { - caCert := certSecret.Data["ca.crt"] - tlsCrt := certSecret.Data["tls.crt"] + caCert, _ := certSecret.LockedData.Get("ca.crt") + tlsCrt, _ := certSecret.LockedData.Get("tls.crt") + chainedCerts := make([]byte, len(caCert)+len(tlsCrt)) nCount := copy(chainedCerts, tlsCrt) nCount += copy(chainedCerts[len(tlsCrt):], caCert) if nCount > 0 { - secretData["certificates"] = chainedCerts + secretData["certificates"] = memguard.NewBufferFromBytes(chainedCerts) } } } if _, found = secretData["ingress-uri"]; !found && ba.GetExpose() != nil && *ba.GetExpose() { host, path, protocol := r.GetIngressInfo(ba) - secretData["ingress-uri"] = []byte(fmt.Sprintf("%s://%s%s%s", protocol, host, path, string(basePath))) + secretData["ingress-uri"] = memguard.NewBufferFromBytes([]byte(fmt.Sprintf("%s://%s%s%s", protocol, host, path, string(basePath)))) } } diff --git a/utils/utils.go b/utils/utils.go index 43b91608f..401ef3ff9 100644 --- a/utils/utils.go +++ b/utils/utils.go @@ -1160,8 +1160,9 @@ func ValidatePrometheusMonitoringEndpoints(recCtx context.Context, ba common.Bas } // Error if any Secret is specified but does not exist for _, secretName := range secretNames { - secret := common.NewSecret(recCtx, secretName, namespace) - if err := client.Get(context.TODO(), types.NamespacedName{Name: secretName, Namespace: namespace}, secret); err != nil { + secret, err := common.GetSecret(client, secretName, namespace) + secret.LockedData.Destroy() + if err != nil { errorMessage := fmt.Sprintf("Could not find Secret '%s' in this namespace.", secretName) return errors.New(errorMessage) } @@ -1657,9 +1658,7 @@ func ServiceAccountPullSecretExists(recCtx context.Context, ba common.BaseCompon // if this is our service account there will be one image pull secret // For others there could be more. either way, just use the first? sName := secrets[0].Name - pullSecret := common.NewSecret(recCtx, sName, ns) - err := client.Get(context.TODO(), types.NamespacedName{Name: pullSecret.Name, Namespace: pullSecret.Namespace}, pullSecret) - if err != nil { + if err := common.CheckSecret(client, sName, ns); err != nil { saErr := errors.New("Service account " + saName + " isn't ready. Reason: " + err.Error()) return saErr } @@ -1856,8 +1855,8 @@ func GetIssuerResourceVersion(recCtx context.Context, client client.Client, cert return "", err } if issuer.Spec.CA != nil { - caSecret := common.NewSecret(recCtx, issuer.Spec.CA.SecretName, certificate.Namespace) - err = client.Get(context.Background(), types.NamespacedName{Name: caSecret.Name, Namespace: caSecret.Namespace}, caSecret) + caSecret, err := common.GetSecret(client, issuer.Spec.CA.SecretName, certificate.Namespace) + caSecret.LockedData.Destroy() if err != nil { return "", err } else { From 9f2df813e6ccac5fb98027c8ec9b5f5a45a94bdd Mon Sep 17 00:00:00 2001 From: Kirby Chin <37311900+kabicin@users.noreply.github.com> Date: Tue, 17 Mar 2026 14:46:34 -0400 Subject: [PATCH 4/7] go mod tidy --- go.mod | 1 + go.sum | 2 ++ 2 files changed, 3 insertions(+) diff --git a/go.mod b/go.mod index d4e7d0dce..a29200875 100644 --- a/go.mod +++ b/go.mod @@ -24,6 +24,7 @@ require ( github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 // indirect github.com/rogpeppe/go-internal v1.14.1 // indirect go.yaml.in/yaml/v3 v3.0.4 // indirect + golang.org/x/crypto v0.47.0 // indirect sigs.k8s.io/structured-merge-diff/v6 v6.3.0 // indirect ) diff --git a/go.sum b/go.sum index 0e57ff946..eca651efa 100644 --- a/go.sum +++ b/go.sum @@ -147,6 +147,8 @@ go.yaml.in/yaml/v3 v3.0.4/go.mod h1:DhzuOOF2ATzADvBadXxruRBLzYTpT36CKvDb3+aBEFg= golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI= golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto= +golang.org/x/crypto v0.47.0 h1:V6e3FRj+n4dbpw86FJ8Fv7XVOql7TEwpHapKoMJ/GO8= +golang.org/x/crypto v0.47.0/go.mod h1:ff3Y9VzzKbwSSEzWqJsJVBnWmRwRSHt/6Op5n9bQc4A= golang.org/x/mod v0.2.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= golang.org/x/mod v0.3.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= golang.org/x/mod v0.32.0 h1:9F4d3PHLljb6x//jOyokMv3eX+YDeepZSEo3mFJy93c= From 05571bff9d98cd1bdba16ff908d69bb3ea12a3c6 Mon Sep 17 00:00:00 2001 From: Kirby Chin <37311900+kabicin@users.noreply.github.com> Date: Tue, 17 Mar 2026 15:02:12 -0400 Subject: [PATCH 5/7] Remove reconcile context in main controller --- common/secret.go | 26 ++++++++++++------- .../controller/runtimecomponent_controller.go | 19 +++++--------- utils/hash.go | 6 ++--- utils/reconciler.go | 8 +++--- utils/reconciler_test.go | 2 +- utils/service_binding_reconciler.go | 11 ++++---- utils/utils.go | 18 ++++++------- utils/utils_test.go | 5 ++-- 8 files changed, 48 insertions(+), 47 deletions(-) diff --git a/common/secret.go b/common/secret.go index f4ac1b432..336509de1 100644 --- a/common/secret.go +++ b/common/secret.go @@ -32,9 +32,9 @@ func (sm SecretMap) Get(key string) ([]byte, bool) { return []byte{}, false } -func (lockedBufferSecret LockedBufferSecret) Destroy() { - if lockedBufferSecret.LockedData != nil { - lockedBufferSecret.LockedData.Destroy() +func (lockedSecret LockedBufferSecret) Destroy() { + if lockedSecret.LockedData != nil { + lockedSecret.LockedData.Destroy() } } @@ -51,19 +51,27 @@ func GetSecret(client client.Client, name string, ns string) (*LockedBufferSecre return nil, err } - lockedBufferSecret := &LockedBufferSecret{} - lockedBufferSecret.TypeMeta = secret.TypeMeta - lockedBufferSecret.ObjectMeta = secret.ObjectMeta - for secretKey, secretValue := range secret.Data { - lockedBufferSecret.LockedData[secretKey] = memguard.NewBufferFromBytes(secretValue) + lockedSecret := &LockedBufferSecret{} + lockedSecret.TypeMeta = secret.TypeMeta + lockedSecret.ObjectMeta = secret.ObjectMeta + if lockedSecret.LockedData == nil { + lockedSecret.LockedData = SecretMap{} } - return lockedBufferSecret, nil + if secret.Data != nil { + for secretKey, secretValue := range secret.Data { + lockedSecret.LockedData[secretKey] = memguard.NewBufferFromBytes(secretValue) + } + } + return lockedSecret, nil } // Copies a Locked Buffer Secret into a core Secret with a corresponding cleanup func func CopySecret(in *LockedBufferSecret, out *corev1.Secret) func() { out.TypeMeta = in.TypeMeta out.ObjectMeta = in.ObjectMeta + if out.Data == nil { + out.Data = map[string][]byte{} + } for key, buf := range in.LockedData { out.Data[key] = buf.Bytes() } diff --git a/internal/controller/runtimecomponent_controller.go b/internal/controller/runtimecomponent_controller.go index c18fa0f31..861092b5a 100644 --- a/internal/controller/runtimecomponent_controller.go +++ b/internal/controller/runtimecomponent_controller.go @@ -101,11 +101,6 @@ func (r *RuntimeComponentReconciler) Reconcile(ctx context.Context, req ctrl.Req ns = r.watchNamespaces[0] } - // 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. - // Resources that require cleanup may use this context to hook into program execution before a function return. - recCtx, cancel := context.WithCancel(ctx) - defer cancel() - configMap, err := r.GetOpConfigMap(OperatorName, ns) if err != nil { reqLogger.Info("Failed to find runtime-component-operator config map") @@ -243,7 +238,7 @@ func (r *RuntimeComponentReconciler) Reconcile(ctx context.Context, req ctrl.Req if serviceAccountName == "" { serviceAccount := &corev1.ServiceAccount{ObjectMeta: defaultMeta} err = r.CreateOrUpdate(serviceAccount, instance, func() error { - return appstacksutils.CustomizeServiceAccount(recCtx, serviceAccount, instance, r.GetClient()) + return appstacksutils.CustomizeServiceAccount(serviceAccount, instance, r.GetClient()) }) if err != nil { reqLogger.Error(err, "Failed to reconcile ServiceAccount") @@ -262,7 +257,7 @@ func (r *RuntimeComponentReconciler) Reconcile(ctx context.Context, req ctrl.Req // Check if the ServiceAccount has a valid pull secret before creating the deployment/statefulset // or setting up knative. Otherwise the pods can go into an ImagePullBackOff loop - saErr := appstacksutils.ServiceAccountPullSecretExists(recCtx, instance, r.GetClient()) + saErr := appstacksutils.ServiceAccountPullSecretExists(instance, r.GetClient()) if saErr != nil { return r.ManageError(saErr, common.StatusConditionTypeReconciled, instance) } @@ -324,7 +319,7 @@ func (r *RuntimeComponentReconciler) Reconcile(ctx context.Context, req ctrl.Req } } - useCertmanager, err := r.GenerateSvcCertSecret(recCtx, ba, "rco", "Runtime Component Operator", "runtime-component-operator") + useCertmanager, err := r.GenerateSvcCertSecret(ba, "rco", "Runtime Component Operator", "runtime-component-operator") if err != nil { reqLogger.Error(err, "Failed to reconcile CertManager Certificate") return r.ManageError(err, common.StatusConditionTypeReconciled, instance) @@ -370,7 +365,7 @@ func (r *RuntimeComponentReconciler) Reconcile(ctx context.Context, req ctrl.Req } } - err = r.ReconcileBindings(recCtx, instance) + err = r.ReconcileBindings(instance) if err != nil { return r.ManageError(err, common.StatusConditionTypeReconciled, ba) } @@ -400,7 +395,7 @@ func (r *RuntimeComponentReconciler) Reconcile(ctx context.Context, req ctrl.Req err = r.CreateOrUpdate(statefulSet, instance, func() error { appstacksutils.CustomizeStatefulSet(statefulSet, instance) appstacksutils.CustomizePodSpec(&statefulSet.Spec.Template, instance) - if err := appstacksutils.CustomizePodWithSVCCertificate(recCtx, &statefulSet.Spec.Template, instance, r.GetClient()); err != nil { + if err := appstacksutils.CustomizePodWithSVCCertificate(&statefulSet.Spec.Template, instance, r.GetClient()); err != nil { return err } appstacksutils.CustomizePersistence(statefulSet, instance) @@ -432,7 +427,7 @@ func (r *RuntimeComponentReconciler) Reconcile(ctx context.Context, req ctrl.Req err = r.CreateOrUpdate(deploy, instance, func() error { appstacksutils.CustomizeDeployment(deploy, instance) appstacksutils.CustomizePodSpec(&deploy.Spec.Template, instance) - if err := appstacksutils.CustomizePodWithSVCCertificate(recCtx, &deploy.Spec.Template, instance, r.GetClient()); err != nil { + if err := appstacksutils.CustomizePodWithSVCCertificate(&deploy.Spec.Template, instance, r.GetClient()); err != nil { return err } return nil @@ -534,7 +529,7 @@ func (r *RuntimeComponentReconciler) Reconcile(ctx context.Context, req ctrl.Req } else if ok { if instance.Spec.Monitoring != nil && (instance.Spec.CreateKnativeService == nil || !*instance.Spec.CreateKnativeService) { // Validate the monitoring endpoints' configuration before creating/updating the ServiceMonitor - if err := appstacksutils.ValidatePrometheusMonitoringEndpoints(recCtx, instance, r.GetClient(), instance.GetNamespace()); err != nil { + if err := appstacksutils.ValidatePrometheusMonitoringEndpoints(instance, r.GetClient(), instance.GetNamespace()); err != nil { return r.ManageError(err, common.StatusConditionTypeReconciled, instance) } sm := &prometheusv1.ServiceMonitor{ObjectMeta: defaultMeta} diff --git a/utils/hash.go b/utils/hash.go index c8425a816..ef0446136 100644 --- a/utils/hash.go +++ b/utils/hash.go @@ -11,8 +11,8 @@ import ( "lukechampine.com/blake3" ) -func HashSecretMapData(data common.SecretMap) string { - return hash(data, serializeSecretMapData) +func HashLockedData(data common.SecretMap) string { + return hash(data, serializeLockedData) } func HashData(data map[string][]byte) string { @@ -34,7 +34,7 @@ func hash(data any, serializer func(any) []byte) string { return hex.EncodeToString(hash) } -func serializeSecretMapData(data any) []byte { +func serializeLockedData(data any) []byte { if _, ok := data.(common.SecretMap); !ok { return []byte{} } diff --git a/utils/reconciler.go b/utils/reconciler.go index 768c12e2c..2eec281ac 100644 --- a/utils/reconciler.go +++ b/utils/reconciler.go @@ -653,7 +653,7 @@ func (r *ReconcilerBase) checkIssuerReady(issuer *certmanagerv1.Issuer) error { return nil } -func (r *ReconcilerBase) GenerateCMIssuer(recCtx context.Context, namespace string, prefix string, CACommonName string, operatorName string) error { +func (r *ReconcilerBase) GenerateCMIssuer(namespace string, prefix string, CACommonName string, operatorName string) error { if ok, err := r.IsGroupVersionSupported(certmanagerv1.SchemeGroupVersion.String(), "Issuer"); err != nil { return err } else if !ok { @@ -750,7 +750,7 @@ func (r *ReconcilerBase) GenerateCMIssuer(recCtx context.Context, namespace stri return nil } -func (r *ReconcilerBase) GenerateSvcCertSecret(recCtx context.Context, ba common.BaseComponent, prefix string, CACommonName string, operatorName string) (bool, error) { +func (r *ReconcilerBase) GenerateSvcCertSecret(ba common.BaseComponent, prefix string, CACommonName string, operatorName string) (bool, error) { delete(ba.GetStatus().GetReferences(), common.StatusReferenceCertSecretName) cleanup := func() { if ok, err := r.IsGroupVersionSupported(certmanagerv1.SchemeGroupVersion.String(), "Certificate"); err != nil { @@ -791,7 +791,7 @@ func (r *ReconcilerBase) GenerateSvcCertSecret(recCtx context.Context, ba common } else if ok { bao := ba.(metav1.Object) - err = r.GenerateCMIssuer(recCtx, bao.GetNamespace(), prefix, CACommonName, operatorName) + err = r.GenerateCMIssuer(bao.GetNamespace(), prefix, CACommonName, operatorName) if err != nil { if errors.Is(err, APIVersionNotFoundError) { return false, nil @@ -854,7 +854,7 @@ func (r *ReconcilerBase) GenerateSvcCertSecret(recCtx context.Context, ba common svcCert.Spec.IssuerRef.Name = customIssuer.Name } - rVersion, _ := GetIssuerResourceVersion(recCtx, r.client, svcCert) + rVersion, _ := GetIssuerResourceVersion(r.client, svcCert) if svcCert.Spec.SecretTemplate == nil { svcCert.Spec.SecretTemplate = &certmanagerv1.CertificateSecretTemplate{ Annotations: map[string]string{}, diff --git a/utils/reconciler_test.go b/utils/reconciler_test.go index e1d318064..5cb68b0d6 100644 --- a/utils/reconciler_test.go +++ b/utils/reconciler_test.go @@ -196,7 +196,7 @@ func TestCreateOrUpdate(t *testing.T) { r := NewReconcilerBase(rcl, cl, s, &rest.Config{}, record.NewFakeRecorder(10)) err := r.CreateOrUpdate(serviceAccount, runtimecomponent, func() error { - CustomizeServiceAccount(context.TODO(), serviceAccount, runtimecomponent, r.GetClient()) + CustomizeServiceAccount(serviceAccount, runtimecomponent, r.GetClient()) return nil }) diff --git a/utils/service_binding_reconciler.go b/utils/service_binding_reconciler.go index b28fde0a7..f7a2351da 100644 --- a/utils/service_binding_reconciler.go +++ b/utils/service_binding_reconciler.go @@ -1,7 +1,6 @@ package utils import ( - "context" "fmt" "strconv" "strings" @@ -20,14 +19,14 @@ const ( ) // ReconcileBindings goes through the reconcile logic for service binding -func (r *ReconcilerBase) ReconcileBindings(recCtx context.Context, ba common.BaseComponent) error { - if err := r.reconcileExpose(recCtx, ba); err != nil { +func (r *ReconcilerBase) ReconcileBindings(ba common.BaseComponent) error { + if err := r.reconcileExpose(ba); err != nil { return err } return nil } -func (r *ReconcilerBase) reconcileExpose(recCtx context.Context, ba common.BaseComponent) error { +func (r *ReconcilerBase) reconcileExpose(ba common.BaseComponent) error { mObj := ba.(metav1.Object) bindingSecret, err := common.GetSecret(r.GetClient(), getExposeBindingSecretName(ba), mObj.GetNamespace()) defer bindingSecret.Destroy() @@ -47,7 +46,7 @@ func (r *ReconcilerBase) reconcileExpose(recCtx context.Context, ba common.BaseC bindingSecret.LockedData = customSecret.LockedData customSecret.LockedData = nil // Apply default values to the override secret if certain values are not set - r.applyDefaultValuesToExpose(recCtx, bindingSecret, ba) + r.applyDefaultValuesToExpose(bindingSecret, ba) if err := r.CreateOrUpdateSecret(bindingSecret, mObj); err != nil { return err @@ -79,7 +78,7 @@ func (r *ReconcilerBase) getCustomValuesToExpose(secret *common.LockedBufferSecr return nil } -func (r *ReconcilerBase) applyDefaultValuesToExpose(recCtx context.Context, secret *common.LockedBufferSecret, ba common.BaseComponent) { +func (r *ReconcilerBase) applyDefaultValuesToExpose(secret *common.LockedBufferSecret, ba common.BaseComponent) { mObj := ba.(metav1.Object) secret.Labels = ba.GetLabels() secret.Annotations = MergeMaps(secret.Annotations, ba.GetAnnotations()) diff --git a/utils/utils.go b/utils/utils.go index 401ef3ff9..5badc81b4 100644 --- a/utils/utils.go +++ b/utils/utils.go @@ -848,7 +848,7 @@ func CustomizePersistence(statefulSet *appsv1.StatefulSet, ba common.BaseCompone } // CustomizeServiceAccount ... -func CustomizeServiceAccount(recCtx context.Context, sa *corev1.ServiceAccount, ba common.BaseComponent, client client.Client) error { +func CustomizeServiceAccount(sa *corev1.ServiceAccount, ba common.BaseComponent, client client.Client) error { sa.Labels = ba.GetLabels() sa.Annotations = MergeMaps(sa.Annotations, ba.GetAnnotations()) @@ -1113,7 +1113,7 @@ func secretShouldExist(secretKeySelector corev1.SecretKeySelector) bool { } // Returns an error if any user specified non-optional Secret or ConfigMap for Prometheus monitoring does not exist, otherwise return nil. -func ValidatePrometheusMonitoringEndpoints(recCtx context.Context, ba common.BaseComponent, client client.Client, namespace string) error { +func ValidatePrometheusMonitoringEndpoints(ba common.BaseComponent, client client.Client, namespace string) error { var basicAuth *prometheusv1.BasicAuth var oauth2 *prometheusv1.OAuth2 var bearerTokenSecret corev1.SecretKeySelector @@ -1623,7 +1623,7 @@ func (r *ReconcilerBase) toJSONFromRaw(content *runtime.RawExtension) (map[strin // Looks for a pull secret in the service account retrieved from the component // Returns nil if there is at least one image pull secret, otherwise an error // Will always return nil if 'skipPullSecretValidation' is specified in the CR -func ServiceAccountPullSecretExists(recCtx context.Context, ba common.BaseComponent, client client.Client) error { +func ServiceAccountPullSecretExists(ba common.BaseComponent, client client.Client) error { obj := ba.(metav1.Object) ns := obj.GetNamespace() saName := obj.GetName() @@ -1769,15 +1769,15 @@ func CustomizePodWithSVCCertificate(pts *corev1.PodTemplateSpec, ba common.BaseC } func addSecretHashAsAnnotation(pts *corev1.PodTemplateSpec, object metav1.Object, client client.Client, secretName string, groupName string) error { - secret := &corev1.Secret{} - err := client.Get(context.Background(), types.NamespacedName{Name: secretName, Namespace: object.GetNamespace()}, secret) + secret, err := common.GetSecret(client, secretName, object.GetNamespace()) + defer secret.Destroy() if err != nil { return fmt.Errorf("Secret %q was not found in namespace %q, %w", secretName, object.GetNamespace(), err) } if pts.ObjectMeta.Annotations == nil { pts.ObjectMeta.Annotations = make(map[string]string) } - pts.ObjectMeta.Annotations[groupName+"/secret-"+secretName] = HashData(secret.Data) + pts.ObjectMeta.Annotations[groupName+"/secret-"+secretName] = HashLockedData(secret.LockedData) return nil } @@ -1847,7 +1847,7 @@ func UpdateConfigMap(configMap *corev1.ConfigMap, key string) { data[key] = common.LoadFromConfig(common.Config, key) } -func GetIssuerResourceVersion(recCtx context.Context, client client.Client, certificate *certmanagerv1.Certificate) (string, error) { +func GetIssuerResourceVersion(client client.Client, certificate *certmanagerv1.Certificate) (string, error) { issuer := &certmanagerv1.Issuer{} err := client.Get(context.Background(), types.NamespacedName{Name: certificate.Spec.IssuerRef.Name, Namespace: certificate.Namespace}, issuer) @@ -1856,11 +1856,11 @@ func GetIssuerResourceVersion(recCtx context.Context, client client.Client, cert } if issuer.Spec.CA != nil { caSecret, err := common.GetSecret(client, issuer.Spec.CA.SecretName, certificate.Namespace) - caSecret.LockedData.Destroy() + defer caSecret.Destroy() if err != nil { return "", err } else { - return issuer.ResourceVersion + "," + HashData(caSecret.Data), nil + return issuer.ResourceVersion + "," + HashLockedData(caSecret.LockedData), nil } } else { return issuer.ResourceVersion, nil diff --git a/utils/utils_test.go b/utils/utils_test.go index b3a454b2d..a3d58bb82 100644 --- a/utils/utils_test.go +++ b/utils/utils_test.go @@ -1,7 +1,6 @@ package utils import ( - "context" "os" "reflect" "strconv" @@ -536,13 +535,13 @@ func TestCustomizeServiceAccount(t *testing.T) { spec := appstacksv1.RuntimeComponentSpec{PullSecret: &pullSecret} sa, runtime := &corev1.ServiceAccount{}, createRuntimeComponent(name, namespace, spec) - CustomizeServiceAccount(context.TODO(), sa, runtime, fcl) + CustomizeServiceAccount(sa, runtime, fcl) emptySAIPS := sa.ImagePullSecrets[0].Name newSecret := "my-new-secret" spec = appstacksv1.RuntimeComponentSpec{PullSecret: &newSecret} runtime = createRuntimeComponent(name, namespace, spec) - CustomizeServiceAccount(context.TODO(), sa, runtime, fcl) + CustomizeServiceAccount(sa, runtime, fcl) testCSA := []Test{ {"ServiceAccount image pull secrets is empty", pullSecret, emptySAIPS}, From fa1a95503f888227485cfd450a85b9ab470afbdf Mon Sep 17 00:00:00 2001 From: Kirby Chin <37311900+kabicin@users.noreply.github.com> Date: Tue, 17 Mar 2026 15:18:10 -0400 Subject: [PATCH 6/7] Remove unsafe destroy on LockedData attribute --- utils/service_binding_reconciler.go | 2 +- utils/utils.go | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/utils/service_binding_reconciler.go b/utils/service_binding_reconciler.go index f7a2351da..346169573 100644 --- a/utils/service_binding_reconciler.go +++ b/utils/service_binding_reconciler.go @@ -128,7 +128,7 @@ func (r *ReconcilerBase) applyDefaultValuesToExpose(secret *common.LockedBufferS if _, found = secretData["certificates"]; !found && ba.GetStatus().GetReferences()[common.StatusReferenceCertSecretName] != "" { certSecret, err := common.GetSecret(r.GetClient(), ba.GetStatus().GetReferences()[common.StatusReferenceCertSecretName], mObj.GetNamespace()) - defer certSecret.LockedData.Destroy() + defer certSecret.Destroy() if err == nil { caCert, _ := certSecret.LockedData.Get("ca.crt") tlsCrt, _ := certSecret.LockedData.Get("tls.crt") diff --git a/utils/utils.go b/utils/utils.go index 5badc81b4..15f203c57 100644 --- a/utils/utils.go +++ b/utils/utils.go @@ -1160,8 +1160,7 @@ func ValidatePrometheusMonitoringEndpoints(ba common.BaseComponent, client clien } // Error if any Secret is specified but does not exist for _, secretName := range secretNames { - secret, err := common.GetSecret(client, secretName, namespace) - secret.LockedData.Destroy() + err := common.CheckSecret(client, secretName, namespace) if err != nil { errorMessage := fmt.Sprintf("Could not find Secret '%s' in this namespace.", secretName) return errors.New(errorMessage) From e42096da4d2961d1373dc325a9bc67969298223f Mon Sep 17 00:00:00 2001 From: Kirby Chin <37311900+kabicin@users.noreply.github.com> Date: Tue, 17 Mar 2026 15:20:38 -0400 Subject: [PATCH 7/7] Update serializeData function name --- utils/hash.go | 4 ++-- utils/hash_test.go | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/utils/hash.go b/utils/hash.go index ef0446136..81e5fbd25 100644 --- a/utils/hash.go +++ b/utils/hash.go @@ -16,7 +16,7 @@ func HashLockedData(data common.SecretMap) string { } func HashData(data map[string][]byte) string { - return hash(data, serializeSecretData) + return hash(data, serializeData) } func hash(data any, serializer func(any) []byte) string { @@ -56,7 +56,7 @@ func serializeLockedData(data any) []byte { return dataBuffer } -func serializeSecretData(data any) []byte { +func serializeData(data any) []byte { if _, ok := data.(map[string][]byte); !ok { return []byte{} } diff --git a/utils/hash_test.go b/utils/hash_test.go index a3f632631..dc0ad0e8a 100644 --- a/utils/hash_test.go +++ b/utils/hash_test.go @@ -15,7 +15,7 @@ func TestHashData(t *testing.T) { data["xyz"] = []byte("contentforxyz") data["abc"] = []byte("1Ag@aZ821Sd1asd1231nkgrniekghis168adf") testGHFD := []Test{ - {"Serialize sample data", []byte("abc\0001Ag@aZ821Sd1asd1231nkgrniekghis168adf\000xyz\000contentforxyz\000"), serializeSecretData(data)}, + {"Serialize sample data", []byte("abc\0001Ag@aZ821Sd1asd1231nkgrniekghis168adf\000xyz\000contentforxyz\000"), serializeData(data)}, {"Get hash from serialized data", "2d0b4d0adc4124bdfb959cb8b584473b5392cf2287b69a11663b288c90cfa010", HashData(data)}, } verifyTests(testGHFD, t)