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

Create dummy A records for _acme-challenge.docs #162

Merged
merged 1 commit into from
Jan 11, 2019

Conversation

ixdy
Copy link
Member

@ixdy ixdy commented Jan 10, 2019

This prevents resolvers from following the wildcard CNAME for *.docs
when trying to resolve the _acme-challenge.docs TXT record.

This should partially address #160; I still need to update the DNS records and then re-request a cert.

/assign @thockin

This prevents resolvers from following the wildcard CNAME for *.docs
when trying to resolve the _acme-challenge.docs TXT record.
@k8s-ci-robot k8s-ci-robot requested review from dims and hh January 10, 2019 23:14
@k8s-ci-robot k8s-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Jan 10, 2019
@munnerz
Copy link
Member

munnerz commented Jan 11, 2019

In cert-manager we've recently added a 'cnameStrategy' option to allow you to dictate how CNAME records should be handled. That said, the proposed fix here sums up/fixes my findings when implementing that PR well too! 😄 cert-manager/cert-manager#1136

@thockin
Copy link
Member

thockin commented Jan 11, 2019

/lgtm
/approve

We'll have to discuss whether cert procurement should be allowed to mutate DNS directly or go through this PR process for short-lved records like the TXT records for this.

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 11, 2019
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ixdy, thockin

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 11, 2019
@k8s-ci-robot k8s-ci-robot merged commit 0fdce80 into kubernetes:master Jan 11, 2019
@ixdy
Copy link
Member Author

ixdy commented Jan 11, 2019

Deployed these changes manually, and working on refreshing the TLS cert to fix #160.

Eventually (hopefully sooner rather than later) I'd like to move to either https://github.com/GoogleCloudPlatform/gke-managed-certs (if/when available) or cert-manager. With cert-manager we may be able to use an HTTP challenge instead of a DNS challenge, which might resolve the DNS access concerns.

@ixdy
Copy link
Member Author

ixdy commented Jan 11, 2019

(x-ref #38; we should setup automation on all the things, not just gcsweb)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants