Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Enable devcontainers in repo. #6491

Merged
merged 16 commits into from
Oct 29, 2024
Merged

Enable devcontainers in repo. #6491

merged 16 commits into from
Oct 29, 2024
+186 −20

Conversation

mitchdenny
Copy link
Member

@mitchdenny mitchdenny commented Oct 25, 2024 •
edited
Loading

Description

This is an initial PR to enable devcontainer support in the repo. The goal is to enable the various remote development scenarios (Codespaces, SSH, etc) so that we can test them. This is important because we've added logic to the apphost that can do things like detect it is running in Codespaces and using the Aspire repo under Codespaces is a good way of casually validating everything is working.

image

Outstanding issues

  • Dashboard URL shows as localhost:port/login?t=; whilst Codespaces will automatically remap that URL for us when we click on it, the redirect flow (GitHub auth) looses the token in the URL

Fixes # (issue)

Checklist

  • Is this feature complete?
    • Yes. Ready to ship.
    • No. Follow-up changes expected.
  • Are you including unit tests for the changes and scenario tests if relevant?
    • Yes
    • No
  • Did you add public API?
    • Yes
      • If yes, did you have an API Review for it?
        • Yes
        • No
      • Did you add <remarks /> and <code /> elements on your triple slash comments?
        • Yes
        • No
    • No
  • Does the change make any security assumptions or guarantees?
    • Yes
      • If yes, have you done a threat model and had a security review?
        • Yes
        • No
    • No
  • Does the change require an update in our Aspire docs?
    • Yes
      • Link to aspire-docs issue:
    • No
Microsoft Reviewers: Open in CodeFlow

@mitchdenny
Copy link
Member Author

Couple of gotchas. For forwarded ports, it can take some time for Codespaces to generate the TLS cert for the endpoint. You just have to be patient. Part of explicitly forwarding the ports in the .devcontainer.json file is to make sure that its mapped as HTTPS or HTTP as appropriate.

@mitchdenny mitchdenny mentioned this pull request Oct 26, 2024
Open
@mitchdenny mitchdenny marked this pull request as ready for review October 28, 2024 00:04
eerhardt
eerhardt reviewed Oct 28, 2024
View reviewed changes
.devcontainer/devcontainer.json Show resolved Hide resolved
src/Aspire.Hosting/Codespaces/CodespacesOptions.cs Outdated Show resolved Hide resolved
src/Aspire.Hosting/Codespaces/CodespacesOptions.cs Outdated Show resolved Hide resolved
src/Aspire.Hosting/Codespaces/CodespacesOptions.cs Outdated Show resolved Hide resolved
src/Aspire.Hosting/Codespaces/CodespacesOptions.cs Outdated Show resolved Hide resolved
src/Aspire.Hosting/Codespaces/CodespacesOptions.cs Outdated Show resolved Hide resolved
mitchdenny and others added 8 commits October 29, 2024 10:41
@mitchdenny @eerhardt
Update src/Aspire.Hosting/Codespaces/CodespacesOptions.cs
b23ccb1 
Co-authored-by: Eric Erhardt <eric.erhardt@microsoft.com>
@mitchdenny @eerhardt
Update src/Aspire.Hosting/Codespaces/CodespacesOptions.cs
d3ff895 
Co-authored-by: Eric Erhardt <eric.erhardt@microsoft.com>
@mitchdenny @eerhardt
Update src/Aspire.Hosting/Codespaces/CodespacesOptions.cs
b0edc56 
Co-authored-by: Eric Erhardt <eric.erhardt@microsoft.com>
@mitchdenny @eerhardt
Update src/Aspire.Hosting/Codespaces/CodespacesOptions.cs
5bde6d6 
Co-authored-by: Eric Erhardt <eric.erhardt@microsoft.com>
@mitchdenny @eerhardt
Update src/Aspire.Hosting/Codespaces/CodespacesOptions.cs
b1f3fdb 
Co-authored-by: Eric Erhardt <eric.erhardt@microsoft.com>
@mitchdenny
Change when command runs to improve reliability.
8ae2a44
@mitchdenny
Syntax error.
ec128aa
@mitchdenny
Remove public API.
c96a2a0
@build-analysis build-analysis bot mentioned this pull request Oct 29, 2024
Open
3 tasks
@davidfowl
Copy link
Member

/azp run

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@mitchdenny mitchdenny enabled auto-merge (squash) October 29, 2024 05:18
@mitchdenny mitchdenny merged commit 9336436 into main Oct 29, 2024
9 checks passed
@mitchdenny mitchdenny deleted the mitchdenny/devcontainer branch October 29, 2024 06:22
@davidfowl
Copy link
Member

@mitchdenny one place where we don't detect the codespace and print the right url is here:

image

We should change this too.

@mitchdenny
Copy link
Member Author

Yeah I was looking at this I made some tweaks last night as an experiment. I can put up a PR.

@davidfowl
Copy link
Member

Also it seems like only certain playground examples work?

@paule96 paule96 mentioned this pull request Oct 30, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@eerhardt eerhardt eerhardt left review comments

@vhvb1989 vhvb1989 vhvb1989 left review comments

@davidfowl davidfowl davidfowl approved these changes

@joperezr joperezr Awaiting requested review from joperezr

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@mitchdenny @davidfowl @eerhardt @vhvb1989