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): rdsDataSource expects databaseCluster instead of serverlessCluster #12567

Closed
monholm opened this issue Jan 18, 2021 · 11 comments · Fixed by #13206
Closed

(aws-appsync): rdsDataSource expects databaseCluster instead of serverlessCluster #12567

monholm opened this issue Jan 18, 2021 · 11 comments · Fixed by #13206
Assignees
Labels
@aws-cdk/aws-appsync Related to AWS AppSync bug This issue is a bug. effort/small Small work item – less than a day of effort p2

Comments

@monholm
Copy link
Contributor

monholm commented Jan 18, 2021

Creating seperate issues for #12503 to improve strucure.

When trying to add a rdsDataSource to appsync, addRdsDataSource and RdsDataSource expects a DatabaseCluster instead of a ServerlessCluster, which is the one used by AppSync.

Reproduction Steps

const api = new GraphqlApi(stack, 'api', {
  name: 'graphqlApi',
  schema: Schema.fromAsset('./testSchema.graphql'),
});

const serverless = new ServerlessCluster(stack, 'cluster', {
  engine: DatabaseClusterEngine.AURORA_MYSQL,
  vpc: new Vpc(stack, 'vpc'),
});

api.addRdsDataSource('rdsDS', serverless, serverless.secret!);

What did you expect to happen?

addRdsDataSource to expect ServerlessCluster.

What actually happened?

It expects a DatabaseCluster.

Environment

  • CDK CLI Version: 1.84.0
  • Framework Version: 1.84.0
  • Node.js Version: 14.5.4
  • OS: MacOS 10.15.7
  • Language (Version): TypeScript (3.9.7)

This is 🐛 Bug Report

@monholm monholm added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Jan 18, 2021
@github-actions github-actions bot added the @aws-cdk/aws-appsync Related to AWS AppSync label Jan 18, 2021
@BerndWessels
Copy link

@Simon-TechForm I've just stumbled on exactly the same issue - any ideas when your fix will be merged?

@monholm
Copy link
Contributor Author

monholm commented Jan 27, 2021

@Simon-TechForm I've just stumbled on exactly the same issue - any ideas when your fix will be merged?

Unfortunately not. It seems like @MrArnoldPalmer is being assigned to every issue/pull request regarding the aws-appsync module, but isn't being particularly active on the repo.

With regard to the current state of the library, we are having a hard time figuring out how a rds data source would ever work out with a "normal" DatabaseCluster in place of a ServerlessCluster, and thus we are having a hard time figuring out why a DatabaseCluster was used in the first place. (This is why I asked for clearence regarding whether or not I missed something obvious in my other issue #12503)

We made a temporary workaround by creating our own custom class extending the "BackedDataSource" class. Let me know if you would like/need any guidance regarding this, as I would be happy to help.

@BerndWessels
Copy link

Hi @Simon-TechForm

I'm currently focused on the ServerlessCluster anyways - so therefore based on your PR I just use as to cast it - which seems to be working for me. (even though I prefer your PR and not having to cast things dangerously)

import * as cdk from "@aws-cdk/core";
import * as appsync from "@aws-cdk/aws-appsync";
import * as cognito from "@aws-cdk/aws-cognito";
import * as rds from "@aws-cdk/aws-rds";
import * as secretsManager from "@aws-cdk/aws-secretsmanager";
/**
 * Stack Properties
 */
export interface ApiStackProps extends cdk.StackProps {
  readonly databaseCluster: rds.ServerlessCluster;
  readonly userPool: cognito.UserPool;
}

/**
 * Stack
 */
export class ApiStack extends cdk.Stack {
  public readonly api: appsync.GraphqlApi;

  constructor(scope: cdk.Construct, id: string, props: ApiStackProps) {
    super(scope, id, props);

    /**
     * Defaults
     */
    const {
      databaseCluster,
      userPool,
    } = props;

    /**
     * GraphQL Api
     */
    const api = new appsync.GraphqlApi(this, "Api", {
      authorizationConfig: {
        defaultAuthorization: {
          authorizationType: appsync.AuthorizationType.USER_POOL,
          userPoolConfig: {
            userPool: userPool,
          },
        },
      },
      name: "my-product-appsync-api",
      schema: appsync.Schema.fromAsset("../graphql/schema.graphql"),
    });

    api.addRdsDataSource(
      "DatabaseDataSource",
      (databaseCluster as unknown) as rds.IDatabaseCluster,
      databaseCluster.secret as secretsManager.ISecret,
      {
        description: "Serverless Aurora RDS Database Data Source",
        name: "DatabaseDataSource",
      }
    );
    
    this.api = api;
  }
}

Is there a way to engage more actively with certain CDK areas of interest, or is it all up to AWS when and what gets merged?

@monholm
Copy link
Contributor Author

monholm commented Jan 27, 2021

Hi @Simon-TechForm

I'm currently focused on the ServerlessCluster anyways - so therefore based on your PR I just use as to cast it - which seems to be working for me. (even though I prefer your PR and not having to cast things dangerously)

import * as cdk from "@aws-cdk/core";
import * as appsync from "@aws-cdk/aws-appsync";
import * as cognito from "@aws-cdk/aws-cognito";
import * as rds from "@aws-cdk/aws-rds";
import * as secretsManager from "@aws-cdk/aws-secretsmanager";
/**
 * Stack Properties
 */
export interface ApiStackProps extends cdk.StackProps {
  readonly databaseCluster: rds.ServerlessCluster;
  readonly userPool: cognito.UserPool;
}

/**
 * Stack
 */
export class ApiStack extends cdk.Stack {
  public readonly api: appsync.GraphqlApi;

  constructor(scope: cdk.Construct, id: string, props: ApiStackProps) {
    super(scope, id, props);

    /**
     * Defaults
     */
    const {
      databaseCluster,
      userPool,
    } = props;

    /**
     * GraphQL Api
     */
    const api = new appsync.GraphqlApi(this, "Api", {
      authorizationConfig: {
        defaultAuthorization: {
          authorizationType: appsync.AuthorizationType.USER_POOL,
          userPoolConfig: {
            userPool: userPool,
          },
        },
      },
      name: "my-product-appsync-api",
      schema: appsync.Schema.fromAsset("../graphql/schema.graphql"),
    });

    api.addRdsDataSource(
      "DatabaseDataSource",
      (databaseCluster as unknown) as rds.IDatabaseCluster,
      databaseCluster.secret as secretsManager.ISecret,
      {
        description: "Serverless Aurora RDS Database Data Source",
        name: "DatabaseDataSource",
      }
    );
    
    this.api = api;
  }
}

Is there a way to engage more actively with certain CDK areas of interest, or is it all up to AWS when and what gets merged?

It seems like it, but I don't really know.

@BerndWessels
Copy link

@Simon-TechForm hi, I'm getting desperate getting

"error": {
            "message": "RDSHttp:{\"message\":\"This API is deprecated and not available. Use ExecuteStatement API instead\"}",
            "type": "400 Bad Request"
        },

even just following the most basic aws tutorials manually.

Have you come across this problem?

#12735

@monholm
Copy link
Contributor Author

monholm commented Jan 28, 2021

@Simon-TechForm hi, I'm getting desperate getting

"error": {
            "message": "RDSHttp:{\"message\":\"This API is deprecated and not available. Use ExecuteStatement API instead\"}",
            "type": "400 Bad Request"
        },

even just following the most basic aws tutorials manually.

Have you come across this problem?

#12735

No, not as far as I remember. My colleague also spent quite a bit of time getting this working, so I will ask him tomorrow if he encountered that error.

I will get back to you.

@monholm
Copy link
Contributor Author

monholm commented Jan 29, 2021

@BerndWessels I'm afraid I can't help you. My colleague didn't encounter that error.

@monholm
Copy link
Contributor Author

monholm commented Feb 14, 2021

Shamelessly going to mention a few people being the last ones active on this repo: @skinny85 @njlynch @rix0rrr

Could someone please shed some light on what to expect about appsync within this project?

I'm aware that the cdk contructs of appsync are marked as experimental, and as such might not be highly prioritized, but it has now been almost a month since we opened this issue (and #12572) with corresponding pr's, and we haven't heard a word from aws.

We are happily using aws-cdk and deemed it the favorable option for coding backend within aws, but we use appsync in a lot of our projects and therefore need to know if it has been left for dead in aws-cdk. Looking at the issues, there have been almost zero activity in 2021 regarding appsync.

@MrArnoldPalmer
Copy link
Contributor

hey @Simon-TechForm, sorry for the delay here. Development on the AppSync constructs isn't dead, but its not currently prioritized by the core team. I greatly appreciate your contributions and am trying to prioritize reviews/feedback in the future, so keep em coming.

Adding some questions to your PR for this one as well.

@monholm
Copy link
Contributor Author

monholm commented Feb 19, 2021

Hello @MrArnoldPalmer

I appreciate you taking the time to get back to me on this. I will hit you up in the pr (and maybe #12575 as well? ;) )

Have a nice weekend.

@MrArnoldPalmer MrArnoldPalmer added effort/small Small work item – less than a day of effort p2 and removed needs-triage This issue or PR still needs to be triaged. labels Feb 22, 2021
monholm added a commit to monholm/aws-cdk that referenced this issue Feb 22, 2021
Resolves: aws#12567

BREAKING CHANGE: RdsDataSource now takes a ServerlessCluster instead of a DatabaseCluster
@mergify mergify bot closed this as completed in #13206 Feb 22, 2021
mergify bot pushed a commit that referenced this issue Feb 22, 2021
Resolves: #12567

BREAKING CHANGE: RdsDataSource now takes a ServerlessCluster instead of a DatabaseCluster


----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@github-actions
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 bug This issue is a bug. effort/small Small work item – less than a day of effort p2
Projects
None yet
3 participants