Add support for a PULUMI_CONSOLE_DOMAIN env var #4410
Conversation
pkg/backend/httpstate/console.go
Outdated
| return "" // We couldn't figure out how to convert the api hostname into a console hostname | ||
| // We couldn't figure out how to convert the api hostname into a console hostname. | ||
| // We skip the host and just print the relative path. | ||
| return path.Join(paths...) |
There was a problem hiding this comment.
This is a behavior change, but it seems like a better way to go. If we want to generate a URL to the Pulumi Console with some paths as a suffix, it seems odd to return "" in that case.
There was a problem hiding this comment.
Don't we want it to return ""? There are places in the codebase that look at the result of calls to cloudConsoleURL and do something different if it returns an empty string.
pkg/backend/httpstate/console.go
Outdated
| func cloudConsoleURL(cloudURL string, paths ...string) string { | ||
| u, err := url.Parse(cloudURL) | ||
| if err != nil { | ||
| return "" | ||
| } | ||
|
|
||
| switch { | ||
| case os.Getenv("PULUMI_CONSOLE_DOMAIN") != "": |
There was a problem hiding this comment.
Nit: Make this a constant, e.g. ConsoleDomainEnvVar, like is typically done with other env variable names?
There was a problem hiding this comment.
I debated it, and went with hard-coding it because it was only used in one place. But I'll make the change, since it's probably a better idea to export it.
pkg/backend/httpstate/console.go
Outdated
| return "" // We couldn't figure out how to convert the api hostname into a console hostname | ||
| // We couldn't figure out how to convert the api hostname into a console hostname. | ||
| // We skip the host and just print the relative path. | ||
| return path.Join(paths...) |
There was a problem hiding this comment.
Don't we want it to return ""? There are places in the codebase that look at the result of calls to cloudConsoleURL and do something different if it returns an empty string.
Great catch. Yeah, I hadn't thought about that. I'll revert that change. |
|
Mind opening an issue in the docs repo so we don't forget to add this to https://github.com/pulumi/docs/blob/master/content/docs/reference/cli/environment-variables.md ? Thanks! |
A Pulumi "backend" is identified by a URL, such as
https://api.pulumi.com,file:///Users/chris/pulumi-checkpoints, ors3://my-pulumi-stacks/. Andfilestate.IsFileStateBackendURLis used to determine which backend to use ("file state" or "http state") based on the URL's scheme.This has served us well, however it poses a problem when you try to couple the Pulumi CLI with the Pulumi Console. The Pulumi Service is served from two different domains: api.pulumi.com (REST API) and app.pulumi.com (web frontend).
The CLI is currently wired so that
pulumi loginis the same as if you had writtenpulumi login --cloud-url https://api.pulumi.com. (Meaning you specify the REST API's domain as the "cloud backend URL".) And so whenever we go to generate a URL to the Pulumi Console, we have a simple heuristic for guessing at what the Pulumi Console domain should be.The problem is that when you are using an on-prem version of the Pulumi Service, and not using the same
api.*andapp.*convention for the domains, we end up generating incorrect URLs. (e.g. if you haveapi.pulumi.contoso.comandconsole.pulumi.contoso.com, at the end of every update we will link you to https://app.pulumi.contoso.com which would be wrong.)This PR adds a small hack, which is to honor a
PULUMI_CONSOLE_DOMAINenvironment variable that would allow you to set the domain we use for links to the Pulumi Console.A better solution would be to somehow couple the "backend URL" with its "console URL". (e.g. when you login, we pass around something like
type HTTPBackendURLs struct { RESTAPIDomain string; ConsoleDomain string }. However, that would be a big undertaking for how we persist credentials and accounts in theworkspacepackage.Another approach would be to have you "log in" using the Pulumi Console domain, and then figure out how to have app.pulumi.com serve both API and web traffic. (That's a reasonable idea, but would require a bunch of work server-side.)
So that leads us to just honoring a new environment variable for now.
Fixes #4888