Refactor/PR comments for auto-cert matching + documentation

[khyew]

• Addressed PR comments from previously-approved (and merged) PR Auto-add certs from ACM by hostname if none are specified via annotation #851

• Use DeepEquals instead of a loop to match wildcard hostnames

• Use k8s sets package instead of string-boolean maps as sets for removing duplicate hosts

• Made inferCertArns a member function of the defaultController struct to avoid having to pass singletons around

• Added documentation and examples for auto-cert matching

[k8s-ci-robot]

Hi @khyew. Thanks for your PR.

I'm waiting for a kubernetes-sigs or kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[khyew]

/assign @M00nF1sh

[M00nF1sh]

awesome!
/ok-to-test

[M00nF1sh]

/lgtm
/approve

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: khyew, M00nF1sh

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [M00nF1sh]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[khyew]

I... don't understand. The unit test passes locally.

[khyew]

Force-pushing after a rebase onto master to see if things are out of sync from my local repo...

[khyew]

@M00nF1sh looks like the unit tests pass now (with no changes other than a rebase and forced re-push). Can you approve again when you get a chance?

[M00nF1sh]

umm..that should indicate some flakes happend..wait me double check your test cases

[M00nF1sh]

Hi,
It's because you are returning sets.String.unsortedList() from inferCertCRN & uniqueHosts.(the order of these returned [] is pure random), and in the test comparsion, order matters when compare [].

Fix:

1. return sets.String instead of []string, and change the expectations in test cases to sets.String{} too.
2. return sets.String.List() (i'm ok with this since this have little performance drawbacks)

[khyew]

oh wow yeah, that totally slipped my mind

[khyew]

I actually would have been in favour of changing the tests to not expect the results to come in a specific order, because the function itself makes no guarantee of specific ordering and this couples the tests a bit too tightly with the current implementation. But in this case since the fix takes literally 5 seconds, I'm just going to convert the calls from UnsortedList() to List().

[M00nF1sh]

/lgtm