Skip to content

[WIP] Add initial support for parentRefs #244

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
+221 −0
Open

[WIP] Add initial support for parentRefs #244

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
95 changes: 95 additions & 0 deletions cmd/print.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"fmt"
"os"
"slices"
"strconv"
"strings"

"github.com/kubernetes-sigs/ingress2gateway/pkg/i2gw"
Expand All @@ -30,6 +31,8 @@ import (
"k8s.io/cli-runtime/pkg/genericclioptions"
"k8s.io/cli-runtime/pkg/printers"
"k8s.io/client-go/tools/clientcmd"
"k8s.io/utils/ptr"
gatewayv1 "sigs.k8s.io/gateway-api/apis/v1"

// Call init function for the providers
_ "github.com/kubernetes-sigs/ingress2gateway/pkg/i2gw/providers/apisix"
Expand Down Expand Up @@ -73,6 +76,19 @@ type PrintRunner struct {

// Provider specific flags --<provider>-<flag>.
providerSpecificFlags map[string]*string

// parentRefs indicates that the created *Route should contain a specific set
// of parentRefs. Its format should be as: --parentRefs=ns/name:sectionname:port=group/kind
Copy link
Member

@LiorLieberman LiorLieberman Sep 2, 2025

Choose a reason for hiding this comment

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

not sure I understand the format.what is the double "=" for?

Copy link
Member

@LiorLieberman LiorLieberman Sep 2, 2025

Choose a reason for hiding this comment

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

like why we use both ":" and "=" as dividers? I understand the code and what it does but probably missing something on why we choose it.

also this would highly benefit from being a type by itself, I think.

Copy link
Member Author

@rikatz rikatz Sep 3, 2025

Choose a reason for hiding this comment

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

It would, do we support types on flags?

So, from the structure of the flag itself, I wanted to follow something that was "as easy as set just the name of the ref" (--parentRef=xpto) but as complex as "I need all of the other fields".

To be really honest, I don't like flags that start to have its own complexity to cover all of the edge cases (eg.: do we really want to add port? sectionName? etc) but also, how can we make it easier once we support ListenerSet to provide the proper conversion? I am open to ideas here

One last comment: this format of the flag was highly inspired on "kubectl create ingress" command, I just did inverted some parameters to make it easier to have something like --parentRefs=Ricardo=XListenerSet

// In case the group is empty it is: --parentRefs=ns/myservice=kind
// In case the namespace is empty (eg.: use the same) it is: --parentRefs=name:sectionname=group/kind
// In case the section name is empty but not the port, it is: --parentRefs=ns/name::port=group/kind
// In case the port or sectionname are empty, they can be passed as: --parentRefs=ns/name=group/kind
// In case just the name should be used it can be passed as: --parentRefs=ns/name (or --parentRefs=name)
// In case the name and kind are desired, it can be passed as: --parentRefs=name=kind
parentRefs []string

// parsedParentRefs represents the parentRefs that should be added to *Route
parsedParentRefs []gatewayv1.ParentReference
}

// PrintGatewayAPIObjects performs necessary steps to digest and print
Expand All @@ -89,6 +105,11 @@ func (pr *PrintRunner) PrintGatewayAPIObjects(cmd *cobra.Command, _ []string) er
return fmt.Errorf("failed to initialize namespace filter: %w", err)
}

err = pr.initializeParentRefs()
if err != nil {
return fmt.Errorf("failed parsing parent refs: %w", err)
}

gatewayResources, notificationTablesMap, err := i2gw.ToGatewayAPIResources(cmd.Context(), pr.namespaceFilter, pr.inputFile, pr.providers, pr.getProviderSpecificFlags())
if err != nil {
return err
Expand Down Expand Up @@ -257,6 +278,68 @@ func (pr *PrintRunner) outputResult(gatewayResources []i2gw.GatewayResources) {
}
}

// initializeParentRefs parses the parentRef flag and sets the desired parentReferences
// to be added to the *Route
// This parser will parse a format like ns/name:sectionname:port=group/kind
func (pr *PrintRunner) initializeParentRefs() error {
if len(pr.parentRefs) == 0 {
return nil
}

pr.parsedParentRefs = make([]gatewayv1.ParentReference, 0)
for _, ref := range pr.parentRefs {
// split resource=group/kind in two
resourceAndGroupKind := strings.Split(ref, "=")
// Simpler case is just ns/name existing, so we short-circuit from here
nsName := strings.SplitN(resourceAndGroupKind[0], "/", 2)
parsedParentRef := gatewayv1.ParentReference{}

var ns, name string
if len(nsName) == 1 {
name = nsName[0]
} else {
ns = nsName[0]
name = nsName[1]
}

// Check and split if name contains :sectionname:port
nameSectionPort := strings.Split(name, ":")

if len(nameSectionPort) > 1 {
name = nameSectionPort[0]
if nameSectionPort[1] != "" {
parsedParentRef.SectionName = ptr.To(gatewayv1.SectionName(nameSectionPort[1]))
}
if len(nameSectionPort) > 2 && nameSectionPort[2] != "" {
portNumber, err := strconv.Atoi(nameSectionPort[2])
if err != nil {
return fmt.Errorf("parentRef %s contains invalid port number", ref)
}
parsedParentRef.Port = ptr.To(gatewayv1.PortNumber(portNumber))
}
}
// Done parsing the first part, we can set the parentRef namespace and name
parsedParentRef.Name = gatewayv1.ObjectName(name)
if ns != "" {
parsedParentRef.Namespace = ptr.To(gatewayv1.Namespace(ns))
}

// If the flag contains group and kind
if len(resourceAndGroupKind) > 1 {
groupKind := strings.SplitN(resourceAndGroupKind[1], "/", 2)
if len(groupKind) == 1 {
parsedParentRef.Kind = ptr.To(gatewayv1.Kind(groupKind[0]))
} else {
parsedParentRef.Group = ptr.To(gatewayv1.Group(groupKind[0]))
parsedParentRef.Kind = ptr.To(gatewayv1.Kind(groupKind[1]))
}
}

pr.parsedParentRefs = append(pr.parsedParentRefs, parsedParentRef)
}
return nil
}

// initializeResourcePrinter assign a specific type of printers.ResourcePrinter
// based on the outputFormat of the printRunner struct.
func (pr *PrintRunner) initializeResourcePrinter() error {
Expand Down Expand Up @@ -339,6 +422,18 @@ if specified with --namespace.`)
cmd.Flags().StringSliceVar(&pr.providers, "providers", []string{},
fmt.Sprintf("If present, the tool will try to convert only resources related to the specified providers, supported values are %v.", i2gw.GetSupportedProviders()))

cmd.Flags().StringSliceVar(&pr.parentRefs, "parent-refs", []string{},
`Allows defining the parentRefs to be set on Route resources. Format is ns/name:sectionname:port=group/kind.
Examples:
--parentRefs=name - Sets only the name of the parent
--parentRefs=ns/name - Sets the namespace/name
--parentRefs=name=kind - Sets the name and kind
--parentRefs=ns/name=group/kind - Sets the namespace, name, group and kind
--parentRefs=name:section=group/kind - Sets the name, section, group and kind
--parentRefs=name::port=group/kind - Sets the name, port, group and kind
`,
)

pr.providerSpecificFlags = make(map[string]*string)
for provider, flags := range i2gw.GetProviderSpecificFlagDefinitions() {
for _, flag := range flags {
Expand Down
126 changes: 126 additions & 0 deletions cmd/print_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,14 @@ import (
"os"
"path/filepath"
"reflect"
"strings"
"testing"

"github.com/davecgh/go-spew/spew"
"github.com/google/go-cmp/cmp"
"k8s.io/cli-runtime/pkg/printers"
"k8s.io/utils/ptr"
gatewayv1 "sigs.k8s.io/gateway-api/apis/v1"
)

func Test_getResourcePrinter(t *testing.T) {
Expand Down Expand Up @@ -81,6 +85,128 @@ func Test_getResourcePrinter(t *testing.T) {
}
}

func Test_initializeParentRefs(t *testing.T) {
testCases := []struct {
name string
parentRefs []string
expectedParentRefs []gatewayv1.ParentReference
expectedError string
}{
{
name: "empty parentref should return null parentRefs",
expectedParentRefs: nil,
},
{
name: "resource only parentRef should return just the object name set",
parentRefs: []string{"someresource"},
expectedParentRefs: []gatewayv1.ParentReference{
{
Name: "someresource",
},
},
},
{
name: "resource and namespace parentRef should return the right object",
parentRefs: []string{"somens/someresource"},
expectedParentRefs: []gatewayv1.ParentReference{
{
Name: "someresource",
Namespace: ptr.To(gatewayv1.Namespace("somens")),
},
},
},
{
name: "resource, namespace and kind parentRef should return the right object",
parentRefs: []string{"somens/someresource=Something"},
expectedParentRefs: []gatewayv1.ParentReference{
{
Name: "someresource",
Namespace: ptr.To(gatewayv1.Namespace("somens")),
Kind: ptr.To(gatewayv1.Kind("Something")),
},
},
},
{
name: "resource, namespace, group kind parentRef should return the right object",
parentRefs: []string{"somens/someresource=somegroup.k8s.io/Something"},
expectedParentRefs: []gatewayv1.ParentReference{
{
Name: "someresource",
Namespace: ptr.To(gatewayv1.Namespace("somens")),
Kind: ptr.To(gatewayv1.Kind("Something")),
Group: ptr.To(gatewayv1.Group("somegroup.k8s.io")),
},
},
},
{
name: "resource, namespace and sectionname parentRef should return the right object",
parentRefs: []string{"somens/someresource:section1"},
expectedParentRefs: []gatewayv1.ParentReference{
{
Name: "someresource",
Namespace: ptr.To(gatewayv1.Namespace("somens")),
SectionName: ptr.To(gatewayv1.SectionName("section1")),
},
},
},
{
name: "resource, namespace and port parentRef should return the right object",
parentRefs: []string{"somens/someresource::12345"},
expectedParentRefs: []gatewayv1.ParentReference{
{
Name: "someresource",
Namespace: ptr.To(gatewayv1.Namespace("somens")),
Port: ptr.To(gatewayv1.PortNumber(12345)),
},
},
},
{
name: "resource, namespace, sectionname, port and resource kind parentRef should return the right object",
parentRefs: []string{"somens/someresource:section1:12345=Gateway"},
expectedParentRefs: []gatewayv1.ParentReference{
{
Name: "someresource",
Namespace: ptr.To(gatewayv1.Namespace("somens")),
SectionName: ptr.To(gatewayv1.SectionName("section1")),
Kind: ptr.To(gatewayv1.Kind("Gateway")),
Port: ptr.To(gatewayv1.PortNumber(12345)),
},
},
},
{
name: "invalid port should return an error",
parentRefs: []string{"somens/someresource:section1:xpto=Gateway"},
expectedError: "Gateway contains invalid port number",
},
}
for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
pr := PrintRunner{
parentRefs: tc.parentRefs,
}
err := pr.initializeParentRefs()

if tc.expectedError != "" {
if err == nil {
t.Errorf("Expected error but got none")
}
if !strings.Contains(err.Error(), tc.expectedError) {
t.Errorf("got invalid error, expecting=%s got=%s", tc.expectedError, err.Error())
}
}

if tc.expectedError == "" && err != nil {
t.Errorf("Expected no error but got %v", err)
}

if tc.expectedError == "" && !reflect.DeepEqual(tc.expectedParentRefs, pr.parsedParentRefs) {
t.Errorf("parsedParentRef does not match. Expected=%s got=%s", spew.Sdump(tc.expectedParentRefs), spew.Sdump(pr.parsedParentRefs))
}
})
}

}

func Test_getNamespaceFilter(t *testing.T) {
testCases := []struct {
name string
Expand Down