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

fix: Only error when we verify they're using a disposable domain. #2406

Merged
merged 1 commit into from
Jan 16, 2025

Conversation

zerbitx
Copy link
Contributor

@zerbitx zerbitx commented Jan 16, 2025

This change ignores any errors in trying to get the disposable domains list and only returns an error if the domain being used is found in the list.

I tested this by providing a disposable domain to get the error and by using a list in an invalid json format to get the logging, and see it ignore the error.

/kind bugfix

What does this pull request do? Which issues does it resolve? (use resolves #<issue_number> if possible)
resolves #ENG-5542

@zerbitx zerbitx requested review from cbron and lizardruss January 16, 2025 21:18
Copy link

netlify bot commented Jan 16, 2025

Deploy Preview for vcluster-docs canceled.

Built without sensitive environment variables

Name Link
🔨 Latest commit 3371d5e
🔍 Latest deploy log https://app.netlify.com/sites/vcluster-docs/deploys/67898044fe5a5400081ea675

cbron
cbron previously approved these changes Jan 16, 2025
Copy link
Contributor

@cbron cbron left a comment

Choose a reason for hiding this comment

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

lgtm, though you should wait for @lizardruss as he would know logging patterns better than I. Debug feels fine since you would almost never want to know or care that this failed. If anything it's a bad thing as it gives a clue to make circumvention easier.

@zerbitx zerbitx requested a review from a team January 16, 2025 21:39
Copy link
Contributor

@lizardruss lizardruss left a comment

Choose a reason for hiding this comment

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

Question regarding the logger initialization.

pkg/cli/email/validate.go Outdated Show resolved Hide resolved
@zerbitx zerbitx merged commit 646c6b0 into loft-sh:main Jan 16, 2025
62 checks passed
@loft-bot
Copy link

💚 All backports created successfully

Status Branch Result
v0.22

Questions ?

Please refer to the Backport tool documentation and see the Github Action logs for details

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants