Automatically use in-cluster Devfile registries if any (redhat-develo…

…per#6622) * Add kubeclient as dependency of the registry client Co-authored-by: Philippe Martin <[email protected]> * Add GetRegistryList method to the kube client interface This is yet to be implemented. Co-authored-by: Philippe Martin <[email protected]> * Implement GetRegistryList * adding test if devfileRegistryListCR is present in cluster Signed-off-by: anandrkskd <[email protected]> * Unit tests (to be continued) * Add unit test cases against kclient#GetRegistryList() Co-authored-by: Philippe Martin <[email protected]> * 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 <[email protected]> * Use proxy when available Signed-off-by: Parthvi Vala <[email protected]> Co-authored-by: Armel Soro <[email protected]> * 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 <[email protected]> Signed-off-by: Parthvi Vala <[email protected]> Co-authored-by: Philippe Martin <[email protected]> Co-authored-by: anandrkskd <[email protected]> Co-authored-by: Parthvi Vala <[email protected]>
anandrkskd · Mar 7, 2023 · 0817608 · 0817608
1 parent 1e834d8
commit 0817608
Show file tree

Hide file tree

Showing 29 changed files with 2,275 additions and 199 deletions.
diff --git a/pkg/api/preference.go b/pkg/api/preference.go
@@ -1,8 +1,18 @@
 package api
 
-import "github.com/redhat-developer/odo/pkg/preference"
+type PreferenceList struct {
+	Items []PreferenceItem `json:"items,omitempty"`
+}
+
+type PreferenceItem struct {
+	Name        string      `json:"name"`
+	Value       interface{} `json:"value"`       // The value set by the user, this will be nil if the user hasn't set it
+	Default     interface{} `json:"default"`     // default value of the preference if the user hasn't set the value
+	Type        string      `json:"type"`        // the type of the preference, possible values int, string, bool
+	Description string      `json:"description"` // The description of the preference
+}
 
 type PreferenceView struct {
-	Preferences []preference.PreferenceItem `json:"preferences,omitempty"`
-	Registries  []preference.Registry       `json:"registries,omitempty"`
+	Preferences []PreferenceItem `json:"preferences,omitempty"`
+	Registries  []Registry       `json:"registries,omitempty"`
 }
diff --git a/pkg/init/backend/flags.go b/pkg/init/backend/flags.go
@@ -14,7 +14,6 @@ import (
 
 	"github.com/redhat-developer/odo/pkg/api"
 	"github.com/redhat-developer/odo/pkg/devfile/location"
-	"github.com/redhat-developer/odo/pkg/preference"
 	"github.com/redhat-developer/odo/pkg/testingutil/filesystem"
 )
 
@@ -29,14 +28,14 @@ const (
 
 // FlagsBackend is a backend that will extract all needed information from flags passed to the command
 type FlagsBackend struct {
-	preferenceClient preference.Client
+	registryClient registry.Client
 }
 
 var _ InitBackend = (*FlagsBackend)(nil)
 
-func NewFlagsBackend(preferenceClient preference.Client) *FlagsBackend {
+func NewFlagsBackend(registryClient registry.Client) *FlagsBackend {
 	return &FlagsBackend{
-		preferenceClient: preferenceClient,
+		registryClient: registryClient,
 	}
 }
 
@@ -51,23 +50,31 @@ func (o *FlagsBackend) Validate(flags map[string]string, fs filesystem.Filesyste
 		return errors.New("only one of --devfile or --devfile-path parameter should be specified")
 	}
 
-	if flags[FLAG_DEVFILE_REGISTRY] != "" {
-		if !o.preferenceClient.RegistryNameExists(flags[FLAG_DEVFILE_REGISTRY]) {
-			return fmt.Errorf("registry %q not found in the list of devfile registries. Please use `odo preference <add/remove> registry` command to configure devfile registries", flags[FLAG_DEVFILE_REGISTRY])
+	registryName := flags[FLAG_DEVFILE_REGISTRY]
+	if registryName != "" {
+		registries, err := o.registryClient.GetDevfileRegistries(registryName)
+		if err != nil {
+			return err
+		}
+		if len(registries) == 0 {
+			//revive:disable:error-strings This is a top-level error message displayed as is to the end user
+			return fmt.Errorf(`Registry %q not found in the list of devfile registries.
+Please use 'odo preference <add/remove> registry'' command to configure devfile registries or add an in-cluster registry (see https://devfile.io/docs/2.2.0/deploying-a-devfile-registry).`,
+				registryName)
+			//revive:enable:error-strings
 		}
-		registries := o.preferenceClient.RegistryList()
 		for _, r := range registries {
 			isGithubRegistry, err := registry.IsGithubBasedRegistry(r.URL)
 			if err != nil {
 				return err
 			}
-			if r.Name == flags[FLAG_DEVFILE_REGISTRY] && isGithubRegistry {
+			if r.Name == registryName && isGithubRegistry {
 				return &registry.ErrGithubRegistryNotSupported{}
 			}
 		}
 	}
 
-	if flags[FLAG_DEVFILE_PATH] != "" && flags[FLAG_DEVFILE_REGISTRY] != "" {
+	if flags[FLAG_DEVFILE_PATH] != "" && registryName != "" {
 		return errors.New("--devfile-registry parameter cannot be used with --devfile-path")
 	}
 

diff --git a/pkg/init/backend/flags_test.go b/pkg/init/backend/flags_test.go
@@ -14,7 +14,7 @@ import (
 	dffilesystem "github.com/devfile/library/v2/pkg/testingutil/filesystem"
 
 	"github.com/redhat-developer/odo/pkg/api"
-	"github.com/redhat-developer/odo/pkg/preference"
+	"github.com/redhat-developer/odo/pkg/registry"
 	"github.com/redhat-developer/odo/pkg/testingutil/filesystem"
 )
 
@@ -70,12 +70,11 @@ func TestFlagsBackend_Validate(t *testing.T) {
 		dir   string
 	}
 	tests := []struct {
-		name               string
-		fields             fields
-		args               args
-		registryNameExists bool
-		registryList       []preference.Registry
-		wantErr            bool
+		name         string
+		fields       fields
+		args         args
+		registryList []api.Registry
+		wantErr      bool
 	}{
 		{
 			name: "no name passed",
@@ -121,13 +120,12 @@ func TestFlagsBackend_Validate(t *testing.T) {
 				},
 				dir: "/tmp",
 			},
-			registryList: []preference.Registry{
+			registryList: []api.Registry{
 				{
 					Name: "aregistry",
 				},
 			},
-			registryNameExists: true,
-			wantErr:            false,
+			wantErr: false,
 		},
 		{
 			name: "devfile and devfile-path passed",
@@ -161,8 +159,12 @@ func TestFlagsBackend_Validate(t *testing.T) {
 				},
 				dir: "/tmp",
 			},
-			registryNameExists: true,
-			wantErr:            false,
+			registryList: []api.Registry{
+				{
+					Name: "aregistry",
+				},
+			},
+			wantErr: false,
 		},
 		{
 			name: "devfile and devfile-registry passed with non existing registry",
@@ -179,8 +181,7 @@ func TestFlagsBackend_Validate(t *testing.T) {
 				},
 				dir: "/tmp",
 			},
-			registryNameExists: false,
-			wantErr:            true,
+			wantErr: true,
 		},
 		{
 			name: "devfile-path and devfile-registry passed",
@@ -197,8 +198,12 @@ func TestFlagsBackend_Validate(t *testing.T) {
 				},
 				dir: "/tmp",
 			},
-			registryNameExists: true,
-			wantErr:            true,
+			registryList: []api.Registry{
+				{
+					Name: "aregistry",
+				},
+			},
+			wantErr: true,
 		},
 		{
 			name: "numeric name",
@@ -272,12 +277,12 @@ func TestFlagsBackend_Validate(t *testing.T) {
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			ctrl := gomock.NewController(t)
-			prefClient := preference.NewMockClient(ctrl)
-			prefClient.EXPECT().RegistryNameExists(gomock.Any()).Return(tt.registryNameExists).AnyTimes()
-			prefClient.EXPECT().RegistryList().Return(tt.registryList).AnyTimes()
+
+			registryClient := registry.NewMockClient(ctrl)
+			registryClient.EXPECT().GetDevfileRegistries(gomock.Eq(tt.args.flags[FLAG_DEVFILE_REGISTRY])).Return(tt.registryList, nil).AnyTimes()
 
 			o := &FlagsBackend{
-				preferenceClient: prefClient,
+				registryClient: registryClient,
 			}
 			if err := o.Validate(tt.args.flags, tt.args.fsys(), tt.args.dir); (err != nil) != tt.wantErr {
 				t.Errorf("FlagsBackend.Validate() error = %v, wantErr %v", err, tt.wantErr)
@@ -288,7 +293,7 @@ func TestFlagsBackend_Validate(t *testing.T) {
 
 func TestFlagsBackend_SelectStarterProject(t *testing.T) {
 	type fields struct {
-		preferenceClient preference.Client
+		registryClient registry.Client
 	}
 	type args struct {
 		devfile func() parser.DevfileObj
@@ -374,7 +379,7 @@ func TestFlagsBackend_SelectStarterProject(t *testing.T) {
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			o := &FlagsBackend{
-				preferenceClient: tt.fields.preferenceClient,
+				registryClient: tt.fields.registryClient,
 			}
 			got1, err := o.SelectStarterProject(tt.args.devfile(), tt.args.flags)
 			if (err != nil) != tt.wantErr {
@@ -390,7 +395,7 @@ func TestFlagsBackend_SelectStarterProject(t *testing.T) {
 
 func TestFlagsBackend_PersonalizeName(t *testing.T) {
 	type fields struct {
-		preferenceClient preference.Client
+		registryClient registry.Client
 	}
 	type args struct {
 		devfile func(fs dffilesystem.Filesystem) parser.DevfileObj
@@ -449,7 +454,7 @@ func TestFlagsBackend_PersonalizeName(t *testing.T) {
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			o := &FlagsBackend{
-				preferenceClient: tt.fields.preferenceClient,
+				registryClient: tt.fields.registryClient,
 			}
 			fs := dffilesystem.NewFakeFs()
 			newName, err := o.PersonalizeName(tt.args.devfile(fs), tt.args.flags)

diff --git a/pkg/init/init.go b/pkg/init/init.go
@@ -44,7 +44,7 @@ func NewInitClient(fsys filesystem.Filesystem, preferenceClient preference.Clien
 	// We create the asker client and the backends here and not at the CLI level, as we want to hide these details to the CLI
 	askerClient := asker.NewSurveyAsker()
 	return &InitClient{
-		flagsBackend:       backend.NewFlagsBackend(preferenceClient),
+		flagsBackend:       backend.NewFlagsBackend(registryClient),
 		interactiveBackend: backend.NewInteractiveBackend(askerClient, registryClient, alizerClient),
 		alizerBackend:      backend.NewAlizerBackend(askerClient, alizerClient),
 		fsys:               fsys,
@@ -182,9 +182,11 @@ func (o *InitClient) downloadFromRegistry(ctx context.Context, registryName stri
 	}
 	defer downloadSpinner.End(false)
 
-	registries := o.preferenceClient.RegistryList()
-	var reg preference.Registry
-	for _, reg = range registries {
+	registries, err := o.registryClient.GetDevfileRegistries(registryName)
+	if err != nil {
+		return err
+	}
+	for _, reg := range registries {
 		if forceRegistry && reg.Name == registryName {
 			err := o.registryClient.PullStackFromRegistry(reg.URL, devfile, dest, registryOptions)
 			if err != nil {

diff --git a/pkg/init/init_test.go b/pkg/init/init_test.go
@@ -37,7 +37,11 @@ func TestInitClient_downloadFromRegistry(t *testing.T) {
 			fields: fields{
 				preferenceClient: func(ctrl *gomock.Controller) preference.Client {
 					client := preference.NewMockClient(ctrl)
-					registryList := []preference.Registry{
+					return client
+				},
+				registryClient: func(ctrl *gomock.Controller) registry.Client {
+					client := registry.NewMockClient(ctrl)
+					registryList := []api.Registry{
 						{
 							Name: "Registry0",
 							URL:  "http://registry0",
@@ -47,11 +51,7 @@ func TestInitClient_downloadFromRegistry(t *testing.T) {
 							URL:  "http://registry1",
 						},
 					}
-					client.EXPECT().RegistryList().Return(registryList)
-					return client
-				},
-				registryClient: func(ctrl *gomock.Controller) registry.Client {
-					client := registry.NewMockClient(ctrl)
+					client.EXPECT().GetDevfileRegistries(gomock.Eq("Registry1")).Return(registryList, nil).Times(1)
 					client.EXPECT().PullStackFromRegistry("http://registry1", "java", gomock.Any(), gomock.Any()).Return(nil).Times(1)
 					return client
 				},
@@ -68,7 +68,11 @@ func TestInitClient_downloadFromRegistry(t *testing.T) {
 			fields: fields{
 				preferenceClient: func(ctrl *gomock.Controller) preference.Client {
 					client := preference.NewMockClient(ctrl)
-					registryList := []preference.Registry{
+					return client
+				},
+				registryClient: func(ctrl *gomock.Controller) registry.Client {
+					client := registry.NewMockClient(ctrl)
+					registryList := []api.Registry{
 						{
 							Name: "Registry0",
 							URL:  "http://registry0",
@@ -78,11 +82,7 @@ func TestInitClient_downloadFromRegistry(t *testing.T) {
 							URL:  "http://registry1",
 						},
 					}
-					client.EXPECT().RegistryList().Return(registryList)
-					return client
-				},
-				registryClient: func(ctrl *gomock.Controller) registry.Client {
-					client := registry.NewMockClient(ctrl)
+					client.EXPECT().GetDevfileRegistries(gomock.Eq("Registry1")).Return(registryList, nil).Times(1)
 					client.EXPECT().PullStackFromRegistry("http://registry1", "java", gomock.Any(), gomock.Any()).Return(errors.New("")).Times(1)
 					return client
 				},
@@ -98,8 +98,11 @@ func TestInitClient_downloadFromRegistry(t *testing.T) {
 			name: "Download devfile from all registries where devfile is present in second registry",
 			fields: fields{
 				preferenceClient: func(ctrl *gomock.Controller) preference.Client {
-					client := preference.NewMockClient(ctrl)
-					registryList := []preference.Registry{
+					return preference.NewMockClient(ctrl)
+				},
+				registryClient: func(ctrl *gomock.Controller) registry.Client {
+					client := registry.NewMockClient(ctrl)
+					registryList := []api.Registry{
 						{
 							Name: "Registry0",
 							URL:  "http://registry0",
@@ -109,11 +112,7 @@ func TestInitClient_downloadFromRegistry(t *testing.T) {
 							URL:  "http://registry1",
 						},
 					}
-					client.EXPECT().RegistryList().Return(registryList)
-					return client
-				},
-				registryClient: func(ctrl *gomock.Controller) registry.Client {
-					client := registry.NewMockClient(ctrl)
+					client.EXPECT().GetDevfileRegistries(gomock.Eq("")).Return(registryList, nil).Times(1)
 					client.EXPECT().PullStackFromRegistry("http://registry0", "java", gomock.Any(), gomock.Any()).Return(errors.New("")).Times(1)
 					client.EXPECT().PullStackFromRegistry("http://registry1", "java", gomock.Any(), gomock.Any()).Return(nil).Times(1)
 					return client
@@ -131,7 +130,11 @@ func TestInitClient_downloadFromRegistry(t *testing.T) {
 			fields: fields{
 				preferenceClient: func(ctrl *gomock.Controller) preference.Client {
 					client := preference.NewMockClient(ctrl)
-					registryList := []preference.Registry{
+					return client
+				},
+				registryClient: func(ctrl *gomock.Controller) registry.Client {
+					client := registry.NewMockClient(ctrl)
+					registryList := []api.Registry{
 						{
 							Name: "Registry0",
 							URL:  "http://registry0",
@@ -141,11 +144,7 @@ func TestInitClient_downloadFromRegistry(t *testing.T) {
 							URL:  "http://registry1",
 						},
 					}
-					client.EXPECT().RegistryList().Return(registryList)
-					return client
-				},
-				registryClient: func(ctrl *gomock.Controller) registry.Client {
-					client := registry.NewMockClient(ctrl)
+					client.EXPECT().GetDevfileRegistries(gomock.Eq("")).Return(registryList, nil).Times(1)
 					client.EXPECT().PullStackFromRegistry("http://registry0", "java", gomock.Any(), gomock.Any()).Return(errors.New("")).Times(1)
 					client.EXPECT().PullStackFromRegistry("http://registry1", "java", gomock.Any(), gomock.Any()).Return(errors.New("")).Times(1)
 					return client

diff --git a/pkg/kclient/dynamic.go b/pkg/kclient/dynamic.go
@@ -85,6 +85,31 @@ func (c *Client) ListDynamicResources(namespace string, gvr schema.GroupVersionR
 	return list, nil
 }
 
+// ListClusterWideDynamicResources returns an unstructured list of instances of a Custom
+// Resource currently deployed cluster-wide in the cluster.
+// If a selector is passed, then it will be used as a label selector to list the resources.
+func (c *Client) ListClusterWideDynamicResources(gvr schema.GroupVersionResource, selector string) (*unstructured.UnstructuredList, error) {
+
+	if c.DynamicClient == nil {
+		return nil, nil
+	}
+
+	listOptions := metav1.ListOptions{}
+	if selector != "" {
+		listOptions.LabelSelector = selector
+	}
+
+	list, err := c.DynamicClient.Resource(gvr).List(context.TODO(), listOptions)
+	if err != nil {
+		if kerrors.IsNotFound(err) {
+			return &unstructured.UnstructuredList{}, nil
+		}
+		return nil, err
+	}
+
+	return list, nil
+}
+
 // GetDynamicResource returns an unstructured instance of a Custom Resource currently deployed in the active namespace
 func (c *Client) GetDynamicResource(gvr schema.GroupVersionResource, name string) (*unstructured.Unstructured, error) {
 	res, err := c.DynamicClient.Resource(gvr).Namespace(c.Namespace).Get(context.TODO(), name, metav1.GetOptions{})