feat(environment-variables): add warn/error for internal variables. fixes #285 (#350)

Author: justinedelson

src/base-variables-command.js

@@ -56,6 +56,8 @@ class BaseVariablesCommand extends Command {
 
     const variables = await this.prepareVariableList(flags, currentVariablesList)
 
+    this.validateVariables(flags, variables)
+
     if (variables.length > 0) {
       cli.action.start('setting variables')
       await this.setVariables(programId, args, variables, flags.imsContextName)
@@ -165,6 +167,8 @@ class BaseVariablesCommand extends Command {
       }
     })
   }
+
+  validateVariables (flags, variables) { }
 }
 
 BaseVariablesCommand.coreSetterFlags = {

src/commands/cloudmanager/environment/set-variables.js

@@ -14,6 +14,7 @@ const BaseEnvironmentVariablesCommand = require('../../../base-environment-varia
 const BaseVariablesCommand = require('../../../base-variables-command')
 const { initSdk, sanitizeEnvironmentId } = require('../../../cloudmanager-helpers')
 const { flags } = require('@oclif/command')
+const Config = require('@adobe/aio-lib-core-config')
 const _ = require('lodash')
 const commonFlags = require('../../../common-flags')
 
@@ -48,6 +49,22 @@ class SetEnvironmentVariablesCommand extends BaseEnvironmentVariablesCommand {
     const sdk = await initSdk(imsContextName)
     return sdk.setEnvironmentVariables(programId, environmentId, variables)
   }
+
+  strictValidationEnabled (flags) {
+    return flags.strict || Config.get('cloudmanager.environmentVariables.strictValidation')
+  }
+
+  validateVariables (flags, variables) {
+    variables.forEach(variable => {
+      if (variable.name && variable.name.startsWith('INTERNAL_')) {
+        if (this.strictValidationEnabled(flags)) {
+          this.error(`The variable name ${variable.name} is reserved for internal usage and will be ignored.`)
+        } else {
+          this.warn(`The variable name ${variable.name} is reserved for internal usage and will be ignored.`)
+        }
+      }
+    })
+  }
 }
 
 SetEnvironmentVariablesCommand.description = 'sets variables set on an environment. These are runtime variables available to components running inside the runtime environment. Use set-pipeline-variables to set build-time variables on a pipeline.'
@@ -60,6 +77,7 @@ SetEnvironmentVariablesCommand.flags = {
   ...commonFlags.global,
   ...commonFlags.programId,
   ...BaseVariablesCommand.setterFlags,
+  strict: flags.boolean({ description: 'performs strict validation of internal variables. Can also be enabled by setting configuration property cloudmanager.environmentVariables.strictValidation to a truthy value.' }),
 }
 
 services.forEach(service => {

test/commands/environment/set-variables.test.js

@@ -21,6 +21,8 @@ let mockFileContent = ''
 
 const originalReadFile = fs.readFile
 
+let warn
+
 beforeAll(() => {
   fs.readFile = jest.fn((fileName, encoding, callback) => {
     if (fileName === mockFileName) {
@@ -37,6 +39,8 @@ beforeAll(() => {
 
 beforeEach(() => {
   resetCurrentOrgId()
+  warn = jest.fn()
+  SetEnvironmentVariablesCommand.prototype.warn = warn
 })
 
 afterAll(() => {
@@ -682,3 +686,56 @@ test('set-environment-variables - stdin - delete from stream with service delete
     service: 'author',
   }])
 })
+
+test('set-environment-variables - internal name strict mode off', async () => {
+  setCurrentOrgId('good')
+  setStore({
+    cloudmanager_programid: '4',
+  })
+
+  expect.assertions(7)
+
+  const runResult = SetEnvironmentVariablesCommand.run(['1', '--variable', 'INTERNAL_foo', 'bar'])
+  await expect(runResult instanceof Promise).toBeTruthy()
+
+  await runResult
+  await expect(init.mock.calls.length).toEqual(3)
+  await expect(init).toHaveBeenCalledWith('good', 'test-client-id', 'fake-token', 'https://cloudmanager.adobe.io')
+  await expect(mockSdk.setEnvironmentVariables.mock.calls.length).toEqual(1)
+  await expect(mockSdk.setEnvironmentVariables).toHaveBeenCalledWith('4', '1', [{
+    name: 'INTERNAL_foo',
+    type: 'string',
+    value: 'bar',
+  }])
+  await expect(warn.mock.calls.length).toEqual(1)
+  await expect(warn).toHaveBeenCalledWith('The variable name INTERNAL_foo is reserved for internal usage and will be ignored.')
+})
+
+test('set-environment-variables - internal name strict mode on via flag', async () => {
+  setCurrentOrgId('good')
+  setStore({
+    cloudmanager_programid: '4',
+  })
+
+  expect.assertions(2)
+
+  const runResult = SetEnvironmentVariablesCommand.run(['1', '--variable', 'INTERNAL_foo', 'bar', '--strict'])
+  await expect(runResult instanceof Promise).toBeTruthy()
+
+  await expect(runResult).rejects.toSatisfy(err => err.message === 'The variable name INTERNAL_foo is reserved for internal usage and will be ignored.')
+})
+
+test('set-environment-variables - internal name strict mode on via config', async () => {
+  setCurrentOrgId('good')
+  setStore({
+    cloudmanager_programid: '4',
+    'cloudmanager.environmentVariables.strictValidation': true,
+  })
+
+  expect.assertions(2)
+
+  const runResult = SetEnvironmentVariablesCommand.run(['1', '--variable', 'INTERNAL_foo', 'bar'])
+  await expect(runResult instanceof Promise).toBeTruthy()
+
+  await expect(runResult).rejects.toSatisfy(err => err.message === 'The variable name INTERNAL_foo is reserved for internal usage and will be ignored.')
+})