Automatically use in-cluster Devfile registries if any (#6622)

* Add kubeclient as dependency of the registry client Co-authored-by: Philippe Martin <phmartin@redhat.com> * Add GetRegistryList method to the kube client interface This is yet to be implemented. Co-authored-by: Philippe Martin <phmartin@redhat.com> * Implement GetRegistryList * adding test if devfileRegistryListCR is present in cluster Signed-off-by: anandrkskd <anandrkskd@gmail.com> * Unit tests (to be continued) * Add unit test cases against kclient#GetRegistryList() Co-authored-by: Philippe Martin <phmartin@redhat.com> * Ignore in-cluster registries with an empty URL This should ideally not happen if the registry operator is installed in the cluster (because it validates the URL to make sure it is reachable), but you never know ;-) * Update error message when trying to remove registry Registries might be found in the cluster. * Pass isSecure value to the registry handler * Make it possible to use in-cluster registries when calling 'odo registry --details' * Remove unused 'preferenceClient' from registry.getRegistryStacks * Handle in-cluster registries in 'odo init' non-interactive mode * Handle in-cluster registries in 'odo init' interactive mode * Add integration test for odo init --devfile-registry Signed-off-by: Parthvi Vala <pvala@redhat.com> * Use proxy when available Signed-off-by: Parthvi Vala <pvala@redhat.com> Co-authored-by: Armel Soro <armel@rm3l.org> * Make sure tests work even if the registry operator is installed in the cluster or if there are cluster-wide registry lists * Add tests for 'odo init' interactive mode * Remove useless CR file CRs are now dynamically created and applied from the tests * fixup! Add tests for 'odo init' interactive mode --------- Signed-off-by: anandrkskd <anandrkskd@gmail.com> Signed-off-by: Parthvi Vala <pvala@redhat.com> Co-authored-by: Philippe Martin <phmartin@redhat.com> Co-authored-by: anandrkskd <anandrkskd@gmail.com> Co-authored-by: Parthvi Vala <pvala@redhat.com>
2025-10-19 03:06:19 +03:00 · 2023-03-06 09:19:02 +01:00
parent 29b5a38f88
commit 4fb7308cfa
29 changed files with 2274 additions and 198 deletions
--- a/pkg/registry/registry.go
+++ b/pkg/registry/registry.go
@@ -18,6 +18,7 @@ import (
 	"github.com/redhat-developer/odo/pkg/api"
 	"github.com/redhat-developer/odo/pkg/devfile"
 	"github.com/redhat-developer/odo/pkg/devfile/location"
+	"github.com/redhat-developer/odo/pkg/kclient"
 	"github.com/redhat-developer/odo/pkg/log"
 	"github.com/redhat-developer/odo/pkg/preference"
 	"github.com/redhat-developer/odo/pkg/segment"
@@ -28,14 +29,16 @@ import (
 type RegistryClient struct {
 	fsys             filesystem.Filesystem
 	preferenceClient preference.Client
+	kubeClient       kclient.ClientInterface
 }

 var _ Client = (*RegistryClient)(nil)

-func NewRegistryClient(fsys filesystem.Filesystem, preferenceClient preference.Client) RegistryClient {
+func NewRegistryClient(fsys filesystem.Filesystem, preferenceClient preference.Client, kubeClient kclient.ClientInterface) RegistryClient {
 	return RegistryClient{
 		fsys:             fsys,
 		preferenceClient: preferenceClient,
+		kubeClient:       kubeClient,
 	}
 }

@@ -59,36 +62,41 @@ func (o RegistryClient) DownloadStarterProject(starterProject *devfilev1.Starter
 // GetDevfileRegistries gets devfile registries from preference file,
 // if registry name is specified return the specific registry, otherwise return all registries
 func (o RegistryClient) GetDevfileRegistries(registryName string) ([]api.Registry, error) {
-	var devfileRegistries []api.Registry
+	var allRegistries []api.Registry

-	hasName := len(registryName) != 0
-	if o.preferenceClient.RegistryList() != nil {
-		registryList := o.preferenceClient.RegistryList()
-		for _, registry := range registryList {
-			if hasName {
-				if registryName == registry.Name {
-					reg := api.Registry{
-						Name:   registry.Name,
-						URL:    registry.URL,
-						Secure: registry.Secure,
-					}
-					devfileRegistries = append(devfileRegistries, reg)
-					return devfileRegistries, nil
-				}
-			} else {
+	if o.kubeClient != nil {
+		clusterRegistries, err := o.kubeClient.GetRegistryList()
+		if err != nil {
+			return nil, err
+		}
+		allRegistries = append(allRegistries, clusterRegistries...)
+	}
+	allRegistries = append(allRegistries, o.preferenceClient.RegistryList()...)
+
+	hasName := registryName != ""
+	var result []api.Registry
+	for _, registry := range allRegistries {
+		if hasName {
+			if registryName == registry.Name {
 				reg := api.Registry{
 					Name:   registry.Name,
 					URL:    registry.URL,
 					Secure: registry.Secure,
 				}
-				devfileRegistries = append(devfileRegistries, reg)
+				result = append(result, reg)
+				return result, nil
 			}
+			continue
 		}
-	} else {
-		return nil, nil
+		reg := api.Registry{
+			Name:   registry.Name,
+			URL:    registry.URL,
+			Secure: registry.Secure,
+		}
+		result = append(result, reg)
 	}

-	return devfileRegistries, nil
+	return result, nil
 }

 // ListDevfileStacks lists all the available devfile stacks in devfile registry
@@ -118,7 +126,7 @@ func (o RegistryClient) ListDevfileStacks(ctx context.Context, registryName, dev
 		registry := reg                 // Needed to prevent the lambda from capturing the value
 		registryPriority := regPriority // Needed to prevent the lambda from capturing the value
 		retrieveRegistryIndices.Add(util.ConcurrentTask{ToRun: func(errChannel chan error) {
-			registryDevfiles, err := getRegistryStacks(ctx, o.preferenceClient, registry)
+			registryDevfiles, err := getRegistryStacks(ctx, registry)
 			if err != nil {
 				log.Warningf("Registry %s is not set up properly with error: %v, please check the registry URL, and credential and remove add the registry again (refer to `odo preference add registry --help`)\n", registry.Name, err)
 				return
@@ -188,7 +196,7 @@ func (o RegistryClient) ListDevfileStacks(ctx context.Context, registryName, dev
 }

 // getRegistryStacks retrieves the registry's index devfile stack entries
-func getRegistryStacks(ctx context.Context, preferenceClient preference.Client, registry api.Registry) ([]api.DevfileStack, error) {
+func getRegistryStacks(ctx context.Context, registry api.Registry) ([]api.DevfileStack, error) {
 	isGithubregistry, err := IsGithubBasedRegistry(registry.URL)
 	if err != nil {
 		return nil, err
@@ -267,8 +275,10 @@ func (o RegistryClient) retrieveDevfileDataFromRegistry(ctx context.Context, reg
 	}
 	defer os.Remove(tmpFile)

-	registries := o.preferenceClient.RegistryList()
-	var reg preference.Registry
+	registries, err := o.GetDevfileRegistries(registryName)
+	if err != nil {
+		return api.DevfileData{}, err
+	}
 	registryOptions := segment.GetRegistryOptions(ctx)
 	registryOptions.NewIndexSchema = true
 	// Get the file and save it to the temporary file
@@ -277,7 +287,7 @@ func (o RegistryClient) retrieveDevfileDataFromRegistry(ctx context.Context, reg
 	// 2. The devfile api library does not support saving in memory
 	// 3. We need to get the file from the registry and save it to the temporary file
 	// 4. We need to read the file from the temporary file, unmarshal it and then return the devfile data
-	for _, reg = range registries {
+	for _, reg := range registries {
 		if reg.Name == registryName {
 			err = o.PullStackFromRegistry(reg.URL, devfileName, tmpFile, registryOptions)
 			if err != nil {
--- a/pkg/registry/registry_test.go
+++ b/pkg/registry/registry_test.go
@@ -2,6 +2,7 @@ package registry

 import (
 	"context"
+	"errors"
 	"net/http"
 	"net/http/httptest"
 	"os"
@@ -14,6 +15,7 @@ import (
 	"github.com/redhat-developer/odo/pkg/api"
 	"github.com/redhat-developer/odo/pkg/config"
 	envcontext "github.com/redhat-developer/odo/pkg/config/context"
+	"github.com/redhat-developer/odo/pkg/kclient"
 	"github.com/redhat-developer/odo/pkg/preference"
 	"github.com/redhat-developer/odo/pkg/testingutil/filesystem"
 )
@@ -43,7 +45,9 @@ OdoSettings:
 	tests := []struct {
 		name         string
 		registryName string
+		kclient      func(ctrl *gomock.Controller) kclient.ClientInterface
 		want         []api.Registry
+		wantErr      bool
 	}{
 		{
 			name:         "Case 1: Test get all devfile registries",
@@ -72,6 +76,57 @@ OdoSettings:
 				},
 			},
 		},
+		{
+			name: "with devfileRegistrylists in cluster",
+			kclient: func(ctrl *gomock.Controller) kclient.ClientInterface {
+				result := kclient.NewMockClientInterface(ctrl)
+				list := []api.Registry{
+					{
+						Name:   "secure-name",
+						URL:    "secure-url",
+						Secure: true,
+					},
+					{
+						Name:   "unsecure-name",
+						URL:    "unsecure-url",
+						Secure: false,
+					},
+				}
+				result.EXPECT().GetRegistryList().Return(list, nil)
+				return result
+			},
+			want: []api.Registry{
+				{
+					Name:   "secure-name",
+					URL:    "secure-url",
+					Secure: true,
+				},
+				{
+					Name:   "unsecure-name",
+					URL:    "unsecure-url",
+					Secure: false,
+				},
+				{
+					Name:   "CheDevfileRegistry",
+					URL:    "https://che-devfile-registry.openshift.io/",
+					Secure: false,
+				},
+				{
+					Name:   "DefaultDevfileRegistry",
+					URL:    "https://registry.devfile.io",
+					Secure: false,
+				},
+			},
+		},
+		{
+			name: "error getting devfileRegistrylists from cluster",
+			kclient: func(ctrl *gomock.Controller) kclient.ClientInterface {
+				result := kclient.NewMockClientInterface(ctrl)
+				result.EXPECT().GetRegistryList().Return(nil, errors.New("an error"))
+				return result
+			},
+			wantErr: true,
+		},
 	}

 	for _, tt := range tests {
@@ -80,11 +135,20 @@ OdoSettings:
 			ctx = envcontext.WithEnvConfig(ctx, config.Configuration{
 				Globalodoconfig: &tempConfigFileName,
 			})
+
+			ctrl := gomock.NewController(t)
+
 			prefClient, _ := preference.NewClient(ctx)
-			catClient := NewRegistryClient(filesystem.NewFakeFs(), prefClient)
+			// TODO(rm3l) Test with both nil and non-nil kubeclient
+			var kc kclient.ClientInterface
+			if tt.kclient != nil {
+				kc = tt.kclient(ctrl)
+			}
+			catClient := NewRegistryClient(filesystem.NewFakeFs(), prefClient, kc)
 			got, err := catClient.GetDevfileRegistries(tt.registryName)
-			if err != nil {
-				t.Errorf("Error message is %v", err)
+
+			if tt.wantErr != (err != nil) {
+				t.Errorf("error = %v, wantErr %v", err, tt.wantErr)
 			}

 			if diff := cmp.Diff(tt.want, got); diff != "" {
@@ -253,13 +317,14 @@ func TestListDevfileStacks(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			ctrl := gomock.NewController(t)
 			prefClient := preference.NewMockClient(ctrl)
-			prefClient.EXPECT().RegistryList().Return([]preference.Registry{
+			prefClient.EXPECT().RegistryList().Return([]api.Registry{
 				{
 					Name: "TestRegistry",
 					URL:  server.URL,
 				},
 			}).AnyTimes()
-			catClient := NewRegistryClient(filesystem.NewFakeFs(), prefClient)
+			// TODO(rm3l) Test with both nil and non-nil kubeclient
+			catClient := NewRegistryClient(filesystem.NewFakeFs(), prefClient, nil)
 			ctx := context.Background()
 			ctx = envcontext.WithEnvConfig(ctx, config.Configuration{})
 			got, err := catClient.ListDevfileStacks(ctx, tt.registryName, tt.devfileName, tt.filter, false)
@@ -521,15 +586,13 @@ func TestGetRegistryDevfiles(t *testing.T) {

 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			ctrl := gomock.NewController(t)
-			prefClient := preference.NewMockClient(ctrl)
 			ctx := envcontext.WithEnvConfig(context.Background(), config.Configuration{})
 			server, url := tt.registryServerProvider(t)
 			if server != nil {
 				defer server.Close()
 			}

-			got, err := getRegistryStacks(ctx, prefClient, api.Registry{Name: registryName, URL: url})
+			got, err := getRegistryStacks(ctx, api.Registry{Name: registryName, URL: url})

 			if tt.wantErr != (err != nil) {
 				t.Errorf("error = %v, wantErr %v", err, tt.wantErr)