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

Add ACME/Let’s Encrypt integration tests #975

Merged
merged 1 commit into from
Dec 22, 2016

Conversation

trecloux
Copy link
Contributor

@trecloux trecloux commented Dec 15, 2016

Tests are based on a boulder image I pushed on my personal docker hub account.

Thx @gwallet for the help.

BaseSuite
}

func (s *AcmeSuite) setupBoulder(c *check.C) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This could be in SetUpSuite and not called by the test I think 👼

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I used the same structure than the ConsulSuite, but yes sounds good :-)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

@@ -52,6 +54,18 @@ func (s *BaseSuite) TearDownSuite(c *check.C) {
func (s *BaseSuite) createComposeProject(c *check.C, name string) {
projectName := fmt.Sprintf("integration-test-%s", name)
composeFile := fmt.Sprintf("resources/compose/%s.yml", name)

addrs, err := net.InterfaceAddrs()
Copy link
Contributor

Choose a reason for hiding this comment

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

@trecloux care to explain this change ? 👼

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The ACME/Boulder service needs to call traefik to check the HTTPS/SNI challenge.
So we need to know the IP address of traefik.
This code is looking for the host non loopback IP V4 address .

Copy link
Contributor

Choose a reason for hiding this comment

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

Hum ok, but who uses DOCKER_HOST_IP ?

Copy link
Contributor

Choose a reason for hiding this comment

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

ah nevermind, didn't look at the compose file 😂

@@ -0,0 +1,44 @@
boulder:
image: trecloux/boulder:20161209-8a6382f4
Copy link
Contributor

Choose a reason for hiding this comment

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

What changes have been made to letsencrypt/boulder?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No change, I just built/pushed the last commit of the release branch 8a6382f4 is the commit id. The official image was not updated for 9 months.

@emilevauge
Copy link
Member

@trecloux If you don't mind, could you use this image instead:
https://hub.docker.com/r/containous/boulder automatically build from https://github.com/containous/boulder ?

@trecloux
Copy link
Contributor Author

@emilevauge I updated the image. Unfortunately the test looks instable on travis

}
return nil
})

Copy link
Member

Choose a reason for hiding this comment

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

You should add c.Assert(err, checker.IsNil) here I think ;)

@trecloux
Copy link
Contributor Author

@emilevauge Thanks for the tip, fixed it.
@vdemeester Ok for you ?

Copy link
Contributor

@vdemeester vdemeester left a comment

Choose a reason for hiding this comment

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

LGTM 🐻
Needs a rebase though 👼

@trecloux
Copy link
Contributor Author

@vdemeester I rebased the branch, should I squash the commits ?

@emilevauge
Copy link
Member

@trecloux

should I squash the commits ?

In this case yeah, we don't need to keep Copy paste is bad :-) commits in history ;)

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