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.

Sign up for GitHub

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

Jump to bottom

[server] Make Stripe usage-based product price IDs configurable #10660

Merged
merged 3 commits into from
Jun 20, 2022
Merged

[server] Make Stripe usage-based product price IDs configurable #10660

merged 3 commits into from
Jun 20, 2022

Conversation

jankeromnes
Copy link
Contributor

@jankeromnes jankeromnes commented Jun 14, 2022 •
edited

Description

Make Stripe usage-based product price IDs configurable.

Companion ops PR: https://github.com/gitpod-io/ops/pull/2819 (internal)

Related Issue(s)

Fixes #10655

How to test

  1. Trigger a custom Werft build (using the config from this PR)
    • E.g. like so: kubectx dev then werft job run github -j .werft/build.yaml -a with-clean-slate-deployment=true
  2. Then, same test flow as in When creating a new Stripe customer, also create a Usage-Based Subscription for them #10630

Release Notes

NONE

Documentation

  • /werft with-payment=true

@roboquat roboquat added size/M and removed size/S labels Jun 14, 2022
@roboquat roboquat added size/L and removed size/M labels Jun 15, 2022
@werft-gitpod-dev-com
Copy link

started the job as gitpod-build-jx-stripe-configmap.5 because the annotations in the pull request description changed
(with .werft/ from main)

@werft-gitpod-dev-com
Copy link

started the job as gitpod-build-jx-stripe-configmap.6 because the annotations in the pull request description changed
(with .werft/ from main)

@werft-gitpod-dev-com

This comment was marked as outdated.

Sign in to view
@werft-gitpod-dev-com

This comment was marked as outdated.

Sign in to view
@jankeromnes jankeromnes force-pushed the jx/stripe-configmap branch 2 times, most recently from 141a440 to 5db28df Compare June 16, 2022 09:23
@werft-gitpod-dev-com

This comment was marked as outdated.

Sign in to view
@jankeromnes

This comment was marked as resolved.

Sign in to view
@jankeromnes jankeromnes force-pushed the jx/stripe-configmap branch 2 times, most recently from 7e2104b to 07779c5 Compare June 16, 2022 12:08
@jankeromnes jankeromnes marked this pull request as ready for review June 16, 2022 12:40
@jankeromnes jankeromnes requested review from a team June 16, 2022 12:40
@github-actions github-actions bot added the team: delivery Issue belongs to the self-hosted team label Jun 16, 2022
@github-actions github-actions bot added the team: webapp Issue belongs to the WebApp team label Jun 16, 2022
@jankeromnes
Copy link
Contributor Author

jankeromnes commented Jun 16, 2022 •
edited

New custom build: https://werft.gitpod-dev.com/job/gitpod-custom-jx-stripe-configmap.9

Triggered with:

werft job run github -j .werft/build.yaml -a with-clean-slate-deployment=true

And works like a charm! 🎉 (Created usage-based subscriptions are now using the new Stripe Product with "graduated pricing", as intended)

Screenshot 2022-06-16 at 14 52 21

@werft-gitpod-dev-com
Copy link

started the job as gitpod-build-jx-stripe-configmap.15 because the annotations in the pull request description changed
(with .werft/ from main)

@jankeromnes jankeromnes requested a review from geropl June 16, 2022 15:08
andrew-farries
andrew-farries reviewed Jun 17, 2022
View reviewed changes
.werft/jobs/build/helm/values.payment.yaml Outdated
Comment on lines 19 to 24
- name: stripe-secret
secret:
secretName: stripe-api-keys
- name: stripe-config
configMap:
name: stripe-config
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand why this whole section is necessary.

Werft installs gitpod using the installer which already configures server's volumes and mounts, so what is this section for?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good question -- I'm actually not sure 🙈 I just basically copied how chargebee-config is implemented.

Do you think I should try without some or all of the changes in this file?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd try it without the changes to this file.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, let's try:

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting -- still works as intended:

Screenshot 2022-06-17 at 11 29 10

Do you think we should delete this YAML file?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think we should delete this YAML file?

Probably not in this PR. Open a separate one and maybe get the Platform team's view on what it's for.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@andrew-farries Indeed, this is a left-over - and we have been cargo-culting here 🙈 Let's remove (incl. all references)! 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good! I'll send a follow-up PR. 🧹

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#10761

@roboquat roboquat added size/M and removed size/L labels Jun 17, 2022
mrsimonemms
mrsimonemms reviewed Jun 17, 2022
View reviewed changes
Copy link
Contributor

@mrsimonemms mrsimonemms left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just one question really - rest looks fine

@mrsimonemms mrsimonemms self-requested a review June 17, 2022 17:06
@roboquat roboquat merged commit a705517 into main Jun 20, 2022
@roboquat roboquat deleted the jx/stripe-configmap branch June 20, 2022 09:22
@jankeromnes jankeromnes mentioned this pull request Jun 20, 2022
Merged
1 task
@roboquat roboquat added the deployed: webapp Meta team change is running in production label Jun 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mrsimonemms mrsimonemms mrsimonemms approved these changes

@andrew-farries andrew-farries andrew-farries approved these changes

@geropl geropl Awaiting requested review from geropl

Assignees
No one assigned
Labels
deployed: webapp Meta team change is running in production release-note-none size/M team: delivery Issue belongs to the self-hosted team team: webapp Issue belongs to the WebApp team
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Configure usage-based Stripe product IDs (i.e. prices)
5 participants
@jankeromnes @mrsimonemms @andrew-farries @geropl @roboquat