Provisioning: unit test local repository in repository package (#104108)
This commit is contained in:
@@ -337,10 +337,18 @@ func (r *githubRepository) Write(ctx context.Context, path string, ref string, d
|
||||
if ref == "" {
|
||||
ref = r.config.Spec.GitHub.Branch
|
||||
}
|
||||
|
||||
ctx, _ = r.logger(ctx, ref)
|
||||
finalPath := safepath.Join(r.config.Spec.GitHub.Path, path)
|
||||
_, err := r.Read(ctx, finalPath, ref)
|
||||
if err != nil && !(errors.Is(err, ErrFileNotFound)) {
|
||||
return fmt.Errorf("failed to check if file exists before writing: %w", err)
|
||||
}
|
||||
if err == nil {
|
||||
return r.Update(ctx, finalPath, ref, data, message)
|
||||
}
|
||||
|
||||
return writeWithReadThenCreateOrUpdate(ctx, r, finalPath, ref, data, message)
|
||||
return r.Create(ctx, finalPath, ref, data, message)
|
||||
}
|
||||
|
||||
func (r *githubRepository) Delete(ctx context.Context, path, ref, comment string) error {
|
||||
|
||||
@@ -112,35 +112,34 @@ func (r *localRepository) Config() *provisioning.Repository {
|
||||
}
|
||||
|
||||
// Validate implements provisioning.Repository.
|
||||
func (r *localRepository) Validate() (fields field.ErrorList) {
|
||||
func (r *localRepository) Validate() field.ErrorList {
|
||||
cfg := r.config.Spec.Local
|
||||
if cfg == nil {
|
||||
fields = append(fields, &field.Error{
|
||||
return field.ErrorList{&field.Error{
|
||||
Type: field.ErrorTypeRequired,
|
||||
Field: "spec.local",
|
||||
})
|
||||
return fields
|
||||
}}
|
||||
}
|
||||
|
||||
// The path value must be set for local provisioning
|
||||
if cfg.Path == "" {
|
||||
fields = append(fields, field.Required(field.NewPath("spec", "local", "path"),
|
||||
"must enter a path to local file"))
|
||||
return field.ErrorList{field.Required(field.NewPath("spec", "local", "path"),
|
||||
"must enter a path to local file")}
|
||||
}
|
||||
|
||||
if err := safepath.IsSafe(cfg.Path); err != nil {
|
||||
return field.ErrorList{field.Invalid(field.NewPath("spec", "local", "path"),
|
||||
cfg.Path, err.Error())}
|
||||
}
|
||||
|
||||
// Check if it is valid
|
||||
_, err := r.resolver.LocalPath(cfg.Path)
|
||||
if err != nil {
|
||||
fields = append(fields, field.Invalid(field.NewPath("spec", "local", "path"),
|
||||
cfg.Path, err.Error()))
|
||||
return field.ErrorList{field.Invalid(field.NewPath("spec", "local", "path"),
|
||||
cfg.Path, err.Error())}
|
||||
}
|
||||
|
||||
if err := safepath.IsSafe(cfg.Path); err != nil {
|
||||
fields = append(fields, field.Invalid(field.NewPath("spec", "local", "path"),
|
||||
cfg.Path, err.Error()))
|
||||
}
|
||||
|
||||
return fields
|
||||
return nil
|
||||
}
|
||||
|
||||
// Test implements provisioning.Repository.
|
||||
@@ -172,18 +171,7 @@ func (r *localRepository) validateRequest(ref string) error {
|
||||
if ref != "" {
|
||||
return apierrors.NewBadRequest("local repository does not support ref")
|
||||
}
|
||||
if r.path == "" {
|
||||
_, err := r.resolver.LocalPath(r.config.Spec.Local.Path)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
return &apierrors.StatusError{
|
||||
ErrStatus: metav1.Status{
|
||||
Message: "the service is missing a root path",
|
||||
Code: http.StatusFailedDependency,
|
||||
},
|
||||
}
|
||||
}
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
@@ -261,7 +249,7 @@ func (r *localRepository) ReadTree(ctx context.Context, ref string) ([]FileTreeE
|
||||
entry.Blob = true
|
||||
entry.Hash, _, err = r.calculateFileHash(path)
|
||||
if err != nil {
|
||||
return fmt.Errorf("failed to read and calculate hash of path %s: %w", path, err)
|
||||
return fmt.Errorf("read and calculate hash of path %s: %w", path, err)
|
||||
}
|
||||
}
|
||||
// TODO: do folders have a trailing slash?
|
||||
@@ -282,7 +270,7 @@ func (r *localRepository) calculateFileHash(path string) (string, int64, error)
|
||||
//nolint:gosec
|
||||
file, err := os.OpenFile(path, os.O_RDONLY, 0)
|
||||
if err != nil {
|
||||
return "", 0, err
|
||||
return "", 0, fmt.Errorf("open file: %w", err)
|
||||
}
|
||||
|
||||
// TODO: Define what hashing algorithm we want to use for the entire repository. Maybe a config option?
|
||||
@@ -290,7 +278,7 @@ func (r *localRepository) calculateFileHash(path string) (string, int64, error)
|
||||
// TODO: context-aware io.Copy? Is that even possible with a reasonable impl?
|
||||
size, err := io.Copy(hasher, file)
|
||||
if err != nil {
|
||||
return "", 0, err
|
||||
return "", 0, fmt.Errorf("copy file: %w", err)
|
||||
}
|
||||
// NOTE: EncodeToString (& hex.Encode for that matter) return lower-case hex.
|
||||
return hex.EncodeToString(hasher.Sum(nil)), size, nil
|
||||
@@ -339,9 +327,14 @@ func (r *localRepository) Update(ctx context.Context, path string, ref string, d
|
||||
return apierrors.NewBadRequest("cannot update a directory")
|
||||
}
|
||||
|
||||
if _, err := os.Stat(path); errors.Is(err, os.ErrNotExist) {
|
||||
f, err := os.Stat(path)
|
||||
if err != nil && errors.Is(err, os.ErrNotExist) {
|
||||
return ErrFileNotFound
|
||||
}
|
||||
if f.IsDir() {
|
||||
return apierrors.NewBadRequest("path exists but it is a directory")
|
||||
}
|
||||
|
||||
return os.WriteFile(path, data, 0600)
|
||||
}
|
||||
|
||||
|
||||
File diff suppressed because it is too large
Load Diff
@@ -2,8 +2,6 @@ package repository
|
||||
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"fmt"
|
||||
"io"
|
||||
"net/http"
|
||||
"time"
|
||||
@@ -201,14 +199,3 @@ type Versioned interface {
|
||||
LatestRef(ctx context.Context) (string, error)
|
||||
CompareFiles(ctx context.Context, base, ref string) ([]VersionedFileChange, error)
|
||||
}
|
||||
|
||||
func writeWithReadThenCreateOrUpdate(ctx context.Context, r ReaderWriter, path, ref string, data []byte, comment string) error {
|
||||
_, err := r.Read(ctx, path, ref)
|
||||
if err != nil && !(errors.Is(err, ErrFileNotFound)) {
|
||||
return fmt.Errorf("failed to check if file exists before writing: %w", err)
|
||||
}
|
||||
if err == nil {
|
||||
return r.Update(ctx, path, ref, data, comment)
|
||||
}
|
||||
return r.Create(ctx, path, ref, data, comment)
|
||||
}
|
||||
|
||||
@@ -175,6 +175,44 @@ func TestValidateRepository(t *testing.T) {
|
||||
// 3. sync interval too low
|
||||
// 4. reserved name
|
||||
},
|
||||
{
|
||||
name: "branch workflow for non-github repository",
|
||||
repository: func() *MockRepository {
|
||||
m := NewMockRepository(t)
|
||||
m.On("Config").Return(&provisioning.Repository{
|
||||
Spec: provisioning.RepositorySpec{
|
||||
Title: "Test Repo",
|
||||
Type: provisioning.LocalRepositoryType,
|
||||
Workflows: []provisioning.Workflow{provisioning.BranchWorkflow},
|
||||
},
|
||||
})
|
||||
m.On("Validate").Return(field.ErrorList{})
|
||||
return m
|
||||
}(),
|
||||
expectedErrs: 1,
|
||||
validateError: func(t *testing.T, errors field.ErrorList) {
|
||||
require.Contains(t, errors.ToAggregate().Error(), "spec.workflow: Invalid value: \"branch\": branch is only supported on git repositories")
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "invalid workflow in the list",
|
||||
repository: func() *MockRepository {
|
||||
m := NewMockRepository(t)
|
||||
m.On("Config").Return(&provisioning.Repository{
|
||||
Spec: provisioning.RepositorySpec{
|
||||
Title: "Test Repo",
|
||||
Type: provisioning.GitHubRepositoryType,
|
||||
Workflows: []provisioning.Workflow{provisioning.WriteWorkflow, "invalid"},
|
||||
},
|
||||
})
|
||||
m.On("Validate").Return(field.ErrorList{})
|
||||
return m
|
||||
}(),
|
||||
expectedErrs: 1,
|
||||
validateError: func(t *testing.T, errors field.ErrorList) {
|
||||
require.Contains(t, errors.ToAggregate().Error(), "spec.workflow: Invalid value: \"invalid\": invalid workflow")
|
||||
},
|
||||
},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
@@ -324,3 +362,67 @@ func TestTester_TestRepository(t *testing.T) {
|
||||
require.Equal(t, http.StatusOK, results.Code)
|
||||
require.True(t, results.Success)
|
||||
}
|
||||
|
||||
func TestFromFieldError(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
fieldError *field.Error
|
||||
expectedCode int
|
||||
expectedField string
|
||||
expectedType metav1.CauseType
|
||||
expectedDetail string
|
||||
}{
|
||||
{
|
||||
name: "required field error",
|
||||
fieldError: &field.Error{
|
||||
Type: field.ErrorTypeRequired,
|
||||
Field: "spec.title",
|
||||
Detail: "a repository title must be given",
|
||||
},
|
||||
expectedCode: http.StatusBadRequest,
|
||||
expectedField: "spec.title",
|
||||
expectedType: metav1.CauseTypeFieldValueRequired,
|
||||
expectedDetail: "a repository title must be given",
|
||||
},
|
||||
{
|
||||
name: "invalid field error",
|
||||
fieldError: &field.Error{
|
||||
Type: field.ErrorTypeInvalid,
|
||||
Field: "spec.sync.intervalSeconds",
|
||||
Detail: "Interval must be at least 10 seconds",
|
||||
},
|
||||
expectedCode: http.StatusBadRequest,
|
||||
expectedField: "spec.sync.intervalSeconds",
|
||||
expectedType: metav1.CauseTypeFieldValueInvalid,
|
||||
expectedDetail: "Interval must be at least 10 seconds",
|
||||
},
|
||||
{
|
||||
name: "not supported field error",
|
||||
fieldError: &field.Error{
|
||||
Type: field.ErrorTypeNotSupported,
|
||||
Field: "spec.workflow",
|
||||
Detail: "branch is only supported on git repositories",
|
||||
},
|
||||
expectedCode: http.StatusBadRequest,
|
||||
expectedField: "spec.workflow",
|
||||
expectedType: metav1.CauseTypeFieldValueNotSupported,
|
||||
expectedDetail: "branch is only supported on git repositories",
|
||||
},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
result := fromFieldError(tt.fieldError)
|
||||
|
||||
require.NotNil(t, result)
|
||||
require.Equal(t, tt.expectedCode, result.Code)
|
||||
require.False(t, result.Success)
|
||||
require.Len(t, result.Errors, 1)
|
||||
|
||||
errorDetail := result.Errors[0]
|
||||
require.Equal(t, tt.expectedField, errorDetail.Field)
|
||||
require.Equal(t, tt.expectedType, errorDetail.Type)
|
||||
require.Equal(t, tt.expectedDetail, errorDetail.Detail)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user