Add a Stripe settings secret to server #10308

jankeromnes · 2022-05-27T16:19:03Z

Description

Add a Stripe settings secret to server, in order to provide the secret key for API calls.

Related Issue(s)

Fixes #

How to test

Builds without payment should succeed (and not have a Stripe volume mounted on server pod)
Build with payment should succeed, and have the Stripe volume mounted:

$ kubectl describe pod server-5995b76459-llfwg
[...]
Volumes:
[...]
  stripe-config:
    Type:        Secret (a volume populated by a Secret)
    SecretName:  stripe-config
    Optional:    false

Release Notes

NONE

Documentation

/werft with-clean-slate-deployment
/werft with-payment=true

jankeromnes · 2022-05-30T09:40:45Z

Whoops, server is in CrashLoopBackoff due to:

Error: ENOENT: no such file or directory, open '/stripe/settings'

🤔

jankeromnes · 2022-05-30T11:11:27Z

Aha, maybe it's because payment is disabled? Let's try:

/werft run with-payment=true

👍 started the job as gitpod-build-jx-stripe-secret.4
(with .werft/ from main)

EDIT: Nope, that wasn't it -- still crash-looping.

geropl · 2022-05-30T12:38:53Z

@jankeromnes Does not have the volume mounted:

Possible reasons:

in the werft scripts, are we using the correct version of the installer (e.g., the one from this branch build?)
somehow the modification did not pan out (debug: print config.yaml - would be nice to have in werft in general!)
🤷

jankeromnes · 2022-05-30T12:40:25Z

Aha, good point! 🎯 Many thanks @geropl. Will re-trigger Werft with the config from my branch.

EDIT: Re-triggered like so:

$ kubectx dev
Switched to context "dev".

$ werft job run github gitpod-io/gitpod:jx/stripe-secret -j .werft/build.yaml
gitpod-custom-jx-stripe-secret.0

https://werft.gitpod-dev.com/job/gitpod-custom-jx-stripe-secret.0

jankeromnes · 2022-05-30T13:36:16Z

Stripe secret successfully mounted! 🎉

$ kubectl describe pod server-5995b76459-llfwg
[...]
Volumes:
[...]
  stripe-config:
    Type:        Secret (a volume populated by a Secret)
    SecretName:  stripe-config
    Optional:    false

(Also, Chargebee still works. ✅)

Now testing a build without payment...

jankeromnes · 2022-05-30T13:57:15Z

Without payment, server crash-loops with:

Unable to load ChargebeeProviderOptions from: /chargebee/providerOptions

and

Error: ENOENT: no such file or directory, open '/stripe/settings'

EDIT: I see that the Chargebee error is just a warning, while the Stripe error is the one that makes server crashloop.

However, when payment is disabled (for example in Self-Hosted), I think it would be much nicer to not even try to load Chargebee or Stripe secrets, as opposed to trying + failing + logging (potentially confusing) warnings about payment-related stuff.

Attempting to fix this like so:

jankeromnes · 2022-05-30T15:12:53Z

Alright, seems to work fine now! 🎉

Without payment

https://werft.gitpod-dev.com/job/gitpod-build-jx-stripe-secret.6 Build succeeded ✅

Logging in works ✅ / No payment UI ✅	Volume is not mounted ✅

With payment

https://werft.gitpod-dev.com/job/gitpod-custom-jx-stripe-secret.3 Build succeeded ✅

Logging in works ✅ / Payment UI is there ✅	Volume is mounted ✅

.werft/jobs/build/payment/stripe-config-secret.yaml

easyCZ · 2022-05-31T07:32:10Z

install/installer/pkg/components/server/deployment.go

+					Name: "stripe-config",
+					VolumeSource: corev1.VolumeSource{
+						Secret: &corev1.SecretVolumeSource{
+							SecretName: stripeSecret,
+						},
+					},
+				})
+
+			volumeMounts = append(volumeMounts, corev1.VolumeMount{
+				Name:      "stripe-config",


Suggested change

Name: "stripe-config",

VolumeSource: corev1.VolumeSource{

Secret: &corev1.SecretVolumeSource{

SecretName: stripeSecret,

},

},

})

volumeMounts = append(volumeMounts, corev1.VolumeMount{

Name: "stripe-config",

stripeConfigVolumeName = "stripe-config"

// ...

Name: stripeConfigVolumeName,

VolumeSource: corev1.VolumeSource{

Secret: &corev1.SecretVolumeSource{

SecretName: stripeSecret,

},

},

})

volumeMounts = append(volumeMounts, corev1.VolumeMount{

Name: stripeConfigVolumeName,

Because they need to match, they should reference the same variable.

easyCZ · 2022-05-31T07:33:19Z

install/installer/pkg/components/server/configmap.go

@@ -208,8 +216,9 @@ func configmap(ctx *common.RenderContext) ([]runtime.Object, error) {
 		ImageBuilderAddr:             "image-builder-mk3:8080",
 		CodeSync:                     CodeSync{},
 		VSXRegistryUrl:               fmt.Sprintf("https://open-vsx.%s", ctx.Config.Domain), // todo(sje): or "https://{{ .Values.vsxRegistry.host | default "open-vsx.org" }}" if not using OpenVSX proxy
-		EnablePayment:                chargebeeSecret != "",
+		EnablePayment:                chargebeeSecret != "" || stripeSecret != "",


Why put this behind payment? Can we not enable this by default? I'm assuming the secret we have would only go into a test mode of stripe anyway so there's no risk enabling. This would also help remove yet another config option.

I don't think we want to enable anything Stripe-related in Self-Hosted, where payment is disabled.

That makes sense. However, currently the stripe config is in experimental mode and only configured for preview. That alone should be enough for it to not be deployed in self-hosted. Am I missing something?

This is actually the other way around, right? I.e. we set EnablePayment to true if either the chargebeeSecret or the stripeSecret is set. 💭

Okay, that makes more sense. Would it make sense to separate them then? We may want to keep Chargbee enabled, but disable Stripe.

I think the code makes sense as-is. "enablePayment" is a legacy field that we should aim to remove when we kill the chargbee integration. Until we're there, and especially in the context of this PR, it make sense to keep it.

Might make sense to try and remove it from the config surface altogether (and only keep it internall where it make sense) in a separate PR.

geropl · 2022-05-31T08:05:06Z

However, when payment is disabled (for example in Self-Hosted), I think it would be much nicer to not even try to load Chargebee or Stripe secrets, as opposed to trying + failing + logging (potentially confusing) warnings about payment-related stuff.

@jankeromnes in Self-Hosted neither stripe nor chargebee files are configured, so this is a non-issue. But am ok with the if (payment && stripeFile) pattern. 👍

corneliusludmann

Approving changes in self-hosted owned file install/installer/pkg/config/v1/experimental/experimental.go to unblock this pull request. 🛹

geropl

👍

jankeromnes · 2022-05-31T09:11:32Z

Many thanks @geropl @easyCZ and @corneliusludmann for the quick & very helpful reviews! 💯

@jankeromnes in Self-Hosted neither stripe nor chargebee files are configured, so this is a non-issue. But am ok with the if (payment && stripeFile) pattern. 👍

FYI I added the payment check because in this Pull Request, when I disabled payment, both Chargebee and Stripe sections failed because of not being configured (Chargebee logged a warning, but Stripe caused a crash).

I assumed this would be the same behavior in Self-Hosted -- if I'm right, when you install Self-Hosted, you'll get a warning about Chargebee not being configured. (Not really a big issue though.)

roboquat added do-not-merge/work-in-progress release-note-none size/M labels May 27, 2022

jankeromnes requested a review from geropl May 30, 2022 08:11

jankeromnes force-pushed the jx/stripe-secret branch from a914b16 to 85f8bb9 Compare May 30, 2022 09:00

jankeromnes force-pushed the jx/stripe-secret branch 2 times, most recently from 3008967 to ce76080 Compare May 30, 2022 10:07

jankeromnes force-pushed the jx/stripe-secret branch from ce76080 to ea4d03e Compare May 30, 2022 11:31

[server][dashboard] Add a Stripe settings secret

9b2df29

jankeromnes force-pushed the jx/stripe-secret branch from ea4d03e to 9b2df29 Compare May 30, 2022 14:25

jankeromnes marked this pull request as ready for review May 30, 2022 15:12

jankeromnes requested review from a team May 30, 2022 15:12

roboquat removed the do-not-merge/work-in-progress label May 30, 2022

github-actions bot added team: delivery Issue belongs to the self-hosted team team: webapp Issue belongs to the WebApp team labels May 30, 2022

jankeromnes assigned geropl May 31, 2022

easyCZ reviewed May 31, 2022

View reviewed changes

.werft/jobs/build/payment/stripe-config-secret.yaml Show resolved Hide resolved

easyCZ reviewed May 31, 2022

View reviewed changes

corneliusludmann approved these changes May 31, 2022

View reviewed changes

geropl approved these changes May 31, 2022

View reviewed changes

roboquat merged commit 92e2e72 into main May 31, 2022

roboquat deleted the jx/stripe-secret branch May 31, 2022 08:10

roboquat added the deployed: webapp Meta team change is running in production label Jun 9, 2022

Add a Stripe settings secret to server #10308

Add a Stripe settings secret to server #10308

Uh oh!

Conversation

jankeromnes commented May 27, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Related Issue(s)

How to test

Release Notes

Documentation

Uh oh!

jankeromnes commented May 30, 2022 • edited by werft-gitpod-dev-com bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jankeromnes commented May 30, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

geropl commented May 30, 2022

Uh oh!

jankeromnes commented May 30, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jankeromnes commented May 30, 2022 • edited by werft-gitpod-dev-com bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jankeromnes commented May 30, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jankeromnes commented May 30, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Without payment

With payment

Uh oh!

Uh oh!

easyCZ May 31, 2022

Choose a reason for hiding this comment

Uh oh!

easyCZ May 31, 2022

Choose a reason for hiding this comment

Uh oh!

jankeromnes May 31, 2022

Choose a reason for hiding this comment

Uh oh!

easyCZ May 31, 2022

Choose a reason for hiding this comment

Uh oh!

jankeromnes May 31, 2022

Choose a reason for hiding this comment

Uh oh!

easyCZ May 31, 2022

Choose a reason for hiding this comment

Uh oh!

geropl May 31, 2022

Choose a reason for hiding this comment

Uh oh!

geropl commented May 31, 2022

Uh oh!

corneliusludmann left a comment

Choose a reason for hiding this comment

Uh oh!

geropl left a comment

Choose a reason for hiding this comment

Uh oh!

jankeromnes commented May 31, 2022 • edited by werft-gitpod-dev-com bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

jankeromnes commented May 27, 2022 •

edited

Loading

jankeromnes commented May 30, 2022 •

edited by werft-gitpod-dev-com bot

Loading

jankeromnes commented May 30, 2022 •

edited

Loading

jankeromnes commented May 30, 2022 •

edited

Loading

jankeromnes commented May 30, 2022 •

edited by werft-gitpod-dev-com bot

Loading

jankeromnes commented May 30, 2022 •

edited

Loading

jankeromnes commented May 30, 2022 •

edited

Loading

jankeromnes commented May 31, 2022 •

edited by werft-gitpod-dev-com bot

Loading