Skip to content

THREESCALE-11948 Fix nil pointer in ApplicationCR #1116

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
tkan145 merged 6 commits into from
Dec 2, 2025
Merged

THREESCALE-11948 Fix nil pointer in ApplicationCR #1116

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
012b5f0
Update 3scale-porta-go-client to v0.12.0
tkan145 Oct 15, 2025
9f0fdac
Application: Remove duplicate functions
tkan145 Nov 26, 2025
c80d8fb
Application: simplify file structure
tkan145 Nov 26, 2025
1f22b35
Application: simplify sync logic
tkan145 Nov 26, 2025
134260e
Application: refactor finalizer logic
tkan145 Nov 27, 2025
9a516f4
Application: further simplify reconcile logic
tkan145 Nov 27, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
121 changes: 57 additions & 64 deletions controllers/capabilities/application_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,12 +54,11 @@ const (
// +kubebuilder:rbac:groups=capabilities.3scale.net,resources=applications/status,verbs=get;update;patch

func (r *ApplicationReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
_ = context.Background()
reqLogger := r.Logger().WithValues("application", req.NamespacedName)
reqLogger.Info("Reconcile Application", "Operator version", version.Version)

application := &capabilitiesv1beta1.Application{}
err := r.Client().Get(context.TODO(), req.NamespacedName, application)
err := r.Client().Get(ctx, req.NamespacedName, application)
if err != nil {
if apierrors.IsNotFound(err) {
// Request object not found, could have been deleted after reconcile request.
Expand All @@ -79,6 +78,26 @@ func (r *ApplicationReconciler) Reconcile(ctx context.Context, req ctrl.Request)
}
reqLogger.V(1).Info(string(jsonData))
}

// Ignore deleted Applications, this can happen when foregroundDeletion is enabled
// https://kubernetes.io/docs/concepts/workloads/controllers/garbage-collection/#foreground-cascading-deletion
if application.GetDeletionTimestamp() != nil {
if controllerutil.ContainsFinalizer(application, applicationFinalizer) {
err = r.removeApplicationFrom3scale(application)
if err != nil {
r.EventRecorder().Eventf(application, corev1.EventTypeWarning, "Failed to delete application", "%v", err)
return ctrl.Result{}, err
}

if controllerutil.RemoveFinalizer(application, applicationFinalizer) {
if err = r.UpdateResource(application); err != nil {
return ctrl.Result{}, err
}
}
}
return ctrl.Result{}, nil
}

// get Account
accountResource := &capabilitiesv1beta1.DeveloperAccount{}
projectMeta := types.NamespacedName{
Expand All @@ -89,83 +108,51 @@ func (r *ApplicationReconciler) Reconcile(ctx context.Context, req ctrl.Request)
err = r.Client().Get(r.Context(), projectMeta, accountResource)
if err != nil {
if apierrors.IsNotFound(err) {
// If the application CR is marked for deletion and the dev account associated doesn't exist, remove the application CR since there is nothing else to do with application.
if application.GetDeletionTimestamp() != nil && controllerutil.ContainsFinalizer(application, applicationFinalizer) {
controllerutil.RemoveFinalizer(application, applicationFinalizer)
err = r.UpdateResource(application)
if err != nil {
return ctrl.Result{}, err
}

return ctrl.Result{}, nil
}
reqLogger.Error(err, "error developer account not found ")
statusReconciler := NewApplicationStatusReconciler(r.BaseReconciler, application, nil, "", err)
statusResult, statusUpdateErr := statusReconciler.Reconcile()
if statusUpdateErr != nil {
if err != nil {
return ctrl.Result{}, fmt.Errorf("failed to reconcile application: %v. Failed to update application status: %w", err, statusUpdateErr)
}

return ctrl.Result{}, fmt.Errorf("failed to update application status: %w", statusUpdateErr)
return ctrl.Result{}, fmt.Errorf("failed to reconcile application: %v. Failed to update application status: %w", err, statusUpdateErr)
}
if statusResult.Requeue {
return statusResult, nil
}
}
}

// get providerAccountRef from account
providerAccount, err := controllerhelper.LookupProviderAccount(r.Client(), accountResource.Namespace, accountResource.Spec.ProviderAccountRef, r.Logger())
if err != nil {
return ctrl.Result{}, err
}

insecureSkipVerify := controllerhelper.GetInsecureSkipVerifyAnnotation(application.GetAnnotations())
threescaleAPIClient, err := controllerhelper.PortaClient(providerAccount, insecureSkipVerify)
metadataUpdated, err := r.reconcileMetadata(application, accountResource)
if err != nil {
r.EventRecorder().Eventf(application, corev1.EventTypeWarning, "Failed to update ownerReferences in application", "%v", err)
return ctrl.Result{}, err
}

// Ignore deleted Applications, this can happen when foregroundDeletion is enabled
// https://kubernetes.io/docs/concepts/workloads/controllers/garbage-collection/#foreground-cascading-deletion
if application.GetDeletionTimestamp() != nil && controllerutil.ContainsFinalizer(application, applicationFinalizer) {
err = r.removeApplicationFrom3scale(application, req, *threescaleAPIClient)
if err != nil {
r.EventRecorder().Eventf(application, corev1.EventTypeWarning, "Failed to delete application", "%v", err)
return ctrl.Result{}, err
}

controllerutil.RemoveFinalizer(application, applicationFinalizer)
if metadataUpdated {
err = r.UpdateResource(application)
if err != nil {
return ctrl.Result{}, err
}

// No need requeue because the reconcile will trigger automatically since updating the Application CR
return ctrl.Result{}, nil
}

if application.GetDeletionTimestamp() != nil {
return ctrl.Result{}, nil
// get providerAccountRef from account
providerAccount, err := controllerhelper.LookupProviderAccount(r.Client(), accountResource.Namespace, accountResource.Spec.ProviderAccountRef, r.Logger())
if err != nil {
return ctrl.Result{}, err
}

metadataUpdated, err := r.reconcileMetadata(application, accountResource)
insecureSkipVerify := controllerhelper.GetInsecureSkipVerifyAnnotation(application.GetAnnotations())
threescaleAPIClient, err := controllerhelper.PortaClient(providerAccount, insecureSkipVerify)
if err != nil {
r.EventRecorder().Eventf(application, corev1.EventTypeWarning, "Failed to update ownerReferences in application", "%v", err)
return ctrl.Result{}, err
}
if metadataUpdated {
err = r.UpdateResource(application)
if err != nil {
return ctrl.Result{}, err
}

// No need requeue because the reconcile will trigger automatically since updating the Application CR
return ctrl.Result{}, nil
}
applicationEntity, reconcileErr := r.applicationReconciler(application, accountResource, threescaleAPIClient)

statusReconciler, reconcileErr := r.applicationReconciler(application, req, threescaleAPIClient, providerAccount.AdminURLStr, accountResource)
statusReconciler := NewApplicationStatusReconciler(r.BaseReconciler, application, applicationEntity, providerAccount.AdminURLStr, reconcileErr)
statusResult, statusUpdateErr := statusReconciler.Reconcile()

if statusUpdateErr != nil {
if reconcileErr != nil {
return ctrl.Result{}, fmt.Errorf("failed to sync application: %v. Failed to update application status: %w", reconcileErr, statusUpdateErr)
Expand Down Expand Up @@ -236,53 +223,47 @@ func (r *ApplicationReconciler) reconcileMetadata(application *capabilitiesv1bet
return changed, nil
}

func (r *ApplicationReconciler) applicationReconciler(applicationResource *capabilitiesv1beta1.Application, req ctrl.Request, threescaleAPIClient *threescaleapi.ThreeScaleClient, providerAccountAdminURLStr string, accountResource *capabilitiesv1beta1.DeveloperAccount) (*ApplicationStatusReconciler, error) {
func (r *ApplicationReconciler) applicationReconciler(applicationResource *capabilitiesv1beta1.Application, accountResource *capabilitiesv1beta1.DeveloperAccount, threescaleAPIClient *threescaleapi.ThreeScaleClient) (*controllerhelper.ApplicationEntity, error) {
// get product
productResource := &capabilitiesv1beta1.Product{}
projectMeta := types.NamespacedName{
Name: applicationResource.Spec.ProductCR.Name,
Namespace: req.Namespace,
Namespace: applicationResource.Namespace,
}

err := r.Client().Get(r.Context(), projectMeta, productResource)
if err != nil {
if apierrors.IsNotFound(err) {
statusReconciler := NewApplicationStatusReconciler(r.BaseReconciler, applicationResource, nil, "", err)
return statusReconciler, err
return nil, err
}
}

err = r.checkExternalResources(applicationResource, accountResource, productResource)
if err != nil {
statusReconciler := NewApplicationStatusReconciler(r.BaseReconciler, applicationResource, nil, "", err)
return statusReconciler, err
return nil, err
}

reconciler := NewApplicationReconciler(r.BaseReconciler, applicationResource, accountResource, productResource, threescaleAPIClient)
ApplicationEntity, err := reconciler.Reconcile()
if err != nil {
statusReconciler := NewApplicationStatusReconciler(r.BaseReconciler, applicationResource, nil, providerAccountAdminURLStr, err)
return statusReconciler, err
}
statusReconciler := NewApplicationStatusReconciler(r.BaseReconciler, applicationResource, ApplicationEntity, providerAccountAdminURLStr, err)
return statusReconciler, err
reconciler := NewApplicationReconciler(r.BaseReconciler, applicationResource, *accountResource.Status.ID, *productResource.Status.ID, threescaleAPIClient)
return reconciler.Reconcile()
}

func (r *ApplicationReconciler) removeApplicationFrom3scale(application *capabilitiesv1beta1.Application, req ctrl.Request, threescaleAPIClient threescaleapi.ThreeScaleClient) error {
func (r *ApplicationReconciler) removeApplicationFrom3scale(application *capabilitiesv1beta1.Application) error {
logger := r.Logger().WithValues("application", client.ObjectKey{Name: application.Name, Namespace: application.Namespace})

// get Account
account := &capabilitiesv1beta1.DeveloperAccount{}
projectMeta := types.NamespacedName{
Name: application.Spec.AccountCR.Name,
Namespace: req.Namespace,
Namespace: application.Namespace,
}

err := r.Client().Get(r.Context(), projectMeta, account)
if err != nil {
if apierrors.IsNotFound(err) {
logger.Info("Account resource not found. Ignoring since object must have been deleted")
return nil
}
return err
}

// Attempt to remove application only if application.Status.ID is present
Expand All @@ -296,6 +277,18 @@ func (r *ApplicationReconciler) removeApplicationFrom3scale(application *capabil
return fmt.Errorf("could not remove application because ID is missing in the satus of developer account %s", account.Name)
}

// get providerAccountRef from account
providerAccount, err := controllerhelper.LookupProviderAccount(r.Client(), account.Namespace, account.Spec.ProviderAccountRef, r.Logger())
if err != nil {
return err
}

insecureSkipVerify := controllerhelper.GetInsecureSkipVerifyAnnotation(application.GetAnnotations())
threescaleAPIClient, err := controllerhelper.PortaClient(providerAccount, insecureSkipVerify)
if err != nil {
return err
}

err = threescaleAPIClient.DeleteApplication(*account.Status.ID, *application.Status.ID)
if err != nil && !threescaleapi.IsNotFound(err) {
return err
Expand Down
Loading