Skip to content

Enhance docstrings and assertions on Ingresses tests#1113

Open
gaelgatelement wants to merge 1 commit intomainfrom
gaelg/enhance-assert-messages-ingresses
Open

Enhance docstrings and assertions on Ingresses tests#1113
gaelgatelement wants to merge 1 commit intomainfrom
gaelg/enhance-assert-messages-ingresses

Conversation

@gaelgatelement
Copy link
Copy Markdown
Member

No description provided.

@gaelgatelement gaelgatelement requested a review from a team as a code owner March 9, 2026 10:44
@gaelgatelement gaelgatelement force-pushed the gaelg/enhance-assert-messages-ingresses branch from 20b9f78 to 926b294 Compare March 9, 2026 10:48

When Helm values configure specific hostnames for ingress resources:
- Ingress rules must use the exact hostnames specified in values
- For well-known deployables, use serverName if no host is specified
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
- For well-known deployables, use serverName if no host is specified
- For the well-known deployable, use serverName

Ingress resources must not have ingressClassName by default.

When no specific ingress class is configured, Ingress resources should not include
an ingressClassName field. This allows the default ingress controller to handle
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
an ingressClassName field. This allows the default ingress controller to handle
an ingressClassName field. This allows the cluster's default IngressClass to handle

- Backend services must use the specified service type (LoadBalancer)
- Services must apply the configured traffic policies (internalTrafficPolicy, externalTrafficPolicy)
- Services must include global annotations
- Services must target named ports
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
- Services must target named ports

While this is asserted, it isn't something we configure

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I'm not sure to follow your reasoning here?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

All of the other items in this list are settings in the values file. This item isn't

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