[api-gateway]: Support http(s) as AppProtocol in Kubernetes svc #6616

Krast76 · 2024-08-18T16:34:49Z

When contour is used as api-gateway in kubernetes clusters, appProtocol set to http or https doesn't work.
This PR, if applied, allow the following protocol (as defined by IANA) :

http and www-http
https

That doesn't allow all the IANA services, but that'll support a common case

Fix #6560

github-actions · 2024-08-18T16:35:43Z

Hi @Krast76! Welcome to our community and thank you for opening your first Pull Request. Someone will review it soon. Thank you for committing to making Contour better. You can also join us on our mailing list and in our channel in the Kubernetes Slack Workspace

…ernetes' service Fix projectcontour#6560 Signed-off-by: Ludovic Logiou <ludovic.logiou@gmail.com>

internal/dag/accessors.go

Signed-off-by: Ludovic Logiou <ludovic.logiou@gmail.com>

tsaarni · 2024-08-19T14:49:18Z

Thanks @Krast76! Could you check also the linter nags here https://github.com/projectcontour/contour/pull/6616/files and add a changelog entry in the PR, the filename should be changelogs/unreleased/6616-Krast76-small.md.

Signed-off-by: Ludovic Logiou <ludovic.logiou@gmail.com>

Krast76 · 2024-08-19T16:22:45Z

Thanks @Krast76! Could you check also the linter nags here https://github.com/projectcontour/contour/pull/6616/files and add a changelog entry in the PR, the filename should be changelogs/unreleased/6616-Krast76-small.md.

Done :)

Signed-off-by: Ludovic Logiou <ludovic.logiou@gmail.com>

Krast76 · 2024-08-23T08:35:19Z

@tsaarni There is something else I should do to make the PR merged ?

The linter still failed, but I don't really understand why. 🤔

tsaarni · 2024-08-23T15:11:25Z

The linter still failed, but I don't really understand why. 🤔

Almost impossible to see in the error, but it was the alignment that it complains :-) Run make format to get it fixed!

tsaarni · 2024-08-23T15:35:55Z

@tsaarni There is something else I should do to make the PR merged ?

It could be good to add a unit test, something like this

diff --git a/internal/dag/accessors_test.go b/internal/dag/accessors_test.go
index bc3e2b86..8c3bc553 100644
--- a/internal/dag/accessors_test.go
+++ b/internal/dag/accessors_test.go
@@ -127,6 +127,18 @@ func TestBuilderLookupService(t *testing.T) {
                                        AppProtocol: ptr.To("kubernetes.io/wss"),
                                        Port:        8444,
                                },
+                               {
+                                       Name:        "iana-https",
+                                       Protocol:    "TCP",
+                                       AppProtocol: ptr.To("https"),
+                                       Port:        8445,
+                               },
+                               {
+                                       Name:        "iana-http",
+                                       Protocol:    "TCP",
+                                       AppProtocol: ptr.To("http"),
+                                       Port:        8446,
+                               },
                        },
                },
        }
@@ -212,12 +224,21 @@ func TestBuilderLookupService(t *testing.T) {
                        port:           8443,
                        want:           appProtcolService(appProtoService, "h2c"),
                },
-
                "lookup service by port number with unsupported k8s app protocol: wss": {
                        NamespacedName: types.NamespacedName{Name: appProtoService.Name, Namespace: appProtoService.Namespace},
                        port:           8444,
                        want:           appProtcolService(appProtoService, "", 1),
                },
+               "lookup service by port number with supported IANA app protocol: https": {
+                       NamespacedName: types.NamespacedName{Name: appProtoService.Name, Namespace: appProtoService.Namespace},
+                       port:           8445,
+                       want:           appProtcolService(appProtoService, "tls", 2),
+               },
+               "lookup service by port number with supported IANA app protocol: http": {
+                       NamespacedName: types.NamespacedName{Name: appProtoService.Name, Namespace: appProtoService.Namespace},
+                       port:           8446,
+                       want:           appProtcolService(appProtoService, "", 3),
+               },
        }

        for name, tc := range tests {

Signed-off-by: Ludovic Logiou <ludovic.logiou@gmail.com>

Krast76 · 2024-08-24T07:36:55Z

The linter still failed, but I don't really understand why. 🤔

Almost impossible to see in the error, but it was the alignment that it complains :-) Run make format to get it fixed!

I should have guessed that 😅

The linter must be ok (for real) this time, and I have added the unit tests you have suggested, thanks ! :)

codecov · 2024-08-24T07:46:40Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 81.76%. Comparing base (f9b6f6f) to head (d609cdc).
Report is 6 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #6616   +/-   ##
=======================================
  Coverage   81.76%   81.76%           
=======================================
  Files         133      133           
  Lines       15942    15944    +2     
=======================================
+ Hits        13035    13037    +2     
  Misses       2614     2614           
  Partials      293      293

Files	Coverage Δ
internal/dag/accessors.go	`87.70% <100.00%> (+0.13%)`	⬆️

tsaarni

Thank You @Krast76!

* add changelog Signed-off-by: gang.liu <gang.liu@daocloud.io> * build(deps): bump actions/upload-artifact in the artifact-actions group (projectcontour#6608) Bumps the artifact-actions group with 1 update: [actions/upload-artifact](https://github.com/actions/upload-artifact). Updates `actions/upload-artifact` from 4.3.5 to 4.3.6 - [Release notes](https://github.com/actions/upload-artifact/releases) - [Commits](actions/upload-artifact@89ef406...834a144) --- updated-dependencies: - dependency-name: actions/upload-artifact dependency-type: direct:production update-type: version-update:semver-patch dependency-group: artifact-actions ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * build(deps): bump github/codeql-action from 3.25.15 to 3.26.0 (projectcontour#6609) Bumps [github/codeql-action](https://github.com/github/codeql-action) from 3.25.15 to 3.26.0. - [Release notes](https://github.com/github/codeql-action/releases) - [Changelog](https://github.com/github/codeql-action/blob/main/CHANGELOG.md) - [Commits](github/codeql-action@afb54ba...eb055d7) --- updated-dependencies: - dependency-name: github/codeql-action dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * build(deps): bump github.com/onsi/ginkgo/v2 from 2.19.1 to 2.20.0 (projectcontour#6607) Bumps [github.com/onsi/ginkgo/v2](https://github.com/onsi/ginkgo) from 2.19.1 to 2.20.0. - [Release notes](https://github.com/onsi/ginkgo/releases) - [Changelog](https://github.com/onsi/ginkgo/blob/master/CHANGELOG.md) - [Commits](onsi/ginkgo@v2.19.1...v2.20.0) --- updated-dependencies: - dependency-name: github.com/onsi/ginkgo/v2 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * docs: Clarify how XFCC headers are handled (projectcontour#6586) Since XFCC headers contain authentication information, it's important to know precisely how Contour (ie Envoy) handle existing XFCC headers from clients - ie, are they blocked, or appended to, and in what circumstances are they blocked? Getting this wrong could allow serious vulnerabilities such as spoofing client certs. This documents Contours behaviour, so that users can know exactly how they are required to handle that header without needing to dive into the Contour source code. My understanding from reading the source code: https://github.com/gautierdelorme/contour/blob/main/internal/envoy/v3/listener.go#L483 as well as the Envoy documentation: https://www.envoyproxy.io/docs/envoy/latest/api-v3/extensions/filters/network/http_connection_manager/v3/http_connection_manager.proto#envoy-v3-api-enum-extensions-filters-network-http-connection-manager-v3-httpconnectionmanager-forwardclientcertdetails is that when forwarding client certificate details is not configured in Contour, Contour leaves `ForwardClientCertDetails` in Envoy unset, which means it defaults to `SANITIZE`, which means incoming headers from clients are blocked. Meanwhile, when forwarding client certificate details is configured in Contour, Contour sets `ForwardClientCertDetails` to `SANITIZE_SET` in Envoy, which means incoming XFCC headers are blocked, and if an incoming cert is present, a new XFCC header is added. Signed-off-by: James Roper <james@jazzy.id.au> * build(deps): bump dario.cat/mergo from 1.0.0 to 1.0.1 (projectcontour#6627) Bumps [dario.cat/mergo](https://github.com/imdario/mergo) from 1.0.0 to 1.0.1. - [Release notes](https://github.com/imdario/mergo/releases) - [Commits](darccio/mergo@v1.0.0...v1.0.1) --- updated-dependencies: - dependency-name: dario.cat/mergo dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * build(deps): bump github/codeql-action from 3.26.0 to 3.26.2 (projectcontour#6622) Bumps [github/codeql-action](https://github.com/github/codeql-action) from 3.26.0 to 3.26.2. - [Release notes](https://github.com/github/codeql-action/releases) - [Changelog](https://github.com/github/codeql-action/blob/main/CHANGELOG.md) - [Commits](github/codeql-action@eb055d7...429e197) --- updated-dependencies: - dependency-name: github/codeql-action dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * build(deps): bump github.com/prometheus/client_golang (projectcontour#6626) Bumps [github.com/prometheus/client_golang](https://github.com/prometheus/client_golang) from 1.19.1 to 1.20.0. - [Release notes](https://github.com/prometheus/client_golang/releases) - [Changelog](https://github.com/prometheus/client_golang/blob/main/CHANGELOG.md) - [Commits](prometheus/client_golang@v1.19.1...v1.20.0) --- updated-dependencies: - dependency-name: github.com/prometheus/client_golang dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * build(deps): bump github.com/envoyproxy/go-control-plane (projectcontour#6625) Bumps [github.com/envoyproxy/go-control-plane](https://github.com/envoyproxy/go-control-plane) from 0.12.1-0.20240111020705-5401a878d8bb to 0.13.0. - [Release notes](https://github.com/envoyproxy/go-control-plane/releases) - [Changelog](https://github.com/envoyproxy/go-control-plane/blob/main/CHANGELOG.md) - [Commits](https://github.com/envoyproxy/go-control-plane/commits/v0.13.0) --- updated-dependencies: - dependency-name: github.com/envoyproxy/go-control-plane dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * docs: Update README.md to be more helpful (projectcontour#6585) The docs/README.md made no sense. Anyone reading it in GitHub clearly wants to contribute to the documentation, that's why they're in the source code of Contour, why else would they have found their way to the source repository? So, it should point to where the documentation lives in the git repository, not to the website where it's served. Signed-off-by: James Roper <james@jazzy.id.au> Co-authored-by: Steve Kriss <stephen.kriss@gmail.com> * [api-gateway]: Support http(s) as AppProtocol in Kubernetes svc (projectcontour#6616) * [api-gateway]: Support http, https and www-http as AppProtocol in kubernetes' service Fix projectcontour#6560 Signed-off-by: Ludovic Logiou <ludovic.logiou@gmail.com> * Remove legacy www-http Signed-off-by: Ludovic Logiou <ludovic.logiou@gmail.com> * Fix undefined vars Signed-off-by: Ludovic Logiou <ludovic.logiou@gmail.com> * Add changelog Signed-off-by: Ludovic Logiou <ludovic.logiou@gmail.com> * Fix issues found by the linter Signed-off-by: Ludovic Logiou <ludovic.logiou@gmail.com> * Fix format and add unit tests Signed-off-by: Ludovic Logiou <ludovic.logiou@gmail.com> --------- Signed-off-by: Ludovic Logiou <ludovic.logiou@gmail.com> * build(deps): bump codespell-project/actions-codespell from 2.0 to 2.1 (projectcontour#6635) Bumps [codespell-project/actions-codespell](https://github.com/codespell-project/actions-codespell) from 2.0 to 2.1. - [Release notes](https://github.com/codespell-project/actions-codespell/releases) - [Commits](codespell-project/actions-codespell@94259cd...406322e) --- updated-dependencies: - dependency-name: codespell-project/actions-codespell dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * build(deps): bump github.com/prometheus/client_golang (projectcontour#6640) Bumps [github.com/prometheus/client_golang](https://github.com/prometheus/client_golang) from 1.20.0 to 1.20.2. - [Release notes](https://github.com/prometheus/client_golang/releases) - [Changelog](https://github.com/prometheus/client_golang/blob/main/CHANGELOG.md) - [Commits](prometheus/client_golang@v1.20.0...v1.20.2) --- updated-dependencies: - dependency-name: github.com/prometheus/client_golang dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * build(deps): bump github.com/onsi/ginkgo/v2 from 2.20.0 to 2.20.1 (projectcontour#6639) Bumps [github.com/onsi/ginkgo/v2](https://github.com/onsi/ginkgo) from 2.20.0 to 2.20.1. - [Release notes](https://github.com/onsi/ginkgo/releases) - [Changelog](https://github.com/onsi/ginkgo/blob/master/CHANGELOG.md) - [Commits](onsi/ginkgo@v2.20.0...v2.20.1) --- updated-dependencies: - dependency-name: github.com/onsi/ginkgo/v2 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * build(deps): bump github.com/vektra/mockery/v2 from 2.44.1 to 2.45.0 (projectcontour#6638) Bumps [github.com/vektra/mockery/v2](https://github.com/vektra/mockery) from 2.44.1 to 2.45.0. - [Release notes](https://github.com/vektra/mockery/releases) - [Changelog](https://github.com/vektra/mockery/blob/master/docs/changelog.md) - [Commits](vektra/mockery@v2.44.1...v2.45.0) --- updated-dependencies: - dependency-name: github.com/vektra/mockery/v2 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Bump kind and kubectl tools (projectcontour#6642) kind: 0.24.0 kubectl: 1.31.0 Signed-off-by: Sunjay Bhatia <sunjayb@vmware.com> * fix lb address Signed-off-by: gang.liu <gang.liu@daocloud.io> * fix ut Signed-off-by: gang.liu <gang.liu@daocloud.io> * revert wrong file Signed-off-by: gang.liu <gang.liu@daocloud.io> --------- Signed-off-by: gang.liu <gang.liu@daocloud.io> Signed-off-by: dependabot[bot] <support@github.com> Signed-off-by: James Roper <james@jazzy.id.au> Signed-off-by: Ludovic Logiou <ludovic.logiou@gmail.com> Signed-off-by: Sunjay Bhatia <sunjayb@vmware.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: James Roper <james@jazzy.id.au> Co-authored-by: Steve Kriss <stephen.kriss@gmail.com> Co-authored-by: Ludovic Logiou <ludovic.logiou@gmail.com> Co-authored-by: Sunjay Bhatia <5337253+sunjayBhatia@users.noreply.github.com>

…ectcontour#6616) * [api-gateway]: Support http, https and www-http as AppProtocol in kubernetes' service Fix projectcontour#6560 Signed-off-by: Ludovic Logiou <ludovic.logiou@gmail.com> * Remove legacy www-http Signed-off-by: Ludovic Logiou <ludovic.logiou@gmail.com> * Fix undefined vars Signed-off-by: Ludovic Logiou <ludovic.logiou@gmail.com> * Add changelog Signed-off-by: Ludovic Logiou <ludovic.logiou@gmail.com> * Fix issues found by the linter Signed-off-by: Ludovic Logiou <ludovic.logiou@gmail.com> * Fix format and add unit tests Signed-off-by: Ludovic Logiou <ludovic.logiou@gmail.com> --------- Signed-off-by: Ludovic Logiou <ludovic.logiou@gmail.com> Signed-off-by: Saman Mahdanian <saman@mahdanian.xyz>

Krast76 requested a review from a team as a code owner August 18, 2024 16:34

Krast76 requested review from tsaarni and skriss and removed request for a team August 18, 2024 16:34

sunjayBhatia requested review from a team, rajatvig and izturn and removed request for a team August 18, 2024 16:35

[api-gateway]: Support http, https and www-http as AppProtocol in kub…

ccb97c6

…ernetes' service Fix projectcontour#6560 Signed-off-by: Ludovic Logiou <ludovic.logiou@gmail.com>

Krast76 force-pushed the add-supported-appprotocol branch from 23dd9c2 to ccb97c6 Compare August 18, 2024 16:55

tsaarni added the release-note/small A small change that needs one line of explanation in the release notes. label Aug 19, 2024

tsaarni reviewed Aug 19, 2024

View reviewed changes

internal/dag/accessors.go Outdated Show resolved Hide resolved

Remove legacy www-http

7c89915

Signed-off-by: Ludovic Logiou <ludovic.logiou@gmail.com>

Krast76 added 2 commits August 19, 2024 18:15

Fix undefined vars

0538054

Signed-off-by: Ludovic Logiou <ludovic.logiou@gmail.com>

Add changelog

190b167

Signed-off-by: Ludovic Logiou <ludovic.logiou@gmail.com>

Krast76 force-pushed the add-supported-appprotocol branch from f81fb6d to 190b167 Compare August 19, 2024 16:21

Fix issues found by the linter

6d78f3b

Signed-off-by: Ludovic Logiou <ludovic.logiou@gmail.com>

Fix format and add unit tests

d609cdc

Signed-off-by: Ludovic Logiou <ludovic.logiou@gmail.com>

tsaarni approved these changes Aug 24, 2024

View reviewed changes

tsaarni merged commit 63f5e19 into projectcontour:main Aug 24, 2024
26 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[api-gateway]: Support http(s) as AppProtocol in Kubernetes svc #6616

[api-gateway]: Support http(s) as AppProtocol in Kubernetes svc #6616

Uh oh!

Krast76 commented Aug 18, 2024 •

edited

Loading

Uh oh!

github-actions bot commented Aug 18, 2024

Uh oh!

Uh oh!

tsaarni commented Aug 19, 2024

Uh oh!

Krast76 commented Aug 19, 2024

Uh oh!

Krast76 commented Aug 23, 2024

Uh oh!

tsaarni commented Aug 23, 2024 •

edited

Loading

Uh oh!

tsaarni commented Aug 23, 2024

Uh oh!

Krast76 commented Aug 24, 2024

Uh oh!

codecov bot commented Aug 24, 2024 •

edited

Loading

Uh oh!

tsaarni left a comment

Uh oh!

Uh oh!

Uh oh!

[api-gateway]: Support http(s) as AppProtocol in Kubernetes svc #6616

[api-gateway]: Support http(s) as AppProtocol in Kubernetes svc #6616

Uh oh!

Conversation

Krast76 commented Aug 18, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Aug 18, 2024

Uh oh!

Uh oh!

tsaarni commented Aug 19, 2024

Uh oh!

Krast76 commented Aug 19, 2024

Uh oh!

Krast76 commented Aug 23, 2024

Uh oh!

tsaarni commented Aug 23, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tsaarni commented Aug 23, 2024

Uh oh!

Krast76 commented Aug 24, 2024

Uh oh!

codecov bot commented Aug 24, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

tsaarni left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Krast76 commented Aug 18, 2024 •

edited

Loading

tsaarni commented Aug 23, 2024 •

edited

Loading

codecov bot commented Aug 24, 2024 •

edited

Loading