Skip to content

Commit

Permalink
Introduce tests: Add test for AWS JSON protocol conflicts
Browse files Browse the repository at this point in the history
This change is prompted by issues in other projects that _use_ the
content-api-firehose-client - in the `apple-news` project, updating another
library transitively updated the `aws-json-protocol` to a version that
was incompatible with the AWS SDK artifacts bought in by
content-api-firehose-client - leading to runtime errors:

guardian/apple-news#386 (comment)

Here we introduce an integration test that exercises the AWS SDK
`DescribeStreamSummary` call that failed - it checks that the response
can be successfully processed by the AWS SDK. Note that although we have
introduced this test, and it _passes_, this doesn't mean we've fixed the
issue with other projects yet - it just allows us to reproduce the issue,
by bumping the version on `aws-json-protocol` (ie by adding
`"software.amazon.awssdk" % "aws-json-protocol" % "2.29.23"` to the deps),
which we do outside of this branch.

In order to get the test running in CI, we have to allow the GitHub Actions
workflow to make the AWS SDK `DescribeStreamSummary` call, which requires
AWS credentials.

We need a IAM role that we can use in the GitHub Actions runners to
have access to the kinesis stream we’re describing - this is the same
approach taken in `facia-scala-client` with
guardian/facia-scala-client#272 .

This commit copies the example cdk stack from facia-scala-client,
updating the version of (gu)cdk used to the latest version, and tweaks
the role policies to match what we need to do.

This includes a fix from Akash to get Region and Account - they were `undefined`
before! As Akash says:

> scope here is of type App. The region and account properties of scope will not be meaningful here as we're not using CDK's Stage construct. That is, they'll return undefined.

#51 (comment)
  • Loading branch information
rtyley committed Dec 6, 2024
1 parent 2a36266 commit 585392d
Show file tree
Hide file tree
Showing 18 changed files with 8,845 additions and 17 deletions.
34 changes: 34 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
name: CI
on:
workflow_dispatch:
pull_request:

# triggering CI default branch improves caching
# see https://docs.github.com/en/free-pro-team@latest/actions/guides/caching-dependencies-to-speed-up-workflows#restrictions-for-accessing-a-cache
push:
branches:
- main

jobs:
test:
runs-on: ubuntu-latest
permissions:
id-token: write
contents: read
steps:
- uses: aws-actions/configure-aws-credentials@v4 # Needed for Kinesis read access for the tests!
with:
# The AWS role is configured as a GitHub Repo secret, the value is the cloudformation-output of the
# 'ContentApiFirehoseClientTesting' cloudformation stack.
role-to-assume: ${{ secrets.AWS_ROLE_FOR_TESTS }}
aws-region: eu-west-1
- name: Checkout
uses: actions/checkout@v4
- uses: guardian/setup-scala@v1
- name: Build and Test
run: sbt -v +test
- name: Test Summary
uses: test-summary/action@v2
with:
paths: "test-results/**/TEST-*.xml"
if: always()
7 changes: 5 additions & 2 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
.*
.gitignore.*
!.gitignore

target/

!.github
!.github

test-results/
.bsp/
9 changes: 5 additions & 4 deletions build.sbt
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@ Compile / doc / scalacOptions := Nil
releaseCrossBuild := true

enablePlugins(plugins.JUnitXmlReportPlugin)
Test / testOptions ++= Seq( Tests.Argument("-u", sys.env.getOrElse("SBT_JUNIT_OUTPUT","/tmp")) )
Test / testOptions +=
Tests.Argument(TestFrameworks.ScalaTest, "-u", s"test-results/scala-${scalaVersion.value}", "-o")

organization := "com.gu"
licenses := Seq(License.Apache2)
Expand All @@ -24,23 +25,23 @@ releaseProcess := Seq[ReleaseStep](
checkSnapshotDependencies,
inquireVersions,
runClean,
runTest,
setReleaseVersion,
commitReleaseVersion,
tagRelease,
setNextVersion,
commitNextVersion
)

resolvers += "Guardian GitHub Repository" at "https://guardian.github.io/maven/repo-releases"
resolvers ++= Resolver.sonatypeOssRepos("releases")

libraryDependencies ++= Seq(
"com.gu" %% "content-api-models-scala" % "25.0.0",
"com.gu" %% "thrift-serializer" % "5.0.5",
"software.amazon.kinesis" % "amazon-kinesis-client" % "2.6.0",
"com.typesafe.scala-logging" %% "scala-logging" % "3.9.5",
"com.twitter" %% "scrooge-core" % "21.12.0")
"com.twitter" %% "scrooge-core" % "21.12.0",
"org.scalatest" %% "scalatest" % "3.2.19" % Test
)

val jacksonVersion = "2.17.2"
dependencyOverrides ++= Seq(
Expand Down
11 changes: 11 additions & 0 deletions cdk/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
*.js
!jest.config.js
!jest.setup.js
!.eslintrc.js
*.d.ts
node_modules
dist

# CDK asset staging directory
.cdk.staging
cdk.out
5 changes: 5 additions & 0 deletions cdk/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
# Infrastructure

This directory defines the components to be deployed to AWS.

See [`package.json`](./package.json) for a list of available scripts.
6 changes: 6 additions & 0 deletions cdk/bin/cdk.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import "source-map-support/register";
import { GuRoot } from "@guardian/cdk/lib/constructs/root";
import { ContentApiFirehoseClientTesting } from "../lib/content-api-firehose-client-testing";

const app = new GuRoot();
new ContentApiFirehoseClientTesting(app, "ContentApiFirehoseClientTesting-euwest-1-INFRA", { stack: "content-api-firehose-client", stage: "INFRA", env: { region: "eu-west-1" } });
7 changes: 7 additions & 0 deletions cdk/cdk.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"app": "npx ts-node bin/cdk.ts",
"context": {
"aws-cdk:enableDiffNoFail": "true",
"@aws-cdk/core:stackRelativeExports": "true"
}
}
1 change: 1 addition & 0 deletions cdk/jest.setup.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
jest.mock("@guardian/cdk/lib/constants/tracking-tag");
112 changes: 112 additions & 0 deletions cdk/lib/__snapshots__/content-api-firehose-client-testing.test.ts.snap
Original file line number Diff line number Diff line change
@@ -0,0 +1,112 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`The ContentApiFirehoseClientTesting stack matches the snapshot 1`] = `
{
"Metadata": {
"gu:cdk:constructs": [
"GuAllowPolicy",
"GuGithubActionsRole",
],
"gu:cdk:version": "TEST",
},
"Outputs": {
"GithubActionsRoleGithubActionsRoleArnC13D9654": {
"Value": {
"Fn::GetAtt": [
"GithubActionsRoleF5CC769F",
"Arn",
],
},
},
},
"Resources": {
"GithubActionsRoleF5CC769F": {
"Properties": {
"AssumeRolePolicyDocument": {
"Statement": [
{
"Action": "sts:AssumeRoleWithWebIdentity",
"Condition": {
"StringLike": {
"token.actions.githubusercontent.com:sub": "repo:guardian/content-api-firehose-client:*",
},
},
"Effect": "Allow",
"Principal": {
"Federated": {
"Fn::Join": [
"",
[
"arn:aws:iam::",
{
"Ref": "AWS::AccountId",
},
":oidc-provider/token.actions.githubusercontent.com",
],
],
},
},
},
],
"Version": "2012-10-17",
},
"Tags": [
{
"Key": "gu:cdk:version",
"Value": "TEST",
},
{
"Key": "gu:repo",
"Value": "guardian/content-api-firehose-client",
},
{
"Key": "Stack",
"Value": "content-api-firehose-client",
},
{
"Key": "Stage",
"Value": "TEST",
},
],
},
"Type": "AWS::IAM::Role",
},
"kinesisstreamaccessFE21B9E2": {
"Properties": {
"PolicyDocument": {
"Statement": [
{
"Action": "kinesis:DescribeStreamSummary",
"Effect": "Allow",
"Resource": {
"Fn::Join": [
"",
[
"arn:aws:kinesis:",
{
"Ref": "AWS::Region",
},
":",
{
"Ref": "AWS::AccountId",
},
":stream/content-api-firehose-v2-CODE",
],
],
},
},
],
"Version": "2012-10-17",
},
"PolicyName": "kinesisstreamaccessFE21B9E2",
"Roles": [
{
"Ref": "GithubActionsRoleF5CC769F",
},
],
},
"Type": "AWS::IAM::Policy",
},
},
}
`;
12 changes: 12 additions & 0 deletions cdk/lib/content-api-firehose-client-testing.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import { App } from "aws-cdk-lib";
import { Template } from "aws-cdk-lib/assertions";
import { ContentApiFirehoseClientTesting } from "./content-api-firehose-client-testing";

describe("The ContentApiFirehoseClientTesting stack", () => {
it("matches the snapshot", () => {
const app = new App();
const stack = new ContentApiFirehoseClientTesting(app, "ContentApiFirehoseClientTesting", { stack: "content-api-firehose-client", stage: "TEST" });
const template = Template.fromStack(stack);
expect(template.toJSON()).toMatchSnapshot();
});
});
25 changes: 25 additions & 0 deletions cdk/lib/content-api-firehose-client-testing.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
import type { GuStackProps } from '@guardian/cdk/lib/constructs/core';
import { GuStack } from '@guardian/cdk/lib/constructs/core';
import { GuGithubActionsRole } from '@guardian/cdk/lib/constructs/iam';
import { GuAllowPolicy } from '@guardian/cdk/lib/constructs/iam/policies/base-policy';
import type { App } from 'aws-cdk-lib';

export class ContentApiFirehoseClientTesting extends GuStack {
constructor(scope: App, id: string, props: GuStackProps) {
super(scope, id, props);
new GuGithubActionsRole(this, {
policies: [
new GuAllowPolicy(this, 'kinesis-stream-access', {
actions: ['kinesis:DescribeStreamSummary'],
resources: [
`arn:aws:kinesis:${this.region}:${this.account}:stream/content-api-firehose-v2-CODE`,
],
}),
],
condition: {
githubOrganisation: 'guardian',
repositories: 'content-api-firehose-client:*',
},
});
}
}
Loading

0 comments on commit 585392d

Please sign in to comment.