zenhack / simp_le

Simple Let's Encrypt client
GNU General Public License v3.0
223 stars 38 forks source link

CI: run tests against pebble #124

Closed buchdag closed 4 years ago

buchdag commented 4 years ago

As discussed in #123, this PR runs integration tests against Pebble rather than Boulder.

As suggested by Pebble's doc, it adds the following feature to make testing with Pebble possible:

client should offer a runtime option to specify a list of trusted root CAs

In this PR Pebble does not run in strict mode because the acme Python module around which simp_le is built is not currently compliant with RFC8555.

@zenhack If you think we should test against both Pebble and Boulder, let me know.

zenhack commented 4 years ago

Quoting Nicolas Duchon (2019-08-06 04:03:34)

Given the fact that Pebble recommend that the client have a way to set an alternate trusted CA via CLI, I'm not sure which method we should prefer. Both suits me.

I don't feel strongly. I'm fine leaving it this way.

buchdag commented 4 years ago

Mind if I ask someone @ Let's Encrypt advice about CLI vs Env var and testing against both Boulder and Pebble vs testing against Pebble only ?

cpu commented 4 years ago

Mind if I ask someone @ Let's Encrypt advice about CLI vs Env var

I don't feel strongly about the one vs the other. When I made the recommendation in the Pebble repo I was mainly concerned with allowing clients to verify the HTTPS certificate of an ACME server when it wasn't using a cert issued by a public CA (e.g. for Pebble, other test scenarios, and internal servers).

testing against both Boulder and Pebble vs testing against Pebble only ?

I think it depends on what your goals are. If you're only testing against Boulder with the test/config directory and not test/config-next you're likely missing out on the chance to catch some bugs earlier. Testing against Pebble and both configurations of Boulder is pretty heavy-weight and likely to slow down CI. However it would probably help catch bugs specific to Let's Encrypt before you get bit in staging/prod.

Testing against just Pebble is a good way to ensure general RFC 8555 compatibility but will be less helpful for catching LE specific bugs.

FWIW I believe the Certbot project are currently testing against both Boulder/Pebble. I'm not sure what their plan is going forward.

zenhack commented 4 years ago

Re: testing against both pebble and boulder, given that we've already got it working and CI perf hasn't been a problem, it feels like it makes sense to keep it. I'd be more ambivalent if we hadn't already done the work. Frankly, from a pragmatic perspective I consider LE compat to be more important than standards conformance, though I certainly think the latter matters.

Quoting Daniel McCarney (2019-08-06 14:53:42)

 Mind if I ask someone @ Let's Encrypt advice about CLI vs Env var

I don't feel strongly about the one vs the other. When I made the recommendation in the Pebble repo I was mainly concerned with allowing clients to verify the HTTPS certificate of an ACME server when it wasn't using a cert issued by a public CA (e.g. for Pebble, other test scenarios, and internal servers).

 testing against both Boulder and Pebble vs testing against Pebble
 only ?

I think it depends on what your goals are. If you're only testing against Boulder with the test/config directory and not test/config-next you're likely missing out on the chance to catch some bugs earlier. Testing against Pebble and both configurations of Boulder is pretty heavy-weight and likely to slow down CI. However it would probably help catch bugs specific to Let's Encrypt before you get bit in staging/prod.

Testing against just Pebble is a good way to ensure general RFC 8555 compatibility but will be less helpful for catching LE specific bugs.

FWIW I believe the Certbot project are currently testing against both Boulder/Pebble. I'm not sure what their plan is going forward.

-- You are receiving this because you were mentioned. Reply to this email directly, [1]view it on GitHub, or [2]mute the thread.

Verweise

  1. https://github.com/zenhack/simp_le/pull/124?email_source=notifications&email_token=AAGXYPQJMNBXP4EAGVKZAZDQDHCDNA5CNFSM4IJR7ILKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD3WDVNA#issuecomment-518798004
  2. https://github.com/notifications/unsubscribe-auth/AAGXYPQU7G4UUYGWEAZM5SDQDHCDNANCNFSM4IJR7ILA
buchdag commented 4 years ago

@zenhack testing against both Boulder and Pebble involve a bit more work as both aren't reachable on the same address after setup, and this address is currently hardcoded in the IntegrationTests test suite. I'll work on this later.

buchdag commented 4 years ago

I had a productive morning break. There you go.

buchdag commented 4 years ago

The CI error seems to be a transient network issue, could you restart the test that failed ?

zenhack commented 4 years ago

LGTM. I restarted that job, assuming it passes I'll merge.

buchdag commented 4 years ago

Looks like all lights are green.