Revert "per-provider-ingress-fetching-isolation"

pkg/i2gw/ingress2gateway.go

@@ -45,19 +45,30 @@ func ToGatewayAPIResources(ctx context.Context, namespace string, inputFile stri
 	}
 	cl = client.NewNamespacedClient(cl, namespace)
 
+	var ingresses networkingv1.IngressList
+
 	providerByName, err := constructProviders(&ProviderConf{
-		Client:    cl,
-		Namespace: namespace,
+		Client: cl,
 	}, providers)
 	if err != nil {
 		return nil, err
 	}
 
+	resources := InputResources{}
+
 	if inputFile != "" {
+		if err = ConstructIngressesFromFile(&ingresses, inputFile, namespace); err != nil {
+			return nil, fmt.Errorf("failed to read ingresses from file: %w", err)
+		}
+		resources.Ingresses = ingresses.Items
 		if err = readProviderResourcesFromFile(ctx, providerByName, inputFile); err != nil {
 			return nil, err
 		}
 	} else {
+		if err = ConstructIngressesFromCluster(ctx, cl, &ingresses); err != nil {
+			return nil, fmt.Errorf("failed to read ingresses from cluster: %w", err)
+		}
+		resources.Ingresses = ingresses.Items
 		if err = readProviderResourcesFromCluster(ctx, providerByName); err != nil {
 			return nil, err
 		}
@@ -68,8 +79,7 @@ func ToGatewayAPIResources(ctx context.Context, namespace string, inputFile stri
 		errs             field.ErrorList
 	)
 	for _, provider := range providerByName {
-		// TODO(#113) Remove input resources from ToGatewayAPI function
-		providerGatewayResources, conversionErrs := provider.ToGatewayAPI(InputResources{})
+		providerGatewayResources, conversionErrs := provider.ToGatewayAPI(resources)
 		errs = append(errs, conversionErrs...)
 		gatewayResources = append(gatewayResources, providerGatewayResources)
 	}
@@ -126,8 +136,7 @@ func constructProviders(conf *ProviderConf, providers []string) (map[ProviderNam
 // ExtractObjectsFromReader extracts all objects from a reader,
 // which is created from YAML or JSON input files.
 // It retrieves all objects, including nested ones if they are contained within a list.
-// The function takes a namespace parameter to optionally return only namespaced resources.
-func ExtractObjectsFromReader(reader io.Reader, namespace string) ([]*unstructured.Unstructured, error) {
+func ExtractObjectsFromReader(reader io.Reader) ([]*unstructured.Unstructured, error) {
 	d := kubeyaml.NewYAMLOrJSONDecoder(reader, 4096)
 	var objs []*unstructured.Unstructured
 	for {
@@ -141,9 +150,6 @@ func ExtractObjectsFromReader(reader io.Reader, namespace string) ([]*unstructur
 		if u == nil {
 			continue
 		}
-		if namespace != "" && u.GetNamespace() != namespace {
-			continue
-		}
 		objs = append(objs, u)
 	}
 
@@ -181,12 +187,15 @@ func ConstructIngressesFromFile(l *networkingv1.IngressList, inputFile string, n
 	}
 
 	reader := bytes.NewReader(stream)
-	objs, err := ExtractObjectsFromReader(reader, namespace)
+	objs, err := ExtractObjectsFromReader(reader)
 	if err != nil {
 		return err
 	}
 
 	for _, f := range objs {
+		if namespace != "" && f.GetNamespace() != namespace {
+			continue
+		}
 		if !f.GroupVersionKind().Empty() && f.GroupVersionKind().Kind == "Ingress" {
 			var i networkingv1.Ingress
 			err = runtime.DefaultUnstructuredConverter.

pkg/i2gw/ingress2gateway_test.go

@@ -17,21 +17,19 @@ limitations under the License.
 package i2gw
 
 import (
-	"bytes"
+	"context"
 	"fmt"
-	"os"
 	"testing"
 
 	"github.com/google/go-cmp/cmp"
 	networkingv1 "k8s.io/api/networking/v1"
 	apiequality "k8s.io/apimachinery/pkg/api/equality"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client/fake"
 )
 
-func Test_ExtractObjectsFromReader(t *testing.T) {
+func Test_constructIngressesFromFile(t *testing.T) {
 	ingress1 := ingress(443, "ingress1", "namespace1")
 	ingress2 := ingress(80, "ingress2", "namespace2")
 	ingressNoNamespace := ingress(80, "ingress-no-namespace", "")
@@ -62,17 +60,10 @@ func Test_ExtractObjectsFromReader(t *testing.T) {
 
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
-			stream, err := os.ReadFile(tc.filePath)
+			gotIngressList := &networkingv1.IngressList{}
+			err := ConstructIngressesFromFile(gotIngressList, tc.filePath, tc.namespace)
 			if err != nil {
-				t.Errorf("failed to read file %s: %v", tc.filePath, err)
-			}
-			unstructuredObjects, err := ExtractObjectsFromReader(bytes.NewReader(stream), tc.namespace)
-			if err != nil {
-				t.Errorf("failed to extract objects: %s", err)
-			}
-			gotIngressList, err := ingressListFromUnstructured(unstructuredObjects)
-			if err != nil {
-				t.Errorf("got unexpected error: %v", err)
+				t.Errorf("Failed to open test file: %v", err)
 			}
 			compareIngressLists(t, gotIngressList, tc.wantIngressList)
 		})
@@ -120,21 +111,6 @@ func ingress(port int32, name, namespace string) networkingv1.Ingress {
 	return ing
 }
 
-func ingressListFromUnstructured(unstructuredObjects []*unstructured.Unstructured) (*networkingv1.IngressList, error) {
-	ingressList := &networkingv1.IngressList{}
-	for _, f := range unstructuredObjects {
-		if !f.GroupVersionKind().Empty() && f.GroupVersionKind().Kind == "Ingress" {
-			var i networkingv1.Ingress
-			err := runtime.DefaultUnstructuredConverter.
-				FromUnstructured(f.UnstructuredContent(), &i)
-			if err != nil {
-				return nil, err
-			}
-			ingressList.Items = append(ingressList.Items, i)
-		}
-	}
-	return ingressList, nil
-}
 func compareIngressLists(t *testing.T, gotIngressList *networkingv1.IngressList, wantIngressList []networkingv1.Ingress) {
 	for i, got := range gotIngressList.Items {
 		want := wantIngressList[i]
@@ -144,6 +120,37 @@ func compareIngressLists(t *testing.T, gotIngressList *networkingv1.IngressList,
 	}
 }
 
+func Test_constructIngressesFromCluster(t *testing.T) {
+	ingress1 := ingress(443, "ingress1", "namespace1")
+	ingress2 := ingress(80, "ingress2", "namespace2")
+	testCases := []struct {
+		name          string
+		runtimeObjs   []runtime.Object
+		wantIngresses []networkingv1.Ingress
+	}{{
+		name:          "Test cluster client with 2 resources",
+		runtimeObjs:   []runtime.Object{&ingress1, &ingress2},
+		wantIngresses: []networkingv1.Ingress{ingress1, ingress2},
+	}, {
+		name:          "Test cluster client without resources",
+		runtimeObjs:   []runtime.Object{},
+		wantIngresses: []networkingv1.Ingress{},
+	},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			gotIngresses := &networkingv1.IngressList{}
+			cl := fake.NewClientBuilder().WithRuntimeObjects(tc.runtimeObjs...).Build()
+			err := ConstructIngressesFromCluster(context.Background(), cl, gotIngresses)
+			if err != nil {
+				t.Errorf("test failed unexpectedly: %v", err)
+			}
+			compareIngressLists(t, gotIngresses, tc.wantIngresses)
+		})
+	}
+}
+
 func Test_constructProviders(t *testing.T) {
 	supportProviders := []string{"ingress-nginx"}
 	for _, provider := range supportProviders {

pkg/i2gw/provider.go

@@ -42,8 +42,7 @@ type ProviderConstructor func(conf *ProviderConf) Provider
 // ProviderConf contains all the configuration required for every concrete
 // Provider implementation.
 type ProviderConf struct {
-	Client    client.Client
-	Namespace string
+	Client client.Client
 }
 
 // The Provider interface specifies the required functionality which needs to be

pkg/i2gw/providers/ingressnginx/converter.go

@@ -19,42 +19,40 @@ package ingressnginx
 import (
 	"github.com/kubernetes-sigs/ingress2gateway/pkg/i2gw"
 	"github.com/kubernetes-sigs/ingress2gateway/pkg/i2gw/providers/common"
-	networkingv1 "k8s.io/api/networking/v1"
 	"k8s.io/apimachinery/pkg/util/validation/field"
 )
 
 // converter implements the ToGatewayAPI function of i2gw.ResourceConverter interface.
 type converter struct {
+	conf *i2gw.ProviderConf
+
 	featureParsers []i2gw.FeatureParser
 }
 
 // newConverter returns an ingress-nginx converter instance.
-func newConverter() *converter {
+func newConverter(conf *i2gw.ProviderConf) *converter {
 	return &converter{
+		conf: conf,
 		featureParsers: []i2gw.FeatureParser{
 			canaryFeature,
 		},
 	}
 }
 
-func (c *converter) convert(storage *storage) (i2gw.GatewayResources, field.ErrorList) {
-
-	// TODO(liorliberman) temporary until we decide to change ToGateway and featureParsers to get a map of [types.NamespacedName]*networkingv1.Ingress instead of a list
-	ingressList := []networkingv1.Ingress{}
-	for _, ing := range storage.Ingresses {
-		ingressList = append(ingressList, *ing)
-	}
+// ToGatewayAPI converts the received i2gw.InputResources to i2gw.GatewayResources
+// including the ingress-nginx specific features.
+func (c *converter) ToGatewayAPI(resources i2gw.InputResources) (i2gw.GatewayResources, field.ErrorList) {
 
 	// Convert plain ingress resources to gateway resources, ignoring all
 	// provider-specific features.
-	gatewayResources, errs := common.ToGateway(ingressList, i2gw.ProviderImplementationSpecificOptions{})
+	gatewayResources, errs := common.ToGateway(resources.Ingresses, i2gw.ProviderImplementationSpecificOptions{})
 	if len(errs) > 0 {
 		return i2gw.GatewayResources{}, errs
 	}
 
 	for _, parseFeatureFunc := range c.featureParsers {
 		// Apply the feature parsing function to the gateway resources, one by one.
-		parseErrs := parseFeatureFunc(i2gw.InputResources{Ingresses: ingressList}, &gatewayResources)
+		parseErrs := parseFeatureFunc(resources, &gatewayResources)
 		// Append the parsing errors to the error list.
 		errs = append(errs, parseErrs...)
 	}

pkg/i2gw/providers/ingressnginx/converter_test.go

@@ -42,14 +42,14 @@ func Test_ToGateway(t *testing.T) {
 
 	testCases := []struct {
 		name                     string
-		ingresses                map[types.NamespacedName]*networkingv1.Ingress
+		ingresses                []networkingv1.Ingress
 		expectedGatewayResources i2gw.GatewayResources
 		expectedErrors           field.ErrorList
 	}{
 		{
 			name: "canary deployment",
-			ingresses: map[types.NamespacedName]*networkingv1.Ingress{
-				{Namespace: "default", Name: "production"}: {
+			ingresses: []networkingv1.Ingress{
+				{
 					ObjectMeta: metav1.ObjectMeta{Name: "production", Namespace: "default"},
 					Spec: networkingv1.IngressSpec{
 						IngressClassName: ptrTo("ingress-nginx"),
@@ -73,7 +73,7 @@ func Test_ToGateway(t *testing.T) {
 						}},
 					},
 				},
-				{Namespace: "default", Name: "canary"}: {
+				{
 					ObjectMeta: metav1.ObjectMeta{
 						Name:      "canary",
 						Namespace: "default",
@@ -168,8 +168,8 @@ func Test_ToGateway(t *testing.T) {
 		},
 		{
 			name: "ImplementationSpecific HTTPRouteMatching",
-			ingresses: map[types.NamespacedName]*networkingv1.Ingress{
-				{Namespace: "default", Name: "implementation-specific-regex"}: {
+			ingresses: []networkingv1.Ingress{
+				{
 					ObjectMeta: metav1.ObjectMeta{
 						Name:      "implementation-specific-regex",
 						Namespace: "default",
@@ -215,22 +215,12 @@ func Test_ToGateway(t *testing.T) {
 
 			provider := NewProvider(&i2gw.ProviderConf{})
 
-			nginxProvider := provider.(*Provider)
-			nginxProvider.storage.Ingresses = tc.ingresses
-
-			// TODO(#113) we pass an empty i2gw.InputResources temporarily until we change ToGatewayAPI function on the interface
-			gatewayResources, errs := provider.ToGatewayAPI(i2gw.InputResources{})
-
-			if len(errs) != len(tc.expectedErrors) {
-				t.Errorf("Expected %d errors, got %d: %+v", len(tc.expectedErrors), len(errs), errs)
-			} else {
-				for i, e := range errs {
-					if errors.Is(e, tc.expectedErrors[i]) {
-						t.Errorf("Unexpected error message at %d index. Got %s, want: %s", i, e, tc.expectedErrors[i])
-					}
-				}
+			resources := i2gw.InputResources{
+				Ingresses: tc.ingresses,
 			}
 
+			gatewayResources, errs := provider.ToGatewayAPI(resources)
+
 			if len(gatewayResources.HTTPRoutes) != len(tc.expectedGatewayResources.HTTPRoutes) {
 				t.Errorf("Expected %d HTTPRoutes, got %d: %+v",
 					len(tc.expectedGatewayResources.HTTPRoutes), len(gatewayResources.HTTPRoutes), gatewayResources.HTTPRoutes)
@@ -258,6 +248,16 @@ func Test_ToGateway(t *testing.T) {
 					}
 				}
 			}
+
+			if len(errs) != len(tc.expectedErrors) {
+				t.Errorf("Expected %d errors, got %d: %+v", len(tc.expectedErrors), len(errs), errs)
+			} else {
+				for i, e := range errs {
+					if errors.Is(e, tc.expectedErrors[i]) {
+						t.Errorf("Unexpected error message at %d index. Got %s, want: %s", i, e, tc.expectedErrors[i])
+					}
+				}
+			}
 		})
 	}
 }

pkg/i2gw/providers/ingressnginx/ingressnginx.go

@@ -17,59 +17,29 @@ limitations under the License.
 package ingressnginx
 
 import (
-	"context"
-	"fmt"
-
 	"github.com/kubernetes-sigs/ingress2gateway/pkg/i2gw"
-	"k8s.io/apimachinery/pkg/util/validation/field"
 )
 
 // The Name of the provider.
 const Name = "ingress-nginx"
-const NginxIngressClass = "nginx"
 
 func init() {
 	i2gw.ProviderConstructorByName[Name] = NewProvider
 }
 
 // Provider implements the i2gw.Provider interface.
 type Provider struct {
-	storage        *storage
-	resourceReader *resourceReader
-	converter      *converter
+	conf *i2gw.ProviderConf
+
+	*resourceReader
+	*converter
 }
 
 // NewProvider constructs and returns the ingress-nginx implementation of i2gw.Provider.
 func NewProvider(conf *i2gw.ProviderConf) i2gw.Provider {
 	return &Provider{
-		storage:        newResourcesStorage(),
+		conf:           conf,
 		resourceReader: newResourceReader(conf),
-		converter:      newConverter(),
-	}
-}
-
-// ToGatewayAPI converts the received i2gw.InputResources to i2gw.GatewayResources
-// including the ingress-nginx specific features.
-func (p *Provider) ToGatewayAPI(_ i2gw.InputResources) (i2gw.GatewayResources, field.ErrorList) {
-	return p.converter.convert(p.storage)
-}
-
-func (p *Provider) ReadResourcesFromCluster(ctx context.Context) error {
-	storage, err := p.resourceReader.readResourcesFromCluster(ctx)
-	if err != nil {
-		return fmt.Errorf("failed to read resources from cluster: %w", err)
-	}
-
-	p.storage = storage
-	return nil
-}
-
-func (p *Provider) ReadResourcesFromFile(ctx context.Context, filename string) error {
-	storage, err := p.resourceReader.readResourcesFromFile(ctx, filename)
-	if err != nil {
-		return fmt.Errorf("failed to read resources from file: %w", err)
+		converter:      newConverter(conf),
 	}
-
-	p.storage = storage
-	return nil
 }