diff --git a/Changelog.md b/Changelog.md index 2b20b8882e1..1efaf893157 100644 --- a/Changelog.md +++ b/Changelog.md @@ -13,6 +13,8 @@ ### Bug Fixes +- Completely move to using networking v1 and extensions v1 ingresses based on cluster support as extensions v1 ingress is deprecated ([#4853](https://github.com/openshift/odo/pull/4853)) + ### Tests ### Documentation diff --git a/pkg/kclient/fakeclient.go b/pkg/kclient/fakeclient.go index b1654fe3489..62b088580e8 100644 --- a/pkg/kclient/fakeclient.go +++ b/pkg/kclient/fakeclient.go @@ -38,6 +38,7 @@ func FakeNewWithIngressSupports(networkingv1Supported, extensionV1Supported bool client.appsClient = fkclientset.Kubernetes.AppsV1() client.isExtensionV1Beta1IngressSupported = extensionV1Supported client.isNetworkingV1IngressSupported = networkingv1Supported + client.checkIngressSupports = false client.SetDiscoveryInterface(NewKubernetesFakedDiscovery(true, true)) return &client, &fkclientset diff --git a/pkg/kclient/ingress.go b/pkg/kclient/ingress.go index c84e266c5b8..ec5d8a3b5b1 100644 --- a/pkg/kclient/ingress.go +++ b/pkg/kclient/ingress.go @@ -5,66 +5,24 @@ import ( "fmt" "github.com/openshift/odo/pkg/unions" - "github.com/pkg/errors" - - extensionsv1 "k8s.io/api/extensions/v1beta1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) -// CreateIngressExtensionV1 creates an ingress object for the given service and with the given labels -func (c *Client) CreateIngressExtensionV1(ingress extensionsv1.Ingress) (*extensionsv1.Ingress, error) { - if ingress.GetName() == "" { - return nil, fmt.Errorf("ingress name is empty") - } - ingressObj, err := c.KubeClient.ExtensionsV1beta1().Ingresses(c.Namespace).Create(context.TODO(), &ingress, metav1.CreateOptions{FieldManager: FieldManager}) - if err != nil { - return nil, errors.Wrap(err, "error creating ingress") - } - return ingressObj, nil -} - -// DeleteIngressExtensionV1 deletes the given ingress -func (c *Client) DeleteIngressExtensionV1(name string) error { - err := c.KubeClient.ExtensionsV1beta1().Ingresses(c.Namespace).Delete(context.TODO(), name, metav1.DeleteOptions{}) - if err != nil { - return errors.Wrap(err, "unable to delete ingress") - } - return nil -} - -// ListIngressesExtensionV1 lists all the ingresses based on the given label selector -func (c *Client) ListIngressesExtensionV1(labelSelector string) ([]extensionsv1.Ingress, error) { - ingressList, err := c.KubeClient.ExtensionsV1beta1().Ingresses(c.Namespace).List(context.TODO(), metav1.ListOptions{ - LabelSelector: labelSelector, - }) - if err != nil { - return nil, errors.Wrap(err, "unable to get ingress list") - } - - return ingressList.Items, nil -} - // GetOneIngressFromSelector gets one ingress with the given selector // if no or multiple ingresses are found with the given selector, it throws an error -func (c *Client) GetOneIngressFromSelector(selector string) (*extensionsv1.Ingress, error) { - ingresses, err := c.ListIngressesExtensionV1(selector) +func (c *Client) GetOneIngressFromSelector(selector string) (*unions.KubernetesIngress, error) { + ingresses, err := c.ListIngresses(selector) if err != nil { return nil, err } - if num := len(ingresses); num == 0 { + if num := len(ingresses.Items); num == 0 { return nil, fmt.Errorf("no ingress was found for the selector: %v", selector) } else if num > 1 { return nil, fmt.Errorf("multiple ingresses exist for the selector: %v. Only one must be present", selector) } - return &ingresses[0], nil -} - -// GetIngressExtensionV1 gets an ingress based on the given name -func (c *Client) GetIngressExtensionV1(name string) (*extensionsv1.Ingress, error) { - ingress, err := c.KubeClient.ExtensionsV1beta1().Ingresses(c.Namespace).Get(context.TODO(), name, metav1.GetOptions{}) - return ingress, err + return ingresses.Items[0], nil } //CreateIngress creates a specified Kubernetes Ingress as a networking v1 or extensions v1beta1 ingress depending on what @@ -75,6 +33,13 @@ func (c *Client) CreateIngress(ingress unions.KubernetesIngress) (*unions.Kubern if !ingress.IsGenerated() { return nil, fmt.Errorf("create ingress should get a generated ingress. If you are hiting this, contact the developer") } + if ingress.GetName() == "" { + return nil, fmt.Errorf("cannot create an ingress without a name") + } + err = c.checkIngressSupport() + if err != nil { + return nil, err + } created := false kubernetesIngress := unions.NewNonGeneratedKubernetesIngress() if c.isNetworkingV1IngressSupported { @@ -98,6 +63,10 @@ func (c *Client) CreateIngress(ingress unions.KubernetesIngress) (*unions.Kubern func (c *Client) DeleteIngress(name string) error { var err error + err = c.checkIngressSupport() + if err != nil { + return err + } if c.isNetworkingV1IngressSupported { err = c.KubeClient.NetworkingV1().Ingresses(c.Namespace).Delete(context.TODO(), name, metav1.DeleteOptions{}) if err != nil { @@ -113,8 +82,12 @@ func (c *Client) DeleteIngress(name string) error { } //ListIngresses lists all the ingresses based on given label selector -func (c *Client) ListIngresses(labelSelector string) ([]*unions.KubernetesIngress, error) { - var kubernetesIngressList []*unions.KubernetesIngress +func (c *Client) ListIngresses(labelSelector string) (*unions.KubernetesIngressList, error) { + kubernetesIngressList := unions.NewEmptyKubernetesIngressList() + err := c.checkIngressSupport() + if err != nil { + return nil, err + } // if networking v1 ingress is supported then extension v1 ingress are automatically wrapped // by net v1 ingresses if c.isNetworkingV1IngressSupported { @@ -127,9 +100,10 @@ func (c *Client) ListIngresses(labelSelector string) ([]*unions.KubernetesIngres for k := range ingressList.Items { ki := unions.NewNonGeneratedKubernetesIngress() ki.NetworkingV1Ingress = &ingressList.Items[k] - kubernetesIngressList = append(kubernetesIngressList, ki) + kubernetesIngressList.Items = append(kubernetesIngressList.Items, ki) } } + return kubernetesIngressList, nil } else if c.isExtensionV1Beta1IngressSupported { ingressList, err := c.KubeClient.ExtensionsV1beta1().Ingresses(c.Namespace).List(context.TODO(), metav1.ListOptions{ LabelSelector: labelSelector, @@ -140,11 +114,12 @@ func (c *Client) ListIngresses(labelSelector string) ([]*unions.KubernetesIngres for k := range ingressList.Items { ki := unions.NewNonGeneratedKubernetesIngress() ki.ExtensionV1Beta1Ingress = &ingressList.Items[k] - kubernetesIngressList = append(kubernetesIngressList, ki) + kubernetesIngressList.Items = append(kubernetesIngressList.Items, ki) } } + return kubernetesIngressList, nil } - return kubernetesIngressList, nil + return kubernetesIngressList, fmt.Errorf("ingresses on cluster are not supported") } func (c *Client) GetIngress(name string) (*unions.KubernetesIngress, error) { diff --git a/pkg/kclient/ingress_test.go b/pkg/kclient/ingress_test.go index 256e91dc3a0..86bfac2eaf1 100644 --- a/pkg/kclient/ingress_test.go +++ b/pkg/kclient/ingress_test.go @@ -2,6 +2,7 @@ package kclient import ( "fmt" + "github.com/openshift/odo/pkg/unions" "testing" "github.com/devfile/library/pkg/devfile/generator" @@ -13,28 +14,66 @@ import ( ktesting "k8s.io/client-go/testing" ) +func ingressNameMatchError(expected, got []string) string { + if len(expected) != len(got) { + return "invalid error string format expected and got length should be the same" + } + val := "ingress name does not match the expected name" + for i := 0; i < len(expected); i++ { + val = fmt.Sprintf("%s, expected %s, got %s", val, expected[i], got[i]) + } + return fmt.Sprintf("ingress name does not match the expected name, expected: %s, got %s", expected, got) +} + func TestCreateIngress(t *testing.T) { tests := []struct { - name string - ingressName string - wantErr bool + name string + ingressName string + wantErr bool + isNetworkingV1Supported bool + ieExtensionV1Supported bool }{ { - name: "Case: Valid ingress name", - ingressName: "testIngress", - wantErr: false, + name: "Case: Valid networking v1 ingress name", + ingressName: "testIngress", + wantErr: false, + isNetworkingV1Supported: true, + ieExtensionV1Supported: false, }, { - name: "Case: Invalid ingress name", - ingressName: "", - wantErr: true, + name: "Case: Invalid networking v1 ingress name", + ingressName: "", + wantErr: true, + isNetworkingV1Supported: true, + ieExtensionV1Supported: false, + }, + { + name: "Case: Valid extensions v1 beta1 ingress name", + ingressName: "testIngress", + wantErr: false, + isNetworkingV1Supported: false, + ieExtensionV1Supported: true, + }, + { + name: "Case: Invalid extensions v1 beta1 ingress name", + ingressName: "", + wantErr: true, + isNetworkingV1Supported: false, + ieExtensionV1Supported: true, + }, + { + name: "Case: fail if neither is supported", + ingressName: "testIngress", + wantErr: true, + isNetworkingV1Supported: false, + ieExtensionV1Supported: false, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { // initialising the fakeclient - fkclient, fkclientset := FakeNew() + fkclient, fkclientset := FakeNewWithIngressSupports(tt.isNetworkingV1Supported, tt.ieExtensionV1Supported) fkclient.Namespace = "default" objectMeta := generator.GetObjectMeta(tt.ingressName, "default", nil, nil) @@ -42,20 +81,20 @@ func TestCreateIngress(t *testing.T) { ObjectMeta: objectMeta, IngressSpecParams: generator.IngressSpecParams{ServiceName: tt.ingressName}, } - ingress := generator.GetIngress(ingressParams) - createdIngress, err := fkclient.CreateIngressExtensionV1(*ingress) + ingress := unions.NewKubernetesIngressFromParams(ingressParams) + createdIngress, err := fkclient.CreateIngress(*ingress) // Checks for unexpected error cases if !tt.wantErr == (err != nil) { - t.Errorf("fkclient.CreateIngressExtensionV1 unexpected error %v, wantErr %v", err, tt.wantErr) + t.Errorf("fkclient.CreateIngress unexpected error %v, wantErr %v", err, tt.wantErr) } if err == nil { if len(fkclientset.Kubernetes.Actions()) != 1 { t.Errorf("expected 1 action, got: %v", fkclientset.Kubernetes.Actions()) } else { - if createdIngress.Name != tt.ingressName { - t.Errorf("ingress name does not match the expected name, expected: %s, got %s", tt.ingressName, createdIngress.Name) + if createdIngress.GetName() != tt.ingressName { + t.Errorf(ingressNameMatchError([]string{tt.ingressName}, []string{createdIngress.GetName()})) } } } @@ -68,51 +107,103 @@ func TestListIngresses(t *testing.T) { componentName := "testcomponent" componentLabel := "componentName" tests := []struct { - name string - labelSelector string - wantIngress []extensionsv1.Ingress + name string + labelSelector string + wantIngress unions.KubernetesIngressList + isNetworkingV1Supported bool + isExtensionV1Supported bool + wantErr bool }{ { - name: "Case: one ingress", + name: "Case: one networking v1 ingress", labelSelector: fmt.Sprintf("%v=%v", componentLabel, componentName), - wantIngress: []extensionsv1.Ingress{ - extensionsv1.Ingress{ - ObjectMeta: metav1.ObjectMeta{ - Name: "testIngress1", - Labels: map[string]string{ - componentLabel: componentName, + wantIngress: unions.KubernetesIngressList{ + Items: []*unions.KubernetesIngress{ + { + NetworkingV1Ingress: &networkingv1.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Name: "testIngress1", + Labels: map[string]string{ + componentLabel: componentName, + }, + }, }, + ExtensionV1Beta1Ingress: nil, }, }, }, + isNetworkingV1Supported: true, + isExtensionV1Supported: false, + wantErr: false, }, { - name: "Case: two ingresses", + name: "Case: One extension v1 beta1 ingress", labelSelector: fmt.Sprintf("%v=%v", componentLabel, componentName), - wantIngress: []extensionsv1.Ingress{ - extensionsv1.Ingress{ - ObjectMeta: metav1.ObjectMeta{ - Name: "testIngress1", - Labels: map[string]string{ - componentLabel: componentName, + wantIngress: unions.KubernetesIngressList{ + Items: []*unions.KubernetesIngress{ + { + NetworkingV1Ingress: nil, + ExtensionV1Beta1Ingress: &extensionsv1.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Name: "testIngress1", + Labels: map[string]string{ + componentLabel: componentName, + }, + }, }, }, }, - extensionsv1.Ingress{ - ObjectMeta: metav1.ObjectMeta{ - Name: "testIngress2", - Labels: map[string]string{ - componentLabel: componentName, + }, + isNetworkingV1Supported: false, + isExtensionV1Supported: true, + wantErr: false, + }, + { + name: "Case: two networking v1 ingresses", + labelSelector: fmt.Sprintf("%v=%v", componentLabel, componentName), + wantIngress: unions.KubernetesIngressList{ + Items: []*unions.KubernetesIngress{ + { + NetworkingV1Ingress: &networkingv1.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Name: "testIngress1", + Labels: map[string]string{ + componentLabel: componentName, + }, + }, }, + ExtensionV1Beta1Ingress: nil, + }, + { + NetworkingV1Ingress: &networkingv1.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Name: "testIngress2", + Labels: map[string]string{ + componentLabel: componentName, + }, + }, + }, + ExtensionV1Beta1Ingress: nil, }, }, }, + isNetworkingV1Supported: true, + isExtensionV1Supported: false, + wantErr: false, + }, + { + name: "Case: fails if none of the ingresses are supported", + labelSelector: fmt.Sprintf("%v=%v", componentLabel, componentName), + wantIngress: unions.KubernetesIngressList{}, + isNetworkingV1Supported: false, + isExtensionV1Supported: false, + wantErr: true, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { // initialising the fakeclient - fkclient, fkclientset := FakeNew() + fkclient, fkclientset := FakeNewWithIngressSupports(tt.isNetworkingV1Supported, tt.isExtensionV1Supported) fkclient.Namespace = "default" fkclientset.Kubernetes.PrependReactor("list", "ingresses", func(action ktesting.Action) (bool, runtime.Object, error) { @@ -120,29 +211,30 @@ func TestListIngresses(t *testing.T) { return true, nil, errors.Errorf("selectors are different") } if action.GetResource().GroupVersion().Group == "networking.k8s.io" { - return true, &networkingv1.Ingress{}, nil + return true, tt.wantIngress.GetNetworkingV1IngressList(true), nil } - ingress := extensionsv1.IngressList{ - Items: tt.wantIngress, - } - return true, &ingress, nil + ingress := tt.wantIngress.GetExtensionV1Beta1IngresList(true) + return true, ingress, nil }) - ingresses, err := fkclient.ListIngressesExtensionV1(tt.labelSelector) + ingresses, err := fkclient.ListIngresses(tt.labelSelector) - if err != nil { - t.Errorf("fkclient.ListIngressesExtensionV1 unexpected error %v", err) + if tt.wantErr && err == nil { + t.Errorf("fkclient.ListIngress expected err got %s", err) + } + if !tt.wantErr && err != nil { + t.Errorf("fkclient.ListIngresses unexpected error %v", err) } if err == nil { if len(fkclientset.Kubernetes.Actions()) != 1 { t.Errorf("expected 1 action, got: %v", fkclientset.Kubernetes.Actions()) } else { - if len(tt.wantIngress) != len(ingresses) { - t.Errorf("IngressList length is different, expected %v, got %v", len(tt.wantIngress), len(ingresses)) - } else if len(ingresses) == 1 && ingresses[0].Name != tt.wantIngress[0].Name { - t.Errorf("ingress name does not match the expected name, expected: %s, got %s", tt.wantIngress[0].Name, ingresses[0].Name) - } else if len(ingresses) == 2 && (ingresses[0].Name != tt.wantIngress[0].Name || ingresses[1].Name != tt.wantIngress[1].Name) { - t.Errorf("ingress name does not match the expected name, expected: %s and %s, got %s and %s", tt.wantIngress[0].Name, tt.wantIngress[1].Name, ingresses[0].Name, ingresses[1].Name) + if len(tt.wantIngress.Items) != len(ingresses.Items) { + t.Errorf("IngressList length is different, expected %v, got %v", len(tt.wantIngress.Items), len(ingresses.Items)) + } else if len(ingresses.Items) == 1 && ingresses.Items[0].GetName() != tt.wantIngress.Items[0].GetName() { + t.Errorf(ingressNameMatchError([]string{tt.wantIngress.Items[0].GetName()}, []string{ingresses.Items[0].GetName()})) + } else if len(ingresses.Items) == 2 && (ingresses.Items[0].GetName() != tt.wantIngress.Items[0].GetName() || ingresses.Items[1].GetName() != tt.wantIngress.Items[1].GetName()) { + t.Errorf(ingressNameMatchError([]string{tt.wantIngress.Items[0].GetName(), tt.wantIngress.Items[1].GetName()}, []string{ingresses.Items[0].GetName(), ingresses.Items[1].GetName()})) } } } @@ -155,27 +247,38 @@ func TestListIngresses(t *testing.T) { func TestDeleteIngress(t *testing.T) { tests := []struct { - name string - ingressName string - wantErr bool + name string + ingressName string + wantErr bool + isNetworkingV1Supported bool + isExtensionV1Supported bool }{ { - name: "delete test", - ingressName: "testIngress", - wantErr: false, + name: "delete networking v1 test", + ingressName: "testIngress", + wantErr: false, + isNetworkingV1Supported: true, + isExtensionV1Supported: false, + }, + { + name: "delete extension v1 beta1 test", + ingressName: "testIngress", + wantErr: false, + isNetworkingV1Supported: false, + isExtensionV1Supported: true, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { // initialising the fakeclient - fkclient, fkclientset := FakeNew() + fkclient, fkclientset := FakeNewWithIngressSupports(tt.isNetworkingV1Supported, tt.isExtensionV1Supported) fkclient.Namespace = "default" fkclientset.Kubernetes.PrependReactor("delete", "ingresses", func(action ktesting.Action) (bool, runtime.Object, error) { return true, nil, nil }) - err := fkclient.DeleteIngressExtensionV1(tt.ingressName) + err := fkclient.DeleteIngress(tt.ingressName) // Checks for unexpected error cases if !tt.wantErr == (err != nil) { @@ -200,31 +303,59 @@ func TestDeleteIngress(t *testing.T) { func TestGetIngresses(t *testing.T) { tests := []struct { - name string - ingressName string - wantErr bool + name string + ingressName string + wantErr bool + isNetworkingV1IngressSupported bool + isExtensionV1IngressSupported bool }{ { - name: "Case: Valid ingress name", - ingressName: "testIngress", - wantErr: false, + name: "Case: Valid ingress name", + ingressName: "testIngress", + wantErr: false, + isNetworkingV1IngressSupported: true, + isExtensionV1IngressSupported: false, + }, + { + name: "Case: Invalid ingress name", + ingressName: "", + wantErr: true, + isNetworkingV1IngressSupported: true, + isExtensionV1IngressSupported: false, + }, + { + name: "Case: valid extension v1 ingress name", + ingressName: "testIngress", + wantErr: false, + isExtensionV1IngressSupported: true, + isNetworkingV1IngressSupported: false, }, { - name: "Case: Invalid ingress name", - ingressName: "", - wantErr: true, + name: "Case: invalid extension v1 ingress name", + ingressName: "", + wantErr: true, + isExtensionV1IngressSupported: true, + isNetworkingV1IngressSupported: false, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { // initialising the fakeclient - fkclient, fkclientset := FakeNew() + fkclient, fkclientset := FakeNewWithIngressSupports(tt.isNetworkingV1IngressSupported, tt.isExtensionV1IngressSupported) fkclient.Namespace = "default" fkclientset.Kubernetes.PrependReactor("get", "ingresses", func(action ktesting.Action) (bool, runtime.Object, error) { if tt.ingressName == "" { return true, nil, errors.Errorf("ingress name is empty") } + if action.GetResource().Group == "networking.k8s.io" { + ingress := networkingv1.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Name: tt.ingressName, + }, + } + return true, &ingress, nil + } ingress := extensionsv1.Ingress{ ObjectMeta: metav1.ObjectMeta{ Name: tt.ingressName, @@ -233,7 +364,7 @@ func TestGetIngresses(t *testing.T) { return true, &ingress, nil }) - ingress, err := fkclient.GetIngressExtensionV1(tt.ingressName) + ingress, err := fkclient.GetIngress(tt.ingressName) // Checks for unexpected error cases if !tt.wantErr == (err != nil) { @@ -244,8 +375,8 @@ func TestGetIngresses(t *testing.T) { if len(fkclientset.Kubernetes.Actions()) != 1 { t.Errorf("expected 1 action, got: %v", fkclientset.Kubernetes.Actions()) } else { - if ingress.Name != tt.ingressName { - t.Errorf("ingress name does not match the expected name, expected: %s, got %s", tt.ingressName, ingress.Name) + if ingress.GetName() != tt.ingressName { + t.Errorf(ingressNameMatchError([]string{tt.ingressName}, []string{ingress.GetName()})) } } } diff --git a/pkg/kclient/kclient.go b/pkg/kclient/kclient.go index ab44e4ee345..e31abb6b422 100644 --- a/pkg/kclient/kclient.go +++ b/pkg/kclient/kclient.go @@ -54,7 +54,10 @@ type Client struct { supportedResources map[string]bool // Is server side apply supported by cluster // Use IsSSASupported() - isSSASupported *bool + isSSASupported *bool + // checkIngressSupports is used to check ingress support + //(used to prevent duplicate checks and disable check in UTs) + checkIngressSupports bool isNetworkingV1IngressSupported bool isExtensionV1Beta1IngressSupported bool } @@ -116,10 +119,7 @@ func NewForConfig(config clientcmd.ClientConfig) (client *Client, err error) { return nil, err } - err = client.checkIngressSupport() - if err != nil { - return nil, err - } + client.checkIngressSupports = true return client, nil } @@ -281,13 +281,16 @@ func (c *Client) IsSSASupported() bool { func (c *Client) checkIngressSupport() error { var err error - c.isNetworkingV1IngressSupported, err = c.IsResourceSupported("networking.k8s.io", "v1", "ingresses") - if err != nil { - return fmt.Errorf("failed to check networking v1 ingress support %w", err) - } - c.isExtensionV1Beta1IngressSupported, err = c.IsResourceSupported("extensions", "v1beta1", "ingresses") - if err != nil { - return fmt.Errorf("failed to check extensions v1beta1 ingress support %w", err) + if c.checkIngressSupports { + c.isNetworkingV1IngressSupported, err = c.IsResourceSupported("networking.k8s.io", "v1", "ingresses") + if err != nil { + return fmt.Errorf("failed to check networking v1 ingress support %w", err) + } + c.isExtensionV1Beta1IngressSupported, err = c.IsResourceSupported("extensions", "v1beta1", "ingresses") + if err != nil { + return fmt.Errorf("failed to check extensions v1beta1 ingress support %w", err) + } + c.checkIngressSupports = false } return nil } diff --git a/pkg/odogenerator/ingressgenerator.go b/pkg/odogenerator/ingressgenerator.go index 0c2698832a0..dac18389f70 100644 --- a/pkg/odogenerator/ingressgenerator.go +++ b/pkg/odogenerator/ingressgenerator.go @@ -1,7 +1,6 @@ package odogenerator import ( - "fmt" "github.com/devfile/library/pkg/devfile/generator" "k8s.io/api/networking/v1" ) @@ -10,6 +9,7 @@ import ( // getNetworkingV1IngressSpec gets an networking v1 ingress spec func getNetworkingV1IngressSpec(ingressSpecParams generator.IngressSpecParams) *v1.IngressSpec { path := "/" + pathType := v1.PathTypeImplementationSpecific if ingressSpecParams.Path != "" { path = ingressSpecParams.Path } @@ -26,12 +26,12 @@ func getNetworkingV1IngressSpec(ingressSpecParams generator.IngressSpecParams) * Service: &v1.IngressServiceBackend{ Name: ingressSpecParams.ServiceName, Port: v1.ServiceBackendPort{ - Name: fmt.Sprintf("%s%d", ingressSpecParams.ServiceName, ingressSpecParams.PortNumber.IntVal), Number: ingressSpecParams.PortNumber.IntVal, }, }, Resource: nil, }, + PathType: &pathType, }, }, }, diff --git a/pkg/unions/kubernetes_ingress.go b/pkg/unions/kubernetes_ingress.go index 79e2e027818..498e74b4cf5 100644 --- a/pkg/unions/kubernetes_ingress.go +++ b/pkg/unions/kubernetes_ingress.go @@ -1,6 +1,7 @@ package unions import ( + "fmt" "github.com/devfile/library/pkg/devfile/generator" "github.com/openshift/odo/pkg/odogenerator" "k8s.io/api/extensions/v1beta1" @@ -43,3 +44,74 @@ func NewKubernetesIngressFromParams(ingressParams generator.IngressParams) *Kube func (ki *KubernetesIngress) IsGenerated() bool { return ki.isGenerated } + +//GetName returns the name of underlying networking v1 or extensions v1 ingress +func (ki *KubernetesIngress) GetName() string { + if ki.NetworkingV1Ingress != nil { + return ki.NetworkingV1Ingress.GetName() + } else if ki.ExtensionV1Beta1Ingress != nil { + return ki.ExtensionV1Beta1Ingress.GetName() + } + return "" +} + +//GetProtocol returns `https` if tls is configured on either networking v1 or extensions v1 ingress, else `http` +func (ki *KubernetesIngress) GetProtocol() string { + if (ki.NetworkingV1Ingress != nil && len(ki.NetworkingV1Ingress.Spec.TLS) > 0) || (ki.ExtensionV1Beta1Ingress != nil && len(ki.ExtensionV1Beta1Ingress.Spec.TLS) > 0) { + return "https" + } + return "http" +} + +//GetHost returns the host of underlying networking v1 or extensions v1 ingress +func (ki *KubernetesIngress) GetHost() string { + if ki.NetworkingV1Ingress != nil { + return ki.NetworkingV1Ingress.Spec.Rules[0].Host + } else if ki.ExtensionV1Beta1Ingress != nil { + return ki.ExtensionV1Beta1Ingress.Spec.Rules[0].Host + } + return "" +} + +//GetURLString returns the fully formed url of the form `GetProtocol()://GetHost()` +func (ki *KubernetesIngress) GetURLString() string { + return fmt.Sprintf("%v://%v", ki.GetProtocol(), ki.GetHost()) +} + +type KubernetesIngressList struct { + Items []*KubernetesIngress +} + +func NewEmptyKubernetesIngressList() *KubernetesIngressList { + return &KubernetesIngressList{} +} + +//GetNetworkingV1IngressList returns a v1.IngressList populated by networking v1 ingresses +//if skipIfExtensionV1Set it true, then if both networking v1 and extension v1 are set for +//specific KubernetesIngress, then it will be skipped form the returned list +func (kil *KubernetesIngressList) GetNetworkingV1IngressList(skipIfExtensionV1Set bool) *v1.IngressList { + il := v1.IngressList{} + for _, it := range kil.Items { + if !skipIfExtensionV1Set || (skipIfExtensionV1Set && it.ExtensionV1Beta1Ingress == nil) { + if it.NetworkingV1Ingress != nil { + il.Items = append(il.Items, *it.NetworkingV1Ingress) + } + } + } + return &il +} + +//GetExtensionV1Beta1IngresList returns a v1beta1.IngressList populated by extensions v1 beta1 ingresses +//if skipIfNetworkingV1Set it true, then if both networking v1 and extension v1 are set for +//specific KubernetesIngress, then it will be skipped form the returned list +func (kil *KubernetesIngressList) GetExtensionV1Beta1IngresList(skipIfNetworkingV1Set bool) *v1beta1.IngressList { + il := v1beta1.IngressList{} + for _, it := range kil.Items { + if !skipIfNetworkingV1Set || (skipIfNetworkingV1Set && it.NetworkingV1Ingress == nil) { + if it.ExtensionV1Beta1Ingress != nil { + il.Items = append(il.Items, *it.ExtensionV1Beta1Ingress) + } + } + } + return &il +} diff --git a/pkg/url/kubernetes.go b/pkg/url/kubernetes.go index ff9fbb9f5ce..1d074d1e50d 100644 --- a/pkg/url/kubernetes.go +++ b/pkg/url/kubernetes.go @@ -2,6 +2,7 @@ package url import ( "fmt" + "github.com/openshift/odo/pkg/unions" "sort" "github.com/devfile/library/pkg/devfile/generator" @@ -46,9 +47,7 @@ func (k kubernetesClient) ListFromCluster() (URLList, error) { } var clusterURLs []URL - for _, i := range ingresses { - clusterURLs = append(clusterURLs, NewURLFromKubernetesIngress(i)) - } + clusterURLs = append(clusterURLs, NewURLsFromKubernetesIngressList(ingresses)...) for _, r := range routes { // ignore the routes created by ingresses if r.OwnerReferences != nil && r.OwnerReferences[0].Kind == "Ingress" { @@ -145,7 +144,7 @@ func (k kubernetesClient) Delete(name string, kind localConfigProvider.URLKind) if err != nil { return err } - return k.client.GetKubeClient().DeleteIngressExtensionV1(ingress.Name) + return k.client.GetKubeClient().DeleteIngress(ingress.GetName()) case localConfigProvider.ROUTE: route, err := k.client.GetOneRouteFromSelector(selector) if err != nil { @@ -258,13 +257,13 @@ func (k kubernetesClient) createIngress(url URL, labels map[string]string) (stri Path: url.Spec.Path, }, } - ingress := generator.GetIngress(ingressParam) + ingress := unions.NewKubernetesIngressFromParams(ingressParam) // Pass in the namespace name, link to the service (componentName) and labels to create a ingress - i, err := k.client.GetKubeClient().CreateIngressExtensionV1(*ingress) + i, err := k.client.GetKubeClient().CreateIngress(*ingress) if err != nil { - return "", errors.Wrap(err, "unable to create ingress") + return "", fmt.Errorf("unable to create ingress %w", err) } - return GetURLString(GetProtocol(routev1.Route{}, *i), "", ingressDomain, false), nil + return i.GetURLString(), nil } // createRoute creates a route for the given URL with the given labels diff --git a/pkg/url/types.go b/pkg/url/types.go index a9cd392a992..146f67b2790 100644 --- a/pkg/url/types.go +++ b/pkg/url/types.go @@ -6,6 +6,7 @@ import ( urlLabels "github.com/openshift/odo/pkg/url/labels" "k8s.io/apimachinery/pkg/apis/meta/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "reflect" ) // URL is @@ -52,6 +53,17 @@ const ( StateTypeLocallyDeleted = "Locally Deleted" ) +func NewURLsFromKubernetesIngressList(kil *unions.KubernetesIngressList) []URL { + var urlList []URL + for _, item := range kil.Items { + urlItem := NewURLFromKubernetesIngress(item) + if !reflect.DeepEqual(urlItem, URL{}) { + urlList = append(urlList, urlItem) + } + } + return urlList +} + func NewURLFromKubernetesIngress(ki *unions.KubernetesIngress) URL { if ki.IsGenerated() { return URL{} diff --git a/pkg/url/url.go b/pkg/url/url.go index 5f7e3390b7b..ffc41fb4c15 100644 --- a/pkg/url/url.go +++ b/pkg/url/url.go @@ -72,10 +72,7 @@ func ListPushedIngress(client *kclient.Client, componentName string) (URLList, e } var urls []URL - for _, i := range ingresses { - urls = append(urls, NewURLFromKubernetesIngress(i)) - } - + urls = append(urls, NewURLsFromKubernetesIngressList(ingresses)...) urlList := getMachineReadableFormatForList(urls) return urlList, nil }