Skip to content

Conversation

@emmanuelmathot
Copy link
Contributor

@emmanuelmathot emmanuelmathot commented Apr 28, 2025

Description

This PR implements a consolidated ingress approach for the eoAPI Helm chart that unifies the NGINX and Traefik implementations, addressing issue #205.

Changes

Created Files

  • helm-chart/eoapi/templates/services/ingress.yaml - The new unified ingress template
  • helm-chart/eoapi/templates/services/doc-server.yaml - Unified document server implementation
  • helm-chart/eoapi/templates/services/traefik-middleware.yaml - Traefik-specific path handling middleware
  • helm-chart/eoapi/tests/ingress_tests.yaml - Comprehensive tests for the new approach
  • docs/unified-ingress.md - Documentation explaining the unified approach

Removed Files

  • helm-chart/eoapi/templates/services/ingress-nginx.yaml
  • helm-chart/eoapi/templates/services/ingress-traefik.yaml
  • helm-chart/eoapi/templates/services/nginx-doc-server.yaml
  • helm-chart/eoapi/templates/services/traefik-doc-server.yaml
  • helm-chart/eoapi/tests/ingress_tests_nginx.yaml

Updated Files

  • helm-chart/eoapi/values.yaml - Added new configuration options for the unified approach
  • helm-chart/eoapi/templates/_helpers.tpl - Removed the Traefik validation function
  • .github/workflows/helm-tests.yml - Updated CI tests to work with the unified approach
  • helm-chart/eoapi/test-k3s-unittest-values.yaml - Updated test values for CI

Implementation Details

Ingress Configuration

  • Single, controller-agnostic template that works for both NGINX and Traefik
  • Path handling strategies:
    • NGINX: Using standard path rewriting with annotations
    • Traefik: Using the stripPrefix middleware to handle path prefixes properly
  • Improved host handling with standard Kubernetes Ingress structure

Traefik Implementation

  • Uses the Traefik stripPrefix middleware to handle path prefixes
  • Dynamically generates prefixes list based on enabled services
  • Temporary solution until uvicorn properly supports real prefixes in ASGI (see uvicorn discussion #2490)

Key Improvements

  1. Eliminated Code Duplication: Consolidated multiple implementation files into a single, maintainable template
  2. Controller Abstraction: Created a clear abstraction layer for controller-specific features
  3. Consistent Path Handling: Standardized path handling across controllers
  4. Removed Artificial Restrictions: Traefik is now fully supported in all environments
  5. Better Extensibility: Easier to add support for additional controllers in the future
  6. Comprehensive Testing: New test suite covers all supported configurations
  7. Thorough Documentation: Provided clear guidance on using the new unified approach

The implementation maintains backward compatibility while providing a more flexible, maintainable, and consistent way to handle ingress across different controllers.

Fixes #205

- Introduced a unified PostgreSQL configuration structure in values.yaml, replacing the old db configuration.
- Added new helper functions for managing PostgreSQL environment variables and secrets based on the selected configuration type (postgrescluster, external-plaintext, external-secret).
- Removed old database-related templates (ConfigMap, Deployment, PVC, Secrets, Service) that are no longer needed.
- Updated the pgstacbootstrap job and configmap templates to align with the new PostgreSQL configuration.
- Implemented validation for PostgreSQL settings to ensure required fields are set based on the selected type.
…he external secret (host, port, database) will override the corresponding values defined in external.host, external.port, and external.database.

Confirmed that the conditional blocks in deployment.yaml were already consolidated to eliminate redundancy. The file was already using a single include statement for PostgreSQL environment variables:

env:
  {{- include "eoapi.postgresqlEnv" $ | nindent 12 }}
Removed the unused eoapi.mapLegacyPostgresql helper function from _helpers.tpl as it wasn't being referenced anywhere in the codebase.
…ik, streamline values.yaml, and update related documentation and tests
@emmanuelmathot emmanuelmathot requested review from pantierra and sunu and removed request for sunu April 28, 2025 20:49
@emmanuelmathot
Copy link
Contributor Author

need #209 to be merged first

@emmanuelmathot emmanuelmathot added the enhancement New feature or request label Apr 28, 2025
@emmanuelmathot emmanuelmathot added this to the Full Cloud Agnostic milestone Apr 28, 2025
Copy link
Member

@batpad batpad left a comment

Choose a reason for hiding this comment

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

This all looks good to me.

Copy link
Member

@sunu sunu left a comment

Choose a reason for hiding this comment

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

Added some minor comments but it looks great to me overall!

@emmanuelmathot emmanuelmathot merged commit c7176cf into main Apr 30, 2025
2 checks passed
@emmanuelmathot emmanuelmathot deleted the unified_ingress branch April 30, 2025 09:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Consolidate Multiple Ingress Implementations into a Single Configuration

4 participants