Skip to content

Commit 5041260

Browse files
Pavan-SAPPavan-SAP
authored
[FEAT] Operator: App Identification enhanced (#331)
Previously, we used the provider's `globalAccountId` and `btpAppName` to identify an app. This approach was not reliable because the `globalAccountId` in the subscription payload might not belong to the provider, making it impossible to uniquely identify the app. To solve this, we now introduce `providerSubaccountId` in the CAPApplication spec. This value is included in the subscription payload and along with `btpAppName` allows us to uniquely and correctly identify the app. **_Eventually, we plan to deprecate `globalAccountId` and replace it with `providerSubaccountId` in CAPApplication spec._**
1 parent 5adc7a1 commit 5041260

File tree

9 files changed

+141
-64
lines changed

9 files changed

+141
-64
lines changed

‎cmd/server/internal/handler.go‎

Lines changed: 56 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ const (
4242

4343
const (
4444
LabelBTPApplicationIdentifierHash = "sme.sap.com/btp-app-identifier-hash"
45+
LabelAppIdHash = "sme.sap.com/app-identifier-hash"
4546
LabelTenantId = "sme.sap.com/btp-tenant-id"
4647
LabelTenantType = "sme.sap.com/tenant-type"
4748
LabelSubscriptionGUID = "sme.sap.com/subscription-guid"
@@ -94,12 +95,13 @@ const (
9495
)
9596

9697
type payloadDetails struct {
97-
subscriptionGUID string
98-
tenantId string
99-
subdomain string
100-
globalAccountId string
101-
appName string
102-
raw *map[string]any
98+
subscriptionGUID string
99+
tenantId string
100+
subdomain string
101+
globalAccountId string
102+
providerSubaccountId string
103+
appName string
104+
raw *map[string]any
103105
}
104106

105107
type requestHeaderDetails struct {
@@ -159,7 +161,7 @@ func (s *SubscriptionHandler) CreateTenant(reqInfo *RequestInfo) *Result {
159161
var smsData *util.SmsCredentials
160162

161163
// Check if CAPApplication instance for the given btpApp exists
162-
ca, err := s.checkCAPApp(reqInfo.payload.globalAccountId, reqInfo.payload.appName)
164+
ca, err := s.checkCAPApp(reqInfo.payload.globalAccountId, reqInfo.payload.providerSubaccountId, reqInfo.payload.appName)
163165
if err != nil {
164166
util.LogError(err, ErrorOccurred, TenantProvisioning, ca, nil)
165167
return &Result{Tenant: nil, Message: err.Error()}
@@ -172,7 +174,7 @@ func (s *SubscriptionHandler) CreateTenant(reqInfo *RequestInfo) *Result {
172174
}
173175

174176
// Check if A CRO for CAPTenant already exists
175-
tenant := s.getTenantByBtpAppIdentifier(reqInfo.payload.globalAccountId, reqInfo.payload.appName, reqInfo.payload.tenantId, ca.Namespace, TenantProvisioning).Tenant
177+
tenant := s.getTenantByBtpAppIdentifier(ca.Spec.GlobalAccountId, reqInfo.payload.appName, reqInfo.payload.tenantId, ca.Namespace, TenantProvisioning).Tenant
176178

177179
// If the resource doesn't exist, we'll create it
178180
if tenant == nil {
@@ -224,7 +226,7 @@ func (s *SubscriptionHandler) createTenant(reqInfo *RequestInfo, ca *v1alpha1.CA
224226
GenerateName: ca.Name + "-consumer-",
225227
Namespace: ca.Namespace,
226228
Labels: map[string]string{
227-
LabelBTPApplicationIdentifierHash: sha1Sum(reqInfo.payload.globalAccountId, reqInfo.payload.appName),
229+
LabelBTPApplicationIdentifierHash: sha1Sum(ca.Spec.GlobalAccountId, reqInfo.payload.appName),
228230
LabelTenantId: reqInfo.payload.tenantId,
229231
LabelSubscriptionGUID: subscriptionGUID,
230232
},
@@ -248,7 +250,7 @@ func (s *SubscriptionHandler) createTenant(reqInfo *RequestInfo, ca *v1alpha1.CA
248250
AnnotationSubscriptionContextSecret: secret.Name, // Store the secret name in the tenant annotation
249251
},
250252
Labels: map[string]string{
251-
LabelBTPApplicationIdentifierHash: sha1Sum(reqInfo.payload.globalAccountId, reqInfo.payload.appName),
253+
LabelBTPApplicationIdentifierHash: sha1Sum(ca.Spec.GlobalAccountId, reqInfo.payload.appName),
252254
LabelTenantId: reqInfo.payload.tenantId,
253255
LabelSubscriptionGUID: subscriptionGUID,
254256
LabelTenantType: "consumer", // Default tenant type for consumer tenants
@@ -456,22 +458,28 @@ func (s *SubscriptionHandler) DeleteTenant(reqInfo *RequestInfo) *Result {
456458

457459
// Check if tenant exists by subscriptionGUID and tenantId
458460
tenant = s.getTenantBySubscriptionGUID(reqInfo.payload.subscriptionGUID, reqInfo.payload.tenantId, TenantDeprovisioning).Tenant
459-
if tenant == nil && reqInfo.subscriptionType == SaaS {
461+
if tenant != nil {
462+
ca, err = s.Clientset.SmeV1alpha1().CAPApplications(tenant.Namespace).Get(context.TODO(), tenant.Spec.CAPApplicationInstance, metav1.GetOptions{})
463+
if err != nil {
464+
util.LogError(err, "CAPApplication not found", TenantDeprovisioning, tenant, nil)
465+
return &Result{Tenant: nil, Message: err.Error()}
466+
}
467+
} else if reqInfo.subscriptionType == SaaS {
468+
ca, err = s.checkCAPApp(reqInfo.payload.globalAccountId, reqInfo.payload.providerSubaccountId, reqInfo.payload.appName)
469+
if err != nil {
470+
util.LogError(err, "CAPApplication not found", TenantDeprovisioning, tenant, nil)
471+
return &Result{Tenant: nil, Message: TenantNotFound}
472+
}
460473
// 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
461474
util.LogInfo("Tenant not found by subscriptionGUID, checking by BTP app identifier", TenantDeprovisioning, "DeleteTenant", nil, "subscriptionGUID", reqInfo.payload.subscriptionGUID)
462-
tenant = s.getTenantByBtpAppIdentifier(reqInfo.payload.globalAccountId, reqInfo.payload.appName, reqInfo.payload.tenantId, metav1.NamespaceAll, TenantDeprovisioning).Tenant
475+
tenant = s.getTenantByBtpAppIdentifier(ca.Spec.GlobalAccountId, reqInfo.payload.appName, reqInfo.payload.tenantId, metav1.NamespaceAll, TenantDeprovisioning).Tenant
463476
}
477+
464478
if tenant == nil {
465479
util.LogWarning("CAPTenant not found", TenantDeprovisioning)
466480
return &Result{Tenant: nil, Message: TenantNotFound}
467481
}
468482

469-
ca, err = s.Clientset.SmeV1alpha1().CAPApplications(tenant.Namespace).Get(context.TODO(), tenant.Spec.CAPApplicationInstance, metav1.GetOptions{})
470-
if err != nil {
471-
util.LogError(err, "CAPApplication not found", TenantDeprovisioning, tenant, nil)
472-
return &Result{Tenant: nil, Message: err.Error()}
473-
}
474-
475483
saasData, smsData, err = s.authorizationCheck(reqInfo.headerDetails, ca, reqInfo.subscriptionType, TenantDeprovisioning)
476484
if err != nil {
477485
util.LogError(err, AuthorizationCheckFailed, TenantDeprovisioning, ca, nil)
@@ -518,14 +526,30 @@ func (s *SubscriptionHandler) authorizationCheck(headerDetails *requestHeaderDet
518526
return
519527
}
520528

521-
func (s *SubscriptionHandler) checkCAPApp(globalAccountId, btpAppName string) (*v1alpha1.CAPApplication, error) {
522-
labelSelector, err := labels.ValidatedSelectorFromSet(map[string]string{
523-
LabelBTPApplicationIdentifierHash: sha1Sum(globalAccountId, btpAppName),
529+
func (s *SubscriptionHandler) checkCAPApp(globalAccountId, providerSubaccountId, btpAppName string) (*v1alpha1.CAPApplication, error) {
530+
// First try to find CAPApplication by providerSubaccountId (appIdHash)
531+
labelSelector, _ := labels.ValidatedSelectorFromSet(map[string]string{
532+
LabelAppIdHash: sha1Sum(providerSubaccountId, btpAppName),
524533
})
525-
if err != nil {
534+
535+
ca, err := s.getAppByLabelSelector(labelSelector)
536+
if err != nil && err.Error() != ResourceNotFound {
526537
return nil, err
527538
}
528539

540+
if ca != nil {
541+
return ca, nil
542+
}
543+
544+
// If no CAPApplication is found by providerSubaccountId, try to find by globalAccountId (btpAppIdentifierHash) to cover previous cases where appIdHash label might not be present
545+
labelSelector, _ = labels.ValidatedSelectorFromSet(map[string]string{
546+
LabelBTPApplicationIdentifierHash: sha1Sum(globalAccountId, btpAppName),
547+
})
548+
549+
return s.getAppByLabelSelector(labelSelector)
550+
}
551+
552+
func (s *SubscriptionHandler) getAppByLabelSelector(labelSelector labels.Selector) (*v1alpha1.CAPApplication, error) {
529553
capAppsList, err := s.Clientset.SmeV1alpha1().CAPApplications(metav1.NamespaceAll).List(context.TODO(), metav1.ListOptions{LabelSelector: labelSelector.String()})
530554
if err != nil {
531555
return nil, err
@@ -959,7 +983,7 @@ func (s *SubscriptionHandler) HandleSMSRequest(w http.ResponseWriter, req *http.
959983
}
960984

961985
func ProcessRequest(req *http.Request, subscriptionType subscriptionType) (*RequestInfo, error) {
962-
var subscriptionGUID, tenantId, subdomain, globalAccountId, appName string
986+
var subscriptionGUID, tenantId, subdomain, globalAccountId, providerSubaccountId, appName string
963987
var jsonPayload map[string]any
964988

965989
if !(req.Method == http.MethodDelete && subscriptionType == SMS) {
@@ -984,6 +1008,7 @@ func ProcessRequest(req *http.Request, subscriptionType subscriptionType) (*Requ
9841008
subdomain = subscriber["subaccountSubdomain"].(string)
9851009
globalAccountId = subscriber["globalAccountId"].(string)
9861010
rootApp := jsonPayload["rootApplication"].(map[string]any)
1011+
providerSubaccountId = rootApp["providerSubaccountId"].(string)
9871012
appName = rootApp["appName"].(string)
9881013
case http.MethodDelete:
9891014
// get paramater from URL
@@ -1005,17 +1030,19 @@ func ProcessRequest(req *http.Request, subscriptionType subscriptionType) (*Requ
10051030
tenantId = jsonPayload["subscribedTenantId"].(string)
10061031
subdomain = jsonPayload["subscribedSubdomain"].(string)
10071032
globalAccountId = jsonPayload["globalAccountGUID"].(string)
1033+
providerSubaccountId = jsonPayload["providerSubaccountId"].(string)
10081034
appName = jsonPayload["subscriptionAppName"].(string)
10091035
}
10101036

10111037
payload := &payloadDetails{
10121038
// GTID
1013-
subscriptionGUID: subscriptionGUID,
1014-
tenantId: tenantId,
1015-
subdomain: subdomain,
1016-
globalAccountId: globalAccountId,
1017-
appName: appName,
1018-
raw: &jsonPayload,
1039+
subscriptionGUID: subscriptionGUID,
1040+
tenantId: tenantId,
1041+
subdomain: subdomain,
1042+
globalAccountId: globalAccountId,
1043+
providerSubaccountId: providerSubaccountId,
1044+
appName: appName,
1045+
raw: &jsonPayload,
10191046
}
10201047
return &RequestInfo{
10211048
subscriptionType: subscriptionType,

0 commit comments

Comments
 (0)