Skip to content

Commit

Permalink
fix: random key for each collection job (#1135)
Browse files Browse the repository at this point in the history
* fix: random key for each collection job

Squashed commit of the following:

commit 12ae1e0
Author: Mx. Corey Frang <gnarf37@gmail.com>
Date:   Mon Jun 17 14:19:08 2024 -0400

    got a little further, still blocked on tests

commit d54ffad
Author: Mx. Corey Frang <gnarf37@gmail.com>
Date:   Mon Jun 17 11:28:10 2024 -0400

    debuggering

commit 7bfa8f8
Author: Mx. Corey Frang <gnarf37@gmail.com>
Date:   Thu May 30 11:59:02 2024 -0400

    turbo logging trying to track down a bug

commit 842a459
Author: Mx. Corey Frang <gnarf37@gmail.com>
Date:   Wed May 29 12:21:15 2024 -0400

    remove server abstraction layer

* update to tests, not sure why needed some of these

* use node side uuid generation

* switch to beforeValidate hook

* only update secret if not there

* remove some minor diff changes from debugging

* Update server/models/services/CollectionJobService.js

Co-authored-by: Howard Edwards <howarde.edwards@gmail.com>

* leftover review feedback

---------

Co-authored-by: Howard Edwards <howarde.edwards@gmail.com>
  • Loading branch information
gnarf and howard-e committed Jul 15, 2024
1 parent 1f61c87 commit 78fab77
Show file tree
Hide file tree
Showing 12 changed files with 347 additions and 342 deletions.
4 changes: 3 additions & 1 deletion server/app.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,9 @@ const transactionRoutes = require('./routes/transactions');
const automationSchedulerRoutes = require('./routes/automation');
const path = require('path');
const apolloServer = require('./graphql-server');
const setupMockAutomationSchedulerServer = require('./tests/util/mock-automation-scheduler-server');
const {
setupMockAutomationSchedulerServer
} = require('./tests/util/mock-automation-scheduler-server');
const transactionMiddleware = require('./middleware/transactionMiddleware');
const app = express();

Expand Down
6 changes: 2 additions & 4 deletions server/controllers/AutomationController.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,6 @@ const getGraphQLContext = require('../graphql-context');
const httpAgent = new http.Agent({ family: 4 });

const axiosConfig = {
headers: {
'x-automation-secret': process.env.AUTOMATION_SCHEDULER_SECRET
},
timeout: 1000,
httpAgent
};
Expand Down Expand Up @@ -267,7 +264,8 @@ const updateJobResults = async (req, res) => {
} = {}
} = req.body;

const job = await getCollectionJobById({ id, transaction });
const job =
req.collectionJob ?? (await getCollectionJobById({ id, transaction }));
if (!job) {
throwNoJobFoundError(id);
}
Expand Down
1 change: 0 additions & 1 deletion server/graphql-context.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ const getGraphQLContext = ({ req }) => {

const atLoader = AtLoader();
const browserLoader = BrowserLoader();

return { user, atLoader, browserLoader, transaction };
};

Expand Down
25 changes: 20 additions & 5 deletions server/middleware/verifyAutomationScheduler.js
Original file line number Diff line number Diff line change
@@ -1,10 +1,25 @@
const verifyAutomationScheduler = (req, res, next) => {
const {
getCollectionJobById
} = require('../models/services/CollectionJobService');

const verifyAutomationScheduler = async (req, res, next) => {
const incomingSecret = req.headers['x-automation-secret'];
const { jobID: id } = req.params;

if (!id) return res.status(404).json({ error: 'unknown jobId param' });
const job = await getCollectionJobById({
id,
transaction: req.transaction
});
if (!job)
return res
.status(404)
.json({ error: `Could not find job with jobId: ${id}` });

// store the collection job on the request to avoid a second lookup
req.collectionJob = job;

if (
incomingSecret &&
incomingSecret === process.env.AUTOMATION_SCHEDULER_SECRET
) {
if (incomingSecret && incomingSecret === job.secret) {
next();
} else {
res.status(403).json({ error: 'Unauthorized' });
Expand Down
27 changes: 27 additions & 0 deletions server/migrations/20240525041559-addCollectionJobSecret.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
'use strict';
const uuid = require('uuid');

/** @type {import('sequelize-cli').Migration} */
module.exports = {
async up(queryInterface, Sequelize) {
return queryInterface.sequelize.transaction(async transaction => {
await queryInterface.addColumn(
'CollectionJob',
'secret',
{
type: Sequelize.DataTypes.UUID,
allowNull: false,
defaultValue: uuid.NIL
},
{ transaction }
);
});
},
async down(queryInterface) {
return queryInterface.sequelize.transaction(async transaction => {
await queryInterface.removeColumn('CollectionJob', 'secret', {
transaction
});
});
}
};
13 changes: 12 additions & 1 deletion server/models/CollectionJob.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
const { COLLECTION_JOB_STATUS } = require('../util/enums');

const { v4: uuid } = require('uuid');
const MODEL_NAME = 'CollectionJob';

module.exports = function (sequelize, DataTypes) {
Expand Down Expand Up @@ -31,9 +31,20 @@ module.exports = function (sequelize, DataTypes) {
onDelete: 'SET NULL',
allowNull: true,
unique: true
},
secret: {
type: DataTypes.UUID,
allowNull: false
}
},
{
hooks: {
beforeValidate: job => {
if (!job.secret) {
job.secret = uuid();
}
}
},
timestamps: false,
tableName: MODEL_NAME
}
Expand Down
28 changes: 8 additions & 20 deletions server/models/services/CollectionJobService.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,22 +19,20 @@ const {
} = require('./TestPlanRunService');
const { getTestPlanReportById } = require('./TestPlanReportService');
const { HttpQueryError } = require('apollo-server-core');
const { default: axios } = require('axios');

const {
default: createGithubWorkflow,
isEnabled: isGithubWorkflowEnabled
} = require('../../services/GithubWorkflowService');

const {
startCollectionJobSimulation
} = require('../../tests/util/mock-automation-scheduler-server');

const runnableTestsResolver = require('../../resolvers/TestPlanReport/runnableTestsResolver');
const getGraphQLContext = require('../../graphql-context');
const { getBotUserByAtId } = require('./UserService');

const axiosConfig = {
headers: {
'x-automation-secret': process.env.AUTOMATION_SCHEDULER_SECRET
},
timeout: 1000
};

// association helpers to be included with Models' results

/**
Expand Down Expand Up @@ -252,7 +250,6 @@ const createCollectionJob = async ({
})),
transaction
});

return ModelService.getById(CollectionJob, {
id: collectionJobResult.id,
attributes: collectionJobAttributes,
Expand Down Expand Up @@ -374,6 +371,7 @@ const getCollectionJobs = async ({
],
pagination,
transaction
// logging: console.log
});
};

Expand All @@ -393,17 +391,7 @@ const triggerWorkflow = async (job, testIds, { transaction }) => {
// TODO: pass the reduced list of testIds along / deal with them somehow
await createGithubWorkflow({ job, directory, gitSha });
} else {
await axios.post(
`${process.env.AUTOMATION_SCHEDULER_URL}/jobs/new`,
{
testPlanVersionGitSha: gitSha,
testIds,
testPlanName: directory,
jobId: job.id,
transactionId: transaction.id
},
axiosConfig
);
await startCollectionJobSimulation(job, transaction);
}
} catch (error) {
console.error(error);
Expand Down
1 change: 1 addition & 0 deletions server/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@
"sequelize": "^6.28.0",
"shared": "1.0.0",
"supertest-session": "^4.1.0",
"uuid": "^10.0.0",
"vhost": "^3.0.2"
},
"devDependencies": {
Expand Down
2 changes: 1 addition & 1 deletion server/services/GithubWorkflowService.js
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ const createGithubWorkflow = async ({ job, directory, gitSha }) => {
const inputs = {
callback_url: `https://${callbackUrlHostname}/api/jobs/${job.id}/test/:testRowNumber`,
status_url: `https://${callbackUrlHostname}/api/jobs/${job.id}`,
callback_header: `x-automation-secret:${process.env.AUTOMATION_SCHEDULER_SECRET}`,
callback_header: `x-automation-secret:${job.secret}`,
work_dir: `tests/${directory}`,
aria_at_ref: gitSha
};
Expand Down
Loading

0 comments on commit 78fab77

Please sign in to comment.