Skip to content

Commit

Permalink
Add tests
Browse files Browse the repository at this point in the history
  • Loading branch information
arielshaqed committed Jul 4, 2022
1 parent b59123b commit 9786efa
Show file tree
Hide file tree
Showing 5 changed files with 70 additions and 4 deletions.
2 changes: 2 additions & 0 deletions api/swagger.yml
Expand Up @@ -3544,5 +3544,7 @@ paths:
format: binary
401:
$ref: "#/components/responses/Unauthorized"
404:
$ref: "#/components/responses/NotFound"
default:
$ref: "#/components/responses/ServerError"
4 changes: 4 additions & 0 deletions pkg/api/controller.go
Expand Up @@ -3269,6 +3269,10 @@ func (c *Controller) ExpandTemplate(w http.ResponseWriter, r *http.Request, p Ex
writeError(w, http.StatusUnauthorized, http.StatusText(http.StatusUnauthorized))
return
}
if errors.Is(err, templater.ErrNotFound) {
writeError(w, http.StatusNotFound, http.StatusText(http.StatusNotFound))
return
}
if err != nil {
writeError(w, http.StatusInternalServerError, "expansion failed")
return
Expand Down
56 changes: 56 additions & 0 deletions pkg/api/controller_test.go
Expand Up @@ -3,6 +3,7 @@ package api_test
import (
"bytes"
"context"
"encoding/json"
"errors"
"fmt"
"io"
Expand All @@ -12,6 +13,7 @@ import (
"net/http/httptest"
"path/filepath"
"reflect"
"regexp"
"sort"
"strconv"
"strings"
Expand Down Expand Up @@ -101,6 +103,7 @@ var tests = map[string]func(*testing.T, bool){
"CreateTag": testController_CreateTag,
"Revert": testController_Revert,
"RevertConflict": testController_RevertConflict,
"ExpandTemplate": testController_ExpandTemplate,
}

func TestKVEnabled(t *testing.T) {
Expand Down Expand Up @@ -2403,3 +2406,56 @@ func testController_RevertConflict(t *testing.T, kvEnabled bool) {
t.Errorf("Revert with a conflict should fail with status %d got %d", http.StatusConflict, resp.HTTPResponse.StatusCode)
}
}

func testController_ExpandTemplate(t *testing.T, kvEnabled bool) {
clt, _ := setupClientWithAdmin(t, kvEnabled)
ctx := context.Background()

t.Run("not-found", func(t *testing.T) {
resp, err := clt.ExpandTemplateWithResponse(ctx, &api.ExpandTemplateParams{TemplateLocation: "no/template/here"})
testutil.Must(t, err)
if resp.HTTPResponse.StatusCode != http.StatusNotFound {
t.Errorf("Expanding a nonexistent template should fail with status %d got %d\n\t%+v", http.StatusNotFound, resp.HTTPResponse.StatusCode, resp)
}
})

t.Run("spark.conf", func(t *testing.T) {
expected := []struct {
name string
pattern string
}{
{"impl", `spark.hadoop.fs.lakefs.impl=io.lakefs.LakeFSFileSystem`},
{"access_key", `spark.hadoop.fs.lakefs.access_key=AKIA.*`},
{"secret_key", `spark.hadoop.fs.lakefs.secret_key=`},
}
resp, err := clt.ExpandTemplateWithResponse(ctx, &api.ExpandTemplateParams{TemplateLocation: "spark.conf.tt"})
testutil.Must(t, err)
if resp.HTTPResponse.StatusCode != http.StatusOK {
t.Errorf("Expanding template spark.conf.tt failed with status %d\n\t%+v", resp.HTTPResponse.StatusCode, resp)
}

for _, e := range expected {
re := regexp.MustCompile(e.pattern)
if !re.Match(resp.Body) {
t.Errorf("Expanded template spark.conf.tt has no %s: /%s/\n\t%s", e.name, e.pattern, string(resp.Body))
}
}
})

t.Run("fail", func(t *testing.T) {
resp, err := clt.ExpandTemplateWithResponse(ctx, &api.ExpandTemplateParams{TemplateLocation: "fail.tt"})
testutil.Must(t, err)
if resp.HTTPResponse.StatusCode != http.StatusInternalServerError {
t.Errorf("Expanding template spark.conf.tt should fail with status %d got %d\n\t%+v", http.StatusInternalServerError, resp.HTTPResponse.StatusCode, resp)
}

parsed := make(map[string]string, 0)
err = json.Unmarshal(resp.Body, &parsed)
if err != nil {
t.Errorf("Unmarshal body: %s", err)
}
if parsed["message"] != "expansion failed" {
t.Errorf("Expected \"expansion failed\" message, got %+v", parsed)
}
})
}
6 changes: 5 additions & 1 deletion pkg/api/serve_test.go
Expand Up @@ -30,8 +30,10 @@ import (
"github.com/treeverse/lakefs/pkg/kv"
"github.com/treeverse/lakefs/pkg/logging"
"github.com/treeverse/lakefs/pkg/stats"
"github.com/treeverse/lakefs/pkg/templater"
"github.com/treeverse/lakefs/pkg/testutil"
"github.com/treeverse/lakefs/pkg/version"
"github.com/treeverse/lakefs/templates"
)

const (
Expand Down Expand Up @@ -142,8 +144,10 @@ func setupHandlerWithWalkerFactory(t testing.TB, factory catalog.WalkerFactory,
auditChecker := version.NewDefaultAuditChecker(cfg.GetSecurityAuditCheckURL())
emailParams, _ := cfg.GetEmailParams()
emailer, err := email.NewEmailer(emailParams)
templater := templater.NewService(templates.Content, cfg, authService)

testutil.Must(t, err)
handler := api.Serve(cfg, c, authenticator, authenticator, authService, c.BlockAdapter, meta, migrator, collector, nil, actionsService, auditChecker, logging.Default(), emailer, nil, nil, nil, nil, nil)
handler := api.Serve(cfg, c, authenticator, authenticator, authService, c.BlockAdapter, meta, migrator, collector, nil, actionsService, auditChecker, logging.Default(), emailer, templater, nil, nil, nil, nil)

return handler, &dependencies{
blocks: c.BlockAdapter,
Expand Down
6 changes: 3 additions & 3 deletions pkg/templater/expander.go
Expand Up @@ -22,7 +22,7 @@ const (
)

var (
ErrTemplateNotFound = errors.New("template not found")
ErrNotFound = errors.New("template not found")
ErrPathTraversalBlocked = errors.New("path traversal blocked")
)

Expand Down Expand Up @@ -127,7 +127,7 @@ func (em *ExpanderMap) Get(ctx context.Context, username, name string) (Expander
// Fast-path through the cache
if e == nil {
// Negative cache
return nil, ErrTemplateNotFound
return nil, ErrNotFound
}
return e, nil
}
Expand All @@ -141,7 +141,7 @@ func (em *ExpanderMap) Get(ctx context.Context, username, name string) (Expander

tmpl, err := fs.ReadFile(em.fs, p)
if errors.Is(err, fs.ErrNotExist) {
return nil, ErrTemplateNotFound
return nil, ErrNotFound
}
if err != nil {
return nil, err
Expand Down

0 comments on commit 9786efa

Please sign in to comment.