test(conformance): tls conformance test enabled

[mlavacca]

What this PR does / why we need it:

Which issue this PR fixes:

Fixes #3678

Special notes for your reviewer:

We had TLSRoute integration tests with both Passthrough and terminate, but KIC currently supports Passthrough only due to an upstream docs inconsistency.
The integration test with terminate was actually using a gateway with passthrough (we were not really testing the terminate mode). For this reason, the integration test with terminate has been removed, and the one with Passthrough has been massively improved.

PR Readiness Checklist:

Complete these before marking the PR as ready to review:

• the CHANGELOG.md release notes have been updated to reflect any significant (and particularly user-facing) changes introduced by this PR

[pmalek]

@mlavacca Do we still need this 🤔 ?

[mlavacca]

@mlavacca Do we still need this 🤔 ?

Yes, we do. I didn't manage to find time for it, I'll get back to it quite soon

[rainest]

@mlavacca it looks like this got sidelined again. I was going to try and pick it up but wasn't sure what was remaining or exactly what was broken before. Can you explain what the change was trying to do?

It looks like we can't actually satisfy the conformance test in KIC alone. It requires serving the TLSRoute on port 443, whereas the test instance has hardcoded HTTPS listens on 443 and stream TLS listens on 8899. We get the validation test cert on 443 (not entirely sure where the config setting that is, but we do) and the passthrough cert (what this test wants) on 8899:

17:07:59-0700 esenin $ openssl s_client -connect 172.18.0.102:8899 -servername abc.example.com -showcerts < /dev/null | openssl x509 -text | grep -A1 Alternative
depth=0 O = Acme Co, CN = default
verify error:num=18:self-signed certificate
verify return:1
depth=0 O = Acme Co, CN = default
verify return:1
DONE
            X509v3 Subject Alternative Name: 
                DNS:abc.example.com

17:09:11-0700 esenin $ openssl s_client -connect 172.18.0.102:443 -servername abc.example.com -showcerts < /dev/null | openssl x509 -text | grep -A1 Alternative 
depth=0 O = Acme Co, CN = default
verify error:num=18:self-signed certificate
verify return:1
depth=0 O = Acme Co, CN = default
verify return:1
DONE
            X509v3 Subject Alternative Name: 
                DNS:*, DNS:*.org

I'm not positive we violate conformance there (the spec probably does want support for both HTTPS and TLS on the same port), but I am fairly certain that the gateway can only serve stream routes on stream listens.

We could satisfy the test (since it only has the one listen and doesn't need both HTTPS and TLS on the same port) with the operator dynamically provisioning kong.conf listen configuration, but we can't satisfy it with the static config we use in the KIC tests. We should probably still add whatever improvement was proposed here, but it won't actually help with that test.

[mlavacca]

@mlavacca it looks like this got sidelined again. I was going to try and pick it up but wasn't sure what was remaining or exactly what was broken before. Can you explain what the change was trying to do?

It looks like we can't actually satisfy the conformance test in KIC alone. It requires serving the TLSRoute on port 443, whereas the test instance has hardcoded HTTPS listens on 443 and stream TLS listens on 8899. We get the validation test cert on 443 (not entirely sure where the config setting that is, but we do) and the passthrough cert (what this test wants) on 8899:

17:07:59-0700 esenin $ openssl s_client -connect 172.18.0.102:8899 -servername abc.example.com -showcerts < /dev/null | openssl x509 -text | grep -A1 Alternative
depth=0 O = Acme Co, CN = default
verify error:num=18:self-signed certificate
verify return:1
depth=0 O = Acme Co, CN = default
verify return:1
DONE
            X509v3 Subject Alternative Name: 
                DNS:abc.example.com

17:09:11-0700 esenin $ openssl s_client -connect 172.18.0.102:443 -servername abc.example.com -showcerts < /dev/null | openssl x509 -text | grep -A1 Alternative 
depth=0 O = Acme Co, CN = default
verify error:num=18:self-signed certificate
verify return:1
depth=0 O = Acme Co, CN = default
verify return:1
DONE
            X509v3 Subject Alternative Name: 
                DNS:*, DNS:*.org

I'm not positive we violate conformance there (the spec probably does want support for both HTTPS and TLS on the same port), but I am fairly certain that the gateway can only serve stream routes on stream listens.

We could satisfy the test (since it only has the one listen and doesn't need both HTTPS and TLS on the same port) with the operator dynamically provisioning kong.conf listen configuration, but we can't satisfy it with the static config we use in the KIC tests. We should probably still add whatever improvement was proposed here, but it won't actually help with that test.

Yes, this was my figuring as well before putting this one aside. I'd like to revamp it and see if what is introduced here is still needed and figure out how we can dynamically configure Kong ports to make this test pass. This is in the 2.11 milestone, hence has quite a high priority now.

[codecov]

Codecov Report

Patch coverage: 67.5% and project coverage change: -0.2% ⚠️

Comparison is base (6006cab) 68.4% compared to head (7839338) 68.3%.
Report is 1 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##            main   #3797     +/-   ##
=======================================
- Coverage   68.4%   68.3%   -0.2%     
=======================================
  Files        162     162             
  Lines      19017   19040     +23     
=======================================
- Hits       13023   13012     -11     
- Misses      5227    5263     +36     
+ Partials     767     765      -2     

Files Changed	Coverage Δ
internal/controllers/gateway/gateway_controller.go	56.2% <63.6%> (-1.2%)	⬇️
internal/manager/config.go	95.1% <100.0%> (+<0.1%)	⬆️
internal/manager/controllerdef.go	98.8% <100.0%> (+<0.1%)	⬆️

... and 3 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[mlavacca]

@rainest you approved but then re-asked yourself for the review. WDYT, can we merge this?

[shaneutt]

I did some brainstorming on this one with @mlavacca today.

Our dilemma is that we have a static-mode Gateway API implementation in KIC, which means we are basically at odds with the dynamism the tests are expecting from Gateway Listeners (flatly, we can't be dynamic at all we can only be preemptive with Helm chart configuration). There are a few ways we could resolve this, including trying to ask for changes in upstream to either allow the port for the test to be changed dynamically, OR ask for the static port to be changed but these come across as "asking upstream to put in hacks for Kong". I think we should leave such a change upstream off the table.

There are a few different alternatives we thought up to solve this problem, but they all end up being "hack around our static mode limitations", and ultimately they don't deliver on "allowing users to use port 443 for TLSRoute". @mlavacca's solution here of following UDPRoute's example for TLSRoute strikes me as pragmatic, reasonably simple and rooted in precedent (so therefore we're being somewhat consistent) while also avoiding "hacks in upstream". This will be consistent with how we've enabled something like this in the past, and it will actually enable it rather than working around it by changing the port somewhere.

So all things considered, I think this is a reasonable solution.

[mlavacca]

Closing as we'll implement TLS conformance in KGO instead, where Dynamic listener configuration is not an issue as it is in KIC.