@sap/cds-mtxs - npm Package Compare versions

Comparing version 2.2.0 to 2.3.0

CHANGELOG.md

@@ -9,2 +9,17 @@ # Change Log
 
+## Version 2.3.0 - 2024-10-28
+
+### Added
+
+- If extensibility is disabled, the upgrade operation now checks if extensions exist to avoid potential data loss. If intended the check can be disabled by setting `cds.requires['cds.xt.DeploymentService'].upgrade.skipExtensionCheck: true`.
+- Requests to Service Manager now forward the correlation ID.
+
+### Fixed
+
+- Migration command `cds-mtx-migrate --syncTenantList` is now more robust.
+- `DEBUG=mtx` redacts Service Manager credentials.
+- When deleting a tenant HDI container, all of its service bindings are deleted – not just the ones labeled with the tenant ID.
+- Extension linter now checks `@mandatory` and `@readonly` more accurately.
+- `cds.xt.JobsService` inserts jobs and tasks in one transaction.
+
 ## Version 2.2.0 - 2024-09-30
@@ -11,0 +26,0 @@

lib/migration/migration.js

@@ -63,2 +63,4 @@ const path = require('path')
 module.exports.syncTenantList = async function syncTenantList(tenants, options, deleteEntries = false) {
+    const EXCLUDED_TENANTS = /^MT_LIB_TENANT-.*/
+
     const migrationResult = new MigrationResult()
@@ -73,3 +75,3 @@ const { dry, force } = options
     if (!force && !deleteEntries && existingTenantLists.length) {
-        migrationResult.log(t0, `Exsisting tenant list not empty. Skipping creation.`)
+        migrationResult.log(t0, `Existing tenant list not empty. Skipping creation.`)
         return migrationResult
@@ -81,3 +83,4 @@ }
         && existingTenantLists.indexOf(c) === -1
-        && (tenants.includes('*') || tenants.includes(c)))
+        && (tenants.includes('*') || tenants.includes(c))
+        && !EXCLUDED_TENANTS.test(c))
     if (!dry && tenantContainers.length) {
@@ -84,0 +87,0 @@ await t0_(INSERT.into(Tenants, tenantContainers.map(c => ({ ID: c, metadata: JSON.stringify({ subscribedTenantId: c }) }))))

package.json

 {
   "name": "@sap/cds-mtxs",
-  "version": "2.2.0",
+  "version": "2.3.0",
   "description": "SAP Cloud Application Programming Model - Multitenancy library",
@@ -5,0 +5,0 @@ "homepage": "https://cap.cloud.sap/",

srv/extensibility-service.js

@@ -12,4 +12,13 @@ const cds = require('@sap/cds/lib'), { fs, path, tar, rimraf } = cds.utils
 const LOG = cds.log('mtx')
+const DEBUG = cds.debug('mtx')
 const _isCSN = str => str.substring(0, 1) === '{'
-const _async = () => cds.context.http?.req?.headers?.prefer === 'respond-async'
+const _async = (req) => {
+  const async = cds.context.http?.req?.headers?.prefer === 'respond-async'
+  DEBUG?.('Request headers for async extensibility')
+  DEBUG?.('cds.context.http.req.headers.prefer: ', cds.context.http?.req?.headers?.prefer)
+  DEBUG?.('req.headers.prefer: ', req.headers?.prefer)
+  // TODO remove
+  if (cds.env.requires['cds.xt.ExtensibilityService']?._disableAsync === true) return false
+  return async
+}
 
@@ -74,3 +83,3 @@ module.exports = class ExtensibilityService extends cds.ApplicationService {
       const job = await _set(req, { extension: [...req.data.csn], resources: req.data.i18n, tag: req.data.ID, tenant: tenant ?? req.tenant })
-      if (_async()) {
+      if (_async(req)) {
         cds.context.http?.res.status(202)
@@ -98,3 +107,3 @@ return job
       const job = await _set(req, { extension: ['{}'], tag: TOMBSTONE_ID, tenant: tenant ?? req.tenant })
-      if (_async()) {
+      if (_async(req)) {
         cds.context.http?.res.status(202)
@@ -236,3 +245,3 @@ return job
 const _activate = async function (tenant, tag, extCsn, bundles, csvs, sources, activate, req) {
-  const async = _async()
+  const async = _async(req)
   try {
@@ -239,0 +248,0 @@ const js = await cds.connect.to('cds.xt.JobsService')

srv/extensibility/linter/annotations.js

@@ -9,2 +9,3 @@ const LinterMessage = require('./message')
   ['@mandatory', _createMandatoryAnnotationWarning],
+  ['@readonly', _createMandatoryAnnotationWarning],
   ['@assert.notNull', _createMandatoryAnnotationWarning],
@@ -51,3 +52,3 @@ ['@assert.range', _createMandatoryAnnotationWarning]
 function _createMandatoryAnnotationWarning(annotationName, annoOrElem) {
-  if (annoOrElem.element.default) return
+  if (annoOrElem.element.default || annoOrElem.element[annotationName] === false) return
   const message = `Annotation '${annotationName}' in '${
@@ -54,0 +55,0 @@ annoOrElem.element.annotate || annoOrElem.element.name || annoOrElem.parent.extend || annoOrElem.parent.annotate

srv/extensibility/linter/index.js

@@ -26,3 +26,3 @@ const cds = require('@sap/cds/lib')
   const messages = [
-    ...(hasConfig ? new NamespaceChecker().check(reflectedCsn, reflectedFullCsn, compileBaseDir, linter_options) : []),
+    ...(hasConfig ? new NamespaceChecker().check(reflectedCsn, reflectedFullCsn, linter_options) : []),
     ...new AnnotationsChecker().check(reflectedCsn, reflectedFullCsn, compileBaseDir, linter_options), // always mandatory
@@ -29,0 +29,0 @@ ...(hasConfig ? new AllowlistChecker().check(reflectedCsn, reflectedFullCsn, compileBaseDir, linter_options) : []),

srv/extensibility/linter/namespace.js

 const LinterMessage = require('./message')
 
 module.exports = class NamespaceChecker {
-  check(extensionCsn, fullCsn, compileDir, mtxConfig) {
-    let elementPrefixes = mtxConfig['element-prefix']
-    let namespaceBlocklist = mtxConfig['namespace-blocklist'] || mtxConfig['namespace-blacklist']
+
+  check(extensionCsn, fullCsn, mtxConfig) {
+    const { 'element-prefix': p, 'namespace-blocklist': b, 'namespace-blacklist': b2 } = mtxConfig
+    let elementPrefixes = p, namespaceBlocklist = b ?? b2
     const messages = []
 
     if (elementPrefixes) {
-      if (!Array.isArray(elementPrefixes)) {
-        elementPrefixes = [elementPrefixes]
-      }
-
+      elementPrefixes = Array.isArray(elementPrefixes) ? elementPrefixes : [elementPrefixes]
       if (extensionCsn.extensions) {
-        // forall switches back to definitions if extensions are undefined
-        extensionCsn.forall(
+        extensionCsn.forall( // forall switches back to definitions if extensions are undefined
           () => true,
           (element, name, parent) => {
-            element.name = name // TODO check if bug
-            this._checkElement(element, parent, elementPrefixes, compileDir, messages)
+            element.name = name // REVISIT: assign name in forall?
+            this._checkElement(element, parent, elementPrefixes, messages)
           },
@@ -25,10 +22,5 @@ extensionCsn.extensions
       }
-
       extensionCsn.forall(
-        element => {
-          return ['entity', 'function', 'action'].includes(element.kind)
-        },
-        entity => {
-          this._checkEntity(entity, extensionCsn, fullCsn, elementPrefixes, compileDir, messages)
-        }
+        element => element.kind in { 'entity':1, 'function':1, 'action':1 },
+        entity => this._checkEntity(entity, extensionCsn, fullCsn, elementPrefixes, messages)
       )
@@ -38,86 +30,36 @@ }
     if (namespaceBlocklist) {
-      if (!Array.isArray(namespaceBlocklist)) {
-        namespaceBlocklist = [namespaceBlocklist]
-      }
-
-      extensionCsn.forall('service', service => {
-        this._checkNamespace(service, namespaceBlocklist, compileDir, messages)
-      })
-
+      namespaceBlocklist = Array.isArray(namespaceBlocklist) ? namespaceBlocklist : [namespaceBlocklist]
+      extensionCsn.forall('service', service => this._checkNamespace(service, namespaceBlocklist, messages))
       extensionCsn.forall(
-        element => {
-          return ['aspect', 'entity', 'type'].includes(element.kind)
-        },
-        entity => {
+        element => element.kind in { 'aspect':1, 'entity':1, 'type':1 },
+        (entity, name) => {
+          entity.name = name // REVISIT: assign name in forall?
           if (entity._unresolved) return // skip unresolved entities
-          this._checkNamespace(entity, namespaceBlocklist, compileDir, messages)
+          this._checkNamespace(entity, namespaceBlocklist, messages)
         }
       )
     }
-
     return messages
   }
 
-  _checkElement(element, parent, elementPrefixes, compileDir, messages) {
-    if (elementPrefixes.length < 1) {
-      return
-    }
-
-    if (!parent) {
-      return
-    }
-
-    if (element.kind === 'extend') { // check additional restrictions later?
-      return
-    }
-
-    for (const elementPrefix of elementPrefixes) {
-      if (!parent.extend || element.name.startsWith(elementPrefix)) {
-        return
-      }
-    }
-
+  _checkElement(element, parent, elementPrefixes, messages) {
+    if (elementPrefixes.length < 1) return
+    if (!parent) return
+    if (element.kind === 'extend') return // check additional restrictions later
+    if (!parent.extend) return
+    if (elementPrefixes.some(prefix => element.name.startsWith(prefix))) return
     messages.push(this._createPrefixWarning(element, parent, elementPrefixes))
   }
 
-  _checkEntity(element, reflectedCsn, reflectedFullCsn, elementPrefixes, compileDir, messages) {
-    if (elementPrefixes.length < 1) {
-      return
-    }
-
+  _checkEntity(element, reflectedCsn, reflectedFullCsn, elementPrefixes, messages) {
+    if (elementPrefixes.length < 1) return
     const parent = this._getEnclosingEntity(reflectedCsn, element)
-
-    // parent exists in extension
-    if (parent) {
-      return
-    }
-
-    // check full csn for parent
-    let elementName
-    const parentFromFullCsn = this._getEnclosingEntity(reflectedFullCsn, element)
-    if (!parentFromFullCsn) {
-      elementName = element.name
-    } else {
-      elementName = this._getNestedEntityName(element, parentFromFullCsn.name) || element.name
-    }
-
-    for (const elementPrefix of elementPrefixes) {
-      if (elementName.startsWith(elementPrefix)) {
-        return
-      }
-    }
-
-    messages.push(this._createPrefixWarning(element, parentFromFullCsn, elementPrefixes))
+    if (parent) return // parent exists in extension
+    const parentFullCsn = this._getEnclosingEntity(reflectedFullCsn, element)
+    const elementName = !parentFullCsn ? element.name : element.name.replace(parentFullCsn.name + '.', '') || element.name
+    if (elementPrefixes.some(prefix => elementName.startsWith(prefix))) return
+    messages.push(this._createPrefixWarning(element, parentFullCsn, elementPrefixes))
   }
 
-  _hasEnclosingEntity(reflectedCsn, element) {
-    const plainEntityName = element.name.replace(reflectedCsn.namespace + '.', '')
-    const splitEntityName = plainEntityName.split('.')
-    if (splitEntityName.length > 1) {
-      return true
-    }
-    return false
-  }
-
-  // TODO set parent entity name / check original test cases
+  // REVISIT: set parent entity name / check original test cases
   _getEnclosingEntity(reflectedCsn, element) {
@@ -132,10 +74,6 @@ const splitEntityName = element.name.split('.')
 
-  _getNestedEntityName(element, parentName) {
-    return element.name.replace(parentName + '.', '')
-  }
-
-  _checkNamespace(element, namespaceBlacklist, compileDir, messages) {
+  _checkNamespace(element, namespaceBlacklist, messages) {
     for (const namespace of namespaceBlacklist) {
       if (element.name.startsWith(namespace)) {
-        messages.push(this._createNamespaceWarning(element, compileDir, namespace))
+        messages.push(this._createNamespaceWarning(element, namespace))
       }
@@ -146,10 +84,10 @@ }
   _createPrefixWarning(element, parent, prefixRule) {
-    let message = `Element '${element.name}' ${parent ? `in '${parent.extend || parent.name}'` : ''} must start with ${prefixRule}`
+    const message = `Element '${element.name}' ${parent ? `in '${parent.extend || parent.name}'` : ''} must start with ${prefixRule}`
     return new LinterMessage(message, element)
   }
 
-  _createNamespaceWarning(element, compileDir, namespace) {
-    let message = `Element '${element.name}' uses a forbidden namespace '${namespace}'`
+  _createNamespaceWarning(element, namespace) {
+    const message = `Element '${element.name}' uses a forbidden namespace '${namespace}'`
     return new LinterMessage(message, element)
   }
 }

srv/jobs-service.js

@@ -76,8 +76,13 @@ const { inspect } = require('util')
     const job = { ID: job_ID, createdAt: (new Date).toISOString(), op, status: QUEUED }
-    await t0_(INSERT.into(Jobs, job))
     const jobs = Object.values(clusters).map(cluster => Array.from(cluster).map(tenant => ({ job_ID, ID: uuid(), tenant, op })))
     const tasks = jobs.flat()
 
+    await t0_(async () => {
+      await INSERT.into(Jobs, job)
+      if (tasks.length) {
+        await INSERT.into(Tasks, tasks)
+      }
+    })
+
     if (tasks.length) {
-      await t0_(INSERT.into(Tasks, tasks))
       jobQueue.enqueue({ job_ID, clusters: jobs, fn: task => {
@@ -84,0 +89,0 @@ const serviceInstance = cds.services[service]

srv/plugins/common.js

@@ -5,2 +5,3 @@ const cds = require('@sap/cds/lib')
 const LOG = cds.log('mtx')
+const main = require('../config')
 
@@ -27,2 +28,21 @@ exports.activated = 'Generic Metadata'
 
+  ds.before ('upgrade', async req => {
+    if (main.requires.extensibility) return // no checks needed
+
+    if (cds.env.requires['cds.xt.DeploymentService']?.upgrade?.skipExtensionCheck === true) return
+
+    // duplicate code, but it must be ensured that the tenant is set for the following operations
+    const { tenant } = req?.data ?? {}
+    if (tenant) cds.context = { tenant }
+
+    let existingExt
+    try {
+      existingExt = await SELECT.one(1).from('cds.xt.Extensions')
+    } catch (e) {
+      LOG.debug('No extensions found', e) // ok, no problem
+    }
+
+    if (existingExt) cds.error(`Extensions exist, but extensibility is disabled. Upgrade aborted to avoid data loss`, { status: 500 })
+  })
+
   ds.after ('subscribe', async (_, req) => {
@@ -29,0 +49,0 @@ const { tenant, metadata } = req.data

srv/plugins/hana.js

@@ -65,3 +65,4 @@ const cds = require('@sap/cds/lib'), {db} = cds.env.requires
       const bindings = await hana.getAll()
-      return bindings.map(({ labels: { tenant_id } }) => tenant_id[0])
+      const tenantIds = bindings.map(({ labels: { tenant_id } }) => tenant_id[0])
+      return [...new Set(tenantIds)]
     })
@@ -68,0 +69,0 @@

srv/plugins/hana/srv-mgr.js

@@ -112,3 +112,3 @@ const https = require('https')
 
-function getAll(tenants = '*', options) { // REVISIT: mirroring @sap/instance-manager, remove in favor of `get(tenants = '*')`
+function getAll(tenants = '*', options) {
   return _bindings4(tenants, options)
@@ -122,3 +122,14 @@ }
 async function remove(tenant) {
-  const bindings = await _bindings4([tenant], { disableCache: true })
+  const instance = await _instance4(tenant)
+  if (!instance) return
+  const fieldQuery = `service_instance_id eq '${instance.id}'`
+  const bindings = []; let token
+  do {
+    const { data } = await fetchApi('service_bindings', {
+      params: { token, fieldQuery }
+    })
+    const { items, token: nextPageToken } = data
+    bindings.push(...items)
+    token = nextPageToken
+  } while (token)
   const _deleteBindings = bindings.map(async ({ id }) =>
@@ -129,9 +140,5 @@ _poll((await fetchApi(`service_bindings/${id}?async=true`, { method: 'DELETE' })).headers.location)
   const failedDeletions = (await Promise.allSettled(_deleteBindings)).filter(d => d.status === 'rejected')
-  //if (failedDeletions.length > 0) throw new AggregateError(failedDeletions.map(d => d.reason)) // REVISIT: Node 15+
-  if (failedDeletions.length > 0) throw failedDeletions[0].reason
-  const instance = await _instance4(tenant)
-  if (instance) {
-    const _deleteInstance = await fetchApi(`service_instances/${instance.id}?async=true`, { method: 'DELETE' })
-    if (_deleteInstance.headers.location) await _poll(_deleteInstance.headers.location)
-  }
+  if (failedDeletions.length > 0) throw new AggregateError(failedDeletions.map(d => d.reason))
+  const _deleteInstance = await fetchApi(`service_instances/${instance.id}?async=true`, { method: 'DELETE' })
+  if (_deleteInstance.headers.location) await _poll(_deleteInstance.headers.location)
 }
@@ -241,2 +248,3 @@
   conf.headers['Client-Version'] ??= version
+  conf.headers['X-CorrelationID'] ??= cds.context?.id
   conf.baseURL ??= sm_url + '/v1/'
@@ -246,2 +254,19 @@ return fetchResiliently(conf.baseURL + url, conf)
 
+const SECRETS = /(passw)|(cert)|(ca)|(secret)|(key)/i
+/**
+ * Masks password-like strings, also reducing clutter in output
+ * @param {any} cred - object or array with credentials
+ * @returns {any}
+ */
+const _redacted = function _redacted(cred) {
+  if (!cred) return cred
+  if (Array.isArray(cred)) return cred.map(c => _redacted(c))
+  if (typeof cred === 'object') {
+    const newCred = Object.assign({}, cred)
+    Object.keys(newCred).forEach(k => (typeof newCred[k] === 'string' && SECRETS.test(k)) ? (newCred[k] = '...') : (newCred[k] = _redacted(newCred[k])))
+    return newCred
+  }
+  return cred
+}
+
 const maxRetries = cds.requires?.multitenancy?.serviceManager?.retries ?? 3
@@ -252,3 +277,3 @@ const fetchResiliently = module.exports.fetchResiliently = async function (url, conf, retriesLeft = maxRetries) {
     DEBUG?.('>', conf.method.toUpperCase(), url, inspect({
-      ...(conf.headers && { headers: conf.headers }),
+      ...(conf.headers && { headers: { ...conf.headers, Authorization: conf.headers.Authorization.split(' ')?.[0] + ' ...' } }),
       ...(conf.params && { params: conf.params }),
@@ -259,3 +284,3 @@ ...(conf.data && { data: conf.data })
     const { status, statusText } = response
-    DEBUG?.('<', conf.method.toUpperCase(), url, status, statusText, inspect(response.data, { depth: 11, colors: COLORS }))
+    DEBUG?.('<', conf.method.toUpperCase(), url, status, statusText, inspect(_redacted(response.data), { depth: 11, colors: COLORS }))
     return response
@@ -267,3 +292,3 @@ } catch (error) {
     const attempt = maxRetries - retriesLeft + 1
-    if (DEBUG) {
+    if (LOG._debug) {
       const e = error.toJSON?.() ?? error
@@ -270,0 +295,0 @@ DEBUG(`fetching ${url} attempt ${attempt} failed with`, {

New alerts

License Policy Violation

License

This package is not allowed per your license policy. Review the package's license to ensure compliance.

Found 1 instance in 1 package

Fixed alerts

License Policy Violation

License

This package is not allowed per your license policy. Review the package's license to ensure compliance.

Found 1 instance in 1 package

Improved metrics

Total package byte prevSize

259553

increased by0.85%

Lines of code

4679

increased by0.19%

No dependency changes