Codespaces: Drupal redirects at unusual places

rfay commented 1 month ago

Is there an existing issue for this?

[X] I have searched the existing issues

Output of `ddev debug test`

Expand `ddev debug test` diagnostic information

``` [COPY-PASTE HERE THE OUTPUT OF `ddev debug test`] ```

Expected Behavior

Codespaces should work fine with a Drupal10 instance

Actual Behavior

From Discord

DDEV with Drupal 10 installs and launches OK
Logging in or visiting "My Account" fails with a 404

DDEV_DEBUG=true ddev launch shows trying to use port 8080 with http (and it warns about redirects: "HTTP may redirect to HTTPS in your browser")

$ DDEV_DEBUG=test ddev launch
HTTP may redirect to HTTPS in your browser
See https://ddev.readthedocs.io/en/stable/users/usage/commands/#launch
FULLURL http://127.0.0.1:8080

Actual launch redirects to https://fictional-space-fortnight-97rqv976p3ppjv-8080.app.github.dev/, which works fine (and you can visit many other URLs)
Until you visit "My Account" (/user) when logged in (or try to log in). Then it lands on https://fictional-space-fortnight-97rqv976p3ppjv-8080.app.github.dev:8080/en/user/1 (note the appended port 8080).
$DDEV_PRIMARY_URL inside the container is http://127.0.0.1:8080

If you visit the URL provided by the ports tab for 8443, https://fictional-space-fortnight-97rqv976p3ppjv-8443.app.github.dev/ then everything seems to work, including logging in or visiting /user when logged in.

It seems that codespaces wants us to use the 8443 port and a full URL for forwarding (for example, https://fictional-space-fortnight-97rqv976p3ppjv-8443.app.github.dev) not localhost

With DDEV v1.23.0-rc2, you can ddev launch :8443 and everything seems to work right.

Steps To Reproduce

Open a codespace using https://github.com/ddev/d10simple

Install using the web installer or ddev drush si -y demo_umami --account-pass=admin

ddev launch

Try logging in using user=admin pass=admin

Anything else?

No response

rfay commented 1 month ago

https://github.com/ddev/ddev/pull/5943

msagliocco commented 1 month ago

additional scenarios where the redirect to :8080 is triggered:

Node/add form submit
Clear cache button
Add menu form submit

rfay commented 1 month ago

Please say whether those redirects to :8080 are triggered if you're using the 8443 URL, thanks.

rfay commented 1 month ago

I'm not entirely sure that codespaces support is working properly in general. I don't see DDEV getting installed by adding the devcontainer.json specifying it.

msagliocco commented 1 month ago

Performed extensive tests on the 8443 port and drupal runs smoothly, no issue at all in any of the reported scenarios and more. Looks like a perfectly viable solution. About ddev getting installed from the devcontainer.json: i had problems too, it did not install a couple of times, a couple of rebuild solved the problem, could not reproduce consistently btw.

rfay commented 1 month ago

Codespaces seems to change things around quite a lot, and we don't know how to do automated tests on either codespaces or gitpod unfortunately. However, gitpod may be a more robust approach for your team, lots of people use DDEV there.

mandrasch commented 6 days ago

Hi! Tested a bit today again with a simple craftcms site https://github.com/mandrasch/craftcms-sprig-green-coding-jobs-demo.

Docker integration by Codespaces still seems to be a bit wonky, it just fails sometimes (although I wait for it in postCreateCommand.sh 🤯 )

But the bigger problem is really that - if docker runs successfully - the Codespaces URL detection does not seem work anymore?

PRIMARY_SITE_URL=http://127.0.0.1:8080

Codespace determination is done here:

func IsCodespaces() bool {
    if os.Getenv("DDEV_PRETEND_CODESPACES") == "true" {
        return true
    }
    return runtime.GOOS == "linux" && os.Getenv("CODESPACES") == "true"
}

https://github.com/ddev/ddev/blob/ea922e29a573011ce1554d8344933ae03c0ee5d8/pkg/nodeps/utils.go#L81

URL parsing is done here in getAllUrls():

if nodeps.IsCodespaces() {
        codespaceName := os.Getenv("CODESPACE_NAME")
        previewDomain := os.Getenv("GITHUB_CODESPACES_PORT_FORWARDING_DOMAIN")
        if codespaceName != "" && previewDomain != "" {
            url := fmt.Sprintf("https://%s-%s.%s", codespaceName, app.HostWebserverPort, previewDomain)
            httpsURLs = append(httpsURLs, url)
        }
    }

https://github.com/ddev/ddev/blob/ea922e29a573011ce1554d8344933ae03c0ee5d8/pkg/ddevapp/ddevapp.go#L2624

I checked printenv, the vars are still there - at least in the codespaces terminal.

CODESPACES=true
GITHUB_CODESPACES_PORT_FORWARDING_DOMAIN=app.github.dev
CODESPACE_NAME=shiny-space-goldfish-4xv7vp7gq24vw

Is there a quick way to step-debug this and check if ddev detects Codespaces in general?

stasadev commented 5 days ago

@mandrasch,

GetPrimaryURL() checks for CanUseHTTPOnly():

https://github.com/ddev/ddev/blob/ea922e29a573011ce1554d8344933ae03c0ee5d8/pkg/ddevapp/ddevapp.go#L2657-L2670

And CanUseHTTPOnly() is self-explanatory:

https://github.com/ddev/ddev/blob/ea922e29a573011ce1554d8344933ae03c0ee5d8/pkg/ddevapp/utils.go#L506-L513

And finally, the code below (for HTTPS) will never be executed, because CanUseHTTPOnly() is always true for Gitpod and Codespaces:

https://github.com/ddev/ddev/blob/ea922e29a573011ce1554d8344933ae03c0ee5d8/pkg/ddevapp/ddevapp.go#L2615-L2631

stasadev commented 5 days ago

I think the code should be such that it fits the other parts (even if it generates HTTPS links):

--- a/pkg/ddevapp/ddevapp.go
+++ b/pkg/ddevapp/ddevapp.go
@@ -2618,7 +2618,7 @@ func (app *DdevApp) GetAllURLs() (httpURLs []string, httpsURLs []string, allURLs
        url, err := exec.RunHostCommand("gp", "url", app.HostWebserverPort)
        if err == nil {
            url = strings.Trim(url, "\n")
-           httpsURLs = append(httpsURLs, url)
+           httpURLs = append(httpURLs, url)
        }
    }
    if nodeps.IsCodespaces() {
@@ -2626,7 +2626,7 @@ func (app *DdevApp) GetAllURLs() (httpURLs []string, httpsURLs []string, allURLs
        previewDomain := os.Getenv("GITHUB_CODESPACES_PORT_FORWARDING_DOMAIN")
        if codespaceName != "" && previewDomain != "" {
            url := fmt.Sprintf("https://%s-%s.%s", codespaceName, app.HostWebserverPort, previewDomain)
-           httpsURLs = append(httpsURLs, url)
+           httpURLs = append(httpURLs, url)
        }
    }

And in describe.go all URLs stanza should be outside this condition:

if !ddevapp.IsRouterDisabled(app) {
    ...
    // All URLs stanza
    _, _, urls := app.GetAllURLs()
    s := strings.Join(urls, ", ")
    urlString := text.WrapSoft(s, int(urlPortWidth))
    t.AppendRow(table.Row{"All URLs", "", urlString})
}

And ddev launch needs to be reviewed.

mandrasch commented 5 days ago

Ah, thanks very much @stasadev 💡💡 - so this was introduced in v1.23.0 via https://github.com/ddev/ddev/commit/0c1baa3725d29f2f9ba027b67f6a616833a708c5? Because in my previous experiments primary url was parsed successfully.

stasadev commented 5 days ago

@mandrasch, yes, most likely a change in logic led to unexpected result.

I will investigate this better, because all I did was try your demo repo and look at the code.

stasadev commented 5 days ago

I reopened #5943, it should fix the port redirection in this issue because $DDEV_PRIMARY_URL will be generated without the port.

~~And it will now show http://127.0.0.1:8080 in the All URLs section of ddev describe so that users know that they can try to open this address with ddev launch if $DDEV_PRIMARY_URL is broken.~~ Edit: I think I understand why there is no All URLs, people will try to open http://127.0.0.1:8080 directly in their browser, they will not try ddev launch http://127.0.0.1:8080

mandrasch commented 1 day ago

Thanks very much for taking care of this, Stasa!

I briefly tested https://github.com/mandrasch/craftcms-sprig-green-coding-jobs-demo with new ddev v1.23.1, url in .env and ddev describe is set correctly now. 👍 🎉

Codespaces creation is still a bit buggy, needed two attemps / new codespaces to succeed with postcreatecommand script execution. This bug creeps in sometimes between commands in my demo:

2024-05-17 14:50:35.619Z: + 2024-05-17 14:50:35.627Z: wait_for_docker
+ ********
+ docker ps
2024-05-17 14:50:38.160Z: + break2024-05-17 14:50:38.175Z: 
2024-05-17 14:50:38.187Z: + 2024-05-17 14:50:38.197Z: echo Docker is ready.2024-05-17 14:50:38.211Z: 
Docker is ready.
+ ddev config global --omit-containers=ddev-router

[...]

2024-05-17 14:14:08.612Z: + ddev debug download-images
2024-05-17 14:14:23.567Z: ERRO[0014] app.FindContainerByType(web) failed 
2024-05-17 14:14:23.583Z: ERRO[0014] app.FindContainerByType(web) failed 
2024-05-17 14:14:23.599Z: Could not connect to a Docker provider. Please start or install a Docker provider.
For install help go to: https://ddev.readthedocs.io/en/stable/users/install/docker-installation/
2024-05-17 14:14:23.604Z: postCreateCommand failed with exit code 1. Skipping any further user-provided commands.

I wonder if we should add a disclaimer to the docs (https://ddev.readthedocs.io/en/stable/#__tabbed_1_5), because the experience in the past months with Codespaces isn't really reliable. Btw: Personally I use Codespaces only for demo/hobby stuff, so not at all urgent for me / no high priority for me. And not really what this ticket is about as well. 🤓

stasadev commented 1 day ago

I'm glad to hear that!

The only thing to note is that I used three words in my nickname, so "a" is the article and I'm Stas 🙂.

I wonder if we should add a disclaimer to the docs

If you know what to change there, please open a PR. I also had some problems with Codespaces, but only locally in VSCode.

mandrasch commented 1 day ago

I'm glad to hear that!

The only thing to note is that I used three words in my nickname, so "a" is the article and I'm Stas 🙂.

Ops, my bad. 🤓 Apologies! Thanks again, Stas!

I wonder if we should add a disclaimer to the docs

If you know what to change there, please open a PR. I also had some problems with Codespaces, but only locally in VSCode.

👍Yeah, I had the discussion with someone else in DDEV discord that Codespaces behaves a bit differently in VSCode as well sometimes. So testing would be needed for both usages of Codespaces. Maybe automated testing will be possible in future.

I'll think about a docs PR or I might open up another ticket at Codespaces after I replicate it some more.

mandrasch commented 20 hours ago

jfyi: Submitted issue https://github.com/devcontainers/features/issues/977

ddev / ddev