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

Update Ingress Configuration and Documentation for LAGO_DOMAIN Path-Based Routing #118

Closed
wants to merge 9 commits into from

Conversation

electrosenpai
Copy link
Contributor

@electrosenpai electrosenpai commented Oct 31, 2024

Title: Update Ingress Configuration and Documentation for LAGO_DOMAIN Path-Based Routing

Description:
This PR updates both the Ingress configurations and documentation to implement a path-based routing setup with LAGO_DOMAIN. Key changes include:

  • Refactored Ingress files:
    • Split Ingress into separate configurations for frontend and API.
    • Applied path-based routing, ensuring API requests use the /api/ prefix.
    • Added rewrite rules to strip the /api/ prefix before requests reach the backend.
  • Documentation update:
    • Provided instructions for setting up path-based routing with NGINX and alternative ingress controllers.
    • Emphasized the importance of the /api/ prefix to prevent routing conflicts with frontend paths.

@electrosenpai electrosenpai marked this pull request as ready for review November 8, 2024 13:56
- Refactor documentation to reflect new path-based routing setup using LAGO_DOMAIN.
- Remove outdated references to configuration-snippet and allow-snippet-annotations.
- Add instructions for separate frontend and API ingress configurations.
- Emphasize importance of /api/ prefix for API paths to avoid routing conflicts.
- Provide clear guidance for users with NGINX and non-NGINX ingress controllers.
@electrosenpai electrosenpai changed the title Consolidate Helm Chart URLs into Single LAGO_DOMAIN Variable to Resolve CORS Issues Update Ingress Configuration and Documentation for LAGO_DOMAIN Path-Based Routing Nov 8, 2024
@@ -82,9 +82,17 @@ spec:
{{- $pdfHost := printf "%s-pdf-svc.%s" .Release.Name .Release.Namespace}}
value: {{ printf "http://%s:%v" $pdfHost .Values.pdf.service.port | quote }}
- name: LAGO_API_URL
value: {{ required "apiUrl value is required" .Values.apiUrl | quote }}
value: {{- if .Values.LAGO_DOMAIN }}
Copy link
Contributor

Choose a reason for hiding this comment

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

we should not use those env var anymore if LAGO_DOMAIN is present

@@ -24,11 +24,19 @@ spec:
containers:
- env:
- name: API_URL
value: {{ .Values.apiUrl | quote }}
value: {{- if .Values.LAGO_DOMAIN }}
Copy link
Contributor

Choose a reason for hiding this comment

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

this is already managed by FE : getlago/lago-front@f74e6e5

We just need to add the LAGO_DOMAIN env var here!

# apiUrl: mydomain.dev
# frontUrl: mydomain.dev

# By setting the LAGO_DOMAIN variable, your application will be available directly on the specified domain.
# For example, if LAGO_DOMAIN is set to getmydomain.dev, your GetLago instance will be accessible at https://getmydomain.dev,
# and your API at https://getmydomain.dev/api/api/v1. Note that adding this variable changes the domain structure:
Copy link
Contributor

Choose a reason for hiding this comment

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

I just think about it, but would be awesome if we can manage to not use /api/api, do you think its possible?

@jdenquin jdenquin closed this Dec 26, 2024
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.

2 participants