From 28d97871139d26a24408f9e113fbec4358abf2b8 Mon Sep 17 00:00:00 2001 From: Mohammed Zeeshan Ahmed Date: Thu, 5 May 2022 17:09:31 +0530 Subject: [PATCH] Fixing as per @armels comments Signed-off-by: Mohammed Zeeshan Ahmed --- pkg/init/backend/flags.go | 6 +++++- pkg/odo/cli/preference/registry/add.go | 6 +++++- pkg/odo/cli/preference/registry/update.go | 6 +++++- pkg/odo/cli/preference/registry/util/util.go | 10 ++++++++-- pkg/odo/cli/preference/registry/util/util_test.go | 11 ++++++++++- pkg/registry/registry.go | 6 +++++- 6 files changed, 38 insertions(+), 7 deletions(-) diff --git a/pkg/init/backend/flags.go b/pkg/init/backend/flags.go index c74bb4d9955..4bd5f7f58a5 100644 --- a/pkg/init/backend/flags.go +++ b/pkg/init/backend/flags.go @@ -52,7 +52,11 @@ func (o *FlagsBackend) Validate(flags map[string]string, fs filesystem.Filesyste } registries := o.preferenceClient.RegistryList() for _, r := range *registries { - if r.Name == flags[FLAG_DEVFILE_REGISTRY] && util.IsGithubBasedRegistry(r.URL) { + isGithubRegistry, err := util.IsGithubBasedRegistry(r.URL) + if err != nil { + return err + } + if r.Name == flags[FLAG_DEVFILE_REGISTRY] && isGithubRegistry { return util.ErrGithubRegistryNotSupported } } diff --git a/pkg/odo/cli/preference/registry/add.go b/pkg/odo/cli/preference/registry/add.go index 2827f1944dc..07f2408c11c 100644 --- a/pkg/odo/cli/preference/registry/add.go +++ b/pkg/odo/cli/preference/registry/add.go @@ -73,7 +73,11 @@ func (o *AddOptions) Validate() (err error) { if err != nil { return err } - if util2.IsGithubBasedRegistry(o.registryURL) { + isGithubRegistry, err := util2.IsGithubBasedRegistry(o.registryURL) + if err != nil { + return err + } + if isGithubRegistry { return util2.ErrGithubRegistryNotSupported } return nil diff --git a/pkg/odo/cli/preference/registry/update.go b/pkg/odo/cli/preference/registry/update.go index cbbe522694f..da9e38745a6 100644 --- a/pkg/odo/cli/preference/registry/update.go +++ b/pkg/odo/cli/preference/registry/update.go @@ -71,7 +71,11 @@ func (o *UpdateOptions) Validate() (err error) { if err != nil { return err } - if registryUtil.IsGithubBasedRegistry(o.registryURL) { + isGithubBasedRegistry, err := registryUtil.IsGithubBasedRegistry(o.registryURL) + if err != nil { + return err + } + if isGithubBasedRegistry { return registryUtil.ErrGithubRegistryNotSupported } return nil diff --git a/pkg/odo/cli/preference/registry/util/util.go b/pkg/odo/cli/preference/registry/util/util.go index f1c04d858a6..9f9d2d18571 100644 --- a/pkg/odo/cli/preference/registry/util/util.go +++ b/pkg/odo/cli/preference/registry/util/util.go @@ -4,9 +4,11 @@ import ( // odo packages "errors" + "fmt" "strings" "github.com/redhat-developer/odo/pkg/preference" + url2 "net/url" ) const ( @@ -30,6 +32,10 @@ func IsSecure(prefClient preference.Client, registryName string) bool { return isSecure } -func IsGithubBasedRegistry(url string) bool { - return strings.Contains(url, "github.com") || strings.Contains(url, "raw.githubusercontent.com") +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) + } + return strings.Contains(pu.Host, "github.com") || strings.Contains(url, "raw.githubusercontent.com"), nil } diff --git a/pkg/odo/cli/preference/registry/util/util_test.go b/pkg/odo/cli/preference/registry/util/util_test.go index e07e7b1f60d..64e9b1cfe84 100644 --- a/pkg/odo/cli/preference/registry/util/util_test.go +++ b/pkg/odo/cli/preference/registry/util/util_test.go @@ -91,11 +91,20 @@ 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, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - if actual := IsGithubBasedRegistry(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) } }) diff --git a/pkg/registry/registry.go b/pkg/registry/registry.go index ee28de0468b..7ab0f5ddfd2 100644 --- a/pkg/registry/registry.go +++ b/pkg/registry/registry.go @@ -133,7 +133,11 @@ func (o RegistryClient) ListDevfileStacks(registryName string) (DevfileStackList // getRegistryStacks retrieves the registry's index devfile stack entries func getRegistryStacks(preferenceClient preference.Client, registry Registry) ([]DevfileStack, error) { - if registryUtil.IsGithubBasedRegistry(registry.URL) { + isGithubregistry, err := registryUtil.IsGithubBasedRegistry(registry.URL) + if err != nil { + return nil, err + } + if isGithubregistry { return nil, registryUtil.ErrGithubRegistryNotSupported } // OCI-based registry