[WIP] Unit tests for haproxy connect

[pierresouchay]

This unit tests ensure changes in Consul won't break haproxy-consul-connect

• It does check the semantics of Consul are consistent across changes
• Similar tests will be added in haproxy-consul-connect by mocking Consul

TODO:

• Adapt haproxy-consul-connect to match changes in recent Consul APIs
• Ensure changes of services are all taken into account
• Find a way to sync changes between repositories
• Validate SPOE validation off certificates

[pierresouchay]

@i0rek @banks Do you already have suggestions, is that approach Ok for you (aka, copy files from https://github.com/haproxytech/haproxy-consul-connect/tree/master/consul in Consul's repo and keep it in sync) or do you have other guidelines?

[hanshasselberg]

Hello @pierresouchay, sorry for the late response! We talked internally and we are a little skeptical about how this will test suite will play out in the future. How much updating it needs, how to keep the implementation in sync with what you have in the actual integration.
Another possibility would be to let our CI run tests that are living in the integration itself. That way it would be easier to maintain and we wouldn't have these tests in our code. The thing that bothers me a little is that they are no API tests, they are more or less mimicking the real implementation and I think this is going to be hard to maintain.

Looking forward to hear what you think!

[pierresouchay]

@i0rek agree, it will be hard to maintain on the long term because files have to be kept in sync.
The goal was more to detect API breakage... but I cannot think to a magical solution as there is a cyclic dependency.

The best is probably not to test Consul's APIs but implementation only:

• we just add several integration tests in Travis CI of haproxy-consul-connect with various Consul versions (but you won't be able to detect your breakage prior to release)

@dcorbett-haproxy @bedis @NickMRamirez @ShimmerGlass @aiharos Do you have any opinion on the matter?

[pierresouchay]

ping @dcorbett-haproxy @bedis @NickMRamirez @ShimmerGlass @aiharos

[aiharos]

@i0rek agree, it will be hard to maintain on the long term because files have to be kept in sync.
The goal was more to detect API breakage... but I cannot think to a magical solution as there is a cyclic dependency.

The best is probably not to test Consul's APIs but implementation only:

• we just add several integration tests in Travis CI of haproxy-consul-connect with various Consul versions (but you won't be able to detect your breakage prior to release)

@dcorbett-haproxy @bedis @NickMRamirez @ShimmerGlass @aiharos Do you have any opinion on the matter?

I read that @i0rek does suggest to have consul API tests there, and that there are reservations about the actual functional tests that are currently being worked on in haproxy-consul-connect

In that case, perhaps we could make 2 groups of tests, (keep the current ones as they are and keep expanding on them), and add a 2nd suite of tests just for making sure the Consul API we are relying upon is still valid?

Then only the Consul API tests could be included/run by Consul upstream.

[dnephin]

@aiharos I think that sounds good, but let me restate, to make sure we are saying the same thing.

1) API Tests

Consul has tests for its API at two levels:

1. Tests for the HTTP endpoint are in agent/*_endpoint_test.go , ex: TestConnectCARoots_list.
2. Tests for the API client library are in api/*_test.go, ex TestAPI_AgentConnectCARoots_list

All of the API endpoints used by haproxy-connect should already be covered by tests, but I have not done an exhaustive search. If you (or really anyone!) is interested in contributing more tests in that style, it would be very much appreciated. These tests generally setup an agent, make an API call, and check the response. If there are particular states or special cases that are not well covered we should absolutely add more tests to cover those cases.

If you have a list of all the API endpoint used by haproxy-connect, I'd also be happy to pair with someone to review the test coverage of those endpoints. If there are any gaps in our coverage we can write up an issue with the list and make sure that tests are added for those cases.

2) System Integration Tests (End-to-end Testing)

We have a CI workflow which runs end-to-test tests to verify the integration of Consul with: Envoy, Nomad, and Vault. You can see an example of this workflow here: workflow test-integrations. If haproxy-connect has an end-to-end test suite, we can have our CI run that suite with a copy of Consul that is built from the current commit. That should catch any functional issues with the integration before any release. This is exactly what we do with Nomad (we run some of the Nomad tests in the nomad-integration job).

My understanding is that this end-to-end test suite doesn't exist yet, but when it does we would add it to the test-integrations workflow. The haproxy-connect end-to-end test suite can be created by following the pattern we use for the envoy tests, but it doesn't need to be.

I think the only requires are:

• it is able to run using docker containers (preferably with a docker-compose file)
• it can accept the name of the docker image that is used to run Consul from an environment variable.

---

That only leaves the question of what to do with this PR. Since it doesn't follow either of the established patterns for testing Consul, I don't think we can accept this change. The established patterns should provide excellent coverage both of the specific endpoints, and deeper integration testing of features, so I don't think we want to accept another type of test, unless it is filling a gap in our existing testing strategy.

I hope that all makes sense. Let me know if anything is not clear, or you have any other questions!

[dnephin]

I believe these tests have moved to the https://github.com/haproxytech/haproxy-consul-connect repo. Thank you everyone for your work on this! I'm going to close this PR.