Skip to content

Conversation

@nmengin
Copy link
Contributor

@nmengin nmengin commented Sep 3, 2018

What does this PR do?

This PR allows users to know if a Host rule is malformed by returning a bool to prevent that a Host Rule has been detected.

Thus if a Host rule is detected and no domain are returned, a specific error message can be created.

Motivation

User reported that ACME has not created domains with the error message Error getting valid domain: unable to generate a certificate when no domain is given..

The error has came from his Host rule but the message has not indicated it.

The PR improves the error management.

More

  • Added/updated tests

Copy link
Contributor

@dtomcej dtomcej left a comment

Choose a reason for hiding this comment

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

LGTM
:shipit:

rules/rules.go Outdated
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you rewrite this section like that:

var cleanDomains []string
for _, domain := range domains {
	canonicalDomain := types.CanonicalDomain(domain)
	if len(canonicalDomain) > 0 {
		cleanDomains = append(cleanDomains, canonicalDomain)
	}
}

return isHostRule, cleanDomains, nil

Copy link
Member

@mmatur mmatur left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@ldez ldez left a comment

Choose a reason for hiding this comment

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

LGTM

@traefiker traefiker force-pushed the hotfix/manage-malformed-host-rule branch from 80a436d to 6ec6251 Compare September 4, 2018 14:57
@traefiker traefiker merged commit 913d873 into traefik:v1.6 Sep 4, 2018
@nmengin nmengin deleted the hotfix/manage-malformed-host-rule branch September 14, 2018 12:24
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.

5 participants