Conversation
cadivus
left a comment
There was a problem hiding this comment.
This should be integrated into infra_gen2.
| name: Kinesis E2E Tests | ||
| on: | ||
| pull_request: | ||
| paths: | ||
| - "packages/kinesis/**" | ||
| - ".github/workflows/kinesis_e2e.yaml" | ||
| workflow_dispatch: |
There was a problem hiding this comment.
How does this differ so much from all the other e2e tests that we need to implement everything custom?
There was a problem hiding this comment.
this custom code is outdated sorry, but still kinesis E2E is fundamentally different, it needs OIDC + Secrets Manager for credentials rather than fetch_backends + amplify pull
There was a problem hiding this comment.
it needs OIDC + Secrets Manager for credentials rather than fetch_backends + amplify pull
But fetch_backends already uses OIDC + Secrets Manager. The only thing it does on top is run tool/pull_test_backend.sh to download amplify_outputs.dart from S3.
For the credentials, the Android PR shows the better approach:
Include auth as a category and grant Kinesis permissions to the authenticated IAM role:
// Android: aws-kinesis/infra/amplify/backend.ts
const backend = defineBackend({ auth });
backend.auth.resources.authenticatedUserIamRole.addToPrincipalPolicy(
new PolicyStatement({
actions: ['kinesis:PutRecord', 'kinesis:PutRecords', 'kinesis:DescribeStream'],
resources: [stream.streamArn],
})
);(link)
This is exactly the same pattern the Flutter repo already uses for Analytics/Pinpoint, which also isn't a "native" Amplify category but piggybacks on auth + custom CDK resources + backend.addOutput().
Also, using some tokens in the app is not a real e2e test. This is not what customers most likely do. Using Cognito instead of some "injected" credentials is way closer to what a customer would to.
And this way, we need the amplify_outputs.dart.
There was a problem hiding this comment.
I would say that these dart only next version packages could be decoupled from the cognito pattern, but for consistency I have updated them to match the other platforms and our own packages
| - name: Install native sqlite3 | ||
| run: sudo apt-get update && sudo apt-get install -y libsqlite3-dev |
There was a problem hiding this comment.
Why is this needed? For building on Android?
.github/workflows/kinesis_e2e.yaml
Outdated
| smithy_aws: | ||
| path: ../../smithy/smithy_aws | ||
| EOF | ||
|
|
There was a problem hiding this comment.
Why don't we use the aft bootstrapping? Why this custom solution?
infra/lib/stack.ts
Outdated
| AuthIntegrationTestStack, | ||
| AuthIntegrationTestStackEnvironmentProps, |
There was a problem hiding this comment.
Why these infra changes? Isn't infra unrelated to this PR?
| new secretsmanager.Secret(kinesisStack, "KinesisE2ESecret", { | ||
| secretName: "kinesis-e2e", | ||
| description: "Kinesis E2E test credentials and resource names", | ||
| secretObjectValue: { | ||
| ACCESS_KEY_ID: cdk.SecretValue.unsafePlainText(accessKey.accessKeyId), | ||
| SECRET_ACCESS_KEY: accessKey.secretAccessKey, | ||
| STREAM_NAME: cdk.SecretValue.unsafePlainText(stream.streamName), | ||
| DELIVERY_STREAM_NAME: cdk.SecretValue.unsafePlainText( | ||
| deliveryStream.deliveryStreamName!, | ||
| ), | ||
| }, | ||
| }); |
There was a problem hiding this comment.
What are these secrets needed for? If they are for fetching the e2e backends, I don't think this is the right place.
There was a problem hiding this comment.
not for fetching but for creating
There was a problem hiding this comment.
but I am pushing out the updated branch, this whole commit was outdated and mistakenly pushed
000928f to
9b582eb
Compare
0a451d4 to
a04df03
Compare
1f9c2b1 to
93c5331
Compare
Adds the foundational package with minimal constructs needed for the Kinesis client libraries: - AmplifyException base class - AWSCredentialsProvider<T> and AWSCredentials sealed hierarchy - Logger interface with AmplifyLogger implementation - LogLevel enum - Result<T, E> sealed type
Provides V2CredentialsProviderBridge to bridge aws_common (V2) credentials to amplify_foundation_dart (V3) credentials. Shared by kinesis client packages to avoid duplicating bridge logic.
…plify backend Provisions Kinesis Data Stream, Firehose delivery stream, IAM user, and Secrets Manager secret via an Amplify Gen 2 backend using backend.createStack() to access CDK directly — no auth or other Amplify categories needed. Removes the standalone KinesisTestStack and KinesisTestResources construct from infra/, along with references in the main integ stack and GitHub stack.
- Revert unrelated whitespace changes to infra/lib/stack.ts and infra/lib/github/github.ts - Replace manual pubspec_overrides.yaml with aft bootstrap - Update workflow to match repo conventions (action versions, submodule init, log_cw_metric_wrapper, timeout, defaults) - Remove IAM user, access key, and Secrets Manager secret from backend.ts (credentials managed outside stack)
…lid category list in log_cw_metric so that\nthe kinesis package working directory is recognized by the CI\nmetric logging step."
…e same pattern as Analytics/Pinpoint and the Android KDS\nlibrary: include auth (Cognito) in the backend and grant the\nauthenticated IAM role permissions to Kinesis/Firehose resources.\n\nThis replaces the custom Secrets Manager credential injection with\nthe standard fetch_backends workflow pattern, making E2E tests use\nCognito-authenticated credentials like real customers would.\n\n- Add auth resource with pre-sign-up auto-confirm trigger\n- Grant authenticatedUserIamRole Kinesis and Firehose permissions\n- Rewrite E2E workflow to use fetch_backends composite action\n- Remove custom KINESIS_E2E secret fetching from workflow"
a04df03 to
18f1e9f
Compare
…ory.kinesis so deploy_gen2.dart can deploy the kinesis\nbackend and upload amplify_outputs.dart to S3. The kinesis backend\nshares its config with both aws_kinesis_datastreams and\naws_amazon_firehose packages via sharedOutputs."
| enforceSSL: true, | ||
| }); | ||
|
|
||
| // --- IAM role for Firehose → S3 --- |
There was a problem hiding this comment.
I think this is unrelated to Kinesis. Can we handle this separately and remove from this PR?
Issue #, if available:
Description of changes:
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.