Skip to content

Commit

Permalink
Make HTTPFileLoader more intelligent (Fixes #128)
Browse files Browse the repository at this point in the history
Splits away the Loader and its implementations into the pkg/files module
and adds more logic to the HTTPFileLoader so that it can append the
target file to the URL if it's missing. Tests are also added to make
sure that the HTTPFileLoader is 100% covered now.
A new interface and a mock implementation has been added for HTTP
requests.
  • Loading branch information
Pavlos Tzianos committed Apr 12, 2024
1 parent 41dbb07 commit 16d6e5f
Show file tree
Hide file tree
Showing 14 changed files with 308 additions and 121 deletions.
10 changes: 6 additions & 4 deletions cmd/draconctl/pipelines/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,12 @@ import (
"path"
"strings"

"github.com/spf13/cobra"
kustomizetypes "sigs.k8s.io/kustomize/api/types"

"github.com/ocurity/dracon/pkg/files"
"github.com/ocurity/dracon/pkg/manifests"
"github.com/ocurity/dracon/pkg/pipelines"
"github.com/spf13/cobra"
kustomizeType "sigs.k8s.io/kustomize/api/types"
)

var buildSubCmd = &cobra.Command{
Expand All @@ -32,7 +34,7 @@ func buildPipeline(cmd *cobra.Command, args []string) error {
}

kustomizationPath := args[0]
kustomizationLoader, err := manifests.NewLoader(".", kustomizationPath, "kustomization.yaml")
kustomizationLoader, err := files.NewLoader(".", kustomizationPath, "kustomization.yaml")
if err != nil {
return fmt.Errorf("%s: could not read contents of file: %w", kustomizationPath, err)
}
Expand All @@ -44,7 +46,7 @@ func buildPipeline(cmd *cobra.Command, args []string) error {
}

// Parse Pipeline kustomization
kustomization := &kustomizeType.Kustomization{}
kustomization := &kustomizetypes.Kustomization{}
if err = kustomization.Unmarshal(fileContents); err != nil {
return fmt.Errorf("%s: could not unmarshal YAML file: %w", kustomizationPath, err)
}
Expand Down
84 changes: 84 additions & 0 deletions pkg/files/httpfileloader.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
package files

import (
"context"
"io"
"net/http"
"net/url"
"path/filepath"
"strings"

"github.com/go-errors/errors"

"github.com/ocurity/dracon/pkg/utils"
)

var (
_ Loader = (*httpFileLoader)(nil)
// ErrInvalidURL is returns when the URL of the remote file is not a URL
// with an HTTPS scheme.
ErrInvalidURL = errors.New("invalid url")
// ErrUnsuccessfulRequest is returned when the
ErrUnsuccessfulRequest = errors.New("unsuccessful request")
)

type httpFileLoader struct {
uri string
client utils.MockableRequestDoer
}

func newHTTPFileLoader(uri, targetFile string) (*httpFileLoader, error) {
fileURL, err := url.Parse(uri)
if err != nil {
return nil, errors.Errorf("%s: invalid URL: %w", uri, err)
} else if fileURL.Scheme != "https" {
return nil, errors.Errorf("%s: %w", uri, ErrInvalidURL)
}

if !strings.HasSuffix(fileURL.Path, targetFile) {
fileURL.Path = filepath.Join(fileURL.Path, targetFile)
}
return &httpFileLoader{uri: fileURL.String()}, nil
}

// Load will read a file using HTTP or HTTPS
func (hfl *httpFileLoader) Load(ctx context.Context) (content []byte, err error) {
if hfl.client == nil {
hfl.client = &http.Client{}
}

req, err := http.NewRequestWithContext(ctx, "GET", hfl.uri, nil)
if err != nil {
return nil, errors.Errorf("could not create a request: %w", err)
}

resp, err := hfl.client.Do(req)
if err != nil {
return nil, errors.Errorf("%s: could not create request: %w", hfl.uri, err)
}

defer func() {
closeErr := resp.Body.Close()
if closeErr != nil && err != nil {
err = errors.Errorf("%w: %w", closeErr, err)
} else if closeErr != nil && err == nil {
err = closeErr
}
}()

if resp.StatusCode < 200 || resp.StatusCode > 299 {
return nil, errors.Errorf("%s: %d: %w", hfl.uri, resp.StatusCode, ErrUnsuccessfulRequest)
}

content, err = io.ReadAll(resp.Body)
if err != nil {
return nil, errors.Errorf("%s: could not read body of response: %w", hfl.uri, err)
}

return content, nil
}

// Path retuns the URL of the file
func (hfl *httpFileLoader) Path() string {
return hfl.uri
}
105 changes: 105 additions & 0 deletions pkg/files/httpfileloader_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
package files

import (
"context"
"net/http"
"net/http/httptest"
"testing"

"github.com/stretchr/testify/require"

"github.com/ocurity/dracon/pkg/http/mock"
"github.com/ocurity/dracon/pkg/utils"
)

func TestHTTPFileLoaderInit(t *testing.T) {
_, err := newHTTPFileLoader("-", "kustomization.yaml")
require.ErrorIs(t, err, ErrInvalidURL)

fl, err := newHTTPFileLoader("https://github.com/ocurity/dracon/pkg", "kustomization.yaml")
require.NoError(t, err)
require.Equal(t, "https://github.com/ocurity/dracon/pkg/kustomization.yaml", fl.Path())

fl, err = newHTTPFileLoader("https://github.com/ocurity/dracon/pkg/kustomization.yaml", "kustomization.yaml")
require.NoError(t, err)
require.Equal(t, "https://github.com/ocurity/dracon/pkg/kustomization.yaml", fl.Path())
}

func TestHTTPFileLoaderLoad(t *testing.T) {
testCases := []struct {
name string
url string
targetFile string
mockRequestDoer utils.MockableRequestDoer
expectedURL string
expectedErr error
}{
{
name: "success",
url: "https://github.com/ocurity/dracon/pkg",
targetFile: "kustomization.yaml",
mockRequestDoer: &mock.HTTPReqDoer{
Hook: func(req *http.Request) (*http.Response, error) {
require.Equal(t, "https://github.com/ocurity/dracon/pkg/kustomization.yaml", req.URL.String())

recorder := httptest.NewRecorder()
recorder.Code = http.StatusOK
_, err := recorder.WriteString(`---
apiVersion: kustomize.config.k8s.io/v1beta1
kind: Kustomization
nameSuffix: -golang-project
resources:
- ../../../components/base/pipeline.yaml
- ../../../components/base/task.yaml
components:
- ../../../components/sources/git`)
require.NoError(t, err)
return recorder.Result(), nil
},
},
},
{
name: "404",
url: "https://github.com/ocurity/dracon/pkg",
targetFile: "kustomization.yaml",
mockRequestDoer: &mock.HTTPReqDoer{
Hook: func(req *http.Request) (*http.Response, error) {
require.Equal(t, "https://github.com/ocurity/dracon/pkg/kustomization.yaml", req.URL.String())

recorder := httptest.NewRecorder()
recorder.Code = http.StatusNotFound
return recorder.Result(), nil
},
},
expectedURL: "https://github.com/ocurity/dracon/pkg/kustomization.yaml",
expectedErr: ErrUnsuccessfulRequest,
},
}

testCtx, cancel := context.WithCancel(context.Background())
defer cancel()

for _, testCase := range testCases {
t.Run(testCase.name, func(t *testing.T) {
fl, err := newHTTPFileLoader("https://github.com/ocurity/dracon/pkg", "kustomization.yaml")
require.NoError(t, err)

runCtx, cancel := context.WithCancel(testCtx)
defer cancel()

fl.client = testCase.mockRequestDoer
_, err = fl.Load(runCtx)
require.ErrorIs(t, err, testCase.expectedErr)
})
}
}

func TestCancelledContext(t *testing.T) {
fl, err := newHTTPFileLoader("https://github.com/ocurity/dracon/pkg", "kustomization.yaml")
require.NoError(t, err)

runCtx, cancel := context.WithCancel(context.Background())
cancel()
_, err = fl.Load(runCtx)
require.ErrorIs(t, err, context.Canceled)
}
32 changes: 32 additions & 0 deletions pkg/files/loader.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
package files

import (
"context"
"strings"

"github.com/go-errors/errors"
)

type Loader interface {
// Root returns the root location for this Loader.
Path() string

// Load returns the bytes read from the location or an error.
Load(ctx context.Context) ([]byte, error)
}

var ErrBadTargetFile = errors.New("targetFile should a filename not a directory")

// NewLoader takes as an argumet a directory containing some configuration, a
// path or a URI and the expected file name that we want to fetch. If the
func NewLoader(configurationDir, pathOrURI, targetFile string) (Loader, error) {
if strings.Contains(targetFile, "/") {
return nil, errors.Errorf("%s: %w", targetFile, ErrBadTargetFile)
}

if IsRemoteFile(pathOrURI) {
return newHTTPFileLoader(pathOrURI, targetFile)
}

return newLocalFileLoader(configurationDir, pathOrURI, targetFile)
}
19 changes: 9 additions & 10 deletions pkg/manifests/localfileloader.go → pkg/files/localfileloader.go
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
package manifests
package files

import (
"context"
"fmt"
"os"
"path"
"path/filepath"
)

var _ Loader = (*localFileLoader)(nil)
Expand All @@ -13,9 +14,9 @@ type localFileLoader struct {
path string
}

func newLocalFileLoader(root, fileOrDir, targetFile string) (*localFileLoader, error) {
if !path.IsAbs(fileOrDir) {
fileOrDir = path.Join(root, fileOrDir)
func newLocalFileLoader(configurationDir, fileOrDir, targetFile string) (*localFileLoader, error) {
if !filepath.IsAbs(fileOrDir) {
fileOrDir = path.Clean(filepath.Join(configurationDir, fileOrDir))
}

info, err := os.Stat(fileOrDir)
Expand All @@ -25,21 +26,19 @@ func newLocalFileLoader(root, fileOrDir, targetFile string) (*localFileLoader, e

if info.IsDir() {
fileOrDir = path.Join(fileOrDir, targetFile)
} else {
if path.Base(fileOrDir) != targetFile {
return nil, fmt.Errorf("path %s should be pointing to a %s", fileOrDir, targetFile)
}
} else if path.Base(fileOrDir) != targetFile {
return nil, fmt.Errorf("path %s should be pointing to a %s", fileOrDir, targetFile)
}

fileOrDir = path.Clean(fileOrDir)

return &localFileLoader{path: fileOrDir}, nil
}

// Path returns the path in the local file system
func (lfl *localFileLoader) Path() string {
return lfl.path
}

// Load reads a file from the local file system
func (lfl *localFileLoader) Load(_ context.Context) ([]byte, error) {
return os.ReadFile(lfl.path)
}
29 changes: 29 additions & 0 deletions pkg/files/mock/mock.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
package manifests

import (
"context"

"github.com/ocurity/dracon/pkg/files"
)

var _ files.Loader = FakeLoader{}

type FakeLoader struct {
path string
mockLoad func() ([]byte, error)
}

func (l FakeLoader) NewFakeLoader(mockPath string, mockLoad func() ([]byte, error)) FakeLoader {
return FakeLoader{
path: mockPath,
mockLoad: mockLoad,
}
}

func (f FakeLoader) Load(ctx context.Context) ([]byte, error) {
return f.mockLoad()
}

func (f FakeLoader) Path() string {
return f.path
}
2 changes: 1 addition & 1 deletion pkg/manifests/utils.go → pkg/files/utils.go
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package manifests
package files

import (
"net/url"
Expand Down
17 changes: 17 additions & 0 deletions pkg/http/mock/http.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
package mock

import (
"net/http"

"github.com/ocurity/dracon/pkg/utils"
)

var _ utils.MockableRequestDoer = (*HTTPReqDoer)(nil)

type HTTPReqDoer struct {
Hook func(*http.Request) (*http.Response, error)
}

func (m *HTTPReqDoer) Do(r *http.Request) (*http.Response, error) {
return m.Hook(r)
}
Loading

0 comments on commit 16d6e5f

Please sign in to comment.