Skip to content

Check if domain is proxied #21

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

Merged
merged 4 commits into from
Aug 7, 2015
Merged

Check if domain is proxied #21

merged 4 commits into from
Aug 7, 2015

Conversation

benbalter
Copy link
Contributor

This adds a proxied? method to check if a domain is served by pages, but is proxied by a third-party service. If so, the Health Check shouldn't raise CNAME/A record errors.

The logic is as follows. A domain is proxied if:

  1. It's served by a Cloudflare IP (existing logic)
  2. The site returns GitHub server headers and it isn't:
    1. Pointed at a GitHub Pages IP, or
    2. Pointed at a GitHub Pages domain

(I also cleaned up some new test-related warnings that recently appeared by explicitly defining an expected error and explicitly vendoring test dependencies)

/cc @github/pages, @konklone

@benbalter benbalter self-assigned this Aug 6, 2015
@benbalter
Copy link
Contributor Author

/cc @jessephus again as an FYI for your /pages check usage. Once merged, in the event that a domain is served by Pages but proxied by a third party, the check will now indicate that.

@konklone
Copy link

konklone commented Aug 6, 2015

👍 A good move to broaden the proxy check to be vendor-agnostic, and should address a few specific proxies I'm aware of.

@PaulSD
Copy link

PaulSD commented Aug 6, 2015

👍

benbalter added a commit that referenced this pull request Aug 7, 2015
Check if domain is proxied
@benbalter benbalter merged commit 574cfe7 into master Aug 7, 2015
@benbalter benbalter deleted the proxy-check branch August 7, 2015 14:56
@benbalter
Copy link
Contributor Author

This is now in production for GitHub Pages builds. Please holler if things are behaving as expected.

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