Skip to content

Add mutation for createRun#75

Merged
josephburgess merged 26 commits intotesterloop:multitenantfrom
josephburgess:add-mutation
Jul 12, 2023
Merged

Add mutation for createRun#75
josephburgess merged 26 commits intotesterloop:multitenantfrom
josephburgess:add-mutation

Conversation

@josephburgess
Copy link
Copy Markdown
Member

  • Add Mutation type to schema, along with UploadInfo and Field types
  • Add createPresignedPost to S3Service
  • Add datasource for createRun
  • Add Mutation.createRun resolver

The directory here: https://s3.console.aws.amazon.com/s3/buckets/otf-lambda-results?prefix=testClient2/&region=eu-west-3 was added through the testerloop-server graphQL API, using one call, and one link, to upload multiple files.

Comment thread src/S3Service.ts Outdated
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we also get presigned get request?
@sbartsa I believe you highlighted that we might need it in testerloop-cli is this still correct?

Copy link
Copy Markdown
Contributor

@dlawrynowicz dlawrynowicz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need to generate runID server side

Comment thread src/schema/mutations/CreateRun.graphql Outdated
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this mutation should return runID (uuid v4) so it should not be a parameter here.

Comment thread src/resolvers/Mutation.ts Outdated
Comment thread src/S3Service.ts Outdated
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will be good to add eslint, not to reformat all files based on user local IDE config. looks like we changed from 2 to 4 spaces

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the standard is actually 4 spaces for us, but this file was erroneously on 2 spaces still... but yes ESLint would be helpful!

@josephburgess
Copy link
Copy Markdown
Member Author

I think we need to generate runID server side

Yep fair enough - thought it was generated on CLI tool side, but that should be simple enough to implement. As discussed will park this until DB is up and running though.

@josephburgess josephburgess changed the base branch from master to multitenant July 11, 2023 13:17
@josephburgess
Copy link
Copy Markdown
Member Author

josephburgess commented Jul 11, 2023

Changes made in line with discussions and addressing ticket #427: https://trello.com/c/EgAaSeJu/427-build-new-mutation-to-register-new-run-with-graphql-api

  • Mutation.CreateRun schema:

    • runEnvironmentDetails required input - stringified/escaped cicd.json data to be passed to server to upload to S3
    • s3Config optional input to contain customerPath and s3BucketName for upload of cicd.json and to configure the presigned POST link returned by server
  • Mutation.CreateRun resolver:

    • Creates runId
    • Checks for s3config parameter
    • If s3Config not provided, resolver checks headers for an api-key
    • If api-key is present, use api-key to look up Organisation in database to use for S3 configuration (customerPath and s3BucketName)
    • Calls dataSources.createRun.uploadCICDFileToS3 to upload cicd.json to ${s3BucketName}/${customerPath}/${runId}/logs
    • Calls dataSources.createRun.getUploadLink to return presigned POST details in the response, which can be used for uploading completed log files to ${s3BucketName}/${customerPath}/${runId}/{...}
  • dataSources.createRun:

    • Takes parameters s3BucketName, customerPath, runID and uses them to:
      • Generate presigned POST url and fields (i.e. form data) for uploading multiple files to that specific S3 path (getUploadLink())
      • Upload runEnvironmentDetails provided by CLI tool in the form of cicd.json file (uploadCICDFileToS3())
  • Other changes:

    • S3Service.ts: add getSignedUrl and putObject functions for obtaining presigned post url and uploading cicd.json respectively
    • context.ts: add request headers to context to be used by resolver to check for valid api-keys
    • db.ts: add getByApiKey function to look up an api-key and return an Organisation along with their ApiKey details
    • Update import statements to use relative imports and .js suffixes where needed
    • Fix typo in Prisma schema (s3CustomPath)

Validation

This directory was created by calling the mutation using a valid api-key in the header of the request (using a local database environment): https://s3.console.aws.amazon.com/s3/buckets/otf-lambda-results?prefix=testOrgCustomPath/&region=eu-west-3

This was created using the S3Config input parameters with no API key: https://s3.console.aws.amazon.com/s3/buckets/otf-lambda-results?region=eu-west-3&prefix=customPathWithS3Config/&showversions=false

Invalid requests e.g. missing S3 details, incorrect API key or in-active API key throw appropriate error responses.

Comment thread src/db.ts Outdated
Comment thread src/resolvers/Mutation.ts Outdated
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should make sure that s3Config was not provided here, otherwise we can be in situation that we checked DB we have s3Config, but we are going to use data from s3Config because of those lines
https://github.com/testerloop/testerloop-server/pull/75/files#diff-b7409741fd2ee242c3d3d2e7bf257d72539f5eb8a57889bc1026af19326eb8d5R36
https://github.com/testerloop/testerloop-server/pull/75/files#diff-b7409741fd2ee242c3d3d2e7bf257d72539f5eb8a57889bc1026af19326eb8d5R38

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done - and refactored the api key handling into a util since we'll probably use it elsewhere

Copy link
Copy Markdown
Contributor

@dlawrynowicz dlawrynowicz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left small comments, looks good!

@josephburgess josephburgess merged commit d456e25 into testerloop:multitenant Jul 12, 2023
@josephburgess josephburgess deleted the add-mutation branch July 12, 2023 09:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants