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

Incorrectly nesting routes under virtualhost is considered valid #2527

Closed
erwbgy opened this issue May 15, 2020 · 3 comments · Fixed by #2916
Closed

Incorrectly nesting routes under virtualhost is considered valid #2527

erwbgy opened this issue May 15, 2020 · 3 comments · Fixed by #2916
Assignees
Labels
area/httpproxy Issues or PRs related to the HTTPProxy API.

Comments

@erwbgy
Copy link
Contributor

erwbgy commented May 15, 2020

What steps did you take and what happened:
If routes is incorrectly nested under virtualhost the HTTPProxy object is still considered valid, but all calls just return 404 Not Found:

$ cat bad-explorer-httpproxy.yaml
apiVersion: projectcontour.io/v1
kind: HTTPProxy
metadata:
  annotations:
    heptio.contour.com/ingress.class: contour
  name: explorer
  namespace: burdik
spec:
  virtualhost:
    fqdn: explorer.my.domain
    routes:
    - services:
      - name: explorer
        port: 8080
$ kubectl create -f bad-explorer-httpproxy.yaml
httpproxy.projectcontour.io/explorer created
$ kubectl get httpproxy
NAME                    FQDN                                  TLS SECRET     STATUS   STATUS DESCRIPTION
explorer                explorer.my.domain                                   valid    valid HTTPProxy
$ curl -I explorer.my.domain
HTTP/1.1 404 Not Found
date: Fri, 15 May 2020 16:55:28 GMT
server: envoy
transfer-encoding: chunked

What did you expect to happen:

I expected that the HTTPProxy status would be invalid because routes is not allowed under virtualhost.

Anything else you would like to add:
With the indentation fixed the call works fine:

$ cat explorer-httpproxy.yaml
apiVersion: projectcontour.io/v1
kind: HTTPProxy
metadata:
  annotations:
    heptio.contour.com/ingress.class: contour
  name: explorer
  namespace: burdik
spec:
  virtualhost:
    fqdn: explorer.my.domain
  routes:
  - services:
    - name: explorer
      port: 8080
$ kubectl replace -f explorer-httpproxy.yaml
httpproxy.projectcontour.io/explorer replaced
$ curl -I explorer.my.domain
HTTP/1.1 200 OK
date: Fri, 15 May 2020 16:56:46 GMT
content-length: 446
content-type: text/html; charset=utf-8
x-envoy-upstream-service-time: 0
server: envoy

Environment:

  • Contour version: 1.4.0
  • Kubernetes version: v1.16.9
  • Kubernetes installer & version: kubeadm v1.16.9
  • Cloud provider or hardware configuration: on-prem
  • OS (e.g. from /etc/os-release): RHEL 8.1
@stevesloka
Copy link
Member

Hey @erwbgy I think what happened is since the yaml wasn't nested properly, it didn't get parsed correctly when Contour got the object, so in a sense, those invalid fields were just missing from the spec.

But what should have ended up is that you had a root proxy with no routes and I think that should be at least invalid (which I thought was already a test case). Let me look to see about that one.

@youngnick
Copy link
Member

Hopefully once we can move to CRD v1, this problem will disappear, as incorrect YAML will be rejected by the apiserver. Agreed that a proxy with no routes should be invalid.

@jpeach jpeach added the area/httpproxy Issues or PRs related to the HTTPProxy API. label Jul 9, 2020
@jpeach
Copy link
Contributor

jpeach commented Jul 9, 2020

Blocked by #2678

@skriss skriss self-assigned this Sep 16, 2020
skriss added a commit to skriss/contour that referenced this issue Sep 16, 2020
Updates the Contour CustomResourceDefinition YAML files
to contain v1 resources instead of v1beta1.

Closes projectcontour#2678
Closes projectcontour#1723
Closes projectcontour#1978
Closes projectcontour#2903
Closes projectcontour#2527

Signed-off-by: Steve Kriss <krisss@vmware.com>
skriss added a commit to skriss/contour that referenced this issue Sep 17, 2020
Updates the Contour CustomResourceDefinition YAML files
to contain v1 resources instead of v1beta1.

Closes projectcontour#2678
Closes projectcontour#1723
Closes projectcontour#1978
Closes projectcontour#2903
Closes projectcontour#2527

Signed-off-by: Steve Kriss <krisss@vmware.com>
skriss added a commit to skriss/contour that referenced this issue Sep 17, 2020
Updates the Contour CustomResourceDefinition YAML files
to contain v1 resources instead of v1beta1.

Closes projectcontour#2678
Closes projectcontour#1723
Closes projectcontour#1978
Closes projectcontour#2903
Closes projectcontour#2527

Signed-off-by: Steve Kriss <krisss@vmware.com>
skriss added a commit to skriss/contour that referenced this issue Sep 17, 2020
Updates the Contour CustomResourceDefinition YAML files
to contain v1 resources instead of v1beta1.

Closes projectcontour#2678
Closes projectcontour#1723
Closes projectcontour#1978
Closes projectcontour#2903
Closes projectcontour#2527

Signed-off-by: Steve Kriss <krisss@vmware.com>
skriss added a commit to skriss/contour that referenced this issue Sep 18, 2020
Updates the Contour CustomResourceDefinition YAML files
to contain v1 resources instead of v1beta1.

Closes projectcontour#2678
Closes projectcontour#1723
Closes projectcontour#1978
Closes projectcontour#2903
Closes projectcontour#2527

Signed-off-by: Steve Kriss <krisss@vmware.com>
skriss added a commit to skriss/contour that referenced this issue Sep 21, 2020
Updates the Contour CustomResourceDefinition YAML files
to contain v1 resources instead of v1beta1.

Closes projectcontour#2678
Closes projectcontour#1723
Closes projectcontour#1978
Closes projectcontour#2903
Closes projectcontour#2527

Signed-off-by: Steve Kriss <krisss@vmware.com>
skriss added a commit that referenced this issue Sep 21, 2020
examples: generate v1 CustomResourceDefinitions

Updates the Contour CustomResourceDefinition YAML files
to contain v1 resources instead of v1beta1 with preserveUnknownFields
set to false. Also adds a check on `contour serve` for non-upgraded 
CRDs, and logs a warning if found.

Closes #2678
Closes #1723
Closes #1978
Closes #2903
Closes #2527

Signed-off-by: Steve Kriss <krisss@vmware.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/httpproxy Issues or PRs related to the HTTPProxy API.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants