[FEAT] Operator: AppId handling enhanced

cmd/controller/main.go

@@ -126,11 +126,16 @@ func main() {
 
 				c := controller.NewController(coreClient, crdClient, istioClient, certClient, certManagerClient, dnsClient, promClient)
 
-				checkDone := make(chan bool, 1)
-				go checkDRs(checkDone, istioClient, crdClient)
-				<-checkDone
+				checkDR := make(chan bool, 1)
+				go checkDRs(checkDR, istioClient, crdClient)
+				<-checkDR
 				klog.InfoS("check & update of DestinationRules done")
 
+				migrateApp := make(chan bool, 1)
+				go migrateApps(migrateApp, crdClient)
+				<-migrateApp
+				klog.InfoS("migration of app Id done")
+
 				// Update the controller's concurrency config before starting the controller
 				maps.Copy(controller.DefaultConcurrentReconciles, concurrencyConfig)
 				go c.Start(ctx)

cmd/controller/tmp.go

@@ -12,6 +12,7 @@ import (
 	"strings"
 	"time"
 
+	"github.com/sap/cap-operator/internal/controller"
 	"github.com/sap/cap-operator/pkg/apis/sme.sap.com/v1alpha1"
 	"github.com/sap/cap-operator/pkg/client/clientset/versioned"
 	"google.golang.org/protobuf/types/known/durationpb"
@@ -26,10 +27,6 @@ import (
 	"k8s.io/klog/v2"
 )
 
-const (
-	LabelOwnerIdentifierHash = "sme.sap.com/owner-identifier-hash"
-)
-
 func checkDRs(checkDone chan bool, istioClient istio.Interface, crdClient versioned.Interface) {
 	// Always set the channel to true in the end
 	defer func() {
@@ -93,7 +90,7 @@ func checkDRs(checkDone chan bool, istioClient istio.Interface, crdClient versio
 	// Delete all DestinationRules in the tenant's namespace that have the relevant ownerId label
 	for _, tenant := range tenants.Items {
 		ownerIdentifierHash := sha1Sum(tenant.Namespace, tenant.Name)
-		ownerLabelHashReq, _ := labels.NewRequirement(LabelOwnerIdentifierHash, selection.Equals, []string{ownerIdentifierHash})
+		ownerLabelHashReq, _ := labels.NewRequirement(controller.LabelOwnerIdentifierHash, selection.Equals, []string{ownerIdentifierHash})
 		ownerLabelHashReqSelector := labels.NewSelector().Add(*ownerLabelHashReq)
 		err := istioClient.NetworkingV1().DestinationRules(tenant.Namespace).DeleteCollection(context.TODO(), metav1.DeleteOptions{}, metav1.ListOptions{
 			LabelSelector: ownerLabelHashReqSelector.String(),
@@ -120,3 +117,122 @@ func sha1Sum(source ...string) string {
 	sum := sha1.Sum([]byte(strings.Join(source, "")))
 	return fmt.Sprintf("%x", sum)
 }
+
+// migrateAppIdLabels replaces the deprecated BTP app identifier label/annotation with
+// the new app identifier label/annotation on the given ObjectMeta.
+func migrateAppIdLabels(object *metav1.ObjectMeta, appIdHash, appId string) {
+	if object.Labels == nil {
+		object.Labels = map[string]string{}
+	}
+	if object.Annotations == nil {
+		object.Annotations = map[string]string{}
+	}
+	object.Labels[controller.LabelAppIdHash] = appIdHash
+	delete(object.Labels, controller.LabelBTPApplicationIdentifierHash)
+	object.Annotations[controller.AnnotationAppId] = appId
+	delete(object.Annotations, controller.AnnotationBTPApplicationIdentifier)
+}
+
+func btpAppIdHashSelector(ca v1alpha1.CAPApplication) string {
+	return labels.SelectorFromSet(map[string]string{
+		controller.LabelBTPApplicationIdentifierHash: sha1Sum(ca.Spec.GlobalAccountId, ca.Spec.BTPAppName),
+	}).String()
+}
+
+func migrateCAPApplicationVersions(crdClient versioned.Interface, ca v1alpha1.CAPApplication, appIdHash, appId string) {
+	cavs, err := crdClient.SmeV1alpha1().CAPApplicationVersions(ca.Namespace).List(context.TODO(), metav1.ListOptions{
+		LabelSelector: btpAppIdHashSelector(ca),
+	})
+	if err != nil {
+		klog.ErrorS(err, "Failed to list CAPApplicationVersions", "capApplication", ca.Name, "namespace", ca.Namespace)
+		return
+	}
+	for _, cav := range cavs.Items {
+		cavCopy := cav.DeepCopy()
+		migrateAppIdLabels(&cavCopy.ObjectMeta, appIdHash, appId)
+		if _, err := crdClient.SmeV1alpha1().CAPApplicationVersions(cav.Namespace).Update(context.TODO(), cavCopy, metav1.UpdateOptions{}); err != nil {
+			klog.ErrorS(err, "Failed to update CAPApplicationVersion", "name", cav.Name, "namespace", cav.Namespace)
+		}
+	}
+}
+
+func migrateCAPTenants(crdClient versioned.Interface, ca v1alpha1.CAPApplication, appIdHash, appId string) {
+	cats, err := crdClient.SmeV1alpha1().CAPTenants(ca.Namespace).List(context.TODO(), metav1.ListOptions{
+		LabelSelector: btpAppIdHashSelector(ca),
+	})
+	if err != nil {
+		klog.ErrorS(err, "Failed to list CAPTenants", "capApplication", ca.Name, "namespace", ca.Namespace)
+		return
+	}
+	for _, cat := range cats.Items {
+		catCopy := cat.DeepCopy()
+		migrateAppIdLabels(&catCopy.ObjectMeta, appIdHash, appId)
+		if _, err := crdClient.SmeV1alpha1().CAPTenants(cat.Namespace).Update(context.TODO(), catCopy, metav1.UpdateOptions{}); err != nil {
+			klog.ErrorS(err, "Failed to update CAPTenant", "name", cat.Name, "namespace", cat.Namespace)
+		}
+	}
+}
+
+func migrateCAPTenantOperations(crdClient versioned.Interface, ca v1alpha1.CAPApplication, appIdHash, appId string) {
+	ctops, err := crdClient.SmeV1alpha1().CAPTenantOperations(ca.Namespace).List(context.TODO(), metav1.ListOptions{
+		LabelSelector: btpAppIdHashSelector(ca),
+	})
+	if err != nil {
+		klog.ErrorS(err, "Failed to list CAPTenants", "capApplication", ca.Name, "namespace", ca.Namespace)
+		return
+	}
+	for _, ctop := range ctops.Items {
+		ctopCopy := ctop.DeepCopy()
+		migrateAppIdLabels(&ctopCopy.ObjectMeta, appIdHash, appId)
+		if _, err := crdClient.SmeV1alpha1().CAPTenantOperations(ctop.Namespace).Update(context.TODO(), ctopCopy, metav1.UpdateOptions{}); err != nil {
+			klog.ErrorS(err, "Failed to update CAPTenant", "name", ctop.Name, "namespace", ctop.Namespace)
+		}
+	}
+}
+
+func needsMigration(ca *v1alpha1.CAPApplication, appIdHash string) bool {
+	if ca.Labels[controller.LabelAppIdHash] != appIdHash {
+		return true
+	}
+	if _, ok := ca.Labels[controller.LabelBTPApplicationIdentifierHash]; ok {
+		return true
+	}
+	return false
+}
+
+func migrateApps(migrationDone chan bool, crdClient versioned.Interface) {
+	// Always set the channel to true in the end
+	defer func() {
+		migrationDone <- true
+	}()
+
+	// Go over all CAP applications and check if spec has ProviderSubaccountId set, if so trigger update after setting LabelAppIdHash and AnnotationAppId and remove LabelBTPApplicationIdentifierHash & AnnotationBTPApplicationIdentifier from all CAs.
+	apps, err := crdClient.SmeV1alpha1().CAPApplications(metav1.NamespaceAll).List(context.TODO(), metav1.ListOptions{})
+	if err != nil {
+		klog.ErrorS(err, "Failed to list CAP applications")
+		return
+	}
+
+	for _, ca := range apps.Items {
+		if ca.Spec.ProviderSubaccountId == "" {
+			continue
+		}
+
+		appIdHash := sha1Sum(ca.Spec.ProviderSubaccountId, ca.Spec.BTPAppName)
+		appId := strings.Join([]string{ca.Spec.ProviderSubaccountId, ca.Spec.BTPAppName}, ".")
+
+		// Update the CAPApplication itself if the new label is not yet set
+		if needsMigration(&ca, appIdHash) {
+			caCopy := ca.DeepCopy()
+			migrateAppIdLabels(&caCopy.ObjectMeta, appIdHash, appId)
+			if _, err := crdClient.SmeV1alpha1().CAPApplications(ca.Namespace).Update(context.TODO(), caCopy, metav1.UpdateOptions{}); err != nil {
+				klog.ErrorS(err, "Failed to update CAPApplication", "name", ca.Name, "namespace", ca.Namespace)
+				continue
+			}
+		}
+
+		migrateCAPApplicationVersions(crdClient, ca, appIdHash, appId)
+		migrateCAPTenants(crdClient, ca, appIdHash, appId)
+		migrateCAPTenantOperations(crdClient, ca, appIdHash, appId)
+	}
+}

cmd/server/internal/handler.go

@@ -183,7 +183,7 @@ func (s *SubscriptionHandler) CreateTenant(reqInfo *RequestInfo) *Result {
 	}
 
 	// Check if A CRO for CAPTenant already exists
-	tenant := s.getTenantByBtpAppIdentifier(ca.Spec.GlobalAccountId, reqInfo.payload.appName, reqInfo.payload.tenantId, ca.Namespace, TenantProvisioning).Tenant
+	tenant := s.getTenantByAppIdentifier(ca.Spec.GlobalAccountId, ca.Spec.ProviderSubaccountId, reqInfo.payload.appName, reqInfo.payload.tenantId, ca.Namespace, TenantProvisioning).Tenant
 
 	// If the resource doesn't exist, we'll create it
 	if tenant == nil {
@@ -235,9 +235,8 @@ func (s *SubscriptionHandler) createTenant(reqInfo *RequestInfo, ca *v1alpha1.CA
 			GenerateName: ca.Name + "-consumer-",
 			Namespace:    ca.Namespace,
 			Labels: map[string]string{
-				LabelBTPApplicationIdentifierHash: sha1Sum(ca.Spec.GlobalAccountId, reqInfo.payload.appName),
-				LabelTenantId:                     reqInfo.payload.tenantId,
-				LabelSubscriptionGUID:             subscriptionGUID,
+				LabelTenantId:         reqInfo.payload.tenantId,
+				LabelSubscriptionGUID: subscriptionGUID,
 			},
 		},
 		StringData: map[string]string{
@@ -259,10 +258,9 @@ func (s *SubscriptionHandler) createTenant(reqInfo *RequestInfo, ca *v1alpha1.CA
 				AnnotationSubscriptionContextSecret: secret.Name, // Store the secret name in the tenant annotation
 			},
 			Labels: map[string]string{
-				LabelBTPApplicationIdentifierHash: sha1Sum(ca.Spec.GlobalAccountId, reqInfo.payload.appName),
-				LabelTenantId:                     reqInfo.payload.tenantId,
-				LabelSubscriptionGUID:             subscriptionGUID,
-				LabelTenantType:                   "consumer", // Default tenant type for consumer tenants
+				LabelTenantId:         reqInfo.payload.tenantId,
+				LabelSubscriptionGUID: subscriptionGUID,
+				LabelTenantType:       "consumer", // Default tenant type for consumer tenants
 			},
 		},
 		Spec: v1alpha1.CAPTenantSpec{
@@ -408,12 +406,20 @@ func (s *SubscriptionHandler) updateSecret(tenant *v1alpha1.CAPTenant, secret *c
 	return err
 }
 
-func (s *SubscriptionHandler) getTenantByBtpAppIdentifier(globalAccountGUID, btpAppName, tenantId, namespace, step string) *Result {
-	labelsMap := map[string]string{
-		LabelBTPApplicationIdentifierHash: sha1Sum(globalAccountGUID, btpAppName),
-		LabelTenantId:                     tenantId,
+func (s *SubscriptionHandler) getTenantByAppIdentifier(globalAccountGUID, providerSubAccountId, btpAppName, tenantId, namespace, step string) (result *Result) {
+	tenantlabels := map[string]string{
+		LabelTenantId: tenantId,
+	}
+
+	labelsMaps := maps.Clone(tenantlabels)
+	labelsMaps[LabelAppIdHash] = sha1Sum(providerSubAccountId, btpAppName)
+
+	if result = s.getTenantByLabels(labelsMaps, namespace, step, "getTenantByAppIdentifier"); result.Tenant == nil {
+		oldLabelsMap := maps.Clone(tenantlabels)
+		oldLabelsMap[LabelBTPApplicationIdentifierHash] = sha1Sum(globalAccountGUID, btpAppName)
+		result = s.getTenantByLabels(oldLabelsMap, namespace, step, "getTenantByAppIdentifier")
 	}
-	return s.getTenantByLabels(labelsMap, namespace, step, "getTenantByBtpAppIdentifier")
+	return
 }
 
 func (s *SubscriptionHandler) getTenantBySubscriptionGUID(subscriptionGUID, tenantId, step string) *Result {
@@ -481,7 +487,7 @@ func (s *SubscriptionHandler) DeleteTenant(reqInfo *RequestInfo) *Result {
 		}
 		// if tenant is not found in SaaS subscription scenario, check if it exists by btpApp identifier to handle cases where tenant was created without subscriptionGUID
 		util.LogInfo("Tenant not found by subscriptionGUID, checking by BTP app identifier", TenantDeprovisioning, "DeleteTenant", nil, "subscriptionGUID", reqInfo.payload.subscriptionGUID)
-		tenant = s.getTenantByBtpAppIdentifier(ca.Spec.GlobalAccountId, reqInfo.payload.appName, reqInfo.payload.tenantId, metav1.NamespaceAll, TenantDeprovisioning).Tenant
+		tenant = s.getTenantByAppIdentifier(ca.Spec.GlobalAccountId, ca.Spec.ProviderSubaccountId, reqInfo.payload.appName, reqInfo.payload.tenantId, metav1.NamespaceAll, TenantDeprovisioning).Tenant
 	}
 
 	if tenant == nil {

cmd/server/internal/handler_test.go

@@ -152,9 +152,8 @@ func createTenantSubscriptionContextSecret(subscriptionContext string) runtime.O
 			Name:      subscriptionContextSecretName,
 			Namespace: v1.NamespaceDefault,
 			Labels: map[string]string{
-				LabelBTPApplicationIdentifierHash: sha1Sum(globalAccountId, appName),
-				LabelTenantId:                     tenantId,
-				LabelSubscriptionGUID:             subscriptionGUID,
+				LabelTenantId:         tenantId,
+				LabelSubscriptionGUID: subscriptionGUID,
 			},
 		},
 		StringData: map[string]string{

crds/sme.sap.com_capapplications.yaml

@@ -117,8 +117,12 @@ spec:
             required:
             - btp
             - btpAppName
-            - globalAccountId
             type: object
+            x-kubernetes-validations:
+            - message: at least one of the fields in [globalAccountId providerSubaccountId]
+                must be set
+              rule: '[has(self.globalAccountId),has(self.providerSubaccountId)].filter(x,x==true).size()
+                >= 1'
           status:
             properties:
               conditions:

internal/controller/dns-manager.go

@@ -90,10 +90,9 @@ func handleDnsEntries[T v1alpha1.DomainEntity](ctx context.Context, c *Controlle
 				GenerateName: subResourceName + "-",
 				Namespace:    subResourceNamespace,
 				Labels: map[string]string{
-					LabelOwnerIdentifierHash:          sha1Sum(ownerId),
-					LabelOwnerGeneration:              fmt.Sprintf("%d", dom.GetMetadata().Generation),
-					LabelBTPApplicationIdentifierHash: info.appId,
-					LabelDNSNameHash:                  dnsHash,
+					LabelOwnerIdentifierHash: sha1Sum(ownerId),
+					LabelOwnerGeneration:     fmt.Sprintf("%d", dom.GetMetadata().Generation),
+					LabelDNSNameHash:         dnsHash,
 				},
 				Annotations: map[string]string{
 					AnnotationResourceHash:     hash,
@@ -132,7 +131,6 @@ func checkRelevantDNSEntries(ctx context.Context, dnsEntries []dnsv1alpha1.DNSEn
 			if entry.Annotations[AnnotationResourceHash] != hash {
 				updateResourceAnnotation(&entry.ObjectMeta, hash)
 				entry.Labels[LabelOwnerGeneration] = fmt.Sprintf("%d", generation)
-				entry.Labels[LabelBTPApplicationIdentifierHash] = info.appId
 				entry.Labels[LabelDNSNameHash] = dnsHash
 				entry.Spec = getDnsEntrySpec(info)
 				_, err = c.gardenerDNSClient.DnsV1alpha1().DNSEntries(entry.Namespace).Update(ctx, &entry, metav1.UpdateOptions{})
@@ -275,8 +273,8 @@ func collectAppSubdomainInfos[T v1alpha1.DomainEntity](c *Controller, dom T) (su
 		if len(ca.Status.ObservedSubdomains) > 0 {
 			for _, subdomain := range ca.Status.ObservedSubdomains {
 				if appId, ok := subdomains[subdomain]; !ok {
-					subdomains[subdomain] = ca.Labels[LabelBTPApplicationIdentifierHash]
-				} else if appId != ca.Labels[LabelBTPApplicationIdentifierHash] {
+					subdomains[subdomain] = string(ca.UID)
+				} else if appId != string(ca.UID) {
 					// this subdomain is already used by another application
 					// skip and raise warning event
 					c.Event(ca, runtime.Object(dom), corev1.EventTypeWarning, DomainEventSubdomainAlreadyInUse, EventActionProcessingDomainResources,