Update SF labels to use explicit methods and add missing labels

[lawrencegripper]

Hi,

This is WIP at the moment but I wanted to check in before I go much further - in case this is the wrong direction.

• I've started to add a few additional labels using explicit methods, as discussed in #3. Adding PassTLSCert, FrameDeny and RequestHeaders
• I've tried to bring existing SF labels in line with those in providers/labels so isExposed -> Enabled
• To make sure they behave as expected I've created a new test
• I've renamed existing label functions to ensure those that return a func are have func in the name and those that don't don't

Let me know if this all makes sense and I'll continue to add the remaining labels from the docker provider implementation in v1.5

[ldez]

FYI:

• master -> Træfik 1.6
• v1.0 -> Træfik 1.5

I assume the tests are in WIP but I think we must change the way that we test:

• updateConfig: must have tests only on the configuration update (add/remove/change)
• buildConfiguration: must have tests on labels

We must remove tests on labels from updateConfig tests.

[lawrencegripper]

Thanks. Wasn't sure what to work off :)

Good Idea, I'll reformat the labels tests I added to use buildConfiguration.

Shall I tackle splitting the updateConfig test in this PR or handle it separately? I think it would be better split into three tests

1. Update config - Is the config sent to the channel. Content is not relevant.
2. Services present - This can use BuildConfiguration. It should check services exposed by the sfsdk are present in the config message
3. Labels tests - Use the new test TestFrontendLabelConfig and create TestBackendLabelConfig to cover these cases

Sound like a good plan?

On an unrelated note is there a reason the MakeFile uses this syntax gometalinter --vendor --enable=misspell ./... not gometalinter --vendor --enable=misspell $(PKGS)

[ldez]

Sound like a very good plan! 😄

gometalinter support vendor system via --vendor since go1.5/1.6

Before go1.9, go test runs tests from vendor folder 😞 I use $(PKGS) in go test due that. But this can be remove because Traefik support only go1.9+.

[lawrencegripper]

Test refactoring is done (hopefully), if it looks good I'll add in additional use cases and start working on the additional labels too.

[lawrencegripper]

@ldez Hey, gometalinter fails on gocyclo tests for the labels test as it's quite long one now.

I've looked at trying to break it up but, imo, it makes it harder to read and update (for example pulling out each validate func into a full func rather than inline). Happy to split it up if that would be your preference.

No rush on this as I'm on leave from today until the new year so unlikely to be doing a huge amount on it.

[ldez]

You can split in 2 test methods:

• one for frontend labels
• one for backend labels

WDYT?

[lawrencegripper]

Good call but already have that. gocyclo still fails because there are lots of frontend labels to test, I'll maybe have a play at pulling out the testcase items into loosely grouped vars outside of the test method. Like security, headers etc.

I did stop for a bit and think that the tests were overkill but as it's easy to edit the tmpl file and not notice it causing a bug with a label I do think they are of value.

WDYT?

[ldez]

I will try to customize the gocylo configuration.

[ldez]

gocyclo doesn't have "real" configuration.

I think we can add // nocyclo on the frond of the test method, not really happy with this but it's the only way.

// nocyclo
func TestFrontendLabelConfig(t *testing.T){
// ...
}

// nocyclo
func TestBackendLabelConfig(t *testing.T){
// ...
}

[ldez]

Sorry mistake, it's // nolint: gocyclo

[lawrencegripper]

np, should have tested it locally - did the trick there are some failing tests which I'm expecting during the refactoring

[lawrencegripper]

@ldez hopefully this is no-longer WIP and ready for review

[jjcollinge]

LGTM