From 50c7fdf5ec1be786a00f6d43b6d4d29580bffc10 Mon Sep 17 00:00:00 2001 From: Blaize M Kaye Date: Tue, 17 Oct 2023 12:47:40 +1300 Subject: [PATCH 1/5] Changes strategy for advanced task validation --- .../task/advancedtasktoolbox.test.ts | 28 +-- .../advancedTaskDefinitionArgument.test.ts | 57 +++++ .../models/advancedTaskDefinitionArgument.ts | 208 +++++++++++++----- .../task/task_definition_resolvers.ts | 189 ++++++++-------- 4 files changed, 318 insertions(+), 164 deletions(-) create mode 100644 services/api/src/resources/task/models/advancedTaskDefinitionArgument.test.ts diff --git a/services/api/src/resources/task/advancedtasktoolbox.test.ts b/services/api/src/resources/task/advancedtasktoolbox.test.ts index 413c1bd422..62b0992112 100644 --- a/services/api/src/resources/task/advancedtasktoolbox.test.ts +++ b/services/api/src/resources/task/advancedtasktoolbox.test.ts @@ -12,9 +12,9 @@ interface IPermissionsMockItem { } const areIKeycloakAuthAttributesEqual = (a: IKeycloakAuthAttributes, b: IKeycloakAuthAttributes) => { - const userSort = R.partial(R.sort, [(a, b) => { return a - b;}]); + const userSort = R.partial(R.sort, [(a, b) => { return a - b; }]); return R.equals(a.project, b.project) && R.equals(a.group, b.group) && - R.equals(userSort(a.users || []), userSort(b.users || [])); + R.equals(userSort(a.users || []), userSort(b.users || [])); } //Mock out the hasPermissions function @@ -23,14 +23,14 @@ const mockHasPermission = (permissions: Array) => { return async (resource, scope, attributes: IKeycloakAuthAttributes = {}) => { let match = false; permissions.forEach(element => { - if(element.resource == resource && + if (element.resource == resource && scope == element.scope && areIKeycloakAuthAttributesEqual(element.attributes, attributes) - ) { + ) { match = true; } }); - if(match) { return true; } + if (match) { return true; } throw new KeycloakUnauthorizedError(`Unauthorized: You don't have permission to "${scope}" on "${resource}".`); } } @@ -41,10 +41,10 @@ describe('advancedtasktoolbox', () => { describe('canUserSeeTaskDefinition', () => { let environmentById = jest.fn((id: number) => { - if(id == 1) { - return {project: 1}; + if (id == 1) { + return { project: 1 }; } - return {project: 2}; + return { project: 2 }; }); let environmentHelpers = { @@ -52,15 +52,15 @@ describe('advancedtasktoolbox', () => { }; //This user has permission to view tasks on - let hasPermissions = mockHasPermission([{resource: 'task', scope: 'view', attributes: {project: 1}}]) + let hasPermissions = mockHasPermission([{ resource: 'task', scope: 'view', attributes: { project: 1 } }]) let ath = advancedTaskFunctionFactory({}, hasPermissions, {}, {}, environmentHelpers, {}); test('test user is granted permission when invoking a project she has access to', () => { - return expect(ath.permissions.canUserSeeTaskDefinition({environment: 1})).resolves.toBe(true); + return expect(ath.permissions.canUserSeeTaskDefinition({ environment: 1 })).resolves.toBe(true); }); test('test user is denied permission to a project she doesnt have access to', () => { - return expect(ath.permissions.canUserSeeTaskDefinition({environment: 2})).resolves.toBe(false); + return expect(ath.permissions.canUserSeeTaskDefinition({ environment: 2 })).resolves.toBe(false); }); }); }); @@ -69,12 +69,12 @@ describe('advancedtasktoolbox', () => { //Let's quickly ensure our test functions are working as expected describe('testSystemsMetaTest', () => { const usersPermissions = [ - {resource: 'task', scope: 'view', attributes: {users: [1,2,3]}}, - {resource: 'nothing', scope: 'whatever', attributes: {}}, + { resource: 'task', scope: 'view', attributes: { users: [1, 2, 3] } }, + { resource: 'nothing', scope: 'whatever', attributes: {} }, ]; test('should match our users permissions when running haspermissions', () => { let hasPermission = mockHasPermission(usersPermissions); - return expect(hasPermission('task', 'view', {users: [2,1,3]})).resolves.toBe(true); + return expect(hasPermission('task', 'view', { users: [2, 1, 3] })).resolves.toBe(true); }); }); diff --git a/services/api/src/resources/task/models/advancedTaskDefinitionArgument.test.ts b/services/api/src/resources/task/models/advancedTaskDefinitionArgument.test.ts new file mode 100644 index 0000000000..7d8b5b3639 --- /dev/null +++ b/services/api/src/resources/task/models/advancedTaskDefinitionArgument.test.ts @@ -0,0 +1,57 @@ +import { AdvancedTaskArgumentValidator, NumberArgument, EnvironmentSourceArgument, OtherEnvironmentSourceNamesArgument, StringArgument } from "./advancedTaskDefinitionArgument"; + +describe("Testing AdvancedTaskArgumentValidator class as a whole", () => { + + let arglist = [ + { name: "stringarg", type: StringArgument.typeName(), range: "", default: "" }, + { name: "envarg", type: EnvironmentSourceArgument.typeName(), range: "", default: "" }, + ]; + + let environmentNames = ["main", "dev", "qa"]; + + let myEnvironmentName = "main"; + + let advancedTaskValidator = new AdvancedTaskArgumentValidator(1, myEnvironmentName, environmentNames, arglist); + + test("string validator", () => { + return expect(advancedTaskValidator.validateArgument("stringarg", "arbitrary string")).toBe(true); + }); + + test("environment source argument validator", () => { + return expect(advancedTaskValidator.validateArgument("envarg", "dev")).toBe(true); + }); + + test("environment source argument validator - not in list", () => { + return expect(advancedTaskValidator.validateArgument("envarg", "notInEnvList")).toBe(false); + }); + +}); + +describe("advanced task def arguments individual validator tests", () => { + + test("numeric validator", () => { + let validator = new NumberArgument(); + return expect(validator.validateInput("5")).toBe(true); + }); + + + let environmentSourceArgument = new EnvironmentSourceArgument(["a", "b", "c"]); + test("EnvironmentSourceArgument with valid input", () => { + return expect(environmentSourceArgument.validateInput("a")).toBe(true); + }); + + test("EnvironmentSourceArgument with invalid input", () => { + return expect(environmentSourceArgument.validateInput("not-in-env-list")).toBe(false); + }); + + let otherEnvironmentSourceNamesArgument = new OtherEnvironmentSourceNamesArgument("myname", ["myname", "dev", "qa", "main"]); + test("OtherEnvironmentSourceNamesArgument test name that isn't mine is valid", () => { + return expect(otherEnvironmentSourceNamesArgument.validateInput("main")).toBe(true); + }); + + test("OtherEnvironmentSourceNamesArgument test name that is mine is invalid", () => { + // since this should filter all environments _except_ the target, this should fail + return expect(otherEnvironmentSourceNamesArgument.validateInput("myname")).toBe(false); + }); + +}); \ No newline at end of file diff --git a/services/api/src/resources/task/models/advancedTaskDefinitionArgument.ts b/services/api/src/resources/task/models/advancedTaskDefinitionArgument.ts index ed303f42f8..9d56caa53d 100644 --- a/services/api/src/resources/task/models/advancedTaskDefinitionArgument.ts +++ b/services/api/src/resources/task/models/advancedTaskDefinitionArgument.ts @@ -1,8 +1,9 @@ +import { sqlClientPool } from '../../../clients/sqlClient'; import { query } from '../../../util/db'; import * as R from 'ramda'; export class ArgumentBase { - async validateInput(input): Promise { + validateInput(input): boolean { return true; } @@ -10,48 +11,35 @@ export class ArgumentBase { return "BASE"; } - public async getArgumentRange() { + public getArgumentRange() { return []; } } export class EnvironmentSourceArgument extends ArgumentBase { - - protected sqlClientPool; - protected environmentId; protected environmentNameList = []; - constructor(sqlClientPool, environmentId) { + constructor(environmentNameList) { super(); - this.sqlClientPool = sqlClientPool; - this.environmentId = environmentId; + this.environmentNameList = environmentNameList; } public static typeName() { return "ENVIRONMENT_SOURCE_NAME"; } - public async getArgumentRange() { - await this.loadEnvNames(); + public getArgumentRange() { return this.environmentNameList; } - protected async loadEnvNames() { - const rows = await query( - this.sqlClientPool, - `select e.name as name from environment as e inner join environment as p on e.project = p.project where p.id = ${this.environmentId}` - ); - this.environmentNameList = R.pluck('name')(rows); - } /** * * @param input Environment name * @returns boolean */ - async validateInput(input): Promise { - await this.loadEnvNames(); + validateInput(input): boolean { return this.environmentNameList.includes(input); } } @@ -59,40 +47,31 @@ export class EnvironmentSourceArgument extends ArgumentBase { export class OtherEnvironmentSourceNamesArgument extends ArgumentBase { - protected sqlClientPool; - protected environmentId; + protected environmentName; protected environmentNameList = []; - constructor(sqlClientPool, environmentId) { + constructor(environmentName, environmentNameList) { super(); - this.sqlClientPool = sqlClientPool; - this.environmentId = environmentId; + this.environmentName = environmentName; + // We simply filter out the target environment name here + this.environmentNameList = environmentNameList.filter((i) => i != environmentName); } public static typeName() { return "ENVIRONMENT_SOURCE_NAME_EXCLUDE_SELF"; } - public async getArgumentRange() { - await this.loadEnvNames(); + public getArgumentRange() { return this.environmentNameList; } - protected async loadEnvNames() { - const rows = await query( - this.sqlClientPool, - `select e.name as name from environment as e inner join environment as p on e.project = p.project where p.id = ${this.environmentId} and e.id != ${this.environmentId}` - ); - this.environmentNameList = R.pluck('name')(rows); - } /** * * @param input Environment name * @returns boolean */ - async validateInput(input): Promise { - await this.loadEnvNames(); + validateInput(input): boolean { return this.environmentNameList.includes(input); } } @@ -104,11 +83,11 @@ export class StringArgument extends ArgumentBase { return "STRING"; } - async validateInput(input): Promise { + validateInput(input): boolean { return true; } - public async getArgumentRange() { + public getArgumentRange() { return null; } } @@ -120,36 +99,147 @@ export class NumberArgument { return "NUMERIC"; } - async validateInput(input): Promise { + validateInput(input): boolean { return /^[0-9\.]+$/.test(input); } - public async getArgumentRange() { + public getArgumentRange() { return null; } } +export const getAdvancedTaskArgumentValidator = (sqlClientPool, environment, taskArguments) => { + const rows = await query( + sqlClientPool, + `select e.name as name from environment as e inner join environment as p on e.project = p.project where p.id = ${environment.id} and e.id != ${environment.id}` + ); + let environmentNameList = R.pluck('name')(rows); + return new AdvancedTaskArgumentValidator(environment.id, environment.name, environmentNameList, taskArguments); +} + +export class AdvancedTaskArgumentValidator { + + protected environmentId; + protected environmentName; + protected relatedEnvironments; + protected taskArguments; -/** - * @param name The name of the advancedTaskDefinition type (stored in field) - */ -export const advancedTaskDefinitionTypeFactory = (sqlClientPool, task, environment) => (name) => { - switch(name) { - case(EnvironmentSourceArgument.typeName()): - return new EnvironmentSourceArgument(sqlClientPool, environment); - break; - case(StringArgument.typeName()): - return new StringArgument(); - break; - case(NumberArgument.typeName()): - return new NumberArgument(); - break; - case(OtherEnvironmentSourceNamesArgument.typeName()): - return new OtherEnvironmentSourceNamesArgument(sqlClientPool, environment); - break; - default: - throw new Error(`Unable to find AdvancedTaskDefinitionType ${name}`); - break; + constructor(environmentId: number, environmentName: string, relatedEnvironments, taskArguments) { + this.environmentId = environmentId; + this.environmentName = environmentName; + this.relatedEnvironments = relatedEnvironments; + this.taskArguments = taskArguments; } + + public validateArgument(argumentName, argumentValue) { + let advancedTaskDefinitionArgument = R.find(R.propEq("name", argumentName), this.taskArguments); + if (advancedTaskDefinitionArgument == null || advancedTaskDefinitionArgument == undefined) { + throw new Error(`Unable to find argument ${argumentName} in argument list for task`); + } + + //@ts-ignore + let typename = advancedTaskDefinitionArgument.type; + let validator = this.getValidatorForArg(typename); + return validator.validateInput(argumentValue); + } + + public validatorForArgument(argumentName) { + let advancedTaskDefinitionArgument = R.find(R.propEq("name", argumentName), this.taskArguments); + if (advancedTaskDefinitionArgument == null || advancedTaskDefinitionArgument == undefined) { + throw new Error(`Unable to find argument ${argumentName} in argument list for task`); + } + + //@ts-ignore + let typename = advancedTaskDefinitionArgument.type; + let validator = this.getValidatorForArg(typename); + return validator; + } + + protected getValidatorForArg(typename) { + + let validator = null; + + switch (typename) { + case (EnvironmentSourceArgument.typeName()): + validator = new EnvironmentSourceArgument(this.relatedEnvironments); + break; + case (StringArgument.typeName()): + validator = new StringArgument(); + break; + case (NumberArgument.typeName()): + validator = new NumberArgument(); + break; + case (OtherEnvironmentSourceNamesArgument.typeName()): + validator = new OtherEnvironmentSourceNamesArgument(this.environmentName, this.relatedEnvironments); + break; + default: + throw new Error(`Unable to find AdvancedTaskDefinitionType ${typename}`); + break; + } + + return validator; + } + } + +// /** +// * This will take in an sql client pool as well as a set of tasks to validate against, will, when passed an argument name and value, will validate the argument +// * importantly, this is really a wrapper around a testable set of classes and functions - this function is the scaffolding that creates the various objects +// * +// * @param sqlClientPool +// * @param environment +// * @param taskArguments +// * @returns +// */ + +// export const getAdvancedTaskArgumentValidator = async (sqlClientPool, environment, taskArguments) => { + +// const rows = await query( +// sqlClientPool, +// `select e.name as name from environment as e inner join environment as p on e.project = p.project where p.id = ${environment.id} and e.id != ${environment.id}` +// ); + +// let environmentNameList = R.pluck('name')(rows); // This is used for validators requiring environment names + +// return async (argumentName, value) => { + +// // Get environment list for task + +// let advancedTaskDefinitionArgument = R.find(R.propEq("name", argumentName), taskArguments); +// if (advancedTaskDefinitionArgument == null || advancedTaskDefinitionArgument == undefined) { +// throw new Error(`Unable to find argument ${argumentName} in argument list for task`); +// } + +// // let range = advancedTaskDefinitionArgument.range; + +// //New up the appropriate type + +// } + +// }; + + + +// /** +// * @param name The name of the advancedTaskDefinition type (stored in field) +// */ +// export const advancedTaskDefinitionTypeFactory = (sqlClientPool, task, environment) => (name) => { +// switch(name) { +// case(EnvironmentSourceArgument.typeName()): +// return new EnvironmentSourceArgument(sqlClientPool, environment); +// break; +// case(StringArgument.typeName()): +// return new StringArgument(); +// break; +// case(NumberArgument.typeName()): +// return new NumberArgument(); +// break; +// case(OtherEnvironmentSourceNamesArgument.typeName()): +// return new OtherEnvironmentSourceNamesArgument(sqlClientPool, environment); +// break; +// default: +// throw new Error(`Unable to find AdvancedTaskDefinitionType ${name}`); +// break; +// } +// } diff --git a/services/api/src/resources/task/task_definition_resolvers.ts b/services/api/src/resources/task/task_definition_resolvers.ts index 407af10c4f..3c172e8615 100644 --- a/services/api/src/resources/task/task_definition_resolvers.ts +++ b/services/api/src/resources/task/task_definition_resolvers.ts @@ -42,11 +42,11 @@ const PermissionsToRBAC = (permission: string) => { return `invoke:${permission.toLowerCase()}`; }; -export const allAdvancedTaskDefinitions = async (root, args, {sqlClientPool, hasPermission, models}) => { +export const allAdvancedTaskDefinitions = async (root, args, { sqlClientPool, hasPermission, models }) => { //is the user a system admin? try { - await hasPermission('advanced_task','create:advanced'); - } catch(e) { + await hasPermission('advanced_task', 'create:advanced'); + } catch (e) { throw new KeycloakUnauthorizedError("Only system admins have access to view all advanced task definitions"); } @@ -57,7 +57,7 @@ export const allAdvancedTaskDefinitions = async (root, args, {sqlClientPool, has const atf = advancedTaskToolbox.advancedTaskFunctions(sqlClientPool, models, hasPermission); - for(let i = 0; i < adTaskDefs.length; i++) { + for (let i = 0; i < adTaskDefs.length; i++) { adTaskDefs[i].advancedTaskDefinitionArguments = await atf.advancedTaskDefinitionArguments(adTaskDefs[i].id); } @@ -76,7 +76,7 @@ export const advancedTaskDefinitionById = async ( id ); - if(await atf.permissions.canUserSeeTaskDefinition(advancedTaskDef) == false) { + if (await atf.permissions.canUserSeeTaskDefinition(advancedTaskDef) == false) { throw new Error("You do not have permission"); } @@ -86,7 +86,7 @@ export const advancedTaskDefinitionById = async ( export const getRegisteredTasksByEnvironmentId = async ( { id }, - {}, + { }, { sqlClientPool, hasPermission, models, keycloakGroups } ) => { let rows; @@ -121,6 +121,7 @@ export const resolveTasksForEnvironment = async ( Sql.selectAdvancedTaskDefinitionsForEnvironment(environment) ); + const proj = await projectHelpers(sqlClientPool).getProjectByEnvironmentId( environment ); @@ -156,17 +157,19 @@ export const resolveTasksForEnvironment = async ( const atf = advancedTaskToolbox.advancedTaskFunctions(sqlClientPool, models, hasPermission); - let typeValidatorFactory = advancedTaskArgument.advancedTaskDefinitionTypeFactory(sqlClientPool, null, environment); + // let typeValidatorFactory = advancedTaskArgument.advancedTaskDefinitionTypeFactory(sqlClientPool, null, environment); + // TODO: this needs to be somehow refactored into all lookups. // we might need a "load task" function or something. - for(let i = 0; i < rows.length; i++ ) { + for (let i = 0; i < rows.length; i++) { //@ts-ignore let argsForTask = await atf.advancedTaskDefinitionArguments(rows[i].id); + let typeValidator = await advancedTaskArgument.getAdvancedTaskArgumentValidator(sqlClientPool, environmentDetails, argsForTask); let processedArgs = []; - for(let i = 0; i < argsForTask.length; i++) { + for (let i = 0; i < argsForTask.length; i++) { let processing = argsForTask[i]; - let validator: advancedTaskArgument.ArgumentBase = typeValidatorFactory(processing.type); - processing.range = await validator.getArgumentRange(); + let validator: advancedTaskArgument.ArgumentBase = typeValidator.validatorForArgument(processing.name); + processing.range = validator.getArgumentRange(); processedArgs.push(processing); } @@ -263,7 +266,7 @@ export const addAdvancedTaskDefinition = async ( // We need to validate the incoming data // first, we actually only want either groupName, environment, or project - if(environment && groupName || environment && project || project && groupName) { + if (environment && groupName || environment && project || project && groupName) { throw Error("Only one of `environment`, `project`, or `groupName` should be set when creating a custom task."); } @@ -337,12 +340,14 @@ export const addAdvancedTaskDefinition = async ( ); //now attach arguments - if(advancedTaskDefinitionArguments) { - for(let i = 0; i < advancedTaskDefinitionArguments.length; i++) { + if (advancedTaskDefinitionArguments) { + const environmentDetails = await environmentHelpers( + sqlClientPool + ).getEnvironmentById(environment); + let typeValidator = await advancedTaskArgument.getAdvancedTaskArgumentValidator(sqlClientPool, environmentDetails, advancedTaskDefinitionArguments); + for (let i = 0; i < advancedTaskDefinitionArguments.length; i++) { if (advancedTaskDefinitionArguments[i].defaultValue) { - let typeValidatorFactory = advancedTaskArgument.advancedTaskDefinitionTypeFactory(sqlClientPool, null, environment); - let validator: advancedTaskArgument.ArgumentBase = typeValidatorFactory(advancedTaskDefinitionArguments[i].type); - if(!(await validator.validateInput(advancedTaskDefinitionArguments[i].defaultValue))) { + if (!(await typeValidator.validateArgument(advancedTaskDefinitionArguments[i].name, advancedTaskDefinitionArguments[i].defaultValue))) { throw new Error(`Invalid input defaultValue "${advancedTaskDefinitionArguments[i].defaultValue}" for type "${advancedTaskDefinitionArguments[i].type}" given for argument "${advancedTaskDefinitionArguments[i].name}"`); } } @@ -419,7 +424,7 @@ export const updateAdvancedTaskDefinition = async ( await checkAdvancedTaskPermissions(task, hasPermission, models, projectObj); // we will merge the patch into the advanced task to double check that the final data is still valid - task = {...task, ...patch}; + task = { ...task, ...patch }; validateAdvancedTaskDefinitionData(task, image, command, type); @@ -452,7 +457,7 @@ export const updateAdvancedTaskDefinition = async ( ); //add advanced task definition arguments - for(let i = 0; i < advancedTaskDefinitionArguments.length; i++) { + for (let i = 0; i < advancedTaskDefinitionArguments.length; i++) { await query( sqlClientPool, Sql.insertAdvancedTaskDefinitionArgument({ @@ -509,6 +514,10 @@ export const invokeRegisteredTask = async ( ) => { await envValidators(sqlClientPool).environmentExists(environment); + const environmentDetails = await environmentHelpers( + sqlClientPool + ).getEnvironmentById(environment); + let task = await getNamedAdvancedTaskForEnvironment( sqlClientPool, hasPermission, @@ -527,26 +536,28 @@ export const invokeRegisteredTask = async ( //let's grab something that'll be able to tell us whether our arguments //are valid - const typeValidatorFactory = advancedTaskArgument.advancedTaskDefinitionTypeFactory(sqlClientPool, task, environment); + // const typeValidatorFactory = advancedTaskArgument.advancedTaskDefinitionTypeFactory(sqlClientPool, task, environment); + let typeValidator = await advancedTaskArgument.getAdvancedTaskArgumentValidator(sqlClientPool, environmentDetails, taskArgs); + - if(argumentValues) { - for(let i = 0; i < argumentValues.length; i++) { + if (argumentValues) { + for (let i = 0; i < argumentValues.length; i++) { //grab the type for this one - let {advancedTaskDefinitionArgumentName, value} = argumentValues[i]; + let { advancedTaskDefinitionArgumentName, value } = argumentValues[i]; let taskArgDef = R.find(R.propEq('name', advancedTaskDefinitionArgumentName))(taskArgs); - if(!taskArgDef) { + if (!taskArgDef) { throw new Error(`Cannot find argument type named ${advancedTaskDefinitionArgumentName}`); } // if the value is empty and there is a default value on the argument if (value.trim() === "" && taskArgDef["defaultValue"]) { - value = taskArgDef["defaultValue"] + value = taskArgDef["defaultValue"] } //@ts-ignore - let validator: advancedTaskArgument.ArgumentBase = typeValidatorFactory(taskArgDef.type); + // let validator: advancedTaskArgument.ArgumentBase = typeValidatorFactory(taskArgDef.type); - if(!(await validator.validateInput(value))) { + if (!(typeValidator.validateArgument(advancedTaskDefinitionArgumentName, value))) { //@ts-ignore throw new Error(`Invalid input "${value}" for type "${taskArgDef.type}" given for argument "${advancedTaskDefinitionArgumentName}"`); } @@ -554,80 +565,76 @@ export const invokeRegisteredTask = async ( } - const environmentDetails = await environmentHelpers( - sqlClientPool - ).getEnvironmentById(environment); - await hasPermission('advanced_task', PermissionsToRBAC(task.permission), { project: environmentDetails.project }); switch (task.type) { - case TaskRegistration.TYPE_STANDARD: + case TaskRegistration.TYPE_STANDARD: - let taskCommandEnvs = ''; - let taskCommand = ""; + let taskCommandEnvs = ''; + let taskCommand = ""; - if(argumentValues && argumentValues.length > 0) { - taskCommandEnvs = R.reduce((acc, val) => { - //@ts-ignore - return `${acc} ${val.advancedTaskDefinitionArgumentName}="${val.value}"` - }, taskCommandEnvs, argumentValues); + if (argumentValues && argumentValues.length > 0) { + taskCommandEnvs = R.reduce((acc, val) => { + //@ts-ignore + return `${acc} ${val.advancedTaskDefinitionArgumentName}="${val.value}"` + }, taskCommandEnvs, argumentValues); - taskCommand += `${taskCommandEnvs}; `; - } + taskCommand += `${taskCommandEnvs}; `; + } - taskCommand += `${task.command}`; - - const taskData = await Helpers(sqlClientPool, hasPermission).addTask({ - name: task.name, - taskName: generateTaskName(), - environment: environment, - service: task.service, - command: taskCommand, - deployTokenInjection: task.deployTokenInjection, - projectKeyInjection: task.projectKeyInjection, - adminOnlyView: task.adminOnlyView, - execute: true - }); - return taskData; - break; - case TaskRegistration.TYPE_ADVANCED: - // the return data here is basically what gets dropped into the DB. - - // get any arguments ready for payload - let payload = {}; - if(argumentValues) { - for(let i = 0; i < argumentValues.length; i++) { - //@ts-ignore - payload[argumentValues[i].advancedTaskDefinitionArgumentName] = argumentValues[i].value; - } + taskCommand += `${task.command}`; + + const taskData = await Helpers(sqlClientPool, hasPermission).addTask({ + name: task.name, + taskName: generateTaskName(), + environment: environment, + service: task.service, + command: taskCommand, + deployTokenInjection: task.deployTokenInjection, + projectKeyInjection: task.projectKeyInjection, + adminOnlyView: task.adminOnlyView, + execute: true + }); + return taskData; + break; + case TaskRegistration.TYPE_ADVANCED: + // the return data here is basically what gets dropped into the DB. + + // get any arguments ready for payload + let payload = {}; + if (argumentValues) { + for (let i = 0; i < argumentValues.length; i++) { + //@ts-ignore + payload[argumentValues[i].advancedTaskDefinitionArgumentName] = argumentValues[i].value; } + } - const advancedTaskData = await Helpers(sqlClientPool, hasPermission).addAdvancedTask({ - name: task.name, - taskName: generateTaskName(), - created: undefined, - started: undefined, - completed: undefined, - environment, - service: task.service || 'cli', - image: task.image, //the return data here is basically what gets dropped into the DB. - payload: payload, - deployTokenInjection: task.deployTokenInjection, - projectKeyInjection: task.projectKeyInjection, - adminOnlyView: task.adminOnlyView, - remoteId: undefined, - execute: true - }); - - return advancedTaskData; - break; - default: - throw new Error('Cannot find matching task'); - break; - } + const advancedTaskData = await Helpers(sqlClientPool, hasPermission).addAdvancedTask({ + name: task.name, + taskName: generateTaskName(), + created: undefined, + started: undefined, + completed: undefined, + environment, + service: task.service || 'cli', + image: task.image, //the return data here is basically what gets dropped into the DB. + payload: payload, + deployTokenInjection: task.deployTokenInjection, + projectKeyInjection: task.projectKeyInjection, + adminOnlyView: task.adminOnlyView, + remoteId: undefined, + execute: true + }); + + return advancedTaskData; + break; + default: + throw new Error('Cannot find matching task'); + break; + } }; const getNamedAdvancedTaskForEnvironment = async ( @@ -637,7 +644,7 @@ const getNamedAdvancedTaskForEnvironment = async ( environment, models, keycloakGroups -):Promise => { +): Promise => { let rows; rows = await resolveTasksForEnvironment( @@ -761,7 +768,7 @@ function validateAdvancedTaskDefinitionData(input: any, image: any, command: any } } -async function checkAdvancedTaskPermissions(input:AdvancedTaskDefinitionInterface, hasPermission: any, models: any, projectObj: any) { +async function checkAdvancedTaskPermissions(input: AdvancedTaskDefinitionInterface, hasPermission: any, models: any, projectObj: any) { if (isAdvancedTaskDefinitionSystemLevelTask(input)) { //if they pass this, they can do basically anything //In the first release, we're not actually supporting this From 8cdbeab2a57c8d6f46cefbd84bda01a85c59f9ba Mon Sep 17 00:00:00 2001 From: Blaize M Kaye Date: Tue, 17 Oct 2023 12:50:48 +1300 Subject: [PATCH 2/5] fixes async call --- .../src/resources/task/models/advancedTaskDefinitionArgument.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/api/src/resources/task/models/advancedTaskDefinitionArgument.ts b/services/api/src/resources/task/models/advancedTaskDefinitionArgument.ts index 9d56caa53d..d5e47aba08 100644 --- a/services/api/src/resources/task/models/advancedTaskDefinitionArgument.ts +++ b/services/api/src/resources/task/models/advancedTaskDefinitionArgument.ts @@ -109,7 +109,7 @@ export class NumberArgument { } -export const getAdvancedTaskArgumentValidator = (sqlClientPool, environment, taskArguments) => { +export const getAdvancedTaskArgumentValidator = async (sqlClientPool, environment, taskArguments) => { const rows = await query( sqlClientPool, `select e.name as name from environment as e inner join environment as p on e.project = p.project where p.id = ${environment.id} and e.id != ${environment.id}` From 176c125f1bcf5b08e63b9560962205f5c94e0133 Mon Sep 17 00:00:00 2001 From: Blaize M Kaye Date: Tue, 17 Oct 2023 13:36:26 +1300 Subject: [PATCH 3/5] Fixes some logic in the adding of environment names --- .../task/models/advancedTaskDefinitionArgument.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/services/api/src/resources/task/models/advancedTaskDefinitionArgument.ts b/services/api/src/resources/task/models/advancedTaskDefinitionArgument.ts index d5e47aba08..5b61ad96cc 100644 --- a/services/api/src/resources/task/models/advancedTaskDefinitionArgument.ts +++ b/services/api/src/resources/task/models/advancedTaskDefinitionArgument.ts @@ -40,6 +40,8 @@ export class EnvironmentSourceArgument extends ArgumentBase { * @returns boolean */ validateInput(input): boolean { + console.log(`Got "${input} as input - list of environments follow:`); + console.log(this.environmentNameList); return this.environmentNameList.includes(input); } } @@ -72,6 +74,9 @@ export class OtherEnvironmentSourceNamesArgument extends ArgumentBase { * @returns boolean */ validateInput(input): boolean { + if (this.environmentName == input) { + return false; // we shouldn't match our own name - just anything that appears in the env list + } return this.environmentNameList.includes(input); } } @@ -112,7 +117,7 @@ export class NumberArgument { export const getAdvancedTaskArgumentValidator = async (sqlClientPool, environment, taskArguments) => { const rows = await query( sqlClientPool, - `select e.name as name from environment as e inner join environment as p on e.project = p.project where p.id = ${environment.id} and e.id != ${environment.id}` + `select e.name as name from environment as e inner join environment as p on e.project = p.project where p.id = ${environment.id}` ); let environmentNameList = R.pluck('name')(rows); return new AdvancedTaskArgumentValidator(environment.id, environment.name, environmentNameList, taskArguments); From 595671c4dd86b751dae0421358d95bcdf8013549 Mon Sep 17 00:00:00 2001 From: Blaize M Kaye Date: Tue, 17 Oct 2023 15:35:22 +1300 Subject: [PATCH 4/5] Removes extraneous code --- .../models/advancedTaskDefinitionArgument.ts | 61 ------------------- 1 file changed, 61 deletions(-) diff --git a/services/api/src/resources/task/models/advancedTaskDefinitionArgument.ts b/services/api/src/resources/task/models/advancedTaskDefinitionArgument.ts index 5b61ad96cc..325d8be9d9 100644 --- a/services/api/src/resources/task/models/advancedTaskDefinitionArgument.ts +++ b/services/api/src/resources/task/models/advancedTaskDefinitionArgument.ts @@ -187,64 +187,3 @@ export class AdvancedTaskArgumentValidator { } } - -// /** -// * This will take in an sql client pool as well as a set of tasks to validate against, will, when passed an argument name and value, will validate the argument -// * importantly, this is really a wrapper around a testable set of classes and functions - this function is the scaffolding that creates the various objects -// * -// * @param sqlClientPool -// * @param environment -// * @param taskArguments -// * @returns -// */ - -// export const getAdvancedTaskArgumentValidator = async (sqlClientPool, environment, taskArguments) => { - -// const rows = await query( -// sqlClientPool, -// `select e.name as name from environment as e inner join environment as p on e.project = p.project where p.id = ${environment.id} and e.id != ${environment.id}` -// ); - -// let environmentNameList = R.pluck('name')(rows); // This is used for validators requiring environment names - -// return async (argumentName, value) => { - -// // Get environment list for task - -// let advancedTaskDefinitionArgument = R.find(R.propEq("name", argumentName), taskArguments); -// if (advancedTaskDefinitionArgument == null || advancedTaskDefinitionArgument == undefined) { -// throw new Error(`Unable to find argument ${argumentName} in argument list for task`); -// } - -// // let range = advancedTaskDefinitionArgument.range; - -// //New up the appropriate type - -// } - -// }; - - - -// /** -// * @param name The name of the advancedTaskDefinition type (stored in field) -// */ -// export const advancedTaskDefinitionTypeFactory = (sqlClientPool, task, environment) => (name) => { -// switch(name) { -// case(EnvironmentSourceArgument.typeName()): -// return new EnvironmentSourceArgument(sqlClientPool, environment); -// break; -// case(StringArgument.typeName()): -// return new StringArgument(); -// break; -// case(NumberArgument.typeName()): -// return new NumberArgument(); -// break; -// case(OtherEnvironmentSourceNamesArgument.typeName()): -// return new OtherEnvironmentSourceNamesArgument(sqlClientPool, environment); -// break; -// default: -// throw new Error(`Unable to find AdvancedTaskDefinitionType ${name}`); -// break; -// } -// } From c256c2fae15b2fcaa4d33dc9f1fba64e0f465943 Mon Sep 17 00:00:00 2001 From: Blaize M Kaye Date: Tue, 17 Oct 2023 15:50:47 +1300 Subject: [PATCH 5/5] Removes console logs --- .../src/resources/task/models/advancedTaskDefinitionArgument.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/services/api/src/resources/task/models/advancedTaskDefinitionArgument.ts b/services/api/src/resources/task/models/advancedTaskDefinitionArgument.ts index 325d8be9d9..371e724abf 100644 --- a/services/api/src/resources/task/models/advancedTaskDefinitionArgument.ts +++ b/services/api/src/resources/task/models/advancedTaskDefinitionArgument.ts @@ -40,8 +40,6 @@ export class EnvironmentSourceArgument extends ArgumentBase { * @returns boolean */ validateInput(input): boolean { - console.log(`Got "${input} as input - list of environments follow:`); - console.log(this.environmentNameList); return this.environmentNameList.includes(input); } }