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

Generic provider #179

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion cmd/ensure.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ func newEnsureCmd() *ensureCmd {
continue
}

p, err := providers.New(binCfg.URL, binCfg.Provider)
p, err := providers.New(binCfg.URL, binCfg.Provider, binCfg.LatestURL)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as comments in install.go

if err != nil {
return err
}
Expand All @@ -71,6 +71,7 @@ func newEnsureCmd() *ensureCmd {
Version: pResult.Version,
Hash: fmt.Sprintf("%x", pResult.Hash.Sum(nil)),
URL: binCfg.URL,
LatestURL: binCfg.LatestURL,
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as comments in install.go

})
if err != nil {
return err
Expand Down
13 changes: 8 additions & 5 deletions cmd/install.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,10 @@ type installCmd struct {
}

type installOpts struct {
force bool
provider string
all bool
all bool
force bool
provider string
latestURL string
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should move this latestURL inside the generic provider only since it doesn't apply to all of them

}

func newInstallCmd() *installCmd {
Expand Down Expand Up @@ -57,7 +58,7 @@ func newInstallCmd() *installCmd {
// TODO check if binary already exists in config
// and triger the update process if that's the case

p, err := providers.New(u, root.opts.provider)
p, err := providers.New(u, root.opts.provider, root.opts.latestURL)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same as the comment above. The provider itself should be the one who parses the latest url from the provided flags instead of sending this argument to all provider initializations. We'll have to refactor the code a bit but I believe it's worth it. We'll have to use different flagsets (https://pkg.go.dev/flag#NewFlagSet) per provider.

if err != nil {
return err
}
Expand Down Expand Up @@ -87,6 +88,7 @@ func newInstallCmd() *installCmd {
Hash: fmt.Sprintf("%x", pResult.Hash.Sum(nil)),
URL: u,
Provider: p.GetID(),
LatestURL: root.opts.latestURL,
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as other comments, having this in the top level of the config seems weird. I was thinking that we could maybe extend the provider key we already have and make it map instead of string. That way, we can save provider specific configurations in the config.

We could add a new GetConfig methgod to the provider interface for this.

PackagePath: pResult.PackagePath,
})

Expand All @@ -101,9 +103,10 @@ func newInstallCmd() *installCmd {
}

root.cmd = cmd
root.cmd.Flags().BoolVarP(&root.opts.force, "force", "f", false, "Force the installation even if the file already exists")
root.cmd.Flags().BoolVarP(&root.opts.all, "all", "a", false, "Show all possible download options (skip scoring & filtering)")
root.cmd.Flags().BoolVarP(&root.opts.force, "force", "f", false, "Force the installation even if the file already exists")
root.cmd.Flags().StringVarP(&root.opts.provider, "provider", "p", "", "Forces to use a specific provider")
root.cmd.Flags().StringVarP(&root.opts.latestURL, "latest-url", "u", "", "Specify a latest version URL for generic provider")
return root
}

Expand Down
6 changes: 3 additions & 3 deletions cmd/update.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ func newUpdateCmd() *updateCmd {
updateFailures := map[*config.Binary]error{}

for _, b := range binsToProcess {
p, err := providers.New(b.URL, b.Provider)
p, err := providers.New(b.URL, b.Provider, b.LatestURL)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as comments in install.go

if err != nil {
return err
}
Expand Down Expand Up @@ -108,8 +108,7 @@ func newUpdateCmd() *updateCmd {
// the same thing as install logic. Refactor to
// use the same code in both places
for ui, b := range toUpdate {

p, err := providers.New(ui.url, b.Provider)
p, err := providers.New(ui.url, b.Provider, b.LatestURL)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as comments in install.go

if err != nil {
return err
}
Expand All @@ -133,6 +132,7 @@ func newUpdateCmd() *updateCmd {
Version: pResult.Version,
Hash: fmt.Sprintf("%x", pResult.Hash.Sum(nil)),
URL: ui.url,
LatestURL: b.LatestURL,
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as comments in install.go

PackagePath: pResult.PackagePath,
})
if err != nil {
Expand Down
1 change: 1 addition & 0 deletions pkg/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ type Binary struct {
Hash string `json:"hash"`
URL string `json:"url"`
Provider string `json:"provider"`
LatestURL string `json:"latest_url"` // for generic provider
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as comments in install.go

// if file is installed from a package format (zip, tar, etc) store
// the package path in config so we don't ask the user to select
// the path again when upgrading
Expand Down
103 changes: 103 additions & 0 deletions pkg/providers/generic.go
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: renane generic.go to http_generic.go? WDYT?

Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
package providers

import (
"crypto/sha256"
"errors"
"fmt"
"io"
"net/http"
"net/url"
"regexp"
"strings"

"github.com/apex/log"
"github.com/marcosnils/bin/pkg/assets"
)

type generic struct {
baseURL string
name string
version string
os string
arch string
ext string
latestURL string
}

func (g *generic) GetID() string {
return "generic"
}

func (g *generic) GetLatestVersion() (string, string, error) {
log.Debugf("Getting latest release for %s", g.name)

resp, err := http.Get(g.latestURL)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should check if latestURL is not empty here so we can add proper debug logs for this with a message like: latest url not found for $name or something

if err != nil {
return "", "", err
}
defer resp.Body.Close()

if resp.StatusCode != http.StatusOK {
return "", "", fmt.Errorf("HTTP response error: %v", resp.StatusCode)
}

body, err := io.ReadAll(resp.Body)
if err != nil {
return "", "", fmt.Errorf("Error reading response body: %v", err)
}

version := strings.TrimSuffix(string(body), "\n")
_, link := g.fmtNameLink(version)

return version, link, nil
}

func (g *generic) Fetch(opts *FetchOpts) (*File, error) {
var version string
if len(g.version) > 0 {
log.Infof("Getting release %s for %s", g.version, g.name)
version = g.version
} else {
latest, _, err := g.GetLatestVersion()
if err != nil {
return nil, err
}
version = latest
}

name, link := g.fmtNameLink(version)
candidates := []*assets.Asset{{Name: name, URL: link}}

f := assets.NewFilter(&assets.FilterOpts{SkipScoring: opts.All, PackagePath: opts.PackagePath, SkipPathCheck: opts.SkipPatchCheck})
gf, err := f.FilterAssets(g.name, candidates)
if err != nil {
return nil, err
}

outFile, err := f.ProcessURL(gf)
if err != nil {
return nil, err
}

return &File{Data: outFile.Source, Name: assets.SanitizeName(outFile.Name, version), Hash: sha256.New(), Version: version}, nil
}

func (g *generic) fmtNameLink(version string) (string, string) {
fname := fmt.Sprintf("%s-%s-%s-%s.%s", g.name, version, g.os, g.arch, g.ext)
return fname, fmt.Sprintf("%s/%s", g.baseURL, fname)
}

func newGeneric(u *url.URL, latestURL string) (Provider, error) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems like this provider is not very generic since it expects the URL to have a specific format or otherwise it'll fail. I'll leave a more general comment about this in the issue.

r := regexp.MustCompile(`^(.+)\/(.+)-(v\d*\.\d*\.\d*)-([a-z]+)-([a-z0-9]+).(.*)$`)

m := r.FindStringSubmatch(u.String())
if len(m) != 7 {
return nil, errors.New("Failed to parse specified URL")
}

if len(latestURL) == 0 {
return nil, errors.New("Latest URL must be specified for generic provider")
}

return &generic{baseURL: m[1], name: m[2], version: m[3], os: m[4], arch: m[5], ext: m[6], latestURL: latestURL}, nil
}
9 changes: 7 additions & 2 deletions pkg/providers/providers.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ type Provider interface {
// Fetch returns the file metadata to retrieve a specific binary given
// for a provider
Fetch(*FetchOpts) (*File, error)

// GetLatestVersion returns the version and the URL of the
// latest version for this binary
GetLatestVersion() (string, string, error)
Expand All @@ -44,16 +45,16 @@ var (
dockerUrlPrefix = regexp.MustCompile("^docker://")
)

func New(u, provider string) (Provider, error) {
func New(u, provider, latestURL string) (Provider, error) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as comments in install.go

if dockerUrlPrefix.MatchString(u) {
return newDocker(u)
}

if !httpUrlPrefix.MatchString(u) {
u = fmt.Sprintf("https://%s", u)
}

purl, err := url.Parse(u)

if err != nil {
return nil, err
}
Expand All @@ -70,5 +71,9 @@ func New(u, provider string) (Provider, error) {
return newHashiCorp(purl)
}

if len(latestURL) != 0 && (provider == "generic" || len(provider) == 0) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as comments in install.go

return newGeneric(purl, latestURL)
}

return nil, fmt.Errorf("Can't find provider for url %s", u)
}