Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Service AppProtocol should allow IANA standard service names like "http", "https" #6560

Closed
kghost opened this issue Jul 17, 2024 · 3 comments · Fixed by #6616
Closed

Service AppProtocol should allow IANA standard service names like "http", "https" #6560

kghost opened this issue Jul 17, 2024 · 3 comments · Fixed by #6616
Labels
kind/bug Categorizes issue or PR as related to a bug.

Comments

@kghost
Copy link

kghost commented Jul 17, 2024

What steps did you take and what happened:

Create a service with appProtocol:

apiVersion: v1
kind: Service
metadata:
  name: hello-service
spec:
  ports:
  - appProtocol: https
    name: https
    port: 443
    protocol: TCP
    targetPort: https

Create a httproute with pointing to to service:

apiVersion: gateway.networking.k8s.io/v1
kind: HTTPRoute
metadata:
  name: hello-route
spec:
  parentRefs:
    - group: gateway.networking.k8s.io
      kind: Gateway
      namespace: projectcontour
      name: contour
      sectionName: https
  hostnames:
    - example.com
  rules:
    - backendRefs:
      - kind: Service
        name: hello-service
        port: 80

What did you expect to happen:

The route is installed successfully, but got an error:

    - lastTransitionTime: "2024-07-17T09:36:00Z"
      message: 'AppProtocol: "http" is unsupported'
      observedGeneration: 3
      reason: UnsupportedProtocol
      status: "False"
      type: ResolvedRefs

According to the spec https://kubernetes.io/docs/concepts/services-networking/service/#application-protocol

appProtocol can be IANA service names listed here: https://www.iana.org/assignments/service-names-port-numbers/service-names-port-numbers.xhtml

These should be all valid:

  • http
  • www
  • www-http
  • https

Anything else you would like to add:

Environment:

  • Contour version: v1.29.1
  • Kubernetes version: (use kubectl version): v1.30.1
  • Kubernetes installer & version:
  • Cloud provider or hardware configuration:
  • OS (e.g. from /etc/os-release):
@kghost kghost added kind/bug Categorizes issue or PR as related to a bug. lifecycle/needs-triage Indicates that an issue needs to be triaged by a project contributor. labels Jul 17, 2024
Copy link

Hey @kghost! Thanks for opening your first issue. We appreciate your contribution and welcome you to our community! We are glad to have you here and to have your input on Contour. You can also join us on our mailing list and in our channel in the Kubernetes Slack Workspace

@lubronzhan
Copy link
Contributor

lubronzhan commented Jul 17, 2024

Yeah it's not supported yet #2431
Working on it

Oh this is for gateway api httproute, then it's probably a bug
/~https://github.com/projectcontour/contour/blob/main/changelogs/CHANGELOG-v1.28.0.md#gateway-api-backend-protocol-selection

@lubronzhan lubronzhan added kind/feature Categorizes issue or PR as related to a new feature. and removed lifecycle/needs-triage Indicates that an issue needs to be triaged by a project contributor. kind/feature Categorizes issue or PR as related to a new feature. labels Jul 17, 2024
@kghost
Copy link
Author

kghost commented Jul 18, 2024

According to the changelog

The accepted values are kubernetes.io/h2c and kubernetes.io/ws

This is too restrictive and doesn't compliance with the spec.

Krast76 added a commit to Krast76/contour that referenced this issue Aug 18, 2024
Krast76 added a commit to Krast76/contour that referenced this issue Aug 18, 2024
…ernetes' service

Fix projectcontour#6560

Signed-off-by: Ludovic Logiou <ludovic.logiou@gmail.com>
izturn added a commit to projectsesame/contour that referenced this issue Aug 27, 2024
* 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>
SamMHD pushed a commit to SamMHD/contour that referenced this issue Sep 8, 2024
…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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Categorizes issue or PR as related to a bug.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants