Add httproute typed sdk - #473
Merged
Merged
Conversation
Adds to the Mattermost CR, reconciling a gateway.networking.k8s.io/v1 HTTPRoute as an alternative to the nginx Ingress. Opt-in and additive: existing CRs have no key, so behaviour is unchanged.
…ay-api SDK The original implementation used *unstructured.Unstructured to avoid a transitive dependency conflict: gateway-api required structured-merge-diff/v6 while the module was pinned to v4 via k8s.io/apimachinery v0.33.x. Now that k8s.io/* has been upgraded to v0.36.x and controller-runtime to v0.24.1 (both of which already depend on structured-merge-diff/v6), the conflict is gone. gateway-api v1.6.1 now adds cleanly. Replace GenerateHTTPRouteV1Beta, CreateHTTPRouteIfNotExists, DeleteHTTPRoute, and CheckHTTPRoute with typed *gatewayv1.HTTPRoute / *gatewayv1.HTTPRouteList. Register the gateway-api scheme in main.go init() and in prepareSchema for tests. Remove the CRD YAML testdata and schema-pruning test — the compiler now checks field names and types directly. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
@andrleite: Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it. DetailsI understand the commands that are listed here |
mm-cloud-bot
added
do-not-merge/release-note-label-needed
release-note
The committed CRD was generated with an older controller-gen binary that truncated descriptions differently than the current CI binary, causing the CI CRD-diff check to fail. Regenerated with the project's pinned binary using `bin/controller-gen "crd:maxDescLen=200"`. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
SetDefaults validates FileStore and Database on every call, but the HTTPRoute test cases were building minimal Mattermost structs without those fields, causing unrelated validation errors to surface. Added a validMMBase() helper that provides the minimum external configs. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
rahb3rt
approved these changes
Aug 31, 2026
nickmisasi
reviewed
Aug 31, 2026
Comment on lines
+223
to
+263
| func (mm *Mattermost) HTTPRouteEnabled() bool { | ||
| if mm.Spec.HTTPRoute != nil { | ||
| return mm.Spec.HTTPRoute.Enabled | ||
| } | ||
| return false | ||
| } | ||
|
|
||
| // GetHTTPRouteHost returns the primary hostname for the HTTPRoute. | ||
| func (mm *Mattermost) GetHTTPRouteHost() string { | ||
| if mm.Spec.HTTPRoute == nil { | ||
| return "" | ||
| } | ||
| return mm.Spec.HTTPRoute.Host | ||
| } | ||
|
|
||
| // GetHTTPRouteHostNames returns all HTTPRoute hostnames, deduplicated. | ||
| func (mm *Mattermost) GetHTTPRouteHostNames() []string { | ||
| if mm.Spec.HTTPRoute == nil || mm.Spec.HTTPRoute.Host == "" { | ||
| return []string{} | ||
| } | ||
|
|
||
| hostsSet := map[string]struct{}{mm.Spec.HTTPRoute.Host: {}} | ||
| hosts := []string{mm.Spec.HTTPRoute.Host} | ||
|
|
||
| for _, host := range mm.Spec.HTTPRoute.Hosts { | ||
| if _, found := hostsSet[host.HostName]; !found { | ||
| hosts = append(hosts, host.HostName) | ||
| hostsSet[host.HostName] = struct{}{} | ||
| } | ||
| } | ||
|
|
||
| return hosts | ||
| } | ||
|
|
||
| // GetHTTPRouteAnnotations returns HTTPRoute annotations. | ||
| func (mm *Mattermost) GetHTTPRouteAnnotations() map[string]string { | ||
| if mm.Spec.HTTPRoute == nil { | ||
| return nil | ||
| } | ||
| return mm.Spec.HTTPRoute.Annotations | ||
| } |
Contributor
There was a problem hiding this comment.
I think it would make more sense to move these under HTTPRoute. For example:
// GetHTTPRouteAnnotations returns HTTPRoute annotations.
func (mm *Mattermost) GetHTTPRouteAnnotations() map[string]string {
if mm.Spec.HTTPRoute == nil {
return nil
}
return mm.Spec.HTTPRoute.Annotations
}
Would become:
func (h *HTTPRoute) Annotations() map[string]string {
if h == nil {
return nil
}
return h.Annotations
}
Contributor
Author
There was a problem hiding this comment.
Hey Nick, good suggestion. One thing: Go doesn't allow a method and a field to share the same name, so Annotations() on a struct that already has an Annotations field won't compile — same for Enabled(). Two options:
Use Get prefix — GetEnabled(), GetAnnotations(), etc.
Keep the accessors on *Mattermost as-is, matching the existing Ingress pattern (GetIngressAnnotations(), GetIngressHost())
Which do you think?
nickmisasi
requested changes
Aug 31, 2026
nickmisasi
left a comment
nickmisasi
left a comment
Contributor
There was a problem hiding this comment.
Overall LGTM. One refactor suggestion
| // See checkMattermostIngress: UseServiceLoadBalancer takes precedence over any | ||
| // L7 routing configuration, and suppressing it means removing the resource, not | ||
| // merely declining to create one. | ||
| if mattermost.Spec.UseServiceLoadBalancer && mattermost.HTTPRouteEnabled() { |
Contributor
There was a problem hiding this comment.
The above suggestion would make access better too IMO. mattermost.HTTPRoute.Enabled()
fmartingr
approved these changes
Sep 1, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request introduces support for configuring an
HTTPRoute(Gateway API) resource in the Mattermost Operator, allowing users to use Gateway API-based routing instead of, or alongside, traditional Kubernetes Ingress resources. The changes add new CRD fields, validation logic, accessors, and comprehensive unit tests to ensure correct behavior and backward compatibility.HTTPRoute (Gateway API) support:
HTTPRoutefield toMattermostSpec, with a correspondingHTTPRouteSpecstruct that allows enabling/disabling HTTPRoute, specifying hosts, referencing a Gateway, setting annotations, and timeouts. This includes a nestedGatewayReferencetype. [1] [2]HTTPRouteandGatewayReferencetypes, ensuring CRD compatibility and proper code generation. [1] [2] [3] [4]Validation and logic changes:
SetDefaultsto validate that when HTTPRoute is enabled, a host and gateway reference name are required, and to ensure Ingress and HTTPRoute interaction is handled correctly.IngressEnabledlogic to allow HTTPRoute to opt out of the default Ingress creation, preserving legacy behavior and supporting side-by-side migration scenarios.Accessors and helpers:
Mattermosttype for accessing HTTPRoute configuration, including enablement, host(s), annotations, and a unified method for determining the site URL host, which respects the new HTTPRoute logic while preserving legacy behavior.Unit tests:
These changes provide a robust foundation for Gateway API support in the Mattermost Operator, while maintaining backward compatibility and a smooth migration path for existing users.
Release Note