Compare commits

...

3 Commits

Author SHA1 Message Date
nexxo 5d12983ef7 Release 1.3.44
tests / pest (push) Failing after 7m56s Details
tests / assets (push) Successful in 30s Details
tests / release (push) Has been skipped Details
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-07-30 02:18:15 +02:00
nexxo 9da1358802 Stop charging VAT to the customers who owe us none
A verified EU business outside Austria gets reverse charge — rate 0, no VAT
line, the note on the invoice — and was still charged 214,80 € for a 179,00 €
package, because one Stripe Price carried the domestic gross for everybody.
There is no VAT line on their document to reclaim, so it was a flat 20 %
surcharge on exactly the customers who read their invoices, and the document
then stated the whole 214,80 € as net at 0 %, so they self-accounted their own
VAT on a base a fifth too large.

A Stripe Price can ask who is buying after all: there are two per sellable
thing now, the domestic gross and the bare net, both live on one Product, and
the checkout picks by TaxTreatment. The rule is Austria B2B 20 %, other EU B2B
without VAT — a domestic business still pays the gross, reclaims it as input
tax, and the price on the website is still the price charged for them.

- stripe:sync-catalogue mints and archives both halves of every pair, per plan
  and per module; `reverse_charge` on stripe_plan_prices/stripe_addon_prices
  says which is which, and joining it on subscriptions.stripe_price_id says
  which one a running contract is billed on. A rate change moves the gross
  Price and leaves the net one alone, because the net owes nothing to the rate.
- A status that changes after the sale converges: a verification (or a lapse)
  makes stripe:reprice-subscriptions move the contract onto the other Price
  with PRORATE_NONE, because the term is already paid for. The module items
  follow the same way.
- Downstream follows: the invoice total equals what was taken, the proof
  register expects the figure this customer is actually charged (so a correct
  reverse-charge sale is no longer flagged as a mismatch, and an overcharged
  one is), the setup fee obeys the same rule, and the booking page quotes what
  it will charge.
- StripeClient gained activatePrice(). Archived Prices were brought back in our
  own table and left inactive at Stripe, so a rate that moved and moved back
  pointed the catalogue at a Price no checkout could be opened for.

An unverified number is still charged the gross — an unchecked string must
never be a discount — and where the net Price is missing the checkout refuses
rather than reaching for the domestic one.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-07-30 02:15:41 +02:00
nexxo 889b401faf Fix nine defects in the provisioning pipelines
The capacity park could not survive its own first poll. ReserveResources
polls every 120 s and declared a maxDuration of 60, and RunRunner measures
a step's budget from a started_at it deliberately does not reset on a poll
— so every re-entry was ruled timed out before the body ran, a timeout
consumes an attempt, and five of them failed a paid order in about six
minutes. The fourteen-day promise, the console's capacity queue and the
"go and buy a server" workflow were unreachable code. maxDuration is now
the whole park plus one poll interval, and a test drives a park through
the runner rather than calling execute() directly, which is why this was
invisible.

Every guest occ call ran as root. `docker compose exec` defaults to root
and the Nextcloud image's console.php exits 1 unless the caller owns
config/config.php, so all five call sites were failing on every instance.
There is now one builder, App\Support\NextcloudOcc, and a test refuses a
second: nothing in app/ may spell the invocation out by hand. deploy/
update.sh had learned this for our own container and nobody carried it
across.

RunAcceptanceChecks was terminal on the first no. certReachable() answers
false for a connect timeout as readily as for a missing certificate, so
one bad second ended a finished, certified Nextcloud as a failed order
with the instance released. The probes retry now and the run still fails
for good with the probe's own reason once the budget is spent; the two
facts a retry cannot change stay terminal.

The in-flight guard asked whether anything was running, not whether it
would do the work. App\Provisioning\WorkInFlight asks the second question,
reading the step lists the runner executes: a domain proven during a
restart is routed, and a storage pack booked during an address run is
delivered instead of charged monthly and forgotten.

The address pipeline ran its steps in the wrong order for the direction it
exists for. Nextcloud before the router in `address` and `plan-change`, as
in `customer`, because a run that got the certificate and then failed left
the customer's own domain serving an untrusted-domain error under a valid
certificate while the portal called it live. Safe in both directions; the
reasoning is written above the pipeline.

A stale guest_ip could never be corrected. ConfigureDnsAndTls compared the
hostnames but never the backend, and nothing re-read the address after the
build. It now records and compares `routed_backend`, and ConfigureNetwork
joins the `restart` pipeline — a cold boot is what moves a DHCP lease — and
asks for the address to be re-applied when the guest has actually moved.

Also: HetznerDnsClient::upsertRecord read only the first page of a zone
that pages at 100, so past a hundred records it created a second A record
for the same name and the cloud was up about half the time; the console's
retry left a live customer's order in `provisioning` for ever for any
maintenance pipeline; and a revived run's first pass burned an attempt on a
timeout that had already happened, because the clause written for it used
`??=`.

Seven test sites wrote config('provisioning.plans.*'), a key that has not
existed since the catalogue moved into the database. Every write was a
no-op, so each test proved something other than its name: the missing
template, the two snapshot regressions and the grandfathered-price scenario
are now actually constructed, ValidateOrder's reasons are asserted rather
than only its type, and the end-to-end run checks that the quota step
delivered something.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-07-30 01:34:55 +02:00
69 changed files with 2654 additions and 524 deletions

View File

@ -1 +1 @@
1.3.43
1.3.44

View File

@ -175,13 +175,17 @@ class ApplyPlanChange
'remaining_days' => $change->remainingDays,
'term_days' => $change->termDays,
// The preview's pro-rata difference, and what the till would
// take for it. Both, because PlanChange prorates the NET
// catalogue figures while the Stripe Prices it prorates
// against are gross — so the net figure alone reads as a fifth
// less than the proration invoice that follows, and somebody
// reconciling the two needs the number to compare.
// take from THIS customer for it. Both, because PlanChange
// prorates the NET catalogue figures while the Stripe Price it
// prorates against is the one that customer is billed on — so
// for anybody who is charged VAT the net figure alone reads as
// a fifth less than the proration invoice that follows, and
// somebody reconciling the two needs the number to compare.
// For a reverse-charge business the two are equal, and saying
// so is what stops a reader looking for a missing fifth.
'prorated_net_cents' => $change->chargeCents,
'prorated_charge_cents' => TaxTreatment::chargedCents($change->chargeCents),
'prorated_charge_cents' => TaxTreatment::for($subscription->customer)
->chargeCents($change->chargeCents),
]],
// Nothing has been charged AT THIS EVENT. Left null rather than
// reported as zero, which the register would read as a free sale.

View File

@ -7,6 +7,7 @@ use App\Models\Order;
use App\Models\ProvisioningRun;
use App\Models\Subscription;
use App\Provisioning\Jobs\AdvanceRunJob;
use App\Provisioning\WorkInFlight;
use Illuminate\Support\Facades\Cache;
use Illuminate\Support\Facades\Log;
@ -89,13 +90,15 @@ class ApplyStorageAllowance
{
$order = $instance->order;
// Checked against the ORDER, because that is the subject every customer
// pipeline shares. A storage run beside an unfinished build would resize
// a disk the build is still writing; one beside a plan change would
// resize it twice from two different figures. Whatever is in flight
// reads the allowance itself when it reaches the quota step, so nothing
// is lost by standing aside.
if ($this->hasRunInFlight($order)) {
// Stand aside only for a run that will deliver the whole allowance: the
// disk, the filesystem inside it and the figure Nextcloud enforces. A
// plan change does all three and there is nothing to add. A `quota` run
// does only the last of them, and letting it stand for the booking would
// enforce a quota over a filesystem that never grew — space the machine
// does not have. `address` and `restart` do none of them, and standing
// aside for one of those was how a pack could be charged monthly and
// never delivered. See App\Provisioning\WorkInFlight.
if (app(WorkInFlight::class)->covers($order, 'storage')) {
return null;
}
@ -125,18 +128,4 @@ class ApplyStorageAllowance
return $run;
}
private function hasRunInFlight(Order $order): bool
{
return ProvisioningRun::query()
->where('subject_type', Order::class)
->where('subject_id', $order->id)
->whereIn('status', [
ProvisioningRun::STATUS_PENDING,
ProvisioningRun::STATUS_RUNNING,
ProvisioningRun::STATUS_WAITING,
ProvisioningRun::STATUS_PAUSED,
])
->exists();
}
}

View File

@ -120,6 +120,7 @@ class EndInstanceService
// ever revived would be announced and routed nowhere.
'route_written' => false,
'routed_hostnames' => null,
'routed_backend' => null,
'cert_ok' => false,
'domain_cert_ok' => false,
]);

View File

@ -2,8 +2,8 @@
namespace App\Actions;
use App\Models\PlanPrice;
use App\Models\Subscription;
use App\Services\Billing\PlanPrices;
use App\Services\Stripe\StripeClient;
use Illuminate\Support\Facades\Log;
use RuntimeException;
@ -47,7 +47,10 @@ use Throwable;
*/
class MoveStripeSubscriptionPrice
{
public function __construct(private StripeClient $stripe) {}
public function __construct(
private StripeClient $stripe,
private PlanPrices $prices,
) {}
/**
* @param string $behaviour one of StripeClient::PRORATE_*
@ -135,21 +138,24 @@ class MoveStripeSubscriptionPrice
}
/**
* The Stripe Price for the package the contract is on now.
* The Stripe Price for the package the contract is on now, for the customer
* who is paying for it.
*
* By plan VERSION and term, which is what a Stripe Price is: one per priced
* row, written by stripe:sync-catalogue. Resolving it by plan name would
* hand back today's terms for a contract that is grandfathered on older
* ones.
* row and per tax treatment, written by stripe:sync-catalogue. Resolving it by
* plan name would hand back today's terms for a contract that is
* grandfathered on older ones.
*
* The treatment is part of the answer, which is what makes a verification
* converge: a business whose VAT id is confirmed the week after they bought
* belongs on the net Price from that moment, and one whose registration has
* lapsed belongs back on the gross one. Neither is a change of package, so
* nothing here looks like one the move is made with PRORATE_NONE by the
* sweep that finds it, because the term is already paid for.
*/
private function targetPrice(Subscription $subscription): ?string
{
$priceId = PlanPrice::query()
->where('plan_version_id', $subscription->plan_version_id)
->where('term', $subscription->term)
->value('stripe_price_id');
return is_string($priceId) && $priceId !== '' ? $priceId : null;
return $this->prices->liveForSubscription($subscription);
}
/**

View File

@ -19,9 +19,11 @@ use Illuminate\Support\Facades\DB;
* cannot reach a customer who has already paid.
*
* `price_cents` is the catalogue's NET price, which is what PlanChange prorates
* against deliberately not `Order::amount_cents`, which holds the GROSS total
* Stripe actually charged. The two can legitimately differ (VAT, and later
* coupons), and reconciling them is not this action's job: the proof register
* against deliberately not `Order::amount_cents`, which holds the total Stripe
* actually charged, whichever of a package's two Prices this customer was sold on.
* The two can legitimately differ (VAT, and later coupons) and for a
* reverse-charge business they legitimately agree, because no VAT is owed to us at
* all and reconciling them is not this action's job: the proof register
* records what was charged per event, and Stripe's invoice is the authority for
* the amount. Copying a gross total into this net field would silently corrupt
* every pro-rata calculation that reads it.
@ -107,10 +109,11 @@ class OpenSubscription
netCents: $subscription->price_cents,
at: $start,
stripe: ['event' => $order->stripe_event_id],
// The order carries what Stripe actually took — gross, including
// whatever tax or discount applied on the day. The contract price
// is what was agreed; this is what was paid, and the register has
// to be able to state both.
// The order carries what Stripe actually took, including whatever tax
// or discount applied on the day. The contract price is what was
// agreed; this is what was paid, and the register has to be able to
// state both — which for a reverse-charge business is the same figure,
// because their Price carries the bare net.
//
// Keyed on the Stripe id, not on the amount being non-zero: a fully
// discounted checkout charges zero, and reading that as "no amount

View File

@ -6,6 +6,7 @@ use App\Models\Instance;
use App\Models\Order;
use App\Models\ProvisioningRun;
use App\Provisioning\Jobs\AdvanceRunJob;
use App\Provisioning\WorkInFlight;
use Illuminate\Support\Facades\Cache;
use Illuminate\Support\Facades\Log;
@ -66,12 +67,14 @@ class ReapplyInstanceAddress
{
$order = $instance->order;
// A run already under way applies whatever the domain state says when
// it gets to the step, which is this run's state or newer. Checked
// against the ORDER, because that is the subject both pipelines share:
// starting an address run beside an unfinished customer run would have
// two runs writing one router and one Nextcloud config.
if ($this->hasRunInFlight($order)) {
// A run already under way applies whatever the domain state says when it
// gets to the step — but only if it HAS the step. Asked of the two steps
// this pipeline consists of rather than of the mere existence of a run:
// `customer` and `plan-change` both carry them and there is nothing to
// add, while `restart`, `storage` and `quota` carry neither, and standing
// aside for one of those was how a domain proven during a restart stayed
// unrouted for good. See App\Provisioning\WorkInFlight.
if (app(WorkInFlight::class)->covers($order, 'address')) {
return null;
}
@ -115,18 +118,4 @@ class ReapplyInstanceAddress
{
return $instance->hasLiveMachine();
}
private function hasRunInFlight(Order $order): bool
{
return ProvisioningRun::query()
->where('subject_type', Order::class)
->where('subject_id', $order->id)
->whereIn('status', [
ProvisioningRun::STATUS_PENDING,
ProvisioningRun::STATUS_RUNNING,
ProvisioningRun::STATUS_WAITING,
ProvisioningRun::STATUS_PAUSED,
])
->exists();
}
}

View File

@ -44,18 +44,18 @@ class RecordCommercialEvent
$agreedNet = $netCents;
// What the till would take for this catalogue figure — the DOMESTIC
// gross, for everybody. Not $tax->grossCents(), which is how this
// customer's DOCUMENT is split: at a rate of zero that returns the net
// figure, and comparing it against the money Stripe actually took
// reported every reverse-charge sale charged at exactly the advertised
// amount as a mismatch. The one flag whose job is "somebody must look at
// this" was false for the whole healthy majority of EU business sales.
// What the till would take from THIS customer for this catalogue figure:
// the domestic gross for everybody who is charged VAT, the bare net for a
// reverse-charge business, because that is the Stripe Price their checkout
// used. Asked of the treatment rather than of a single domestic figure,
// which is what it used to be — and every reverse-charge sale, charged
// exactly the amount it should have been, was then reported as a mismatch
// by the one flag whose job is "somebody must look at this".
//
// TaxTreatment::chargedCents() is the single place a gross is formed, and
// it is what the Stripe Price carries — so this is the figure the charge
// can honestly be held against.
$expectedGross = TaxTreatment::chargedCents($agreedNet);
// chargeCents() is the single place the amount taken is formed, and it is
// what the Stripe Price carries — so this is the figure the charge can
// honestly be held against.
$expectedGross = $tax->chargeCents($agreedNet);
// The flat columns describe the TRANSACTION, not the contract: gross is
// what was taken from the customer, and net and tax are that gross

View File

@ -111,10 +111,12 @@ class RestartInstance
$order = $instance->order;
// Checked against the ORDER, because that is the subject every customer
// pipeline shares. A restart beside an unfinished build would stop the
// machine the build is still writing to; a restart beside a plan change
// would race the config PUT it is about to boot. Whatever is in flight
// gets to finish, and the customer can press the button again after.
// pipeline shares. ANY run, and deliberately not the narrower "does it do
// this work?" question ReapplyInstanceAddress and ApplyStorageAllowance
// ask: taking the guest away underneath a run is harmful to every one of
// them, and unlike a proven domain or a booked pack a restart is somebody
// pressing a button, so it can be pressed again once the run is done
// rather than needing to be remembered for them.
if ($this->hasRunInFlight($order)) {
return null;
}
@ -152,12 +154,7 @@ class RestartInstance
return ProvisioningRun::query()
->where('subject_type', Order::class)
->where('subject_id', $order->id)
->whereIn('status', [
ProvisioningRun::STATUS_PENDING,
ProvisioningRun::STATUS_RUNNING,
ProvisioningRun::STATUS_WAITING,
ProvisioningRun::STATUS_PAUSED,
])
->inFlight()
->exists();
}
}

View File

@ -5,6 +5,7 @@ namespace App\Actions;
use App\Models\Subscription;
use App\Models\SubscriptionAddon;
use App\Services\Billing\AddonPrices;
use App\Services\Billing\TaxTreatment;
use App\Services\Stripe\StripeClient;
use Illuminate\Support\Collection;
use Illuminate\Support\Facades\Log;
@ -108,13 +109,19 @@ class SyncStripeAddonItems
/**
* Move every module item this contract carries onto the Price that charges
* today's figure for it.
* today's figure for it, to the customer who is paying for it.
*
* The catalogue moved from pushing NET amounts to Stripe to pushing GROSS
* ones, and a Stripe Price cannot be edited so every running module is on a
* Price that takes too little. __invoke() above will not find them: the item
* exists and the quantity is right, so it has nothing to reconcile. This is
* the deliberate second pass, run by stripe:reprice-subscriptions.
* Two reasons an item is on the wrong Price, and __invoke() above finds
* neither of them: the item exists and the quantity is right, so it has
* nothing to reconcile. This is the deliberate second pass, run by
* stripe:reprice-subscriptions.
*
* The first is a change of figure the catalogue moved from pushing NET
* amounts to Stripe to pushing the amount actually charged, and a Stripe Price
* cannot be edited, so every running module was on a Price that takes too
* little. The second is a change of CUSTOMER: a business whose VAT id is
* verified after they booked owes no VAT on the module either, and belongs on
* its net Price from then on the reverse too, when a registration lapses.
*
* PRORATE_NONE throughout. The customer has already paid for the term they
* are in, at whatever was taken then; charging the difference for days
@ -154,6 +161,7 @@ class SyncStripeAddonItems
(int) $first->price_cents,
(string) $first->currency,
(string) $subscription->term,
TaxTreatment::for($subscription->customer),
);
$current = $stamped->first()?->stripe_price_id;
@ -177,6 +185,7 @@ class SyncStripeAddonItems
(int) $first->price_cents,
(string) $first->currency,
(string) $subscription->term,
TaxTreatment::for($subscription->customer),
);
if ($target === null) {
@ -284,6 +293,12 @@ class SyncStripeAddonItems
(int) $first->price_cents,
(string) $first->currency,
(string) $subscription->term,
// Whoever holds the contract decides which of the module's Prices its
// item bills through: the domestic gross, or the bare net where the
// customer is a verified business in another member state. A module is
// a supply like the package beside it and cannot be taxed differently
// from it on the same invoice.
TaxTreatment::for($subscription->customer),
);
if ($priceId === null) {

View File

@ -157,17 +157,19 @@ class ApplyStorageQuotas extends Command
AdvanceRunJob::dispatch($run->uuid);
}
/**
* ANY run, deliberately unlike the narrower "will it do this work?" question
* App\Provisioning\WorkInFlight answers for the two actions that must not lose
* a change they were asked to make. This is a repair sweep: it runs again, it
* skips instances whose quota is already enforced, and an instance passed over
* today is picked up on the next pass. Nothing is forgotten by waiting.
*/
private function hasRunInFlight(Instance $instance): bool
{
return ProvisioningRun::query()
->where('subject_type', Order::class)
->where('subject_id', $instance->order_id)
->whereIn('status', [
ProvisioningRun::STATUS_PENDING,
ProvisioningRun::STATUS_RUNNING,
ProvisioningRun::STATUS_WAITING,
ProvisioningRun::STATUS_PAUSED,
])
->inFlight()
->exists();
}
}

View File

@ -4,8 +4,8 @@ namespace App\Console\Commands;
use App\Actions\MoveStripeSubscriptionPrice;
use App\Actions\SyncStripeAddonItems;
use App\Models\PlanPrice;
use App\Models\Subscription;
use App\Services\Billing\PlanPrices;
use App\Services\Stripe\StripeClient;
use Illuminate\Console\Command;
@ -37,6 +37,16 @@ use Illuminate\Console\Command;
* throws MoveStripeSubscriptionPrice parks its failures on the contract and
* clupilot:sync-stripe-subscriptions retries them hourly.
*
* **Two reasons a contract moves, not one.** The first is a change of FIGURE, as
* above. The second is a change of CUSTOMER: a business whose VAT id is verified
* the week after they bought must be billed the net Price from then on, because
* reverse charge means they owe no VAT to us at all and one whose registration
* has lapsed must go back onto the gross one. Neither is a change of package, so
* nothing else in the system would ever notice, and both converge here on the next
* run. PlanPrices::liveForSubscription() answers "which Price does this contract
* belong on?" for both, which is why this command has no resolver of its own: two
* places working that out is how they come to disagree.
*
* The contract's own `price_cents` is not touched by any of this. It is the
* catalogue's NET figure, it is frozen, and PlanChange prorates against it.
*/
@ -52,6 +62,7 @@ class RepriceStripeSubscriptions extends Command
StripeClient $stripe,
MoveStripeSubscriptionPrice $move,
SyncStripeAddonItems $modules,
PlanPrices $prices,
): int {
$dryRun = (bool) $this->option('dry-run');
@ -76,14 +87,24 @@ class RepriceStripeSubscriptions extends Command
$failed = 0;
foreach ($contracts as $contract) {
$target = $this->packagePrice($contract);
// Whether the catalogue prices this contract's package at all, asked
// separately from WHICH Price it should be on. A version that was never
// priced is a contract this command has nothing to say about; one that
// is priced but has no Stripe Price for this customer's treatment is a
// contract that must move, and the move is what mints it.
$priced = $prices->rowFor($contract) !== null;
$target = $priced ? $prices->liveForSubscription($contract) : null;
if ($target !== null && $contract->stripe_price_id !== $target) {
if ($priced && $contract->stripe_price_id !== $target) {
$this->line(sprintf(
' package %s %s → %s',
$contract->uuid,
$contract->stripe_price_id ?? '(unknown)',
$target,
// Null where the catalogue has no Price for what this customer
// should be charged. Said out loud rather than skipped: the move
// below parks the reason on the contract and the run warns about
// it, and the cure is one command.
$target ?? '(not synced)',
));
$packages++;
@ -128,22 +149,4 @@ class RepriceStripeSubscriptions extends Command
return self::SUCCESS;
}
/**
* The Stripe Price the catalogue sells this contract's package on now.
*
* By plan VERSION and term, exactly as MoveStripeSubscriptionPrice resolves
* it resolving it by plan name would hand a grandfathered contract today's
* terms. Null where the version was never synced, which is a contract this
* command has nothing to say about.
*/
private function packagePrice(Subscription $subscription): ?string
{
$priceId = PlanPrice::query()
->where('plan_version_id', $subscription->plan_version_id)
->where('term', $subscription->term)
->value('stripe_price_id');
return is_string($priceId) && $priceId !== '' ? $priceId : null;
}
}

View File

@ -5,10 +5,10 @@ namespace App\Console\Commands;
use App\Models\PlanFamily;
use App\Models\PlanPrice;
use App\Models\PlanVersion;
use App\Models\StripePlanPrice;
use App\Models\Subscription;
use App\Services\Billing\AddonCatalogue;
use App\Services\Billing\AddonPrices;
use App\Services\Billing\PlanPrices;
use App\Services\Billing\TaxTreatment;
use App\Services\Stripe\StripeClient;
use Illuminate\Console\Command;
@ -21,12 +21,22 @@ use Illuminate\Console\Command;
* invoice numbering so it needs to know what it is billing for. It does not
* need to know how big the VM is, and it is not asked.
*
* **What it pushes is the GROSS figure.** The catalogue's `amount_cents` is net
* and stays net it is frozen onto every contract and PlanChange prorates
* against it but the amount Stripe takes is that figure at the till, so the
* charge equals the price on the website and equals the total on the document.
* Pushing the net was the defect: a customer quoted 214,80 paid 179,00 and
* was then invoiced 179,00 plus 35,80 VAT that nobody had collected.
* **It pushes TWO Prices for everything it sells.** The catalogue's `amount_cents`
* is net and stays net it is frozen onto every contract and PlanChange prorates
* against it and what Stripe is asked to take from it depends on who is buying:
*
* - the DOMESTIC GROSS, which is the figure on the website and what everybody
* who is charged VAT pays, private and business alike. Pushing the net was the
* original defect: a customer quoted 214,80 paid 179,00 and was then
* invoiced 179,00 plus 35,80 VAT that nobody had collected;
* - the BARE NET, for a business in another member state whose VAT id is
* verified. Reverse charge means no VAT is owed to us at all, so the net is
* the whole of what they owe. Charging them the gross was a flat 20 % surcharge
* with no VAT line on the document for them to reclaim.
*
* Which of the two a checkout uses is TaxTreatment's decision see
* App\Services\Billing\PlanPrices, which owns the plan half of the mirror, and
* App\Services\Billing\AddonPrices, which owns the module half.
*
* Stripe's `automatic_tax` is deliberately NOT used. TaxTreatment is the single
* tax authority here; a second rate computed by Stripe would charge a German
@ -34,10 +44,11 @@ use Illuminate\Console\Command;
*
* Idempotent by construction, and safe to re-run after a rate change: a row
* whose live Price already charges the right figure is skipped, and one that
* does not gets a NEW Price with the old one archived. That matters more here
* than usual, because a Stripe Price cannot be edited, so a run that minted
* duplicates would leave two live prices for one plan and no way to tell which a
* customer is on.
* does not gets a NEW Price with the old one archived. A rate change moves the
* gross Price of each pair and leaves the net one untouched, because the net does
* not depend on the rate. That matters more here than usual, because a Stripe
* Price cannot be edited, so a run that minted duplicates would leave two live
* prices for one plan at one figure and no way to tell which a customer is on.
*
* It does not move anybody. Archiving a Price stops it being SOLD and leaves
* every subscription already on it exactly where it was moving those is
@ -104,7 +115,7 @@ class SyncStripeCatalogue extends Command
foreach ($published as $version) {
foreach ($version->prices as $price) {
$created += $this->syncPrice($stripe, $family, $version, $price, $productId, $dryRun);
$created += $this->syncPrice($family, $version, $price, $productId, $dryRun);
}
}
}
@ -127,20 +138,19 @@ class SyncStripeCatalogue extends Command
}
/**
* The Stripe Price one priced catalogue row is sold on, brought into step.
* The Stripe Prices one priced catalogue row is sold on, brought into step.
*
* Three outcomes, and which one happens is decided by the CHARGED figure
* the row's net at today's rate and never by whether an id happens to be
* stored:
* Both treatments, always: the domestic gross AND the bare net a
* reverse-charge business is charged. Unconditionally, rather than only when
* such a customer exists the Price has to be there BEFORE the checkout that
* needs it, and a verified business meeting a refused checkout is worse than
* an unused Price sitting in Stripe.
*
* - the live Price already charges it, and the row points at that Price:
* nothing to do, which is what makes a second run free;
* - a Price for that figure exists but is archived or is not the one the row
* points at a rate moved and moved back, or a run died between Stripe
* creating the Price and us storing its id: it is brought back rather than
* minted a second time;
* - nothing charges it: a new Price is created, the row is repointed, and
* whatever it was pointing at is archived in Stripe and here.
* What "in step" means, and the find-or-mint-or-replace that follows from it,
* belongs to PlanPrices: it is the same question the checkout and the plan
* swap ask, and asking it in two places is how they come to disagree. All
* this adds is the reporting, because an operator running a sync has to be
* able to see which half of which pair moved.
*
* Archiving stops a Price being offered and leaves every subscription on it
* untouched, which is exactly what is wanted: those are moved deliberately,
@ -149,87 +159,48 @@ class SyncStripeCatalogue extends Command
* @return int how many objects this row changed, for the run's own count
*/
private function syncPrice(
StripeClient $stripe,
PlanFamily $family,
PlanVersion $version,
PlanPrice $price,
?string $productId,
bool $dryRun,
): int {
$charged = TaxTreatment::chargedCents((int) $price->amount_cents);
$prices = app(PlanPrices::class);
$changed = 0;
$existing = StripePlanPrice::query()
->where('plan_price_id', $price->id)
->where('charged_cents', $charged)
->first();
if ($existing !== null && $existing->isLive() && $price->stripe_price_id === $existing->stripe_price_id) {
return 0;
foreach ($this->treatments() as $label => $treatment) {
if ($prices->inStep($price, $treatment)) {
continue;
}
$this->line(sprintf(
' price %s v%d %s %d %s net → %d %s charged%s',
$family->key, $version->version, $price->term,
' price %s v%d %s %s %d %s net → %d %s charged',
$family->key, $version->version, $price->term, $label,
$price->amount_cents, $price->currency,
$charged, $price->currency,
$existing === null ? '' : ' (Price already exists)',
PlanPrices::chargedCents($price, $treatment), $price->currency,
));
$changed++;
// Nothing to create against: the Product for this family is minted
// by the caller, and on a dry run it does not exist yet either.
if ($dryRun || $productId === null) {
return 1;
continue;
}
$priceId = $existing?->stripe_price_id ?? $stripe->createPrice(
productId: $productId,
amountCents: $charged,
currency: $price->currency,
interval: $price->term === 'yearly' ? 'year' : 'month',
metadata: [
'plan_family' => $family->key,
'plan_version' => (string) $version->version,
'plan_version_id' => (string) $version->id,
'plan_price_id' => (string) $price->id,
],
// The CHARGED amount is part of the key. Without it a run after a
// rate change would replay the Price minted at the old figure, and
// Stripe would hand back the very object this is replacing.
idempotencyKey: "clupilot-price-{$price->id}-{$charged}",
);
StripePlanPrice::query()->updateOrCreate(
['plan_price_id' => $price->id, 'charged_cents' => $charged],
['stripe_price_id' => $priceId, 'archived_at' => null],
);
// Everything else this row was ever sold on stops being offered — after
// the replacement exists, never before, so a failure in between leaves
// the old Price selling rather than nothing at all.
$superseded = StripePlanPrice::query()
->where('plan_price_id', $price->id)
->where('charged_cents', '!=', $charged)
->whereNull('archived_at')
->get();
foreach ($superseded as $old) {
$stripe->archivePrice((string) $old->stripe_price_id);
$old->update(['archived_at' => now()]);
$prices->ensure($price, $treatment);
}
// Written straight through the query builder: stripe_price_id is not
// part of what publication froze, and the model would otherwise have to
// be re-read first.
PlanPrice::query()->whereKey($price->id)->update(['stripe_price_id' => $priceId]);
return 1;
return $changed;
}
/**
* A Product and two Prices monthly and yearly for every module on sale.
* A Product and four Prices for every module on sale: monthly and yearly,
* each at the domestic gross and at the bare net.
*
* Both terms, unconditionally, rather than only the ones somebody has bought
* a contract on: the Price has to exist BEFORE the booking that needs it, and
* a customer on a yearly package booking their first module would otherwise
* discover the gap at the moment their money was due.
* Both terms and both treatments, unconditionally, rather than only the ones
* somebody has bought a contract on: the Price has to exist BEFORE the booking
* that needs it, and a customer on a yearly package booking their first module
* would otherwise discover the gap at the moment their money was due.
*
* @return int how many objects were created, for the run's own count
*/
@ -253,18 +224,19 @@ class SyncStripeCatalogue extends Command
}
foreach ([Subscription::TERM_MONTHLY, Subscription::TERM_YEARLY] as $term) {
foreach ($this->treatments() as $label => $treatment) {
// Asked of the CHARGED figure, so a rate change is seen as a
// Price that has to be replaced rather than as one that is
// already there.
if ($prices->liveFor($key, $monthly, $currency, $term) !== null) {
if ($prices->liveFor($key, $monthly, $currency, $term, $treatment) !== null) {
continue;
}
$this->line(sprintf(
' module %s %s %d %s net → %d %s charged',
$key, $term,
' module %s %s %s %d %s net → %d %s charged',
$key, $term, $label,
AddonPrices::termNetCents($monthly, $term), $currency,
AddonPrices::chargedCents($monthly, $term), $currency,
AddonPrices::chargedCents($monthly, $term, $treatment), $currency,
));
$created++;
@ -272,13 +244,32 @@ class SyncStripeCatalogue extends Command
continue;
}
$prices->ensure($key, $monthly, $currency, $term);
$prices->ensure($key, $monthly, $currency, $term, $treatment);
}
}
}
return $created;
}
/**
* The two customers everything on sale is mirrored for, labelled for the
* output so an operator reading a run can see which of a pair moved.
*
* Built from TaxTreatment rather than from a rate written down here: it is the
* single tax authority, and a second list of treatments is how the catalogue
* would come to sell a figure no invoice agrees with.
*
* @return array<string, TaxTreatment>
*/
private function treatments(): array
{
return [
'domestic' => TaxTreatment::domestic(),
'reverse-charge' => TaxTreatment::reverseCharge(),
];
}
/** Versions whose prices are live in Stripe, for the status line. */
public static function syncedVersions(): int
{

View File

@ -5,7 +5,9 @@ namespace App\Http\Controllers;
use App\Models\Customer;
use App\Models\Subscription;
use App\Services\Billing\PlanCatalogue;
use App\Services\Billing\PlanPrices;
use App\Services\Billing\SetupFee;
use App\Services\Billing\TaxTreatment;
use App\Services\Provisioning\HostCapacity;
use App\Services\Stripe\StripeClient;
use Illuminate\Http\RedirectResponse;
@ -42,8 +44,12 @@ class CheckoutController extends Controller
* charged is the one the catalogue holds for the version on sale right now,
* never a figure carried in the request.
*/
public function start(Request $request, StripeClient $stripe, PlanCatalogue $catalogue): RedirectResponse
{
public function start(
Request $request,
StripeClient $stripe,
PlanCatalogue $catalogue,
PlanPrices $planPrices,
): RedirectResponse {
$data = $request->validate([
'plan' => ['required', 'string', 'max:64'],
'term' => ['nullable', 'in:'.Subscription::TERM_MONTHLY.','.Subscription::TERM_YEARLY],
@ -62,11 +68,19 @@ class CheckoutController extends Controller
$term = $data['term'] ?? Subscription::TERM_MONTHLY;
$user = $request->user();
// Read once and handed to everything below that has to know who is
// buying. Which of a package's two Stripe Prices this session uses is
// decided from it, and a second lookup is a second chance to decide it
// differently.
$customer = $user === null
? null
: Customer::query()->where('email', $user->email)->first();
// Somebody who already has a running contract is not buying a second
// one by accident: changing package is a plan change, which prorates
// and keeps their data where it is. A second checkout would build them
// a second, empty cloud and bill for both.
if ($this->hasLiveContract($user?->email)) {
if ($this->hasLiveContract($customer)) {
return redirect()->route('billing')->with('status', __('checkout.already_customer'));
}
@ -89,26 +103,43 @@ class CheckoutController extends Controller
return back()->withErrors(['plan' => __('checkout.plan_gone')]);
}
// A price with no Stripe id was never synced. Sending the customer on
// would open a checkout for nothing.
if ($price === null || blank($price->stripe_price_id)) {
Log::error('Checkout for a plan that is not on sale in Stripe', [
// WHO is buying decides WHICH of the package's two Stripe Prices this
// session uses: the domestic gross for everybody who is charged VAT, the
// bare net for a business in another member state whose VAT id is
// verified, because reverse charge means no VAT is owed to us at all.
// Decided by TaxTreatment and read from PlanPrices — nothing about the
// customer is examined here, or there would be two answers to one
// question and no way to tell which an invoice was written against.
$treatment = TaxTreatment::for($customer);
$priceId = $price === null ? null : $planPrices->liveFor($price, $treatment);
// A Price Stripe has never been told about. Sending the customer on would
// open a checkout for nothing — and for a reverse-charge business, refusing
// is the only safe answer even though a Price does exist: the domestic one
// would take a fifth more than they owe, which is the overcharge this whole
// rule exists to end. A refused checkout is loud, one command cures it, and
// the log line names the command.
if ($priceId === null) {
Log::error('Checkout for a plan that is not on sale in Stripe. Run stripe:sync-catalogue.', [
'plan' => $data['plan'],
'term' => $term,
'reverse_charge' => $treatment->reverseCharge,
]);
return back()->withErrors(['plan' => __('checkout.plan_gone')]);
}
// The one-off setup fee, gross, or null where the operator has set none.
// Advertised on the price sheet and on the booking page for months and
// charged to nobody; it rides along as a second, non-recurring line on
// this session, which Stripe puts on the initial invoice only.
$setup = SetupFee::checkoutLine((string) $price->currency);
// The one-off setup fee as this customer is charged it, or null where the
// operator has set none. Advertised on the price sheet and on the booking
// page for months and charged to nobody; it rides along as a second,
// non-recurring line on this session, which Stripe puts on the initial
// invoice only. Subject to the same rule as the package: a reverse-charge
// business pays the bare net of it.
$setup = SetupFee::checkoutLine((string) $price->currency, $treatment);
try {
$url = $stripe->createCheckoutSession(
priceId: (string) $price->stripe_price_id,
priceId: $priceId,
successUrl: route('checkout.done'),
cancelUrl: route('order'),
// Exactly the keys StripeWebhookController reads back. The
@ -157,15 +188,9 @@ class CheckoutController extends Controller
return redirect()->route('dashboard')->with('status', __('checkout.thanks'));
}
/** Does this address already have a contract that is being billed? */
private function hasLiveContract(?string $email): bool
/** Does this customer already have a contract that is being billed? */
private function hasLiveContract(?Customer $customer): bool
{
if ($email === null) {
return false;
}
$customer = Customer::query()->where('email', $email)->first();
return $customer !== null && Subscription::query()
->where('customer_id', $customer->id)
->whereIn('status', ['active', 'past_due'])

View File

@ -217,7 +217,7 @@ class LandingController extends Controller
'label' => rtrim(rtrim(number_format($tax, 1, ',', '.'), '0'), ','),
],
'setup' => $setupNet === 0 ? null : [
'gross' => $this->money(SetupFee::chargedCents(), Subscription::catalogueCurrency()),
'gross' => $this->money(SetupFee::chargedCents(TaxTreatment::domestic()), Subscription::catalogueCurrency()),
'net' => $this->money($setupNet, Subscription::catalogueCurrency()),
],
'baseline' => $this->baseline($plans),
@ -562,23 +562,23 @@ class LandingController extends Controller
* "179,00".
*/
/**
* A net figure as it is actually charged.
* A net figure as the ordinary customer is charged it.
*
* Handed straight to TaxTreatment rather than worked out here. It is the same
* call the Stripe catalogue makes when it decides what a Price takes and the
* same one an invoice makes when it decides what a document totals to, so the
* figure on this page, the figure on the card statement and the figure on the
* document cannot drift apart. It used to be a multiplication of its own,
* which is a second source of the same number and therefore a second answer
* waiting to happen.
* call the domestic Stripe Price is formed with, so the figure on this page and
* the figure on the card statement cannot drift apart. It used to be a
* multiplication of its own, which is a second source of the same number and
* therefore a second answer waiting to happen.
*
* Cross-border reverse charge is deliberately not modelled here, and it makes
* no difference to the price: a reverse-charge business is charged this
* figure like everybody else and their invoice states all of it as net.
* This page is read by visitors nobody has met, so the domestic gross is the
* only honest figure to put on it. A business in another member state with a
* verified VAT id is charged less the bare net, no VAT owed to us at all
* and learns that on the pages that know who they are, not here, where
* advertising it would quote every private visitor a price they cannot have.
*/
private function gross(int $netCents): int
{
return TaxTreatment::chargedCents($netCents);
return TaxTreatment::advertisedCents($netCents);
}
private function money(int $cents, string $currency): string

View File

@ -47,11 +47,22 @@ class Provisioning extends Component
return;
}
// Move the subject out of its error state so the console reflects the retry.
// Move the subject out of its error state so the console reflects the
// retry — but only when the run being retried is the one that BUILDS it.
//
// A maintenance pipeline shares the subject (see the `address`, `restart`,
// `storage`, `quota` and `plan-change` entries in config/provisioning.php)
// and only `customer` ever puts the order back to `active`, in
// CompleteProvisioning. So retrying a failed address run used to leave a
// live, paying customer's order reading `provisioning` for ever: nothing
// in the product would move it again, the customer saw nothing wrong, and
// the console's view of them became a lie. The same reasoning RunRunner's
// failRun() applies on the way down — a maintenance failure must not
// condemn what it maintains — applies on the way back up.
$subject = $run->subject;
if ($subject instanceof Host) {
$subject->update(['status' => 'onboarding']);
} elseif ($subject instanceof Order) {
} elseif ($subject instanceof Order && $run->pipeline === $subject->provisioningPipeline()) {
$subject->update(['status' => 'provisioning']);
Instance::query()->where('order_id', $subject->id)->where('status', 'failed')->update(['status' => 'provisioning']);
}

View File

@ -6,6 +6,7 @@ use App\Models\Customer;
use App\Models\Subscription;
use App\Services\Billing\PlanCatalogue;
use App\Services\Billing\SetupFee;
use App\Services\Billing\TaxTreatment;
use App\Services\Provisioning\HostCapacity;
use App\Support\CompanyProfile;
use Illuminate\Support\Facades\Auth;
@ -20,11 +21,14 @@ use Throwable;
* the same working day. That is an afternoon of somebody's time per customer,
* for a product whose whole point is that the machine does the work.
*
* The prices here are the catalogue's, gross, exactly as the public sheet shows
* them: a customer who was quoted 58,80 must not meet 49 on the page where
* they press the button, nor the other way round. The delivery promise is the
* estate's own answer (HostCapacity), so nobody is told "sofort" while a server
* still has to be bought.
* The prices here are the catalogue's at the till, exactly as the public sheet
* shows them: a customer who was quoted 58,80 must not meet 49 on the page
* where they press the button, nor the other way round. This page differs from the
* public sheet in one respect, and it is allowed to because it knows who is
* reading it: a business in another member state with a verified VAT id is quoted
* and charged the bare net, because reverse charge means no VAT is owed to us
* at all. The delivery promise is the estate's own answer (HostCapacity), so
* nobody is told "sofort" while a server still has to be bought.
*
* Nothing is charged here. The button opens a Stripe hosted checkout; the
* purchase becomes real on the webhook, which is the only place allowed to
@ -46,26 +50,40 @@ class Order extends Component
->whereIn('status', ['active', 'past_due'])
->first();
// Resolved once and used for every figure on the page, so the package
// price, the setup fee and the sentence about VAT underneath them cannot
// state three different treatments of one sale. It is the same call the
// checkout makes to pick the Stripe Price, so what is shown here is what
// will be taken.
$tax = TaxTreatment::for($customer);
return view('livewire.order', [
'plans' => $this->plans(),
'plans' => $this->plans($tax),
'tax' => $tax,
'contract' => $contract,
'vat' => rtrim(rtrim(number_format(CompanyProfile::taxRate(), 1, ',', '.'), '0'), ','),
// Gross, like the package price above it and like the public sheet,
// which quotes "Einrichtung kostet einmalig 118,80 €". This page was
// quoting the NET figure under the same sentence, so the site said
// 118,80 and the page with the button on it said 99,00 — and the
// checkout now charges the gross of it, which is what a visitor was
// shown out there.
'setup' => SetupFee::chargedCents(),
// At the till, like the package price above it and like the public
// sheet, which quotes "Einrichtung kostet einmalig 118,80 €". This page
// was quoting the NET figure under the same sentence, so the site said
// 118,80 and the page with the button on it said 99,00. Through the
// same treatment as the package, because a fee is a supply like any
// other and cannot be taxed differently from the thing it sets up.
'setup' => SetupFee::chargedCents($tax),
]);
}
/**
* The packages on sale, in the words and figures the public sheet uses.
* The packages on sale, in the words and figures the public sheet uses
* priced for whoever is reading them.
*
* `gross` is what this customer's card would be charged, formed by
* TaxTreatment and not by a multiplication of its own. It used to be the
* latter, which is a second source for a number the checkout also forms, and
* therefore a second answer waiting to happen.
*
* @return array<int, array<string, mixed>>
*/
private function plans(): array
private function plans(TaxTreatment $tax): array
{
try {
$sellable = app(PlanCatalogue::class)->sellable();
@ -78,7 +96,6 @@ class Order extends Component
}
$capacity = app(HostCapacity::class);
$rate = CompanyProfile::taxRate();
$plans = [];
foreach ($sellable as $key => $plan) {
@ -88,7 +105,7 @@ class Order extends Component
'key' => $key,
'name' => (string) $plan['name'],
'audience' => (string) ($plan['audience'] ?? ''),
'gross' => (int) round($net * (1 + $rate / 100)),
'gross' => $tax->chargeCents($net),
'net' => $net,
'currency' => (string) $plan['currency'],
'quota_gb' => (int) $plan['quota_gb'],

View File

@ -21,7 +21,7 @@ class Instance extends Model
'customer_id', 'order_id', 'host_id', 'vmid', 'guest_ip', 'plan', 'quota_gb', 'quota_applied_gb', 'traffic_addons', 'disk_gb',
'ram_mb', 'cores', 'restart_required_since',
'subdomain', 'custom_domain', 'nc_admin_ref', 'admin_password', 'credentials_acknowledged_at',
'route_written', 'routed_hostnames', 'cert_ok', 'status', 'cancel_requested_at', 'service_ends_at',
'route_written', 'routed_hostnames', 'routed_backend', 'cert_ok', 'status', 'cancel_requested_at', 'service_ends_at',
'domain_token', 'domain_verified_at', 'domain_cert_ok', 'domain_checked_at', 'domain_error', 'domain_failures',
];

View File

@ -89,15 +89,17 @@ class Order extends Model implements ProvisioningSubject
}
/**
* What this order actually cost the customer, tax included.
* What this order actually cost the customer, whatever tax was in it.
*
* `amount_cents` means two things, and this is the one place that resolves
* which. An order created from a Stripe checkout carries the session's
* `amount_total` the gross that was taken from the card, and the catalogue
* is pushed to Stripe gross, so it already includes the VAT. An order that
* never went through a checkout (a grant, a line in the portal cart) carries
* the catalogue's NET figure, and what a checkout would have taken for it is
* that figure at the till.
* `amount_total` what was actually taken from the card, whichever of a
* package's two Prices this customer was sold on, so it needs nothing added to
* it. An order that never went through a checkout (a grant, a line in the
* portal cart) carries the catalogue's NET figure, and what a checkout would
* have taken for it is that figure through this customer's own treatment: the
* domestic gross where VAT is charged, the bare net for a reverse-charge
* business.
*
* Keyed on `stripe_event_id` for exactly the reason OpenSubscription keys the
* register's charged amount on it, and not on the amount being non-zero: a
@ -107,7 +109,7 @@ class Order extends Model implements ProvisioningSubject
{
return $this->stripe_event_id !== null
? (int) $this->amount_cents
: TaxTreatment::chargedCents((int) $this->amount_cents);
: $this->taxTreatment()->chargeCents((int) $this->amount_cents);
}
/**

View File

@ -44,6 +44,27 @@ class ProvisioningRun extends Model
return $this->morphTo();
}
/**
* Runs that have not finished one way or the other.
*
* The four statuses that mean "this will do more work" were written out at
* four call sites that ask the same question and a fifth status added to the
* list above would have had to be remembered at every one of them. What the
* callers do with the answer differs, and App\Provisioning\WorkInFlight is
* where the difference is explained.
*
* @param \Illuminate\Database\Eloquent\Builder<self> $query
*/
public function scopeInFlight($query): void
{
$query->whereIn('status', [
self::STATUS_PENDING,
self::STATUS_RUNNING,
self::STATUS_WAITING,
self::STATUS_PAUSED,
]);
}
public function events(): HasMany
{
return $this->hasMany(ProvisioningStepEvent::class, 'run_id');

View File

@ -13,10 +13,17 @@ use Illuminate\Database\Eloquent\Model;
* Price: a booking is frozen at the price it was booked at, and a Stripe Price
* cannot be edited.
*
* `amount_cents` is what the Price CHARGES, which is gross; `net_cents` is the
* catalogue figure it was formed from, which is what a booking is frozen at. A
* Price superseded by a rate change is archived rather than deleted, because a
* contract may still be billing on it.
* `amount_cents` is what the Price CHARGES; `net_cents` is the catalogue figure
* it was formed from, which is what a booking is frozen at. A Price superseded by
* a rate change is archived rather than deleted, because a contract may still be
* billing on it.
*
* `reverse_charge` is the other half of a module's identity, and it is why there
* are four Prices per module and not two. A customer who is charged VAT pays the
* gross of the catalogue figure; a business in another member state with a
* verified VAT id owes the bare net and is charged exactly that, so for them
* `amount_cents` and `net_cents` are the same number. Which of the two a booking
* bills on is TaxTreatment's decision see App\Services\Billing\AddonPrices.
*/
class StripeAddonPrice extends Model
{
@ -27,6 +34,7 @@ class StripeAddonPrice extends Model
return [
'amount_cents' => 'integer',
'net_cents' => 'integer',
'reverse_charge' => 'boolean',
'archived_at' => 'datetime',
];
}

View File

@ -8,12 +8,25 @@ use Illuminate\Database\Eloquent\Relations\BelongsTo;
/**
* One Stripe Price a priced catalogue row has had, live or superseded.
*
* `plan_prices.stripe_price_id` points at the one that is on sale; this is the
* history behind it, and it exists because a Stripe Price cannot be edited. When
* the amount a row is charged at changes the move from net to gross, or a
* change to the VAT rate a new Price is minted, this row is archived, and the
* old Price stays live in Stripe for as long as a contract is still billing on
* it. See the migration for why that history has to be readable.
* Two of them are live at any moment, not one: `reverse_charge` says whether a
* row is the DOMESTIC GROSS Price, which is what the website quotes and what
* everybody who is charged VAT pays, or the BARE NET one, which is the whole of
* what a business in another member state with a verified VAT id owes. Which of
* the two a checkout uses is TaxTreatment's decision see
* App\Services\Billing\PlanPrices.
*
* `plan_prices.stripe_price_id` points at the domestic one, because that is the
* ordinary sale and it must have a single source; the net one is only ever read
* from here. And because a Price id is unique in this table, joining it on
* `subscriptions.stripe_price_id` is what says which of the two a running
* contract is actually billed on.
*
* The rest is history, and it exists because a Stripe Price cannot be edited.
* When the amount a row is charged at changes a change to the VAT rate moves
* the gross Price and leaves the net one alone a new Price is minted, the old
* row is archived, and the old Price stays in Stripe for as long as a contract is
* still billing on it. See the migrations for why that history has to be
* readable.
*/
class StripePlanPrice extends Model
{
@ -23,6 +36,7 @@ class StripePlanPrice extends Model
{
return [
'charged_cents' => 'integer',
'reverse_charge' => 'boolean',
'archived_at' => 'datetime',
];
}

View File

@ -5,6 +5,7 @@ namespace App\Provisioning\Jobs;
use App\Models\Instance;
use App\Services\Proxmox\ProxmoxClient;
use App\Services\Wireguard\ConfigHandoff;
use App\Support\NextcloudOcc;
use App\Support\ProvisioningSettings;
use Illuminate\Bus\Queueable;
use Illuminate\Contracts\Queue\ShouldQueue;
@ -69,9 +70,10 @@ class IssueInstanceAdminAccess implements ShouldQueue
$result = $proxmox->forHost($instance->host)->guestExec(
$instance->host->node ?? 'pve',
(int) $instance->vmid,
'cd /opt/nextcloud && OC_PASS='.escapeshellarg($password).
' docker compose exec -T -e OC_PASS app php occ user:resetpassword --password-from-env '.
escapeshellarg($username),
NextcloudOcc::command(
'user:resetpassword --password-from-env '.escapeshellarg($username),
['OC_PASS' => $password],
),
);
} catch (Throwable $e) {
// An unreachable Proxmox or guest agent throws rather than

View File

@ -28,6 +28,41 @@ class PipelineRegistry
return count($this->stepsFor($pipeline));
}
/**
* Will a run of `$pipeline` standing at `$fromStep` still carry out every
* step of `$work`?
*
* The question a caller about to start a `$work` run has to ask about the run
* already in flight, and the reason it is asked step by step rather than by
* pipeline name: `plan-change` contains the whole of `address` and the whole
* of `storage`, `quota` contains only the last third of `storage`, and
* `restart` contains neither. A name comparison would have to be kept in step
* with config/provisioning.php by hand; this reads the same lists the runner
* executes.
*
* EVERY step, not any: the three steps of a storage change are ordered
* because a quota Nextcloud enforces over a filesystem that never grew is a
* promise of space the machine does not have. A run that would apply the new
* quota and nothing else does not cover the work it delivers half of it,
* which is worse than delivering none.
*
* Steps at `$fromStep` itself count as still to come. The step there is
* either about to run or running, and every step reads the machine's state
* when it gets there rather than when the run was created.
*/
public function stillCovers(string $pipeline, int $fromStep, string $work): bool
{
$remaining = array_slice($this->stepsFor($pipeline), max($fromStep, 0));
foreach ($this->stepsFor($work) as $step) {
if (! in_array($step, $remaining, true)) {
return false;
}
}
return true;
}
public function resolve(string $pipeline, int $index): ProvisioningStep
{
$steps = $this->stepsFor($pipeline);

View File

@ -66,8 +66,18 @@ class RunRunner
}
// started_at marks when the current step began (drives the timeout).
//
// A PENDING run is starting a step now, whatever it says: either it has
// never run, or something put it back to the beginning — see
// StartCustomerProvisioning::reviveRunStrandedWithoutAContract(), which
// resets the status, the step, the attempt and the error. `??=` here made
// the second half of this condition dead code, so a revived run kept the
// started_at of the attempt that failed hours or days earlier: its very
// first pass was ruled timed out before the step body ran, which spent one
// of five attempts and wrote 'step timed out' into the event log for a run
// that had not started yet.
if ($run->started_at === null || $run->status === ProvisioningRun::STATUS_PENDING) {
$run->started_at ??= now();
$run->started_at = now();
}
$run->status = ProvisioningRun::STATUS_RUNNING;
$run->save();

View File

@ -6,6 +6,7 @@ use App\Models\ProvisioningRun;
use App\Provisioning\StepResult;
use App\Services\Billing\StorageAllowance;
use App\Services\Proxmox\ProxmoxClient;
use App\Support\NextcloudOcc;
/**
* The storage a plan change actually delivers.
@ -61,11 +62,10 @@ class ApplyStorageQuota extends CustomerStep
}
$pve = $this->pve->forHost($instance->host);
$occ = 'cd /opt/nextcloud && docker compose exec -T app php occ ';
// Idempotent, like every other occ call in this pipeline: writing the
// same value again is a no-op and exits 0.
$this->guest($pve, $run, $occ.'config:app:set files default_quota --value='.escapeshellarg($quota.' GB'));
$this->guest($pve, $run, NextcloudOcc::command('config:app:set files default_quota --value='.escapeshellarg($quota.' GB')));
// Recorded only AFTER the guest accepted it — guest() throws on a
// non-zero exit, so nothing below runs for a call that failed. Without

View File

@ -75,14 +75,30 @@ class ConfigureDnsAndTls extends CustomerStep
// written. `route_written` alone would short-circuit exactly the case
// this step is re-run for — an address that has changed — and the
// customer's domain would be announced but never routed, or stay routed
// after it was withdrawn. Comparing the hostname list keeps the retry
// after it was withdrawn. Comparing what was written keeps the retry
// cheap (a second attempt at the same address writes nothing) without
// making a re-apply a no-op.
if (! $instance->route_written || $instance->routed_hostnames !== $hostnames) {
//
// BOTH halves of the file are compared, because a router names a backend
// as well as a set of hostnames, and the backend is a DHCP address. The
// hostname list on its own said "nothing has changed" for a machine that
// had moved: cloud-init gives every guest `ip=dhcp`, a lease expiry or a
// cold boot can put it somewhere else, and the file kept pointing at where
// it used to be — a 502 on the customer's own cloud that nothing would
// ever rewrite. ConfigureNetwork re-reads that address on a restart; this
// is what makes the router follow it.
$trafficHost = $host->wg_ip ?? $host->public_ip;
$backend = $instance->guest_ip ?: $host->public_ip;
if (! $instance->route_written
|| $instance->routed_hostnames !== $hostnames
|| $instance->routed_backend !== $backend) {
$this->traefik->write($trafficHost, $instance->subdomain, $hostnames, $backend);
$instance->update(['route_written' => true, 'routed_hostnames' => $hostnames]);
$instance->update([
'route_written' => true,
'routed_hostnames' => $hostnames,
'routed_backend' => $backend,
]);
}
// TLS via HTTP-01 — poll until the certificate is served.

View File

@ -2,6 +2,8 @@
namespace App\Provisioning\Steps\Customer;
use App\Actions\ReapplyInstanceAddress;
use App\Models\Instance;
use App\Models\ProvisioningRun;
use App\Provisioning\StepResult;
use App\Services\Proxmox\ProxmoxClient;
@ -43,6 +45,8 @@ class ConfigureNetwork extends CustomerStep
}
$instance->update(['guest_ip' => $guestIp]);
$this->repairAddressIfTheGuestMoved($run, $instance, $guestIp);
// Allow HTTP/HTTPS, deny everything else (management stays on the tunnel).
$pve->applyFirewall($node, $vmid, [
['type' => 'in', 'action' => 'ACCEPT', 'proto' => 'tcp', 'dport' => '80'],
@ -51,4 +55,44 @@ class ConfigureNetwork extends CustomerStep
return StepResult::advance();
}
/**
* The guest came up somewhere else put its address back together.
*
* cloud-init hands every guest `ip=dhcp`, so the address is a lease and not a
* property of the machine: it can move on an expiry or a cold boot, and this
* step is the only thing in the product that ever reads it. The router on the
* serving host names it as the backend, so a move that nobody follows up is a
* customer's cloud answering 502 from then on, for good.
*
* The follow-up is an address run, because writing that file is the address
* pipeline's job and not this step's one place writes the router, and this
* one only knows that it is now wrong. That run is allowed to start beside
* this one because the guard asks whether the run in flight will carry out the
* address steps rather than whether anything at all is running, and a restart
* carries neither of them (App\Provisioning\WorkInFlight).
*
* Only on a CHANGE from an address we already had. A first build has no
* previous address to have moved from and its instance is not live yet, so
* the re-apply would decline anyway and a retry of this step reads the
* address it just wrote, so it asks once.
*/
private function repairAddressIfTheGuestMoved(ProvisioningRun $run, Instance $instance, string $guestIp): void
{
$routed = $instance->routed_backend;
if ($routed === null || $routed === $guestIp) {
return;
}
$run->events()->create([
'step' => $this->key(),
'attempt' => $run->attempt,
'outcome' => 'info',
'message' => "Die Maschine ist mit einer neuen Adresse zurückgekommen ({$routed}{$guestIp}). "
.'Die Weiterleitung wird neu geschrieben.',
]);
app(ReapplyInstanceAddress::class)($instance->fresh());
}
}

View File

@ -5,6 +5,7 @@ namespace App\Provisioning\Steps\Customer;
use App\Models\ProvisioningRun;
use App\Provisioning\StepResult;
use App\Services\Proxmox\ProxmoxClient;
use App\Support\NextcloudOcc;
use App\Support\ProvisioningSettings;
class ConfigureNextcloud extends CustomerStep
@ -27,10 +28,9 @@ class ConfigureNextcloud extends CustomerStep
$pve = $this->pve->forHost($instance->host);
$fqdn = $instance->subdomain.'.'.ProvisioningSettings::dnsZone();
$occ = 'cd /opt/nextcloud && docker compose exec -T app php occ ';
// Idempotent occ calls (setting the same values is a no-op).
$this->guest($pve, $run, $occ.'config:system:set trusted_domains 1 --value='.escapeshellarg($fqdn));
$this->guest($pve, $run, NextcloudOcc::command('config:system:set trusted_domains 1 --value='.escapeshellarg($fqdn)));
// Only a VERIFIED domain goes into trusted_domains. Nextcloud will
// answer for anything listed here, so an unproven hostname added at
// provisioning time would serve this customer's files to whoever
@ -45,12 +45,12 @@ class ConfigureNextcloud extends CustomerStep
// exits 0 when the key is already absent, so the step stays idempotent
// for the overwhelmingly common case of an instance that never had one.
if ($instance->domainIsVerified()) {
$this->guest($pve, $run, $occ.'config:system:set trusted_domains 2 --value='.escapeshellarg($instance->custom_domain));
$this->guest($pve, $run, NextcloudOcc::command('config:system:set trusted_domains 2 --value='.escapeshellarg($instance->custom_domain)));
} else {
$this->guest($pve, $run, $occ.'config:system:delete trusted_domains 2');
$this->guest($pve, $run, NextcloudOcc::command('config:system:delete trusted_domains 2'));
}
$this->guest($pve, $run, $occ.'background:cron');
$this->guest($pve, $run, $occ.'config:system:set default_phone_region --value=DE');
$this->guest($pve, $run, NextcloudOcc::command('background:cron'));
$this->guest($pve, $run, NextcloudOcc::command('config:system:set default_phone_region --value=DE'));
return StepResult::advance();
}

View File

@ -5,6 +5,7 @@ namespace App\Provisioning\Steps\Customer;
use App\Models\ProvisioningRun;
use App\Provisioning\StepResult;
use App\Services\Proxmox\ProxmoxClient;
use App\Support\NextcloudOcc;
use Illuminate\Support\Facades\Crypt;
use Illuminate\Support\Str;
@ -36,21 +37,18 @@ class CreateCustomerAdmin extends CustomerStep
$username = 'admin';
$password = Str::random(20);
$occ = 'cd /opt/nextcloud && docker compose exec -T';
// Retry-safe: if a prior crashed attempt already created the user, reset
// its password instead of re-running user:add (which Nextcloud rejects).
$exists = (int) ($pve->guestExec($node, $vmid, $occ.' app php occ user:info '.escapeshellarg($username))['exitcode'] ?? 1) === 0;
$exists = (int) ($pve->guestExec($node, $vmid, NextcloudOcc::command('user:info '.escapeshellarg($username)))['exitcode'] ?? 1) === 0;
$action = $exists
? 'user:resetpassword --password-from-env '.escapeshellarg($username)
: 'user:add --password-from-env --group=admin '.escapeshellarg($username);
// Pass the password via env (OC_PASS) on the docker invocation itself —
// an assignment before `cd` would not survive the `&&`.
$this->guest($pve, $run,
'cd /opt/nextcloud && OC_PASS='.escapeshellarg($password).
' docker compose exec -T -e OC_PASS app php occ '.$action);
// The password travels as OC_PASS rather than as an argument, so it never
// shows up in the guest's process list — see NextcloudOcc::command().
$this->guest($pve, $run, NextcloudOcc::command($action, ['OC_PASS' => $password]));
// Persist only the username (encrypted ref); hand the password to step 15
// encrypted-in-context for delivery, then it is scrubbed. Never plaintext.

View File

@ -10,6 +10,9 @@ use Illuminate\Support\Str;
class DeployApplicationStack extends CustomerStep
{
/** How long the stack has to come up and report itself installed. */
private const READY_DEADLINE = 1080;
public function __construct(private ProxmoxClient $pve) {}
public function key(): string
@ -19,6 +22,7 @@ class DeployApplicationStack extends CustomerStep
public function maxDuration(): int
{
// Above READY_DEADLINE, so the step's own named failure fires first.
return 1200;
}
@ -47,11 +51,29 @@ class DeployApplicationStack extends CustomerStep
$run->mergeContext(['stack_deployed' => true]);
}
// Poll until Nextcloud answers (poll doesn't consume the retry budget).
// Poll until Nextcloud is INSTALLED, not merely answering (poll doesn't
// consume the retry budget).
//
// `curl -sf` alone was the whole readiness test, and status.php answers
// 200 on an instance that has never been installed — the body then reads
// {"installed":false,…}. So the run marched on into `occ config:system:set`
// and `occ user:add`, neither of which is even REGISTERED as a command on
// an uninstalled Nextcloud, and the failure surfaced three steps later as
// an unexplained non-zero exit inside the guest. The flag is the one thing
// status.php says that distinguishes a web server from a Nextcloud.
$health = $pve->guestExec($node, $vmid,
'curl -sf http://localhost/status.php >/dev/null 2>&1 && echo ok || echo wait');
'curl -sf http://localhost/status.php 2>/dev/null'
.' | grep -qE \'"installed"[[:space:]]*:[[:space:]]*true\' && echo ok || echo wait');
if (trim($health['out-data'] ?? '') !== 'ok') {
return StepResult::poll(15, 'waiting for application stack');
// Its own deadline, below maxDuration, so what an operator reads is
// what happened rather than 'step timed out'. Long: the first boot
// pulls the images, and a slow mirror is not a fault.
if ($run->started_at !== null && $run->started_at->copy()->addSeconds(self::READY_DEADLINE)->isPast()) {
return StepResult::fail('nextcloud_not_installed');
}
return StepResult::poll(15, 'waiting for Nextcloud to report itself installed');
}
$run->forgetContext('db_password'); // scrub once the stack is up

View File

@ -4,10 +4,10 @@ namespace App\Provisioning\Steps\Customer;
use App\Models\Host;
use App\Models\Instance;
use App\Services\Provisioning\HostCapacity;
use App\Models\Order;
use App\Models\ProvisioningRun;
use App\Provisioning\StepResult;
use App\Services\Provisioning\HostCapacity;
use Illuminate\Support\Facades\Cache;
use Illuminate\Support\Facades\Log;
use Illuminate\Support\Str;
@ -25,9 +25,33 @@ class ReserveResources extends CustomerStep
return 'reserve_resources';
}
/**
* The whole park, not the second the body takes.
*
* RunRunner measures this from `started_at` and deliberately does NOT reset
* `started_at` on a poll that is what lets a polling step's own deadline
* accumulate across re-entries instead of restarting every time. So for a step
* that polls, maxDuration is its TOTAL budget, and it has to sit above the
* step's own deadline. Every other polling step in the repo already does:
* ConfigureDnsAndTls waits 840 s for a certificate inside 960 s, ConfigureNetwork
* 90 s inside 150 s.
*
* At 60 s against a 120 s poll this one could not survive its own first wait.
* The run came back due, the runner saw `started_at` two minutes old, turned that
* into a timeout retry before the body ran, and five timeouts later failRun()
* marked a paid order and its instance failed and released the instance about
* six minutes for a wait that promises fourteen days. Everything built on that
* promise (the console's capacity queue, HostCapacity::parked(), queueDemand(),
* the operator going and buying a machine) was unreachable code, and a customer
* who paid when nothing had room got an incident instead of a delivery date.
*
* One poll interval of headroom, so the step's own `no_capacity` fail below is
* always what fires. That failure names what the operator has to do; a timeout
* names nothing.
*/
public function maxDuration(): int
{
return 60;
return self::PARK_DAYS * 86400 + self::PARK_POLL_SECONDS;
}
public function execute(ProvisioningRun $run): StepResult

View File

@ -7,15 +7,41 @@ use App\Provisioning\StepResult;
use App\Services\Monitoring\MonitoringClient;
use App\Services\Proxmox\ProxmoxClient;
use App\Services\Traefik\TraefikWriter;
use App\Support\NextcloudOcc;
use App\Support\ProvisioningSettings;
/**
* Gate: nothing grants the customer access until every check here actually
* passes the cert is served, Nextcloud reports healthy, the admin account is
* usable, a backup exists, and monitoring is green.
*
* A check that says no is asked again before it condemns anything. Every one of
* them is a PROBE an HTTPS request from this VM, a command inside the guest
* and each has its own ways of answering no without anything being wrong:
* TraefikWriter::certReachable() turns a connect timeout, a TLS handshake that
* came in late and a DNS blip into exactly the same `false` as a missing
* certificate, and a guest agent call fails outright while the guest is busy.
* This step used to return fail() on the first no, which bypasses the retry
* budget entirely so one bad second on our side ended a fully built, certified,
* working Nextcloud as a `failed` order with the instance released, which is the
* worst outcome the pipeline can produce and the one it produced most easily.
*
* So the probes retry, like the rest of the pipeline, and the run still fails for
* good once the budget is gone: five noes in a row is not a blip. What stays
* terminal is the two facts a retry cannot change state we wrote down
* ourselves, earlier in this same run.
*/
class RunAcceptanceChecks extends CustomerStep
{
/**
* How long before a probe that answered no is asked again.
*
* Long enough for a warming Traefik router or a busy guest to settle, short
* enough that five attempts still finish inside the delivery the customer is
* watching.
*/
private const PROBE_RETRY_SECONDS = 30;
public function __construct(
private ProxmoxClient $pve,
private TraefikWriter $traefik,
@ -39,29 +65,40 @@ class RunAcceptanceChecks extends CustomerStep
$vmid = (int) $run->context('vmid');
$fqdn = $instance->subdomain.'.'.ProvisioningSettings::dnsZone();
$pve = $this->pve->forHost($instance->host);
$occ = 'cd /opt/nextcloud && docker compose exec -T app php occ ';
// TLS + routing actually serving.
if (! $instance->cert_ok || ! $this->traefik->certReachable($fqdn)) {
// Terminal, and the only cert case that is: `cert_ok` is written by
// ConfigureDnsAndTls, which fails the run itself if the platform
// certificate never appears. Standing here without it means the pipeline
// reached acceptance in a state it cannot reach, and no number of
// attempts fixes that.
if (! $instance->cert_ok) {
return StepResult::fail('acceptance_failed:cert');
}
// TLS + routing actually serving, asked of the outside world.
if (! $this->traefik->certReachable($fqdn)) {
return $this->askAgain('acceptance_failed:cert');
}
// Nextcloud installed, not in maintenance, DB reachable.
$status = $pve->guestExec($node, $vmid, $occ.'status --output=json');
$status = $pve->guestExec($node, $vmid, NextcloudOcc::command('status --output=json'));
$health = json_decode($status['out-data'] ?? '', true) ?: [];
if ((int) ($status['exitcode'] ?? 1) !== 0
|| ($health['installed'] ?? false) !== true
|| ($health['maintenance'] ?? false) !== false) {
return StepResult::fail('acceptance_failed:nextcloud');
return $this->askAgain('acceptance_failed:nextcloud');
}
// The admin account exists and is queryable.
$admin = $pve->guestExec($node, $vmid, $occ.'user:info '.escapeshellarg((string) $instance->nc_admin_ref));
$admin = $pve->guestExec($node, $vmid, NextcloudOcc::command('user:info '.escapeshellarg((string) $instance->nc_admin_ref)));
if ((int) ($admin['exitcode'] ?? 1) !== 0) {
return StepResult::fail('acceptance_failed:admin');
return $this->askAgain('acceptance_failed:admin');
}
// A backup job is registered.
// Terminal for the same reason as cert_ok above: this is a breadcrumb THIS
// run wrote at RegisterBackup, read out of our own database. If it is not
// there, no backup job was registered, and asking again five times only
// delays saying so.
if (! $this->hasResource($run, 'backup_job_id')) {
return StepResult::fail('acceptance_failed:backup');
}
@ -91,4 +128,19 @@ class RunAcceptanceChecks extends CustomerStep
return StepResult::advance();
}
/**
* Ask the probes again, and keep the reason for the day they run out.
*
* retry(), not poll(): a poll does not consume the budget, so a genuinely
* broken instance would sit here re-probing until the step's own maxDuration
* turned it into a nameless timeout. A retry is bounded by max_attempts and
* RunRunner hands this exact reason to failRun() when the last one is spent,
* so an operator still reads `acceptance_failed:nextcloud` rather than
* "step timed out".
*/
private function askAgain(string $reason): StepResult
{
return StepResult::retry(self::PROBE_RETRY_SECONDS, $reason);
}
}

View File

@ -0,0 +1,75 @@
<?php
namespace App\Provisioning;
use App\Models\Order;
use App\Models\ProvisioningRun;
use InvalidArgumentException;
/**
* Is the work somebody is about to start already going to happen anyway?
*
* Every customer pipeline shares one subject the instance's own purchase Order
* so an action that wants to start a run has to look at what is already in
* flight against it. Two runs writing one router, or resizing one disk from two
* different figures, is the failure this guards.
*
* It used to ask whether ANYTHING was in flight, and stand aside if it was, on
* the reasoning that whatever is running reads the current state when it gets
* there. That is true only of a run that actually contains the steps in question.
* It was false for `restart`, which has no address steps, and for `address`, which
* has no storage steps with two consequences that both cost the customer
* something real:
*
* - a domain proven while a restart was running was never routed, and nothing
* looked again: the nightly proof check only re-applies an address on the
* FLIP from unproven to proven (see VerifyCustomDomains), and that flip had
* already happened.
* - a storage pack booked while an address run was going was charged every
* month and never delivered, because nothing else ever grows a disk.
*
* So the question is now what it always meant: will the run in flight carry out
* this work? Where the answer is no, the new run starts beside it. That is safe
* for the pairs it allows, and only for those: the caller has already established
* that the machine is LIVE (Instance::hasLiveMachine()), which rules out standing
* beside the build; a run whose pipeline covers the work still wins; and what is
* left writes different things a router file and a DNS record on one side, a
* disk and a filesystem on the other, with Nextcloud's config.php and its
* appconfig table apart from each other. The one genuinely awkward pair is a
* `restart`, where the guest goes away underneath the newcomer; those steps throw
* on an unreachable guest, which the runner turns into a retry, and the backoff
* outlasts a reboot.
*/
class WorkInFlight
{
public function __construct(private PipelineRegistry $registry) {}
/**
* Will a run already going against this order carry out every step of
* `$pipeline`?
*/
public function covers(Order $order, string $pipeline): bool
{
$runs = ProvisioningRun::query()
->where('subject_type', Order::class)
->where('subject_id', $order->id)
->inFlight()
->get(['pipeline', 'current_step']);
foreach ($runs as $run) {
try {
if ($this->registry->stillCovers($run->pipeline, $run->current_step, $pipeline)) {
return true;
}
} catch (InvalidArgumentException) {
// A pipeline that no longer exists (renamed in a deploy while a
// run was in flight) covers nothing. The run will fail at its
// next advance for the same reason; refusing to start the work
// over it would lose the work as well.
continue;
}
}
return false;
}
}

View File

@ -29,13 +29,18 @@ use Illuminate\Database\UniqueConstraintViolationException;
* behind years ago and refusing to bill those would be the same bug in a new
* place. Stripe's idempotency key makes the on-demand path safe to repeat.
*
* **What the Price charges is GROSS.** `amount_cents` on the row is what Stripe
* takes, and it is the catalogue's net figure with the domestic rate on it
* because the figure on the price sheet is the figure charged, for everybody.
* `net_cents` beside it is the catalogue figure it was formed from, which is what
* a booking is frozen at and what identifies the Price when the rate moves. A
* Price at the wrong figure is archived rather than edited: Stripe does not allow
* editing one, and a contract may still be billing on it.
* **What the Price charges depends on who is billed.** `amount_cents` on the row
* is what Stripe takes: the catalogue's net figure with the domestic rate on it
* for everybody who is charged VAT, because the figure on the price sheet is the
* figure charged and the bare net for a business in another member state whose
* VAT id is verified, because reverse charge means no VAT is owed to us at all
* and the net is the whole of it. So a module has TWO live Prices per interval,
* not one, and `reverse_charge` on the row says which is which.
*
* `net_cents` beside it is the catalogue figure the Price was formed from, which
* is what a booking is frozen at and what identifies the Price when the rate
* moves. A Price at the wrong figure is archived rather than edited: Stripe does
* not allow editing one, and a contract may still be billing on it.
*/
final class AddonPrices
{
@ -55,10 +60,17 @@ final class AddonPrices
return $term === Subscription::TERM_YEARLY ? $monthlyNetCents * 12 : $monthlyNetCents;
}
/** What Stripe is asked to take for a term of this module. */
public static function chargedCents(int $monthlyNetCents, string $term): int
/**
* What Stripe is asked to take for a term of this module from a customer
* treated so.
*
* The treatment is a parameter rather than the domestic one assumed, because
* a reverse-charge business owes the bare net and there is a Price of their
* own for it. Grossed ONCE, after the term is formed, for the reason above.
*/
public static function chargedCents(int $monthlyNetCents, string $term, TaxTreatment $treatment): int
{
return TaxTreatment::chargedCents(self::termNetCents($monthlyNetCents, $term));
return $treatment->chargeCents(self::termNetCents($monthlyNetCents, $term));
}
/**
@ -69,20 +81,27 @@ final class AddonPrices
* a zero-amount item on a subscription is a line on every invoice that says
* the customer owes nothing for it.
*/
public function ensure(string $addonKey, int $unitNetCents, string $currency, string $term): ?string
{
public function ensure(
string $addonKey,
int $unitNetCents,
string $currency,
string $term,
TaxTreatment $treatment,
): ?string {
if ($unitNetCents <= 0) {
return null;
}
$interval = $term === Subscription::TERM_YEARLY ? 'year' : 'month';
$currency = strtoupper($currency);
$reverseCharge = $treatment->reverseCharge;
$netCents = self::termNetCents($unitNetCents, $term);
$amount = TaxTreatment::chargedCents($netCents);
$amount = $treatment->chargeCents($netCents);
$existing = StripeAddonPrice::query()
->where('addon_key', $addonKey)
->where('reverse_charge', $reverseCharge)
->where('amount_cents', $amount)
->where('currency', $currency)
->where('interval', $interval)
@ -91,9 +110,15 @@ final class AddonPrices
if ($existing !== null) {
// Brought back rather than minted again. A rate that moves and then
// moves back — or a run interrupted between Stripe and us — must not
// leave a second Stripe Price for one figure.
// leave a second Stripe Price for one figure. Brought back at Stripe
// too where it had been archived there: a Price that is only live in
// our own table is one a checkout is refused for.
if ($existing->archived_at !== null) {
$this->stripe->activatePrice((string) $existing->stripe_price_id);
}
$existing->update(['archived_at' => null, 'net_cents' => $netCents]);
$this->archiveSuperseded($addonKey, $netCents, $currency, $interval, $amount);
$this->archiveSuperseded($addonKey, $reverseCharge, $netCents, $currency, $interval, $amount);
return (string) $existing->stripe_price_id;
}
@ -109,17 +134,27 @@ final class AddonPrices
// Read back when a Stripe invoice line has to be turned into
// wording a customer can read — see StripeInvoiceLines.
'addon' => $addonKey,
// Which of the module's two Prices this is, for anyone reading
// Stripe's own dashboard, where they would otherwise differ only
// by an amount.
'tax_treatment' => $reverseCharge ? 'reverse_charge' : 'domestic',
],
// Keyed on what the Price IS, so a crash between Stripe creating it
// and us storing its id gives back the same Price on the next
// attempt rather than a second one at the same money. The amount is
// the CHARGED one, so the move from net to gross does not replay the
// net Price the old key was minted under.
idempotencyKey: "clupilot-addon-price-{$addonKey}-{$interval}-{$amount}-{$currency}",
// net Price the old key was minted under — and the treatment is in
// there because at a rate of nought the two Prices are the same
// amount: one key would have Stripe hand the same object back for
// both, and the two rows would then share a Price, so archiving the
// gross one at the next rate change would withdraw the very Price the
// net side is still selling.
idempotencyKey: "clupilot-addon-price-{$addonKey}-{$interval}-{$amount}-{$currency}"
.($reverseCharge ? '-rc' : ''),
);
$this->remember($addonKey, $amount, $netCents, $currency, $interval, $productId, $priceId);
$this->archiveSuperseded($addonKey, $netCents, $currency, $interval, $amount);
$this->remember($addonKey, $reverseCharge, $amount, $netCents, $currency, $interval, $productId, $priceId);
$this->archiveSuperseded($addonKey, $reverseCharge, $netCents, $currency, $interval, $amount);
return $priceId;
}
@ -129,27 +164,40 @@ final class AddonPrices
*
* Asked with the MONTHLY net the booking is frozen at, because that is what
* a caller has: the charged figure is worked out here, from the one place
* that knows the rate.
* that knows the rate and from the treatment, because a reverse-charge
* business is billed on a Price of their own at the bare net.
*/
public function liveFor(string $addonKey, int $unitNetCents, string $currency, string $term): ?string
{
public function liveFor(
string $addonKey,
int $unitNetCents,
string $currency,
string $term,
TaxTreatment $treatment,
): ?string {
if ($unitNetCents <= 0) {
return null;
}
return $this->find(
$addonKey,
self::chargedCents($unitNetCents, $term),
self::chargedCents($unitNetCents, $term, $treatment),
$currency,
$term === Subscription::TERM_YEARLY ? 'year' : 'month',
$treatment->reverseCharge,
);
}
/** The live Price we already have for this charged figure, or null. */
public function find(string $addonKey, int $amountCents, string $currency, string $interval): ?string
{
public function find(
string $addonKey,
int $amountCents,
string $currency,
string $interval,
bool $reverseCharge,
): ?string {
$id = StripeAddonPrice::query()
->where('addon_key', $addonKey)
->where('reverse_charge', $reverseCharge)
->where('amount_cents', $amountCents)
->where('currency', strtoupper($currency))
->where('interval', $interval)
@ -165,8 +213,13 @@ final class AddonPrices
* Only ones formed from the SAME net: a customer grandfathered at ten euros
* keeps their own Price, and archiving that would leave their subscription
* billing on something Stripe no longer sells. What is archived here is the
* Price for today's figure at yesterday's rate the net one this whole
* change replaces.
* Price for today's figure at yesterday's rate.
*
* And only ones for the same TREATMENT. A change of rate moves the gross
* Price and leaves the net one exactly where it was, so a sweep that did not
* know the difference would archive the half it had not just replaced and
* every reverse-charge business would find their module Price withdrawn the
* next time the VAT rate moved.
*
* Archived in Stripe as well as here. The Price object stays, as Stripe
* requires, because a contract may still be billing on it until
@ -174,6 +227,7 @@ final class AddonPrices
*/
private function archiveSuperseded(
string $addonKey,
bool $reverseCharge,
int $netCents,
string $currency,
string $interval,
@ -181,6 +235,7 @@ final class AddonPrices
): void {
$superseded = StripeAddonPrice::query()
->where('addon_key', $addonKey)
->where('reverse_charge', $reverseCharge)
->where('net_cents', $netCents)
->where('currency', $currency)
->where('interval', $interval)
@ -220,6 +275,7 @@ final class AddonPrices
private function remember(
string $addonKey,
bool $reverseCharge,
int $amountCents,
int $netCents,
string $currency,
@ -230,6 +286,7 @@ final class AddonPrices
try {
StripeAddonPrice::create([
'addon_key' => $addonKey,
'reverse_charge' => $reverseCharge,
'amount_cents' => $amountCents,
'net_cents' => $netCents,
'currency' => $currency,

View File

@ -30,12 +30,14 @@ use RuntimeException;
*
* ## Two kinds of line, and why
*
* Everything sold from the catalogue is priced GROSS: the price sheet, the
* Stripe Price and the card charge are one figure. A document for such a sale
* therefore starts from what was CHARGED and divides it see
* InvoiceMath::fromCharged() because a document that rebuilds the total from a
* net figure and a rate states a sum nobody was ever charged. That was the whole
* defect: 179,00 taken and 214,80 invoiced.
* Everything sold from the catalogue is priced at the till: the price sheet, the
* Stripe Price and the card charge are one figure the domestic gross for
* everybody who is charged VAT, the bare net for a reverse-charge business, whose
* Stripe Price carries exactly that. A document for such a sale therefore starts
* from what was CHARGED and divides it see InvoiceMath::fromCharged() because
* a document that rebuilds the total from a net figure and a rate states a sum
* nobody was ever charged. That was the whole defect: 179,00 taken and
* 214,80 invoiced.
*
* A service invoice an operator types is the other kind. Nothing has been
* charged yet, the figures are what was agreed on the telephone, and they are
@ -135,10 +137,11 @@ final class IssueInvoice
*
* The line is what the cycle CHARGED. Where Stripe reported a total it is
* that total, to the cent; where it did not, it is the contract's own frozen
* net price at the till (TaxTreatment::chargedCents), which is the amount the
* Stripe Price for that contract carries. `price_cents` itself stays the
* catalogue's net figure and is not what a document totals to the customer
* was charged the gross of it.
* net price put through this customer's treatment, which is the amount the
* Stripe Price this contract is billed on carries the domestic gross where
* VAT is charged, the bare net for a reverse-charge business. `price_cents`
* itself stays the catalogue's net figure and is not by itself what a document
* totals to.
*
* **The fallback, not the ordinary path.** Modules are items on the Stripe
* subscription now, so what a cycle actually charged for is on Stripe's own
@ -180,7 +183,8 @@ final class IssueInvoice
])],
'quantity_milli' => 1000,
'unit' => '',
'unit_net_cents' => $chargedCents ?? TaxTreatment::chargedCents((int) $subscription->price_cents),
'unit_net_cents' => $chargedCents
?? TaxTreatment::for($customer)->chargeCents((int) $subscription->price_cents),
]];
return $this->issue(
@ -443,10 +447,11 @@ final class IssueInvoice
$rate = $treatment->basisPoints();
if ($linesAreCharged) {
// The charged figure is the fixed point: a reverse-charge customer
// paid the same total as everybody else, so at a rate of zero the
// whole of it becomes net rather than the document shrinking by the
// VAT that was in the price.
// The charged figure is the fixed point, whatever it was: at a rate of
// zero the whole of it becomes net rather than the document shrinking
// by a VAT the price never contained. That is what keeps the total
// equal to the money for a reverse-charge business, who is charged the
// bare net and whose document therefore states that same net at 0 %.
['lines' => $lines, 'totals' => $totals] = InvoiceMath::fromCharged($lines, $rate);
$totals += ['rate_basis_points' => $rate];
} else {

View File

@ -0,0 +1,259 @@
<?php
namespace App\Services\Billing;
use App\Models\PlanPrice;
use App\Models\StripePlanPrice;
use App\Models\Subscription;
use App\Services\Stripe\StripeClient;
/**
* The Stripe Price a package is sold on and there are two of them.
*
* A priced catalogue row is mirrored into Stripe twice: once at the DOMESTIC
* GROSS, which is what the website quotes and what everybody who is charged VAT
* pays, and once at the BARE NET, which is the whole of what a reverse-charge
* business in another member state owes. Both are live at the same time, both
* hang off the same Product, and which one a checkout uses is TaxTreatment's
* decision and nobody else's.
*
* Before this, one Price served everybody. A verified EU business was charged
* 214,80 for a 179,00 package and their document then stated the whole
* 214,80 as net at 0 % a flat 20 % surcharge with no VAT line anywhere for
* them to reclaim, on exactly the customers who read their invoices.
*
* **Where each of the two is written down.** `plan_prices.stripe_price_id` keeps
* the meaning it has always had the Price an ordinary domestic customer is sold
* on because the checkout, the plan swap and every test read it there and the
* ordinary sale must have one source. The net Price gets no column of its own:
* nothing but a reverse-charge business is ever sold on it, so it is read out of
* `stripe_plan_prices`, where `reverse_charge` says which of the two a row is.
* That register is also what answers "which of the two is this contract billed
* on?" — join it on `subscriptions.stripe_price_id`.
*
* **Minting is the sync's job and nothing else's.** A Stripe Price is immutable,
* so a changed figure means a new Price with the old one archived, and doing that
* from a web request or from a sweep over live contracts would put the catalogue
* in two hands. Everything outside stripe:sync-catalogue reads; a Price that is
* missing is reported as a catalogue that has not been synced, which is a
* sentence an operator can act on.
*/
final class PlanPrices
{
public function __construct(private readonly StripeClient $stripe) {}
/** What Stripe is asked to take for this row from a customer treated so. */
public static function chargedCents(PlanPrice $price, TaxTreatment $treatment): int
{
return $treatment->chargeCents((int) $price->amount_cents);
}
/**
* The Price a customer with this treatment is sold on, or null where the
* catalogue has never been mirrored.
*
* The domestic answer falls back to the catalogue row's own column, which is
* where it lived before this register existed and where an install that has
* not run the sync since still has it. There is deliberately NO such fallback
* for a reverse-charge business: handing them the domestic Price is the
* overcharge this whole file exists to end, and refusing the sale until the
* catalogue is synced is the one outcome that cannot take money nobody owes.
*/
public function liveFor(PlanPrice $price, TaxTreatment $treatment): ?string
{
$registered = $this->registered($price, $treatment);
if ($registered !== null) {
return (string) $registered->stripe_price_id;
}
if ($treatment->reverseCharge) {
return null;
}
return blank($price->stripe_price_id) ? null : (string) $price->stripe_price_id;
}
/**
* Is Stripe selling this row at the right figure for this treatment?
*
* Asked of the CHARGED figure and of the pointer together, which is what
* makes a second sync run free and a run that died halfway recoverable: a
* rate change leaves the register live at yesterday's amount, and a crash
* between Stripe minting the Price and us storing its id leaves the register
* right and the catalogue row empty.
*/
public function inStep(PlanPrice $price, TaxTreatment $treatment): bool
{
$registered = $this->registered($price, $treatment);
if ($registered === null) {
return false;
}
// The net Price has no pointer to be out of step with — see the class
// comment on where each of the two is written down.
return $treatment->reverseCharge
|| $price->stripe_price_id === $registered->stripe_price_id;
}
/**
* The Price this row is sold on for this treatment, created at Stripe if it
* has never been told about it.
*
* Three outcomes, decided by the charged figure and never by whether an id
* happens to be stored:
*
* - the register is live at that figure and the catalogue row points at it:
* nothing to do;
* - a Price for that figure exists but is archived, or is not the one the row
* points at a rate moved and moved back, or a run died between Stripe
* creating the Price and us storing its id: it is brought back, at Stripe
* as well as here, rather than minted a second time;
* - nothing charges it: a new Price, the pointer moved, and whatever it was
* pointing at archived after the replacement exists, never before, so a
* failure in between leaves the old Price selling rather than nothing.
*
* Null only where the family has no Product yet, which means the run that
* creates Products has not reached it.
*/
public function ensure(PlanPrice $price, TaxTreatment $treatment): ?string
{
$version = $price->version;
$family = $version?->family;
$productId = $family?->stripe_product_id;
if ($version === null || $family === null || blank($productId)) {
return null;
}
$charged = self::chargedCents($price, $treatment);
$existing = $this->registered($price, $treatment, includeArchived: true);
$priceId = $existing?->stripe_price_id;
if ($priceId === null) {
$priceId = $this->stripe->createPrice(
productId: (string) $productId,
amountCents: $charged,
currency: (string) $price->currency,
interval: $price->term === Subscription::TERM_YEARLY ? 'year' : 'month',
metadata: [
'plan_family' => $family->key,
'plan_version' => (string) $version->version,
'plan_version_id' => (string) $version->id,
'plan_price_id' => (string) $price->id,
// Read back by anything that has a Price id and needs to know
// what kind of sale it was — and by a person looking at
// Stripe's own dashboard, where two Prices on one Product
// would otherwise differ only by an amount.
'tax_treatment' => $treatment->reverseCharge ? 'reverse_charge' : 'domestic',
],
// The CHARGED amount is part of the key, so a run after a rate
// change cannot replay the Price minted at the old figure. So is
// the treatment, and it has to be: at a rate of nought the two
// Prices are the same amount, and one key would have Stripe hand
// back the same object for both — which the register, where a
// Price id is unique, would refuse to record twice.
idempotencyKey: "clupilot-price-{$price->id}-{$charged}"
.($treatment->reverseCharge ? '-rc' : ''),
);
} elseif ($existing?->archived_at !== null) {
// Unarchived at Stripe too. Bringing it back only in our own table
// would leave the catalogue pointing at a Price Stripe refuses to
// open a checkout for, which is the failure a customer meets and
// nobody else does.
$this->stripe->activatePrice($priceId);
}
StripePlanPrice::query()->updateOrCreate(
[
'plan_price_id' => $price->id,
'reverse_charge' => $treatment->reverseCharge,
'charged_cents' => $charged,
],
['stripe_price_id' => $priceId, 'archived_at' => null],
);
$this->archiveSuperseded($price, $treatment, $charged);
// The pointer is the domestic Price and only ever that. Written straight
// through the query builder: stripe_price_id is not part of what
// publication froze, and the model would otherwise have to be re-read.
if (! $treatment->reverseCharge) {
PlanPrice::query()->whereKey($price->id)->update(['stripe_price_id' => $priceId]);
}
return $priceId;
}
/** The priced catalogue row a contract is billed through, or null. */
public function rowFor(Subscription $subscription): ?PlanPrice
{
return PlanPrice::query()
->where('plan_version_id', $subscription->plan_version_id)
->where('term', $subscription->term)
->first();
}
/**
* The Price this contract should be billed on, given who is paying for it.
*
* By plan VERSION and term, because that pair is what a Stripe Price is
* resolving it by plan name would hand a grandfathered contract today's
* terms and then by the customer's own treatment, because a business whose
* registration was verified after they bought belongs on the net Price from
* that moment, and one whose registration has lapsed belongs back on the
* gross one.
*/
public function liveForSubscription(Subscription $subscription): ?string
{
$row = $this->rowFor($subscription);
return $row === null
? null
: $this->liveFor($row, TaxTreatment::for($subscription->customer));
}
/** The register's row for this figure, live unless asked otherwise. */
private function registered(
PlanPrice $price,
TaxTreatment $treatment,
bool $includeArchived = false,
): ?StripePlanPrice {
return StripePlanPrice::query()
->where('plan_price_id', $price->id)
->where('reverse_charge', $treatment->reverseCharge)
->where('charged_cents', self::chargedCents($price, $treatment))
->when(! $includeArchived, fn ($query) => $query->whereNull('archived_at'))
->first();
}
/**
* Stop offering every other Price this row was sold on to customers treated
* the same way.
*
* Within ONE treatment, which is the whole reason `reverse_charge` is part of
* the register's key: a change of rate moves the gross Price and leaves the
* net one exactly where it was, and a sweep that did not know the difference
* would archive the half it had not just replaced.
*
* Archived in Stripe as well as here. The Price object stays, as Stripe
* requires, because a contract may still be billing on it until
* stripe:reprice-subscriptions has moved it.
*/
private function archiveSuperseded(PlanPrice $price, TaxTreatment $treatment, int $keepChargedCents): void
{
$superseded = StripePlanPrice::query()
->where('plan_price_id', $price->id)
->where('reverse_charge', $treatment->reverseCharge)
->where('charged_cents', '!=', $keepChargedCents)
->whereNull('archived_at')
->get();
foreach ($superseded as $old) {
$this->stripe->archivePrice((string) $old->stripe_price_id);
$old->update(['archived_at' => now()]);
}
}
}

View File

@ -19,13 +19,19 @@ use App\Support\CompanyProfile;
* this class, so a change of rate or of fee cannot move one without moving the
* others.
*
* **The figure is GROSS, like every other price this platform charges.** The
* console asks the operator for the net amount, because that is the commercial
* number and the field says so; what a customer pays is that with the domestic
* rate on it, formed by TaxTreatment::chargedCents(), which is the single place a
* gross is formed. That is also why the fee is the same for a consumer in Vienna
* and for a reverse-charge business in Rotterdam: the price on the page is the
* price at the till, for everybody, and the invoice is where the split differs.
* **What is charged for it follows the same rule as a package.** The console asks
* the operator for the NET amount, because that is the commercial number and the
* field says so. What a customer pays is that amount put through their own
* TaxTreatment: the domestic gross for everybody who is charged VAT, so the
* figure at the till is the figure the price sheet quoted and the bare net for
* a business in another member state whose VAT id is verified, because reverse
* charge means no VAT is owed to us on the fee either. A fee is a supply like any
* other; taxing it differently from the package on the same invoice would be a
* document at two rates.
*
* The public price sheet quotes the domestic gross, because it is read by
* visitors nobody has met TaxTreatment::advertisedCents(), the same figure the
* ordinary checkout takes.
*
* Zero means there is no such fee. Every reader has to treat that as "no line at
* all" rather than "a line of nought" — a checkout that itemises nothing owed is
@ -39,12 +45,18 @@ final class SetupFee
return CompanyProfile::setupFeeCents();
}
/** What a customer is actually charged for it, tax included. */
public static function chargedCents(): int
/**
* What a customer treated so is actually charged for it.
*
* The treatment is a required argument rather than the domestic one assumed,
* so that every call site has to say whose fee it is quoting. The page that
* quotes it to a stranger passes TaxTreatment::domestic(), and says so.
*/
public static function chargedCents(TaxTreatment $treatment): int
{
$net = self::netCents();
return $net === 0 ? 0 : TaxTreatment::chargedCents($net);
return $net === 0 ? 0 : $treatment->chargeCents($net);
}
/**
@ -58,9 +70,9 @@ final class SetupFee
*
* @return array{amount_cents: int, label: string, currency: string}|null
*/
public static function checkoutLine(string $currency): ?array
public static function checkoutLine(string $currency, TaxTreatment $treatment): ?array
{
$charged = self::chargedCents();
$charged = self::chargedCents($treatment);
if ($charged === 0) {
return null;

View File

@ -29,11 +29,17 @@ use Illuminate\Support\Carbon;
* line that was charged is worse than an ugly one: the total would still be
* right and the reader would have no way to see what they had paid for.
*
* Amounts are what was CHARGED our catalogue is pushed to Stripe gross, so
* every figure here already contains the VAT. IssueInvoice divides the sum
* according to the customer's own treatment rather than adding a rate to it; a
* reverse-charge customer paid the same total as everybody else and their
* document states the whole of it as net at 0 %. See TaxTreatment.
* Amounts are what was CHARGED, which is not the same figure for everybody: our
* catalogue is pushed to Stripe twice, at the domestic gross and at the bare net,
* and the Price a contract is billed on decides which of the two its lines carry.
* IssueInvoice divides the sum according to the customer's own treatment rather
* than adding a rate to it so a domestic line is split into net and VAT, and a
* reverse-charge line, which contained no VAT to begin with, becomes net at 0 %.
* Either way the document totals to the money. See TaxTreatment.
*
* Naming a line is unaffected by there being two Prices per sellable thing: a
* Price id is unique in both registers, so it resolves to one catalogue row
* whichever of the pair it is.
*/
final class StripeInvoiceLines
{

View File

@ -39,18 +39,27 @@ use App\Support\CompanyProfile;
* off, not a guess in a config file so those customers fall back to the
* domestic rate, which over-collects rather than under-collects.
*
* ## The price at the till is the domestic gross, for everybody
* ## The price at the till is the domestic gross — except where no VAT is charged
*
* The treatment above decides how a DOCUMENT is split, and it decides nothing
* about the amount taken from the card. That amount is chargedCents(): the net
* catalogue figure with the domestic rate on it, and it is the same number for a
* consumer in Vienna and for a reverse-charge business in Rotterdam. The owner's
* rule is that the figure on the website is the figure charged, and a Stripe
* Price cannot ask who is buying it carries one amount for everyone.
* The treatment above decides how a DOCUMENT is split, and it also decides the
* amount taken from the card: chargeCents() below. For everybody who is charged
* VAT that amount is the domestic gross, and the owner's rule holds exactly
* the figure on the website is the figure charged, for a private person and for
* an Austrian business alike, because the VAT is a pass-through that a business
* reclaims as input tax.
*
* So a reverse-charge customer pays that same total and their invoice states the
* whole of it as net at 0 %. That is a real commercial consequence and it is
* deliberate: see App\Services\Billing\IssueInvoice.
* A reverse-charge sale carries no VAT at all, and there the same rule points
* the other way: the bare net figure is the whole of what is owed, so the bare
* net figure is what is charged. Charging them the gross was a flat 20 %
* surcharge with no VAT line anywhere for them to reclaim, and their document
* then stated the whole of it as net at 0 % so they self-accounted their own
* VAT on a base a fifth too large. The rule is: Austria B2B 20 %, other EU B2B
* without VAT.
*
* That is why a Stripe Price CAN ask who is buying. There are two per sellable
* thing the domestic gross and the bare net both live, and the checkout
* picks by this class. See App\Services\Billing\PlanPrices and
* App\Services\Billing\AddonPrices.
*
* Stripe's own `automatic_tax` is deliberately NOT enabled anywhere. This class
* is the single tax authority here, knowingly over-collecting on cross-border
@ -81,7 +90,7 @@ final readonly class TaxTreatment
*
* This is the ONE place the rate is read. Everything that has to form a
* gross figure the price sheet, the Stripe catalogue, the module prices
* goes through chargedCents() below rather than multiplying by a percentage
* goes through chargeCents() below rather than multiplying by a percentage
* of its own.
*/
public static function domestic(): self
@ -90,17 +99,34 @@ final readonly class TaxTreatment
}
/**
* What a net catalogue figure is actually charged as.
* The intra-EU business treatment itself, with nobody in particular in mind.
*
* The number on the website, the amount on the Stripe Price and the total on
* the invoice are all this one, so it is computed once and here. It takes no
* customer on purpose: a Stripe Price carries a single amount and cannot ask
* who is at the checkout, and the owner's rule is that everybody pays the
* figure they were shown.
* For the catalogue mirror, which has to create the Price a reverse-charge
* business will be sold on BEFORE any such customer has reached the checkout
* so it needs the treatment without having anybody to derive it from. Who
* actually GETS it is for() below and is never decided here: this is a rate
* and a flag, not a judgement about a customer.
*/
public static function chargedCents(int $netCents): int
public static function reverseCharge(): self
{
return self::domestic()->grossCents($netCents);
return new self(0.0, true);
}
/**
* The figure the website quotes, with nobody in particular in mind.
*
* The public price sheet is read by visitors nobody has met, so it can only
* state the domestic gross and it must state the same number the ordinary
* checkout takes, which is what makes this the domestic treatment's own
* chargeCents() rather than a second multiplication.
*
* A reverse-charge business is charged less than this and is shown so on the
* pages that know who they are. Advertising the net out here instead is not
* an option: it would quote every private visitor a price they cannot have.
*/
public static function advertisedCents(int $netCents): int
{
return self::domestic()->chargeCents($netCents);
}
public static function for(?Customer $customer): self
@ -134,7 +160,7 @@ final readonly class TaxTreatment
&& $country !== $seller
&& preg_match('/^[A-Z]{2}[0-9A-Z]{8,12}$/', $vatId) === 1;
return $eligible ? new self(0.0, true) : $domestic;
return $eligible ? self::reverseCharge() : $domestic;
}
public function grossCents(int $netCents): int
@ -142,6 +168,27 @@ final readonly class TaxTreatment
return (int) round($netCents * (1 + $this->rate));
}
/**
* What THIS customer's card is charged for a net catalogue figure.
*
* The one place the amount taken is formed, and therefore the one place that
* decides which of a sellable thing's two Stripe Prices a checkout uses, what
* the proof register expects to have been taken, and what a document has to
* total to. A second reader forming the figure from a rate of its own is how
* the charge, the Price and the invoice come to disagree.
*
* Written on the reverse-charge flag rather than on the rate, although at a
* rate of nought grossCents() would return the same number. The two are
* different questions that happen to share an answer here: grossCents() says
* how a document is SPLIT, and this says what is TAKEN and if cross-border
* B2C is ever taxed at the customer's own rate under OSS, the two part
* company and this must go on charging what was quoted.
*/
public function chargeCents(int $netCents): int
{
return $this->reverseCharge ? $netCents : $this->grossCents($netCents);
}
/**
* The rate in the form the invoice arithmetic works in 2000 for 20 %.
*

View File

@ -16,6 +16,16 @@ use RuntimeException;
*/
class HttpHetznerDnsClient implements HetznerDnsClient
{
/** Hetzner's own maximum; asking for more is answered with 100 anyway. */
private const PAGE_SIZE = 100;
/**
* A ceiling on the record walk 10 000 records in one zone is far past
* anything this product will hold, and far short of a request loop that never
* ends because a page count came back wrong.
*/
private const MAX_PAGES = 100;
private function http(): PendingRequest
{
// Read HERE, at the point of use, rather than in a constructor or
@ -41,8 +51,7 @@ class HttpHetznerDnsClient implements HetznerDnsClient
$zoneId = $this->zoneId();
$name = rtrim(str_replace('.'.ProvisioningSettings::dnsZone(), '', $fqdn), '.');
$existing = collect($this->http()->get('/records', ['zone_id' => $zoneId])->throw()->json('records', []))
->first(fn ($r) => $r['name'] === $name && $r['type'] === $type);
$existing = $this->findRecord($zoneId, $name, $type);
if ($existing) {
$this->http()->put("/records/{$existing['id']}", [
@ -57,6 +66,49 @@ class HttpHetznerDnsClient implements HetznerDnsClient
])->throw()->json('record.id');
}
/**
* The record with this name and type in the zone, or null.
*
* Paged, because Hetzner pages. `GET /records` answers with at most 100
* entries and this asked only for the first page so once the zone held more
* than a hundred records the lookup stopped finding entries that were plainly
* there, fell through to POST, and Hetzner accepted a SECOND A record for the
* same name. Two addresses for one instance, resolved round-robin, one of them
* a host the customer's machine is not on: the cloud was up about half the
* time, and every subsequent run made it worse, because the `address` and
* `plan-change` pipelines upsert on every pass.
*
* A hundred is not a distant number here. The zone carries one A record per
* customer instance plus one per host, and nothing prunes it faster than
* instances are sold.
*
* @return array<string, mixed>|null
*/
private function findRecord(string $zoneId, string $name, string $type): ?array
{
$page = 1;
do {
$response = $this->http()
->get('/records', ['zone_id' => $zoneId, 'page' => $page, 'per_page' => self::PAGE_SIZE])
->throw();
foreach ($response->json('records', []) as $record) {
if (($record['name'] ?? null) === $name && ($record['type'] ?? null) === $type) {
return $record;
}
}
// Absent or nonsensical pagination means "this was the lot": a client
// that trusted the field to be there could loop for ever against an
// API having a bad day, and MAX_PAGES is the floor under that even
// when it answers.
$lastPage = (int) $response->json('meta.pagination.last_page', $page);
} while ($page++ < min($lastPage, self::MAX_PAGES));
return null;
}
public function deleteRecord(string $recordId): void
{
$response = $this->http()->delete("/records/{$recordId}");

View File

@ -7,6 +7,7 @@ use App\Models\Instance;
use App\Models\MailTemplate;
use App\Models\Operator;
use App\Models\Subscription;
use App\Services\Billing\TaxTreatment;
use App\Support\CompanyProfile;
use Illuminate\Support\Number;
@ -96,10 +97,10 @@ final class MailTemplateRenderer
'contact' => (string) ($customer->contact_name ?: $customer->name),
'email' => (string) $customer->email,
'plan' => $subscription === null ? '' : __('billing.plan.'.$subscription->plan),
// Gross, because that is the figure the customer knows: it is what
// the price sheet quoted them and what their bank statement says.
// What was actually charged, because that is the figure the customer
// knows: it is what their bank statement says.
'amount' => $subscription === null ? '' : Number::currency(
$this->grossCents($subscription) / 100,
$this->chargedCents($subscription, $customer) / 100,
in: (string) ($subscription->currency ?: 'EUR'),
locale: app()->getLocale(),
),
@ -125,17 +126,18 @@ final class MailTemplateRenderer
}
/**
* The contract's monthly figure with VAT on it.
* The contract's monthly figure as this customer is charged it.
*
* The domestic rate, from the same place an invoice reads it. Reverse charge
* is deliberately not applied: this figure goes into a sentence a person
* reads, and a business quoting itself a net price in a mail is a different
* problem from a document that has to be lawful.
* Through TaxTreatment, which is the one place the amount taken is formed, and
* therefore the same figure the Stripe Price for this contract carries. It used
* to be a multiplication at the domestic rate written out here, with the
* reasoning that reverse charge was a matter for documents and not for
* sentences and that stopped being true the moment a reverse-charge business
* began to be charged the bare net. A mail naming a fifth more than the bank
* statement is a mail the customer writes back about.
*/
private function grossCents(Subscription $subscription): int
private function chargedCents(Subscription $subscription, Customer $customer): int
{
$net = (int) $subscription->price_cents;
return (int) round($net * (1 + CompanyProfile::taxRate() / 100));
return TaxTreatment::for($customer)->chargeCents((int) $subscription->price_cents);
}
}

View File

@ -25,6 +25,15 @@ class FakeStripeClient implements StripeClient
/** @var array<int, string> */
public array $archived = [];
/**
* Every Price that was put back on sale, in order a rate that moved and
* moved back reaches for the Price it already has rather than minting a
* second one for the same amount.
*
* @var array<int, string>
*/
public array $activated = [];
/** Idempotency key → the id first returned for it. @var array<string, string> */
public array $keys = [];
@ -189,6 +198,19 @@ class FakeStripeClient implements StripeClient
$this->archived[] = $priceId;
}
public function activatePrice(string $priceId): void
{
// Taken off the archived list rather than merely appended to a second
// one, so a test asking "is this Price still being sold?" gets the
// answer Stripe would give instead of a history it has to interpret.
$this->archived = array_values(array_filter(
$this->archived,
fn (string $id) => $id !== $priceId,
));
$this->activated[] = $priceId;
}
public function updateSubscriptionPrice(
string $subscriptionId,
string $itemId,

View File

@ -131,6 +131,17 @@ class HttpStripeClient implements StripeClient
->throw();
}
public function activatePrice(string $priceId): void
{
// `active` is one of the very few fields a Stripe Price does let you
// change, which is what makes bringing one back possible at all — the
// amount is not, and that is why a changed figure is always a new Price.
$this->request()
->asForm()
->post($this->url('prices/'.$priceId), ['active' => 'true'])
->throw();
}
public function updateSubscriptionPrice(
string $subscriptionId,
string $itemId,

View File

@ -66,8 +66,16 @@ interface StripeClient
* subscription: the session's copy is what StripeWebhookController reads,
* the subscription's is what any later billing event carries.
*
* `$priceId` is one of the TWO Prices the catalogue sells that package on
* the domestic gross or the bare net a reverse-charge business owes and which
* one it is has been decided by TaxTreatment before this is called. Nothing
* here computes tax, and `automatic_tax` is deliberately never enabled: a
* second rate worked out by Stripe would charge a German consumer 19 % while
* our own document said 20 %.
*
* `$oneOff` is the setup fee: a second line item with no recurrence, whose
* `amount_cents` is GROSS like every other figure this platform charges.
* `amount_cents` is what this same customer is charged for it, formed the same
* way and by the same authority as the Price above.
* Stripe allows one-time prices in a subscription-mode session and puts them
* on the INITIAL invoice only, which is precisely what a setup fee is so it
* is charged once, with the first month, and never again. Null means there is
@ -103,7 +111,9 @@ interface StripeClient
/**
* Create a Price. Stripe Prices are immutable and carry their own interval,
* which is why one exists per plan version AND term.
* which is why one exists per plan version AND term and, since a
* reverse-charge business is charged the bare net while everybody else pays
* the gross, two per priced row on each term.
*/
public function createPrice(
string $productId,
@ -117,6 +127,18 @@ interface StripeClient
/** Stop a Price being offered. The Price itself stays, as Stripe requires. */
public function archivePrice(string $priceId): void;
/**
* Offer an archived Price again.
*
* The other half of archivePrice(), and it was missing. A rate that moves and
* then moves back finds the Price for the figure it has come back to already
* in our register: the sync brings that row back rather than minting a second
* Price for one amount, which is right but the Price was archived AT STRIPE,
* and only this makes it sellable again. Without it the catalogue pointed at
* an inactive Price and the failure was a customer meeting a refused checkout.
*/
public function activatePrice(string $priceId): void;
/**
* Move a subscription onto another Price.
*

View File

@ -0,0 +1,63 @@
<?php
namespace App\Support;
/**
* The one place a guest command that runs Nextcloud's `occ` is built.
*
* `docker compose exec` runs as **root** unless it is told otherwise, and the
* official Nextcloud image will not have it: `console.php` compares the calling
* uid against the owner of `config/config.php` www-data and exits 1 with
* "Console has to be executed with the user that owns the file config/config.php"
* before a single command is dispatched. Every occ call in this product was
* therefore failing on every instance, and the ones that go through
* CustomerStep::guest() turned that exit code into a retry and then into a failed
* run for a machine that was otherwise finished.
*
* The same default already cost this project days on its OWN container, where
* `artisan optimize` ran as root and left storage/logs/laravel.log unwritable
* see deploy/update.sh's in_app() and tests/Feature/DeploymentRunsAsTheAppUserTest.
* That lesson was written down for the deployment and not for the guests.
*
* It lives here, in one place, because the prefix was pasted into five files and
* a sixth call site would have got it wrong again. Nothing else in app/ may spell
* `docker compose exec` out by hand; the test above refuses it.
*/
final class NextcloudOcc
{
/** Where the compose project sits inside every guest this product builds. */
public const DIRECTORY = '/opt/nextcloud';
/**
* The account that owns the Nextcloud installation inside the image.
*
* Named rather than defaulted: the rule is that somebody chose the user, so
* a reader can tell a deliberate root from a forgotten one.
*/
public const USER = 'www-data';
/**
* A guest shell command that runs `occ <arguments>` as that account.
*
* Environment values are passed on the docker invocation itself and handed
* through with `-e`, because an assignment placed before the `cd` would not
* survive the `&&` this is how OC_PASS reaches `user:add
* --password-from-env` without the password ever appearing in an occ
* argument (and so in the guest's process list).
*
* @param array<string, string> $env
*/
public static function command(string $arguments, array $env = []): string
{
$assignments = '';
$forwards = '';
foreach ($env as $name => $value) {
$assignments .= $name.'='.escapeshellarg($value).' ';
$forwards .= '-e '.$name.' ';
}
return 'cd '.self::DIRECTORY.' && '.$assignments
.'docker compose exec -T -u '.self::USER.' '.$forwards.'app php occ '.$arguments;
}
}

View File

@ -73,9 +73,31 @@ return [
/*
| Making an existing instance's address real again, without rebuilding
| anything. Exactly the two steps an address consists of: the router,
| the DNS record and the certificate, then the hostname Nextcloud
| itself will answer to.
| anything. Exactly the two steps an address consists of: the hostname
| Nextcloud itself will answer to, then the router, the DNS record and
| the certificate.
|
| In THAT order, which is the order the `customer` pipeline has always
| used, and it is not interchangeable. A half-finished address is the
| ordinary outcome of a failed run, so the question is which half is safe
| to be left holding, and the answer differs by direction:
|
| - A domain just proven. Router first meant getting the certificate and
| then failing at Nextcloud so the customer's own domain served
| Nextcloud's "Zugriff über nicht vertrauenswürdige Domain" error page
| under a valid certificate, while `domain_cert_ok` was already true
| and the portal was calling that address live. Nextcloud first leaves
| a trusted hostname that nothing routes to, which nobody can reach and
| nothing announces.
| - A domain withdrawn. Nextcloud first stops it being ANSWERED, which is
| what withdrawal means; if the router write then fails the name still
| reaches us and is refused. Router first stops it being reached while
| Nextcloud would still have answered it the same end state, arrived
| at with the trust left standing.
|
| So Nextcloud first is safe in both directions and router first is safe
| in only one. There is no third step to sequence and no state either step
| reads from the other.
|
| Same subject as `customer` (the Order), because that is what
| CustomerStep::order()/instance() resolve a run against any other
@ -85,8 +107,8 @@ return [
| carries one.
*/
'address' => [
Customer\ConfigureDnsAndTls::class,
Customer\ConfigureNextcloud::class,
Customer\ConfigureDnsAndTls::class,
],
/*
@ -104,7 +126,8 @@ return [
|
| The two address steps are the `address` pipeline itself, reused rather
| than repeated: a downgrade onto a package without an own domain has to
| stop serving that domain, and that is exactly what those two do.
| stop serving that domain, and that is exactly what those two do in
| the same order and for the same reason, spelled out above that pipeline.
*/
'plan-change' => [
Customer\ResizeVirtualMachine::class,
@ -115,8 +138,8 @@ return [
// life of the machine.
Customer\GrowGuestFilesystem::class,
Customer\ApplyStorageQuota::class,
Customer\ConfigureDnsAndTls::class,
Customer\ConfigureNextcloud::class,
Customer\ConfigureDnsAndTls::class,
Customer\SettlePlanServices::class,
],
@ -130,16 +153,27 @@ return [
| because nothing in the product could do this, which is how a paid
| upgrade could stay invisible forever.
|
| StartVirtualMachine and WaitForGuestAgent are the customer pipeline's
| own steps, reused rather than repeated starting a machine and waiting
| for its agent is the same operation whichever run needs it. Started by
| StartVirtualMachine, WaitForGuestAgent and ConfigureNetwork are the
| customer pipeline's own steps, reused rather than repeated starting a
| machine, waiting for its agent and reading the address it came up on is
| the same operation whichever run needs it. Started by
| App\Actions\RestartInstance, from the customer's cloud page or the
| console's instance list.
|
| ConfigureNetwork is here because a cold boot is the one event that can
| move the guest: cloud-init hands it `ip=dhcp`, and this step is the only
| writer of `instances.guest_ip` which is the backend the router on the
| serving host points at. Without it a restarted machine could come back on
| a different address, the router would go on pointing at the old one, and
| the customer's cloud would answer 502 for the rest of its life with no
| pipeline able to repair it. The step itself asks for the address to be
| re-applied when it has actually changed; see ConfigureNetwork.
*/
'restart' => [
Customer\ShutDownVirtualMachine::class,
Customer\StartVirtualMachine::class,
Customer\WaitForGuestAgent::class,
Customer\ConfigureNetwork::class,
Customer\CompleteRestart::class,
],

View File

@ -0,0 +1,39 @@
<?php
use Illuminate\Database\Migrations\Migration;
use Illuminate\Database\Schema\Blueprint;
use Illuminate\Support\Facades\Schema;
/**
* Where the router was last told to send the traffic.
*
* The hostname list was already compared before the router file is rewritten, so
* an address that had changed would be re-applied but the same file names a
* BACKEND, and nothing compared that. cloud-init gives every guest `ip=dhcp`, so
* a lease expiry or a cold boot can move the machine; the router went on pointing
* at the address it had, the customer's cloud answered 502, and no run anywhere
* would rewrite it, because the hostnames still matched and `route_written` still
* said a file existed.
*
* Left null for every existing row on purpose: null does not match the backend
* ConfigureDnsAndTls computes, so the first run to touch each instance rewrites
* its router once and records what it wrote. Backfilling it from `guest_ip` would
* be asserting that the file on the host says what we hope it says, which is the
* assumption this column exists to stop making.
*/
return new class extends Migration
{
public function up(): void
{
Schema::table('instances', function (Blueprint $table) {
$table->string('routed_backend')->nullable()->after('routed_hostnames');
});
}
public function down(): void
{
Schema::table('instances', function (Blueprint $table) {
$table->dropColumn('routed_backend');
});
}
};

View File

@ -0,0 +1,96 @@
<?php
use Illuminate\Database\Migrations\Migration;
use Illuminate\Database\Schema\Blueprint;
use Illuminate\Support\Facades\Schema;
/**
* What it takes to charge a reverse-charge business the figure it actually owes.
*
* The catalogue was mirrored into Stripe at ONE amount per priced row the
* domestic gross because a Stripe Price carries a single figure and was taken
* to be unable to ask who is buying. For a domestic customer that is exactly
* right, business or private: the VAT is on the invoice and an Austrian business
* reclaims it as input tax, so the gross is a pass-through and the price on the
* website is the price charged.
*
* It breaks for the one case where no VAT is charged at all. A business in
* another member state with a VERIFIED VAT id gets reverse charge rate zero,
* no VAT line, the note on the document and was still charged 214,80 for a
* 179,00 package, with the document then stating the whole 214,80 as net at
* 0 %. There is no VAT line on it for them to reclaim, so it was a flat 20 %
* surcharge on precisely the customers who read their invoices, and they
* self-accounted their own VAT on a base a fifth too big.
*
* So a Price CAN ask who is buying: there are two of them per sellable thing,
* and the checkout picks. The domestic gross and the bare net, both live, both
* pointing at the same Product and `reverse_charge` below is what says which
* is which, on both sides of the mirror.
*
* `plan_prices.stripe_price_id` keeps its meaning untouched: the Price an
* ordinary domestic customer is sold on. Nothing but a reverse-charge business
* is ever sold on the net one, so it gets no column of its own on the catalogue
* row and is read out of the register here instead.
*
* What a running contract is billed on stays readable, which is the whole reason
* these registers are tables: `subscriptions.stripe_price_id` names the Price,
* and joining it here says whether that Price is the gross or the net one so a
* customer whose registration is verified next week, or whose registration
* lapses, can be seen to be on the wrong one and moved.
*/
return new class extends Migration
{
public function up(): void
{
Schema::table('stripe_plan_prices', function (Blueprint $table) {
// Which of the two Prices for this row it is. False is the ordinary
// sale and therefore the default: every Price that already exists
// was minted for a domestic customer, whatever it happens to charge.
$table->boolean('reverse_charge')->default(false)->after('plan_price_id');
// The old key was (row, figure), which cannot hold both Prices at a
// rate of nought — where the gross and the net are the same number —
// and, worse, could not say which of two rows at one figure the
// checkout should pick. The treatment belongs in the identity.
$table->dropUnique('stripe_plan_prices_unique');
$table->unique(['plan_price_id', 'reverse_charge', 'charged_cents'], 'stripe_plan_prices_unique');
});
Schema::table('stripe_addon_prices', function (Blueprint $table) {
$table->boolean('reverse_charge')->default(false)->after('addon_key');
$table->dropUnique('stripe_addon_prices_unique');
$table->unique(
['addon_key', 'reverse_charge', 'amount_cents', 'currency', 'interval'],
'stripe_addon_prices_unique',
);
// Read when a Price is superseded by a rate change, which happens
// within ONE treatment: the gross Price for today's figure moves and
// the net one beside it does not, so the treatment has to be part of
// the index the sweep reads or it would archive the other half.
$table->dropIndex('stripe_addon_prices_net');
$table->index(
['addon_key', 'reverse_charge', 'net_cents', 'currency', 'interval'],
'stripe_addon_prices_net',
);
});
}
public function down(): void
{
Schema::table('stripe_addon_prices', function (Blueprint $table) {
$table->dropIndex('stripe_addon_prices_net');
$table->dropUnique('stripe_addon_prices_unique');
$table->dropColumn('reverse_charge');
$table->unique(['addon_key', 'amount_cents', 'currency', 'interval'], 'stripe_addon_prices_unique');
$table->index(['addon_key', 'net_cents', 'currency', 'interval'], 'stripe_addon_prices_net');
});
Schema::table('stripe_plan_prices', function (Blueprint $table) {
$table->dropUnique('stripe_plan_prices_unique');
$table->dropColumn('reverse_charge');
$table->unique(['plan_price_id', 'charged_cents'], 'stripe_plan_prices_unique');
});
}
};

View File

@ -10,6 +10,9 @@ return [
'recommended' => 'Empfohlen',
'per_month' => '/Monat',
'incl_vat' => 'inkl. :rate % MwSt.',
// Für ein bestätigtes EU-Unternehmen außerhalb Österreichs: keine Umsatzsteuer,
// also auch keine zweite Zahl daneben. Der Nettobetrag IST der Preis.
'reverse_charge' => 'ohne USt. — Steuerschuldnerschaft des Leistungsempfängers (Reverse Charge)',
'net' => 'netto',
'setup' => 'zzgl. Einrichtung einmalig :amount',

View File

@ -10,6 +10,9 @@ return [
'recommended' => 'Recommended',
'per_month' => '/month',
'incl_vat' => 'incl. :rate % VAT',
// For a verified EU business outside Austria: no VAT, and therefore no second
// figure beside it either. The net amount IS the price.
'reverse_charge' => 'no VAT — reverse charge, VAT payable by the recipient',
'net' => 'net',
'setup' => 'plus one-off setup :amount',

View File

@ -45,16 +45,24 @@
<p class="mt-1 text-xs text-muted">{{ $plan['audience'] }}</p>
@endif
{{-- Gross, like the public sheet. A customer quoted 58,80
out there must not meet 49 on the page with the
button on it. --}}
{{-- What this customer's card will be charged, like the public
sheet. A customer quoted 58,80 out there must not meet
49 on the page with the button on it. --}}
<p class="mt-5 flex items-baseline gap-1.5">
<span class="font-mono text-2xl font-semibold tabular-nums text-ink">{{ Number::currency($plan['gross'] / 100, in: $plan['currency'], locale: app()->getLocale()) }}</span>
<span class="text-xs text-muted">{{ __('order.per_month') }}</span>
</p>
<p class="mt-1 text-xs text-muted">
{{-- A reverse-charge business is charged the bare net, so
there is no VAT to state and no second figure to show:
saying "inkl. 20 %" over a net price would describe a
tax nothing collects. --}}
@if ($tax->reverseCharge)
{{ __('order.reverse_charge') }}
@else
{{ __('order.incl_vat', ['rate' => $vat]) }} ·
{{ __('order.net') }} {{ Number::currency($plan['net'] / 100, in: $plan['currency'], locale: app()->getLocale()) }}
@endif
</p>
@if ($setup > 0)
<p class="mt-1 text-xs text-muted">

View File

@ -7,9 +7,11 @@ use App\Models\Host;
use App\Models\Instance;
use App\Models\Order;
use App\Models\ProvisioningRun;
use App\Provisioning\RunRunner;
use App\Provisioning\Steps\Customer\ReserveResources;
use App\Services\Provisioning\HostCapacity;
use Illuminate\Support\Facades\Http;
use Illuminate\Support\Facades\Queue;
use Livewire\Livewire;
/**
@ -66,6 +68,43 @@ it('parks a paid order instead of failing it when nothing has room', function ()
->and($result->afterSeconds)->toBe(ReserveResources::PARK_POLL_SECONDS);
});
it('keeps a park alive through the runner, not merely through the step', function () {
// Both park tests around this one call execute() directly, and that is exactly
// why this stayed invisible for the whole life of the feature. The step
// returned poll(120) correctly; the RUNNER then measured the step's own
// maxDuration from a started_at it deliberately does not reset on a poll, and
// maxDuration was 60. So every single re-entry was ruled timed out BEFORE the
// body ran, a timeout consumes an attempt, and five of them had failRun()
// marking a paid order and its instance failed and releasing the instance —
// about six minutes for a wait that promises fourteen days. The console's
// capacity queue, HostCapacity::parked(), queueDemand() and the whole "go and
// buy a server" workflow were unreachable code.
Queue::fake();
$run = parkedOrder();
$run->update(['status' => ProvisioningRun::STATUS_RUNNING]);
$runner = app(RunRunner::class);
// Four full poll intervals: further than the old sixty-second budget, and
// further than five attempts would ever have carried it.
for ($i = 0; $i < 4; $i++) {
$runner->advance($run->fresh());
$polled = $run->fresh();
expect($polled->status)->toBe(ProvisioningRun::STATUS_WAITING)
->and($polled->attempt)->toBe(0) // a poll costs nothing
->and($polled->error)->toBeNull();
$this->travel(ReserveResources::PARK_POLL_SECONDS + 1)->seconds();
}
// Still parked, still one of the runs the console is asking somebody to buy a
// machine for, and nothing has been declared failed along the way.
expect(app(HostCapacity::class)->isParked($run->fresh()))->toBeTrue()
->and($run->fresh()->events()->where('outcome', 'failed')->exists())->toBeFalse()
->and($run->fresh()->events()->where('message', 'like', '%timed out%')->exists())->toBeFalse();
});
it('fails only once the order has waited longer than a machine can take', function () {
// Long enough to buy, rack and onboard a server over a weekend; short
// enough that a forgotten order surfaces instead of waiting for ever.

View File

@ -2,6 +2,8 @@
use App\Livewire\Admin\Provisioning;
use App\Models\Host;
use App\Models\Instance;
use App\Models\Order;
use App\Models\ProvisioningRun;
use App\Provisioning\Jobs\AdvanceRunJob;
use Illuminate\Support\Facades\Queue;
@ -27,6 +29,50 @@ it('retries a failed run from the provisioning console', function () {
Queue::assertPushed(AdvanceRunJob::class);
});
it('leaves a live customer alone when the run being retried is a maintenance one', function () {
// The retry set the order to `provisioning` for ANY pipeline, and only the
// `customer` pipeline ever puts it back to `active` (CompleteProvisioning).
// So retrying a failed address, restart, storage, quota or plan-change run
// left a paying customer's order reading `provisioning` for ever: nothing in
// the product would move it again, the customer saw nothing wrong, and the
// console's view of them was simply false. Same reasoning as RunRunner's
// failRun() on the way down — a maintenance run must not condemn what it
// maintains.
Queue::fake();
$order = Order::factory()->create(['status' => 'active']);
$instance = Instance::factory()->create([
'order_id' => $order->id, 'customer_id' => $order->customer_id, 'status' => 'active',
]);
$run = ProvisioningRun::factory()->create([
'subject_type' => Order::class, 'subject_id' => $order->id, 'pipeline' => 'address',
'status' => ProvisioningRun::STATUS_FAILED, 'error' => 'boom',
]);
Livewire::actingAs(admin(), 'operator')->test(Provisioning::class)->call('retry', $run->uuid);
expect($run->fresh()->status)->toBe(ProvisioningRun::STATUS_RUNNING)
->and($order->fresh()->status)->toBe('active')
->and($instance->fresh()->status)->toBe('active');
Queue::assertPushed(AdvanceRunJob::class);
});
it('does put a customer build back into provisioning when that is the run being retried', function () {
Queue::fake();
$order = Order::factory()->create(['status' => 'failed']);
$instance = Instance::factory()->create([
'order_id' => $order->id, 'customer_id' => $order->customer_id, 'status' => 'failed',
]);
$run = ProvisioningRun::factory()->create([
'subject_type' => Order::class, 'subject_id' => $order->id, 'pipeline' => 'customer',
'status' => ProvisioningRun::STATUS_FAILED, 'error' => 'boom',
]);
Livewire::actingAs(admin(), 'operator')->test(Provisioning::class)->call('retry', $run->uuid);
expect($order->fresh()->status)->toBe('provisioning')
->and($instance->fresh()->status)->toBe('provisioning');
});
it('ignores retry on a run that is not failed', function () {
Queue::fake();
$host = Host::factory()->active()->create();

View File

@ -148,7 +148,13 @@ it('reads the direction from the plan rank, not from a grandfathered price', fun
]);
$subscription->forceFill(['price_cents' => 9900])->saveQuietly(); // grandfathered
config()->set('provisioning.plans.team.price_cents', 17900); // today's team costs more
// The premise, asserted rather than written into config('provisioning.plans.*')
// — a key that has not existed since the catalogue moved into the database, so
// that line set nothing and this test never built the scenario it is named for.
// Today's team has to cost MORE than this customer's grandfathered business
// price, or a price-led reading of the direction would agree with the rank one
// and the test would prove nothing.
expect(Subscription::snapshotFrom('team')['price_cents'])->toBeGreaterThan(9900);
$change = PlanChange::evaluate($subscription->fresh(), 'team', Illuminate\Support\Carbon::parse('2026-08-15'));
@ -184,9 +190,10 @@ it('never invoices a customer for downgrading', function () {
'current_period_start' => $start,
'current_period_end' => $start->copy()->addMonth(),
]);
// Grandfathered below today's smaller plan.
// Grandfathered below today's smaller plan — asserted, for the reason given in
// the direction test above: the config write this replaces was a no-op.
$subscription->forceFill(['price_cents' => 9900])->saveQuietly();
config()->set('provisioning.plans.team.price_cents', 17900);
expect(Subscription::snapshotFrom('team')['price_cents'])->toBeGreaterThan(9900);
$credit = PlanChange::goodwillCredit($subscription->fresh(), 'team', Illuminate\Support\Carbon::parse('2026-08-15'));

View File

@ -307,34 +307,41 @@ it('states net, tax and gross rather than leaving them to be recomputed', functi
$subscription = Subscription::factory()->plan('team')->create(['customer_id' => reverseChargeCustomer()->id]);
// What a real reverse-charge purchase records: OpenSubscription always passes
// the charged figure, and the charged figure is the domestic gross, because a
// Stripe Price carries one amount and everybody pays the one they were shown.
$record = app(RecordCommercialEvent::class)('purchase', $subscription, 17900, chargedGrossCents: 21480);
// the charged figure, and for this customer that figure is the BARE NET —
// their checkout used the net Stripe Price, because reverse charge means no
// VAT is owed to us at all.
$record = app(RecordCommercialEvent::class)('purchase', $subscription, 17900, chargedGrossCents: 17900);
// The whole of it is net at 0 %. Recorded as it stood on the day — a rate
// that changes later must not change the answer for this sale.
expect($record->net_cents)->toBe(21480)
expect($record->net_cents)->toBe(17900)
->and($record->tax_cents)->toBe(0)
->and($record->gross_cents)->toBe(21480)
->and($record->gross_cents)->toBe(17900)
->and($record->reverse_charge)->toBeTrue();
});
it('does not call a reverse-charge sale charged at the advertised amount a mismatch', function () {
it('calls a correctly charged reverse-charge sale a match, and an overcharged one a mismatch', function () {
$subscription = Subscription::factory()->plan('team')->create(['customer_id' => reverseChargeCustomer()->id]);
// 179,00 net at the till is 214,80 for everybody — the figure on the website,
// the figure on the Stripe Price, the figure taken from the card. The flag was
// holding it against the customer's OWN rate, which is zero, and so reported
// every single EU business sale as charged at the wrong amount. A flag that
// means "somebody must look at this" cannot be false for the healthy majority.
$record = app(RecordCommercialEvent::class)('purchase', $subscription, 17900, chargedGrossCents: 21480);
// 179,00 net is the WHOLE of what this customer owes: no VAT is charged to
// them, so there is none in the price either, and their checkout used the net
// Stripe Price. The register expects that figure, and finding it is a match.
$record = app(RecordCommercialEvent::class)('purchase', $subscription, 17900, chargedGrossCents: 17900);
expect($record->snapshot['amounts']['expected_gross_cents'])->toBe(21480)
->and($record->snapshot['amounts']['charged_gross_cents'])->toBe(21480)
expect($record->snapshot['amounts']['expected_gross_cents'])->toBe(17900)
->and($record->snapshot['amounts']['charged_gross_cents'])->toBe(17900)
->and($record->snapshot['amounts']['matches_catalogue'])->toBeTrue();
// And it still catches the thing it is for: a reverse-charge customer who was
// charged something other than the advertised amount.
// And now it catches the thing it is for, in the direction that actually cost
// somebody money: the domestic gross taken from a reverse-charge business is a
// flat 20 % surcharge with no VAT line anywhere for them to reclaim. That is
// the defect this whole change ends, and the flag whose job is "somebody must
// look at this" has to be the thing that would have found it.
$overcharged = app(RecordCommercialEvent::class)('purchase', $subscription, 17900, chargedGrossCents: 21480);
expect($overcharged->snapshot['amounts']['matches_catalogue'])->toBeFalse();
// A short charge is still a mismatch, in the other direction.
$short = app(RecordCommercialEvent::class)('purchase', $subscription, 17900, chargedGrossCents: 14900);
expect($short->snapshot['amounts']['matches_catalogue'])->toBeFalse();

View File

@ -0,0 +1,400 @@
<?php
use App\Actions\BookAddon;
use App\Models\Customer;
use App\Models\Invoice;
use App\Models\PlanPrice;
use App\Models\StripeAddonPrice;
use App\Models\StripePlanPrice;
use App\Models\Subscription;
use App\Models\SubscriptionRecord;
use App\Models\User;
use App\Services\Billing\PlanPrices;
use App\Services\Billing\TaxTreatment;
use App\Services\Stripe\FakeStripeClient;
use App\Services\Stripe\StripeClient;
use App\Services\Tax\FakeVatIdVerifier;
use App\Services\Tax\VatIdVerifier;
use App\Support\CompanyProfile;
use App\Support\Settings;
use Illuminate\Contracts\Console\Kernel;
use Illuminate\Support\Facades\Queue;
/**
* The price on the website is the price charged and for the one customer who is
* charged no VAT, that is the net.
*
* A domestic customer pays the gross whether they are a private person or a
* business, and that is right: the VAT is on their invoice and an Austrian
* business reclaims it as input tax, so it passes straight through them. A
* business in another member state with a VERIFIED VAT id gets reverse charge
* rate nought, no VAT line, the note on the document and there is nothing on
* their invoice to reclaim. Charging them the gross was therefore a flat 20 %
* surcharge on exactly the customers who read their invoices, and their document
* then stated the whole of it as net at 0 %, so they self-accounted their own VAT
* on a base a fifth too large.
*
* The fix is that a Stripe Price CAN ask who is buying: there are two per sellable
* thing, and the checkout picks by TaxTreatment. Everything here is about those
* two Prices who gets which, what happens when a customer's status changes after
* they have bought, and that the sync which mints them stays safe to re-run.
*/
beforeEach(function () {
$this->stripe = new FakeStripeClient;
app()->instance(StripeClient::class, $this->stripe);
// The catalogue mirrored into Stripe: two Prices per priced row and per
// module interval. Nothing here mints a Price on demand, deliberately — see
// App\Services\Billing\PlanPrices — so a checkout has to find one waiting.
app(Kernel::class)->call('stripe:sync-catalogue');
CompanyProfile::put([
'name' => 'CluPilot Cloud e.U.', 'address' => 'Dreherstraße 66/1/8',
'postcode' => '1110', 'city' => 'Wien', 'vat_id' => 'ATU00000000',
]);
});
/**
* Somebody who can reach the checkout, and the customer record the treatment is
* read from.
*
* @param array<string, mixed> $attributes
* @return array{0: User, 1: Customer}
*/
function buyerWith(array $attributes): array
{
$customer = Customer::factory()->create($attributes);
$user = User::factory()->create([
'email' => $customer->email,
'email_verified_at' => now(),
]);
return [$user, $customer];
}
/** A business in another member state whose number the register knows. */
function verifiedEuBusiness(): array
{
$verifier = new FakeVatIdVerifier;
$verifier->registered('NL123456789B01', 'Berger B.V.');
app()->instance(VatIdVerifier::class, $verifier);
return buyerWith([
'customer_type' => Customer::TYPE_BUSINESS,
'vat_id' => 'NL123456789B01',
'vat_id_verified_at' => now(),
'vat_id_verified_value' => 'NL123456789B01',
]);
}
/** What the Price a checkout was opened on actually charges. */
function chargedByCheckout(FakeStripeClient $stripe, int $index = 0): int
{
return (int) $stripe->prices[$stripe->checkouts[$index]['price']]['amount'];
}
/** The Stripe Price the catalogue sells a package on to a customer treated so. */
function packagePrice(string $plan, TaxTreatment $treatment, string $term = Subscription::TERM_MONTHLY): string
{
$row = PlanPrice::query()
->whereHas('version.family', fn ($query) => $query->where('key', $plan))
->where('term', $term)
->sole();
return (string) app(PlanPrices::class)->liveFor($row, $treatment);
}
it('charges a domestic consumer and a domestic business the same gross, to the cent', function () {
// The owner's rule, and it is right for both of them: the business pays the
// gross, the VAT is on the invoice, they reclaim it as input tax.
[$consumer] = buyerWith(['customer_type' => Customer::TYPE_CONSUMER]);
[$business] = buyerWith([
'customer_type' => Customer::TYPE_BUSINESS,
// Austrian, and verified. Reverse charge is an INTRA-EU rule and this is
// our own member state, so it changes nothing at all: Austria B2B 20 %.
'vat_id' => 'ATU12345678',
'vat_id_verified_at' => now(),
'vat_id_verified_value' => 'ATU12345678',
]);
$this->actingAs($consumer)->post(route('checkout.start'), ['plan' => 'team', 'immediate_start' => '1'])
->assertRedirect();
$this->actingAs($business)->post(route('checkout.start'), ['plan' => 'team', 'immediate_start' => '1'])
->assertRedirect();
// 179,00 € plus 20 %, which is the figure on the website.
expect(chargedByCheckout($this->stripe, 0))->toBe(21480)
->and(chargedByCheckout($this->stripe, 1))->toBe(21480)
->and($this->stripe->checkouts[0]['price'])
->toBe($this->stripe->checkouts[1]['price'])
->and($this->stripe->checkouts[0]['price'])
->toBe(packagePrice('team', TaxTreatment::domestic()));
});
it('charges a verified business in another member state the bare net, and invoices exactly that', function () {
Queue::fake();
[$user] = verifiedEuBusiness();
$this->actingAs($user)->post(route('checkout.start'), ['plan' => 'team', 'immediate_start' => '1'])
->assertRedirect();
// The bare net: no VAT is owed to us, so there is none in the price either.
// A different Stripe Price from the domestic one, on the same Product.
expect(chargedByCheckout($this->stripe))->toBe(17900)
->and($this->stripe->checkouts[0]['price'])
->toBe(packagePrice('team', TaxTreatment::reverseCharge()))
->and($this->stripe->checkouts[0]['price'])
->not->toBe(packagePrice('team', TaxTreatment::domestic()));
// And what Stripe took is what the document says. This is the half that made
// the overcharge unlawful as well as expensive: 214,80 € taken, and a document
// stating all of it as net at 0 %, so the customer self-accounted their VAT on
// a fifth too much.
$this->postJson(route('webhooks.stripe'), [
'id' => 'evt_rc', 'type' => 'checkout.session.completed',
'data' => ['object' => [
'id' => 'cs_rc', 'payment_status' => 'paid', 'subscription' => 'sub_rc',
'customer_details' => ['email' => $user->email, 'name' => 'Berger B.V.'],
'amount_total' => 17900, 'currency' => 'eur',
'metadata' => ['plan' => 'team', 'datacenter' => 'fsn'],
]],
])->assertOk();
$invoice = Invoice::query()->sole();
expect($invoice->gross_cents)->toBe(17900)
->and($invoice->net_cents)->toBe(17900)
->and($invoice->tax_cents)->toBe(0)
// The note that makes a zero-rated document lawful.
->and($invoice->snapshot['meta']['closing'])->toBe(__('invoice.reverse_charge'));
// The register holds the charge against what this customer should have been
// charged, which is the net — so a correct reverse-charge sale is no longer
// reported as a mismatch, and an overcharged one would be.
$record = SubscriptionRecord::query()->where('event', SubscriptionRecord::EVENT_PURCHASE)->sole();
expect($record->snapshot['amounts']['expected_gross_cents'])->toBe(17900)
->and($record->snapshot['amounts']['charged_gross_cents'])->toBe(17900)
->and($record->snapshot['amounts']['matches_catalogue'])->toBeTrue()
->and($record->reverse_charge)->toBeTrue();
});
it('charges an unverified EU business the gross, because an unchecked number is no discount', function () {
// The number looks exactly like the verified one. Nobody has asked the
// register about it, and a self-declared string must never be able to zero a
// tax — otherwise typing "NL…" is a twenty percent discount.
[$user] = buyerWith([
'customer_type' => Customer::TYPE_BUSINESS,
'vat_id' => 'NL123456789B01',
]);
$this->actingAs($user)->post(route('checkout.start'), ['plan' => 'team', 'immediate_start' => '1'])
->assertRedirect();
expect(chargedByCheckout($this->stripe))->toBe(21480)
->and($this->stripe->checkouts[0]['price'])->toBe(packagePrice('team', TaxTreatment::domestic()));
});
it('charges the setup fee under the same rule as the package', function () {
// 99,00 € net, which is the 118,80 € the public price sheet quotes at 20 %.
Settings::set('company.setup_fee_cents', 9900);
[$domestic] = buyerWith(['customer_type' => Customer::TYPE_CONSUMER]);
[$business] = verifiedEuBusiness();
$this->actingAs($domestic)->post(route('checkout.start'), ['plan' => 'team', 'immediate_start' => '1']);
$this->actingAs($business)->post(route('checkout.start'), ['plan' => 'team', 'immediate_start' => '1']);
// A fee is a supply like the package it sets up, so it cannot be taxed
// differently from it on the same invoice — and the webhook is told how much
// of the total was the fee, so the figure has to be this customer's own.
expect($this->stripe->checkouts[0]['one_off']['amount_cents'])->toBe(11880)
->and($this->stripe->checkouts[0]['metadata']['setup_fee_cents'])->toBe('11880')
->and($this->stripe->checkouts[1]['one_off']['amount_cents'])->toBe(9900)
->and($this->stripe->checkouts[1]['metadata']['setup_fee_cents'])->toBe('9900');
});
it('bills a booked module on the net price too, for the customer who owes no VAT', function () {
[, $customer] = verifiedEuBusiness();
$contract = Subscription::factory()->plan('team')->create([
'customer_id' => $customer->id,
'stripe_subscription_id' => 'sub_rc_mod',
'stripe_item_id' => 'si_rc_plan',
]);
app(BookAddon::class)($contract, 'priority_support'); // 29,00 net / month
$item = collect($this->stripe->itemsOn('sub_rc_mod'))->sole();
// 29,00 € and not 34,80 €. A module taxed differently from the package beside
// it would be one invoice at two rates.
expect((int) $this->stripe->prices[$item['price']]['amount'])->toBe(2900)
->and(StripeAddonPrice::query()
->where('stripe_price_id', $item['price'])
->value('reverse_charge'))->toBeTruthy();
});
it('moves a contract onto the net price when the VAT id is verified after the sale, without prorating', function () {
// Bought as an ordinary business: the number was on the record, nobody had
// checked it, so the gross was charged and that was correct at the time.
$verifier = new FakeVatIdVerifier;
$verifier->registered('NL123456789B01', 'Berger B.V.');
app()->instance(VatIdVerifier::class, $verifier);
[, $customer] = buyerWith([
'customer_type' => Customer::TYPE_BUSINESS,
'vat_id' => 'NL123456789B01',
]);
$contract = Subscription::factory()->plan('team')->create([
'customer_id' => $customer->id,
'stripe_subscription_id' => 'sub_late',
'stripe_item_id' => 'si_late',
'stripe_price_id' => packagePrice('team', TaxTreatment::domestic()),
]);
// Nothing to do while the number is unchecked, and a run that moved them here
// would be a run that trusted a string somebody typed.
$this->artisan('stripe:reprice-subscriptions')->assertSuccessful();
expect($this->stripe->priceChanges)->toBeEmpty();
// A week later the register answers, and from that moment they owe no VAT.
$this->artisan('clupilot:verify-vat-ids')->assertSuccessful();
expect($customer->fresh()->hasVerifiedVatId())->toBeTrue();
$this->artisan('stripe:reprice-subscriptions')->assertSuccessful();
expect($this->stripe->priceChanges)->toHaveCount(1)
->and($this->stripe->priceChanges[0]['price'])->toBe(packagePrice('team', TaxTreatment::reverseCharge()))
// NEVER prorated. The term they are in is paid for, and charging or
// crediting the difference for days already served would be a bill
// nobody agreed to.
->and($this->stripe->priceChanges[0]['proration'])->toBe(StripeClient::PRORATE_NONE)
// And the contract records which of the two Prices it is now billed on,
// which is what makes a second run free.
->and($contract->fresh()->stripe_price_id)
->toBe(packagePrice('team', TaxTreatment::reverseCharge()))
->and(StripePlanPrice::query()
->where('stripe_price_id', $contract->fresh()->stripe_price_id)
->value('reverse_charge'))->toBeTruthy();
// Converged: the second run finds nothing left to move.
$this->artisan('stripe:reprice-subscriptions')->assertSuccessful();
expect($this->stripe->priceChanges)->toHaveCount(1);
// The contract's own frozen figure is untouched by all of it. It is the
// catalogue's NET amount, every pro-rata sum reads it, and reinterpreting it
// would corrupt every plan change ever computed.
expect($contract->fresh()->price_cents)->toBe(17900);
});
it('moves a contract back onto the gross price when a registration lapses', function () {
[, $customer] = verifiedEuBusiness();
$contract = Subscription::factory()->plan('team')->create([
'customer_id' => $customer->id,
'stripe_subscription_id' => 'sub_lapse',
'stripe_item_id' => 'si_lapse',
'stripe_price_id' => packagePrice('team', TaxTreatment::reverseCharge()),
]);
$this->artisan('stripe:reprice-subscriptions')->assertSuccessful();
expect($this->stripe->priceChanges)->toBeEmpty();
// The company is wound up, or the registration is withdrawn. From that moment
// we are the ones liable for the VAT nobody collected, so the customer goes
// back onto the domestic Price.
$verifier = new FakeVatIdVerifier;
$verifier->notRegistered('NL123456789B01');
app()->instance(VatIdVerifier::class, $verifier);
$this->artisan('clupilot:verify-vat-ids')->assertSuccessful();
expect($customer->fresh()->hasVerifiedVatId())->toBeFalse();
$this->artisan('stripe:reprice-subscriptions')->assertSuccessful();
expect($this->stripe->priceChanges)->toHaveCount(1)
->and($this->stripe->priceChanges[0]['price'])->toBe(packagePrice('team', TaxTreatment::domestic()))
->and($this->stripe->priceChanges[0]['proration'])->toBe(StripeClient::PRORATE_NONE)
->and($contract->fresh()->stripe_price_id)->toBe(packagePrice('team', TaxTreatment::domestic()));
});
it('creates no second Price for a figure it already has, however often it is run', function () {
$before = count($this->stripe->prices);
// A Stripe Price cannot be edited or deleted, so a run that minted duplicates
// would leave two live Prices for one row at one figure for ever — and nothing
// could tell afterwards which of them a customer is billed on.
$this->artisan('stripe:sync-catalogue')
->expectsOutputToContain('already in step')
->assertSuccessful();
$this->artisan('stripe:sync-catalogue')->assertSuccessful();
expect($this->stripe->prices)->toHaveCount($before)
// Two rows per priced catalogue row, both live, one per treatment.
->and(StripePlanPrice::query()->count())->toBe(16)
->and(StripePlanPrice::query()->whereNull('archived_at')->count())->toBe(16)
->and(StripePlanPrice::query()->where('reverse_charge', true)->count())->toBe(8)
// And no Price at one figure for one row twice, which is the thing the
// register's unique key exists to make impossible.
->and(StripePlanPrice::query()->distinct()->count('stripe_price_id'))->toBe(16);
});
it('leaves the net Price alone when the VAT rate moves, and puts a Price back on sale rather than minting a second', function () {
$domesticBefore = packagePrice('team', TaxTreatment::domestic());
$netPrice = packagePrice('team', TaxTreatment::reverseCharge());
$count = count($this->stripe->prices);
$modules = count(array_merge(array_keys((array) config('provisioning.addons')), ['storage']));
// The rate moves. Only the GROSS half of each pair is wrong: the net owes
// nothing to the rate, so a sweep that did not know the difference would
// withdraw every reverse-charge Price the first time an operator touched the
// Finance page.
Settings::set('company.tax_rate', 10.0);
$this->artisan('stripe:sync-catalogue')->assertSuccessful();
expect(packagePrice('team', TaxTreatment::domestic()))->not->toBe($domesticBefore)
->and($this->stripe->archived)->toContain($domesticBefore)
->and(packagePrice('team', TaxTreatment::reverseCharge()))->toBe($netPrice)
->and($this->stripe->archived)->not->toContain($netPrice);
// And back again. A Stripe Price cannot be edited, so the one for the figure
// we have returned to is still there — brought back on sale rather than
// duplicated, AT STRIPE as well as here. Only in our own table it would leave
// the catalogue pointing at an inactive Price, and the failure would be a
// customer meeting a refused checkout.
Settings::set('company.tax_rate', 20.0);
$this->artisan('stripe:sync-catalogue')->assertSuccessful();
expect(packagePrice('team', TaxTreatment::domestic()))->toBe($domesticBefore)
->and($this->stripe->activated)->toContain($domesticBefore)
->and($this->stripe->archived)->not->toContain($domesticBefore)
// What the rate change minted and nothing more: one 10 % gross Price for
// each of the eight priced rows and for each module on each interval. No
// duplicate of anything that existed before, and nothing new on the net
// side at all.
->and($this->stripe->prices)->toHaveCount($count + 8 + $modules * 2);
});
it('refuses the sale rather than overcharging a business the catalogue has no net price for', function () {
// The state a deploy leaves behind until stripe:sync-catalogue has run: the
// domestic Price exists and the net one does not. There IS a fallback here and
// it must not be taken — putting them on the domestic Price takes a fifth more
// than they owe, which is the whole defect. A refused checkout is loud, and one
// command cures it.
StripePlanPrice::query()->where('reverse_charge', true)->delete();
[$user] = verifiedEuBusiness();
$this->actingAs($user)->post(route('checkout.start'), ['plan' => 'team', 'immediate_start' => '1'])
->assertSessionHasErrors('plan');
expect($this->stripe->checkouts)->toBeEmpty();
});

View File

@ -53,8 +53,10 @@ it('charges the fee it advertises, as a one-off line beside the monthly one', fu
$checkout = $this->stripe->checkouts[0];
// Gross, like every other figure this platform charges: the price on the page
// is the price at the till, for everybody.
// Gross, like every other figure a domestic customer is charged: the price on
// the page is the price at the till. A verified business in another member
// state is charged the bare net of the fee, exactly as of the package — see
// ReverseChargePriceTest.
expect($checkout['one_off'])->not->toBeNull()
->and($checkout['one_off']['amount_cents'])->toBe(11880)
->and($checkout['one_off']['currency'])->toBe('EUR')

View File

@ -88,24 +88,39 @@ function addonPlanPrice(Subscription $contract): string
->value('stripe_price_id');
}
/** The Stripe Price a module is sold at, monthly, at today's catalogue figure. */
/**
* The Stripe Price a module is sold at, monthly, at today's catalogue figure
* to a customer who is charged VAT.
*
* There are two Prices per module and interval now: this one and the bare net a
* verified business in another member state is charged. The contracts in this
* file are domestic, so the domestic treatment is the one their items bill on.
*/
function addonModulePrice(string $key, int $monthlyCents): string
{
return (string) app(AddonPrices::class)->liveFor($key, $monthlyCents, 'EUR', Subscription::TERM_MONTHLY);
return (string) app(AddonPrices::class)->liveFor(
$key,
$monthlyCents,
'EUR',
Subscription::TERM_MONTHLY,
TaxTreatment::domestic(),
);
}
/**
* What Stripe actually takes for a net catalogue figure.
* What Stripe actually takes for a net catalogue figure from an ordinary,
* domestic customer.
*
* The catalogue is pushed to Stripe GROSS, so every amount on a Stripe invoice
* line already contains the VAT. The fixtures below state the net figure a
* reader recognises from the price sheet and put it through the same call the
* sync does, rather than writing 21480 and leaving the next reader to work out
* where it came from.
* Their Price carries the GROSS, so every amount on a Stripe invoice line for them
* already contains the VAT. The fixtures below state the net figure a reader
* recognises from the price sheet and put it through the same call the sync does,
* rather than writing 21480 and leaving the next reader to work out where it came
* from. A reverse-charge business is charged the bare net instead, and that is
* tested where it belongs, in ReverseChargePriceTest.
*/
function atTheTill(int $netCents): int
{
return TaxTreatment::chargedCents($netCents);
return TaxTreatment::advertisedCents($netCents);
}
/**

View File

@ -37,10 +37,19 @@ function stripeContract(string $stripeId = 'sub_1'): Subscription
]);
}
/** The plan half of the mirror — modules live in the same lists. */
function planPrices(FakeStripeClient $stripe): Collection
/**
* The plan half of the mirror modules live in the same lists.
*
* Two Prices per priced row, not one: the domestic gross everybody who is charged
* VAT pays, and the bare net a verified business in another member state owes.
* `$treatment` picks one of the two where a test is about a figure rather than
* about how many objects there are.
*/
function planPrices(FakeStripeClient $stripe, ?string $treatment = null): Collection
{
return collect($stripe->prices)->filter(fn ($price) => isset($price['metadata']['plan_family']));
return collect($stripe->prices)
->filter(fn ($price) => isset($price['metadata']['plan_family']))
->filter(fn ($price) => $treatment === null || $price['metadata']['tax_treatment'] === $treatment);
}
function planProducts(FakeStripeClient $stripe): Collection
@ -48,7 +57,7 @@ function planProducts(FakeStripeClient $stripe): Collection
return collect($stripe->products)->filter(fn ($product) => isset($product['metadata']['plan_family']));
}
/** The module half: a Price per module and term, so a booking has one to bill on. */
/** The module half: a Price per module, term and treatment, so a booking has one to bill on. */
function modulePrices(FakeStripeClient $stripe): Collection
{
return collect($stripe->prices)->filter(fn ($price) => isset($price['metadata']['addon']));
@ -59,32 +68,44 @@ it('mirrors the catalogue into Stripe, once', function () {
$this->artisan('stripe:sync-catalogue')->assertSuccessful();
// A product per family, a price per priced row of a published version.
// A product per family, and TWO prices per priced row of a published
// version: the domestic gross and the bare net a reverse-charge business is
// charged. Both are minted up front, because the Price has to exist before
// the checkout that needs it.
expect(planProducts($stripe))->toHaveCount(4)
->and(planPrices($stripe))->toHaveCount(8)
->and(planPrices($stripe))->toHaveCount(16)
->and(planPrices($stripe, 'domestic'))->toHaveCount(8)
->and(planPrices($stripe, 'reverse_charge'))->toHaveCount(8)
->and(PlanFamily::query()->whereNull('stripe_product_id')->count())->toBe(0)
// The catalogue row still points at the DOMESTIC Price, which is the
// ordinary sale; the net one is only ever read out of the register.
->and(PlanPrice::query()->whereNull('stripe_price_id')->count())->toBe(0);
// And every module we sell, on both terms: a Stripe Price carries its own
// interval, so a module on a yearly contract needs a yearly one. Without
// these a booked module could never become an item on the subscription,
// which is why it was charged in the month it was booked and never again.
// Times two again, for the same reason as the packages.
$modules = array_merge(array_keys((array) config('provisioning.addons')), ['storage']);
$storageYear = StripeAddonPrice::query()->where('addon_key', 'storage')->where('interval', 'year');
// A yearly module Price is twelve months of the NET figure, grossed once —
// which is what a yearly customer is actually charged for it.
expect(modulePrices($stripe))->toHaveCount(count($modules) * 2)
->and(StripeAddonPrice::query()->where('addon_key', 'storage')->where('interval', 'year')->value('amount_cents'))
->toBe(TaxTreatment::chargedCents(12 * (int) config('provisioning.storage_addon.price_cents')))
->and(StripeAddonPrice::query()->where('addon_key', 'storage')->where('interval', 'year')->value('net_cents'))
// which is what a yearly customer is actually charged for it. The
// reverse-charge Price beside it charges that net figure and nothing on top.
expect(modulePrices($stripe))->toHaveCount(count($modules) * 4)
->and((clone $storageYear)->where('reverse_charge', false)->value('amount_cents'))
->toBe(TaxTreatment::advertisedCents(12 * (int) config('provisioning.storage_addon.price_cents')))
->and((clone $storageYear)->where('reverse_charge', true)->value('amount_cents'))
->toBe(12 * (int) config('provisioning.storage_addon.price_cents'))
->and((clone $storageYear)->where('reverse_charge', false)->value('net_cents'))
->toBe(12 * (int) config('provisioning.storage_addon.price_cents'));
// A Stripe Price cannot be edited, so a second run that minted duplicates
// would leave two live prices for one plan and no way to tell them apart.
$this->artisan('stripe:sync-catalogue')->assertSuccessful();
expect(planPrices($stripe))->toHaveCount(8)
->and(modulePrices($stripe))->toHaveCount(count($modules) * 2)
expect(planPrices($stripe))->toHaveCount(16)
->and(modulePrices($stripe))->toHaveCount(count($modules) * 4)
->and($stripe->products)->toHaveCount(4 + count($modules));
});
@ -92,19 +113,28 @@ it('gives each price its own recurring interval', function () {
$stripe = fakeStripe();
$this->artisan('stripe:sync-catalogue');
$intervals = planPrices($stripe)->groupBy('interval')->map->count();
$intervals = planPrices($stripe, 'domestic')->groupBy('interval')->map->count();
// Monthly and yearly cannot share a Price, because the interval belongs to
// the Price itself.
expect($intervals['month'])->toBe(4)
->and($intervals['year'])->toBe(4);
// The GROSS figure, which is the one on the website and the one taken from
// the card. The catalogue row stays net.
$team = planPrices($stripe)->first(fn ($p) => $p['metadata']['plan_family'] === 'team' && $p['interval'] === 'month');
// The DOMESTIC GROSS, which is the figure on the website and the one taken
// from the card of everybody who is charged VAT. The catalogue row stays net.
$team = planPrices($stripe, 'domestic')
->first(fn ($p) => $p['metadata']['plan_family'] === 'team' && $p['interval'] === 'month');
expect($team['amount'])->toBe(21480)
->and($team['amount'])->toBe(TaxTreatment::chargedCents(17900))
->and($team['amount'])->toBe(TaxTreatment::advertisedCents(17900))
->and($team['metadata']['plan_version'])->toBe('1');
// And the bare net beside it, on the same Product, for the business that owes
// no VAT at all. One Price for everybody was the overcharge: 214,80 € taken
// for a 179,00 € package from exactly the customers who read their invoices.
$reverse = planPrices($stripe, 'reverse_charge')
->first(fn ($p) => $p['metadata']['plan_family'] === 'team' && $p['interval'] === 'month');
expect($reverse['amount'])->toBe(17900)
->and($reverse['product'])->toBe($team['product']);
});
it('does not put an unpublished draft in the price list', function () {
@ -121,7 +151,7 @@ it('does not put an unpublished draft in the price list', function () {
// A draft has promised nothing; a Price for it would be a price list entry
// for something that may never exist.
expect(planPrices($stripe))->toHaveCount(8)
expect(planPrices($stripe))->toHaveCount(16)
->and(planPrices($stripe)->pluck('amount'))->not->toContain(19900);
});
@ -140,7 +170,7 @@ it('does not mint a second object when a run is interrupted before the id is sto
// catalogue reconnects to what is already there instead of duplicating it —
// and a Price cannot be deleted afterwards to tidy up.
expect(planProducts($stripe))->toHaveCount(4)
->and(planPrices($stripe))->toHaveCount(8)
->and(planPrices($stripe))->toHaveCount(16)
->and(PlanPrice::query()->whereNull('stripe_price_id')->count())->toBe(0);
});

View File

@ -7,6 +7,7 @@ use App\Models\Instance;
use App\Models\Order;
use App\Models\ProvisioningRun;
use App\Models\User;
use App\Provisioning\RunRunner;
use App\Provisioning\Steps\Customer\ConfigureDnsAndTls;
use App\Provisioning\Steps\Customer\ConfigureNextcloud;
use App\Services\Billing\CustomDomainAccess;
@ -44,9 +45,13 @@ function servedInstance(array $attributes = [], string $subdomain = 'berger'): a
'guest_ip' => '10.20.0.7',
'subdomain' => $subdomain,
'status' => 'active',
// The state a re-apply actually finds: built, routed, certified.
// The state a re-apply actually finds: built, routed, certified. Both
// halves of what the router file says are recorded, hostnames AND
// backend — an instance whose `routed_backend` were left null would be
// one the step has to rewrite, which is not the state being described.
'route_written' => true,
'routed_hostnames' => [platformAddress($subdomain)],
'routed_backend' => '10.20.0.7',
'cert_ok' => true,
], $attributes));
@ -311,6 +316,77 @@ it('does not start a second run while one is already going', function () {
->and(ProvisioningRun::query()->where('pipeline', 'address')->count())->toBe(1);
});
it('does start one beside a run that will not apply the address itself', function () {
// The guard used to stand aside for ANY run in flight, on the reasoning that
// whatever is running reads the domain state when it gets to the step. True of
// `customer` and `plan-change`, which carry both address steps. False of a
// restart, which carries neither — and nothing looks again afterwards, because
// the nightly proof check only re-applies on the FLIP from unproven to proven
// and that flip has already happened. So a domain proven while the customer's
// machine happened to be going round was announced, charged for, and never
// routed for the life of the instance.
Queue::fake();
$fixture = servedInstance([
'custom_domain' => 'cloud.berger.at', 'domain_token' => 'tok', 'domain_verified_at' => now(),
]);
foreach (['restart', 'storage', 'quota'] as $pipeline) {
ProvisioningRun::query()->delete();
ProvisioningRun::factory()->create([
'subject_type' => Order::class,
'subject_id' => $fixture['order']->id,
'pipeline' => $pipeline,
'status' => ProvisioningRun::STATUS_RUNNING,
]);
expect(app(ReapplyInstanceAddress::class)($fixture['instance']))->not->toBeNull()
->and(ProvisioningRun::query()->where('pipeline', 'address')->count())->toBe(1);
}
// And still stands aside for a plan change, which reuses these two steps and
// will read the same domain state when it reaches them.
ProvisioningRun::query()->delete();
ProvisioningRun::factory()->create([
'subject_type' => Order::class,
'subject_id' => $fixture['order']->id,
'pipeline' => 'plan-change',
'status' => ProvisioningRun::STATUS_RUNNING,
]);
expect(app(ReapplyInstanceAddress::class)($fixture['instance']))->toBeNull();
});
it('keeps the domain unannounced when the run fails before it has been routed', function () {
// The half-finished address, which is the ordinary outcome of a failed run.
// With the router written first, a run that got the certificate and then fell
// over at Nextcloud left the customer's own domain serving Nextcloud's
// untrusted-domain error page under a valid certificate — and `domain_cert_ok`
// was already true, so the portal was printing that address as live.
Queue::fake();
$s = fakeServices();
$fixture = servedInstance([
'custom_domain' => 'cloud.berger.at', 'domain_token' => 'tok', 'domain_verified_at' => now(),
]);
// Nextcloud refuses the trusted_domains write; guest() throws, which the
// runner turns into a retry, so the run never reaches the router at all.
$s['pve']->guestScript('config:system:set trusted_domains', 1);
$run = addressRun($fixture);
$run->update(['status' => ProvisioningRun::STATUS_RUNNING, 'current_step' => 0]);
app(RunRunner::class)->advance($run);
$instance = $fixture['instance']->fresh();
expect($run->fresh()->status)->toBe(ProvisioningRun::STATUS_WAITING)
->and($run->fresh()->current_step)->toBe(0)
// Nothing announced, nothing routed: the customer is still on the platform
// address, which is the honest half to be left holding.
->and($instance->domain_cert_ok)->toBeFalse()
->and($instance->routed_hostnames)->toBe([platformAddress()])
->and($s['traefik']->serves('berger', 'cloud.berger.at'))->toBeFalse();
});
it('stops serving the old domain the moment the customer changes or clears it', function () {
Queue::fake();

View File

@ -1,9 +1,10 @@
<?php
use App\Support\NextcloudOcc;
use Illuminate\Support\Facades\File;
/**
* Nothing runs inside the app container without saying who it is running as.
* Nothing runs inside a container without saying who it is running as.
*
* `docker compose exec` is root unless told otherwise, and the deployment
* scripts relied on that default. When `artisan optimize` failed during a
@ -50,6 +51,37 @@ it('never runs a command in the app container as whoever docker felt like', func
expect($offenders)->toBe([]);
});
it('never runs occ in a customer guest as whoever docker felt like either', function () {
// The same default, the same lesson, a different subsystem — and it was learned
// here second. `docker compose exec` is root, the official Nextcloud image's
// console.php exits 1 unless the caller owns config/config.php (www-data), and
// the prefix was pasted into five separate files, so every occ call in the
// product was failing on every instance. The ones that go through
// CustomerStep::guest() turned that into a retry and then a failed run for a
// machine that was otherwise finished.
//
// So there is one builder, and this refuses a second: nothing in app/ may spell
// the invocation out by hand, which is the only way a sixth call site can be
// stopped from getting it wrong again.
$offenders = [];
foreach (File::allFiles(app_path()) as $file) {
if ($file->getExtension() !== 'php'
|| $file->getRelativePathname() === 'Support/NextcloudOcc.php') {
continue;
}
if (str_contains(File::get($file->getRealPath()), 'docker compose exec')) {
$offenders[] = $file->getRelativePathname();
}
}
expect($offenders)->toBe([])
// And the one builder names the user Nextcloud insists on.
->and(NextcloudOcc::command('status'))
->toContain('docker compose exec -T -u www-data ');
});
it('repairs ownership before it needs it, not after', function () {
// A server already carrying the damage has to heal on its next deployment.
// Nothing else ever will: a root-owned log file stays root-owned, and the

View File

@ -46,7 +46,16 @@ it('provisions a paid order all the way to active (mocked)', function () {
->and($instance->status)->toBe('active')
->and($instance->vmid)->not->toBeNull()
->and($instance->cert_ok)->toBeTrue()
->and($instance->onboardingTasks()->count())->toBeGreaterThan(0);
->and($instance->onboardingTasks()->count())->toBeGreaterThan(0)
// The storage allowance was DELIVERED, not merely walked past. This test
// covered all sixteen steps and asserted nothing about the quota one, so it
// proved that inserting the step did not break the pipeline rather than
// that the step does anything — and quota_applied_gb is written only after
// the guest accepted the figure, which is the whole distinction between an
// enforced allowance and a number in our database.
->and($instance->quota_applied_gb)->toBe($instance->quota_gb)
->and($instance->quota_applied_gb)->toBeGreaterThan(0)
->and($s['pve']->guestRan('config:app:set files default_quota'))->toBeTrue();
// Every external resource created exactly once.
foreach (['instance_id', 'vmid', 'dns_record_id', 'nc_admin', 'backup_job_id', 'monitoring_target_id'] as $kind) {

View File

@ -5,7 +5,9 @@ use App\Models\Instance;
use App\Models\Order;
use App\Models\ProvisioningRun;
use App\Models\RunResource;
use App\Models\Subscription;
use App\Notifications\CloudReady;
use App\Provisioning\RunRunner;
use App\Provisioning\Steps\Customer\CloneVirtualMachine;
use App\Provisioning\Steps\Customer\CompleteProvisioning;
use App\Provisioning\Steps\Customer\ConfigureCloudInit;
@ -64,9 +66,19 @@ it('validates a paid order and moves it to provisioning', function () {
->and($run->subject->fresh()->status)->toBe('provisioning');
});
it('fails validation for an unpaid order or unknown plan', function () {
expect(app(ValidateOrder::class)->execute(orderRun(['status' => 'pending']))->type)->toBe('fail');
expect(app(ValidateOrder::class)->execute(orderRun(['status' => 'paid', 'plan' => 'ghost']))->type)->toBe('fail');
it('fails validation for an unpaid order, and for a paid one with no contract', function () {
// Named for what the step actually checks. There is no plan check in
// ValidateOrder at all — an unknown plan fails because OpenSubscription could
// not price it and so froze no contract, which is a different sentence and the
// one that matters: nothing is built from today's catalogue. Asserting the
// reason is what keeps the two apart; on ->type alone this passed either way.
$unpaid = app(ValidateOrder::class)->execute(orderRun(['status' => 'pending']));
expect($unpaid->type)->toBe('fail')
->and($unpaid->reason)->toBe('order_not_paid');
$ghost = app(ValidateOrder::class)->execute(orderRun(['status' => 'paid', 'plan' => 'ghost']));
expect($ghost->type)->toBe('fail')
->and($ghost->reason)->toBe('no_subscription');
});
// 2. ReserveResources
@ -150,18 +162,39 @@ it('recovers a lost-task clone by waiting for the lock then re-cloning cleanly',
->and(RunResource::where('run_id', $run->id)->where('kind', 'vmid')->exists())->toBeFalse();
});
it('fails clone when the plan has no template', function () {
it('fails clone when the contract carries no template', function () {
// This used to write config('provisioning.plans.start.template_vmid') — a key
// that has not existed since the catalogue moved into the database, so the
// write was a no-op and the run failed for want of a CONTRACT instead. The
// case the test is named for was never exercised.
//
// The template comes off the frozen snapshot, so the state to build is a
// contract with no blueprint on it: what a pre-catalogue subscription looks
// like before the backfill fills it in. PlanCatalogue::publish() has refused
// to put a version on sale without one ever since, and this is the failure
// that refusal exists to keep away from a paying customer.
fakeServices();
$host = Host::factory()->active()->create(['node' => 'pve']);
$order = Order::factory()->create(['plan' => 'start']);
$instance = Instance::factory()->create(['order_id' => $order->id, 'customer_id' => $order->customer_id, 'host_id' => $host->id]);
Subscription::factory()->create([
'customer_id' => $order->customer_id,
'order_id' => $order->id,
'instance_id' => $instance->id,
'template_vmid' => null,
]);
$run = ProvisioningRun::factory()->create([
'subject_type' => Order::class, 'subject_id' => $order->id, 'pipeline' => 'customer',
'context' => ['instance_id' => $instance->id, 'node' => 'pve'],
]);
config()->set('provisioning.plans.start.template_vmid', 0);
expect(app(CloneVirtualMachine::class)->execute($run)->type)->toBe('fail');
$result = app(CloneVirtualMachine::class)->execute($run);
expect($result->type)->toBe('fail')
->and($result->reason)->toBe('template_missing')
// It is the template that is missing and not the contract — otherwise this
// is the previous test over again under a misleading name.
->and($order->subscription()->exists())->toBeTrue();
});
// 4. ConfigureCloudInit
@ -346,13 +379,74 @@ it('passes acceptance only when everything is genuinely green', function () {
config()->set('provisioning.monitoring.required', false);
$s['monitoring']->healthy = true;
// Nextcloud not installed → fail
// A probe that says no asks again rather than condemning a finished cloud:
// certReachable() returns the same false for a connect timeout as for a
// missing certificate, and this step returning fail() bypassed the retry
// budget entirely — one bad second ended a working Nextcloud as a failed
// order with the instance released.
$s['pve']->guestScript('occ status', 0, '{"installed":false}');
expect(app(RunAcceptanceChecks::class)->execute($run->fresh())->type)->toBe('fail');
$nextcloud = app(RunAcceptanceChecks::class)->execute($run->fresh());
expect($nextcloud->type)->toBe('retry')
->and($nextcloud->reason)->toBe('acceptance_failed:nextcloud');
// cert not reachable → fail
$s['traefik']->certReady = false;
expect(app(RunAcceptanceChecks::class)->execute($run->fresh())->type)->toBe('fail');
$cert = app(RunAcceptanceChecks::class)->execute($run->fresh());
expect($cert->type)->toBe('retry')
->and($cert->reason)->toBe('acceptance_failed:cert');
});
it('still fails for good when what is wrong is a fact no retry can change', function () {
// The two checks that stay terminal read state THIS run wrote earlier, out of
// our own database. Asking again five times only delays saying so.
$s = fakeServices();
$s['pve']->guestScript('occ status', 0, '{"installed":true,"maintenance":false}');
['run' => $run, 'instance' => $instance, 'host' => $host] = reservedRun([], ['cert_ok' => true]);
$instance->monitoringTargets()->create(['external_id' => 'mon-1', 'url' => 'https://x/status.php', 'status' => 'up']);
// No backup breadcrumb: RegisterBackup never registered a job.
$missingBackup = app(RunAcceptanceChecks::class)->execute($run->fresh());
expect($missingBackup->type)->toBe('fail')
->and($missingBackup->reason)->toBe('acceptance_failed:backup');
RunResource::create(['run_id' => $run->id, 'host_id' => $host->id, 'kind' => 'backup_job_id', 'external_id' => 'b']);
// No platform certificate — which ConfigureDnsAndTls fails the run over
// itself, so standing here without it is a state the pipeline cannot reach.
$instance->update(['cert_ok' => false]);
$missingCert = app(RunAcceptanceChecks::class)->execute($run->fresh());
expect($missingCert->type)->toBe('fail')
->and($missingCert->reason)->toBe('acceptance_failed:cert');
});
it('spends the retry budget and then fails with the reason the probe gave', function () {
// The whole point of retrying instead of failing: five noes in a row is not a
// blip, and the run still ends failed — named for what was wrong, not
// 'step timed out'.
$s = fakeServices();
$s['pve']->guestScript('occ status', 0, '{"installed":false}');
['run' => $run, 'instance' => $instance, 'host' => $host] = reservedRun([], ['cert_ok' => true]);
RunResource::create(['run_id' => $run->id, 'host_id' => $host->id, 'kind' => 'backup_job_id', 'external_id' => 'b']);
$instance->monitoringTargets()->create(['external_id' => 'mon-1', 'url' => 'https://x/status.php', 'status' => 'up']);
$index = array_search(RunAcceptanceChecks::class, config('provisioning.pipelines.customer'), true);
$run->update(['current_step' => $index, 'status' => 'running', 'max_attempts' => 3]);
$runner = app(RunRunner::class);
for ($i = 0; $i < 5; $i++) {
// Re-read before writing. Saving the stale model would put the attempt
// count back to what it was and the loop would never reach the budget.
$run = $run->fresh();
if ($run->status === 'failed') {
break;
}
$run->forceFill(['next_attempt_at' => null, 'started_at' => now()])->save();
$runner->advance($run);
}
expect($run->fresh()->status)->toBe('failed')
->and($run->fresh()->error)->toBe('acceptance_failed:nextcloud');
});
// 15. CompleteProvisioning

View File

@ -13,6 +13,7 @@ use App\Models\User;
use App\Provisioning\RunRunner;
use App\Provisioning\Steps\Customer\CompleteRestart;
use App\Provisioning\Steps\Customer\ShutDownVirtualMachine;
use App\Support\ProvisioningSettings;
use Illuminate\Auth\Access\AuthorizationException;
use Illuminate\Support\Facades\Queue;
use Livewire\Livewire;
@ -63,6 +64,11 @@ function restartableInstance(array $instanceAttributes = []): array
$pve->startVm('pve', 101);
$pve->setCloudInit('pve', 101, ['cores' => 8, 'memory' => 16384]);
// The address the guest comes back on. The restart pipeline reads it — a
// cold boot is the one event that can move a DHCP lease — and would poll for
// it until the step's deadline if the fake answered nothing.
$pve->guestScript('hostname -I', 0, '10.20.0.7');
return compact('services', 'pve', 'host', 'order', 'instance');
}
@ -185,13 +191,18 @@ it('leaves the pending restart alone until the very last step of the run', funct
$run = app(RestartInstance::class)($instance);
$runner = app(RunRunner::class);
// Shutdown asked, shutdown observed, machine started, agent answered — and
// through all of it the flag stands, because none of those is the machine
// running on what was bought.
for ($i = 0; $i < 6; $i++) {
// Read off the pipeline rather than written out, so inserting a step cannot
// quietly turn "the last one" into "the one before it" and leave this test
// green while proving something weaker.
$last = count((array) config('provisioning.pipelines.restart')) - 1;
// Shutdown asked, shutdown observed, machine started, agent answered, address
// read — and through all of it the flag stands, because none of those is the
// machine running on what was bought.
for ($i = 0; $i < 10; $i++) {
$run->refresh();
if ($run->current_step >= 3) {
if ($run->current_step >= $last) {
break;
}
@ -201,7 +212,7 @@ it('leaves the pending restart alone until the very last step of the run', funct
expect($instance->fresh()->restartIsPending())->toBeTrue();
}
expect($run->refresh()->current_step)->toBe(3)
expect($run->refresh()->current_step)->toBe($last)
->and($instance->fresh()->restartIsPending())->toBeTrue();
driveRun($run);
@ -314,6 +325,39 @@ it('refuses the console action to an operator who does not hold the capability',
expect(ProvisioningRun::query()->where('pipeline', 'restart')->count())->toBe(0);
});
it('follows the guest when a restart brings it back on another address', function () {
// cloud-init hands every guest ip=dhcp, so the address is a lease and not a
// property of the machine. The router file on the serving host names it as the
// backend — so a machine that comes back somewhere else, with nothing
// rewriting that file, is a customer's cloud answering 502 for the rest of its
// life. Nothing in the product re-read the address at all, and the route guard
// compared only the hostname list, so neither half could notice.
Queue::fake();
['pve' => $pve, 'instance' => $instance] = restartableInstance([
// What the router was last told, and where the machine actually is now.
'routed_backend' => '10.20.0.7',
'route_written' => true,
'routed_hostnames' => ['berger.'.ProvisioningSettings::dnsZone()],
'subdomain' => 'berger',
'cert_ok' => true,
]);
$pve->guestScript('hostname -I', 0, '10.20.0.44 fe80::1');
$this->actingAs(admin(), 'operator');
driveRun(app(RestartInstance::class)($instance));
expect($instance->fresh()->guest_ip)->toBe('10.20.0.44')
// An address run, because writing the router is that pipeline's job. It is
// allowed to start beside the restart because the guard asks whether the
// run in flight carries out the address steps, and a restart carries
// neither of them.
->and(ProvisioningRun::query()->where('pipeline', 'address')->count())->toBe(1);
driveRun(ProvisioningRun::query()->where('pipeline', 'address')->sole());
expect($instance->fresh()->routed_backend)->toBe('10.20.0.44');
});
it('gives the shutdown step nothing to do when the guest is already stopped', function () {
Queue::fake();
['pve' => $pve, 'instance' => $instance, 'order' => $order] = restartableInstance();

View File

@ -1,6 +1,11 @@
<?php
use App\Models\Order;
use App\Models\ProvisioningRun;
use App\Provisioning\PipelineRegistry;
use App\Provisioning\Steps\Customer\ConfigureDnsAndTls;
use App\Provisioning\Steps\Customer\ConfigureNextcloud;
use App\Provisioning\WorkInFlight;
use Tests\Support\Steps\FakeAdvanceStep;
use Tests\Support\Steps\FakeRetryStep;
@ -19,3 +24,86 @@ it('throws for an unknown pipeline', function () {
it('throws for an out-of-range step index', function () {
(new PipelineRegistry(['host' => [FakeAdvanceStep::class]]))->resolve('host', 5);
})->throws(InvalidArgumentException::class);
it('answers whether a run still to come carries out another pipeline whole', function () {
// The question an action about to start a run has to ask about the run already
// in flight. Asking merely "is something running?" is what made a domain
// proven during a restart never get routed, and a storage pack booked during
// an address run get charged monthly and never delivered.
$registry = new PipelineRegistry([
'both' => [FakeAdvanceStep::class, FakeRetryStep::class],
'first' => [FakeAdvanceStep::class],
'second' => [FakeRetryStep::class],
]);
// Everything still ahead of it.
expect($registry->stillCovers('both', 0, 'first'))->toBeTrue()
->and($registry->stillCovers('both', 0, 'second'))->toBeTrue()
// EVERY step, not any: half of an ordered piece of work is worse than
// none of it — a quota enforced over a filesystem that never grew.
->and($registry->stillCovers('first', 0, 'both'))->toBeFalse()
// Already past it: a run standing at its second step will not go back and
// do its first, which is the case a name comparison cannot see at all.
->and($registry->stillCovers('both', 1, 'first'))->toBeFalse()
->and($registry->stillCovers('both', 1, 'second'))->toBeTrue()
// The step it is standing on counts as still to come — it is about to run,
// or running, and every step reads the state when it gets there.
->and($registry->stillCovers('second', 0, 'second'))->toBeTrue();
});
it('asks that question of the runs actually in flight against an order', function () {
$order = Order::factory()->create();
$work = app(WorkInFlight::class);
$running = fn (string $pipeline) => ProvisioningRun::factory()->create([
'subject_type' => Order::class, 'subject_id' => $order->id,
'pipeline' => $pipeline, 'status' => ProvisioningRun::STATUS_RUNNING,
]);
expect($work->covers($order, 'address'))->toBeFalse()
->and($work->covers($order, 'storage'))->toBeFalse();
// A quota run writes the figure Nextcloud enforces and nothing else — it does
// not grow the disk that figure is a promise about, so it does not stand for a
// storage change. This is the pair that had a booked pack charged every month
// and never delivered.
$running('quota');
expect($work->covers($order, 'storage'))->toBeFalse()
->and($work->covers($order, 'address'))->toBeFalse();
// A plan change carries both pipelines whole; there is genuinely nothing to add.
ProvisioningRun::query()->delete();
$running('plan-change');
expect($work->covers($order, 'storage'))->toBeTrue()
->and($work->covers($order, 'address'))->toBeTrue();
// A run that has finished covers nothing, however much it contained.
ProvisioningRun::query()->update(['status' => ProvisioningRun::STATUS_COMPLETED]);
expect($work->covers($order, 'storage'))->toBeFalse()
->and($work->covers($order, 'address'))->toBeFalse();
// A pipeline that no longer exists — renamed by a deploy while a run was in
// flight — covers nothing either. Refusing to start the work over it would
// lose the work as well as the run.
ProvisioningRun::query()->delete();
$running('gone-in-a-deploy');
expect($work->covers($order, 'address'))->toBeFalse();
});
it('lays the maintenance pipelines out so a half-finished address is a safe one', function () {
// Nextcloud BEFORE the router, in the address pipeline and in the plan change
// that reuses it. Router first meant a run could get the certificate and then
// fail at Nextcloud — leaving the customer's own domain serving Nextcloud's
// untrusted-domain error under a valid certificate, with domain_cert_ok true
// so the portal called it live. The reasoning for both directions is written
// out above the `address` pipeline in config/provisioning.php.
$nextcloud = ConfigureNextcloud::class;
$dns = ConfigureDnsAndTls::class;
foreach (['address', 'plan-change', 'customer'] as $pipeline) {
$steps = (array) config('provisioning.pipelines.'.$pipeline);
expect(array_search($nextcloud, $steps, true))
->toBeLessThan(array_search($dns, $steps, true));
}
});

View File

@ -175,6 +175,31 @@ it('re-executes a step after a timeout retry instead of timing out again', funct
expect($run->fresh()->status)->toBe('completed');
});
it('starts the clock afresh for a run that has been put back to pending', function () {
// A revived run — see StartCustomerProvisioning::reviveRunStrandedWithout-
// AContract(), which resets the status, the step, the attempt and the error
// when the contract a payment was missing finally appears. It does not reset
// started_at, and the clause here that was written for exactly this case used
// `??=`, so it never fired: the run kept the started_at of the attempt that
// failed hours earlier, its very first pass was ruled timed out before the
// step body ran, and that spent one of five attempts and wrote 'step timed
// out' into the log of a run that had not started.
Queue::fake();
bindPipeline(['test' => [FakeAdvanceStep::class]]);
$run = ProvisioningRun::factory()->create([
'pipeline' => 'test',
'status' => ProvisioningRun::STATUS_PENDING,
'started_at' => now()->subDays(2),
'max_attempts' => 5,
]);
app(RunRunner::class)->advance($run);
expect($run->fresh()->status)->toBe('completed')
->and($run->fresh()->attempt)->toBe(0)
->and($run->fresh()->events()->where('message', 'like', '%timed out%')->exists())->toBeFalse();
});
it('fails terminally when the pipeline step cannot be resolved', function () {
bindPipeline(['test' => [FakeAdvanceStep::class]]);
$run = ProvisioningRun::factory()->create(['pipeline' => 'test', 'current_step' => 9]);

View File

@ -9,6 +9,7 @@ use App\Services\Ssh\CommandResult;
use App\Services\Ssh\FakeRemoteShell;
use App\Services\Wireguard\FakeWireguardHub;
use App\Services\Wireguard\LocalWireguardHub;
use App\Support\ProvisioningSettings;
use Illuminate\Support\Facades\File;
use Illuminate\Support\Facades\Http;
@ -119,6 +120,52 @@ it('treats an already-deleted DNS record as done', function () {
(new HttpHetznerDnsClient)->deleteRecord('rec-123');
})->throwsNoExceptions();
it('finds an existing DNS record past the first page instead of creating a second one', function () {
// Hetzner pages `GET /records` at a hundred entries, and this asked only for
// the first page. Past a hundred records the lookup missed an entry that was
// plainly there, fell through to POST, and Hetzner accepted a SECOND A record
// for the same name: two addresses round-robined for one instance, one of them
// a host the machine is not on, so the cloud was up about half the time. The
// `address` and `plan-change` pipelines upsert on every run, so it compounded.
$filler = fn (int $from) => collect(range($from, $from + 99))
->map(fn (int $i) => ['id' => 'rec-'.$i, 'name' => 'other-'.$i, 'type' => 'A'])
->all();
// Matched by reading the page out of the query rather than with a URL glob:
// `page=1` is a substring of `per_page=100`, so a pattern would answer page
// two with page one's records and the test would pass against the bug.
Http::fake(function ($request) use ($filler) {
parse_str((string) parse_url($request->url(), PHP_URL_QUERY), $query);
if (str_contains($request->url(), '/zones')) {
return Http::response(['zones' => [['id' => 'zone-1']]]);
}
if ($request->method() === 'GET') {
$page = (int) ($query['page'] ?? 1);
return Http::response([
'records' => $page === 2
? array_merge($filler(101), [['id' => 'rec-berger', 'name' => 'berger', 'type' => 'A']])
: $filler(1),
'meta' => ['pagination' => ['page' => $page, 'per_page' => 100, 'last_page' => 2]],
]);
}
// A POST is the bug: the record exists and has to be updated.
return Http::response(['record' => ['id' => 'rec-new']], 201);
});
$id = (new HttpHetznerDnsClient)->upsertRecord(
'berger.'.ProvisioningSettings::dnsZone(), 'A', '203.0.113.9'
);
expect($id)->toBe('rec-berger');
Http::assertSent(fn ($request) => $request->method() === 'PUT'
&& str_contains($request->url(), '/records/rec-berger'));
Http::assertNotSent(fn ($request) => $request->method() === 'POST');
});
it('writes and removes a real hostsdir entry on disk (FileHostDnsDirectory)', function () {
$dir = sys_get_temp_dir().'/clupilot-dns-hosts-test-'.uniqid();
config()->set('provisioning.dns.hosts_dir', $dir);

View File

@ -4,12 +4,14 @@ use App\Actions\OpenSubscription;
use App\Models\Host;
use App\Models\Instance;
use App\Models\Order;
use App\Models\PlanVersion;
use App\Models\ProvisioningRun;
use App\Models\Subscription;
use App\Provisioning\Jobs\AdvanceRunJob;
use App\Provisioning\Steps\Customer\CloneVirtualMachine;
use App\Provisioning\Steps\Customer\ReserveResources;
use App\Provisioning\Steps\Customer\ValidateOrder;
use App\Services\Billing\PlanCatalogue;
use App\Services\Proxmox\ProxmoxClient;
use Illuminate\Support\Facades\DB;
use Illuminate\Support\Facades\Queue;
@ -38,6 +40,48 @@ function paidOrderWithSubscription(string $plan = 'team'): array
return compact('host', 'order', 'subscription', 'run');
}
/**
* The operator changes what a plan is, after somebody has already bought it.
*
* Publishing a successor and closing the old window is the ONLY way the catalogue
* can move: a published version is immutable, precisely so that what a customer
* bought keeps describing what they are owed. So it is also the only way this
* regression can be constructed and the tests below used to try it with
* config('provisioning.plans.*') writes, against a key that has not existed since
* the catalogue moved into plan_families/plan_versions/plan_prices. Every one of
* those writes was a no-op, so each test proved only that the contract agrees with
* itself.
*
* @param array<string, mixed> $changes
*/
function publishSuccessor(string $plan, array $changes): PlanVersion
{
$catalogue = app(PlanCatalogue::class);
$current = $catalogue->currentVersion($plan);
$currency = Subscription::catalogueCurrency();
$price = $current->priceFor(Subscription::TERM_MONTHLY)->amount_cents;
// Half-open windows: the old one ends at the instant the new one starts, so
// there is neither a gap nor two versions on sale at once.
$catalogue->schedule($current, $current->available_from, now());
$successor = PlanVersion::query()->create([
...$current->only([
'plan_family_id', 'quota_gb', 'traffic_gb', 'seats', 'ram_mb',
'cores', 'disk_gb', 'performance', 'template_vmid',
]),
...$changes,
'version' => $current->version + 1,
'features' => $current->features,
'available_from' => now(),
]);
$successor->prices()->create(['term' => 'monthly', 'amount_cents' => $price, 'currency' => $currency]);
$successor->prices()->create(['term' => 'yearly', 'amount_cents' => $price * 12, 'currency' => $currency]);
return $catalogue->publish($successor, now());
}
it('freezes the catalogue onto a subscription when the checkout is paid', function () {
Queue::fake();
@ -93,11 +137,15 @@ it('opens exactly one contract when Stripe retries the webhook', function () {
it('sizes the machine from the contract, not from a catalogue edited afterwards', function () {
['order' => $order, 'run' => $run] = paidOrderWithSubscription('team');
// The operator shrinks the team plan after this customer has paid for it.
config()->set('provisioning.plans.team.ram_mb', 4096);
config()->set('provisioning.plans.team.cores', 2);
config()->set('provisioning.plans.team.disk_gb', 120);
config()->set('provisioning.plans.team.quota_gb', 100);
// The operator cuts the team plan back after this customer has paid for it.
publishSuccessor('team', ['ram_mb' => 4096, 'cores' => 2, 'disk_gb' => 120, 'quota_gb' => 100]);
// The premise, asserted and not assumed. This edit used to be four
// config('provisioning.plans.team.*') writes — a key that has not existed
// since the catalogue moved into the database — so the catalogue was never
// touched at all and the test could not detect the regression it exists for.
expect(Subscription::snapshotFrom('team')['ram_mb'])->toBe(4096)
->and(Subscription::snapshotFrom('team')['disk_gb'])->toBe(120);
expect(app(ReserveResources::class)->execute($run)->type)->toBe('advance');
@ -128,8 +176,12 @@ it('clones the template the contract was sold with, not a later one', function (
]);
$run->mergeContext(['instance_id' => $instance->id, 'host_id' => $host->id, 'node' => 'pve']);
// A new blueprint is published after this customer bought.
config()->set('provisioning.plans.team.template_vmid', 9999);
// A new blueprint is published after this customer bought — the real thing,
// a successor version, because a published version's template is frozen with
// it. The line here was a config write against a key that no longer exists.
publishSuccessor('team', ['template_vmid' => 9999]);
expect(Subscription::snapshotFrom('team')['template_vmid'])->toBe(9999);
$cloned = null;
$pve = Mockery::mock(ProxmoxClient::class);