Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Regression in 0.4.12 when using private gitlab #377

Closed
jocelynthode opened this issue Dec 6, 2018 · 1 comment
Closed

Regression in 0.4.12 when using private gitlab #377

jocelynthode opened this issue Dec 6, 2018 · 1 comment
Labels
bug Something isn't working provider/gitlab

Comments

@jocelynthode
Copy link
Contributor

Hey,

While upgrading to the 0.4.12 version, I noticed that the feature introduced in #366 specifically in commit ba04564

The problem appears at this line: ba04564#diff-4431759fdf270c83e7713adcd0cf7d5aR52

We use URL.Parse() on a domain which does not create an error but returns an empty string when calling Hostname() afterwards.

I would suggest to either rename the parameter GitlabHostname to GitlabURL and use a URL from now on or jsut before calling the URL parse, format the string to contain the scheme so that we can parse it correctly.
I would also add a unit test to verify that this continue to work in the future.

@lkysow I would gladly submit a PR for this issue once we have worked out what's the best solution if that's ok with you.

P.S.: Here is an example code to reproduce the issue:

package main

import (
	"fmt"
	"log"
	"net/url"
)

func main() {
	u, err := url.Parse("https://example.org")
	if err != nil {
		log.Fatal(err)
	}
	fmt.Println(u.Hostname())
	
	u, err = url.Parse("example.org")
	if err != nil {
		log.Fatal(err)
	}
	fmt.Println(u.Hostname())

}

Output:

example.org

@lkysow
Copy link
Member

lkysow commented Dec 6, 2018

Ahh good catch. I'm thinking something like:

// If not using gitlab.com we need to set the URL to the API.
if hostname != "gitlab.com" {
	if !strings.HasPrefix(hostname, "http://") && !strings.HasPrefix(hostname, "https://") {
		hostname = "https://" + hostname
	}

	// Warn if this hostname isn't resolvable. The GitLab client
	// doesn't give good error messages in this case.
	u, err := url.Parse(hostname)
	if err != nil {
		return nil, errors.Wrapf(err, "parsing hostname %q", hostname)
	}
	ips, err := net.LookupIP(u.Hostname())
	if err != nil {
		logger.Warn("unable to resolve %q: %s", u.Hostname(), err)
	} else if len(ips) == 0 {
		logger.Warn("found no IPs while resolving %q", u.Hostname())
	}

	// Now we're ready to construct the client.
	hostname = strings.TrimSuffix(hostname, "/")
	apiURL := fmt.Sprintf("%s/api/v4/", hostname)
	if err := client.Client.SetBaseURL(apiURL); err != nil {
		return nil, errors.Wrapf(err, "setting GitLab API URL: %s", apiURL)
	}
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working provider/gitlab
Projects
None yet
Development

No branches or pull requests

3 participants