Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make AppSync and MappingTemplate L2-constructs work with CodePipeline deployments #8052

Closed
2 tasks
asterikx opened this issue May 18, 2020 · 9 comments
Closed
2 tasks
Labels
@aws-cdk/aws-appsync Related to AWS AppSync @aws-cdk/aws-codepipeline Related to AWS CodePipeline closed-for-staleness This issue was automatically closed because it hadn't received any attention in a while. effort/medium Medium work item – several days of effort feature-request A feature should be added or improved. p2

Comments

@asterikx
Copy link
Contributor

Deploy an AppSync API with a schema definition file and resolver mapping templates in a CodePipeline using CloudFormation actions.

Use Case

I want to deploy an AppSync API in a CodePipeline and I don't want to inline the schema into the CDK code because I want to generate model classes from it.

Currently, the schema definition must be provided either as a string or as an asset (not yet supported by CloudFormations actions).
It is not possible to provide S3-locations without redefining the current L2-construct.

CDK source code:

let definition;
if (props.schemaDefinition) {
definition = props.schemaDefinition;
} else if (props.schemaDefinitionFile) {
definition = readFileSync(props.schemaDefinitionFile).toString('UTF-8');
} else {
throw new Error('Missing Schema definition. Provide schemaDefinition or schemaDefinitionFile');
}

Proposed Solution

Provide S3-locations of the schema definition file and any resolver mapping templates to the synthesized template using CloudFormation parameters, similar to CfnParametersCode (see example here).

Since the number of mapping templates can be very high, a solution that requires only one parameter representing the S3-folder containing the mapping templates would be useful.

Creating an AppSync API and mapping templates could look like:

const schema = new cdk.CfnParameter(this, 'GraphQLSchemaS3Location');
const resolversDir = new cdk.CfnParameter(this, 'GraphQLMappingTemplatesS3Location');

const api = new appsync.GraphQLApi(this, 'GraphQLApi', {
  name: `GraphQLApi`,
  schemaDefinitionLocation: schema,
});

someDataSource.createQueryResolver({
  field: 'getHello',
  requestTemplate:  appsync.MappingTemplate.fromS3Location(resolversDir, 'getHello-request-mapping-template.vtl'),
  responseTemplate: appsync.MappingTemplate.fromS3Location(resolversDir, 'getHello-response-mapping-template.vtl'),
});

Other

Related question on Gitter.

  • 👋 I may be able to implement this feature request
  • ⚠️ This feature might incur a breaking change

This is a 🚀 Feature Request

@asterikx asterikx added feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged. labels May 18, 2020
@asterikx asterikx changed the title Make AppSync and MappingTemplate L2-constructs work with CodePipeline. Make AppSync and MappingTemplate L2-constructs work with CodePipeline deployments May 18, 2020
@SomayaB SomayaB added @aws-cdk/aws-appsync Related to AWS AppSync @aws-cdk/aws-codepipeline Related to AWS CodePipeline labels May 19, 2020
@asterikx
Copy link
Contributor Author

Is this covered by aws/aws-cdk-rfcs#49?

I can't deploy using CI/CD at the moment because of missing assets support 😞

@skinny85 skinny85 added effort/medium Medium work item – several days of effort and removed needs-triage This issue or PR still needs to be triaged. labels Jun 29, 2020
@BryanPan342
Copy link
Contributor

BryanPan342 commented Jul 17, 2020

It looks like CloudFormations already has support for using s3 locations: resolver docs and schema docs.

Seems like the most minimal solution would be to offer a prop for GraphQLApi that takes a S3 location and to open up props in resolver for requestMappingTemplateS3Location and responseMappingTemplateS3Location.

Both would be all would be strings as that's what the CloudFormation props take...

@asterikx what do you think? I know this was a little different with the proposed solution.

@asterikx
Copy link
Contributor Author

asterikx commented Jul 17, 2020

@BryanPan342 Thanks for you response.

Both would be all would be strings as that's what the CloudFormation props take...

I don't completely understand. Could you maybe sketch your solution? My idea was that the CDK would take care of uploading the referenced schema/resolver files to S3 and use the corresponding S3 locations (strings) as input parameters to the synthesized CFN template during deployment. Very similar to how lambda.CfnParametersCode, see this example.

Since pipelines.CdkPipeline now supports Assets, I think sth. like the following would best cover all use cases (cdk deploy and pipeline deployments using pipelines.CdkPipeline)

const api = new appsync.GraphQLApi(this, 'GraphQLApi', {
  name: `GraphQLApi`,
  schemaDefinition: appsync.SchemaDefinition.fromAsset('./schema.graphql'),
});

someDataSource.createQueryResolver({
  field: 'getHello',
  requestTemplate:  appsync.MappingTemplate.fromAsset('./resolvers/getHello-request-mapping-template.vtl'),
  responseTemplate: appsync.MappingTemplate.fromAsset('./resolvers/getHello-response-mapping-template.vtl'),
});

@BryanPan342
Copy link
Contributor

@asterikx

Ah I see. The feature you are suggesting is for CDK to upload assets with AppSync and route the S3 location into the CloudFormation definition.

I'm going to investigate this further, but what's the issue of using something like this for schema:

const api = new appsync.GraphQLApi(this, 'GraphQLApi', {
  name: `GraphQLApi`,
  schemaDefinitionFile: path.join(__dirname, 'schema.graphql'),
});

@BryanPan342 BryanPan342 added the investigating This issue is being investigated and/or work is in progress to resolve the issue. label Jul 20, 2020
@asterikx
Copy link
Contributor Author

asterikx commented Jul 20, 2020

Ah I see. The feature you are suggesting is for CDK to upload assets with AppSync and route the S3 location into the CloudFormation definition.

Yes.

I'm going to investigate this further, but what's the issue of using something like this for schema

True, this was a conceptual misunderstanding from my side. Indeed schemaDefinitionFile will simply inline the schema during synthesis.

if (props.schemaDefinition) {
definition = props.schemaDefinition;
} else if (props.schemaDefinitionFile) {
definition = readFileSync(props.schemaDefinitionFile).toString('UTF-8');
} else {
throw new Error('Missing Schema definition. Provide schemaDefinition or schemaDefinitionFile');
}

I thought schemaDefinitionFile would use Assets under the hood, which is not supported by codepipeline.Pipeline (see this issue for example).

Also, pipelines.CdkPipeline now supports asset publishing to S3. So it would be useful to support S3 locations in AppSync. In this way, the size of the synthesized CFN templates could also be reduced, as the inlined schema definition and the resolver templates are significantly larger (depending on the application) than the corresponding references to S3. One problem I see is that the number of assets explodes as the API grows, as we need to two assets per operation (request and response mapping templates). Not quite sure if that scales with the current asset publishing implementation (see here and here). What do you think @BryanPan342 @skinny85?

@BryanPan342
Copy link
Contributor

One problem I see is that the number of assets explodes as the API grows, as we need to two assets per operation (request and response mapping templates)

@asterikx this is a really good point that we have also talked about. For now, we thought that it would be okay not to have s3 asset publishing because we haven't gotten an issue raised about size yet. And even if schema.graphql was large, a workaround would be to segment your GraphQL API across multiple APIs.

I dont quite understand this part.

One problem I see is that the number of assets explodes as the API grows, as we need to two assets per operation (request and response mapping templates)

I'm not an expert on cdkPipeline but if im not mistaken, the mapping templates aren't techinically assets because they are attributes to a AWS::AppSync::Resolver.

@asterikx
Copy link
Contributor Author

asterikx commented Jul 21, 2020

One problem I see is that the number of assets explodes as the API grows, as we need to two assets per operation (request and response mapping templates)

With the current implementation, there are no assets. Sorry for the confusion, I was hypothesizing here. If we were to support RequestMappingTemplateS3Location and ResponseMappingTemplateS3Location through assets, the number of required assets for an AppSync API would grow linearly with the number of resolvers, i.e. the number of operations.

I'm fine with closing this for now. There might be more critical issues at this time.

@BryanPan342
Copy link
Contributor

Ah I see. I think this discussion would be great to keep open.

I think if we could change the issue name/message to reflect our discussion that would be fantastic!

@BryanPan342 BryanPan342 removed the investigating This issue is being investigated and/or work is in progress to resolve the issue. label Jul 21, 2020
@skinny85 skinny85 added the p2 label Sep 11, 2020
@github-actions
Copy link

This issue has not received any attention in 1 year. If you want to keep this issue open, please leave a comment below and auto-close will be canceled.

@github-actions github-actions bot added closing-soon This issue will automatically close in 4 days unless further comments are made. closed-for-staleness This issue was automatically closed because it hadn't received any attention in a while. and removed closing-soon This issue will automatically close in 4 days unless further comments are made. labels Jun 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-appsync Related to AWS AppSync @aws-cdk/aws-codepipeline Related to AWS CodePipeline closed-for-staleness This issue was automatically closed because it hadn't received any attention in a while. effort/medium Medium work item – several days of effort feature-request A feature should be added or improved. p2
Projects
None yet
Development

No branches or pull requests

4 participants