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

Return empty path for termination passthrough #773

Conversation

mosuke5
Copy link
Contributor

@mosuke5 mosuke5 commented Jun 9, 2022

Description

Fixed to return empty path when ingress termination is "passthrough".
Passthrough mode doesn't require path setting.

Relevant issues/tickets

#766

Type of change

  • Bug fix (non-breaking change which fixes an issue)

Checklist

  • This change requires a documentation update
  • I have added tests that prove my fix is effective or that my feature works
  • I have added a test case that will be used to verify my changes
  • Verified independently on a cluster by reviewer

Verification steps

  1. Check environment
$ oc version
Client Version: 4.9.21
Server Version: 4.10.15
Kubernetes Version: v1.23.5+3afdacb
  1. Create Grafana custom resouce like bellow.
apiVersion: integreatly.org/v1alpha1
kind: Grafana
metadata:
  name: grafana-sample
spec:
  ingress:
    enabled: True
    termination: passthrough
  config:
    security:
      admin_user: admin
      admin_password: admin
  1. Check generated route resouce
oc get route grafana-route -o yaml
apiVersion: route.openshift.io/v1
kind: Route
metadata:
  annotations:
    openshift.io/host.generated: "true"
  creationTimestamp: "2022-06-08T14:01:35Z"
  name: grafana-route
  namespace: grafana-operator-system
  ownerReferences:
  - apiVersion: integreatly.org/v1alpha1
    blockOwnerDeletion: true
    controller: true
    kind: Grafana
    name: grafana-sample
    uid: 17cd2c5c-553d-4363-a12e-1c77b448f94b
  resourceVersion: "178223"
  uid: a4100a36-a8e0-4307-b1c1-3ddbb56c3672
spec:
  host: grafana-route-grafana-operator-system.apps.xxxx.com
  port:
    targetPort: 3000
  tls:
    termination: passthrough
  to:
    kind: Service
    name: grafana-service
    weight: 100
  wildcardPolicy: None
status:
  ingress:
  - conditions:
    - lastTransitionTime: "2022-06-08T14:01:35Z"
      status: "True"
      type: Admitted
    host: grafana-route-grafana-operator-system.apps.xxxx.com
    routerCanonicalHostname: router-default.apps.xxxx.com
    routerName: default
    wildcardPolicy: None

if cr.Spec.Ingress.Termination == "passthrough" {
return ""
}

Copy link
Contributor

@weisdd weisdd Jun 10, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if cr.Spec.Ingress.Termination == "passthrough" {
return ""
}
if cr.Spec.Ingress.Termination == v1.TLSTerminationPassthrough {
return ""
}

I think it should rather rely on the value that's provided by the github.com/openshift/api/route/v1 library :)

Copy link
Contributor Author

@mosuke5 mosuke5 Jun 11, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@weisdd Thank you for your review. I fixed it.

Copy link
Member

@HubertStefanski HubertStefanski left a comment

Thanks @mosuke5, did you manage to verify this on an openshift cluster

@HubertStefanski HubertStefanski merged commit bf86d76 into grafana-operator:master Jun 12, 2022
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants