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

fix: #29 #52

Closed
wants to merge 19 commits into from
Closed
Show file tree
Hide file tree
Changes from 2 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
111 changes: 106 additions & 5 deletions files/files.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,12 @@ import (
"compress/gzip"
"context"
"encoding/hex"
"errors"
"fmt"
"io"
"io/ioutil"
"math/rand"
"net/url"
"os"
"os/exec"
"path"
Expand All @@ -22,6 +24,16 @@ import (
"github.com/ulikunitz/xz"
)

// Errors
//
var (
ErrExists = errors.New("file already exist")
ErrNotExists = errors.New("file doesn't exist")
ErrInvalidPath = errors.New("file path invalid")
ErrIsDir = errors.New("path is a directory")
ErrInvalidURL = errors.New("invalid url")
)

//GzipFile takes the path to a file and returns a Gzip comppressed byte slice
func GzipFile(path string) ([]byte, error) {
var buf bytes.Buffer
Expand Down Expand Up @@ -316,14 +328,25 @@ func GetBaseName(filename string) string {

// Getter gets a directory or file using the Hashicorp go-getter library
// See https://github.com/hashicorp/go-getter
func Getter(url, dst string) error {
func Getter(source, dst string) error {

// Validate the URL first.
url, err := url.ParseRequestURI(source)
Copy link
Member

Choose a reason for hiding this comment

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

Why do we need to validate the URL first? Are we sure that url.ParseRequestURI will not return an error for all URLS/URI's supported by go-getter ?

Copy link
Author

@mrinalwahal mrinalwahal Sep 11, 2022

Choose a reason for hiding this comment

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

I think you are right. I added the URI parser primarily for branching the code to handle local files and URLs separately. But then I forgot I shifted it to ResolveFile() function. I'm pushing the change.

if err != nil {
return err
}

pwd, _ := os.Getwd()

// If the destination is empty,
// choose the current working directory.
if dst == "" {
dst, _ = os.Getwd()
}

stashed := false
if Exists(dst + "/.git") {
cmd := exec.Command("git", "stash")
cmd.Dir = pwd

cmd.Dir = dst
cmd.Stderr = os.Stderr
cmd.Stdout = os.Stdout
Expand All @@ -335,14 +358,16 @@ func Getter(url, dst string) error {
}
client := &getter.Client{
Ctx: context.TODO(),
Src: url,
Src: source,
Dst: dst,
Pwd: pwd,
Mode: getter.ClientModeDir,
Options: []getter.ClientOption{},
}

logger.Infof("Downloading %s -> %s", url, dst)
err := client.Get()
err = client.Get()

if stashed {
cmd := exec.Command("git", "stash", "pop")
cmd.Dir = dst
Expand All @@ -359,3 +384,79 @@ func TempFileName(prefix, suffix string) string {
rand.Read(randBytes)
return filepath.Join(os.TempDir(), prefix+hex.EncodeToString(randBytes)+suffix)
}

// Resolve File
//
// If file location passed as argument is a valid,
// then it returns the contents of the file.
// Otherwise, an error.
func ResolveFile(file string) (string, error) {

var payload string

// Ensure it is a valid file path,
// by ensuring there an extension suffixed at the end.
//
// This will validate both a file in the filesystem,
// as well as the URL source for a file.
if filepath.Ext(file) == "" {
Copy link
Member

Choose a reason for hiding this comment

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

Why does a file or URL need an extension?

$value == 1.00 is not a valid path or URL, yet would pass this test?

And likewise
/etc/hosts is a valid path ?

Copy link
Author

Choose a reason for hiding this comment

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

Aah, I only added the extension check to return an error if its a directory instead of a file. I totally forgot about files like /etc/hosts with no extensions. Let me improve this.

return payload, ErrInvalidPath
}

// Try to parse the file as a URL,
// and go-get the file if it a valid URL.
//
// For some reason, this inbuilt parser is broken
// and it even matches relative paths that need not be URLs.
// But we still need to parse the URL so that we can
// validate the scheme to detect a URL.
url, err := url.ParseRequestURI(file)

// Ensure the scheme is HTTP or HTTPS.
if err == nil &&
url.Scheme != "" &&
(strings.ToLower(url.Scheme) == "http" ||
Copy link
Member

Choose a reason for hiding this comment

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

What about the other schemes supported by go-getter ?

Copy link
Author

Choose a reason for hiding this comment

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

I only added the scheme check because FS files came with file:// scheme. And I wanted to exclude them. Because the url.ParseRequestURI(source) method is parsing even the URLs without a host as valid ones. So, I needed a quick way out.

Copy link
Author

@mrinalwahal mrinalwahal Sep 11, 2022

Choose a reason for hiding this comment

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

Oh, wait! I didn't know that go-getter also supports local files. I just checked their docs. I thought it only supported remote files over networks 🤦

This calls for changing the entire function code now.

Apologies for this mishap! I'll get this done.

Copy link
Author

Choose a reason for hiding this comment

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

@moshloop, should I add an extra argument for the destination to where we have to resolve the file? The issue description doesn't mention it, that's why I am asking. In absence of a valid destination, I'll initialize a temporary directory to resolve the files over there.

Copy link
Author

@mrinalwahal mrinalwahal Sep 11, 2022

Choose a reason for hiding this comment

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

Pushed new changes. Supports all go-getter schemes now 👍 Including git://. Added a new test for it.

strings.ToLower(url.Scheme) == "https") {

// Try to go-get the file.
return payload, Getter(filepath.Dir(file), "")
}

// Since, it is not a URL,
// we must validate as a source in the filesystem.

// Ensure the file exists.
if !Exists(file) {
return payload, ErrNotExists
}

data, err := os.ReadFile(file)
if err != nil {
return payload, err
}

// Update the response payload.
payload = string(data)

return payload, nil
}

// ResolveFiles
//
// Calls ResolveFile(filepath) function on an array of files.
func ResolveFiles(files []string) (map[string]string, error) {

var payload map[string]string

for _, source := range files {

response, err := ResolveFile(source)
if err != nil {
return nil, errors.New(strings.Join([]string{source, err.Error()}, " : "))
}

payload[source] = response
}

return payload, nil
}
29 changes: 28 additions & 1 deletion files/files_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package files

import "testing"
import (
"testing"
)

func TestIsValidPathType(t *testing.T) {
type args struct {
Expand All @@ -26,3 +28,28 @@ func TestIsValidPathType(t *testing.T) {
})
}
}

func TestResolveFile(t *testing.T) {
type args struct {
file string
}
tests := []struct {
name string
args args
wantErr bool
}{
{"directory", args{"/Users/mrinalwahal/go/src/github.com/flanksource/regen"}, true},
{"correctFile", args{"/Users/mrinalwahal/go/src/github.com/flanksource/regen/file.temp"}, false},
{"incorrectFile", args{"https/Users/mrinalwahal/go/src/github.com/flanksource/regen/file1.temp"}, true},
// {"url", args{"https://github.com/mrinalwahal/portfolio/README.md"}, false},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
_, err := ResolveFile(tt.args.file)
if (err != nil) != tt.wantErr {
t.Errorf("%v: error = %v, wantErr %v", tt.name, err, tt.wantErr)
return
}
})
}
}
37 changes: 26 additions & 11 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -3,26 +3,41 @@ module github.com/flanksource/commons
go 1.13

require (
contrib.go.opencensus.io/resource v0.1.1 // indirect
github.com/Azure/azure-amqp-common-go/v2 v2.1.0 // indirect
github.com/Masterminds/semver v1.5.0 // indirect
github.com/Masterminds/sprig v2.22.0+incompatible
github.com/antonmedv/expr v1.9.0
github.com/boltdb/bolt v1.3.1 // indirect
github.com/cpuguy83/go-md2man v1.0.10 // indirect
github.com/dustin/go-humanize v1.0.0
github.com/dustin/gojson v0.0.0-20160307161227-2e71ec9dd5ad // indirect
github.com/fullsailor/pkcs7 v0.0.0-20190404230743-d7302db945fa // indirect
github.com/ghodss/yaml v1.0.0
github.com/hairyhenderson/gomplate/v3 v3.6.0
github.com/go-ldap/ldap v3.0.2+incompatible // indirect
github.com/googleapis/gax-go v2.0.2+incompatible // indirect
github.com/hairyhenderson/gomplate/v3 v3.11.2
github.com/hashicorp/go-getter v1.4.1
github.com/hashicorp/vault/api v1.0.4
github.com/hashicorp/go.net v0.0.1 // indirect
github.com/hashicorp/vault/api v1.6.0
github.com/huandu/xstrings v1.3.2 // indirect
github.com/imdario/mergo v0.3.12 // indirect
github.com/kr/pretty v0.2.0
github.com/onsi/gomega v1.9.0
github.com/kr/pretty v0.2.1
github.com/mitchellh/gox v0.4.0 // indirect
github.com/mitchellh/iochan v1.0.0 // indirect
github.com/onsi/gomega v1.19.0
github.com/pkg/errors v0.9.1
github.com/sirupsen/logrus v1.7.0
github.com/smartystreets/assertions v1.0.0 // indirect
github.com/rainycape/unidecode v0.0.0-20150907023854-cb7f23ec59be // indirect
github.com/sirupsen/logrus v1.8.1
github.com/smartystreets/goconvey v1.6.4 // indirect
github.com/spf13/pflag v1.0.5
github.com/spf13/viper v1.3.2 // indirect
github.com/ulikunitz/xz v0.5.10
github.com/vbauerster/mpb/v5 v5.0.3
go.uber.org/zap v1.15.0
golang.org/x/crypto v0.0.0-20200317142112-1b76d66859c6
gopkg.in/check.v1 v1.0.0-20190902080502-41f04d3bba15
gopkg.in/flanksource/yaml.v3 v3.1.0
github.com/zealic/xignore v0.3.3 // indirect
go.uber.org/zap v1.21.0
golang.org/x/crypto v0.0.0-20220525230936-793ad666bf5e
gopkg.in/asn1-ber.v1 v1.0.0-20181015200546-f715ec2f112d // indirect
gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c
gopkg.in/src-d/go-git.v4 v4.13.1 // indirect
k8s.io/client-go v0.25.0 // indirect
)
Loading