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

feat(api): add ExternalHost property for routes #877

Merged
merged 3 commits into from
Jun 7, 2024

Conversation

ebaron
Copy link
Member

@ebaron ebaron commented Jun 7, 2024

Welcome to Cryostat! 👋

Before contributing, make sure you have:

  • Read the contributing guidelines
  • Linked a relevant issue which this PR resolves
  • Linked any other relevant issues, PR's, or documentation, if any
  • Resolved all conflicts, if any
  • Rebased your branch PR on top of the latest upstream main branch
  • Attached at least one of the following labels to the PR: [chore, ci, docs, feat, fix, test]
  • Signed all commits: git commit -S -m "YOUR_COMMIT_MESSAGE"

Fixes: #726

Description of the change:

  • Adds spec.networkOptions.coreConfig.externalHost to the Cryostat v1beta2 CRD.
  • Route spec.host is immutable once created. I had considered some logic to recreate the route if the user attempts to change the host in the CR, but this is further complicated due to OpenShift modifying the spec.host field in the default case where it dynamically assigns a hostname. I think trying to accommodate this is too risky for 3.0.

Motivation for the change:

  • Allows users to specify a custom hostname for the Cryostat route on OpenShift

How to manually test:

  1. Create a Cryostat CR with spec.networkOptions.coreConfig.externalHost defined to a custom value.

@ebaron ebaron added feat New feature or request backport labels Jun 7, 2024
@ebaron ebaron requested review from andrewazores and a team June 7, 2024 19:33
@mergify mergify bot added the safe-to-test label Jun 7, 2024
Copy link
Member

@andrewazores andrewazores left a comment

Choose a reason for hiding this comment

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

Looks good. I tested creating a CR without the new property and I got the old behaviour. Then I tried this CR:

apiVersion: v1
items:
- apiVersion: operator.cryostat.io/v1beta2
  kind: Cryostat
  metadata:
    creationTimestamp: "2024-06-07T19:40:32Z"
    finalizers:
    - operator.cryostat.io/cryostat.finalizer
    generation: 1
    name: cryostat-sample
    namespace: cryostat3
    resourceVersion: "201366"
    uid: f3abec79-1dec-41bb-b070-b3e596ffea60
  spec:
    enableCertManager: true
    networkOptions:
      coreConfig:
        externalHost: cryostat.example.com
    reportOptions:
      resources: {}
    storageOptions:
      pvc:
        spec:
          resources: {}
    targetNamespaces:
    - cryostat3
kind: List
metadata:
  resourceVersion: ""

and the result:

$ oc get route
NAME              HOST/PORT              PATH   SERVICES          PORT   TERMINATION          WILDCARD
cryostat-sample   cryostat.example.com          cryostat-sample   4180   reencrypt/Redirect   None

@ebaron ebaron merged commit 2fc2faa into cryostatio:main Jun 7, 2024
5 checks passed
mergify bot pushed a commit that referenced this pull request Jun 7, 2024
* feat(api): add ExternalHost property for routes

* Handle reverting hostname to default

* Host is immutable after creation

(cherry picked from commit 2fc2faa)

# Conflicts:
#	bundle/manifests/cryostat-operator.clusterserviceversion.yaml
ebaron added a commit that referenced this pull request Jun 7, 2024
* feat(api): add ExternalHost property for routes (#877)

* feat(api): add ExternalHost property for routes

* Handle reverting hostname to default

* Host is immutable after creation

(cherry picked from commit 2fc2faa)

# Conflicts:
#	bundle/manifests/cryostat-operator.clusterserviceversion.yaml

* Fix conflicts

---------

Co-authored-by: Elliott Baron <ebaron@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport feat New feature or request safe-to-test
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

[Bug] Allow to configure the custom routes domain for cryostat and grafana
2 participants