Skip to content

feat: add default backend for ingress #215

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

Merged
merged 2 commits into from
Mar 26, 2025
Merged

Conversation

markphelps
Copy link
Contributor

@markphelps markphelps requested a review from a team as a code owner March 26, 2025 14:23
@markphelps markphelps requested review from Copilot and removed request for a team March 26, 2025 14:23
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds a default backend configuration for the ingress, addressing a previously reported nil pointer issue. Key changes include:

  • New documentation in README.md outlining ingress configuration.
  • Updates to the ingress template to provide default backend values.
  • A version bump in Chart.yaml and supplementary updates in values.yaml and the chart README to support the new feature.

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
README.md Added documentation for ingress configuration
charts/flipt/templates/ingress.yaml Updated template to set default backend values and service port
charts/flipt/values.yaml Documented the optional backend configuration and its defaults
charts/flipt/README.md Linked to higher-level README and values.yaml for more information
charts/flipt/Chart.yaml Bumped the chart version to reflect the new feature
Comments suppressed due to low confidence (1)

charts/flipt/templates/ingress.yaml:60

  • Similarly, using default "" for .backend here could introduce nil pointer errors. Switch to (default dict .backend) to match the handling of servicePort.
serviceName: {{ default $fullName (default "" .backend).serviceName }}

Signed-off-by: Mark Phelps <[email protected]>
@markphelps markphelps requested a review from Copilot March 26, 2025 16:20
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds a default backend configuration for ingress to address a nil pointer issue encountered during Helm deployments. It updates the values and ingress templates to use default fallbacks, bumps the chart version, and enhances documentation for ingress configuration.

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

Show a summary per file
File Description
charts/flipt/values.yaml Added comments to clarify optional backend configuration in the ingress.
charts/flipt/templates/ingress.yaml Refactored default handling using the default dict pattern to avoid nil pointers.
charts/flipt/Chart.yaml Bumped the chart version from 0.81.0 to 0.82.0.
charts/flipt/README.md Updated docs to include examples for ingress configuration.
README.md Expanded overall documentation with detailed ingress configuration info.
Comments suppressed due to low confidence (1)

charts/flipt/templates/ingress.yaml:52

  • [nitpick] Consider using a with block to assign a local variable for (default dict .backend) to improve readability and reduce repetition in accessing backend fields.
name: {{ default $fullName ((default dict .backend).serviceName) }}

@markphelps markphelps merged commit dea6b9e into main Mar 26, 2025
2 checks passed
@markphelps markphelps deleted the default-ingress-backend branch March 26, 2025 18:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant