Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
97 changes: 79 additions & 18 deletions pkg/clouds/pulumi/vm/compose_stack.go
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,33 @@ func VmComposeStack(ctx *sdk.Context, stack api.Stack, input api.ResourceInput,
// run at once, and what the stack's URL is.
servesDomain := vm.ServesDomain(in, parentVm.IngressEnabled)

// The scheme the machine answers on. Caddy either terminates TLS or serves plain
// HTTP for an edge that does (`ingress.originTls: false` on the vm resource) — and
// every log line and export below has to say which, because "http" in a URL an
// operator pastes into a browser is the only visible difference between a working
// no-origin-TLS fleet and a broken HTTPS one.
originScheme := lo.If(parentVm.NoOriginTls, "http").Else("https")

// `domainProxied: true` asks for a Cloudflare-proxied record, and the proxy
// terminates TLS at the edge. That breaks the HTTP-01 challenge Caddy runs for
// itself — so it is legal only on a machine that runs no ACME.
//
// Refused here and not in validateStackConfig (clouds/vm/convert.go, which still
// carries the note) because the answer lives in the PARENT stack's resource and the
// convert tier holds no parent reference. The cost is a later refusal; the benefit
// is that the arrangement is reachable at all.
if lo.FromPtr(stackConfig.DomainProxied) && !parentVm.NoOriginTls {
return nil, errors.Errorf(
"stack %q sets `domainProxied: true`, but vm %q terminates TLS itself: Caddy obtains the "+
"certificate over HTTP-01, and a proxied domain answers that challenge at Cloudflare's edge "+
"instead of on the machine, so no certificate ever issues. Either set `ingress.originTls: "+
"false` on the `type: %s` resource in the parent stack and re-provision it — the machine then "+
"serves plain HTTP for Cloudflare to terminate in front — or drop `domainProxied` and let the "+
"machine serve HTTPS directly",
stackName, in.VmResource, vm.ResourceTypeVm,
)
}

// The ingress host port is published only when Caddy is NOT in front. That is not
// a tidiness choice: a published port is a singleton, and it is the one thing that
// prevents both slots from running at once (see stopOtherFirst below). Setting it
Expand All @@ -153,9 +180,9 @@ func VmComposeStack(ctx *sdk.Context, stack api.Stack, input api.ResourceInput,
"stack %q declares no ingress service, so nothing is reachable from outside the machine", stackName)
case servesDomain:
params.Log.Info(ctx.Context(),
"vm %q serves %q of stack %q on https://%s — no host port is published, and the deploy flips slots "+
"vm %q serves %q of stack %q on %s://%s — no host port is published, and the deploy flips slots "+
"with a Caddy reload",
in.VmResource, in.Deployment.IngressContainer.Name, stackName, stackConfig.Domain)
in.VmResource, in.Deployment.IngressContainer.Name, stackName, originScheme, stackConfig.Domain)
default:
hostPort = vm.IngressHostPort(stackName, env)
params.Log.Info(ctx.Context(), "publishing ingress service %q of stack %q on host port %d",
Expand Down Expand Up @@ -284,8 +311,9 @@ func VmComposeStack(ctx *sdk.Context, stack api.Stack, input api.ResourceInput,
ctx: ctx, stack: stack, input: input, params: params, in: in,
stackName: stackName, env: env, version: version, stackConfig: stackConfig,
plan: plan, hostPort: hostPort, servesDomain: servesDomain,
recreate: in.EffectiveStrategy() == vm.StrategyRecreate,
envHash: envHash, envContent: envContent,
noOriginTls: parentVm.NoOriginTls,
recreate: in.EffectiveStrategy() == vm.StrategyRecreate,
envHash: envHash, envContent: envContent,
}

// Rolling, not fanned out. The machines are independent and Pulumi would happily
Expand All @@ -311,11 +339,14 @@ func VmComposeStack(ctx *sdk.Context, stack api.Stack, input api.ResourceInput,
// on a host port and an A record at `domain` would resolve to a machine with
// nothing listening on 443. The warnings above have already said why in that case.
//
// `DomainProxied` is nil on this path today — validateStackConfig refuses it
// (clouds/vm/convert.go) because a proxied domain breaks the ACME challenge Caddy
// runs itself — so this reads false. It is read rather than hardcoded because the
// refusal is due to become conditional on the fleet declaring no origin TLS, and a
// hardcoded false would then be a silently ignored setting.
// `DomainProxied` is read rather than hardcoded because it is now reachable. The
// refusal moved into this function (above) and allows it on a fleet that declares
// `ingress.originTls: false`, where proxying is not merely permitted but the entire
// front door: Cloudflare terminates TLS and the machines answer plain HTTP, so an
// unproxied record would hand clients a cleartext origin. On a fleet that terminates
// its own TLS the refusal has already returned by here, so this still reads false
// there — a proxied domain answers the HTTP-01 challenge at the edge instead of on
// the machine and no certificate ever issues.
if servesDomain && stackConfig.Domain != "" {
if err := fleetDnsRecords(ctx, params, stackName, stackConfig.Domain,
parentVm.Addresses, lo.FromPtr(stackConfig.DomainProxied)); err != nil {
Expand Down Expand Up @@ -364,9 +395,13 @@ type composeDeploy struct {
plan *imagePlan
hostPort int
servesDomain bool
recreate bool
envHash string
envContent string
// noOriginTls is parentVmFacts.NoOriginTls, carried down because the site file and
// the exported URL are both written per host and both have to name the scheme the
// RESOURCE decided.
noOriginTls bool
recreate bool
envHash string
envContent string
}

// deployComposeToHost runs the whole deploy against ONE machine of the fleet.
Expand Down Expand Up @@ -622,10 +657,11 @@ func deployComposeToHost(
if servesDomain {
routeOut := slotOut.ApplyT(func(slot string) (string, error) {
site, err := vm.RenderCaddySite(in, vm.RenderOptions{
Stack: stackName,
Env: env,
Slot: slot,
Version: version,
Stack: stackName,
Env: env,
Slot: slot,
Version: version,
NoOriginTls: d.noOriginTls,
})
if err != nil {
return "", errors.Wrapf(err, "failed to render the Caddy site of stack %q slot %q", stackName, slot)
Expand Down Expand Up @@ -720,9 +756,15 @@ func deployComposeToHost(
// whether a domain is configured but from whether it is actually SERVED — a stack
// whose domain was ignored for one of the reasons warned about above must not
// export an https:// URL that answers nothing.
//
// The scheme follows the resource, not the hope: on `ingress.originTls: false` the
// machine answers on :80 and only the edge in front speaks HTTPS, so exporting
// `https://` here would hand an operator a URL that fails against the origin and
// works only once Cloudflare is actually in place.
url := sdk.Sprintf("http://%s:%d", host.Address, hostPort)
if servesDomain {
url = sdk.String("https://" + stackConfig.Domain).ToStringOutput()
scheme := lo.If(d.noOriginTls, "http").Else("https")
url = sdk.String(scheme + "://" + stackConfig.Domain).ToStringOutput()
}
// Exported by the caller, which is the only place that knows this host's index
// in the fleet.
Expand Down Expand Up @@ -771,6 +813,11 @@ type parentVmFacts struct {
// filtering SC does not manage, or a parent stack provisioned before the
// export existed.
OpenPorts []vm.PortRange
// NoOriginTls is `ingress.originTls: false` on the resource: Caddy serves plain
// HTTP and runs no ACME. False on a parent stack that has never heard of the
// setting, which is the old behaviour and the reason the export carries this sense
// rather than its opposite.
NoOriginTls bool
// Addresses is every machine's address, in host order — the same strings the
// HostFacts carry, kept as literals because a DNS record's value has to be one.
// HostFacts lifts the address into an Output for the driver side, where a cloud
Expand Down Expand Up @@ -900,7 +947,21 @@ func (r parentReader) vmFacts() (parentVmFacts, error) {
return parentVmFacts{}, errors.Wrapf(err, "failed to read the open ports of vm %q", r.vmResource)
}

return parentVmFacts{IngressEnabled: ingressEnabled, OpenPorts: vm.ParsePortSpec(openPortsSpec)}, nil
// Same read-through-toBool treatment, same tolerance: an unreadable or absent
// value means "the machine terminates TLS", which is what every parent stack
// provisioned before this export did.
noOriginTlsRaw, err := pApi.GetValueFromStack[any](r.ctx, r.refName(0, "no-origin-tls"), r.parentRef,
ToIngressNoOriginTlsExport(r.vmResource), false)
if err != nil {
return parentVmFacts{}, errors.Wrapf(err, "failed to read the origin-TLS setting of vm %q", r.vmResource)
}
noOriginTls, _ := toBool(noOriginTlsRaw)

return parentVmFacts{
IngressEnabled: ingressEnabled,
NoOriginTls: noOriginTls,
OpenPorts: vm.ParsePortSpec(openPortsSpec),
}, nil
}

// host reads one machine of the fleet, returning its facts and — separately — its
Expand Down
97 changes: 97 additions & 0 deletions pkg/clouds/pulumi/vm/compose_stack_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ package vm

import (
"context"
"encoding/base64"
"fmt"
"strings"
"sync"
Expand Down Expand Up @@ -721,3 +722,99 @@ func TestVmCompose_TheShipKeepsTheDeployKeyOffTheCommandLine(t *testing.T) {
Expect(logging.StringValue()).To(Equal("none"),
"the ship streams a customer image over a connection authenticated by the key on stdin")
}

// withNoOriginTls models a machine the operator provisioned with
// `ingress.originTls: false`: Caddy fronts the stacks but terminates no TLS, because
// something in front of it does.
func (f *composeFixture) withNoOriginTls() *composeFixture {
f.mocks.outputs[resource.PropertyKey(ToIngressNoOriginTlsExport(testVmResource))] = resource.NewBoolProperty(true)
return f
}

// `domainProxied: true` used to be refused in the convert tier for every vm resource.
// It is now decided here, against the parent — and both outcomes matter, because the
// refusal is the thing standing between an operator and a fleet whose certificates
// never issue, while the allow is the only front door available on a cloud with no
// bucket.
func TestVmCompose_DomainProxiedNeedsNoOriginTls(t *testing.T) {
RegisterTestingT(t)

// The machine terminates TLS: a proxied domain sends the HTTP-01 challenge to
// Cloudflare's edge, so no certificate ever issues. Refused, with the remedy named.
refused := newComposeFixture().withIngress()
refused.in.Deployment.StackConfig.DomainProxied = lo.ToPtr(true)
_, err := refused.run()
Expect(err).To(HaveOccurred())
Expect(err.Error()).To(ContainSubstring("terminates TLS itself"))
Expect(err.Error()).To(ContainSubstring("ingress.originTls"))

// The same stack onto a machine that declares no origin TLS: allowed, and the URL
// it hands back says http, because that is what the origin answers on.
allowed := newComposeFixture().withIngress().withNoOriginTls()
allowed.in.Deployment.StackConfig.DomainProxied = lo.ToPtr(true)
_, err = allowed.run()
Expect(err).To(BeNil())
Expect(allowed.url).To(Equal("http://" + testDomain))

// `domainProxied: false` is a no-op and must not be refused on either machine —
// it says "do not proxy", which is already what SC does.
unproxied := newComposeFixture().withIngress()
unproxied.in.Deployment.StackConfig.DomainProxied = lo.ToPtr(false)
_, err = unproxied.run()
Expect(err).To(BeNil())
}

// The scheme has to reach the site file too, not just the exported URL: the site
// address is what suppresses automatic HTTPS for that site and puts the listener on
// :80. A deploy that exported `http://` and wrote an HTTPS site block would answer
// nothing at all.
func TestVmCompose_NoOriginTlsRendersAnHttpSite(t *testing.T) {
RegisterTestingT(t)

f := newComposeFixture().withIngress().withNoOriginTls()
_, err := f.run()
Expect(err).To(BeNil())
Expect(f.url).To(Equal("http://" + testDomain))

route := f.mocks.commands()["route"]
Expect(route).ToNot(BeNil(), "a stack whose domain is served must write a site file")
create, _ := unwrapSecret(route["create"])
// The site file travels inside the script base64-encoded (it is written with
// `printf | base64 -d`, which is what keeps a multi-line config out of the shell's
// quoting rules), so the assertion has to decode it rather than grep the script.
Expect(siteFromRouteScript(create.StringValue())).To(ContainSubstring("http://" + testDomain + " {"))
}

// Backwards compatibility at the seam that actually carries the risk: a parent stack
// provisioned before `ingress.originTls` existed exports nothing for it, and
// GetValueFromStack returns the zero value for an output that is not there. The zero
// has to mean "the machine terminates TLS" — the behaviour that fleet already had.
// Exporting the positive sense instead would have flipped every un-re-provisioned
// fleet to plain HTTP on its next deploy.
func TestVmCompose_OldParentStackStillServesHttps(t *testing.T) {
RegisterTestingT(t)

f := newComposeFixture().withIngress()
// withIngress sets the ingress export and nothing else, so the origin-TLS output
// is genuinely absent here — the same state an old parent stack presents.
Expect(f.mocks.outputs).ToNot(HaveKey(
resource.PropertyKey(ToIngressNoOriginTlsExport(testVmResource)),
))

_, err := f.run()
Expect(err).To(BeNil())
Expect(f.url).To(Equal("https://" + testDomain))
}

// siteFromRouteScript pulls the rendered Caddy site back out of a route command.
func siteFromRouteScript(script string) string {
const marker = "printf %s '"
i := strings.Index(script, marker)
Expect(i).To(BeNumerically(">=", 0), "the route script must embed the site file")
rest := script[i+len(marker):]
j := strings.Index(rest, "'")
Expect(j).To(BeNumerically(">=", 0), "the embedded site file must be quoted")
decoded, err := base64.StdEncoding.DecodeString(rest[:j])
Expect(err).ToNot(HaveOccurred())
return string(decoded)
}
28 changes: 24 additions & 4 deletions pkg/clouds/pulumi/vm/dns.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,8 @@ func fleetDnsRecords(
// failure with the cause two stacks away.
params.Log.Warn(ctx.Context(),
"stack %q claims domain %q but the parent stack declares no registrar, so SC creates no DNS record. "+
"Point %q at %s yourself, unproxied — the machine answers the ACME challenge itself",
stackName, domain, domain, addresses)
"Point %q at %s yourself, %s",
stackName, domain, domain, addresses, manualRecordAdvice(proxied))
return nil
}

Expand All @@ -79,8 +79,8 @@ func fleetDnsRecords(
// descriptor exists but is not configured.
params.Log.Warn(ctx.Context(),
"stack %q claims domain %q but the parent stack's registrar is not configured, so SC creates "+
"no DNS record. Point %q at %s yourself, unproxied",
stackName, domain, domain, addresses)
"no DNS record. Point %q at %s yourself, %s",
stackName, domain, domain, addresses, manualRecordAdvice(proxied))
return nil
}
return errors.Wrapf(err, "failed to create the A record for %q at %s (host %d of the fleet)",
Expand Down Expand Up @@ -137,3 +137,23 @@ func fleetDnsRecords(
stackName, domain, len(addresses), addresses, len(addresses))
return nil
}

// manualRecordAdvice is what to tell an operator who has to create the records by hand
// because the parent stack has no usable registrar.
//
// Which half is right depends on the fleet's front door, and the wrong half is not
// merely unhelpful. Told to proxy a fleet that terminates its own TLS, they get a name
// whose HTTP-01 challenge is answered at Cloudflare's edge instead of on the machine, so
// no certificate ever issues and Caddy retries forever. Told NOT to proxy a fleet
// running `ingress.originTls: false`, they point clients straight at an origin that
// serves plain HTTP on :80 by design — the edge that was supposed to terminate TLS is
// bypassed, and the only thing that was ever protecting that hop is gone.
//
// Both halves used to be one hardcoded "unproxied", correct until #566 made
// `domainProxied: true` reachable.
func manualRecordAdvice(proxied bool) string {
if proxied {
return "proxied — the fleet serves plain HTTP and the edge in front of it terminates TLS"
}
return "unproxied — the machines answer the ACME challenge themselves"
}
Loading
Loading