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

Generalize Ingress creation, use wildcard cert with OCP apps domain #305

Merged
merged 15 commits into from
Mar 7, 2025

Conversation

ThisIsntTheWay
Copy link
Contributor

@ThisIsntTheWay ThisIsntTheWay commented Feb 5, 2025

Summary

  • Generalize ingress creation logic
  • Create separate ingresses that bundle FQDNs in the following way:
    • FQDN is part of an OCP clusters default apps domain and only has one subdomain (sub1.apps...)
    • FQDN is either not part of said domain or has 2 or more subdomains (sub2.sub1.apps...)
      • Bundled ingresses will have "letsencrypt" or "wildcard" added to their name according to their TLS config
    • Services that only have one FQDN will only create a single ingress with no extra name according to those rules
  • Use wildcard certs for the ingress bundling eligible FQDNs (empty TLS config)

Given an apps domain example.com:

❯ k get ing -o custom-columns="NAME:.metadata.name,HOSTS:.spec.rules[*].host"
NAME                               HOSTS
nextcloud-collabora-code-ingress   k-collabora.example.com
nextcloud-letsencrypt-ingress      next.nextcloud.example.com,past.nextcloud.example.com,presentcloud.my-domain.ch,nextcloud.my-domain.ch,future.tensecloud.my-domain.ch
nextcloud-wildcard-ingress         nextcloud.example.com,2-nextcloud.example.com

❯ k get ing nextcloud-letsencrypt-ingress  -o yaml | yq .spec.tls
- hosts:
    - next.nextcloud.example.com
    - past.nextcloud.example.com
    - presentcloud.my-domain.ch
    - nextcloud.my-domain.ch
    - future.tensecloud.my-domain.ch
  secretName: nextcloud-ingress-cert

❯ k get ing nextcloud-wildcard-ingress -o yaml | yq .spec.tls
- {}

This affects the following services:

  • Nextcloud
  • Keycloak
  • Collabora
  • Forgejo

Related PR in component-appcat: vshn/component-appcat#623

Checklist

  • Categorize the PR by setting a good title and adding one of the labels:
    bug, enhancement, documentation, change, breaking, dependency
    as they show up in the changelog
  • Update tests.

Sorry, something went wrong.

@ThisIsntTheWay ThisIsntTheWay added the enhancement New feature or request label Feb 5, 2025
@ThisIsntTheWay ThisIsntTheWay self-assigned this Feb 5, 2025
@ThisIsntTheWay ThisIsntTheWay requested review from a team, Kidswiss, TheBigLee and wejdross and removed request for a team February 10, 2025 15:15
@ThisIsntTheWay ThisIsntTheWay marked this pull request as draft February 12, 2025 14:11
@ThisIsntTheWay ThisIsntTheWay force-pushed the change/ingress-tls branch 4 times, most recently from 80c9829 to f6fb4eb Compare February 26, 2025 11:09
@ThisIsntTheWay ThisIsntTheWay marked this pull request as ready for review February 27, 2025 09:35
@ThisIsntTheWay ThisIsntTheWay requested a review from zugao February 27, 2025 09:36
@ThisIsntTheWay ThisIsntTheWay changed the title Add logic to identify apps domain Generalize Ingress creation, use wildcard cert with OCP apps domain Feb 27, 2025
Copy link
Collaborator

@zugao zugao left a comment

Choose a reason for hiding this comment

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

This PR will touch existing services. Make sure you check the crossplane-diff of this change.

Copy link
Contributor

@Kidswiss Kidswiss left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Collaborator

@zugao zugao left a comment

Choose a reason for hiding this comment

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

LGTM

@ThisIsntTheWay
Copy link
Contributor Author

Thanks for the approval, will merge once I got to verify the diff (unless you guys have been able to do that already)

ThisIsntTheWay and others added 10 commits March 7, 2025 08:46
Previously, ingresses were (mostly) created using helm values.
With this change, ingresses are generated in the common package.
Meaning: Ingresses are now explicitly defined and no longer templated.

This also allows to properly handle OCP default apps wildcard certs
without the need of copy/pasting the same code for each service.
Co-authored-by: Gabriel Saratura <58511627+zugao@users.noreply.github.com>
@ThisIsntTheWay ThisIsntTheWay merged commit 71a09a5 into master Mar 7, 2025
8 checks passed
@ThisIsntTheWay ThisIsntTheWay deleted the change/ingress-tls branch March 7, 2025 10:06
@ThisIsntTheWay ThisIsntTheWay restored the change/ingress-tls branch March 7, 2025 10:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request minor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants