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

(aws_appsync): Unable to reference external SourceApis in Defintions.fromSourceApis() #27098

Closed
richardsonmarkj opened this issue Sep 11, 2023 · 4 comments
Labels
@aws-cdk/aws-appsync Related to AWS AppSync effort/small Small work item – less than a day of effort good first issue Related to contributions. See CONTRIBUTING.md p1

Comments

@richardsonmarkj
Copy link

Describe the bug

I believe this is simply a typo in the creation of the reference type SourceApi. The issue is that SourceApi.sourceApi is of a GraphqlApi type. This requires that the source API be created in the stack. This should instead be a IGraphqlApi type so that the results of a lookup using GraphqlApi.fromGraphqlApiAttributes

Expected Behavior

The following Typescript code compiles:

import * as cdk from "aws-cdk-lib"
import {Definition, GraphqlApi, MergeType} from "aws-cdk-lib/aws-appsync"
import {Construct} from "constructs"

export class BadStack extends cdk.Stack {
	constructor(scope:Construct, id:string, props?: cdk.StackProps) {
		super(scope, id, props)
		
		const apiId = "API_ID"
		const apiArn = "API_ARN"
		const api = new GraphqlApi(this, 'merged-api', {
			name: 'MergedAPI',
			definition: Definition.fromSourceApis({
				sourceApis: [
					{
						sourceApi: GraphqlApi.fromGraphqlApiAttributes(this, 'source-api', {
							graphqlApiId: apiId,
							graphqlApiArn: apiArn,
						}),
						mergeType: MergeType.AUTO_MERGE,
					}
				]
			})
		})
	}
}

Current Behavior

The code fails to compile reporting the following error

TS2740: Type 'IGraphqlApi' is missing the following properties from type 'GraphqlApi': graphqlUrl, name, schema, modes, and 24 more.  
graphqlapi.d.ts(304, 14): The expected type comes from property 'sourceApi' which is declared here on type 'SourceApi'

Reproduction Steps

import * as cdk from "aws-cdk-lib"
import {Definition, GraphqlApi, MergeType} from "aws-cdk-lib/aws-appsync"
import {Construct} from "constructs"

export class BadStack extends cdk.Stack {
	constructor(scope:Construct, id:string, props?: cdk.StackProps) {
		super(scope, id, props)
		
		const apiId = "API_ID"
		const apiArn = "API_ARN"
		const api = new GraphqlApi(this, 'merged-api', {
			name: 'MergedAPI',
			definition: Definition.fromSourceApis({
				sourceApis: [
					{
						sourceApi: GraphqlApi.fromGraphqlApiAttributes(this, 'source-api', {
							graphqlApiId: apiId,
							graphqlApiArn: apiArn,
						}),
						mergeType: MergeType.AUTO_MERGE,
					}
				]
			})
		})
	}
}

After cdk init app --language typescript replace the contents of ./lib/<name>-stack.ts with the above code. The compiler should report the error

TS2740: Type 'IGraphqlApi' is missing the following properties from type 'GraphqlApi': graphqlUrl, name, schema, modes, and 24 more.  
graphqlapi.d.ts(304, 14): The expected type comes from property 'sourceApi' which is declared here on type 'SourceApi'

Possible Solution

Change the type of SourceApi.sourceApi to IGraphqlApi

Additional Information/Context

No response

CDK CLI Version

2.94.0 (build 987c329)

Framework Version

2.95.1

Node.js Version

v18.7.0

OS

MacOs

Language

Typescript

Language Version

3.9.7

Other information

No response

@richardsonmarkj richardsonmarkj added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Sep 11, 2023
@github-actions github-actions bot added the @aws-cdk/aws-appsync Related to AWS AppSync label Sep 11, 2023
@peterwoodworth peterwoodworth added good first issue Related to contributions. See CONTRIBUTING.md p1 effort/small Small work item – less than a day of effort and removed bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Sep 11, 2023
@peterwoodworth
Copy link
Contributor

Yes, as far as I can tell this should be able to seamlessly accept an IGraphqlApi - as the only values that are read that I can find are accessible on IGraphqlApi. Thanks for the issue

private setupSourceApiAssociations() {
this.definition.sourceApiOptions?.sourceApis.forEach(sourceApiOption => {
new CfnSourceApiAssociation(this, `${sourceApiOption.sourceApi.node.id}Association`, {
sourceApiIdentifier: sourceApiOption.sourceApi.apiId,
mergedApiIdentifier: this.api.attrApiId,
sourceApiAssociationConfig: {
mergeType: sourceApiOption.mergeType ?? MergeType.AUTO_MERGE,
},
});
});
}
private setupMergedApiExecutionRole(sourceApiOptions: SourceApiOptions) {
if (sourceApiOptions.mergedApiExecutionRole) {
this.mergedApiExecutionRole = sourceApiOptions.mergedApiExecutionRole;
} else {
const sourceApiArns = sourceApiOptions.sourceApis?.map(sourceApiOption => {
return sourceApiOption.sourceApi.arn;
});
this.mergedApiExecutionRole = new Role(this, 'MergedApiExecutionRole', {
assumedBy: new ServicePrincipal('appsync.amazonaws.com'),
});
this.mergedApiExecutionRole.addToPolicy(new PolicyStatement({
resources: sourceApiArns,
actions: ['appsync:SourceGraphQL'],
}));

@ndejaco2
Copy link
Contributor

ndejaco2 commented Sep 13, 2023

The PR #27121 I have in progress should partly address this as it would allow you to create the SourceApiAssociation as a standalone construct for the cases when the source api or merged api is not in the same stack as long as you have the identifier (api id for same account / ARN for cross account use case )

@scanlonp
Copy link
Contributor

This was fixed by #27121. If this does not address your issue, ping me on this thread.

Copy link

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

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 effort/small Small work item – less than a day of effort good first issue Related to contributions. See CONTRIBUTING.md p1
Projects
None yet
Development

No branches or pull requests

4 participants