Use Configs.addResults() instead of throwing exceptions.

Author: frankfarzan

ts/demo-functions/package-lock.json

@@ -24,7 +24,7 @@
   "dependencies": {
     "glob": "^7.1.3",
     "js-yaml": "^3.13.1",
-    "kpt-functions": "^0.13.0"
+    "kpt-functions": "^0.14.0-rc.1"
   },
   "devDependencies": {
     "create-kpt-functions": "^0.16.0",

ts/demo-functions/package.json

@@ -23,11 +23,17 @@ import { expandTeamCr } from './expand_team_cr';
 const RUNNER = new TestRunner(expandTeamCr);
 
 describe(expandTeamCr.name, () => {
-  it('does nothing to empty repos', RUNNER.assertCallback());
+  it(
+    'does nothing to empty repos',
+    RUNNER.assertCallback(undefined, 'unchanged')
+  );
 
   it(
     'does nothing to non-Team objects',
-    RUNNER.assertCallback(new Configs([Namespace.named('backend')]))
+    RUNNER.assertCallback(
+      new Configs([Namespace.named('backend')]),
+      'unchanged'
+    )
   );
 
   it(

ts/demo-functions/src/expand_team_test.ts

@@ -30,7 +30,7 @@ const FUNC_CONFIG: ConfigMap = new ConfigMap({
 describe('labelNamespace', () => {
   it(
     'empty input ok',
-    RUNNER.assertCallback(new Configs(undefined, FUNC_CONFIG))
+    RUNNER.assertCallback(new Configs(undefined, FUNC_CONFIG), 'unchanged')
   );
 
   it(

ts/demo-functions/src/label_namespace_test.ts

@@ -21,7 +21,7 @@ import { mutatePsp } from './mutate_psp';
 const RUNNER = new TestRunner(mutatePsp);
 
 describe('mutatePsp', () => {
-  it('empty configs is noop', RUNNER.assertCallback());
+  it('empty configs is noop', RUNNER.assertCallback(undefined, 'unchanged'));
 
   it(
     'modifies PSP with allowPrivilegeEscalation = true to false',
@@ -30,7 +30,7 @@ describe('mutatePsp', () => {
 
   it(
     'leaves PSP with allowPrivilegeEscalation = false alone',
-    RUNNER.assertCallback(new Configs([psp(false)]))
+    RUNNER.assertCallback(new Configs([psp(false)]), 'unchanged')
   );
 });
 

ts/demo-functions/src/mutate_psp_test.ts

@@ -21,13 +21,10 @@ import { Role } from './gen/io.k8s.api.rbac.v1';
 const RUNNER = new TestRunner(noOp);
 
 describe('noOp', () => {
-  it('empty', RUNNER.assertCallback());
+  it('empty passes', RUNNER.assertCallback(undefined, 'unchanged'));
 
   it(
     'pass through',
-    RUNNER.assertCallback(
-      new Configs([Role.named('alice')]),
-      new Configs([Role.named('alice')])
-    )
+    RUNNER.assertCallback(new Configs([Role.named('alice')]), 'unchanged')
   );
 });

ts/demo-functions/src/no_op_test.ts

@@ -20,23 +20,22 @@ import { safeLoadAll } from 'js-yaml';
 import * as path from 'path';
 import {
   Configs,
-  ConfigError,
-  MultiConfigError,
   isKubernetesObject,
-  ConfigFileError,
   addAnnotation,
+  configFileResult,
   SOURCE_PATH_ANNOTATION,
   SOURCE_INDEX_ANNOTATION,
+  Result,
 } from 'kpt-functions';
 
 export const SOURCE_DIR = 'source_dir';
-export const FILTER_IVNALID = 'filter_invalid';
+export const FILTER_INVALID = 'filter_invalid';
 
 export async function readYaml(configs: Configs) {
   // Get the parameters.
   const sourceDir = configs.getFunctionConfigValueOrThrow(SOURCE_DIR);
   const ignoreInvalid =
-    configs.getFunctionConfigValue(FILTER_IVNALID) === 'true';
+    configs.getFunctionConfigValue(FILTER_INVALID) === 'true';
 
   // Discard any input objects since this is a source function.
   configs.deleteAll();
@@ -45,17 +44,10 @@ export async function readYaml(configs: Configs) {
   const files = glob.sync(sourceDir + '/**/*.+(yaml|yml)');
 
   // Parse each file and convert it to a KubernetesObject.
-  const errors: ConfigError[] = files
+  files
     .map(f => parseFile(configs, sourceDir, f, ignoreInvalid))
-    .filter(err => err !== undefined)
-    .map(err => err as ConfigError);
-
-  if (errors.length) {
-    throw new MultiConfigError(
-      `Found files containing invalid objects. To filter invalid objects set ${FILTER_IVNALID} to 'true'.`,
-      errors
-    );
-  }
+    .filter(result => result !== undefined)
+    .forEach(result => configs.addResults(result as Result));
 }
 
 readYaml.usage = `
@@ -64,7 +56,7 @@ Reads a directory of kubernetes YAML configs recursively.
 Configured using a ConfigMap with the following keys:
 
 ${SOURCE_DIR}: Path to the config directory to read.
-${FILTER_IVNALID}: [Optional] If 'true', ignores invalid Kubernetes objects instead of failing.
+${FILTER_INVALID}: [Optional] If 'true', ignores invalid Kubernetes objects instead of failing.
 
 Example:
 
@@ -81,7 +73,7 @@ function parseFile(
   sourceDir: string,
   file: string,
   ignoreInvalid: boolean
-): ConfigError | undefined {
+): Result | undefined {
   const contents = readFileOrThrow(file);
   let objects = safeLoadAll(contents);
 
@@ -91,7 +83,7 @@ function parseFile(
     if (ignoreInvalid) {
       objects = objects.filter(o => isKubernetesObject(o));
     } else {
-      return new ConfigFileError(
+      return configFileResult(
         `File contains invalid Kubernetes objects '${JSON.stringify(
           invalidObjects
         )}'`,

ts/demo-functions/src/read_yaml.ts

@@ -22,18 +22,19 @@ import {
   Configs,
   readConfigs,
   FileFormat,
-  MultiConfigError,
+  configFileResult,
 } from 'kpt-functions';
 
 const RUNNER = new TestRunner(readYaml);
 
 describe('readYaml', () => {
   const functionConfig = ConfigMap.named('config');
-  functionConfig.data = {};
 
   it('works on empty dir', async () => {
     const sourceDir = path.resolve(__dirname, '../test-data/source/empty');
-    functionConfig.data![SOURCE_DIR] = sourceDir;
+    functionConfig.data = {
+      [SOURCE_DIR]: sourceDir,
+    };
     const configs = new Configs(undefined, functionConfig);
 
     await readYaml(configs);
@@ -51,17 +52,27 @@ describe('readYaml', () => {
       expectedIntermediateFile,
       FileFormat.YAML
     );
-    functionConfig.data![SOURCE_DIR] = sourceDir;
+    functionConfig.data = {
+      [SOURCE_DIR]: sourceDir,
+    };
     const actualConfigs = new Configs(undefined, functionConfig);
 
     await RUNNER.assert(actualConfigs, expectedConfigs);
   });
 
   it('fails for invalid KubernetesObjects', async () => {
     const sourceDir = path.resolve(__dirname, '../test-data/source/invalid');
-    functionConfig.data![SOURCE_DIR] = sourceDir;
+    functionConfig.data = {
+      [SOURCE_DIR]: sourceDir,
+    };
     const actualConfigs = new Configs(undefined, functionConfig);
+    const expectedConfigs = new Configs(undefined, functionConfig, [
+      configFileResult(
+        `File contains invalid Kubernetes objects '[{"kind":"I don't have apiVersion"}]'`,
+        path.join(sourceDir, 'invalid.yaml')
+      ),
+    ]);
 
-    await RUNNER.assert(actualConfigs, undefined, MultiConfigError);
+    await RUNNER.assert(actualConfigs, expectedConfigs);
   });
 });

ts/demo-functions/src/read_yaml_test.ts

@@ -1,30 +1,23 @@
-import {
-  Configs,
-  KubernetesObjectError,
-  MultiConfigError,
-} from 'kpt-functions';
+import { Configs, kubernetesObjectResult } from 'kpt-functions';
 import { isPodSecurityPolicy } from './gen/io.k8s.api.policy.v1beta1';
 
 export async function suggestPsp(configs: Configs) {
   // Iterate over all PodSecurityPolicy objects in the input and flag any
   // that do not have 'allowPrivilegeEscalation' field set to true.
-  const errors = configs
+  const results = configs
     .get(isPodSecurityPolicy)
     .filter(psp => psp.spec && psp.spec.allowPrivilegeEscalation !== false)
-    .map(
-      psp =>
-        new KubernetesObjectError(
-          `Suggest explicitly disabling privilege escalation`,
-          psp,
-          { path: `spec.allowPrivilegeEscalation`, suggestedValue: false },
-          'warn',
-          { category: 'security' }
-        )
+    .map(psp =>
+      kubernetesObjectResult(
+        'Suggest explicitly disabling privilege escalation',
+        psp,
+        { path: 'spec.allowPrivilegeEscalation', suggestedValue: false },
+        'warn',
+        { category: 'security' }
+      )
     );
 
-  if (errors.length) {
-    throw new MultiConfigError(undefined, errors);
-  }
+  configs.addResults(...results);
 }
 
 suggestPsp.usage = `

ts/demo-functions/src/suggest_psp.ts

@@ -14,27 +14,34 @@
  * limitations under the License.
  */
 
-import { Configs, TestRunner, ConfigError } from 'kpt-functions';
+import { Configs, TestRunner, kubernetesObjectResult } from 'kpt-functions';
 import { PodSecurityPolicy } from './gen/io.k8s.api.policy.v1beta1';
 import { suggestPsp } from './suggest_psp';
 
 const RUNNER = new TestRunner(suggestPsp);
 
 describe('suggestPsp', () => {
-  it('empty configs is noop', RUNNER.assertCallback());
+  it('empty configs is noop', RUNNER.assertCallback(undefined, 'unchanged'));
 
   it(
     'suggest PSP with allowPrivilegeEscalation = true to false',
     RUNNER.assertCallback(
       new Configs([psp(true)]),
-      new Configs([psp(true)]),
-      ConfigError
+      new Configs([psp(true)], undefined, [
+        kubernetesObjectResult(
+          'Suggest explicitly disabling privilege escalation',
+          psp(true),
+          { path: 'spec.allowPrivilegeEscalation', suggestedValue: false },
+          'warn',
+          { category: 'security' }
+        ),
+      ])
     )
   );
 
   it(
     'leaves PSP with allowPrivilegeEscalation = false alone',
-    RUNNER.assertCallback(new Configs([psp(false)]))
+    RUNNER.assertCallback(new Configs([psp(false)]), 'unchanged')
   );
 });