search

kubernetes-sigs / ingress-controller-conformance

Repository for a compliance specification of ingress-controllers.
Apache License 2.0
43 stars 36 forks source link

Run tests using the echoserver image and add support for TLS connections #61

Closed aledbf closed 3 years ago

aledbf commented 3 years ago

/assign @alexgervais

aledbf commented 3 years ago

First tests (almost passing)

--- Failed steps:

  Scenario: An Ingress with exact path rules should not match requests with trailing slash # features/path_rules.feature:103
    When I send a "GET" request to "http://exact-path-rules/foo/" # features/path_rules.feature:106
      Error: unexpected response (statuscode: 404, length: 0): 

  Scenario: An Ingress with exact path rules should be case sensitive # features/path_rules.feature:109
    When I send a "GET" request to "http://exact-path-rules/FOO" # features/path_rules.feature:112
      Error: unexpected response (statuscode: 404, length: 0): 

  Scenario: An Ingress with exact path rules should not match any other label # features/path_rules.feature:115
    When I send a "GET" request to "http://exact-path-rules/bar" # features/path_rules.feature:118
      Error: unexpected response (statuscode: 404, length: 0): 

  Scenario: An Ingress with prefix path rules should be case sensitive # features/path_rules.feature:137
    When I send a "GET" request to "http://prefix-path-rules/FOO" # features/path_rules.feature:140
      Error: unexpected response (statuscode: 404, length: 0): 

  Scenario: An Ingress with prefix path rules should match each labels string prefix # features/path_rules.feature:167
    Then the response status-code must be 404 # features/path_rules.feature:171
      Error: expected status code 404 but 200 was returned

  Scenario: An Ingress with a trailing slashes in a prefix path rule should ignore the trailing slash and send traffic to the matching backend service # features/path_rules.feature:189
    When I send a "GET" request to "http://trailing-slash-path-rules/aaa/bbb" # features/path_rules.feature:192
      Error: unexpected response (statuscode: 301, length: 0): 

  Scenario: An Ingress with a trailing slashes in an exact path rule should not match requests without a trailing slash # features/path_rules.feature:205
    When I send a "GET" request to "http://trailing-slash-path-rules/foo" # features/path_rules.feature:208
      Error: unexpected response (statuscode: 301, length: 0): 

16 scenarios (9 passed, 7 failed)
84 steps (69 passed, 7 failed, 8 skipped)
14m48.481880395s
--- FAIL: TestSuite (980.40s)
    conformance_test.go:115: unexpected exit code running test: 1
FAIL
exit status 1
FAIL    sigs.k8s.io/ingress-controller-conformance  980.433s
aledbf commented 3 years ago

@alexgervais please take a look

aledbf commented 3 years ago

Second pass: only three errors :)

download (2)

aledbf commented 3 years ago

Testing TLS I get some errors

--- Failed steps:

  Scenario: An Ingress with a host rule should send traffic to the matching backend service # features/host_rules.feature:56
    And the response status-code must be 200 # features/host_rules.feature:60
      Error: expected status code 200 but 308 was returned

  Scenario: An Ingress with a wildcard host rule should send traffic to the matching backend service # features/host_rules.feature:70
    Then the response status-code must be 200 # features/host_rules.feature:74
      Error: expected status code 200 but 308 was returned

  Scenario: An Ingress with a wildcard host rule should not route traffic matching on more than a single dns label # features/host_rules.feature:78
    Then the response status-code must be 404 # features/host_rules.feature:82
      Error: expected status code 404 but 200 was returned
bowei commented 3 years ago

One thing -- can we make sure the commit comments reflect the changes? The title of this PR is a little bit misleading (it does more than echoserver)

aledbf commented 3 years ago

One thing -- can we make sure the commit comments reflect the changes? The title of this PR is a little bit misleading (it does more than echoserver)

Sorry about that. By default github uses the first commit for the description

bowei commented 3 years ago

/approve

k8s-ci-robot commented 3 years ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: aledbf, bowei

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](https://github.com/kubernetes-sigs/ingress-controller-conformance/blob/master/OWNERS)~~ [bowei] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment
bowei commented 3 years ago

Seems like some minor nits -- ping for lgtm

alexgervais commented 3 years ago

LGTM, only missing the redirect info in CapturedResponse.

aledbf commented 3 years ago

ok, I think we should merge this PR and iterate.

Right now, there is only two failures when I run the suite against ingress-nginx and I am not sure if that's ok or what we should check:

- HTTPS -> Creating a connection to an invalid host, what should we check?
- HTTPS -> Creating a connection to an invalid host, what SSL certificate should be returned?
--- Failed steps:

  Scenario: An Ingress with a wildcard host rule should send traffic to the matching backend service # features/host_rules.feature:70
    When I send a "GET" request to "http://bar.foo.com" # features/host_rules.feature:73
      Error: Get "https://localhost/": x509: certificate is valid for ingress.local, not bar.foo.com

  Scenario: An Ingress with a wildcard host rule should not route traffic matching on more than a single dns label # features/host_rules.feature:78
    Then the response status-code must be 404 # features/host_rules.feature:82
      Error: expected status code 404 but 200 was returned

what we should set as "conformance" for these two cases?

In ingress-nginx we set a default SSL certificate, that is used when there is no match for the requested SNI.

alexgervais commented 3 years ago

In ingress-nginx we set a default SSL certificate, that is used when there is no match for the requested SNI.

I believe we do the same in Ambassador. I'm pretty sure the behaviour is not part of any specification (or conformance). @bowei any thoughts?

aledbf commented 3 years ago

@bowei can we merge the PR and continue?

bowei commented 3 years ago

/lgtm

please send the next PR to fix the follow ups