Removing support for github based registries (#5708)

* Removing support for github based registries

Signed-off-by: Mohammed Zeeshan Ahmed <mohammed.zee1000@gmail.com>

* Updating tests

Signed-off-by: Mohammed Zeeshan Ahmed <mohammed.zee1000@gmail.com>

* Renaming some funcs

Signed-off-by: Mohammed Zeeshan Ahmed <mohammed.zee1000@gmail.com>

* Adding https://github.com/devfile/registry-support info

Signed-off-by: Mohammed Zeeshan Ahmed <mohammed.zee1000@gmail.com>

* Commenting out unused const

Signed-off-by: Mohammed Zeeshan Ahmed <mohammed.zee1000@gmail.com>

* Removing potential for github registry use in odo init

Signed-off-by: Mohammed Zeeshan Ahmed <mohammed.zee1000@gmail.com>

* Removing unused constant

Signed-off-by: Mohammed Zeeshan Ahmed <mohammed.zee1000@gmail.com>

* Reversing order

Signed-off-by: Mohammed Zeeshan Ahmed <mohammed.zee1000@gmail.com>

* Making error more readable

Signed-off-by: Mohammed Zeeshan Ahmed <mohammed.zee1000@gmail.com>

* Simpifying test

Signed-off-by: Mohammed Zeeshan Ahmed <mohammed.zee1000@gmail.com>

* Fixing as per @armels comments

Signed-off-by: Mohammed Zeeshan Ahmed <mohammed.zee1000@gmail.com>

* Update pkg/odo/cli/preference/registry/util/util.go

Co-authored-by: Armel Soro <armel@rm3l.org>

* Updating uts

Signed-off-by: Mohammed Zeeshan Ahmed <mohammed.zee1000@gmail.com>

Co-authored-by: Armel Soro <armel@rm3l.org>

pkg/init/backend/flags.go

@@ -3,6 +3,7 @@ package backend
 import (
 	"errors"
 	"fmt"
+	"github.com/redhat-developer/odo/pkg/odo/cli/preference/registry/util"
 
 	"github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2"
 	"github.com/devfile/library/pkg/devfile/parser"
@@ -45,8 +46,20 @@ 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] != "" && !o.preferenceClient.RegistryNameExists(flags[FLAG_DEVFILE_REGISTRY]) {
-		return fmt.Errorf("registry %q not found in the list of devfile registries. Please use `odo preference registry` command to configure devfile registries", flags[FLAG_DEVFILE_REGISTRY])
+	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 registry` command to configure devfile registries", flags[FLAG_DEVFILE_REGISTRY])
+		}
+		registries := o.preferenceClient.RegistryList()
+		for _, r := range *registries {
+			isGithubRegistry, err := util.IsGithubBasedRegistry(r.URL)
+			if err != nil {
+				return err
+			}
+			if r.Name == flags[FLAG_DEVFILE_REGISTRY] && isGithubRegistry {
+				return util.ErrGithubRegistryNotSupported
+			}
+		}
 	}
 
 	if flags[FLAG_DEVFILE_PATH] != "" && flags[FLAG_DEVFILE_REGISTRY] != "" {

pkg/odo/cli/preference/registry/add.go

@@ -73,8 +73,12 @@ func (o *AddOptions) Validate() (err error) {
 	if err != nil {
 		return err
 	}
-	if util2.IsGitBasedRegistry(o.registryURL) {
-		util2.PrintGitRegistryDeprecationWarning()
+	isGithubRegistry, err := util2.IsGithubBasedRegistry(o.registryURL)
+	if err != nil {
+		return err
+	}
+	if isGithubRegistry {
+		return util2.ErrGithubRegistryNotSupported
 	}
 	return nil
 }

pkg/odo/cli/preference/registry/list.go

@@ -15,8 +15,6 @@ import (
 
 	// Third-party packages
 
-	// odo packages
-	"github.com/redhat-developer/odo/pkg/odo/cli/preference/registry/util"
 	"github.com/redhat-developer/odo/pkg/odo/cmdline"
 	"github.com/redhat-developer/odo/pkg/odo/genericclioptions"
 	"github.com/redhat-developer/odo/pkg/odo/genericclioptions/clientset"
@@ -38,8 +36,6 @@ var (
 type ListOptions struct {
 	// Clients
 	clientset *clientset.Clientset
-
-	printGitRegistryDeprecationWarning bool
 }
 
 // NewListOptions creates a new ListOptions instance
@@ -74,9 +70,6 @@ func (o *ListOptions) Run(ctx context.Context) (err error) {
 	fmt.Fprintln(w, "NAME", "\t", "URL", "\t", "SECURE")
 	o.printRegistryList(w, registryList)
 	w.Flush()
-	if o.printGitRegistryDeprecationWarning {
-		util.PrintGitRegistryDeprecationWarning()
-	}
 	return nil
 }
 
@@ -94,9 +87,6 @@ func (o *ListOptions) printRegistryList(w io.Writer, registryList *[]preference.
 			secure = "Yes"
 		}
 		fmt.Fprintln(w, registry.Name, "\t", registry.URL, "\t", secure)
-		if util.IsGitBasedRegistry(registry.URL) {
-			o.printGitRegistryDeprecationWarning = true
-		}
 	}
 }
 

pkg/odo/cli/preference/registry/update.go

@@ -71,8 +71,12 @@ func (o *UpdateOptions) Validate() (err error) {
 	if err != nil {
 		return err
 	}
-	if registryUtil.IsGitBasedRegistry(o.registryURL) {
-		registryUtil.PrintGitRegistryDeprecationWarning()
+	isGithubBasedRegistry, err := registryUtil.IsGithubBasedRegistry(o.registryURL)
+	if err != nil {
+		return err
+	}
+	if isGithubBasedRegistry {
+		return registryUtil.ErrGithubRegistryNotSupported
 	}
 	return nil
 }

pkg/odo/cli/preference/registry/util/util.go

@@ -3,16 +3,20 @@ package util
 import (
 	// odo packages
 
+	"errors"
+	"fmt"
 	"strings"
 
-	"github.com/redhat-developer/odo/pkg/log"
 	"github.com/redhat-developer/odo/pkg/preference"
+	url2 "net/url"
 )
 
 const (
 	RegistryUser = "default"
 )
 
+var ErrGithubRegistryNotSupported = errors.New("github based registries are no longer supported, use OCI based registries instead, see https://github.com/devfile/registry-support")
+
 // IsSecure checks if the registry is secure
 func IsSecure(prefClient preference.Client, registryName string) bool {
 	isSecure := false
@@ -28,10 +32,15 @@ func IsSecure(prefClient preference.Client, registryName string) bool {
 	return isSecure
 }
 
-func IsGitBasedRegistry(url string) bool {
-	return strings.Contains(url, "github.com") || strings.Contains(url, "raw.githubusercontent.com")
-}
-
-func PrintGitRegistryDeprecationWarning() {
-	log.Deprecate("Git based registries", "Please see https://github.com/redhat-developer/odo/tree/main/docs/public/git-registry-deprecation.adoc")
+func IsGithubBasedRegistry(url string) (bool, error) {
+	pu, err := url2.Parse(url)
+	if err != nil {
+		return false, fmt.Errorf("unable to parse registry url %w", err)
+	}
+	for _, d := range []string{"github.com", "raw.githubusercontent.com"} {
+		if pu.Host == d || strings.HasSuffix(pu.Host, "."+d) {
+			return true, nil
+		}
+	}
+	return false, nil
 }

pkg/odo/cli/preference/registry/util/util_test.go

@@ -83,7 +83,7 @@ func TestIsGitBasedRegistry(t *testing.T) {
 		},
 		{
 			name:        "Case 2: Returns false if URL does not contain github",
-			registryURL: " https://registry.devfile.io",
+			registryURL: "https://registry.devfile.io",
 			want:        false,
 		},
 		{
@@ -91,11 +91,25 @@ func TestIsGitBasedRegistry(t *testing.T) {
 			registryURL: "https://raw.githubusercontent.com/odo-devfiles/registry",
 			want:        true,
 		},
+		{
+			name:        "Case 4: Returns false if URL contains github.com in a non-host position",
+			registryURL: "https://my.registry.example.com/github.com",
+			want:        false,
+		},
+		{
+			name:        "Case 5: Returns false if URL contains raw.githubusercontent.com in a non-host position",
+			registryURL: "https://my.registry.example.com/raw.githubusercontent.com",
+			want:        false,
+		},
 	}
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			if actual := IsGitBasedRegistry(tt.registryURL); actual != tt.want {
+			actual, err := IsGithubBasedRegistry(tt.registryURL)
+			if err != nil {
+				t.Errorf("unexpected error %s occoured", err.Error())
+			}
+			if actual != tt.want {
 				t.Errorf("failed checking if registry is git based, got %t want %t", actual, tt.want)
 			}
 		})

pkg/registry/registry.go

@@ -1,18 +1,12 @@
 package registry
 
 import (
-	"encoding/json"
-	"fmt"
-	"net/url"
-	"strings"
 	"sync"
 
 	devfilev1 "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2"
 	dfutil "github.com/devfile/library/pkg/util"
 	indexSchema "github.com/devfile/registry-support/index/generator/schema"
 	"github.com/devfile/registry-support/registry-library/library"
-	"github.com/zalando/go-keyring"
-
 	registryUtil "github.com/redhat-developer/odo/pkg/odo/cli/preference/registry/util"
 
 	"github.com/redhat-developer/odo/pkg/component"
@@ -137,91 +131,19 @@ func (o RegistryClient) ListDevfileStacks(registryName string) (DevfileStackList
 	return *catalogDevfileList, nil
 }
 
-// convertURL converts GitHub regular URL to GitHub raw URL, do nothing if the URL is not GitHub URL
-// For example:
-// GitHub regular URL: https://github.com/elsony/devfile-registry/tree/johnmcollier-crw
-// GitHub raw URL: https://raw.githubusercontent.com/elsony/devfile-registry/johnmcollier-crw
-func convertURL(URL string) (string, error) {
-	url, err := url.Parse(URL)
-	if err != nil {
-		return "", err
-	}
-
-	if strings.Contains(url.Host, "github") && !strings.Contains(url.Host, "raw") {
-		// Convert path part of the URL
-		URLSlice := strings.Split(URL, "/")
-		if len(URLSlice) > 2 && URLSlice[len(URLSlice)-2] == "tree" {
-			// GitHub raw URL doesn't have "tree" structure in the URL, need to remove it
-			URL = strings.Replace(URL, "/tree", "", 1)
-		} else {
-			// Add "master" branch for GitHub raw URL by default if branch is not specified
-			URL = URL + "/master"
-		}
-
-		// Convert host part of the URL
-		if url.Host == "github.com" {
-			URL = strings.Replace(URL, "github.com", "raw.githubusercontent.com", 1)
-		} else {
-			URL = strings.Replace(URL, url.Host, "raw."+url.Host, 1)
-		}
-	}
-
-	return URL, nil
-}
-
-const indexPath = "/devfiles/index.json"
-
 // getRegistryStacks retrieves the registry's index devfile stack entries
 func getRegistryStacks(preferenceClient preference.Client, registry Registry) ([]DevfileStack, error) {
-	if !strings.Contains(registry.URL, "github") {
-		// OCI-based registry
-		devfileIndex, err := library.GetRegistryIndex(registry.URL, segment.GetRegistryOptions(), indexSchema.StackDevfileType)
-		if err != nil {
-			return nil, err
-		}
-		return createRegistryDevfiles(registry, devfileIndex)
-	}
-	// Github-based registry
-	URL, err := convertURL(registry.URL)
+	isGithubregistry, err := registryUtil.IsGithubBasedRegistry(registry.URL)
 	if err != nil {
-		return nil, fmt.Errorf("unable to convert URL %s: %w", registry.URL, err)
+		return nil, err
 	}
-	registry.URL = URL
-	indexLink := registry.URL + indexPath
-	request := dfutil.HTTPRequestParams{
-		URL: indexLink,
+	if isGithubregistry {
+		return nil, registryUtil.ErrGithubRegistryNotSupported
 	}
-
-	secure := registryUtil.IsSecure(preferenceClient, registry.Name)
-	if secure {
-		token, e := keyring.Get(fmt.Sprintf("%s%s", dfutil.CredentialPrefix, registry.Name), registryUtil.RegistryUser)
-		if e != nil {
-			return nil, fmt.Errorf("unable to get secure registry credential from keyring: %w", e)
-		}
-		request.Token = token
-	}
-
-	jsonBytes, err := dfutil.HTTPGetRequest(request, preferenceClient.GetRegistryCacheTime())
+	// OCI-based registry
+	devfileIndex, err := library.GetRegistryIndex(registry.URL, segment.GetRegistryOptions(), indexSchema.StackDevfileType)
 	if err != nil {
-		return nil, fmt.Errorf("unable to download the devfile index.json from %s: %w", indexLink, err)
-	}
-
-	var devfileIndex []indexSchema.Schema
-	err = json.Unmarshal(jsonBytes, &devfileIndex)
-	if err != nil {
-		if err := util.CleanDefaultHTTPCacheDir(); err != nil {
-			log.Warning("Error while cleaning up cache dir.")
-		}
-		// we try once again
-		jsonBytes, err := dfutil.HTTPGetRequest(request, preferenceClient.GetRegistryCacheTime())
-		if err != nil {
-			return nil, fmt.Errorf("unable to download the devfile index.json from %s: %w", indexLink, err)
-		}
-
-		err = json.Unmarshal(jsonBytes, &devfileIndex)
-		if err != nil {
-			return nil, fmt.Errorf("unable to unmarshal the devfile index.json from %s: %w", indexLink, err)
-		}
+		return nil, err
 	}
 	return createRegistryDevfiles(registry, devfileIndex)
 }

pkg/registry/registry_test.go

@@ -159,40 +159,3 @@ func TestGetRegistryDevfiles(t *testing.T) {
 		})
 	}
 }
-
-func TestConvertURL(t *testing.T) {
-	tests := []struct {
-		name    string
-		URL     string
-		wantURL string
-	}{
-		{
-			name:    "Case 1: GitHub regular URL without specifying branch",
-			URL:     "https://github.com/GeekArthur/registry",
-			wantURL: "https://raw.githubusercontent.com/GeekArthur/registry/master",
-		},
-		{
-			name:    "Case 2: GitHub regular URL with master branch specified",
-			URL:     "https://github.ibm.com/Jingfu-J-Wang/registry/tree/master",
-			wantURL: "https://raw.github.ibm.com/Jingfu-J-Wang/registry/master",
-		},
-		{
-			name:    "Case 3: GitHub regular URL with non-master branch specified",
-			URL:     "https://github.com/elsony/devfile-registry/tree/johnmcollier-crw",
-			wantURL: "https://raw.githubusercontent.com/elsony/devfile-registry/johnmcollier-crw",
-		},
-	}
-
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			gotURL, err := convertURL(tt.URL)
-			if err != nil {
-				t.Error(err)
-			}
-
-			if !reflect.DeepEqual(gotURL, tt.wantURL) {
-				t.Errorf("Got url: %s, want URL: %s", gotURL, tt.wantURL)
-			}
-		})
-	}
-}

tests/integration/devfile/cmd_devfile_registry_test.go

@@ -1,8 +1,6 @@
 package devfile
 
 import (
-	"fmt"
-
 	. "github.com/onsi/ginkgo"
 
 	"github.com/redhat-developer/odo/tests/helper"
@@ -80,43 +78,8 @@ var _ = Describe("odo devfile registry command tests", func() {
 		})
 	})
 
-	When("using a git based registries", func() {
-		var deprecated, docLink, out, err, co string
-		BeforeEach(func() {
-			deprecated = "Deprecated"
-			docLink = "https://github.com/redhat-developer/odo/tree/main/docs/public/git-registry-deprecation.adoc"
-		})
-
-		It("should not show deprication warning, if git registry is not used", func() {
-			out, err = helper.Cmd("odo", "preference", "registry", "list").ShouldPass().OutAndErr()
-			helper.DontMatchAllInOutput(fmt.Sprintln(out, err), []string{deprecated, docLink})
-		})
-
-		When("adding git based registries", func() {
-			BeforeEach(func() {
-				out, err = helper.Cmd("odo", "preference", "registry", "add", "RegistryFromGitHub", "https://github.com/devfile/registry").ShouldPass().OutAndErr()
-
-			})
-			It("should show deprecation warning", func() {
-				co = fmt.Sprintln(out, err)
-				helper.MatchAllInOutput(co, []string{deprecated, docLink})
-
-				By("odo preference registry list is executed, should show the warning", func() {
-					out, err = helper.Cmd("odo", "preference", "registry", "list").ShouldPass().OutAndErr()
-					co = fmt.Sprintln(out, err)
-					helper.MatchAllInOutput(co, []string{deprecated, docLink})
-				})
-				// TODO: Should `odo init` support GitHub based registries
-				// By("should successfully delete registry", func() {
-				// 	out, err = helper.Cmd("odo", "init", "--name", "aname", "--devfile", "nodejs", "--devfile-registry", "RegistryFromGitHub").ShouldPass().OutAndErr()
-				// 	co = fmt.Sprintln(out, err)
-				// 	helper.MatchAllInOutput(co, []string{deprecated, docLink})
-				// })
-			})
-			It("should not show deprecation warning if git registry is not used for component creation", func() {
-				out, err = helper.Cmd("odo", "init", "--name", "aname", "--devfile", "nodejs", "--devfile-registry", "DefaultDevfileRegistry").ShouldPass().OutAndErr()
-				helper.DontMatchAllInOutput(fmt.Sprintln(out, err), []string{deprecated, docLink})
-			})
-		})
+	It("should fail when adding a git based registry", func() {
+		err := helper.Cmd("odo", "preference", "registry", "add", "RegistryFromGitHub", "https://github.com/devfile/registry").ShouldFail().Err()
+		helper.MatchAllInOutput(err, []string{"github", "no", "supported", "https://github.com/devfile/registry-support"})
 	})
 })