connect: endpoints controller deletes ACL token when service is deregistered

[ishustava]

Fixes #540

Changes proposed in this PR:

• Modify endpoints controller to delete ACL tokens for each service instance that it deregisters
• Remove TLS+ACLs table tests from endpoints controller tests. These tests were testing that endpoints controller works with a client configured to have TLS and ACLs. I thought this test was not necessary because there isn't any code in the controller that behaves differently if the consul client is configured with any of those and as a result there's no way these tests could fail. The tests testing to the new ACL logic are there but they are only testing the logic that was added and configure test agent to accommodate for that.
• Create test package under helper and move GenerateServerCerts function from subcommand/common there because it was used outside of subcommand.
• Create a helper test function to set up auth methods and refactor existing connect-init command tests to use that function.
• Minor editing fixes of comments etc.

How I've tested this PR:

• unit tests
• acceptance tests in acceptance-tests: connect tests verify that the ACL token was deleted consul-helm#1056

How I expect reviewers to test this PR:

• code review

Checklist:

• Tests added
• CHANGELOG entry added (HashiCorp engineers only, community PRs should not add a changelog entry)

[thisisnotashwin]

It took a while to read but this is an excellent PR!! I'm surprised that we only had to make changes to the endpoints controller in order for the ACL deletion to work (along with the acl write policy). Have a few suggestions but this looks excellent!

I havent tested this myself though.

[ishustava]

Thank you so much for the review @thisisnotashwin 🙏 🙏 I know it was a bit of a handful to read with all the refactors!

[ndhanushkodi]

Wow this touches a lot of places and makes it so nice and clean to read! I especially like that you pulled the K8sAuthMethod functions out into test_util. Awesome work, and I just had a few questions.

[ndhanushkodi]

Is this removed because it's just a test and what we care about is the ACL behaviour and not whether the traffic is TLS?

[ishustava]

I'm removing because there isn't any behavior in the endpoints controller that's specific to TLS. Since we're passing the consul client to the endpoints controller struct, so long as that client is configured correctly, everything will work. We're configuring that client in the test as well, and so there's no way this test could ever fail. Since it can't fail, I don't think it's that useful to us.

What triggered it though is that with TLS and ACL table tests, adding new tests was getting a hard. At first, I was thinking of separating those tests and refactoring to make them a bit more readable with the new ACL tests added. But then realized that they can't fail, and so we should probably just remove them.