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.

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

Feature/add ngrok hosting #367

Merged
merged 12 commits into from
Jan 9, 2025

Conversation

esskar
Copy link
Contributor

@esskar esskar commented Jan 4, 2025

Closes ##366

This PR introduces an integration that allows Aspire-hosted applications to leverage ngrok for exposing local services via secure, public URLs.

PR Checklist

  • Created a feature/dev branch in your fork (vs. submitting directly from a commit on main)
  • Based off latest main branch of toolkit
  • PR doesn't include merge commits (always rebase on top of our main, if needed)
  • New integration
    • Docs are written
    • Added description of major feature to project description for NuGet package (4000 total character limit, so don't push entire description over that)
  • Tests for the changes have been added (for bug fixes / features) (if applicable)
  • Contains NO breaking changes
  • Every new API (including internal ones) has full XML docs
  • Code follows all style conventions

Other information

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot reviewed 11 out of 26 changed files in this pull request and generated no comments.

Files not reviewed (15)
  • CommunityToolkit.Aspire.sln: Language not supported
  • examples/ngrok/CommunityToolkit.Aspire.Hosting.Ngrok.ApiService/CommunityToolkit.Aspire.Hosting.Ngrok.ApiService.csproj: Language not supported
  • examples/ngrok/CommunityToolkit.Aspire.Hosting.Ngrok.ApiService/Properties/launchSettings.json: Language not supported
  • examples/ngrok/CommunityToolkit.Aspire.Hosting.Ngrok.ApiService/appsettings.json: Language not supported
  • examples/ngrok/CommunityToolkit.Aspire.Hosting.Ngrok.AppHost/.gitignore: Language not supported
  • examples/ngrok/CommunityToolkit.Aspire.Hosting.Ngrok.AppHost/CommunityToolkit.Aspire.Hosting.Ngrok.AppHost.csproj: Language not supported
  • examples/ngrok/CommunityToolkit.Aspire.Hosting.Ngrok.AppHost/Properties/launchSettings.json: Language not supported
  • examples/ngrok/CommunityToolkit.Aspire.Hosting.Ngrok.AppHost/appsettings.json: Language not supported
  • examples/ngrok/CommunityToolkit.Aspire.Hosting.Ngrok.ServiceDefauls/CommunityToolkit.Aspire.Hosting.Ngrok.ServiceDefauls.csproj: Language not supported
  • src/CommunityToolkit.Aspire.Hosting.Ngrok/CommunityToolkit.Aspire.Hosting.Ngrok.csproj: Language not supported
  • src/CommunityToolkit.Aspire.Hosting.Ngrok/PublicAPI.Shipped.txt: Language not supported
  • src/CommunityToolkit.Aspire.Hosting.Ngrok/PublicAPI.Unshipped.txt: Language not supported
  • src/CommunityToolkit.Aspire.Hosting.Ngrok/NgrokResource.cs: Evaluated as low risk
  • examples/ngrok/CommunityToolkit.Aspire.Hosting.Ngrok.ApiService/Program.cs: Evaluated as low risk
  • examples/ngrok/CommunityToolkit.Aspire.Hosting.Ngrok.AppHost/Program.cs: Evaluated as low risk
Comments suppressed due to low confidence (1)

src/CommunityToolkit.Aspire.Hosting.Ngrok/NgrokEndpoint.cs:4

  • The summary comment should be updated to 'Describes an ngrok endpoint.'
/// Describes a ngrok endpoint.
Copy link
Member

@aaronpowell aaronpowell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Slight change for nuget package version management.

Would it be possible to run a proper e2e test, assuming we have a ngrok key that is in the secrets of GitHub Actions? Or would it not be possible to get the ngrok generated URL within the test to then call it?

@esskar
Copy link
Contributor Author

esskar commented Jan 8, 2025

Slight change for nuget package version management.

Would it be possible to run a proper e2e test, assuming we have a ngrok key that is in the secrets of GitHub Actions? Or would it not be possible to get the ngrok generated URL within the test to then call it?

Yes, this would be possible. Is there a naming convention for such secret names?
ngrok allows to either define a fixed url (in free tier) or it generates a new one; would a integration test that checks if a publicUrl is returned and is not null-or-empty enough?

@aaronpowell
Copy link
Member

Hmm, I'm trying to think about how the test would work, it might not be practical though as it could break the local Dev loop (you'd need a token and account configured correctly for tests to pass).

Also, the secret wouldn't be available to PR runs, so every PR would have test failures.

Might be best to not add that test...

Copy link
Member

@aaronpowell aaronpowell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Last question @esskar - are you interested in being the code owner for this integration?

@esskar
Copy link
Contributor Author

esskar commented Jan 8, 2025

@aaronpowell cool. thanks. If it helps you, i dont mind being a code owner

@aaronpowell
Copy link
Member

@aaronpowell cool. thanks. If it helps you, i dont mind being a code owner

Just sent an invite.

Can you add this to the CODEOWNERS file in the repo root:

# CommunityToolkit.Aspire.MassTransit.RabbitMQ

/examples/ngrok/ @esskar
/src/CommunityToolkit.Aspire.Hosting.Ngrok/ @esskar
/tests/CommunityToolkit.Aspire.Hosting.Ngrok.Tests/ @esskar

@aaronpowell aaronpowell merged commit 4ab0c04 into CommunityToolkit:main Jan 9, 2025
9 checks passed
@Alirexaa Alirexaa added this to the 9.2 milestone Feb 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants