knative
/
func
mirror of https://github.com/knative/func.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Refactor: templates (#961) 

* Refactor: use fs.WalkDir to copy from FS

Signed-off-by: Matej Vasek <mvasek@redhat.com>

* Refactor: rename and added doc comment

Signed-off-by: Matej Vasek <mvasek@redhat.com>

* Refactor: clean up templates

Make `fn.Template` interface not struct.

Signed-off-by: Matej Vasek <mvasek@redhat.com>

* Clean up: mask manifest.yaml from template FS

Signed-off-by: Matej Vasek <mvasek@redhat.com>

* Refactor: receiver is not pointer

Signed-off-by: Matej Vasek <mvasek@redhat.com>

* Refactor: use const instead of string literal

Signed-off-by: Matej Vasek <mvasek@redhat.com>

* fixup: TemplatesPath defaulting at correct place

Signed-off-by: Matej Vasek <mvasek@redhat.com>

* fixup: mask manifest.yaml in template code

Signed-off-by: Matej Vasek <mvasek@redhat.com>

* fixup: remove unnecessary else

Signed-off-by: Matej Vasek <mvasek@redhat.com>

* fixup: put docstring on interface

Signed-off-by: Matej Vasek <mvasek@redhat.com>

* fixup: test calls better API

Signed-off-by: Matej Vasek <mvasek@redhat.com>

* fixup: check return value

Signed-off-by: Matej Vasek <mvasek@redhat.com>
This commit is contained in:
Matej Vasek 2022-04-14 13:29:10 +02:00 committed by GitHub
parent 4ffb1f9cba
commit 34cb893545
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 258 additions and 192 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
client.go
View File

@ -546,7 +546,7 @@ func (c *Client) Create(cfg Function) (err error) {
// Templates contain values which may result in the Function being mutated
// (default builders, etc), so a new (potentially mutated) Function is
// returned from Templates.Write
f, err = c.Templates().Write(f)
err = c.Templates().Write(&f)
if err != nil {
return
}

150
filesystem.go
View File

@ -8,7 +8,6 @@ import (
"os"
"path"
"path/filepath"
"sort"
"strings"
billy "github.com/go-git/go-billy/v5"
@ -145,70 +144,107 @@ func (o osFilesystem) Stat(name string) (fs.FileInfo, error) {
return os.Stat(filepath.Join(o.root, name))
}
// copy
func copy(src, dest string, accessor Filesystem) (err error) {
node, err := accessor.Stat(src)
if err != nil {
return
}
if node.IsDir() {
return copyNode(src, dest, accessor)
} else {
return copyLeaf(src, dest, accessor)
}
// subFS exposes subdirectory of underlying FS, this is similar to `chroot`.
type subFS struct {
root string
fs Filesystem
}
func copyNode(src, dest string, accessor Filesystem) (err error) {
// Ideally we should use the file mode of the src node
// but it seems the git module is reporting directories
// as 0644 instead of 0755. For now, just do it this way.
// See https://github.com/go-git/go-git/issues/364
// Upon resolution, return accessor.Stat(src).Mode()
err = os.MkdirAll(dest, 0755)
if err != nil {
return
}
children, err := readDir(src, accessor)
if err != nil {
return
}
for _, child := range children {
if err = copy(path.Join(src, child.Name()), filepath.Join(dest, child.Name()), accessor); err != nil {
return
}
}
return
func (o subFS) Open(name string) (fs.File, error) {
return o.fs.Open(path.Join(o.root, name))
}
func readDir(src string, accessor Filesystem) ([]fs.DirEntry, error) {
list, err := accessor.ReadDir(src)
func (o subFS) ReadDir(name string) ([]fs.DirEntry, error) {
return o.fs.ReadDir(path.Join(o.root, name))
}
func (o subFS) Stat(name string) (fs.FileInfo, error) {
return o.fs.Stat(path.Join(o.root, name))
}
type maskingFS struct {
masked func(path string) bool
fs Filesystem
}
func (m maskingFS) Open(name string) (fs.File, error) {
if m.masked(name) {
return nil, &fs.PathError{Op: "open", Path: name, Err: fs.ErrNotExist}
}
return m.fs.Open(name)
}
func (m maskingFS) ReadDir(name string) ([]fs.DirEntry, error) {
if m.masked(name) {
return nil, &fs.PathError{Op: "readdir", Path: name, Err: fs.ErrNotExist}
}
des, err := m.fs.ReadDir(name)
if err != nil {
return nil, err
}
sort.Slice(list, func(i, j int) bool { return list[i].Name() < list[j].Name() })
return list, nil
result := make([]fs.DirEntry, 0, len(des))
for _, de := range des {
if !m.masked(path.Join(name, de.Name())) {
result = append(result, de)
}
}
return result, nil
}
func copyLeaf(src, dest string, accessor Filesystem) (err error) {
srcFile, err := accessor.Open(src)
if err != nil {
return
func (m maskingFS) Stat(name string) (fs.FileInfo, error) {
if m.masked(name) {
return nil, &fs.PathError{Op: "stat", Path: name, Err: fs.ErrNotExist}
}
defer srcFile.Close()
srcFileInfo, err := accessor.Stat(src)
if err != nil {
return
}
destFile, err := os.OpenFile(dest, os.O_RDWR|os.O_CREATE|os.O_TRUNC, srcFileInfo.Mode())
if err != nil {
return
}
defer destFile.Close()
_, err = io.Copy(destFile, srcFile)
return
return m.fs.Stat(name)
}
// copyFromFS copies files from the `src` dir on the accessor Filesystem to local filesystem into `dest` dir.
// The src path uses slashes as their separator.
// The dest path uses OS specific separator.
func copyFromFS(src, dest string, accessor Filesystem) (err error) {
return fs.WalkDir(accessor, src, func(path string, de fs.DirEntry, err error) error {
if err != nil {
return err
}
if path == src {
return nil
}
p, err := filepath.Rel(filepath.FromSlash(src), filepath.FromSlash(path))
if err != nil {
return err
}
dest := filepath.Join(dest, p)
if de.IsDir() {
// Ideally we should use the file mode of the src node
// but it seems the git module is reporting directories
// as 0644 instead of 0755. For now, just do it this way.
// See https://github.com/go-git/go-git/issues/364
// Upon resolution, return accessor.Stat(src).Mode()
return os.MkdirAll(dest, 0755)
}
fi, err := de.Info()
if err != nil {
return err
}
destFile, err := os.OpenFile(dest, os.O_RDWR|os.O_CREATE|os.O_TRUNC, fi.Mode())
if err != nil {
return err
}
defer destFile.Close()
srcFile, err := accessor.Open(path)
if err != nil {
return err
}
defer srcFile.Close()
_, err = io.Copy(destFile, srcFile)
return err
})
}

62
repository.go
View File

@ -145,6 +145,18 @@ func NewRepository(name, uri string) (r Repository, err error) {
if err != nil {
return
}
// Validate custom path if defined
if r.TemplatesPath != "" {
if err = checkDir(r.FS, r.TemplatesPath); err != nil {
err = fmt.Errorf("templates path '%v' does not exist in repo '%v'. %v",
r.TemplatesPath, r.Name, err)
return
}
} else {
r.TemplatesPath = DefaultTemplatesPath
}
r.Name, err = repositoryDefaultName(r.DefaultName, uri) // choose default name
if err != nil {
return
@ -233,20 +245,7 @@ func filesystemFromPath(uri string) (f Filesystem, err error) {
func repositoryRuntimes(r Repository) (runtimes []Runtime, err error) {
runtimes = []Runtime{}
// Validate custom path if defined
if r.TemplatesPath != "" {
if err = checkDir(r.FS, r.TemplatesPath); err != nil {
err = fmt.Errorf("templates path '%v' does not exist in repo '%v'. %v",
r.TemplatesPath, r.Name, err)
return
}
}
// Load runtimes
if r.TemplatesPath == "" {
r.TemplatesPath = "."
}
fis, err := r.FS.ReadDir(r.TemplatesPath)
if err != nil {
return
@ -289,8 +288,6 @@ func repositoryRuntimes(r Repository) (runtimes []Runtime, err error) {
// runtime for defaults of BuildConfig andHealthEndpoints. The template itself
// can override these by including a manifest.
func runtimeTemplates(r Repository, runtime Runtime) (templates []Template, err error) {
templates = []Template{}
// Validate runtime directory exists and is a directory
runtimePath := path.Join(r.TemplatesPath, runtime.Name)
if err = checkDir(r.FS, runtimePath); err != nil {
@ -309,14 +306,17 @@ func runtimeTemplates(r Repository, runtime Runtime) (templates []Template, err
continue
}
// Template, defaulted to values inherited from the runtime
t := Template{
Name: fi.Name(),
Repository: r.Name,
Runtime: runtime.Name,
BuildConfig: runtime.BuildConfig,
HealthEndpoints: runtime.HealthEndpoints,
BuildEnvs: runtime.BuildEnvs,
Invocation: runtime.Invocation,
t := template{
name: fi.Name(),
repository: r.Name,
runtime: runtime.Name,
manifest: templateConfig{
BuildConfig: runtime.BuildConfig,
HealthEndpoints: runtime.HealthEndpoints,
BuildEnvs: runtime.BuildEnvs,
Invocation: runtime.Invocation,
},
fs: subFS{root: path.Join(runtimePath, fi.Name()), fs: r.FS},
}
// Template Manifeset
@ -326,7 +326,7 @@ func runtimeTemplates(r Repository, runtime Runtime) (templates []Template, err
if err != nil {
return
}
templates = append(templates, t)
templates = append(templates, &t)
}
return
}
@ -390,8 +390,8 @@ func applyRuntimeManifest(repo Repository, runtime Runtime) (Runtime, error) {
// is the template with values from the manifest populated preferentailly. An
// error is not returned for a missing manifest file (the passed template is
// returned), but errors decoding the file are.
func applyTemplateManifest(repo Repository, t Template) (Template, error) {
file, err := repo.FS.Open(path.Join(repo.TemplatesPath, t.Runtime, t.Name, templateManifest))
func applyTemplateManifest(repo Repository, t template) (template, error) {
file, err := repo.FS.Open(path.Join(repo.TemplatesPath, t.runtime, t.Name(), templateManifest))
if err != nil {
if os.IsNotExist(err) {
return t, nil
@ -399,7 +399,7 @@ func applyTemplateManifest(repo Repository, t Template) (Template, error) {
return t, err
}
decoder := yaml.NewDecoder(file)
return t, decoder.Decode(&t)
return t, decoder.Decode(&t.manifest)
}
// check that the given path is an accessible directory or error.
@ -421,11 +421,11 @@ func (r *Repository) Template(runtimeName, name string) (t Template, err error)
return
}
for _, t := range runtime.Templates {
if t.Name == name {
if t.Name() == name {
return t, nil
}
}
return Template{}, ErrTemplateNotFound
return nil, ErrTemplateNotFound
}
// Templates returns the set of all templates for a given runtime.
@ -436,7 +436,7 @@ func (r *Repository) Templates(runtimeName string) ([]Template, error) {
return runtime.Templates, nil
}
}
return []Template{}, nil
return nil, nil
}
// Runtime of the given name within the repository.
@ -482,7 +482,7 @@ func (r *Repository) Write(path string) (err error) {
}
fs = billyFilesystem{fs: wt.Filesystem}
}
return copy(".", path, fs)
return copyFromFS(".", path, fs)
}
// URL attempts to read the remote git origin URL of the repository. Best

63
repository_test.go
View File

@ -4,6 +4,7 @@
package function_test
import (
"context"
"testing"
"github.com/google/go-cmp/cmp"
@ -29,8 +30,8 @@ func TestRepository_TemplatesPath(t *testing.T) {
}
// degenerate case: API of a custom repository should return what it was
// expressly asked for at minimum (known good request)
if template.Name != "customTemplate" {
t.Logf("expected custom language pack repo to yield a template named 'customTemplate', got '%v'", template.Name)
if template.Name() != "customTemplate" {
t.Logf("expected custom language pack repo to yield a template named 'customTemplate', got '%v'", template.Name())
}
}
@ -39,13 +40,9 @@ func TestRepository_TemplatesPath(t *testing.T) {
// and template level. The tests check for both embedded structures:
// HealthEndpoints BuildConfig.
func TestRepository_Inheritance(t *testing.T) {
var err error
client := fn.New(fn.WithRepositoriesPath("testdata/repositories"))
// The repo ./testdata/repositories/customLanguagePack includes a manifest
// which defines custom readiness and liveness endpoints.
// The runtime "manifestedRuntime" includes a manifest which sets these
// for all templates within, and the template "manifestedTemplate" sets
// them explicitly for itself.
repo, err := client.Repositories().Get("customLanguagePackRepo")
if err != nil {
t.Fatal(err)
@ -54,36 +51,62 @@ func TestRepository_Inheritance(t *testing.T) {
// Template A: from a path containing no settings other than the repo root.
// Should have a readiness and liveness equivalent to that defined in
// [repo]/manifest.yaml
fA := fn.Function{
Name: "fn-a",
Root: t.TempDir(),
}
tA, err := repo.Template("customRuntime", "customTemplate")
if err != nil {
t.Fatal(err)
t.Error(err)
}
err = tA.Write(context.Background(), &fA)
if err != nil {
t.Error(err)
}
// Template B: from a path containing runtime-wide settings, but no
// template-level settings.
fB := fn.Function{
Name: "fn-b",
Root: t.TempDir(),
}
tB, err := repo.Template("manifestedRuntime", "customTemplate")
if err != nil {
t.Fatal(err)
t.Error(err)
}
err = tB.Write(context.Background(), &fB)
if err != nil {
t.Error(err)
}
// Template C: from a runtime with a manifest which sets endpoints, and
// itself includes a manifest which explicitly sets.
fC := fn.Function{
Name: "fn-c",
Root: t.TempDir(),
}
tC, err := repo.Template("manifestedRuntime", "manifestedTemplate")
if err != nil {
t.Fatal(err)
t.Error(err)
}
err = tC.Write(context.Background(), &fC)
if err != nil {
t.Error(err)
}
// Assert Template A reflects repo-level settings
if tA.Readiness != "/repoReadiness" {
t.Errorf("Repository-level HealthEndpoint not loaded to template, got %q", tA.Readiness)
if fA.HealthEndpoints.Readiness != "/repoReadiness" {
t.Errorf("Repository-level HealthEndpoint not loaded to template, got %q", fA.HealthEndpoints.Readiness)
}
if diff := cmp.Diff([]string{"repoBuildpack"}, tA.Buildpacks); diff != "" {
if diff := cmp.Diff([]string{"repoBuildpack"}, fA.Buildpacks); diff != "" {
t.Errorf("Repository-level Buildpack differs (-want, +got): %s", diff)
}
// Assert Template B reflects runtime-level settings
if tB.Readiness != "/runtimeReadiness" {
t.Errorf("Runtime-level HealthEndpoint not loaded to template, got %q", tB.Readiness)
if fB.HealthEndpoints.Readiness != "/runtimeReadiness" {
t.Errorf("Runtime-level HealthEndpoint not loaded to template, got %q", fB.HealthEndpoints.Readiness)
}
if diff := cmp.Diff([]string{"runtimeBuildpack"}, tB.Buildpacks); diff != "" {
if diff := cmp.Diff([]string{"runtimeBuildpack"}, fB.Buildpacks); diff != "" {
t.Errorf("Runtime-level Buildpack differs (-want, +got): %s", diff)
}
@ -96,15 +119,15 @@ func TestRepository_Inheritance(t *testing.T) {
},
}
if diff := cmp.Diff(envs, tB.BuildEnvs); diff != "" {
if diff := cmp.Diff(envs, fB.BuildEnvs); diff != "" {
t.Fatalf("Unexpected difference between repository's manifest.yaml buildEnvs and Function BuildEnvs (-want, +got): %v", diff)
}
// Assert Template C reflects template-level settings
if tC.Readiness != "/templateReadiness" {
t.Fatalf("Template-level HealthEndpoint not loaded to template, got %q", tC.Readiness)
if fC.HealthEndpoints.Readiness != "/templateReadiness" {
t.Fatalf("Template-level HealthEndpoint not loaded to template, got %q", fC.HealthEndpoints.Readiness)
}
if diff := cmp.Diff([]string{"templateBuildpack"}, tC.Buildpacks); diff != "" {
if diff := cmp.Diff([]string{"templateBuildpack"}, fC.Buildpacks); diff != "" {
t.Fatalf("Template-level Buildpack differs (-want, +got): %s", diff)
}
}

93
template.go
View File

@ -1,20 +1,29 @@
package function
// Template
type Template struct {
// Name (short name) of this template within the repository.
// See .Fullname for the calculated field which is the unique primary id.
Name string `yaml:"-"` // use filesystem for name, not yaml
import (
"context"
"path"
)
type Template interface {
// Name of this template.
Name() string
// Runtime for which this template applies.
Runtime string
Runtime() string
// Repository within which this template is contained. Value is set to the
// currently effective name of the repository, which may vary. It is user-
// defined when the repository is added, and can be set to "default" when
// the client is loaded in single repo mode. I.e. not canonical.
Repository string
Repository() string
// Fullname is a calculated field of [repo]/[name] used
// to uniquely reference a template which may share a name
// with one in another repository.
Fullname() string
// Write updates fields of Function f and writes project files to path pointed by f.Root.
Write(ctx context.Context, f *Function) error
}
type templateConfig struct {
// BuildConfig defines builders and buildpacks. the denormalized view of
// members which can be defined per repo or per runtime first.
BuildConfig `yaml:",inline"`
@ -32,9 +41,67 @@ type Template struct {
Invocation Invocation `yaml:"invocation,omitempty"`
}
// Fullname is a calculated field of [repo]/[name] used
// to uniquely reference a template which may share a name
// with one in another repository.
func (t Template) Fullname() string {
return t.Repository + "/" + t.Name
// template
type template struct {
name string
runtime string
repository string
fs Filesystem
manifest templateConfig
}
func (t template) Name() string {
return t.name
}
func (t template) Runtime() string {
return t.runtime
}
func (t template) Repository() string {
return t.repository
}
func (t template) Fullname() string {
return t.repository + "/" + t.name
}
func (t template) Write(ctx context.Context, f *Function) error {
// Apply fields from the template onto the function itself (Denormalize).
// The template is already the denormalized view of repo->runtime->template
// so it's values are treated as defaults.
// TODO: this begs the question: should the Template's manifest.yaml actually
// be a partially-populated func.yaml?
if f.Builder == "" { // as a special first case, this default comes from itself
f.Builder = f.Builders["default"]
if f.Builder == "" { // still nothing? then use the template
f.Builder = t.manifest.Builders["default"]
}
}
if len(f.Builders) == 0 {
f.Builders = t.manifest.Builders
}
if len(f.Buildpacks) == 0 {
f.Buildpacks = t.manifest.Buildpacks
}
if len(f.BuildEnvs) == 0 {
f.BuildEnvs = t.manifest.BuildEnvs
}
if f.HealthEndpoints.Liveness == "" {
f.HealthEndpoints.Liveness = t.manifest.HealthEndpoints.Liveness
}
if f.HealthEndpoints.Readiness == "" {
f.HealthEndpoints.Readiness = t.manifest.HealthEndpoints.Readiness
}
if f.Invocation.Format == "" {
f.Invocation.Format = t.manifest.Invocation.Format
}
isManifest := func(p string) bool {
_, f := path.Split(p)
return f == templateManifest
}
return copyFromFS(".", f.Root, maskingFS{fs: t.fs, masked: isManifest}) // copy everything but manifest.yaml
}

72
templates.go
View File

@ -1,10 +1,8 @@
package function
import (
"context"
"errors"
"os"
"path"
"path/filepath"
"strings"
)
@ -49,7 +47,7 @@ func (t *Templates) List(runtime string) ([]string, error) {
}
for _, t := range tt {
if r.Name == DefaultRepositoryName {
names = append(names, t.Name)
names = append(names, t.Name())
} else {
extended.Add(t.Fullname())
}
@ -95,76 +93,18 @@ func (t *Templates) Get(runtime, fullname string) (Template, error) {
// Returns a Function which may have been modified dependent on the content
// of the template (which can define default Function fields, builders,
// buildpacks, etc)
func (t *Templates) Write(f Function) (Function, error) {
func (t *Templates) Write(f *Function) error {
// Templates require an initially valid Function to write
// (has name, path, runtime etc)
if err := f.Validate(); err != nil {
return f, err
return err
}
// The Function's Template
template, err := t.Get(f.Runtime, f.Template)
if err != nil {
return f, err
return err
}
// The Function's Template Repository
repo, err := t.client.Repositories().Get(template.Repository)
if err != nil {
return f, err
}
// Validate paths: (repo/)[templates/]<runtime>/<template>
templatesPath := repo.TemplatesPath
if templatesPath == "" {
templatesPath = "."
}
if _, err := repo.FS.Stat(templatesPath); err != nil {
return f, ErrTemplatesNotFound
}
runtimePath := path.Join(templatesPath, template.Runtime)
if _, err := repo.FS.Stat(runtimePath); err != nil {
return f, ErrRuntimeNotFound
}
templatePath := path.Join(runtimePath, template.Name)
if _, err := repo.FS.Stat(templatePath); err != nil {
return f, ErrTemplateNotFound
}
// Apply fields from the template onto the function itself (Denormalize).
// The template is already the denormalized view of repo->runtime->template
// so it's values are treated as defaults.
// TODO: this begs the question: should the Template's manifest.yaml actually
// be a partially-populated func.yaml?
if f.Builder == "" { // as a special first case, this default comes from itself
f.Builder = f.Builders["default"]
if f.Builder == "" { // still nothing? then use the template
f.Builder = template.Builders["default"]
}
}
if len(f.Builders) == 0 {
f.Builders = template.Builders
}
if len(f.Buildpacks) == 0 {
f.Buildpacks = template.Buildpacks
}
if len(f.BuildEnvs) == 0 {
f.BuildEnvs = template.BuildEnvs
}
if f.HealthEndpoints.Liveness == "" {
f.HealthEndpoints.Liveness = template.HealthEndpoints.Liveness
}
if f.HealthEndpoints.Readiness == "" {
f.HealthEndpoints.Readiness = template.HealthEndpoints.Readiness
}
if f.Invocation.Format == "" {
f.Invocation.Format = template.Invocation.Format
}
// Copy the template files from the repo filesystem to the new Function's root
// removing the manifest (if it exists; errors ignored)
err = copy(templatePath, f.Root, repo.FS) // copy everything
_ = os.Remove(filepath.Join(f.Root, templateManifest)) // except the manifest
return f, err
return template.Write(context.TODO(), f)
}

8
templates_test.go
View File

@ -78,9 +78,9 @@ func TestTemplates_Get(t *testing.T) {
t.Fatal(err)
}
if embedded.Runtime != "go" || embedded.Repository != "default" || embedded.Name != "http" {
if embedded.Runtime() != "go" || embedded.Repository() != "default" || embedded.Name() != "http" {
t.Logf("Expected template from embedded to have runtime 'go' repo 'default' name 'http', got '%v', '%v', '%v',",
embedded.Runtime, embedded.Repository, embedded.Name)
embedded.Runtime(), embedded.Repository(), embedded.Name())
}
// Check extended
@ -89,9 +89,9 @@ func TestTemplates_Get(t *testing.T) {
t.Fatal(err)
}
if embedded.Runtime != "go" || embedded.Repository != "default" || embedded.Name != "http" {
if embedded.Runtime() != "go" || embedded.Repository() != "default" || embedded.Name() != "http" {
t.Logf("Expected template from extended repo to have runtime 'go' repo 'customTemplateRepo' name 'customTemplate', got '%v', '%v', '%v',",
extended.Runtime, extended.Repository, extended.Name)
extended.Runtime(), extended.Repository(), extended.Name())
}
}