new lambda to get stripe setup intent ids for the client side #2111

johnduffell · 2019-10-04T10:20:04Z

This PR is open but I need to self review it before it really makes sense. Feel free to review now or wait until it's tidied up and I've added my own comments.

Why are you doing this?

This is part of the programme to get stripe SCA support up and running.
This is step 1 from https://stripe.com/docs/payments/cards/saving-cards-after-payment

This is implemented as a lambda as serverless is a good thing for us to move towards where sensible, and this functionality has nothing to do with support frontend play app, and doesn't need access to most of the secrets and libraries that support-frontend has.
This is created as an independent lambda from the support-workers jar, as support workers already has a lot of dependencies and the startup time is 7+ seconds on a 51MB jar. This jar comes out at 40+M which is mostly down to the aws s3 sdk library, but there is a lot of huge DLLs for gitapi for some bizarre reason https://github.com/luben/zstd-jni
Keeping things in a big mono-jar is not really a sustainable way to create lambdas for other uses so I have teased apart those dependencies that we really need and tried to structure things so that the dependencies are more reasonable. Hopefully this would be a more sustainable way if we want to make more use of lambdas in future.
This has required a bit of restructuring but hopefully I've got the essential without going overboard.

TODO

test on code - DONE
add unit tests
upload stripe live key mappings - DONE
consider adding a health check or similar
alarms needed - DONE

Trello Card

johnduffell · 2019-10-04T16:13:20Z

build.sbt

@@ -49,7 +49,9 @@ lazy val commonSettings = Seq(
    "scm:git:[email protected]:guardian/support-frontend.git"
  )),
  // https://www.scala-sbt.org/1.x/docs/Cached-Resolution.html
-  updateOptions := updateOptions.value.withCachedResolution(true)
+  updateOptions := updateOptions.value.withCachedResolution(true),
+  dependencyStats / aggregate := false,


this command is useful to see what the jar size will be.

johnduffell · 2019-10-07T09:36:50Z

project/LibraryVersions.scala

@@ -1,7 +1,7 @@

 object LibraryVersions {
  val circeVersion = "0.11.1"
-  val awsClientVersion = "1.11.226"
+  val awsClientVersion = "1.11.642"


just bumping an old version

johnduffell · 2019-10-07T09:44:44Z

project/plugins.sbt

@@ -25,7 +25,7 @@ addSbtPlugin("com.timushev.sbt" % "sbt-updates" % "0.3.3")
 addSbtPlugin("net.vonbuchholtz" % "sbt-dependency-check" % "0.2.0")

 // dependency tracker plugin - needed for snyk cli integration
-addSbtPlugin("net.virtual-void" % "sbt-dependency-graph" % "0.9.0")
+addSbtPlugin("net.virtual-void" % "sbt-dependency-graph" % "0.10.0-RC1")


we are on sbt1.3.2 now but there are bugs with its interaction with this plugin. this version of sbt dependency graph will appear to work rather than crash sbt/sbt-dependency-graph#178 (comment)
however it doesn't always produce the right result coursier/coursier#1375
so we will need another version bump at some point, but this is just a small helper tool rather than something from the build itself.
Another option is just to remove it, and people can add it to their local ~/.sbt, plugins.sbt file if they like to use it.

johnduffell · 2019-10-07T09:48:26Z

support-lambdas/stripe-intent/README.md

+It is a separate jar from the support-workers jar because the unused
+code would add to be loaded on every single step of the functions.
+Hopefully we can be a bit more cohesive and minimise dependencies if
+we separate things better.


this turned out to be harder than expected, for example because pulling in support-models pulls in acquisition-event-producer which pulls in various thrift stuff including a 3.6mb binaries of compression algorithm used by capi. I think sometimes pulling in a "com.gu" library can get more than we bargained for as we are not always disciplined at not adding dependencies that not everyone would need. Client side devs have some nice tooling so every import shows the size in KB of the file in the IDE, but server side we have no idea.

johnduffell · 2019-10-07T11:14:22Z

support-lambdas/stripe-intent/cfn.yaml

+      DomainName: stripe-intent-code.support.guardianapis.com
+      StripeConfig: test
+    PROD:
+      DomainName: stripe-intent.support.guardianapis.com


we (@tomrf1 @JoemG ) had a very brief discussion about appropriate host names for prod/code as they are making a similar style lambda. We have the existing code.support.dev-guardianapis.com convention but no certificate so this one is a bit easier. But now would be a good time for objections. These conventions are also present in support-service-lambdas repo.

johnduffell · 2019-10-07T11:28:53Z

support-lambdas/stripe-intent/cfn.yaml

+      RestApiId: !Ref ServerlessRestApi # auto generated by the Transform
+      DomainName: !Ref DomainName
+      Stage: !Sub Prod
+    DependsOn: ServerlessRestApiProdStage # auto generated by the Transform


this is a hack based on the predicted names of the low level resources. AWS are working on proper support for custom domain names, at which point we can feel free to switch over to that aws/serverless-application-model#783

support-lambdas/stripe-intent/src/main/scala/com/gu/handler/impure/S3Loader.scala

tomrf1 · 2019-10-07T13:28:26Z

support-lambdas/stripe-intent/src/main/scala/com/gu/stripeIntent/Handler.scala

+
+
+
+object ManualTest {


should this be somewhere else?

probably shouldn't exist at all, I think maybe an it:test would make more sense, as then we can check assertions.

support-lambdas/stripe-intent/src/main/scala/com/gu/stripeIntent/StripeService.scala

support-workers/src/main/scala/com/gu/stripe/StripeService.scala

…find existing HTTP lib to use

tomrf1 · 2019-10-23T10:46:00Z

support-frontend/assets/helpers/tracking/acquisitions.js

@@ -44,7 +45,7 @@ export type ReferrerAcquisitionData = {|
  abTests: ?AcquisitionABTest[],
  // these aren't in the referrer acquisition data model on frontend, but they're convenient to include
  // as we want to include query parameters in the acquisition event to e.g. facilitate off-platform tracking
-  queryParameters: ?AcquisitionQueryParameters,
+  queryParameters: AcquisitionQueryParameters,


not sure how this change relates to this PR?

…into stripe-intent-lambda

…pport-frontend into stripe-intent-lambda

prout-bot · 2019-10-24T16:15:42Z

Seen on PROD (created by @johnduffell and merged by @alexflav23 10 minutes and 11 seconds ago)

Keep an eye out for post deployment test results on Travis

Sentry Release: support-client-side, support

johnduffell added 9 commits October 4, 2019 11:05

new lambda to get stripe setup intent ids for the client side

e6b51f2

newer AWS library

319ab98

correct sbt name for support-rest subproject

0bbde3f

aggregate lambda into support-workers build

5e81101

use correct build number in riff raff upload path

aafc256

return client secret rather than intent id

66bdf6a

stripe intent lambda is support not membership

15a54fc

make stripe-intent base path mapping depend on the stage

a5074a4

fix json for error response

5cdf424

johnduffell commented Oct 4, 2019

View reviewed changes

johnduffell commented Oct 7, 2019

View reviewed changes

change hostname of prod stripe intent lambda

81b7f1e

johnduffell commented Oct 7, 2019

View reviewed changes

add simple end to end test for stripe intent logic

fb615db

tomrf1 reviewed Oct 7, 2019

View reviewed changes

support-lambdas/stripe-intent/src/main/scala/com/gu/handler/impure/S3Loader.scala Show resolved Hide resolved

tomrf1 reviewed Oct 7, 2019

View reviewed changes

support-lambdas/stripe-intent/src/main/scala/com/gu/stripeIntent/StripeService.scala Outdated Show resolved Hide resolved

tomrf1 reviewed Oct 7, 2019

View reviewed changes

support-workers/src/main/scala/com/gu/stripe/StripeService.scala Outdated Show resolved Hide resolved

johnduffell and others added 9 commits October 7, 2019 16:16

get rid of duplicate stripe error object

2201930

cors

0288a48

cors

12234cd

wildcard for CODE

4642280

AllowHeaders

500006e

pls work

7f236a8

Add cors headers with wildcards for now

918830d

do not use wildcard in PROD

706f14d

fix test

3d1b5c6

alexflav23 added 5 commits October 22, 2019 15:30

Merging all latest changes

bd6f2ca

Fixing import error

64179e0

Adding logging

bba62ac

Remove ADT For HTTP response codes, dangerous duplication. We should …

08b8228

…find existing HTTP lib to use

Fixing the final test for HandlerSpec

9c3e3ac

tomrf1 reviewed Oct 23, 2019

View reviewed changes

alexflav23 added 4 commits October 23, 2019 14:56

Remove loginfo to pass yarn validate

725b4b3

Merge branch 'master' of https://github.com/guardian/support-frontend …

eba1fab

…into stripe-intent-lambda

Trying to fix issues

116ff99

Fixing remote merge conflicts

b6e7803

alexflav23 approved these changes Oct 23, 2019

View reviewed changes

alexflav23 and others added 10 commits October 23, 2019 17:21

Merge branch 'master' of https://github.com/guardian/support-frontend …

2755b6b

…into stripe-intent-lambda

Merge branch 'master' of https://github.com/guardian/support-frontend …

1f0da69

…into stripe-intent-lambda

Merge branch 'master' of https://github.com/guardian/support-frontend …

6f0265a

…into stripe-intent-lambda

alarms, api gateway stage, description

f57a453

Merge branch 'master' of https://github.com/guardian/support-frontend …

79b4c6d

…into stripe-intent-lambda

Adding stripe setup intent

7bac40e

Merge branch 'stripe-intent-lambda' of https://github.com/guardian/su…

8332d74

…pport-frontend into stripe-intent-lambda

Undo change to query params

259dd0d

Adding setup intent

671c724

set Stage env var

35f0a1f

alexflav23 force-pushed the stripe-intent-lambda branch from c309706 to 35f0a1f Compare October 24, 2019 15:38

tomrf1 approved these changes Oct 24, 2019

View reviewed changes

Fixing compilation

7c60c25

alexflav23 merged commit f944439 into master Oct 24, 2019

alexflav23 deleted the stripe-intent-lambda branch October 24, 2019 16:05

prout-bot added the Pending-on-PROD label Oct 24, 2019

prout-bot added Seen-on-PROD and removed Pending-on-PROD labels Oct 24, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

new lambda to get stripe setup intent ids for the client side #2111

new lambda to get stripe setup intent ids for the client side #2111

johnduffell commented Oct 4, 2019 •

edited by alexflav23

Loading

johnduffell Oct 4, 2019

johnduffell Oct 7, 2019

johnduffell Oct 7, 2019

johnduffell Oct 7, 2019

johnduffell Oct 7, 2019

johnduffell Oct 7, 2019

tomrf1 Oct 7, 2019

johnduffell Oct 7, 2019

tomrf1 Oct 23, 2019

prout-bot commented Oct 24, 2019

new lambda to get stripe setup intent ids for the client side #2111

new lambda to get stripe setup intent ids for the client side #2111

Conversation

johnduffell commented Oct 4, 2019 • edited by alexflav23 Loading

Why are you doing this?

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

prout-bot commented Oct 24, 2019

Sentry Release: support-client-side, support

johnduffell commented Oct 4, 2019 •

edited by alexflav23

Loading