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

Added ValidateAspNetCoreUrls #2466

Merged
merged 10 commits into from
Jan 7, 2025
Merged

Added ValidateAspNetCoreUrls #2466

merged 10 commits into from
Jan 7, 2025

Conversation

JerryNixon
Copy link
Contributor

This pull request introduces a validation check for the ASPNETCORE_URLS environment variable to ensure it is in the correct format before starting the engine in the Program class. This change helps in preventing runtime errors due to misconfigured URLs.

Key changes:

  • Added a new method ValidateAspNetCoreUrls to check the format of the ASPNETCORE_URLS environment variable. This method splits the environment variable by commas or semicolons and ensures each part is a valid absolute URI.
  • Integrated the ValidateAspNetCoreUrls method into the Main method to validate the URLs before proceeding with the engine start. If the validation fails, an error message is printed, and the application exits with an error code.

Why make this change?

The primary motivation is to return a more clear error message to the user.

Closes #2465

What is this change?

Startup validation.

How was this tested?

  • Integration Tests
  • Unit Tests

Sample Request(s)

Valid value in END file: ASPNETCORE_URLS="http://localhost:5000;https://localhost:5001"

Invalid value in END file: ASPNETCORE_URLS=http://localhost:5000;https://localhost:5001

@abhishekkumams
Copy link
Contributor

@JerryNixon , you can run dotnet format command to fix formatting issues

@JerryNixon JerryNixon enabled auto-merge (squash) November 20, 2024 21:04
@aaronburtle
Copy link
Contributor

/azp run

@aaronburtle
Copy link
Contributor

aaronburtle commented Nov 21, 2024

The failing unit test appears to be caused by stale environment variables from one test causing another to fail due to the new validation, when that test expects the program main function to work.

I was able to get the pipeline to pass on another branch (https://github.com/Azure/data-api-builder/tree/dev/aaronburtle/ASP_URL_DEBUGG) by adding in the Environment variable that you are validating to the function in test helper that unsets all DAB environment variables.

https://dev.azure.com/sqldab/Data%20API%20builder%20Dependency%20Packages/_build/results?buildId=7640&view=results

This is the change that seems to fix it, which also requires that the environment variable constant be moved to the class as a public field so we can borrow its use for testhelper.

image

Copy link
Contributor

@aaronburtle aaronburtle 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, just needs a small update so all the tests will pass.

@JerryNixon
Copy link
Contributor Author

/azp run

@JerryNixon
Copy link
Contributor Author

/azp run

@abhishekkumams
Copy link
Contributor

/azp run

@JerryNixon JerryNixon merged commit d86e630 into main Jan 7, 2025
7 checks passed
@JerryNixon JerryNixon deleted the jerry-aspneturls-errormessage branch January 7, 2025 07:22
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.

[Bug]: Incomprehensible error message
4 participants