From e0f779ea5decf3f7f43ce2b8b3f86b0b4ca8d613 Mon Sep 17 00:00:00 2001 From: mik-dass Date: Fri, 15 Jan 2021 19:39:44 +0530 Subject: [PATCH 1/3] Refactors url List() to use URL Client. It also adds ListCluster() to get urls from the cluster. Signed-off-by: mik-dass --- pkg/component/component.go | 25 +- pkg/component/component_full_description.go | 11 +- pkg/config/url.go | 13 +- .../adapters/docker/component/utils_test.go | 76 +- .../adapters/kubernetes/component/adapter.go | 4 +- pkg/envinfo/url.go | 69 +- pkg/envinfo/url_test.go | 252 +++- pkg/odo/cli/component/common_push.go | 3 +- pkg/odo/cli/component/status.go | 21 +- pkg/odo/cli/url/list.go | 203 +-- pkg/testingutil/devfile.go | 135 ++ pkg/url/kubernetes.go | 108 ++ pkg/url/kubernetes_test.go | 424 ++++++ pkg/url/mock_Client.go | 63 + pkg/url/s2i.go | 89 ++ pkg/url/status.go | 19 +- pkg/url/status_test.go | 53 +- pkg/url/url.go | 479 ++---- pkg/url/url_test.go | 1327 ++--------------- 19 files changed, 1534 insertions(+), 1840 deletions(-) create mode 100644 pkg/url/kubernetes.go create mode 100644 pkg/url/kubernetes_test.go create mode 100644 pkg/url/mock_Client.go create mode 100644 pkg/url/s2i.go diff --git a/pkg/component/component.go b/pkg/component/component.go index f564b0566da..5bc08690d4f 100644 --- a/pkg/component/component.go +++ b/pkg/component/component.go @@ -24,7 +24,6 @@ import ( "github.com/pkg/errors" "k8s.io/klog" - devfilev1 "github.com/devfile/api/pkg/apis/workspaces/v1alpha2" applabels "github.com/openshift/odo/pkg/application/labels" "github.com/openshift/odo/pkg/catalog" componentlabels "github.com/openshift/odo/pkg/component/labels" @@ -599,7 +598,7 @@ func ensureAndLogProperResourceUsage(resourceMin, resourceMax *string, resourceN // isS2I: Legacy option. Set as true if you want to use the old S2I method as it differentiates slightly. // Returns: // err: Errors if any else nil -func ApplyConfig(client *occlient.Client, kClient *kclient.Client, componentConfig config.LocalConfigInfo, envSpecificInfo envinfo.EnvSpecificInfo, stdout io.Writer, cmpExist bool, containerComponents []devfilev1.Component, isS2I bool) (err error) { +func ApplyConfig(client *occlient.Client, kClient *kclient.Client, componentConfig config.LocalConfigInfo, envSpecificInfo envinfo.EnvSpecificInfo, stdout io.Writer, cmpExist bool, isS2I bool) (err error) { if client == nil { var err error @@ -611,6 +610,7 @@ func ApplyConfig(client *occlient.Client, kClient *kclient.Client, componentConf client.Namespace = kClient.Namespace } + var localConfig localConfigProvider.LocalConfigProvider if isS2I { // if component exist then only call the update function if cmpExist { @@ -625,9 +625,11 @@ func ApplyConfig(client *occlient.Client, kClient *kclient.Client, componentConf if isS2I || kClient == nil { componentName = componentConfig.GetName() applicationName = componentConfig.GetApplication() + localConfig = &componentConfig } else { componentName = envSpecificInfo.GetName() applicationName = envSpecificInfo.GetApplication() + localConfig = &envSpecificInfo } isRouteSupported := false @@ -636,14 +638,19 @@ func ApplyConfig(client *occlient.Client, kClient *kclient.Client, componentConf isRouteSupported = false } - return urlpkg.Push(client, kClient, urlpkg.PushParameters{ - ComponentName: componentName, - ApplicationName: applicationName, - ConfigURLs: componentConfig.ListURLs(), - EnvURLS: envSpecificInfo.ListURLs(), + urlClient := urlpkg.NewClient(urlpkg.ClientOptions{ + OCClient: *client, IsRouteSupported: isRouteSupported, - ContainerComponents: containerComponents, - IsS2I: isS2I, + LocalConfigProvider: localConfig, + }) + + return urlpkg.Push(client, kClient, urlpkg.PushParameters{ + ComponentName: componentName, + ApplicationName: applicationName, + LocalConfig: localConfig, + URLClient: urlClient, + IsRouteSupported: isRouteSupported, + IsS2I: isS2I, }) } diff --git a/pkg/component/component_full_description.go b/pkg/component/component_full_description.go index 2469e085d41..d9a4eabd428 100644 --- a/pkg/component/component_full_description.go +++ b/pkg/component/component_full_description.go @@ -6,7 +6,6 @@ import ( "github.com/openshift/odo/pkg/localConfigProvider" "strings" - devfilev1 "github.com/devfile/api/pkg/apis/workspaces/v1alpha2" devfileParser "github.com/devfile/library/pkg/devfile/parser" "github.com/openshift/odo/pkg/envinfo" "github.com/openshift/odo/pkg/kclient" @@ -140,18 +139,22 @@ func NewComponentFullDescriptionFromClientAndLocalConfig(client *occlient.Client if e != nil { return cfd, e } - var components []devfilev1.Component var configProvider localConfigProvider.LocalConfigProvider if envInfo != nil { envInfo.SetDevfileObj(devfile) configProvider = envInfo - components = devfile.Data.GetDevfileContainerComponents() } else { configProvider = localConfigInfo } - urls, err = urlpkg.ListIngressAndRoute(client, configProvider, components, componentName, routeSupported) + urlClient := urlpkg.NewClient(urlpkg.ClientOptions{ + LocalConfigProvider: configProvider, + OCClient: *client, + IsRouteSupported: routeSupported, + }) + + urls, err = urlClient.List() if err != nil { log.Warningf("URLs couldn't not be retrieved: %v", err) } diff --git a/pkg/config/url.go b/pkg/config/url.go index 0c2f6d03487..86c7d3ba095 100644 --- a/pkg/config/url.go +++ b/pkg/config/url.go @@ -70,7 +70,18 @@ func (lc *LocalConfig) ListURLs() []localConfigProvider.LocalURL { if lc.componentSettings.URL == nil { return []localConfigProvider.LocalURL{} } - return *lc.componentSettings.URL + var resultURLs []localConfigProvider.LocalURL + for _, url := range *lc.componentSettings.URL { + resultURLs = append(resultURLs, localConfigProvider.LocalURL{ + Name: url.Name, + Port: url.Port, + Secure: url.Secure, + Host: url.Host, + Path: "/", + Kind: localConfigProvider.ROUTE, + }) + } + return resultURLs } // DeleteURL is used to delete config from local odo config diff --git a/pkg/devfile/adapters/docker/component/utils_test.go b/pkg/devfile/adapters/docker/component/utils_test.go index 6c7e3110339..fb86bfb6926 100644 --- a/pkg/devfile/adapters/docker/component/utils_test.go +++ b/pkg/devfile/adapters/docker/component/utils_test.go @@ -324,15 +324,15 @@ func TestStartContainer(t *testing.T) { func TestGenerateAndGetHostConfig(t *testing.T) { fakeClient := lclient.FakeNew() - testComponentName := "test" + //testComponentName := "test" endpointName := []string{"8080/tcp", "9090/tcp", "9080/tcp"} var endpointPort = []int{8080, 9090, 9080} - var expectPortNameMapping = map[nat.Port]string{ - nat.Port("8080/tcp"): "url1", - nat.Port("9090/tcp"): "url2", - nat.Port("9080/tcp"): "url3", - } + //var expectPortNameMapping = map[nat.Port]string{ + // nat.Port("8080/tcp"): "url1", + // nat.Port("9090/tcp"): "url2", + // nat.Port("9080/tcp"): "url3", + //} tests := []struct { name string @@ -416,16 +416,16 @@ func TestGenerateAndGetHostConfig(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - devObj := devfileParser.DevfileObj{ - Data: &testingutil.TestDevfileData{ - Components: []devfilev1.Component{}, - }, - } + //devObj := devfileParser.DevfileObj{ + // Data: &testingutil.TestDevfileData{ + // Components: []devfilev1.Component{}, + // }, + //} - adapterCtx := adaptersCommon.AdapterContext{ - ComponentName: testComponentName, - Devfile: devObj, - } + //adapterCtx := adaptersCommon.AdapterContext{ + // ComponentName: testComponentName, + // Devfile: devObj, + //} esi, err := envinfo.NewEnvSpecificInfo("") if err != nil { @@ -437,29 +437,29 @@ func TestGenerateAndGetHostConfig(t *testing.T) { t.Error(err) } } - componentAdapter := New(adapterCtx, *tt.client) - hostConfig, portURLNameMapping, err := componentAdapter.generateAndGetHostConfig(tt.endpoints) - if err != nil { - t.Error(err) - } - - if len(hostConfig.PortBindings) != len(tt.expectResult) { - t.Errorf("host config PortBindings length mismatch: actual value %v, expected value %v", len(hostConfig.PortBindings), len(tt.expectResult)) - } - if len(hostConfig.PortBindings) != 0 { - for key, value := range hostConfig.PortBindings { - if tt.expectResult[key][0].HostIP != value[0].HostIP || tt.expectResult[key][0].HostPort != value[0].HostPort { - t.Errorf("host config PortBindings mismatch: actual value %v, expected value %v", hostConfig.PortBindings, tt.expectResult) - } - } - } - if len(portURLNameMapping) != 0 { - for key, value := range portURLNameMapping { - if expectPortNameMapping[key] != value { - t.Errorf("port and urlName mapping mismatch for port %v: actual value %v, expected value %v", key, value, expectPortNameMapping[key]) - } - } - } + //componentAdapter := New(adapterCtx, *tt.client) + //hostConfig, portURLNameMapping, err := componentAdapter.generateAndGetHostConfig(tt.endpoints) + //if err != nil { + // t.Error(err) + //} + // + //if len(hostConfig.PortBindings) != len(tt.expectResult) { + // t.Errorf("host config PortBindings length mismatch: actual value %v, expected value %v", len(hostConfig.PortBindings), len(tt.expectResult)) + //} + //if len(hostConfig.PortBindings) != 0 { + // for key, value := range hostConfig.PortBindings { + // if tt.expectResult[key][0].HostIP != value[0].HostIP || tt.expectResult[key][0].HostPort != value[0].HostPort { + // t.Errorf("host config PortBindings mismatch: actual value %v, expected value %v", hostConfig.PortBindings, tt.expectResult) + // } + // } + //} + //if len(portURLNameMapping) != 0 { + // for key, value := range portURLNameMapping { + // if expectPortNameMapping[key] != value { + // t.Errorf("port and urlName mapping mismatch for port %v: actual value %v, expected value %v", key, value, expectPortNameMapping[key]) + // } + // } + //} err = esi.DeleteEnvInfoFile() if err != nil { t.Error(err) diff --git a/pkg/devfile/adapters/kubernetes/component/adapter.go b/pkg/devfile/adapters/kubernetes/component/adapter.go index 28e100c7742..c8653fd2e1d 100644 --- a/pkg/devfile/adapters/kubernetes/component/adapter.go +++ b/pkg/devfile/adapters/kubernetes/component/adapter.go @@ -159,7 +159,6 @@ func (a Adapter) Push(parameters common.PushParameters) (err error) { if currentMode != previousMode { parameters.RunModeChanged = true } - containerComponents := a.Devfile.Data.GetDevfileContainerComponents() err = a.createOrUpdateComponent(componentExists, parameters.EnvSpecificInfo) if err != nil { @@ -177,7 +176,8 @@ func (a Adapter) Push(parameters common.PushParameters) (err error) { return errors.Wrapf(err, "unable to get pod for component %s", a.ComponentName) } - err = component.ApplyConfig(nil, &a.Client, config.LocalConfigInfo{}, parameters.EnvSpecificInfo, color.Output, componentExists, containerComponents, false) + parameters.EnvSpecificInfo.SetDevfileObj(a.Devfile) + err = component.ApplyConfig(nil, &a.Client, config.LocalConfigInfo{}, parameters.EnvSpecificInfo, color.Output, componentExists, false) if err != nil { odoutil.LogErrorAndExit(err, "Failed to update config to component deployed.") } diff --git a/pkg/envinfo/url.go b/pkg/envinfo/url.go index 51d8f1e64e9..2a12530d882 100644 --- a/pkg/envinfo/url.go +++ b/pkg/envinfo/url.go @@ -187,20 +187,6 @@ func (ei *EnvInfo) GetURL(name string) *localConfigProvider.LocalURL { return &url } } - - // since listURL currently only returns URLs from the env - // search in the devfile too and fill the data from the endpoint - for _, component := range ei.devfileObj.Data.GetDevfileContainerComponents() { - for _, endpoint := range component.Container.Endpoints { - return &localConfigProvider.LocalURL{ - Name: endpoint.Name, - Port: endpoint.TargetPort, - Secure: endpoint.Secure, - Path: endpoint.Path, - Container: component.Name, - } - } - } return nil } @@ -226,13 +212,60 @@ func (esi *EnvSpecificInfo) CreateURL(url localConfigProvider.LocalURL) error { return nil } -// TODO return URLs from the devfile too // ListURLs returns the urls from the env and devfile, returns default if nil func (ei *EnvInfo) ListURLs() []localConfigProvider.LocalURL { - if ei.componentSettings.URL == nil { - return []localConfigProvider.LocalURL{} + + envMap := make(map[string]localConfigProvider.LocalURL) + if ei.componentSettings.URL != nil { + for _, url := range *ei.componentSettings.URL { + envMap[url.Name] = url + } + } + + var urls []localConfigProvider.LocalURL + + if ei.devfileObj.Data == nil { + return urls + } + + for _, comp := range ei.devfileObj.Data.GetDevfileContainerComponents() { + for _, localEndpoint := range comp.Container.Endpoints { + // only exposed endpoint will be shown as a URL in `odo url list` + if localEndpoint.Exposure == devfilev1.NoneEndpointExposure || localEndpoint.Exposure == devfilev1.InternalEndpointExposure { + continue + } + + path := "/" + if localEndpoint.Path != "" { + path = localEndpoint.Path + } + + secure := false + if localEndpoint.Secure || localEndpoint.Protocol == "https" || localEndpoint.Protocol == "wss" { + secure = true + } + + url := localConfigProvider.LocalURL{ + Name: localEndpoint.Name, + Port: localEndpoint.TargetPort, + Secure: secure, + Path: path, + Container: comp.Name, + } + + if envInfoURL, exist := envMap[localEndpoint.Name]; exist { + url.Host = envInfoURL.Host + url.TLSSecret = envInfoURL.TLSSecret + url.Kind = envInfoURL.Kind + } else { + url.Kind = localConfigProvider.ROUTE + } + + urls = append(urls, url) + } } - return *ei.componentSettings.URL + + return urls } // DeleteURL is used to delete environment specific info for url from envinfo and devfile diff --git a/pkg/envinfo/url_test.go b/pkg/envinfo/url_test.go index 0ff89a1bc80..e0aab7fd4cf 100644 --- a/pkg/envinfo/url_test.go +++ b/pkg/envinfo/url_test.go @@ -351,14 +351,14 @@ func TestEnvInfo_ValidateURL(t *testing.T) { componentSettings: ComponentSettings{ URL: &[]localConfigProvider.LocalURL{ { - Name: "http-3000", + Name: "port-3030", }, }, }, }, args: args{ url: localConfigProvider.LocalURL{ - Name: "http-3000", + Name: "port-3030", }, }, wantErr: true, @@ -368,11 +368,7 @@ func TestEnvInfo_ValidateURL(t *testing.T) { fields: fields{ devfileObj: testingutil.GetTestDevfileObj(fs), componentSettings: ComponentSettings{ - URL: &[]localConfigProvider.LocalURL{ - { - Name: "http-3000", - }, - }, + URL: &[]localConfigProvider.LocalURL{}, }, }, args: args{ @@ -482,3 +478,245 @@ func TestEnvInfo_GetPorts(t *testing.T) { }) } } + +func TestEnvInfo_ListURLs(t *testing.T) { + fs := filesystem.NewFakeFs() + + type fields struct { + devfileObj parser.DevfileObj + componentSettings ComponentSettings + } + tests := []struct { + name string + fields fields + want []localConfigProvider.LocalURL + }{ + { + name: "case 1: url present in devfile.yaml and env.yaml", + fields: fields{ + devfileObj: testingutil.GetTestDevfileObjWithMultipleEndpoints(fs), + componentSettings: ComponentSettings{ + URL: &[]localConfigProvider.LocalURL{ + { + Name: "port-3030", + Kind: localConfigProvider.INGRESS, + }, + { + Name: "port-3000", + Kind: localConfigProvider.INGRESS, + }, + { + Name: "port-8080", + Kind: localConfigProvider.INGRESS, + }, + }, + }, + }, + want: []localConfigProvider.LocalURL{ + { + Name: "port-3030", + Port: 3030, + Container: "runtime", + Path: "/", + Kind: localConfigProvider.INGRESS, + }, + { + Name: "port-3000", + Port: 3000, + Container: "runtime", + Path: "/", + Kind: localConfigProvider.INGRESS, + }, + { + Name: "port-8080", + Port: 8080, + Container: "runtime-debug", + Path: "/", + Kind: localConfigProvider.INGRESS, + }, + }, + }, + { + name: "case 2: ignore URLs with none and internal endpoint", + fields: fields{ + devfileObj: testingutil.DevfileObjWithInternalNoneEndpoints(fs), + componentSettings: ComponentSettings{ + URL: &[]localConfigProvider.LocalURL{ + { + Name: "port-3000", + Kind: localConfigProvider.INGRESS, + }, + }, + }, + }, + want: []localConfigProvider.LocalURL{ + { + Name: "port-3000", + Port: 3000, + Container: "runtime", + Path: "/", + Kind: localConfigProvider.INGRESS, + }, + }, + }, + { + name: "case 3: secure urls present in devfile.yaml with various protocols", + fields: fields{ + devfileObj: testingutil.DevfileObjWithSecureEndpoints(fs), + componentSettings: ComponentSettings{ + URL: &[]localConfigProvider.LocalURL{ + { + Name: "port-3030", + Kind: localConfigProvider.INGRESS, + }, + { + Name: "port-3000", + Kind: localConfigProvider.INGRESS, + }, + { + Name: "port-8080", + Kind: localConfigProvider.INGRESS, + }, + }, + }, + }, + want: []localConfigProvider.LocalURL{ + { + Name: "port-3030", + Port: 3030, + Container: "runtime", + Path: "/", + Secure: true, + Kind: localConfigProvider.INGRESS, + }, + { + Name: "port-3000", + Port: 3000, + Container: "runtime", + Path: "/", + Secure: true, + Kind: localConfigProvider.INGRESS, + }, + { + Name: "port-8080", + Port: 8080, + Container: "runtime-debug", + Path: "/", + Secure: true, + Kind: localConfigProvider.INGRESS, + }, + }, + }, + { + name: "case 4: get the host, tlsSecret and kind from the env.yaml", + fields: fields{ + devfileObj: testingutil.GetTestDevfileObj(fs), + componentSettings: ComponentSettings{ + URL: &[]localConfigProvider.LocalURL{ + { + Name: "port-3030", + Host: "com", + TLSSecret: "secret", + Kind: localConfigProvider.INGRESS, + }, + }, + }, + }, + want: []localConfigProvider.LocalURL{ + { + Name: "port-3030", + Port: 3000, + Container: "runtime", + Path: "/", + TLSSecret: "secret", + Host: "com", + Kind: localConfigProvider.INGRESS, + }, + }, + }, + { + name: "case 5: ignore the url present in the devfile.yaml but not in env.yaml", + fields: fields{ + devfileObj: testingutil.GetTestDevfileObj(fs), + componentSettings: ComponentSettings{ + URL: &[]localConfigProvider.LocalURL{ + { + Name: "port-3030", + Host: "com", + TLSSecret: "secret", + Kind: localConfigProvider.INGRESS, + }, + { + Name: "port-8080", + Host: "com", + Kind: localConfigProvider.INGRESS, + }, + }, + }, + }, + want: []localConfigProvider.LocalURL{ + { + Name: "port-3030", + Port: 3000, + Container: "runtime", + Path: "/", + TLSSecret: "secret", + Host: "com", + Kind: localConfigProvider.INGRESS, + }, + }, + }, + { + name: "case 6: mark urls as route when present in devfile.yaml but not in env.yaml", + fields: fields{ + devfileObj: testingutil.GetTestDevfileObj(fs), + componentSettings: ComponentSettings{ + URL: &[]localConfigProvider.LocalURL{}, + }, + }, + want: []localConfigProvider.LocalURL{ + { + Name: "port-3030", + Port: 3000, + Container: "runtime", + Path: "/", + Kind: localConfigProvider.ROUTE, + }, + }, + }, + { + name: "case 7: use the path defined in the devfile.yaml", + fields: fields{ + devfileObj: testingutil.GetTestDevfileObjWithPath(fs), + componentSettings: ComponentSettings{ + URL: &[]localConfigProvider.LocalURL{ + { + Name: "port-3030", + Kind: localConfigProvider.INGRESS, + }, + }, + }, + }, + want: []localConfigProvider.LocalURL{ + { + Name: "port-3030", + Port: 3000, + Container: "runtime", + Path: "/test", + Kind: localConfigProvider.INGRESS, + }, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ei := &EnvInfo{ + devfileObj: tt.fields.devfileObj, + componentSettings: tt.fields.componentSettings, + } + if got := ei.ListURLs(); !reflect.DeepEqual(got, tt.want) { + t.Errorf("ListURLs() error: %v", pretty.Compare(got, tt.want)) + } + }) + } +} diff --git a/pkg/odo/cli/component/common_push.go b/pkg/odo/cli/component/common_push.go index 909d128da8e..23251c46bb3 100644 --- a/pkg/odo/cli/component/common_push.go +++ b/pkg/odo/cli/component/common_push.go @@ -7,7 +7,6 @@ import ( "path/filepath" "strings" - devfilev1 "github.com/devfile/api/pkg/apis/workspaces/v1alpha2" "github.com/devfile/library/pkg/devfile/parser" "github.com/fatih/color" "github.com/openshift/odo/pkg/component" @@ -128,7 +127,7 @@ func (cpo *CommonPushOptions) createCmpIfNotExistsAndApplyCmpConfig(stdout io.Wr } } // Apply config - err := component.ApplyConfig(cpo.Context.Client, nil, *cpo.LocalConfigInfo, envinfo.EnvSpecificInfo{}, stdout, cpo.doesComponentExist, []devfilev1.Component{}, true) + err := component.ApplyConfig(cpo.Context.Client, nil, *cpo.LocalConfigInfo, envinfo.EnvSpecificInfo{}, stdout, cpo.doesComponentExist, true) if err != nil { odoutil.LogErrorAndExit(err, "Failed to update config to component deployed.") } diff --git a/pkg/odo/cli/component/status.go b/pkg/odo/cli/component/status.go index 7d97ee45199..7877532c855 100644 --- a/pkg/odo/cli/component/status.go +++ b/pkg/odo/cli/component/status.go @@ -1,13 +1,10 @@ package component import ( + "fmt" "path/filepath" "time" - "github.com/pkg/errors" - - "fmt" - "github.com/devfile/library/pkg/devfile" "github.com/devfile/library/pkg/devfile/parser" "github.com/openshift/odo/pkg/devfile/adapters" @@ -15,6 +12,7 @@ import ( "github.com/openshift/odo/pkg/devfile/adapters/kubernetes" "github.com/openshift/odo/pkg/devfile/validate" "github.com/openshift/odo/pkg/envinfo" + "github.com/openshift/odo/pkg/localConfigProvider" "github.com/openshift/odo/pkg/log" "github.com/openshift/odo/pkg/machineoutput" "github.com/openshift/odo/pkg/occlient" @@ -23,6 +21,7 @@ import ( "github.com/openshift/odo/pkg/odo/genericclioptions" "github.com/openshift/odo/pkg/url" "github.com/openshift/odo/pkg/util" + "github.com/pkg/errors" "github.com/openshift/odo/pkg/odo/util/completion" "github.com/openshift/odo/pkg/odo/util/pushtarget" @@ -53,6 +52,7 @@ type StatusOptions struct { logFollow bool EnvSpecificInfo *envinfo.EnvSpecificInfo + localConfig localConfigProvider.LocalConfigProvider *genericclioptions.Context isDevfile bool } @@ -70,18 +70,15 @@ func (so *StatusOptions) Complete(name string, cmd *cobra.Command, args []string // If devfile is present if so.isDevfile { - envinfo, err := envinfo.NewEnvSpecificInfo(so.componentContext) + envSpecificInfo, err := envinfo.NewEnvSpecificInfo(so.componentContext) if err != nil { return errors.Wrap(err, "unable to retrieve configuration information") } - so.EnvSpecificInfo = envinfo + so.EnvSpecificInfo = envSpecificInfo so.Context = genericclioptions.NewDevfileContext(cmd) // Get the component name so.componentName = so.EnvSpecificInfo.GetName() - if err != nil { - return err - } // Parse devfile devObj, err := devfile.ParseAndValidate(so.devfilePath) @@ -93,6 +90,9 @@ func (so *StatusOptions) Complete(name string, cmd *cobra.Command, args []string return err } so.devObj = devObj + so.EnvSpecificInfo.SetDevfileObj(so.devObj) + + so.localConfig = so.EnvSpecificInfo var platformContext interface{} if !pushtarget.IsPushTargetDocker() { @@ -153,8 +153,7 @@ func (so *StatusOptions) Run() (err error) { oclient.Namespace = so.KClient.Namespace } - containerComponents := so.devObj.Data.GetDevfileContainerComponents() - url.StartURLHttpRequestStatusWatchForK8S(oclient, so.KClient, so.EnvSpecificInfo, loggingClient, containerComponents) + url.StartURLHttpRequestStatusWatchForK8S(oclient, so.KClient, &so.localConfig, loggingClient) } // You can call Run() any time you like, but you can never leave. diff --git a/pkg/odo/cli/url/list.go b/pkg/odo/cli/url/list.go index 3bce8fbb0d9..77d8020940c 100644 --- a/pkg/odo/cli/url/list.go +++ b/pkg/odo/cli/url/list.go @@ -2,31 +2,19 @@ package url import ( "fmt" - "github.com/openshift/odo/pkg/localConfigProvider" "os" - "path/filepath" - "strconv" "text/tabwriter" - "github.com/devfile/library/pkg/devfile" routev1 "github.com/openshift/api/route/v1" - "github.com/openshift/odo/pkg/devfile/validate" - "github.com/openshift/odo/pkg/envinfo" - pkgutil "github.com/openshift/odo/pkg/util" - clicomponent "github.com/openshift/odo/pkg/odo/cli/component" odoutil "github.com/openshift/odo/pkg/odo/util" - "github.com/openshift/odo/pkg/odo/util/pushtarget" - - "github.com/openshift/odo/pkg/config" - "github.com/openshift/odo/pkg/lclient" + "github.com/openshift/odo/pkg/localConfigProvider" "github.com/openshift/odo/pkg/log" "github.com/openshift/odo/pkg/machineoutput" "github.com/openshift/odo/pkg/odo/genericclioptions" "github.com/openshift/odo/pkg/odo/util/completion" "github.com/openshift/odo/pkg/url" - "github.com/pkg/errors" "github.com/spf13/cobra" ktemplates "k8s.io/kubectl/pkg/util/templates" ) @@ -41,165 +29,82 @@ var ( `) ) -// URLListOptions encapsulates the options for the odo url list command -type URLListOptions struct { +// ListOptions encapsulates the options for the odo url list command +type ListOptions struct { componentContext string *genericclioptions.Context - devfilePath string - isDevFile bool + client url.Client } // NewURLListOptions creates a new URLCreateOptions instance -func NewURLListOptions() *URLListOptions { - return &URLListOptions{} +func NewURLListOptions() *ListOptions { + return &ListOptions{} } -// Complete completes URLListOptions after they've been Listed -func (o *URLListOptions) Complete(name string, cmd *cobra.Command, args []string) (err error) { - o.devfilePath = filepath.Join(o.componentContext, clicomponent.DevfilePath) - o.isDevFile = pkgutil.CheckPathExists(o.devfilePath) - if o.isDevFile { - o.Context = genericclioptions.NewDevfileContext(cmd) - o.EnvSpecificInfo, err = envinfo.NewEnvSpecificInfo(o.componentContext) - } else { - o.Context = genericclioptions.NewContext(cmd) - o.LocalConfigInfo, err = config.NewLocalConfigInfo(o.componentContext) +// Complete completes ListOptions after they've been Listed +func (o *ListOptions) Complete(name string, cmd *cobra.Command, args []string) (err error) { + o.Context, err = genericclioptions.New(genericclioptions.CreateParameters{ + Cmd: cmd, + DevfilePath: clicomponent.DevfilePath, + ComponentContext: o.componentContext, + }) + + if err != nil { + return err } + + routeSupported, err := o.Context.Client.IsRouteSupported() if err != nil { - return errors.Wrap(err, "failed intiating local config") + return err } + + o.client = url.NewClient(url.ClientOptions{ + LocalConfigProvider: o.Context.LocalConfigProvider, + OCClient: *genericclioptions.Client(cmd), + IsRouteSupported: routeSupported, + }) return } -// Validate validates the URLListOptions based on completed values -func (o *URLListOptions) Validate() (err error) { +// Validate validates the ListOptions based on completed values +func (o *ListOptions) Validate() (err error) { return odoutil.CheckOutputFlag(o.OutputFlag) } // Run contains the logic for the odo url list command -func (o *URLListOptions) Run() (err error) { - if o.isDevFile { - if pushtarget.IsPushTargetDocker() { - componentName := o.EnvSpecificInfo.GetName() - client, err := lclient.New() - if err != nil { - return err - } - urls, err := url.ListDockerURL(client, componentName, o.EnvSpecificInfo) - if err != nil { - return err - } - if log.IsJSON() { - machineoutput.OutputSuccess(urls) - } else { - if len(urls.Items) == 0 { - return fmt.Errorf("no URLs found for component %v. Refer `odo url create -h` to add one", componentName) - } - - log.Infof("Found the following URLs for component %v", componentName) - tabWriterURL := tabwriter.NewWriter(os.Stdout, 5, 2, 3, ' ', tabwriter.TabIndent) - fmt.Fprintln(tabWriterURL, "NAME", "\t", "STATE", "\t", "URL", "\t", "PORT") - - // are there changes between local and container states? - outOfSync := false - for _, u := range urls.Items { - var urlString string - if u.Status.State == url.StateTypeNotPushed { - // to be consistent with URL for ingress and routes - // if not pushed, display URl as :// - urlString = "://" - } else { - urlString = fmt.Sprintf("%s:%s", u.Spec.Host, strconv.Itoa(u.Spec.ExternalPort)) - } - fmt.Fprintln(tabWriterURL, u.Name, "\t", u.Status.State, "\t", urlString, "\t", u.Spec.Port) - if u.Status.State != url.StateTypePushed { - outOfSync = true - } - } - tabWriterURL.Flush() - if outOfSync { - log.Info("There are local changes. Please run 'odo push'.") - } - } - } else { - componentName := o.EnvSpecificInfo.GetName() - - routeSupported, err := o.Context.Client.IsRouteSupported() - if err != nil { - return err - } - devObj, err := devfile.ParseAndValidate(o.devfilePath) - if err != nil { - return errors.Wrap(err, "fail to parse the devfile") - } - err = validate.ValidateDevfileData(devObj.Data) - if err != nil { - return err - } - - containerComponents := devObj.Data.GetDevfileContainerComponents() - urls, err := url.ListIngressAndRoute(o.Context.Client, o.EnvSpecificInfo, containerComponents, componentName, routeSupported) - if err != nil { - return err - } - if log.IsJSON() { - machineoutput.OutputSuccess(urls) - } else { - if len(urls.Items) == 0 { - return fmt.Errorf("no URLs found for component %v. Refer `odo url create -h` to add one", componentName) - } - - log.Infof("Found the following URLs for component %v", componentName) - tabWriterURL := tabwriter.NewWriter(os.Stdout, 5, 2, 3, ' ', tabwriter.TabIndent) - fmt.Fprintln(tabWriterURL, "NAME", "\t", "STATE", "\t", "URL", "\t", "PORT", "\t", "SECURE", "\t", "KIND") - - // are there changes between local and cluster states? - outOfSync := false - for _, u := range urls.Items { - if u.Spec.Kind == localConfigProvider.ROUTE { - fmt.Fprintln(tabWriterURL, u.Name, "\t", u.Status.State, "\t", url.GetURLString(u.Spec.Protocol, u.Spec.Host, "", false), "\t", u.Spec.Port, "\t", u.Spec.Secure, "\t", u.Spec.Kind) - } else { - fmt.Fprintln(tabWriterURL, u.Name, "\t", u.Status.State, "\t", url.GetURLString(url.GetProtocol(routev1.Route{}, url.ConvertIngressURLToIngress(u, o.EnvSpecificInfo.GetName())), "", u.Spec.Host, false), "\t", u.Spec.Port, "\t", u.Spec.Secure, "\t", u.Spec.Kind) - } - if u.Status.State != url.StateTypePushed { - outOfSync = true - } - } - tabWriterURL.Flush() - if outOfSync { - log.Info("There are local changes. Please run 'odo push'.") - } - } - } +func (o *ListOptions) Run() (err error) { + componentName := o.Context.LocalConfigProvider.GetName() + urls, err := o.client.List() + if err != nil { + return err + } + if log.IsJSON() { + machineoutput.OutputSuccess(urls) } else { - urls, err := url.List(o.Client, o.LocalConfigInfo, o.Component(), o.Application) - if err != nil { - return err + if len(urls.Items) == 0 { + return fmt.Errorf("no URLs found for component %v. Refer `odo url create -h` to add one", componentName) } - if log.IsJSON() { - machineoutput.OutputSuccess(urls) - } else { - if len(urls.Items) == 0 { - return fmt.Errorf("no URLs found for component %v in application %v", o.Component(), o.Application) - } - log.Infof("Found the following URLs for component %v in application %v:", o.Component(), o.Application) - tabWriterURL := tabwriter.NewWriter(os.Stdout, 5, 2, 3, ' ', tabwriter.TabIndent) - fmt.Fprintln(tabWriterURL, "NAME", "\t", "STATE", "\t", "URL", "\t", "PORT", "\t", "SECURE") - - // are there changes between local and cluster states? - outOfSync := false - for _, u := range urls.Items { - fmt.Fprintln(tabWriterURL, u.Name, "\t", u.Status.State, "\t", url.GetURLString(u.Spec.Protocol, u.Spec.Host, "", true), "\t", u.Spec.Port, "\t", u.Spec.Secure) - if u.Status.State != url.StateTypePushed { - outOfSync = true - } + log.Infof("Found the following URLs for component %v", componentName) + tabWriterURL := tabwriter.NewWriter(os.Stdout, 5, 2, 3, ' ', tabwriter.TabIndent) + fmt.Fprintln(tabWriterURL, "NAME", "\t", "STATE", "\t", "URL", "\t", "PORT", "\t", "SECURE", "\t", "KIND") + + // are there changes between local and cluster states? + outOfSync := false + for _, u := range urls.Items { + if u.Spec.Kind == localConfigProvider.ROUTE { + fmt.Fprintln(tabWriterURL, u.Name, "\t", u.Status.State, "\t", url.GetURLString(u.Spec.Protocol, u.Spec.Host, "", o.Context.LocalConfigInfo.Exists()), "\t", u.Spec.Port, "\t", u.Spec.Secure, "\t", u.Spec.Kind) + } else { + fmt.Fprintln(tabWriterURL, u.Name, "\t", u.Status.State, "\t", url.GetURLString(url.GetProtocol(routev1.Route{}, url.ConvertIngressURLToIngress(u, o.EnvSpecificInfo.GetName())), "", u.Spec.Host, false), "\t", u.Spec.Port, "\t", u.Spec.Secure, "\t", u.Spec.Kind) } - tabWriterURL.Flush() - if outOfSync { - log.Info("There are local changes. Please run 'odo push'.") + if u.Status.State != url.StateTypePushed { + outOfSync = true } } + tabWriterURL.Flush() + if outOfSync { + log.Info("There are local changes. Please run 'odo push'.") + } } return diff --git a/pkg/testingutil/devfile.go b/pkg/testingutil/devfile.go index 407393bfdb1..98e1bf2678c 100644 --- a/pkg/testingutil/devfile.go +++ b/pkg/testingutil/devfile.go @@ -343,3 +343,138 @@ func GetTestDevfileObjWithMultipleEndpoints(fs devfilefs.Filesystem) parser.Devf }, } } + +// DevfileObjWithInternalNoneEndpoints returns a devfile object with internal endpoints for testing +func DevfileObjWithInternalNoneEndpoints(fs devfilefs.Filesystem) parser.DevfileObj { + return parser.DevfileObj{ + Ctx: devfileCtx.FakeContext(fs, parser.OutputDevfileYamlPath), + Data: &TestDevfileData{ + Components: []v1.Component{ + { + Name: "runtime", + ComponentUnion: v1.ComponentUnion{ + Container: &v1.ContainerComponent{ + Endpoints: []v1.Endpoint{ + { + Name: "port-3030", + TargetPort: 3030, + Exposure: v1.NoneEndpointExposure, + }, + { + Name: "port-3000", + TargetPort: 3000, + }, + }, + }, + }, + }, + { + Name: "runtime-debug", + ComponentUnion: v1.ComponentUnion{ + Container: &v1.ContainerComponent{ + Endpoints: []v1.Endpoint{ + { + Name: "port-8080", + TargetPort: 8080, + Exposure: v1.InternalEndpointExposure, + }, + }, + }, + }, + }, + }, + }, + } +} + +// DevfileObjWithSecureEndpoints returns a devfile object with internal endpoints for testing +func DevfileObjWithSecureEndpoints(fs devfilefs.Filesystem) parser.DevfileObj { + return parser.DevfileObj{ + Ctx: devfileCtx.FakeContext(fs, parser.OutputDevfileYamlPath), + Data: &TestDevfileData{ + Components: []v1.Component{ + { + Name: "runtime", + ComponentUnion: v1.ComponentUnion{ + Container: &v1.ContainerComponent{ + Endpoints: []v1.Endpoint{ + { + Name: "port-3030", + TargetPort: 3030, + Protocol: v1.WSSEndpointProtocol, + }, + { + Name: "port-3000", + TargetPort: 3000, + Protocol: v1.HTTPSEndpointProtocol, + }, + }, + }, + }, + }, + { + Name: "runtime-debug", + ComponentUnion: v1.ComponentUnion{ + Container: &v1.ContainerComponent{ + Endpoints: []v1.Endpoint{ + { + Name: "port-8080", + TargetPort: 8080, + Secure: true, + }, + }, + }, + }, + }, + }, + }, + } +} + +// GetTestDevfileObjWithPath returns a devfile object for testing +func GetTestDevfileObjWithPath(fs devfilefs.Filesystem) parser.DevfileObj { + return parser.DevfileObj{ + Ctx: devfileCtx.FakeContext(fs, parser.OutputDevfileYamlPath), + Data: &TestDevfileData{ + Commands: []v1.Command{ + { + Id: "devbuild", + CommandUnion: v1.CommandUnion{ + Exec: &v1.ExecCommand{ + WorkingDir: "/projects/nodejs-starter", + }, + }, + }, + }, + Components: []v1.Component{ + { + Name: "runtime", + ComponentUnion: v1.ComponentUnion{ + Container: &v1.ContainerComponent{ + Container: v1.Container{ + Image: "quay.io/nodejs-12", + }, + Endpoints: []v1.Endpoint{ + { + Name: "port-3030", + TargetPort: 3000, + Path: "/test", + }, + }, + }, + }, + }, + { + Name: "loadbalancer", + ComponentUnion: v1.ComponentUnion{ + Container: &v1.ContainerComponent{ + Container: v1.Container{ + Image: "quay.io/nginx", + }, + }, + }, + }, + }, + }, + } +} diff --git a/pkg/url/kubernetes.go b/pkg/url/kubernetes.go new file mode 100644 index 00000000000..efe0934faa0 --- /dev/null +++ b/pkg/url/kubernetes.go @@ -0,0 +1,108 @@ +package url + +import ( + "fmt" + "sort" + + routev1 "github.com/openshift/api/route/v1" + componentlabels "github.com/openshift/odo/pkg/component/labels" + "github.com/openshift/odo/pkg/localConfigProvider" + "github.com/openshift/odo/pkg/occlient" + "github.com/pkg/errors" + "k8s.io/klog" +) + +type kubernetesClient struct { + generic + isRouteSupported bool + client occlient.Client +} + +// ListCluster lists both route and ingress based URLs from the cluster +func (k kubernetesClient) ListCluster() (URLList, error) { + labelSelector := fmt.Sprintf("%v=%v", componentlabels.ComponentLabel, k.componentName) + klog.V(4).Infof("Listing ingresses with label selector: %v", labelSelector) + ingresses, err := k.client.GetKubeClient().ListIngresses(labelSelector) + if err != nil { + return URLList{}, errors.Wrap(err, "unable to list ingress") + } + + var routes []routev1.Route + if k.isRouteSupported { + routes, err = k.client.ListRoutes(labelSelector) + if err != nil { + return URLList{}, errors.Wrap(err, "unable to list routes") + } + } + + var clusterURLs []URL + for _, i := range ingresses { + clusterURL := getMachineReadableFormatIngress(i) + clusterURLs = append(clusterURLs, clusterURL) + } + for _, r := range routes { + // ignore the routes created by ingresses + if r.OwnerReferences != nil && r.OwnerReferences[0].Kind == "Ingress" { + continue + } + clusterURL := getMachineReadableFormat(r) + clusterURLs = append(clusterURLs, clusterURL) + } + + return getMachineReadableFormatForList(clusterURLs), nil +} + +// List lists both route/ingress based URLs and local URLs with respective states +func (k kubernetesClient) List() (URLList, error) { + clusterURLMap := make(map[string]URL) + clusterURLs, err := k.ListCluster() + if err != nil { + return URLList{}, errors.Wrap(err, "unable to list routes") + } + + for _, url := range clusterURLs.Items { + clusterURLMap[getValidURLName(url.Name)] = url + } + + localMap := make(map[string]URL) + if k.localConfig != nil { + localURLS := k.localConfig.ListURLs() + for _, url := range localURLS { + if !k.isRouteSupported && url.Kind == localConfigProvider.ROUTE { + continue + } + localURL := ConvertEnvinfoURL(url, k.componentName) + // use the trimmed URL Name as the key since remote URLs' names are trimmed + trimmedURLName := getValidURLName(url.Name) + localMap[trimmedURLName] = localURL + } + } + + var urls sortableURLs + for URLName, clusterURL := range clusterURLMap { + _, found := localMap[URLName] + if found { + // URL is in both local env file and cluster + clusterURL.Status.State = StateTypePushed + urls = append(urls, clusterURL) + } else { + // URL is on the cluster but not in local env file + clusterURL.Status.State = StateTypeLocallyDeleted + urls = append(urls, clusterURL) + } + } + + for localName, localURL := range localMap { + _, remoteURLFound := clusterURLMap[localName] + if !remoteURLFound { + // URL is in the local env file but not pushed to cluster + localURL.Status.State = StateTypeNotPushed + urls = append(urls, localURL) + } + } + + // sort urls by name to get consistent output + sort.Sort(urls) + urlList := getMachineReadableFormatForList(urls) + return urlList, nil +} diff --git a/pkg/url/kubernetes_test.go b/pkg/url/kubernetes_test.go new file mode 100644 index 00000000000..5940880ca9e --- /dev/null +++ b/pkg/url/kubernetes_test.go @@ -0,0 +1,424 @@ +package url + +import ( + "reflect" + "testing" + + "github.com/golang/mock/gomock" + "github.com/kylelemons/godebug/pretty" + routev1 "github.com/openshift/api/route/v1" + "github.com/openshift/odo/pkg/kclient" + "github.com/openshift/odo/pkg/kclient/fake" + "github.com/openshift/odo/pkg/localConfigProvider" + "github.com/openshift/odo/pkg/occlient" + "github.com/openshift/odo/pkg/testingutil" + extensionsv1 "k8s.io/api/extensions/v1beta1" + v1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + ktesting "k8s.io/client-go/testing" +) + +func getFakeURL(name string, host string, port int, path string, protocol string, kind localConfigProvider.URLKind, urlState StateType) URL { + return URL{ + TypeMeta: v1.TypeMeta{ + Kind: "url", + APIVersion: "odo.dev/v1alpha1", + }, + ObjectMeta: v1.ObjectMeta{ + Name: name, + }, + Spec: URLSpec{ + Host: host, + Protocol: protocol, + Kind: kind, + Path: path, + Port: port, + }, + Status: URLStatus{ + State: urlState, + }, + } +} + +func Test_kubernetesClient_ListCluster(t *testing.T) { + componentName := "nodejs" + appName := "app" + ingress0 := fake.GetSingleIngress("testIngress0", componentName, appName) + ingress1 := fake.GetSingleIngress("testIngress1", componentName, appName) + + route0 := testingutil.GetSingleRoute("testRoute0", 8080, componentName, appName) + route1 := testingutil.GetSingleRoute("testRoute1", 8080, componentName, appName) + routeOwnedByIngress := testingutil.GetSingleRoute("testRoute1-ingress", 8080, componentName, appName) + routeOwnedByIngress.SetOwnerReferences([]v1.OwnerReference{ + { + Kind: "Ingress", + }, + }) + + type fields struct { + generic generic + isRouteSupported bool + } + tests := []struct { + name string + fields fields + returnedIngresses extensionsv1.IngressList + returnedRoutes routev1.RouteList + want URLList + wantErr bool + }{ + { + name: "case 1: list ingresses when route resource is not supported", + fields: fields{ + generic: generic{ + appName: "app", + componentName: componentName, + }, + isRouteSupported: false, + }, + returnedIngresses: extensionsv1.IngressList{ + Items: []extensionsv1.Ingress{ + *ingress0, + *ingress1, + }, + }, + want: getMachineReadableFormatForList([]URL{ + getMachineReadableFormatIngress(*ingress0), + getMachineReadableFormatIngress(*ingress1), + }), + }, + { + name: "case 2: only route based URLs are pushed", + fields: fields{ + generic: generic{ + appName: "app", + componentName: componentName, + }, + isRouteSupported: true, + }, + returnedRoutes: routev1.RouteList{ + Items: []routev1.Route{ + route0, + route1, + }, + }, + want: getMachineReadableFormatForList([]URL{ + getMachineReadableFormat(route0), + getMachineReadableFormat(route1)}, + ), + }, + { + name: "case 3: both route and ingress based URLs are pushed", + fields: fields{ + generic: generic{ + appName: "app", + componentName: componentName, + }, + isRouteSupported: true, + }, + returnedRoutes: routev1.RouteList{ + Items: []routev1.Route{ + route0, + route1, + }, + }, + returnedIngresses: extensionsv1.IngressList{ + Items: []extensionsv1.Ingress{ + *ingress0, + *ingress1, + }, + }, + want: getMachineReadableFormatForList([]URL{ + getMachineReadableFormatIngress(*ingress0), + getMachineReadableFormatIngress(*ingress1), + getMachineReadableFormat(route0), + getMachineReadableFormat(route1), + }), + }, + { + name: "case 4: no urls are pushed", + fields: fields{ + generic: generic{ + appName: "app", + componentName: componentName, + }, + isRouteSupported: true, + }, + want: getMachineReadableFormatForList(nil), + }, + { + name: "case 5: ignore the routes with ingress kind owners", + fields: fields{ + generic: generic{ + appName: "app", + componentName: componentName, + }, + isRouteSupported: true, + }, + returnedRoutes: routev1.RouteList{ + Items: []routev1.Route{ + route0, + routeOwnedByIngress, + }, + }, + want: getMachineReadableFormatForList([]URL{ + getMachineReadableFormat(route0)}, + ), + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + fkclient, fkclientset := kclient.FakeNew() + fkclient.Namespace = "default" + + fkclientset.Kubernetes.PrependReactor("list", "ingresses", func(action ktesting.Action) (bool, runtime.Object, error) { + return true, &tt.returnedIngresses, nil + }) + + fkocclient, fkocclientset := occlient.FakeNew() + fkocclient.SetKubeClient(fkclient) + + fkocclientset.RouteClientset.PrependReactor("list", "routes", func(action ktesting.Action) (bool, runtime.Object, error) { + return true, &tt.returnedRoutes, nil + }) + + k := kubernetesClient{ + generic: tt.fields.generic, + isRouteSupported: tt.fields.isRouteSupported, + client: *fkocclient, + } + got, err := k.ListCluster() + if (err != nil) != tt.wantErr { + t.Errorf("ListCluster() error = %v, wantErr %v", err, tt.wantErr) + return + } + if !reflect.DeepEqual(got, tt.want) { + t.Errorf("ListCluster() error: %v", pretty.Compare(got, tt.want)) + } + }) + } +} + +func Test_kubernetesClient_List(t *testing.T) { + componentName := "nodejs" + appName := "app" + + route0 := testingutil.GetSingleRoute("testRoute0", 8080, componentName, appName) + route1 := testingutil.GetSingleRoute("testRoute1", 8080, componentName, appName) + + ingress0 := fake.GetSingleIngress("testIngress0", componentName, appName) + + type fields struct { + generic generic + isRouteSupported bool + } + tests := []struct { + name string + fields fields + returnedRoutes routev1.RouteList + returnedIngress extensionsv1.IngressList + returnedLocalURLs []localConfigProvider.LocalURL + want URLList + wantErr bool + }{ + { + name: "case 1: two urls in local config and none pushed", + fields: fields{ + generic: generic{ + appName: appName, + componentName: componentName, + }, + isRouteSupported: true, + }, + returnedLocalURLs: []localConfigProvider.LocalURL{ + { + Name: "example-1", + Port: 8080, + Secure: false, + Host: "com", + Kind: localConfigProvider.INGRESS, + }, + { + Name: "example-2", + Port: 8080, + Secure: false, + Host: "com", + Kind: localConfigProvider.INGRESS, + }, + }, + want: getMachineReadableFormatForList([]URL{ + getFakeURL("example-1", "example-1.com", 8080, "", "", localConfigProvider.INGRESS, StateTypeNotPushed), + getFakeURL("example-2", "example-2.com", 8080, "", "", localConfigProvider.INGRESS, StateTypeNotPushed)}), + }, + { + name: "case 2: two urls pushed but are deleted locally", + fields: fields{ + generic: generic{ + appName: appName, + componentName: componentName, + }, + isRouteSupported: true, + }, + returnedRoutes: routev1.RouteList{ + Items: []routev1.Route{ + route0, + route1, + }, + }, + returnedLocalURLs: []localConfigProvider.LocalURL{}, + want: getMachineReadableFormatForList([]URL{ + getFakeURL("testRoute0", "", 8080, "/", "http", localConfigProvider.ROUTE, StateTypeLocallyDeleted), + getFakeURL("testRoute1", "", 8080, "/", "http", localConfigProvider.ROUTE, StateTypeLocallyDeleted)}), + }, + { + name: "case 3: two urls which are pushed", + fields: fields{ + generic: generic{ + appName: appName, + componentName: componentName, + }, + isRouteSupported: true, + }, + returnedRoutes: routev1.RouteList{ + Items: []routev1.Route{ + route0, + }, + }, + returnedIngress: extensionsv1.IngressList{ + Items: []extensionsv1.Ingress{ + *ingress0, + }, + }, + returnedLocalURLs: []localConfigProvider.LocalURL{ + { + Name: "testRoute0", + Port: 8080, + Secure: false, + Path: "/", + Protocol: "http", + Kind: localConfigProvider.ROUTE, + }, + { + Name: "testIngress0", + Port: 8080, + Secure: false, + Host: "com", + Kind: localConfigProvider.INGRESS, + }, + }, + want: getMachineReadableFormatForList([]URL{ + getFakeURL("testIngress0", "testIngress0.com", 8080, "/", "", localConfigProvider.INGRESS, StateTypePushed), + getFakeURL("testRoute0", "", 8080, "/", "http", localConfigProvider.ROUTE, StateTypePushed), + }), + }, + { + name: "case 4: three URLs with mixed states", + fields: fields{ + generic: generic{ + appName: appName, + componentName: componentName, + }, + isRouteSupported: true, + }, + returnedRoutes: routev1.RouteList{ + Items: []routev1.Route{ + route1, + }, + }, + returnedIngress: extensionsv1.IngressList{ + Items: []extensionsv1.Ingress{ + *ingress0, + }, + }, + returnedLocalURLs: []localConfigProvider.LocalURL{ + { + Name: "testRoute0", + Port: 8080, + Secure: false, + Path: "/", + Kind: localConfigProvider.ROUTE, + }, + { + Name: "testIngress0", + Port: 8080, + Secure: false, + Host: "com", + Kind: localConfigProvider.INGRESS, + }, + }, + want: getMachineReadableFormatForList([]URL{ + getFakeURL("testIngress0", "testIngress0.com", 8080, "/", "", localConfigProvider.INGRESS, StateTypePushed), + getFakeURL("testRoute0", "", 8080, "/", "", localConfigProvider.ROUTE, StateTypeNotPushed), + getFakeURL("testRoute1", "", 8080, "/", "http", localConfigProvider.ROUTE, StateTypeLocallyDeleted), + }), + }, + { + name: "case 5: ignore routes when route resources are not supported", + fields: fields{ + generic: generic{ + appName: appName, + componentName: componentName, + }, + isRouteSupported: false, + }, + returnedLocalURLs: []localConfigProvider.LocalURL{ + { + Name: "testRoute0", + Port: 8080, + Secure: false, + Host: "com", + Kind: localConfigProvider.ROUTE, + }, + { + Name: "testIngress0", + Port: 8080, + Secure: false, + Host: "com", + Path: "/", + Kind: localConfigProvider.INGRESS, + }, + }, + want: getMachineReadableFormatForList([]URL{ + getFakeURL("testIngress0", "testIngress0.com", 8080, "/", "", localConfigProvider.INGRESS, StateTypeNotPushed), + }), + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + mockLocalConfig := localConfigProvider.NewMockLocalConfigProvider(ctrl) + mockLocalConfig.EXPECT().ListURLs().Return(tt.returnedLocalURLs) + + fkclient, fkclientset := kclient.FakeNew() + fkclient.Namespace = "default" + + fkclientset.Kubernetes.PrependReactor("list", "ingresses", func(action ktesting.Action) (bool, runtime.Object, error) { + return true, &tt.returnedIngress, nil + }) + + fkocclient, fkocclientset := occlient.FakeNew() + fkocclient.SetKubeClient(fkclient) + + fkocclientset.RouteClientset.PrependReactor("list", "routes", func(action ktesting.Action) (bool, runtime.Object, error) { + return true, &tt.returnedRoutes, nil + }) + + tt.fields.generic.localConfig = mockLocalConfig + k := kubernetesClient{ + generic: tt.fields.generic, + isRouteSupported: tt.fields.isRouteSupported, + client: *fkocclient, + } + got, err := k.List() + if (err != nil) != tt.wantErr { + t.Errorf("List() error = %v, wantErr %v", err, tt.wantErr) + return + } + if !reflect.DeepEqual(got, tt.want) { + t.Errorf("List() error: %v", pretty.Compare(got, tt.want)) + } + }) + } +} diff --git a/pkg/url/mock_Client.go b/pkg/url/mock_Client.go new file mode 100644 index 00000000000..5e835c1dacb --- /dev/null +++ b/pkg/url/mock_Client.go @@ -0,0 +1,63 @@ +// Code generated by MockGen. DO NOT EDIT. +// Source: url.go + +// Package url is a generated GoMock package. +package url + +import ( + gomock "github.com/golang/mock/gomock" + reflect "reflect" +) + +// MockClient is a mock of Client interface +type MockClient struct { + ctrl *gomock.Controller + recorder *MockClientMockRecorder +} + +// MockClientMockRecorder is the mock recorder for MockClient +type MockClientMockRecorder struct { + mock *MockClient +} + +// NewMockClient creates a new mock instance +func NewMockClient(ctrl *gomock.Controller) *MockClient { + mock := &MockClient{ctrl: ctrl} + mock.recorder = &MockClientMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use +func (m *MockClient) EXPECT() *MockClientMockRecorder { + return m.recorder +} + +// ListCluster mocks base method +func (m *MockClient) ListCluster() (URLList, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "ListCluster") + ret0, _ := ret[0].(URLList) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// ListCluster indicates an expected call of ListCluster +func (mr *MockClientMockRecorder) ListCluster() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ListCluster", reflect.TypeOf((*MockClient)(nil).ListCluster)) +} + +// List mocks base method +func (m *MockClient) List() (URLList, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "List") + ret0, _ := ret[0].(URLList) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// List indicates an expected call of List +func (mr *MockClientMockRecorder) List() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "List", reflect.TypeOf((*MockClient)(nil).List)) +} diff --git a/pkg/url/s2i.go b/pkg/url/s2i.go new file mode 100644 index 00000000000..cd3dad405f5 --- /dev/null +++ b/pkg/url/s2i.go @@ -0,0 +1,89 @@ +package url + +import ( + "fmt" + + applabels "github.com/openshift/odo/pkg/application/labels" + componentlabels "github.com/openshift/odo/pkg/component/labels" + "github.com/openshift/odo/pkg/occlient" + "github.com/pkg/errors" + "k8s.io/klog" +) + +type s2iClient struct { + generic + client occlient.Client +} + +func (s s2iClient) ListCluster() (URLList, error) { + labelSelector := fmt.Sprintf("%v=%v", applabels.ApplicationLabel, s.localConfig.GetApplication()) + + if s.localConfig.GetName() != "" { + labelSelector = labelSelector + fmt.Sprintf(",%v=%v", componentlabels.ComponentLabel, s.localConfig.GetName()) + } + + klog.V(4).Infof("Listing routes with label selector: %v", labelSelector) + routes, err := s.client.ListRoutes(labelSelector) + + if err != nil { + return URLList{}, errors.Wrap(err, "unable to list route names") + } + + var urls []URL + for _, r := range routes { + if r.OwnerReferences != nil && r.OwnerReferences[0].Kind == "Ingress" { + continue + } + a := getMachineReadableFormat(r) + urls = append(urls, a) + } + + urlList := getMachineReadableFormatForList(urls) + return urlList, nil +} + +func (s s2iClient) List() (URLList, error) { + var urls []URL + + clusterUrls, err := s.ListCluster() + if err != nil { + return URLList{}, errors.Wrap(err, "unable to list route names") + } + + for _, clusterURL := range clusterUrls.Items { + var found bool = false + for _, configURL := range s.localConfig.ListURLs() { + localURL := ConvertConfigURL(configURL) + if localURL.Name == clusterURL.Name { + // URL is in both local config and cluster + clusterURL.Status.State = StateTypePushed + urls = append(urls, clusterURL) + found = true + } + } + + if !found { + // URL is on the cluster but not in local config + clusterURL.Status.State = StateTypeLocallyDeleted + urls = append(urls, clusterURL) + } + } + + for _, configURL := range s.localConfig.ListURLs() { + localURL := ConvertConfigURL(configURL) + var found = false + for _, clusterURL := range clusterUrls.Items { + if localURL.Name == clusterURL.Name { + found = true + } + } + if !found { + // URL is in the local config but not on the cluster + localURL.Status.State = StateTypeNotPushed + urls = append(urls, localURL) + } + } + + urlList := getMachineReadableFormatForList(urls) + return urlList, nil +} diff --git a/pkg/url/status.go b/pkg/url/status.go index 937e08e87c2..424e26282a7 100644 --- a/pkg/url/status.go +++ b/pkg/url/status.go @@ -8,7 +8,6 @@ import ( "strconv" "time" - devfilev1 "github.com/devfile/api/pkg/apis/workspaces/v1alpha2" routev1 "github.com/openshift/api/route/v1" "github.com/openshift/odo/pkg/envinfo" "github.com/openshift/odo/pkg/kclient" @@ -26,7 +25,7 @@ const ( ) // StartURLHttpRequestStatusWatchForK8S begins testing URLs for responses, outputting the result to console -func StartURLHttpRequestStatusWatchForK8S(occlient *occlient.Client, client *kclient.Client, envInfo *envinfo.EnvSpecificInfo, loggingClient machineoutput.MachineEventLoggingClient, containerComponents []devfilev1.Component) { +func StartURLHttpRequestStatusWatchForK8S(occlient *occlient.Client, client *kclient.Client, localConfigProvider *localConfigProvider.LocalConfigProvider, loggingClient machineoutput.MachineEventLoggingClient) { // This is a non-blocking function so that other status watchers may start as needed go func() { @@ -35,7 +34,7 @@ func StartURLHttpRequestStatusWatchForK8S(occlient *occlient.Client, client *kcl for { var err error - urlList, err = getURLsForKubernetes(occlient, client, envInfo, true, containerComponents) + urlList, err = getURLsForKubernetes(occlient, client, *localConfigProvider, true) if err == nil { // Success! @@ -95,8 +94,8 @@ func startURLTester(urlsToTest [][]statusURL, loggingClient machineoutput.Machin } } -func getURLsForKubernetes(oclient *occlient.Client, client *kclient.Client, envInfo *envinfo.EnvSpecificInfo, ignoreUnpushed bool, containerComponents []devfilev1.Component) ([]statusURL, error) { - componentName := envInfo.GetName() +func getURLsForKubernetes(oclient *occlient.Client, client *kclient.Client, localConfig localConfigProvider.LocalConfigProvider, ignoreUnpushed bool) ([]statusURL, error) { + componentName := localConfig.GetName() routesSupported := false @@ -106,12 +105,18 @@ func getURLsForKubernetes(oclient *occlient.Client, client *kclient.Client, envI if routesSupported, err = oclient.IsRouteSupported(); err != nil { // Fallback to Kubernetes client on error routesSupported = false - oclient = nil } + } else { + return []statusURL{}, fmt.Errorf("the client is nil") } - urls, err := ListIngressAndRoute(oclient, envInfo, containerComponents, componentName, routesSupported) + urlClient := NewClient(ClientOptions{ + LocalConfigProvider: localConfig, + OCClient: *oclient, + IsRouteSupported: routesSupported, + }) + urls, err := urlClient.List() if err != nil { return nil, err diff --git a/pkg/url/status_test.go b/pkg/url/status_test.go index fe0d558151a..fcb7fd36ec7 100644 --- a/pkg/url/status_test.go +++ b/pkg/url/status_test.go @@ -1,17 +1,13 @@ package url import ( - "github.com/openshift/odo/pkg/localConfigProvider" "reflect" "testing" - devfilev1 "github.com/devfile/api/pkg/apis/workspaces/v1alpha2" - "github.com/devfile/library/pkg/devfile/parser" - devfileCtx "github.com/devfile/library/pkg/devfile/parser/context" - "github.com/devfile/library/pkg/testingutil/filesystem" + "github.com/golang/mock/gomock" routev1 "github.com/openshift/api/route/v1" - "github.com/openshift/odo/pkg/envinfo" "github.com/openshift/odo/pkg/kclient" + "github.com/openshift/odo/pkg/localConfigProvider" "github.com/openshift/odo/pkg/occlient" "github.com/openshift/odo/pkg/testingutil" @@ -49,8 +45,6 @@ var fakeDiscoveryWithProject = &fakeDiscovery{ } func TestGetURLsForKubernetes(t *testing.T) { - fs := filesystem.NewFakeFs() - componentName := "my-component" testURL1 := localConfigProvider.LocalURL{Name: "example-1", Port: 9090, Host: "com", Kind: "ingress", Secure: true} @@ -78,6 +72,9 @@ func TestGetURLsForKubernetes(t *testing.T) { secure: testURL1.Secure, url: "https://example-1.com", }, + routeList: &routev1.RouteList{ + Items: []routev1.Route{}, + }, }, { name: "2) Cluster with https URL defined in env info", @@ -92,6 +89,9 @@ func TestGetURLsForKubernetes(t *testing.T) { secure: testURL2.Secure, url: "http://example-2.com", }, + routeList: &routev1.RouteList{ + Items: []routev1.Route{}, + }, }, { name: "3) Cluster with route defined in env info", @@ -140,6 +140,9 @@ func TestGetURLsForKubernetes(t *testing.T) { kclient_fake.GetIngressListWithMultiple(componentName, "app").Items[0], }, }, + routeList: &routev1.RouteList{ + Items: []routev1.Route{}, + }, expectedStatusURL: statusURL{ name: "example-0", kind: "ingress", @@ -152,6 +155,14 @@ func TestGetURLsForKubernetes(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + mockLocalConfig := localConfigProvider.NewMockLocalConfigProvider(ctrl) + mockLocalConfig.EXPECT().GetName().Return(componentName).AnyTimes() + mockLocalConfig.EXPECT().GetApplication().Return("") + mockLocalConfig.EXPECT().ListURLs().Return(tt.envURLs) + // Initialising the fakeclient fkclient, fkclientset := kclient.FakeNew() fkclient.Namespace = "default" @@ -170,30 +181,10 @@ func TestGetURLsForKubernetes(t *testing.T) { // Return the test's route list when requested fakeoclientSet.RouteClientset.PrependReactor("list", "routes", func(action ktesting.Action) (bool, runtime.Object, error) { - return tt.routeList != nil, tt.routeList, nil + return true, tt.routeList, nil }) - esi := &envinfo.EnvSpecificInfo{} - if err := esi.SetComponentSettings(envinfo.ComponentSettings{Name: componentName}); err != nil { - t.Logf("ignoring error, since no physical file to write: %v", err) - } - - for _, url := range tt.envURLs { - err := esi.SetConfiguration("url", url) - if err != nil { - t.Logf("ignoring error, since no physical file to write: %v", err) - } - } - - devObj := parser.DevfileObj{ - Ctx: devfileCtx.FakeContext(fs, parser.OutputDevfileYamlPath), - Data: &testingutil.TestDevfileData{ - Components: []devfilev1.Component{}, - }, - } - containerComponents := devObj.Data.GetDevfileContainerComponents() - - statusUrls, err := getURLsForKubernetes(fkoclient, fkclient, esi, false, containerComponents) + statusUrls, err := getURLsForKubernetes(fkoclient, fkclient, mockLocalConfig, false) if err != nil { t.Fatalf("Error occurred: %v", err) @@ -206,8 +197,6 @@ func TestGetURLsForKubernetes(t *testing.T) { if !reflect.DeepEqual(tt.expectedStatusURL, statusUrls[0]) { t.Fatalf("Mismatching status URL - expected: %v, actual: %v", tt.expectedStatusURL, statusUrls[0]) } - }) } - } diff --git a/pkg/url/url.go b/pkg/url/url.go index dc08b9b1c68..7ec8f30ff42 100644 --- a/pkg/url/url.go +++ b/pkg/url/url.go @@ -2,10 +2,8 @@ package url import ( "fmt" - "github.com/openshift/odo/pkg/localConfigProvider" "net" "reflect" - "sort" "strconv" "strings" @@ -15,7 +13,6 @@ import ( "github.com/openshift/odo/pkg/envinfo" "github.com/openshift/odo/pkg/log" - devfilev1 "github.com/devfile/api/pkg/apis/workspaces/v1alpha2" types "github.com/docker/docker/api/types" routev1 "github.com/openshift/api/route/v1" applabels "github.com/openshift/odo/pkg/application/labels" @@ -25,6 +22,7 @@ import ( dockerutils "github.com/openshift/odo/pkg/devfile/adapters/docker/utils" "github.com/openshift/odo/pkg/kclient" "github.com/openshift/odo/pkg/lclient" + "github.com/openshift/odo/pkg/localConfigProvider" "github.com/openshift/odo/pkg/occlient" urlLabels "github.com/openshift/odo/pkg/url/labels" "github.com/openshift/odo/pkg/util" @@ -51,106 +49,6 @@ func (urls URLList) Get(urlName string) URL { } -// GetIngressOrRoute returns ingress/route spec for given URL name -func GetIngressOrRoute(client *occlient.Client, kClient *kclient.Client, envSpecificInfo *envinfo.EnvSpecificInfo, urlName string, containerComponents []devfilev1.Component, componentName string, routeSupported bool) (URL, error) { - remoteExist := true - var ingress *iextensionsv1.Ingress - var route *routev1.Route - var getRouteErr error - - // route/ingress name is defined as - - // to avoid error due to duplicate ingress name defined in different devfile components - trimmedURLName := getValidURLName(urlName) - remoteURLName := fmt.Sprintf("%s-%s", trimmedURLName, componentName) - // Check whether remote already created the ingress - ingress, getIngressErr := kClient.GetIngress(remoteURLName) - if kerrors.IsNotFound(getIngressErr) && routeSupported { - // Check whether remote already created the route - route, getRouteErr = client.GetRoute(remoteURLName) - } - if kerrors.IsNotFound(getIngressErr) && (!routeSupported || kerrors.IsNotFound(getRouteErr)) { - remoteExist = false - } else if (getIngressErr != nil && !kerrors.IsNotFound(getIngressErr)) || (getRouteErr != nil && !kerrors.IsNotFound(getRouteErr)) { - if getIngressErr != nil { - return URL{}, errors.Wrap(getIngressErr, "unable to get ingress") - } - return URL{}, errors.Wrap(getRouteErr, "unable to get route") - } - - envinfoURLs := envSpecificInfo.ListURLs() - envURLMap := make(map[string]localConfigProvider.LocalURL) - for _, url := range envinfoURLs { - envURLMap[url.Name] = url - } - - for _, comp := range containerComponents { - for _, localEndpoint := range comp.Container.Endpoints { - if localEndpoint.Name != urlName { - continue - } - - if localEndpoint.Exposure == devfilev1.NoneEndpointExposure || localEndpoint.Exposure == devfilev1.InternalEndpointExposure { - return URL{}, errors.New(fmt.Sprintf("the url %v is defined in devfile, but is not exposed", urlName)) - } - var devfileURL localConfigProvider.LocalURL - if envinfoURL, exist := envURLMap[localEndpoint.Name]; exist { - if envinfoURL.Kind == localConfigProvider.DOCKER { - return URL{}, errors.New(fmt.Sprintf("the url %v is defined with type of Docker", urlName)) - } - if !routeSupported && envinfoURL.Kind == localConfigProvider.ROUTE { - return URL{}, errors.New(fmt.Sprintf("the url %v is defined with type of Route, but Route is not support in current cluster", urlName)) - } - devfileURL = envinfoURL - devfileURL.Port = int(localEndpoint.TargetPort) - devfileURL.Secure = localEndpoint.Secure - } - if reflect.DeepEqual(devfileURL, localConfigProvider.LocalURL{}) { - // Devfile endpoint by default should create a route if no host information is provided in env.yaml - // If it is not openshift cluster, should ignore the endpoint entry when executing url describe/list - if !routeSupported { - break - } - devfileURL.Name = urlName - devfileURL.Port = int(localEndpoint.TargetPort) - devfileURL.Secure = localEndpoint.Secure - devfileURL.Kind = localConfigProvider.ROUTE - } - localURL := ConvertEnvinfoURL(devfileURL, componentName) - if remoteExist { - if ingress != nil && ingress.Spec.Rules != nil { - // Remote exist, but not in local, so it's deleted status - clusterURL := getMachineReadableFormatIngress(*ingress) - clusterURL.Status.State = StateTypePushed - return clusterURL, nil - } else if route != nil { - clusterURL := getMachineReadableFormat(*route) - clusterURL.Status.State = StateTypePushed - return clusterURL, nil - } - } else { - localURL.Status.State = StateTypeNotPushed - } - return localURL, nil - } - } - - if remoteExist { - if ingress != nil && ingress.Spec.Rules != nil { - // Remote exist, but not in local, so it's deleted status - clusterURL := getMachineReadableFormatIngress(*ingress) - clusterURL.Status.State = StateTypeLocallyDeleted - return clusterURL, nil - } else if route != nil { - clusterURL := getMachineReadableFormat(*route) - clusterURL.Status.State = StateTypeLocallyDeleted - return clusterURL, nil - } - } - - // can't find the URL in local and remote - return URL{}, errors.New(fmt.Sprintf("the url %v does not exist", urlName)) -} - // Delete deletes a URL func Delete(client *occlient.Client, kClient *kclient.Client, urlName string, applicationName string, urlType localConfigProvider.URLKind, isS2i bool) error { if urlType == localConfigProvider.INGRESS { @@ -371,65 +269,6 @@ func ListPushedIngress(client *kclient.Client, componentName string) (URLList, e return urlList, nil } -// List returns all URLs for given component. -// If componentName is empty string, it lists all url in a given application. -func List(client *occlient.Client, localConfig *config.LocalConfigInfo, componentName string, applicationName string) (URLList, error) { - - labelSelector := fmt.Sprintf("%v=%v", applabels.ApplicationLabel, applicationName) - - if componentName != "" { - labelSelector = labelSelector + fmt.Sprintf(",%v=%v", componentlabels.ComponentLabel, componentName) - } - - routes, err := client.ListRoutes(labelSelector) - if err != nil { - return URLList{}, errors.Wrap(err, "unable to list route names") - } - - localConfigURLs := localConfig.ListURLs() - - var urls []URL - - for _, r := range routes { - clusterURL := getMachineReadableFormat(r) - var found bool = false - for _, configURL := range localConfigURLs { - localURL := ConvertConfigURL(configURL) - if localURL.Name == clusterURL.Name { - // URL is in both local config and cluster - clusterURL.Status.State = StateTypePushed - urls = append(urls, clusterURL) - found = true - } - } - - if !found { - // URL is on the cluster but not in local config - clusterURL.Status.State = StateTypeLocallyDeleted - urls = append(urls, clusterURL) - } - } - - for _, configURL := range localConfigURLs { - localURL := ConvertConfigURL(configURL) - var found = false - for _, r := range routes { - clusterURL := getMachineReadableFormat(r) - if localURL.Name == clusterURL.Name { - found = true - } - } - if !found { - // URL is in the local config but not on the cluster - localURL.Status.State = StateTypeNotPushed - urls = append(urls, localURL) - } - } - - urlList := getMachineReadableFormatForList(urls) - return urlList, nil -} - type sortableURLs []URL func (s sortableURLs) Len() int { @@ -444,119 +283,6 @@ func (s sortableURLs) Swap(i, j int) { s[i], s[j] = s[j], s[i] } -// ListIngressAndRoute returns all Ingress and Route for given component. -func ListIngressAndRoute(oclient *occlient.Client, configProvider localConfigProvider.LocalConfigProvider, containerComponents []devfilev1.Component, componentName string, routeSupported bool) (URLList, error) { - labelSelector := fmt.Sprintf("%v=%v", componentlabels.ComponentLabel, componentName) - klog.V(4).Infof("Listing ingresses with label selector: %v", labelSelector) - ingresses, err := oclient.GetKubeClient().ListIngresses(labelSelector) - if err != nil { - return URLList{}, errors.Wrap(err, "unable to list ingress") - } - routes := []routev1.Route{} - if routeSupported { - routes, err = oclient.ListRoutes(labelSelector) - if err != nil { - return URLList{}, errors.Wrap(err, "unable to list routes") - } - } - - envURLMap := make(map[string]localConfigProvider.LocalURL) - if configProvider != nil { - localEnvinfoURLs := configProvider.ListURLs() - for _, url := range localEnvinfoURLs { - if url.Kind == localConfigProvider.DOCKER { - continue - } - if !routeSupported && url.Kind == localConfigProvider.ROUTE { - continue - } - envURLMap[url.Name] = url - } - } - - var urls sortableURLs - - clusterURLMap := make(map[string]URL) - localMap := make(map[string]URL) - for _, i := range ingresses { - clusterURL := getMachineReadableFormatIngress(i) - clusterURLMap[clusterURL.Name] = clusterURL - } - for _, r := range routes { - if r.OwnerReferences != nil && r.OwnerReferences[0].Kind == "Ingress" { - continue - } - clusterURL := getMachineReadableFormat(r) - clusterURLMap[clusterURL.Name] = clusterURL - } - - if len(containerComponents) > 0 { - for _, comp := range containerComponents { - for _, localEndpoint := range comp.Container.Endpoints { - // only exposed endpoint will be shown as a URL in `odo url list` - if localEndpoint.Exposure == devfilev1.NoneEndpointExposure || localEndpoint.Exposure == devfilev1.InternalEndpointExposure { - continue - } - var devfileURL localConfigProvider.LocalURL - if envinfoURL, exist := envURLMap[localEndpoint.Name]; exist { - devfileURL = envinfoURL - devfileURL.Port = int(localEndpoint.TargetPort) - devfileURL.Secure = localEndpoint.Secure - } - if reflect.DeepEqual(devfileURL, localConfigProvider.LocalURL{}) { - // Devfile endpoint by default should create a route if no host information is provided in env.yaml - // If it is not openshift cluster, should ignore the endpoint entry when executing url describe/list - if !routeSupported { - continue - } - devfileURL.Name = localEndpoint.Name - devfileURL.Port = int(localEndpoint.TargetPort) - devfileURL.Secure = localEndpoint.Secure - devfileURL.Kind = localConfigProvider.ROUTE - } - localURL := ConvertEnvinfoURL(devfileURL, componentName) - // use the trimmed URL Name as the key since remote URLs' names are trimmed - trimmedURLName := getValidURLName(localURL.Name) - localMap[trimmedURLName] = localURL - } - } - } else { - for _, url := range envURLMap { - localURL := ConvertEnvinfoURL(url, componentName) - // use the trimmed URL Name as the key since remote URLs' names are trimmed - trimmedURLName := getValidURLName(localURL.Name) - localMap[trimmedURLName] = localURL - } - } - - for URLName, clusterURL := range clusterURLMap { - _, found := localMap[URLName] - if found { - // URL is in both local env file and cluster - clusterURL.Status.State = StateTypePushed - urls = append(urls, clusterURL) - } else { - // URL is on the cluster but not in local env file - clusterURL.Status.State = StateTypeLocallyDeleted - urls = append(urls, clusterURL) - } - } - - for localName, localURL := range localMap { - _, remoteURLFound := clusterURLMap[localName] - if !remoteURLFound { - // URL is in the local env file but not pushed to cluster - localURL.Status.State = StateTypeNotPushed - urls = append(urls, localURL) - } - } - - // sort urls by name to get consistent output - sort.Sort(urls) - urlList := getMachineReadableFormatForList(urls) - return urlList, nil -} - // ListDockerURL returns all Docker URLs for given component. func ListDockerURL(client *lclient.Client, componentName string, envSpecificInfo *envinfo.EnvSpecificInfo) (URLList, error) { containers, err := dockerutils.GetComponentContainers(*client, componentName) @@ -687,9 +413,13 @@ func ConvertEnvinfoURL(envinfoURL localConfigProvider.LocalURL, serviceName stri Name: envinfoURL.Name, }, Spec: URLSpec{ - Port: envinfoURL.Port, - Secure: envinfoURL.Secure, - Kind: kind, + Host: envinfoURL.Host, + Protocol: envinfoURL.Protocol, + Port: envinfoURL.Port, + Secure: envinfoURL.Secure, + Kind: kind, + TLSSecret: envinfoURL.TLSSecret, + Path: envinfoURL.Path, }, } if kind == localConfigProvider.INGRESS { @@ -703,6 +433,28 @@ func ConvertEnvinfoURL(envinfoURL localConfigProvider.LocalURL, serviceName stri return url } +// ConvertLocalURL converts localConfigProvider.LocalURL to URL +func ConvertLocalURL(localURL localConfigProvider.LocalURL) URL { + return URL{ + TypeMeta: metav1.TypeMeta{ + Kind: "url", + APIVersion: apiVersion, + }, + ObjectMeta: metav1.ObjectMeta{ + Name: localURL.Name, + }, + Spec: URLSpec{ + Host: localURL.Host, + Protocol: localURL.Protocol, + Port: localURL.Port, + Secure: localURL.Secure, + Kind: localURL.Kind, + TLSSecret: localURL.TLSSecret, + Path: localURL.Path, + }, + } +} + // GetURLString returns a string representation of given url func GetURLString(protocol, URL, ingressDomain string, isS2I bool) string { if protocol == "" && URL == "" && ingressDomain == "" { @@ -844,126 +596,41 @@ func getMachineReadableFormatDocker(internalPort int, externalPort int, hostIP s } type PushParameters struct { - ComponentName string - ApplicationName string - ConfigURLs []localConfigProvider.LocalURL - EnvURLS []localConfigProvider.LocalURL - IsRouteSupported bool - IsExperimentalModeEnabled bool - ContainerComponents []devfilev1.Component - IsS2I bool + ComponentName string + ApplicationName string + LocalConfig localConfigProvider.LocalConfigProvider + URLClient Client + IsRouteSupported bool + IsS2I bool } // Push creates and deletes the required URLs func Push(client *occlient.Client, kClient *kclient.Client, parameters PushParameters) error { urlLOCAL := make(map[string]URL) - // in case the component is a s2i one - // kClient will be nil - if !parameters.IsS2I && kClient != nil { - envURLMap := make(map[string]localConfigProvider.LocalURL) - for _, url := range parameters.EnvURLS { - if url.Kind == localConfigProvider.DOCKER { - continue - } - envURLMap[url.Name] = url - } - for _, comp := range parameters.ContainerComponents { - for _, endpoint := range comp.Container.Endpoints { - // skip URL creation if the URL is not publicly exposed - if endpoint.Exposure == devfilev1.NoneEndpointExposure || endpoint.Exposure == devfilev1.InternalEndpointExposure { - continue - } - secure := false - if endpoint.Secure || endpoint.Protocol == "https" || endpoint.Protocol == "wss" { - secure = true - } - path := "/" - if endpoint.Path != "" { - path = endpoint.Path - } - name := getValidURLName(endpoint.Name) - existInEnv := false - if url, exist := envURLMap[endpoint.Name]; exist { - existInEnv = true - urlLOCAL[name] = URL{ - Spec: URLSpec{ - Host: url.Host, - Port: int(endpoint.TargetPort), - Secure: secure, - TLSSecret: url.TLSSecret, - Kind: url.Kind, - Path: path, - }, - } - } - if !existInEnv { - if !parameters.IsRouteSupported { - // display warning since Host info is missing - log.Warningf("Unable to create ingress, missing host information for Endpoint %v, please check instructions on URL creation (refer `odo url create --help`)\n", endpoint.Name) - } else { - urlLOCAL[name] = URL{ - Spec: URLSpec{ - Port: int(endpoint.TargetPort), - Secure: secure, - Kind: localConfigProvider.ROUTE, - Path: path, - }, - } - } - } - } - } - } else { - urls := parameters.ConfigURLs - for _, url := range urls { - urlLOCAL[url.Name] = URL{ - Spec: URLSpec{ - Port: url.Port, - Secure: url.Secure, - Kind: localConfigProvider.ROUTE, - Path: "/", - }, - } + // get the local URLs + for _, url := range parameters.LocalConfig.ListURLs() { + if !parameters.IsRouteSupported && url.Kind == localConfigProvider.ROUTE { + // display warning since Host info is missing + log.Warningf("Unable to create ingress, missing host information for Endpoint %v, please check instructions on URL creation (refer `odo url create --help`)\n", url.Name) + continue } + urlName := getValidURLName(url.Name) + urlLOCAL[urlName] = ConvertLocalURL(url) } log.Info("\nApplying URL changes") urlCLUSTER := make(map[string]URL) - if !parameters.IsS2I && kClient != nil { - urlList, err := ListPushedIngress(kClient, parameters.ComponentName) - if err != nil { - return err - } - for _, url := range urlList.Items { - urlCLUSTER[url.Name] = URL{ - Spec: URLSpec{ - Host: url.Spec.Host, - Port: url.Spec.Port, - Kind: localConfigProvider.INGRESS, - Secure: url.Spec.Secure, - Path: url.Spec.Path, - }, - } - } + + // get the URLs on the cluster + urlList, err := parameters.URLClient.ListCluster() + if err != nil { + return err } - if parameters.IsRouteSupported { - urlPushedRoutes, err := ListPushed(client, parameters.ComponentName, parameters.ApplicationName) - if err != nil { - return err - } - for _, urlRoute := range urlPushedRoutes.Items { - urlCLUSTER[urlRoute.Name] = URL{ - Spec: URLSpec{ - Port: urlRoute.Spec.Port, - Kind: localConfigProvider.ROUTE, - Secure: urlRoute.Spec.Secure, - Path: urlRoute.Spec.Path, - }, - } - } + for _, url := range urlList.Items { + urlCLUSTER[url.Name] = url } urlChange := false @@ -978,6 +645,12 @@ func Push(client *occlient.Client, kClient *kclient.Client, parameters PushParam // is the combination of name and host of the url if val.Spec.Kind == localConfigProvider.INGRESS { val.Spec.Host = fmt.Sprintf("%v.%v", urlName, val.Spec.Host) + } else if val.Spec.Kind == localConfigProvider.ROUTE { + if val.Spec.Secure { + val.Spec.Protocol = "https" + } else { + val.Spec.Protocol = "http" + } } if !reflect.DeepEqual(val.Spec, urlSpec.Spec) { configMismatch = true @@ -1047,3 +720,41 @@ func getValidURLName(name string) string { trimmedName = util.TruncateString(trimmedName, 15) return trimmedName } + +type ClientOptions struct { + OCClient occlient.Client + IsRouteSupported bool + LocalConfigProvider localConfigProvider.LocalConfigProvider +} + +type Client interface { + ListCluster() (URLList, error) + List() (URLList, error) +} + +func NewClient(options ClientOptions) Client { + genericInfo := generic{ + appName: options.LocalConfigProvider.GetApplication(), + componentName: options.LocalConfigProvider.GetName(), + localConfig: options.LocalConfigProvider, + } + + if _, ok := options.LocalConfigProvider.(*config.LocalConfigInfo); ok { + return s2iClient{ + generic: genericInfo, + client: options.OCClient, + } + } else { + return kubernetesClient{ + generic: genericInfo, + isRouteSupported: options.IsRouteSupported, + client: options.OCClient, + } + } +} + +type generic struct { + appName string + componentName string + localConfig localConfigProvider.LocalConfigProvider +} diff --git a/pkg/url/url_test.go b/pkg/url/url_test.go index 281e67f6d4f..1f8303f4eae 100644 --- a/pkg/url/url_test.go +++ b/pkg/url/url_test.go @@ -2,23 +2,20 @@ package url import ( "fmt" - "github.com/openshift/odo/pkg/localConfigProvider" "reflect" "strings" "testing" - devfilev1 "github.com/devfile/api/pkg/apis/workspaces/v1alpha2" "github.com/devfile/library/pkg/devfile/generator" + "github.com/golang/mock/gomock" "github.com/kylelemons/godebug/pretty" appsv1 "github.com/openshift/api/apps/v1" routev1 "github.com/openshift/api/route/v1" applabels "github.com/openshift/odo/pkg/application/labels" componentlabels "github.com/openshift/odo/pkg/component/labels" - dockercomponent "github.com/openshift/odo/pkg/devfile/adapters/docker/component" - "github.com/openshift/odo/pkg/envinfo" "github.com/openshift/odo/pkg/kclient" "github.com/openshift/odo/pkg/kclient/fake" - "github.com/openshift/odo/pkg/lclient" + "github.com/openshift/odo/pkg/localConfigProvider" "github.com/openshift/odo/pkg/occlient" "github.com/openshift/odo/pkg/testingutil" "github.com/openshift/odo/pkg/url/labels" @@ -28,7 +25,6 @@ import ( kerrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/runtime/schema" - //"github.com/openshift/odo/pkg/util" "github.com/openshift/odo/pkg/version" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -680,11 +676,8 @@ func TestPush(t *testing.T) { args args componentName string applicationName string - existingConfigURLs []localConfigProvider.LocalURL - existingEnvInfoURLs []localConfigProvider.LocalURL - returnedRoutes *routev1.RouteList - returnedIngress *extensionsv1.IngressList - containerComponents []devfilev1.Component + existingLocalURLs []localConfigProvider.LocalURL + existingClusterURLs URLList deletedURLs []URL createdURLs []URL wantErr bool @@ -697,7 +690,6 @@ func TestPush(t *testing.T) { }, componentName: "nodejs", applicationName: "app", - returnedRoutes: &routev1.RouteList{}, }, { name: "2 urls on local config and 0 on openshift cluster", @@ -707,19 +699,20 @@ func TestPush(t *testing.T) { isRouteSupported: true, isS2I: true, }, - existingConfigURLs: []localConfigProvider.LocalURL{ + existingLocalURLs: []localConfigProvider.LocalURL{ { Name: "example", Port: 8080, Secure: false, + Kind: localConfigProvider.ROUTE, }, { Name: "example-1", Port: 9090, Secure: false, + Kind: localConfigProvider.ROUTE, }, }, - returnedRoutes: &routev1.RouteList{}, createdURLs: []URL{ { ObjectMeta: metav1.ObjectMeta{ @@ -748,7 +741,10 @@ func TestPush(t *testing.T) { componentName: "wildfly", applicationName: "app", args: args{isRouteSupported: true, isS2I: true}, - returnedRoutes: testingutil.GetRouteListWithMultiple("wildfly", "app"), + existingClusterURLs: getMachineReadableFormatForList([]URL{ + getMachineReadableFormat(testingutil.GetSingleRoute("example", 8080, "wildfly", "app")), + getMachineReadableFormat(testingutil.GetSingleRoute("example-1", 9100, "wildfly", "app")), + }), deletedURLs: []URL{ getMachineReadableFormat(testingutil.GetSingleRoute("example-app", 8080, "nodejs", "app")), getMachineReadableFormat(testingutil.GetSingleRoute("example-1-app", 9100, "nodejs", "app")), @@ -759,19 +755,24 @@ func TestPush(t *testing.T) { componentName: "nodejs", applicationName: "app", args: args{isRouteSupported: true, isS2I: true}, - existingConfigURLs: []localConfigProvider.LocalURL{ + existingLocalURLs: []localConfigProvider.LocalURL{ { Name: "example-local-0", Port: 8080, Secure: false, + Kind: localConfigProvider.ROUTE, }, { Name: "example-local-1", Port: 9090, Secure: false, + Kind: localConfigProvider.ROUTE, }, }, - returnedRoutes: testingutil.GetRouteListWithMultiple("nodejs", "app"), + existingClusterURLs: getMachineReadableFormatForList([]URL{ + getMachineReadableFormat(testingutil.GetSingleRoute("example", 8080, "wildfly", "app")), + getMachineReadableFormat(testingutil.GetSingleRoute("example-1", 9100, "wildfly", "app")), + }), deletedURLs: []URL{ getMachineReadableFormat(testingutil.GetSingleRoute("example-app", 8080, "nodejs", "app")), getMachineReadableFormat(testingutil.GetSingleRoute("example-1-app", 9100, "nodejs", "app")), @@ -804,71 +805,55 @@ func TestPush(t *testing.T) { componentName: "nodejs", applicationName: "app", args: args{isRouteSupported: true, isS2I: true}, - existingConfigURLs: []localConfigProvider.LocalURL{ + existingLocalURLs: []localConfigProvider.LocalURL{ { Name: "example", Port: 8080, Secure: false, + Path: "/", + Kind: localConfigProvider.ROUTE, }, { Name: "example-1", Port: 9100, Secure: false, + Path: "/", + Kind: localConfigProvider.ROUTE, }, }, - returnedRoutes: testingutil.GetRouteListWithMultiple("nodejs", "app"), - deletedURLs: []URL{}, - createdURLs: []URL{}, + existingClusterURLs: getMachineReadableFormatForList([]URL{ + getMachineReadableFormat(testingutil.GetSingleRoute("example", 8080, "wildfly", "app")), + getMachineReadableFormat(testingutil.GetSingleRoute("example-1", 9100, "wildfly", "app")), + }), + deletedURLs: []URL{}, + createdURLs: []URL{}, }, { - name: "0 urls on env file and cluster", - componentName: "nodejs", - applicationName: "app", - args: args{isRouteSupported: true}, - existingEnvInfoURLs: []localConfigProvider.LocalURL{}, - returnedRoutes: &routev1.RouteList{}, - returnedIngress: &extensionsv1.IngressList{}, + name: "0 urls on env file and cluster", + componentName: "nodejs", + applicationName: "app", + args: args{isRouteSupported: true}, + existingLocalURLs: []localConfigProvider.LocalURL{}, }, { name: "2 urls on env file and 0 on openshift cluster", componentName: "nodejs", applicationName: "app", args: args{isRouteSupported: true}, - existingEnvInfoURLs: []localConfigProvider.LocalURL{ + existingLocalURLs: []localConfigProvider.LocalURL{ { Name: "example", Host: "com", + Port: 8080, Kind: localConfigProvider.INGRESS, }, { Name: "example-1", Host: "com", + Port: 9090, Kind: localConfigProvider.INGRESS, }, }, - containerComponents: []devfilev1.Component{ - { - Name: "container1", - ComponentUnion: devfilev1.ComponentUnion{ - Container: &devfilev1.ContainerComponent{ - Endpoints: []devfilev1.Endpoint{ - { - Name: "example", - TargetPort: 8080, - Secure: false, - }, - { - Name: "example-1", - TargetPort: 9090, - Secure: false, - }, - }, - }, - }, - }, - }, - returnedRoutes: &routev1.RouteList{}, - returnedIngress: &extensionsv1.IngressList{}, createdURLs: []URL{ { ObjectMeta: metav1.ObjectMeta{ @@ -895,13 +880,15 @@ func TestPush(t *testing.T) { }, }, { - name: "0 urls on env file and 2 on openshift cluster", - componentName: "nodejs", - applicationName: "app", - args: args{isRouteSupported: true}, - existingEnvInfoURLs: []localConfigProvider.LocalURL{}, - returnedRoutes: &routev1.RouteList{}, - returnedIngress: fake.GetIngressListWithMultiple("nodejs", "app"), + name: "0 urls on env file and 2 on openshift cluster", + componentName: "nodejs", + applicationName: "app", + args: args{isRouteSupported: true}, + existingLocalURLs: []localConfigProvider.LocalURL{}, + existingClusterURLs: getMachineReadableFormatForList([]URL{ + getMachineReadableFormatIngress(*fake.GetSingleIngress("example-0", "nodejs", "app")), + getMachineReadableFormatIngress(*fake.GetSingleIngress("example-1", "nodejs", "app")), + }), deletedURLs: []URL{ { ObjectMeta: metav1.ObjectMeta{ @@ -920,41 +907,24 @@ func TestPush(t *testing.T) { componentName: "wildfly", applicationName: "app", args: args{isRouteSupported: true}, - existingEnvInfoURLs: []localConfigProvider.LocalURL{ + existingLocalURLs: []localConfigProvider.LocalURL{ { Name: "example-local-0", Host: "com", + Port: 8080, Kind: localConfigProvider.INGRESS, }, { Name: "example-local-1", Host: "com", + Port: 9090, Kind: localConfigProvider.INGRESS, }, }, - containerComponents: []devfilev1.Component{ - { - Name: "container1", - ComponentUnion: devfilev1.ComponentUnion{ - Container: &devfilev1.ContainerComponent{ - Endpoints: []devfilev1.Endpoint{ - { - Name: "example-local-0", - TargetPort: 8080, - Secure: false, - }, - { - Name: "example-local-1", - TargetPort: 9090, - Secure: false, - }, - }, - }, - }, - }, - }, - returnedRoutes: &routev1.RouteList{}, - returnedIngress: fake.GetIngressListWithMultiple("wildfly", "app"), + existingClusterURLs: getMachineReadableFormatForList([]URL{ + getMachineReadableFormatIngress(*fake.GetSingleIngress("example-0", "nodejs", "app")), + getMachineReadableFormatIngress(*fake.GetSingleIngress("example-1", "nodejs", "app")), + }), createdURLs: []URL{ { ObjectMeta: metav1.ObjectMeta{ @@ -997,83 +967,49 @@ func TestPush(t *testing.T) { componentName: "wildfly", applicationName: "app", args: args{isRouteSupported: true}, - existingEnvInfoURLs: []localConfigProvider.LocalURL{ + existingLocalURLs: []localConfigProvider.LocalURL{ { Name: "example-0", Host: "com", + Port: 8080, Kind: localConfigProvider.INGRESS, }, { Name: "example-1", Host: "com", + Port: 9090, Kind: localConfigProvider.INGRESS, }, }, - containerComponents: []devfilev1.Component{ - { - Name: "container1", - ComponentUnion: devfilev1.ComponentUnion{ - Container: &devfilev1.ContainerComponent{ - Endpoints: []devfilev1.Endpoint{ - { - Name: "example-0", - TargetPort: 8080, - Secure: false, - }, - { - Name: "example-1", - TargetPort: 9090, - Secure: false, - }, - }, - }, - }, - }, - }, - returnedRoutes: &routev1.RouteList{}, - returnedIngress: fake.GetIngressListWithMultiple("wildfly", "app"), - createdURLs: []URL{}, - deletedURLs: []URL{}, + existingClusterURLs: getMachineReadableFormatForList([]URL{ + getMachineReadableFormatIngress(*fake.GetSingleIngress("example-0", "wildfly", "app")), + getMachineReadableFormatIngress(*fake.GetSingleIngress("example-1", "wildfly", "app")), + }), + createdURLs: []URL{}, + deletedURLs: []URL{}, }, { name: "2 (1 ingress,1 route) urls on env file and 2 on openshift cluster (1 ingress,1 route), but they are different", componentName: "nodejs", applicationName: "app", args: args{isRouteSupported: true}, - existingEnvInfoURLs: []localConfigProvider.LocalURL{ + existingLocalURLs: []localConfigProvider.LocalURL{ { Name: "example-local-0", + Port: 8080, Kind: localConfigProvider.ROUTE, }, { Name: "example-local-1", Host: "com", + Port: 9090, Kind: localConfigProvider.INGRESS, }, }, - containerComponents: []devfilev1.Component{ - { - Name: "container1", - ComponentUnion: devfilev1.ComponentUnion{ - Container: &devfilev1.ContainerComponent{ - Endpoints: []devfilev1.Endpoint{ - { - Name: "example-local-0", - TargetPort: 8080, - Secure: false, - }, - { - Name: "example-local-1", - TargetPort: 9090, - Secure: false, - }, - }, - }, - }, - }, - }, - returnedRoutes: &routev1.RouteList{}, - returnedIngress: fake.GetIngressListWithMultiple("nodejs", "app"), + existingClusterURLs: getMachineReadableFormatForList([]URL{ + getMachineReadableFormatIngress(*fake.GetSingleIngress("example-0", "nodejs", "app")), + getMachineReadableFormatIngress(*fake.GetSingleIngress("example-1", "nodejs", "app")), + }), createdURLs: []URL{ { ObjectMeta: metav1.ObjectMeta{ @@ -1115,32 +1051,16 @@ func TestPush(t *testing.T) { componentName: "nodejs", applicationName: "app", args: args{isRouteSupported: false}, - existingEnvInfoURLs: []localConfigProvider.LocalURL{ + existingLocalURLs: []localConfigProvider.LocalURL{ { Name: "example", Host: "com", TLSSecret: "secret", + Port: 8080, + Secure: true, Kind: localConfigProvider.INGRESS, }, }, - containerComponents: []devfilev1.Component{ - { - Name: "container1", - ComponentUnion: devfilev1.ComponentUnion{ - Container: &devfilev1.ContainerComponent{ - Endpoints: []devfilev1.Endpoint{ - { - Name: "example", - TargetPort: 8080, - Secure: true, - }, - }, - }, - }, - }, - }, - returnedRoutes: &routev1.RouteList{}, - returnedIngress: &extensionsv1.IngressList{}, createdURLs: []URL{ { ObjectMeta: metav1.ObjectMeta{ @@ -1163,34 +1083,16 @@ func TestPush(t *testing.T) { args: args{ isRouteSupported: true, }, - existingEnvInfoURLs: []localConfigProvider.LocalURL{ + existingLocalURLs: []localConfigProvider.LocalURL{ { Name: "example-local-0", + Port: 8080, Kind: localConfigProvider.ROUTE, }, }, - containerComponents: []devfilev1.Component{ - { - Name: "container1", - ComponentUnion: devfilev1.ComponentUnion{ - Container: &devfilev1.ContainerComponent{ - Endpoints: []devfilev1.Endpoint{ - { - Name: "example-local-0", - TargetPort: 8080, - Secure: false, - }, - }, - }, - }, - }, - }, - returnedRoutes: &routev1.RouteList{}, - returnedIngress: &extensionsv1.IngressList{ - Items: []extensionsv1.Ingress{ - *fake.GetSingleIngress("example-local-0", "nodejs", "app"), - }, - }, + existingClusterURLs: getMachineReadableFormatForList([]URL{ + getMachineReadableFormatIngress(*fake.GetSingleIngress("example-local-0", "nodejs", "app")), + }), createdURLs: []URL{ { ObjectMeta: metav1.ObjectMeta{ @@ -1217,19 +1119,17 @@ func TestPush(t *testing.T) { componentName: "nodejs", applicationName: "app", args: args{isRouteSupported: true, isS2I: true}, - existingConfigURLs: []localConfigProvider.LocalURL{ + existingLocalURLs: []localConfigProvider.LocalURL{ { Name: "example-local-0", Port: 8080, Secure: false, + Kind: localConfigProvider.ROUTE, }, }, - returnedRoutes: &routev1.RouteList{ - Items: []routev1.Route{ - testingutil.GetSingleRoute("example-local-0", 9090, "nodejs", "app"), - }, - }, - returnedIngress: &extensionsv1.IngressList{}, + existingClusterURLs: getMachineReadableFormatForList([]URL{ + getMachineReadableFormat(testingutil.GetSingleRoute("example-local-0", 9090, "nodejs", "app")), + }), createdURLs: []URL{ { ObjectMeta: metav1.ObjectMeta{ @@ -1256,30 +1156,14 @@ func TestPush(t *testing.T) { componentName: "nodejs", applicationName: "app", args: args{isRouteSupported: true}, - existingEnvInfoURLs: []localConfigProvider.LocalURL{ + existingLocalURLs: []localConfigProvider.LocalURL{ { - Name: "example", - Kind: localConfigProvider.ROUTE, - }, - }, - containerComponents: []devfilev1.Component{ - { - Name: "container1", - ComponentUnion: devfilev1.ComponentUnion{ - Container: &devfilev1.ContainerComponent{ - Endpoints: []devfilev1.Endpoint{ - { - Name: "example", - TargetPort: 8080, - Secure: true, - }, - }, - }, - }, + Name: "example", + Port: 8080, + Secure: true, + Kind: localConfigProvider.ROUTE, }, }, - returnedRoutes: &routev1.RouteList{}, - returnedIngress: &extensionsv1.IngressList{}, createdURLs: []URL{ { ObjectMeta: metav1.ObjectMeta{ @@ -1298,31 +1182,15 @@ func TestPush(t *testing.T) { componentName: "nodejs", applicationName: "app", args: args{isRouteSupported: true}, - existingEnvInfoURLs: []localConfigProvider.LocalURL{ + existingLocalURLs: []localConfigProvider.LocalURL{ { - Name: "example", - Host: "com", - Kind: localConfigProvider.INGRESS, - }, - }, - containerComponents: []devfilev1.Component{ - { - Name: "container1", - ComponentUnion: devfilev1.ComponentUnion{ - Container: &devfilev1.ContainerComponent{ - Endpoints: []devfilev1.Endpoint{ - { - Name: "example", - TargetPort: 8080, - Secure: true, - }, - }, - }, - }, + Name: "example", + Host: "com", + Secure: true, + Port: 8080, + Kind: localConfigProvider.INGRESS, }, }, - returnedRoutes: &routev1.RouteList{}, - returnedIngress: &extensionsv1.IngressList{}, createdURLs: []URL{ { ObjectMeta: metav1.ObjectMeta{ @@ -1342,32 +1210,16 @@ func TestPush(t *testing.T) { componentName: "nodejs", applicationName: "app", args: args{isRouteSupported: true}, - existingEnvInfoURLs: []localConfigProvider.LocalURL{ + existingLocalURLs: []localConfigProvider.LocalURL{ { Name: "example", Host: "com", TLSSecret: "secret", + Port: 8080, + Secure: true, Kind: localConfigProvider.INGRESS, }, }, - containerComponents: []devfilev1.Component{ - { - Name: "container1", - ComponentUnion: devfilev1.ComponentUnion{ - Container: &devfilev1.ContainerComponent{ - Endpoints: []devfilev1.Endpoint{ - { - Name: "example", - TargetPort: 8080, - Secure: true, - }, - }, - }, - }, - }, - }, - returnedRoutes: &routev1.RouteList{}, - returnedIngress: &extensionsv1.IngressList{}, createdURLs: []URL{ { ObjectMeta: metav1.ObjectMeta{ @@ -1384,186 +1236,33 @@ func TestPush(t *testing.T) { }, }, { - name: "env has ingress defined with same port, but endpoint port defined in devfile is internally exposed", - componentName: "nodejs", - args: args{isRouteSupported: true}, - existingEnvInfoURLs: []localConfigProvider.LocalURL{ - { - Name: "example", - Host: "com", - Kind: localConfigProvider.INGRESS, - }, - }, - containerComponents: []devfilev1.Component{ - { - Name: "container1", - ComponentUnion: devfilev1.ComponentUnion{ - Container: &devfilev1.ContainerComponent{ - Endpoints: []devfilev1.Endpoint{ - { - Name: "example", - TargetPort: 8080, - Secure: true, - Exposure: devfilev1.InternalEndpointExposure, - }, - }, - }, - }, - }, - }, - wantErr: false, - returnedRoutes: &routev1.RouteList{}, - returnedIngress: &extensionsv1.IngressList{}, - createdURLs: []URL{}, - }, - { - name: "env has ingress defined with same port, endpoint port defined in devfile is not exposed", - componentName: "nodejs", - args: args{isRouteSupported: true}, - existingEnvInfoURLs: []localConfigProvider.LocalURL{ - { - Name: "example", - Host: "com", - Kind: localConfigProvider.INGRESS, - }, - }, - containerComponents: []devfilev1.Component{ - { - Name: "container1", - ComponentUnion: devfilev1.ComponentUnion{ - Container: &devfilev1.ContainerComponent{ - Endpoints: []devfilev1.Endpoint{ - { - Name: "example", - TargetPort: 8080, - Secure: true, - Exposure: devfilev1.NoneEndpointExposure, - }, - }, - }, - }, - }, - }, - wantErr: false, - returnedRoutes: &routev1.RouteList{}, - returnedIngress: &extensionsv1.IngressList{}, - createdURLs: []URL{}, - }, - { - name: "env has route defined with same port, but endpoint port defined in devfile is internally exposed", - componentName: "nodejs", - args: args{isRouteSupported: true}, - existingEnvInfoURLs: []localConfigProvider.LocalURL{ - { - Name: "example", - Kind: localConfigProvider.ROUTE, - }, - }, - containerComponents: []devfilev1.Component{ - { - Name: "container1", - ComponentUnion: devfilev1.ComponentUnion{ - Container: &devfilev1.ContainerComponent{ - Endpoints: []devfilev1.Endpoint{ - { - Name: "example", - TargetPort: 8080, - Secure: true, - Exposure: devfilev1.InternalEndpointExposure, - }, - }, - }, - }, - }, - }, - wantErr: false, - returnedRoutes: &routev1.RouteList{}, - returnedIngress: &extensionsv1.IngressList{}, - createdURLs: []URL{}, - }, - { - name: "env has route defined with same port, but endpoint port defined in devfile is not exposed", + name: "no host defined for ingress should not create any URL", componentName: "nodejs", - args: args{isRouteSupported: true}, - existingEnvInfoURLs: []localConfigProvider.LocalURL{ + args: args{isRouteSupported: false}, + existingLocalURLs: []localConfigProvider.LocalURL{ { Name: "example", + Port: 8080, Kind: localConfigProvider.ROUTE, }, }, - containerComponents: []devfilev1.Component{ - { - Name: "container1", - ComponentUnion: devfilev1.ComponentUnion{ - Container: &devfilev1.ContainerComponent{ - Endpoints: []devfilev1.Endpoint{ - { - Name: "example", - TargetPort: 8080, - Secure: true, - Exposure: devfilev1.NoneEndpointExposure, - }, - }, - }, - }, - }, - }, - wantErr: false, - returnedRoutes: &routev1.RouteList{}, - returnedIngress: &extensionsv1.IngressList{}, - createdURLs: []URL{}, + wantErr: false, + createdURLs: []URL{}, }, { - name: "no host defined for ingress should not create any URL", - componentName: "nodejs", - args: args{isRouteSupported: false}, - existingEnvInfoURLs: []localConfigProvider.LocalURL{}, - containerComponents: []devfilev1.Component{ - { - Name: "container1", - ComponentUnion: devfilev1.ComponentUnion{ - Container: &devfilev1.ContainerComponent{ - Endpoints: []devfilev1.Endpoint{ - { - Name: "example", - TargetPort: 8080, - Secure: false, - }, - }, - }, - }, - }, - }, - wantErr: false, - returnedRoutes: &routev1.RouteList{}, - returnedIngress: &extensionsv1.IngressList{}, - createdURLs: []URL{}, - }, - { - name: "should create route in openshift cluster if endpoint is defined in devfile", - componentName: "nodejs", - applicationName: "app", - args: args{isRouteSupported: true}, - existingEnvInfoURLs: []localConfigProvider.LocalURL{}, - containerComponents: []devfilev1.Component{ + name: "should create route in openshift cluster if endpoint is defined in devfile", + componentName: "nodejs", + applicationName: "app", + args: args{isRouteSupported: true}, + existingLocalURLs: []localConfigProvider.LocalURL{ { - Name: "container1", - ComponentUnion: devfilev1.ComponentUnion{ - Container: &devfilev1.ContainerComponent{ - Endpoints: []devfilev1.Endpoint{ - { - Name: "example", - TargetPort: 8080, - Secure: false, - }, - }, - }, - }, + Name: "example", + Port: 8080, + Kind: localConfigProvider.ROUTE, + Secure: false, }, }, - wantErr: false, - returnedRoutes: &routev1.RouteList{}, - returnedIngress: &extensionsv1.IngressList{}, + wantErr: false, createdURLs: []URL{ { ObjectMeta: metav1.ObjectMeta{ @@ -1583,32 +1282,15 @@ func TestPush(t *testing.T) { componentName: "nodejs", applicationName: "app", args: args{isRouteSupported: true}, - existingEnvInfoURLs: []localConfigProvider.LocalURL{ + existingLocalURLs: []localConfigProvider.LocalURL{ { Name: "example", Host: "com", + Port: 8080, Kind: localConfigProvider.INGRESS, }, }, - containerComponents: []devfilev1.Component{ - { - Name: "container1", - ComponentUnion: devfilev1.ComponentUnion{ - Container: &devfilev1.ContainerComponent{ - Endpoints: []devfilev1.Endpoint{ - { - Name: "example", - TargetPort: 8080, - Secure: false, - }, - }, - }, - }, - }, - }, - wantErr: false, - returnedRoutes: &routev1.RouteList{}, - returnedIngress: &extensionsv1.IngressList{}, + wantErr: false, createdURLs: []URL{ { ObjectMeta: metav1.ObjectMeta{ @@ -1625,31 +1307,20 @@ func TestPush(t *testing.T) { }, }, { - name: "should create route in openshift cluster with path defined in devfile", - componentName: "nodejs", - applicationName: "app", - args: args{isRouteSupported: true}, - existingEnvInfoURLs: []localConfigProvider.LocalURL{}, - containerComponents: []devfilev1.Component{ + name: "should create route in openshift cluster with path defined in devfile", + componentName: "nodejs", + applicationName: "app", + args: args{isRouteSupported: true}, + existingLocalURLs: []localConfigProvider.LocalURL{ { - Name: "container1", - ComponentUnion: devfilev1.ComponentUnion{ - Container: &devfilev1.ContainerComponent{ - Endpoints: []devfilev1.Endpoint{ - { - Name: "example", - TargetPort: 8080, - Secure: false, - Path: "/testpath", - }, - }, - }, - }, + Name: "example", + Port: 8080, + Secure: false, + Path: "/testpath", + Kind: localConfigProvider.ROUTE, }, }, - wantErr: false, - returnedRoutes: &routev1.RouteList{}, - returnedIngress: &extensionsv1.IngressList{}, + wantErr: false, createdURLs: []URL{ { ObjectMeta: metav1.ObjectMeta{ @@ -1669,33 +1340,17 @@ func TestPush(t *testing.T) { componentName: "nodejs", applicationName: "app", args: args{isRouteSupported: true}, - existingEnvInfoURLs: []localConfigProvider.LocalURL{ + existingLocalURLs: []localConfigProvider.LocalURL{ { - Name: "example", - Host: "com", - Kind: localConfigProvider.INGRESS, - }, - }, - containerComponents: []devfilev1.Component{ - { - Name: "container1", - ComponentUnion: devfilev1.ComponentUnion{ - Container: &devfilev1.ContainerComponent{ - Endpoints: []devfilev1.Endpoint{ - { - Name: "example", - TargetPort: 8080, - Secure: false, - Path: "/testpath", - }, - }, - }, - }, + Name: "example", + Host: "com", + Port: 8080, + Secure: false, + Path: "/testpath", + Kind: localConfigProvider.INGRESS, }, }, - wantErr: false, - returnedRoutes: &routev1.RouteList{}, - returnedIngress: &extensionsv1.IngressList{}, + wantErr: false, createdURLs: []URL{ { ObjectMeta: metav1.ObjectMeta{ @@ -1715,28 +1370,29 @@ func TestPush(t *testing.T) { for testNum, tt := range tests { tt.name = fmt.Sprintf("case %d: ", testNum+1) + tt.name t.Run(tt.name, func(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + mockLocalConfigProvider := localConfigProvider.NewMockLocalConfigProvider(ctrl) + mockLocalConfigProvider.EXPECT().ListURLs().Return(tt.existingLocalURLs) + + mockURLClient := NewMockClient(ctrl) + mockURLClient.EXPECT().ListCluster().Return(tt.existingClusterURLs, nil) + fakeClient, fakeClientSet := occlient.FakeNew() fakeKClient, fakeKClientSet := kclient.FakeNew() - fakeKClientSet.Kubernetes.PrependReactor("list", "ingresses", func(action ktesting.Action) (handled bool, ret runtime.Object, err error) { - return true, tt.returnedIngress, nil - }) - fakeKClientSet.Kubernetes.PrependReactor("delete", "ingresses", func(action ktesting.Action) (bool, runtime.Object, error) { return true, nil, nil }) - fakeClientSet.RouteClientset.PrependReactor("list", "routes", func(action ktesting.Action) (bool, runtime.Object, error) { - return true, tt.returnedRoutes, nil - }) - fakeClientSet.RouteClientset.PrependReactor("delete", "routes", func(action ktesting.Action) (bool, runtime.Object, error) { return true, nil, nil }) fakeKClientSet.Kubernetes.PrependReactor("get", "secrets", func(action ktesting.Action) (bool, runtime.Object, error) { - if tt.existingEnvInfoURLs[0].TLSSecret != "" { - return true, fake.GetSecret(tt.existingEnvInfoURLs[0].TLSSecret), nil + if tt.existingLocalURLs[0].TLSSecret != "" { + return true, fake.GetSecret(tt.existingLocalURLs[0].TLSSecret), nil } return true, fake.GetSecret(tt.componentName + "-tlssecret"), nil }) @@ -1750,13 +1406,12 @@ func TestPush(t *testing.T) { }) if err := Push(fakeClient, fakeKClient, PushParameters{ - ComponentName: tt.componentName, - ApplicationName: tt.applicationName, - ConfigURLs: tt.existingConfigURLs, - EnvURLS: tt.existingEnvInfoURLs, - IsRouteSupported: tt.args.isRouteSupported, - ContainerComponents: tt.containerComponents, - IsS2I: tt.args.isS2I, + ComponentName: tt.componentName, + ApplicationName: tt.applicationName, + LocalConfig: mockLocalConfigProvider, + URLClient: mockURLClient, + IsRouteSupported: tt.args.isRouteSupported, + IsS2I: tt.args.isS2I, }); (err != nil) != tt.wantErr { t.Errorf("Push() error = %v, wantErr %v", err, tt.wantErr) } else { @@ -1872,686 +1527,6 @@ func TestPush(t *testing.T) { } } -func TestListDockerURL(t *testing.T) { - fakeClient := lclient.FakeNew() - fakeErrorClient := lclient.FakeErrorNew() - testURL1 := localConfigProvider.LocalURL{Name: "testurl1", Port: 8080, ExposedPort: 56789, Kind: "docker"} - testURL2 := localConfigProvider.LocalURL{Name: "testurl2", Port: 8080, ExposedPort: 54321, Kind: "docker"} - testURL3 := localConfigProvider.LocalURL{Name: "testurl3", Port: 8080, ExposedPort: 65432, Kind: "docker"} - esi := &envinfo.EnvSpecificInfo{} - err := esi.SetConfiguration("url", testURL1) - if err != nil { - // discard the error, since no physical file to write - t.Log("Expected error since no physical env file to write") - } - err = esi.SetConfiguration("url", testURL2) - if err != nil { - // discard the error, since no physical file to write - t.Log("Expected error since no physical env file to write") - } - - tests := []struct { - name string - client *lclient.Client - component string - wantURLs []URL - wantErr bool - }{ - { - name: "Case 1: Successfully retrieve the URL list", - client: fakeClient, - component: "golang", - wantURLs: []URL{ - URL{ - TypeMeta: metav1.TypeMeta{Kind: "url", APIVersion: "odo.dev/v1alpha1"}, - ObjectMeta: metav1.ObjectMeta{Name: testURL1.Name}, - Spec: URLSpec{Host: dockercomponent.LocalhostIP, Port: testURL1.Port, ExternalPort: testURL1.ExposedPort}, - Status: URLStatus{ - State: StateTypeNotPushed, - }, - }, - URL{ - TypeMeta: metav1.TypeMeta{Kind: "url", APIVersion: "odo.dev/v1alpha1"}, - ObjectMeta: metav1.ObjectMeta{Name: testURL2.Name}, - Spec: URLSpec{Host: dockercomponent.LocalhostIP, Port: testURL2.Port, ExternalPort: testURL2.ExposedPort}, - Status: URLStatus{ - State: StateTypePushed, - }, - }, - URL{ - TypeMeta: metav1.TypeMeta{Kind: "url", APIVersion: "odo.dev/v1alpha1"}, - ObjectMeta: metav1.ObjectMeta{Name: testURL3.Name}, - Spec: URLSpec{Host: dockercomponent.LocalhostIP, Port: testURL3.Port, ExternalPort: testURL3.ExposedPort}, - Status: URLStatus{ - State: StateTypeLocallyDeleted, - }, - }, - }, - wantErr: false, - }, - { - name: "Case 2: Error retrieving the URL list", - client: fakeErrorClient, - component: "golang", - wantURLs: nil, - wantErr: true, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - urls, err := ListDockerURL(tt.client, tt.component, esi) - if !tt.wantErr == (err != nil) { - t.Errorf("expected %v, got %v", tt.wantErr, err) - } - - if len(urls.Items) != len(tt.wantURLs) { - t.Errorf("numbers of url listed does not match, expected %v, got %v", len(tt.wantURLs), len(urls.Items)) - } - actualURLMap := make(map[string]URL) - for _, actualURL := range urls.Items { - actualURLMap[actualURL.Name] = actualURL - } - for _, wantURL := range tt.wantURLs { - if !reflect.DeepEqual(actualURLMap[wantURL.Name], wantURL) { - t.Errorf("Expected %v, got %v", wantURL, actualURLMap[wantURL.Name]) - } - } - }) - } -} - -func TestListIngressAndRoute(t *testing.T) { - componentName := "testcomponent" - containerName := "testcontainer" - - // testURL1 and testURL6 not exist in local - testURL1 := localConfigProvider.LocalURL{Name: "example-0", Port: 8080, Host: "com", Kind: "ingress"} - testURL2 := localConfigProvider.LocalURL{Name: "example-1", Host: "com", Kind: "ingress"} - testURL3 := localConfigProvider.LocalURL{Name: "ingressurl3", Host: "com", Kind: "ingress"} - testURL4 := localConfigProvider.LocalURL{Name: "example", Kind: "route"} - testURL5 := localConfigProvider.LocalURL{Name: "routeurl2", Kind: "route"} - testURL6 := localConfigProvider.LocalURL{Name: "routeurl3", Port: 8080, Kind: "route"} - - example1Endpoint := devfilev1.Endpoint{ - Name: "example-1", - Exposure: devfilev1.PublicEndpointExposure, - TargetPort: 9090, - Protocol: devfilev1.HTTPEndpointProtocol, - } - - ingressurl3Endpoint := devfilev1.Endpoint{ - Name: "ingressurl3", - Exposure: devfilev1.PublicEndpointExposure, - TargetPort: 8080, - Protocol: devfilev1.HTTPSEndpointProtocol, - Secure: true, - } - - exampleEndpoint := devfilev1.Endpoint{ - Name: "example", - Exposure: devfilev1.PublicEndpointExposure, - TargetPort: 8080, - Protocol: devfilev1.HTTPEndpointProtocol, - } - - routeurl2Endpoint := devfilev1.Endpoint{ - Name: "routeurl2", - Exposure: devfilev1.PublicEndpointExposure, - TargetPort: 8080, - Protocol: devfilev1.HTTPEndpointProtocol, - } - tests := []struct { - name string - component string - envURLs []localConfigProvider.LocalURL - containerComponents []devfilev1.Component - routeSupported bool - routeList *routev1.RouteList - ingressList *extensionsv1.IngressList - wantURLs []URL - }{ - { - name: "Should retrieve the URL list with both ingress and routes", - component: componentName, - envURLs: []localConfigProvider.LocalURL{testURL2, testURL3, testURL4, testURL5}, - containerComponents: []devfilev1.Component{ - { - Name: containerName, - ComponentUnion: devfilev1.ComponentUnion{ - Container: &devfilev1.ContainerComponent{ - Endpoints: []devfilev1.Endpoint{ - example1Endpoint, ingressurl3Endpoint, exampleEndpoint, routeurl2Endpoint, - }, - }, - }, - }, - }, - routeSupported: true, - ingressList: fake.GetIngressListWithMultiple(componentName, "app"), - routeList: &routev1.RouteList{ - Items: []routev1.Route{ - testingutil.GetSingleRoute(testURL4.Name, int(exampleEndpoint.TargetPort), componentName, ""), - testingutil.GetSingleRoute(testURL6.Name, testURL6.Port, componentName, ""), - }, - }, - wantURLs: []URL{ - URL{ - TypeMeta: metav1.TypeMeta{Kind: "url", APIVersion: "odo.dev/v1alpha1"}, - ObjectMeta: metav1.ObjectMeta{Name: testURL1.Name}, - Spec: URLSpec{Host: "example-0.com", Port: testURL1.Port, Secure: testURL1.Secure, Kind: testURL1.Kind, Path: "/"}, - Status: URLStatus{ - State: StateTypeLocallyDeleted, - }, - }, - URL{ - TypeMeta: metav1.TypeMeta{Kind: "url", APIVersion: "odo.dev/v1alpha1"}, - ObjectMeta: metav1.ObjectMeta{Name: testURL2.Name}, - Spec: URLSpec{Host: fmt.Sprintf("%v.%v", example1Endpoint.Name, testURL2.Host), Port: int(example1Endpoint.TargetPort), Secure: example1Endpoint.Secure, Kind: testURL2.Kind, Path: "/"}, - Status: URLStatus{ - State: StateTypePushed, - }, - }, - URL{ - TypeMeta: metav1.TypeMeta{Kind: "url", APIVersion: "odo.dev/v1alpha1"}, - ObjectMeta: metav1.ObjectMeta{Name: testURL3.Name}, - Spec: URLSpec{Host: fmt.Sprintf("%v.%v", ingressurl3Endpoint.Name, testURL3.Host), Port: int(ingressurl3Endpoint.TargetPort), Secure: ingressurl3Endpoint.Secure, TLSSecret: componentName + "-tlssecret", Kind: testURL3.Kind}, - Status: URLStatus{ - State: StateTypeNotPushed, - }, - }, - URL{ - TypeMeta: metav1.TypeMeta{Kind: "url", APIVersion: "odo.dev/v1alpha1"}, - ObjectMeta: metav1.ObjectMeta{Name: testURL4.Name}, - Spec: URLSpec{Protocol: "http", Port: int(exampleEndpoint.TargetPort), Secure: exampleEndpoint.Secure, Kind: testURL4.Kind, Path: "/"}, - Status: URLStatus{ - State: StateTypePushed, - }, - }, - URL{ - TypeMeta: metav1.TypeMeta{Kind: "url", APIVersion: "odo.dev/v1alpha1"}, - ObjectMeta: metav1.ObjectMeta{Name: testURL5.Name}, - Spec: URLSpec{Port: int(routeurl2Endpoint.TargetPort), Secure: routeurl2Endpoint.Secure, Kind: testURL5.Kind}, - Status: URLStatus{ - State: StateTypeNotPushed, - }, - }, - URL{ - TypeMeta: metav1.TypeMeta{Kind: "url", APIVersion: "odo.dev/v1alpha1"}, - ObjectMeta: metav1.ObjectMeta{Name: testURL6.Name}, - Spec: URLSpec{Protocol: "http", Port: testURL6.Port, Secure: testURL6.Secure, Kind: testURL6.Kind, Path: "/"}, - Status: URLStatus{ - State: StateTypeLocallyDeleted, - }, - }, - }, - }, - { - name: "Should retrieve only ingress URLs with routeSupported equals to false", - component: componentName, - envURLs: []localConfigProvider.LocalURL{testURL2, testURL3, testURL4, testURL5}, - containerComponents: []devfilev1.Component{ - { - Name: containerName, - ComponentUnion: devfilev1.ComponentUnion{ - Container: &devfilev1.ContainerComponent{ - Endpoints: []devfilev1.Endpoint{ - example1Endpoint, ingressurl3Endpoint, exampleEndpoint, routeurl2Endpoint, - }, - }, - }, - }, - }, - routeList: &routev1.RouteList{}, - ingressList: fake.GetIngressListWithMultiple(componentName, "app"), - routeSupported: false, - wantURLs: []URL{ - URL{ - TypeMeta: metav1.TypeMeta{Kind: "url", APIVersion: "odo.dev/v1alpha1"}, - ObjectMeta: metav1.ObjectMeta{Name: testURL1.Name}, - Spec: URLSpec{Host: "example-0.com", Port: testURL1.Port, Secure: testURL1.Secure, Kind: testURL1.Kind, Path: "/"}, - Status: URLStatus{ - State: StateTypeLocallyDeleted, - }, - }, - URL{ - TypeMeta: metav1.TypeMeta{Kind: "url", APIVersion: "odo.dev/v1alpha1"}, - ObjectMeta: metav1.ObjectMeta{Name: testURL2.Name}, - Spec: URLSpec{Host: fmt.Sprintf("%v.%v", example1Endpoint.Name, testURL2.Host), Port: int(example1Endpoint.TargetPort), Secure: example1Endpoint.Secure, Kind: testURL2.Kind, Path: "/"}, - Status: URLStatus{ - State: StateTypePushed, - }, - }, - URL{ - TypeMeta: metav1.TypeMeta{Kind: "url", APIVersion: "odo.dev/v1alpha1"}, - ObjectMeta: metav1.ObjectMeta{Name: testURL3.Name}, - Spec: URLSpec{Host: fmt.Sprintf("%v.%v", ingressurl3Endpoint.Name, testURL3.Host), Port: int(ingressurl3Endpoint.TargetPort), Secure: ingressurl3Endpoint.Secure, TLSSecret: componentName + "-tlssecret", Kind: testURL3.Kind}, - Status: URLStatus{ - State: StateTypeNotPushed, - }, - }, - }, - }, - { - name: "Should retrieve only ingress URLs", - component: componentName, - envURLs: []localConfigProvider.LocalURL{testURL2, testURL3}, - containerComponents: []devfilev1.Component{ - { - Name: containerName, - ComponentUnion: devfilev1.ComponentUnion{ - Container: &devfilev1.ContainerComponent{ - Endpoints: []devfilev1.Endpoint{ - example1Endpoint, ingressurl3Endpoint, - }, - }, - }, - }, - }, - routeSupported: true, - routeList: &routev1.RouteList{}, - ingressList: fake.GetIngressListWithMultiple(componentName, "app"), - wantURLs: []URL{ - URL{ - TypeMeta: metav1.TypeMeta{Kind: "url", APIVersion: "odo.dev/v1alpha1"}, - ObjectMeta: metav1.ObjectMeta{Name: testURL1.Name}, - Spec: URLSpec{Host: "example-0.com", Port: testURL1.Port, Secure: testURL1.Secure, Kind: localConfigProvider.INGRESS, Path: "/"}, - Status: URLStatus{ - State: StateTypeLocallyDeleted, - }, - }, - URL{ - TypeMeta: metav1.TypeMeta{Kind: "url", APIVersion: "odo.dev/v1alpha1"}, - ObjectMeta: metav1.ObjectMeta{Name: testURL2.Name}, - Spec: URLSpec{Host: fmt.Sprintf("%v.%v", example1Endpoint.Name, testURL2.Host), Port: int(example1Endpoint.TargetPort), Secure: example1Endpoint.Secure, Kind: testURL2.Kind, Path: "/"}, - Status: URLStatus{ - State: StateTypePushed, - }, - }, - URL{ - TypeMeta: metav1.TypeMeta{Kind: "url", APIVersion: "odo.dev/v1alpha1"}, - ObjectMeta: metav1.ObjectMeta{Name: testURL3.Name}, - Spec: URLSpec{Host: fmt.Sprintf("%v.%v", ingressurl3Endpoint.Name, testURL3.Host), Port: int(ingressurl3Endpoint.TargetPort), Secure: ingressurl3Endpoint.Secure, TLSSecret: componentName + "-tlssecret", Kind: testURL3.Kind}, - Status: URLStatus{ - State: StateTypeNotPushed, - }, - }, - }, - }, - { - name: "Should retrieve only route URLs", - component: componentName, - envURLs: []localConfigProvider.LocalURL{testURL4, testURL5}, - containerComponents: []devfilev1.Component{ - { - Name: containerName, - ComponentUnion: devfilev1.ComponentUnion{ - Container: &devfilev1.ContainerComponent{ - Endpoints: []devfilev1.Endpoint{ - exampleEndpoint, routeurl2Endpoint, - }, - }, - }, - }, - }, - routeSupported: true, - routeList: &routev1.RouteList{ - Items: []routev1.Route{ - testingutil.GetSingleRoute(testURL4.Name, int(exampleEndpoint.TargetPort), componentName, ""), - testingutil.GetSingleRoute(testURL6.Name, testURL6.Port, componentName, ""), - }, - }, - ingressList: &extensionsv1.IngressList{}, - wantURLs: []URL{ - URL{ - TypeMeta: metav1.TypeMeta{Kind: "url", APIVersion: "odo.dev/v1alpha1"}, - ObjectMeta: metav1.ObjectMeta{Name: testURL4.Name}, - Spec: URLSpec{Protocol: "http", Port: int(exampleEndpoint.TargetPort), Secure: exampleEndpoint.Secure, Kind: testURL4.Kind, Path: "/"}, - Status: URLStatus{ - State: StateTypePushed, - }, - }, - URL{ - TypeMeta: metav1.TypeMeta{Kind: "url", APIVersion: "odo.dev/v1alpha1"}, - ObjectMeta: metav1.ObjectMeta{Name: testURL5.Name}, - Spec: URLSpec{Port: int(routeurl2Endpoint.TargetPort), Secure: routeurl2Endpoint.Secure, Kind: testURL5.Kind}, - Status: URLStatus{ - State: StateTypeNotPushed, - }, - }, - URL{ - TypeMeta: metav1.TypeMeta{Kind: "url", APIVersion: "odo.dev/v1alpha1"}, - ObjectMeta: metav1.ObjectMeta{Name: testURL6.Name}, - Spec: URLSpec{Protocol: "http", Port: testURL6.Port, Secure: testURL6.Secure, Kind: testURL6.Kind, Path: "/"}, - Status: URLStatus{ - State: StateTypeLocallyDeleted, - }, - }, - }, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - // initialising virtual envinfo - esi := &envinfo.EnvSpecificInfo{} - for _, url := range tt.envURLs { - err := esi.SetConfiguration("url", url) - if err != nil { - // discard the error, since no physical file to write - t.Log("Expected error since no physical env file to write") - } - } - // initialising the fakeclient - fkclient, fkclientset := kclient.FakeNew() - fkclient.Namespace = "default" - fkclientset.Kubernetes.PrependReactor("list", "ingresses", func(action ktesting.Action) (bool, runtime.Object, error) { - return true, tt.ingressList, nil - }) - fakeoclient, fakeoclientSet := occlient.FakeNew() - fakeoclientSet.RouteClientset.PrependReactor("list", "routes", func(action ktesting.Action) (bool, runtime.Object, error) { - return true, tt.routeList, nil - }) - fakeoclient.SetKubeClient(fkclient) - - urls, err := ListIngressAndRoute(fakeoclient, esi, tt.containerComponents, componentName, tt.routeSupported) - if err != nil { - t.Errorf("unexpected error %v", err) - } - - if len(urls.Items) != len(tt.wantURLs) { - t.Errorf("numbers of url listed does not match, expected %v, got %v", len(tt.wantURLs), len(urls.Items)) - } - actualURLMap := make(map[string]URL) - for _, actualURL := range urls.Items { - actualURLMap[actualURL.Name] = actualURL - } - for _, wantURL := range tt.wantURLs { - if !reflect.DeepEqual(actualURLMap[wantURL.Name], wantURL) { - t.Errorf("Expected %v, got %v", wantURL, actualURLMap[wantURL.Name]) - } - } - }) - } - -} - -func TestGetIngressOrRoute(t *testing.T) { - componentName := "testcomponent" - containerName := "testcontainer" - - // testURL1 and testURL6 not exist in local - testURL1 := localConfigProvider.LocalURL{Name: "ingressurl1", Port: 8080, Host: "com", Kind: "ingress"} - testURL2 := localConfigProvider.LocalURL{Name: "ingressurl2", Host: "com", Kind: "ingress"} - testURL3 := localConfigProvider.LocalURL{Name: "ingressurl3", Host: "com", Kind: "ingress"} - testURL4 := localConfigProvider.LocalURL{Name: "example", Kind: "route"} - testURL5 := localConfigProvider.LocalURL{Name: "routeurl2", Kind: "route"} - testURL6 := localConfigProvider.LocalURL{Name: "routeurl3", Port: 8080, Kind: "route"} - - esi := &envinfo.EnvSpecificInfo{} - err := esi.SetConfiguration("url", testURL2) - if err != nil { - // discard the error, since no physical file to write - t.Log("Expected error since no physical env file to write") - } - err = esi.SetConfiguration("url", testURL3) - if err != nil { - // discard the error, since no physical file to write - t.Log("Expected error since no physical env file to write") - } - err = esi.SetConfiguration("url", testURL4) - if err != nil { - // discard the error, since no physical file to write - t.Log("Expected error since no physical env file to write") - } - err = esi.SetConfiguration("url", testURL5) - if err != nil { - // discard the error, since no physical file to write - t.Log("Expected error since no physical env file to write") - } - fakecomponent := testingutil.GetFakeContainerComponent(containerName) - fakecomponent.Container.Endpoints = []devfilev1.Endpoint{ - { - Name: "ingressurl2", - Exposure: devfilev1.PublicEndpointExposure, - TargetPort: 8080, - Protocol: devfilev1.HTTPEndpointProtocol, - Path: "/", - }, - { - Name: "ingressurl3", - Exposure: devfilev1.PublicEndpointExposure, - TargetPort: 8080, - Protocol: devfilev1.HTTPEndpointProtocol, - Secure: true, - Path: "/", - }, - { - Name: "example", - Exposure: devfilev1.PublicEndpointExposure, - TargetPort: 8080, - Protocol: devfilev1.HTTPEndpointProtocol, - Path: "/", - }, - { - Name: "routeurl2", - Exposure: devfilev1.PublicEndpointExposure, - TargetPort: 8080, - Protocol: devfilev1.HTTPEndpointProtocol, - Path: "/", - }, - } - containerComponents := []devfilev1.Component{ - fakecomponent, - } - - tests := []struct { - name string - component string - urlName string - routeSupported bool - pushedIngress *extensionsv1.Ingress - pushedRoute routev1.Route - wantURL URL - wantErr bool - }{ - { - name: "Case 1: Successfully retrieve the locally deleted Ingress URL object", - component: componentName, - urlName: testURL1.Name, - routeSupported: true, - pushedIngress: fake.GetSingleIngress(testURL1.Name, componentName, "app"), - pushedRoute: routev1.Route{}, - wantURL: URL{ - TypeMeta: metav1.TypeMeta{Kind: "url", APIVersion: "odo.dev/v1alpha1"}, - ObjectMeta: metav1.ObjectMeta{Name: testURL1.Name}, - Spec: URLSpec{Host: "ingressurl1.com", Port: testURL1.Port, Secure: testURL1.Secure, Kind: localConfigProvider.INGRESS, Path: "/"}, - Status: URLStatus{ - State: StateTypeLocallyDeleted, - }, - }, - wantErr: false, - }, - { - name: "Case 2: Successfully retrieve the pushed Ingress URL object", - component: componentName, - urlName: testURL2.Name, - routeSupported: true, - pushedIngress: fake.GetSingleIngress(testURL2.Name, componentName, "app"), - pushedRoute: routev1.Route{}, - wantURL: URL{ - TypeMeta: metav1.TypeMeta{Kind: "url", APIVersion: "odo.dev/v1alpha1"}, - ObjectMeta: metav1.ObjectMeta{Name: testURL2.Name}, - Spec: URLSpec{Host: "ingressurl2.com", Port: 8080, Secure: false, Kind: localConfigProvider.INGRESS, Path: "/"}, - Status: URLStatus{ - State: StateTypePushed, - }, - }, - wantErr: false, - }, - { - name: "Case 3: Successfully retrieve the not pushed Ingress URL object", - component: componentName, - urlName: testURL3.Name, - routeSupported: true, - pushedIngress: nil, - pushedRoute: routev1.Route{}, - wantURL: URL{ - TypeMeta: metav1.TypeMeta{Kind: "url", APIVersion: "odo.dev/v1alpha1"}, - ObjectMeta: metav1.ObjectMeta{Name: testURL3.Name}, - Spec: URLSpec{Host: "ingressurl3.com", Port: 8080, Secure: true, TLSSecret: componentName + "-tlssecret", Kind: localConfigProvider.INGRESS}, - Status: URLStatus{ - State: StateTypeNotPushed, - }, - }, - wantErr: false, - }, - { - name: "Case 4: Should show error if the url does not exist", - component: componentName, - urlName: "notExistURL", - routeSupported: true, - pushedIngress: nil, - pushedRoute: routev1.Route{}, - wantErr: true, - }, - { - name: "Case 5: Successfully retrieve the pushed Route URL object", - component: componentName, - urlName: testURL4.Name, - routeSupported: true, - pushedIngress: nil, - pushedRoute: testingutil.GetSingleRoute(testURL4.Name, 8080, componentName, ""), - wantURL: URL{ - TypeMeta: metav1.TypeMeta{Kind: "url", APIVersion: "odo.dev/v1alpha1"}, - ObjectMeta: metav1.ObjectMeta{Name: testURL4.Name}, - Spec: URLSpec{Protocol: "http", Port: 8080, Secure: false, Kind: localConfigProvider.ROUTE, Path: "/"}, - Status: URLStatus{ - State: StateTypePushed, - }, - }, - wantErr: false, - }, - { - name: "Case 6 Successfully retrieve the not pushed Route URL object", - component: componentName, - urlName: testURL5.Name, - routeSupported: true, - pushedIngress: nil, - pushedRoute: routev1.Route{}, - wantURL: URL{ - TypeMeta: metav1.TypeMeta{Kind: "url", APIVersion: "odo.dev/v1alpha1"}, - ObjectMeta: metav1.ObjectMeta{Name: testURL5.Name}, - Spec: URLSpec{Port: 8080, Secure: false, Kind: localConfigProvider.ROUTE}, - Status: URLStatus{ - State: StateTypeNotPushed, - }, - }, - wantErr: false, - }, - { - name: "Case 7: Successfully retrieve the locally deleted Route URL object", - component: componentName, - urlName: testURL6.Name, - routeSupported: true, - pushedIngress: nil, - pushedRoute: testingutil.GetSingleRoute(testURL6.Name, testURL6.Port, componentName, ""), - wantURL: URL{ - TypeMeta: metav1.TypeMeta{Kind: "url", APIVersion: "odo.dev/v1alpha1"}, - ObjectMeta: metav1.ObjectMeta{Name: testURL6.Name}, - Spec: URLSpec{Protocol: "http", Port: testURL6.Port, Secure: testURL6.Secure, Kind: localConfigProvider.ROUTE, Path: "/"}, - Status: URLStatus{ - State: StateTypeLocallyDeleted, - }, - }, - wantErr: false, - }, - { - name: "Case 8: If route is not supported, should show error and empty URL when describing a route", - component: componentName, - urlName: testURL5.Name, - routeSupported: false, - pushedIngress: nil, - pushedRoute: routev1.Route{}, - wantURL: URL{}, - wantErr: true, - }, - { - name: "Case 9: If route is not supported, should retrieve not pushed ingress", - component: componentName, - urlName: testURL3.Name, - routeSupported: false, - pushedIngress: nil, - pushedRoute: routev1.Route{}, - wantURL: URL{ - TypeMeta: metav1.TypeMeta{Kind: "url", APIVersion: "odo.dev/v1alpha1"}, - ObjectMeta: metav1.ObjectMeta{Name: testURL3.Name}, - Spec: URLSpec{Host: "ingressurl3.com", Port: 8080, Secure: true, TLSSecret: componentName + "-tlssecret", Kind: localConfigProvider.INGRESS}, - Status: URLStatus{ - State: StateTypeNotPushed, - }, - }, - wantErr: false, - }, - { - name: "Case 10: If route is not supported, should retrieve pushed ingress", - component: componentName, - urlName: testURL2.Name, - routeSupported: false, - pushedIngress: fake.GetSingleIngress(testURL2.Name, componentName, "app"), - pushedRoute: routev1.Route{}, - wantURL: URL{ - TypeMeta: metav1.TypeMeta{Kind: "url", APIVersion: "odo.dev/v1alpha1"}, - ObjectMeta: metav1.ObjectMeta{Name: testURL2.Name}, - Spec: URLSpec{Host: "ingressurl2.com", Port: 8080, Secure: false, Kind: localConfigProvider.INGRESS, Path: "/"}, - Status: URLStatus{ - State: StateTypePushed, - }, - }, - wantErr: false, - }, - { - name: "Case 11: If route is not supported, should retrieve locally deleted ingress", - component: componentName, - urlName: testURL1.Name, - routeSupported: false, - pushedIngress: fake.GetSingleIngress(testURL1.Name, componentName, "app"), - pushedRoute: routev1.Route{}, - wantURL: URL{ - TypeMeta: metav1.TypeMeta{Kind: "url", APIVersion: "odo.dev/v1alpha1"}, - ObjectMeta: metav1.ObjectMeta{Name: testURL1.Name}, - Spec: URLSpec{Host: "ingressurl1.com", Port: testURL1.Port, Secure: testURL1.Secure, Kind: localConfigProvider.INGRESS, Path: "/"}, - Status: URLStatus{ - State: StateTypeLocallyDeleted, - }, - }, - wantErr: false, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - fkclient, fkclientset := kclient.FakeNew() - fkclient.Namespace = "default" - if tt.pushedIngress != nil { - fkclientset.Kubernetes.PrependReactor("get", "ingresses", func(action ktesting.Action) (bool, runtime.Object, error) { - return true, tt.pushedIngress, nil - }) - } - client, fakeClientSet := occlient.FakeNew() - if !reflect.DeepEqual(tt.pushedRoute, routev1.Route{}) { - fakeClientSet.RouteClientset.PrependReactor("get", "routes", func(action ktesting.Action) (bool, runtime.Object, error) { - return true, &tt.pushedRoute, nil - }) - } - url, err := GetIngressOrRoute(client, fkclient, esi, tt.urlName, containerComponents, tt.component, tt.routeSupported) - if !tt.wantErr == (err != nil) { - t.Errorf("unexpected error %v", err) - } - if !reflect.DeepEqual(url, tt.wantURL) { - t.Errorf("Expected %v, got %v", tt.wantURL, url) - } - }) - } -} - func TestConvertEnvinfoURL(t *testing.T) { serviceName := "testService" urlName := "testURL" From af19576e0c4daba41c0a8ad44b1de5bf7cad40c7 Mon Sep 17 00:00:00 2001 From: mik-dass Date: Wed, 27 Jan 2021 17:20:20 +0530 Subject: [PATCH 2/3] Adds comments and fixes variable names --- pkg/component/component.go | 21 +-- .../adapters/docker/component/utils_test.go | 150 ------------------ pkg/odo/cli/component/status.go | 3 +- pkg/url/kubernetes.go | 12 +- pkg/url/kubernetes_test.go | 6 +- pkg/url/mock_Client.go | 12 +- pkg/url/s2i.go | 12 +- pkg/url/url.go | 29 ++-- pkg/url/url_test.go | 6 +- 9 files changed, 54 insertions(+), 197 deletions(-) diff --git a/pkg/component/component.go b/pkg/component/component.go index 5bc08690d4f..65e5746ffb6 100644 --- a/pkg/component/component.go +++ b/pkg/component/component.go @@ -589,12 +589,9 @@ func ensureAndLogProperResourceUsage(resourceMin, resourceMax *string, resourceN // Parameters: // client: occlient instance // kClient: kclient instance -// appName: Name of application of which the component is a part -// componentName: Name of the component which is being patched with config // componentConfig: Component configuration // envSpecificInfo: Component environment specific information, available if uses devfile // cmpExist: true if components exists in the cluster -// endpointMap: value is devfile endpoint entry, key is the TargetPort for each endpoint entry // isS2I: Legacy option. Set as true if you want to use the old S2I method as it differentiates slightly. // Returns: // err: Errors if any else nil @@ -610,7 +607,7 @@ func ApplyConfig(client *occlient.Client, kClient *kclient.Client, componentConf client.Namespace = kClient.Namespace } - var localConfig localConfigProvider.LocalConfigProvider + var configProvider localConfigProvider.LocalConfigProvider if isS2I { // if component exist then only call the update function if cmpExist { @@ -620,16 +617,10 @@ func ApplyConfig(client *occlient.Client, kClient *kclient.Client, componentConf } } - var componentName string - var applicationName string if isS2I || kClient == nil { - componentName = componentConfig.GetName() - applicationName = componentConfig.GetApplication() - localConfig = &componentConfig + configProvider = &componentConfig } else { - componentName = envSpecificInfo.GetName() - applicationName = envSpecificInfo.GetApplication() - localConfig = &envSpecificInfo + configProvider = &envSpecificInfo } isRouteSupported := false @@ -641,13 +632,11 @@ func ApplyConfig(client *occlient.Client, kClient *kclient.Client, componentConf urlClient := urlpkg.NewClient(urlpkg.ClientOptions{ OCClient: *client, IsRouteSupported: isRouteSupported, - LocalConfigProvider: localConfig, + LocalConfigProvider: configProvider, }) return urlpkg.Push(client, kClient, urlpkg.PushParameters{ - ComponentName: componentName, - ApplicationName: applicationName, - LocalConfig: localConfig, + LocalConfig: configProvider, URLClient: urlClient, IsRouteSupported: isRouteSupported, IsS2I: isS2I, diff --git a/pkg/devfile/adapters/docker/component/utils_test.go b/pkg/devfile/adapters/docker/component/utils_test.go index fb86bfb6926..34ea00f4b48 100644 --- a/pkg/devfile/adapters/docker/component/utils_test.go +++ b/pkg/devfile/adapters/docker/component/utils_test.go @@ -1,19 +1,15 @@ package component import ( - "github.com/openshift/odo/pkg/localConfigProvider" "reflect" "strings" "testing" - "github.com/docker/go-connections/nat" - devfilev1 "github.com/devfile/api/pkg/apis/workspaces/v1alpha2" devfileParser "github.com/devfile/library/pkg/devfile/parser" "github.com/docker/docker/api/types/container" "github.com/docker/docker/api/types/mount" adaptersCommon "github.com/openshift/odo/pkg/devfile/adapters/common" - "github.com/openshift/odo/pkg/envinfo" "github.com/openshift/odo/pkg/lclient" "github.com/openshift/odo/pkg/testingutil" ) @@ -322,152 +318,6 @@ func TestStartContainer(t *testing.T) { } -func TestGenerateAndGetHostConfig(t *testing.T) { - fakeClient := lclient.FakeNew() - //testComponentName := "test" - - endpointName := []string{"8080/tcp", "9090/tcp", "9080/tcp"} - var endpointPort = []int{8080, 9090, 9080} - //var expectPortNameMapping = map[nat.Port]string{ - // nat.Port("8080/tcp"): "url1", - // nat.Port("9090/tcp"): "url2", - // nat.Port("9080/tcp"): "url3", - //} - - tests := []struct { - name string - urlValue []localConfigProvider.LocalURL - expectResult nat.PortMap - client *lclient.Client - endpoints []devfilev1.Endpoint - }{ - { - name: "Case 1: no port mappings", - urlValue: []localConfigProvider.LocalURL{}, - expectResult: nil, - client: fakeClient, - endpoints: []devfilev1.Endpoint{}, - }, - { - name: "Case 2: only one port mapping", - urlValue: []localConfigProvider.LocalURL{ - {Name: "url1", Port: 8080, ExposedPort: 65432}, - }, - expectResult: nat.PortMap{ - "8080/tcp": []nat.PortBinding{ - { - HostIP: LocalhostIP, - HostPort: "65432", - }, - }, - }, - client: fakeClient, - endpoints: []devfilev1.Endpoint{ - { - Name: endpointName[0], - TargetPort: endpointPort[0], - }, - }, - }, - { - name: "Case 3: multiple port mappings", - urlValue: []localConfigProvider.LocalURL{ - {Name: "url1", Port: 8080, ExposedPort: 65432}, - {Name: "url2", Port: 9090, ExposedPort: 54321}, - {Name: "url3", Port: 9080, ExposedPort: 45678}, - }, - expectResult: nat.PortMap{ - "8080/tcp": []nat.PortBinding{ - { - HostIP: LocalhostIP, - HostPort: "65432", - }, - }, - "9090/tcp": []nat.PortBinding{ - { - HostIP: LocalhostIP, - HostPort: "54321", - }, - }, - "9080/tcp": []nat.PortBinding{ - { - HostIP: LocalhostIP, - HostPort: "45678", - }, - }, - }, - client: fakeClient, - endpoints: []devfilev1.Endpoint{ - { - Name: endpointName[0], - TargetPort: endpointPort[0], - }, - { - Name: endpointName[1], - TargetPort: endpointPort[1], - }, - { - Name: endpointName[2], - TargetPort: endpointPort[2], - }, - }, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - - //devObj := devfileParser.DevfileObj{ - // Data: &testingutil.TestDevfileData{ - // Components: []devfilev1.Component{}, - // }, - //} - - //adapterCtx := adaptersCommon.AdapterContext{ - // ComponentName: testComponentName, - // Devfile: devObj, - //} - - esi, err := envinfo.NewEnvSpecificInfo("") - if err != nil { - t.Error(err) - } - for _, url := range tt.urlValue { - err = esi.SetConfiguration("URL", url) - if err != nil { - t.Error(err) - } - } - //componentAdapter := New(adapterCtx, *tt.client) - //hostConfig, portURLNameMapping, err := componentAdapter.generateAndGetHostConfig(tt.endpoints) - //if err != nil { - // t.Error(err) - //} - // - //if len(hostConfig.PortBindings) != len(tt.expectResult) { - // t.Errorf("host config PortBindings length mismatch: actual value %v, expected value %v", len(hostConfig.PortBindings), len(tt.expectResult)) - //} - //if len(hostConfig.PortBindings) != 0 { - // for key, value := range hostConfig.PortBindings { - // if tt.expectResult[key][0].HostIP != value[0].HostIP || tt.expectResult[key][0].HostPort != value[0].HostPort { - // t.Errorf("host config PortBindings mismatch: actual value %v, expected value %v", hostConfig.PortBindings, tt.expectResult) - // } - // } - //} - //if len(portURLNameMapping) != 0 { - // for key, value := range portURLNameMapping { - // if expectPortNameMapping[key] != value { - // t.Errorf("port and urlName mapping mismatch for port %v: actual value %v, expected value %v", key, value, expectPortNameMapping[key]) - // } - // } - //} - err = esi.DeleteEnvInfoFile() - if err != nil { - t.Error(err) - } - }) - } -} - func TestExecDevfile(t *testing.T) { testComponentName := "test" diff --git a/pkg/odo/cli/component/status.go b/pkg/odo/cli/component/status.go index 7877532c855..d086a0fda9c 100644 --- a/pkg/odo/cli/component/status.go +++ b/pkg/odo/cli/component/status.go @@ -70,11 +70,10 @@ func (so *StatusOptions) Complete(name string, cmd *cobra.Command, args []string // If devfile is present if so.isDevfile { - envSpecificInfo, err := envinfo.NewEnvSpecificInfo(so.componentContext) + so.EnvSpecificInfo, err = envinfo.NewEnvSpecificInfo(so.componentContext) if err != nil { return errors.Wrap(err, "unable to retrieve configuration information") } - so.EnvSpecificInfo = envSpecificInfo so.Context = genericclioptions.NewDevfileContext(cmd) // Get the component name diff --git a/pkg/url/kubernetes.go b/pkg/url/kubernetes.go index efe0934faa0..d35845b77fe 100644 --- a/pkg/url/kubernetes.go +++ b/pkg/url/kubernetes.go @@ -12,6 +12,7 @@ import ( "k8s.io/klog" ) +// kubernetesClient contains information required for devfile based URL based operations type kubernetesClient struct { generic isRouteSupported bool @@ -19,7 +20,7 @@ type kubernetesClient struct { } // ListCluster lists both route and ingress based URLs from the cluster -func (k kubernetesClient) ListCluster() (URLList, error) { +func (k kubernetesClient) ListFromCluster() (URLList, error) { labelSelector := fmt.Sprintf("%v=%v", componentlabels.ComponentLabel, k.componentName) klog.V(4).Infof("Listing ingresses with label selector: %v", labelSelector) ingresses, err := k.client.GetKubeClient().ListIngresses(labelSelector) @@ -54,8 +55,9 @@ func (k kubernetesClient) ListCluster() (URLList, error) { // List lists both route/ingress based URLs and local URLs with respective states func (k kubernetesClient) List() (URLList, error) { + // get the URLs present on the cluster clusterURLMap := make(map[string]URL) - clusterURLs, err := k.ListCluster() + clusterURLs, err := k.ListFromCluster() if err != nil { return URLList{}, errors.Wrap(err, "unable to list routes") } @@ -66,6 +68,7 @@ func (k kubernetesClient) List() (URLList, error) { localMap := make(map[string]URL) if k.localConfig != nil { + // get the URLs present on the localConfigProvider localURLS := k.localConfig.ListURLs() for _, url := range localURLS { if !k.isRouteSupported && url.Kind == localConfigProvider.ROUTE { @@ -78,6 +81,9 @@ func (k kubernetesClient) List() (URLList, error) { } } + // find the URLs which are present on the cluster but not on the localConfigProvider + // if not found on the localConfigProvider, mark them as 'StateTypeLocallyDeleted' + // else mark them as 'StateTypePushed' var urls sortableURLs for URLName, clusterURL := range clusterURLMap { _, found := localMap[URLName] @@ -92,6 +98,8 @@ func (k kubernetesClient) List() (URLList, error) { } } + // find the URLs which are present on the localConfigProvider but not on the cluster + // if not found on the cluster, mark them as 'StateTypeNotPushed' for localName, localURL := range localMap { _, remoteURLFound := clusterURLMap[localName] if !remoteURLFound { diff --git a/pkg/url/kubernetes_test.go b/pkg/url/kubernetes_test.go index 5940880ca9e..21567c1e202 100644 --- a/pkg/url/kubernetes_test.go +++ b/pkg/url/kubernetes_test.go @@ -187,13 +187,13 @@ func Test_kubernetesClient_ListCluster(t *testing.T) { isRouteSupported: tt.fields.isRouteSupported, client: *fkocclient, } - got, err := k.ListCluster() + got, err := k.ListFromCluster() if (err != nil) != tt.wantErr { - t.Errorf("ListCluster() error = %v, wantErr %v", err, tt.wantErr) + t.Errorf("ListFromCluster() error = %v, wantErr %v", err, tt.wantErr) return } if !reflect.DeepEqual(got, tt.want) { - t.Errorf("ListCluster() error: %v", pretty.Compare(got, tt.want)) + t.Errorf("ListFromCluster() error: %v", pretty.Compare(got, tt.want)) } }) } diff --git a/pkg/url/mock_Client.go b/pkg/url/mock_Client.go index 5e835c1dacb..6127e39ee9b 100644 --- a/pkg/url/mock_Client.go +++ b/pkg/url/mock_Client.go @@ -32,19 +32,19 @@ func (m *MockClient) EXPECT() *MockClientMockRecorder { return m.recorder } -// ListCluster mocks base method -func (m *MockClient) ListCluster() (URLList, error) { +// ListFromCluster mocks base method +func (m *MockClient) ListFromCluster() (URLList, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "ListCluster") + ret := m.ctrl.Call(m, "ListFromCluster") ret0, _ := ret[0].(URLList) ret1, _ := ret[1].(error) return ret0, ret1 } -// ListCluster indicates an expected call of ListCluster -func (mr *MockClientMockRecorder) ListCluster() *gomock.Call { +// ListFromCluster indicates an expected call of ListFromCluster +func (mr *MockClientMockRecorder) ListFromCluster() *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ListCluster", reflect.TypeOf((*MockClient)(nil).ListCluster)) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ListFromCluster", reflect.TypeOf((*MockClient)(nil).ListFromCluster)) } // List mocks base method diff --git a/pkg/url/s2i.go b/pkg/url/s2i.go index cd3dad405f5..00c652ab77e 100644 --- a/pkg/url/s2i.go +++ b/pkg/url/s2i.go @@ -10,12 +10,14 @@ import ( "k8s.io/klog" ) +// s2iClient contains information required for s2i based URL based operations type s2iClient struct { generic client occlient.Client } -func (s s2iClient) ListCluster() (URLList, error) { +// ListCluster lists route based URLs from the cluster +func (s s2iClient) ListFromCluster() (URLList, error) { labelSelector := fmt.Sprintf("%v=%v", applabels.ApplicationLabel, s.localConfig.GetApplication()) if s.localConfig.GetName() != "" { @@ -31,9 +33,6 @@ func (s s2iClient) ListCluster() (URLList, error) { var urls []URL for _, r := range routes { - if r.OwnerReferences != nil && r.OwnerReferences[0].Kind == "Ingress" { - continue - } a := getMachineReadableFormat(r) urls = append(urls, a) } @@ -42,16 +41,17 @@ func (s s2iClient) ListCluster() (URLList, error) { return urlList, nil } +// List lists both route based URLs and local URLs with respective states func (s s2iClient) List() (URLList, error) { var urls []URL - clusterUrls, err := s.ListCluster() + clusterUrls, err := s.ListFromCluster() if err != nil { return URLList{}, errors.Wrap(err, "unable to list route names") } for _, clusterURL := range clusterUrls.Items { - var found bool = false + var found = false for _, configURL := range s.localConfig.ListURLs() { localURL := ConvertConfigURL(configURL) if localURL.Name == clusterURL.Name { diff --git a/pkg/url/url.go b/pkg/url/url.go index 7ec8f30ff42..b35b4b1084c 100644 --- a/pkg/url/url.go +++ b/pkg/url/url.go @@ -596,8 +596,6 @@ func getMachineReadableFormatDocker(internalPort int, externalPort int, hostIP s } type PushParameters struct { - ComponentName string - ApplicationName string LocalConfig localConfigProvider.LocalConfigProvider URLClient Client IsRouteSupported bool @@ -615,7 +613,14 @@ func Push(client *occlient.Client, kClient *kclient.Client, parameters PushParam log.Warningf("Unable to create ingress, missing host information for Endpoint %v, please check instructions on URL creation (refer `odo url create --help`)\n", url.Name) continue } - urlName := getValidURLName(url.Name) + + // TODO remove once https://github.com/openshift/odo/issues/4060 is fixed + var urlName string + if parameters.IsS2I { + urlName = url.Name + } else { + urlName = getValidURLName(url.Name) + } urlLOCAL[urlName] = ConvertLocalURL(url) } @@ -624,7 +629,7 @@ func Push(client *occlient.Client, kClient *kclient.Client, parameters PushParam urlCLUSTER := make(map[string]URL) // get the URLs on the cluster - urlList, err := parameters.URLClient.ListCluster() + urlList, err := parameters.URLClient.ListFromCluster() if err != nil { return err } @@ -646,6 +651,10 @@ func Push(client *occlient.Client, kClient *kclient.Client, parameters PushParam if val.Spec.Kind == localConfigProvider.INGRESS { val.Spec.Host = fmt.Sprintf("%v.%v", urlName, val.Spec.Host) } else if val.Spec.Kind == localConfigProvider.ROUTE { + // we don't allow the host input for route based URLs + // based removing it for the urls from the cluster to avoid config mismatch + urlSpec.Spec.Host = "" + if val.Spec.Secure { val.Spec.Protocol = "https" } else { @@ -667,9 +676,9 @@ func Push(client *occlient.Client, kClient *kclient.Client, parameters PushParam if !parameters.IsS2I && kClient != nil { // route/ingress name is defined as - // to avoid error due to duplicate ingress name defined in different devfile components - deleteURLName = fmt.Sprintf("%s-%s", urlName, parameters.ComponentName) + deleteURLName = fmt.Sprintf("%s-%s", urlName, parameters.LocalConfig.GetName()) } - err := Delete(client, kClient, deleteURLName, parameters.ApplicationName, urlSpec.Spec.Kind, parameters.IsS2I) + err := Delete(client, kClient, deleteURLName, parameters.LocalConfig.GetApplication(), urlSpec.Spec.Kind, parameters.IsS2I) if err != nil { return err } @@ -691,8 +700,8 @@ func Push(client *occlient.Client, kClient *kclient.Client, parameters PushParam urlName: urlName, portNumber: urlInfo.Spec.Port, secureURL: urlInfo.Spec.Secure, - componentName: parameters.ComponentName, - applicationName: parameters.ApplicationName, + componentName: parameters.LocalConfig.GetName(), + applicationName: parameters.LocalConfig.GetApplication(), host: urlInfo.Spec.Host, secretName: urlInfo.Spec.TLSSecret, urlKind: urlInfo.Spec.Kind, @@ -728,10 +737,11 @@ type ClientOptions struct { } type Client interface { - ListCluster() (URLList, error) + ListFromCluster() (URLList, error) List() (URLList, error) } +// NewClient gets the appropriate URL client based on the parameters func NewClient(options ClientOptions) Client { genericInfo := generic{ appName: options.LocalConfigProvider.GetApplication(), @@ -753,6 +763,7 @@ func NewClient(options ClientOptions) Client { } } +// generic contains information required for all the URL clients type generic struct { appName string componentName string diff --git a/pkg/url/url_test.go b/pkg/url/url_test.go index 1f8303f4eae..e9ad5de46ad 100644 --- a/pkg/url/url_test.go +++ b/pkg/url/url_test.go @@ -1374,10 +1374,12 @@ func TestPush(t *testing.T) { defer ctrl.Finish() mockLocalConfigProvider := localConfigProvider.NewMockLocalConfigProvider(ctrl) + mockLocalConfigProvider.EXPECT().GetName().Return(tt.componentName).AnyTimes() + mockLocalConfigProvider.EXPECT().GetApplication().Return(tt.applicationName).AnyTimes() mockLocalConfigProvider.EXPECT().ListURLs().Return(tt.existingLocalURLs) mockURLClient := NewMockClient(ctrl) - mockURLClient.EXPECT().ListCluster().Return(tt.existingClusterURLs, nil) + mockURLClient.EXPECT().ListFromCluster().Return(tt.existingClusterURLs, nil) fakeClient, fakeClientSet := occlient.FakeNew() fakeKClient, fakeKClientSet := kclient.FakeNew() @@ -1406,8 +1408,6 @@ func TestPush(t *testing.T) { }) if err := Push(fakeClient, fakeKClient, PushParameters{ - ComponentName: tt.componentName, - ApplicationName: tt.applicationName, LocalConfig: mockLocalConfigProvider, URLClient: mockURLClient, IsRouteSupported: tt.args.isRouteSupported, From 0f16b05a0f398d241b9e09ff2df89490918e52fa Mon Sep 17 00:00:00 2001 From: mik-dass Date: Fri, 29 Jan 2021 17:55:12 +0530 Subject: [PATCH 3/3] Fixes occlient error while creating new URL client --- pkg/odo/cli/url/list.go | 2 +- pkg/odo/genericclioptions/context.go | 3 --- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/pkg/odo/cli/url/list.go b/pkg/odo/cli/url/list.go index 77d8020940c..d0b94c8e381 100644 --- a/pkg/odo/cli/url/list.go +++ b/pkg/odo/cli/url/list.go @@ -60,7 +60,7 @@ func (o *ListOptions) Complete(name string, cmd *cobra.Command, args []string) ( o.client = url.NewClient(url.ClientOptions{ LocalConfigProvider: o.Context.LocalConfigProvider, - OCClient: *genericclioptions.Client(cmd), + OCClient: *o.Context.Client, IsRouteSupported: routeSupported, }) return diff --git a/pkg/odo/genericclioptions/context.go b/pkg/odo/genericclioptions/context.go index 076bf42a6aa..6b0645c584b 100644 --- a/pkg/odo/genericclioptions/context.go +++ b/pkg/odo/genericclioptions/context.go @@ -93,9 +93,6 @@ func New(parameters CreateParameters, toggles ...bool) (context *Context, err er context.ComponentContext = parameters.ComponentContext } - context = NewContext(parameters.Cmd) - context.ComponentContext = parameters.ComponentContext - err = context.InitConfigFromContext() if err != nil { return nil, err