Initial support for buildEnvs in manifest.yaml (#646)

* initial support for buildEnvs for review

* fix codestyle

* more codestyle

* goimports now working

* adding repo and template level buildEnv checks

* fixing repository test

* updating Envs to []env

* missing Envs

* updating Using

* EOF in yaml file

* go fmt

* go fmt in repository test

repository.go

@@ -27,7 +27,7 @@ const (
 	// DefaultTemplatesPath is the root of the defined repository
 	DefaultTemplatesPath = "."
 
-	// Defaults for Builder and Builders not expressly defined as a pourposeful
+	// Defaults for Builder and Builders not expressly defined as a purposeful
 	// delegation of choice.
 )
 
@@ -57,6 +57,9 @@ type Repository struct {
 	// HealthEndpoints for all templates in the repository.  Serves as the
 	// default option which may be overridden per runtime and per template.
 	HealthEndpoints `yaml:"healthEndpoints,omitempty"`
+	// BuildEnvs define environment variables for builders that can be used
+	//  to parameterize different builders
+	BuildEnvs []Env `yaml:"buildEnvs,omitempty"`
 	// Runtimes containing Templates loaded from the repo
 	Runtimes []Runtime
 	// FS is the filesystem underlying the repository, loaded from URI
@@ -72,13 +75,17 @@ type Repository struct {
 // and libraries)
 type Runtime struct {
 	// Name of the runtime
-	Name string `yaml:"-"` // use filesysem for names
+	Name string `yaml:"-"` // use filesystem for names
 
 	// HealthEndpoints for all templates in the runtime.  May be overridden
 	// per template.
 	HealthEndpoints `yaml:"healthEndpoints,omitempty"`
 
-	// BuildConfig defines attriutes 'builders' and 'buildpacks'.  Here it serves
+	// BuildEnvs for all the templates in the runtime. May be overridden
+	// per template.
+	BuildEnvs []Env `yaml:"buildEnvs,omitempty"`
+
+	// BuildConfig defines attributes 'builders' and 'buildpacks'.  Here it serves
 	// as the default option which may be overridden per template. Note that
 	// unlike HealthEndpoints, it is inline, so no 'buildConfig' attribute is
 	// added/expected; rather the Buildpacks and Builders are direct descendants
@@ -103,9 +110,9 @@ type BuildConfig struct {
 
 // NewRepository creates a repository instance from any of: a path on disk, a
 // remote or local URI, or from the embedded default repo if uri not provided.
-// Name (optional), if provided takes precidence over name derived from repo at
+// Name (optional), if provided takes precedence over name derived from repo at
 //   the given URI.
-// URI (optional), the path either locally or remote from which to load the
+// URI (optional), the path either locally or remote from which to load
 //    the repository files.  If not provided, the internal default is assumed.
 func NewRepository(name, uri string) (r Repository, err error) {
 	r = Repository{
@@ -136,7 +143,7 @@ func NewRepository(name, uri string) (r Repository, err error) {
 
 // filesystemFromURI returns a filesystem from the data located at the
 // given URI.  If URI is not provided, indicates the embedded repo should
-// be loaded.  URI can be a remote git repository (http:// https:// etc),
+// be loaded.  URI can be a remote git repository (http:// https:// etc.),
 // or a local file path (file://) which can be a git repo or a plain directory.
 func filesystemFromURI(uri string) (f Filesystem, err error) {
 	// If not provided, indicates embedded.
@@ -144,7 +151,7 @@ func filesystemFromURI(uri string) (f Filesystem, err error) {
 		return pkgerFilesystem{}, nil
 	}
 
-	// Attempt to get a filesystm from the uri as a remote repo.
+	// Attempt to get a filesystem from the uri as a remote repo.
 	f, err = filesystemFromRepo(uri)
 	if f != nil || err != nil {
 		return // found a filesystem and/or an error
@@ -232,10 +239,11 @@ func repositoryRuntimes(r Repository) (runtimes []Runtime, err error) {
 			Name:            fi.Name(),
 			BuildConfig:     r.BuildConfig,
 			HealthEndpoints: r.HealthEndpoints,
+			BuildEnvs:       r.BuildEnvs,
 		}
 		// Runtime Manifest
 		// Load the file if it exists, which may override values inherited from the
-		// repo such as builders, buildpacks and health endponts.
+		// repo such as builders, buildpacks and health endpoints.
 		runtime, err = applyRuntimeManifest(r, runtime)
 		if err != nil {
 			return
@@ -284,6 +292,7 @@ func runtimeTemplates(r Repository, runtime Runtime) (templates []Template, err
 			Runtime:         runtime.Name,
 			BuildConfig:     runtime.BuildConfig,
 			HealthEndpoints: runtime.HealthEndpoints,
+			BuildEnvs:       runtime.BuildEnvs,
 		}
 
 		// Template Manifeset

repository_test.go

@@ -7,6 +7,8 @@ import (
 	"reflect"
 	"testing"
 
+	"github.com/google/go-cmp/cmp"
+
 	fn "knative.dev/kn-plugin-func"
 )
 
@@ -86,6 +88,21 @@ func TestRepositoryInheritance(t *testing.T) {
 		t.Fatalf("Repository-level HealthEndpoint not loaded to template")
 	}
 
+	envVarName := "TEST_RUNTIME_VARIABLE"
+	envVarValue := "test-runtime"
+	envs := []fn.Env{
+		{
+			Name:  &envVarName,
+			Value: &envVarValue,
+		},
+	}
+
+	if !reflect.DeepEqual(tB.BuildEnvs, envs) {
+		if diff := cmp.Diff(tB.BuildEnvs, envs); 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("Repository-level HealthEndpoint not loaded to template")
@@ -93,4 +110,5 @@ func TestRepositoryInheritance(t *testing.T) {
 	if !reflect.DeepEqual(tC.Buildpacks, []string{"templateBuildpack"}) {
 		t.Fatalf("Repository-level HealthEndpoint not loaded to template")
 	}
+
 }

template.go

@@ -18,6 +18,9 @@ type Template struct {
 	// HealthEndpoints.  The denormalized view of members which can be defined
 	// first per repo or per runtime.
 	HealthEndpoints `yaml:"healthEndpoints,omitempty"`
+	// BuildEnvs defines environment variables related to the builders,
+	// this can be used to parameterize the builders
+	BuildEnvs []Env `yaml:"buildEnvs,omitempty"`
 }
 
 // Fullname is a calculated field of [repo]/[name] used

templates.go

@@ -132,7 +132,7 @@ func (t *Templates) Write(f Function) (Function, error) {
 	// 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 fist case, this default comes from itself
+	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"]
@@ -144,6 +144,9 @@ func (t *Templates) Write(f Function) (Function, error) {
 	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
 	}

templates_test.go

@@ -12,6 +12,8 @@ import (
 	"runtime"
 	"testing"
 
+	"github.com/google/go-cmp/cmp"
+
 	fn "knative.dev/kn-plugin-func"
 	. "knative.dev/kn-plugin-func/testing"
 )
@@ -378,3 +380,151 @@ func TestTemplateModeRemote(t *testing.T) {
 }
 
 // TODO: test typed errors for custom and remote (embedded checked)
+
+// TestRuntimeManifestBuildEnvs ensures that BuildEnvs specified in a
+// runtimes's manifest are included in the final Function.
+func TestRuntimeManifestBuildEnvs(t *testing.T) {
+	// create test directory
+	root := "testdata/testRuntimeManifestBuildEnvs"
+	defer Using(t, root)()
+
+	// Client whose internal templates will be used.
+	client := fn.New(
+		fn.WithRegistry(TestRegistry),
+		fn.WithRepositories("testdata/repositories"))
+
+	// write out a template
+	err := client.Create(fn.Function{
+		Root:     root,
+		Runtime:  "manifestedRuntime",
+		Template: "customLanguagePackRepo/customTemplate",
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// Assert file exists as expected
+	_, err = os.Stat(filepath.Join(root, "func.yaml"))
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	testVariableName := "TEST_RUNTIME_VARIABLE"
+	testVariableValue := "test-runtime"
+
+	envs := []fn.Env{
+		{
+			Name:  &testVariableName,
+			Value: &testVariableValue,
+		},
+	}
+
+	f, err := fn.NewFunction(root)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if !reflect.DeepEqual(f.BuildEnvs, envs) {
+		if diff := cmp.Diff(f.BuildEnvs, envs); diff != "" {
+			t.Fatalf("Unexpected difference between runtime's manifest.yaml buildEnvs and Function BuildEnvs (-want, +got): %v", diff)
+		}
+	}
+}
+
+// TestTemplateManifestBuildEnvs ensures that BuildEnvs specified in a
+// template's manifest are included in the final Function.
+func TestTemplateManifestBuildEnvs(t *testing.T) {
+	// create test directory
+	root := "testdata/testTemplateManifestBuildEnvs"
+	defer Using(t, root)()
+
+	// Client whose internal templates will be used.
+	client := fn.New(
+		fn.WithRegistry(TestRegistry),
+		fn.WithRepositories("testdata/repositories"))
+
+	// write out a template
+	err := client.Create(fn.Function{
+		Root:     root,
+		Runtime:  "manifestedRuntime",
+		Template: "customLanguagePackRepo/manifestedTemplate",
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// Assert file exists as expected
+	_, err = os.Stat(filepath.Join(root, "func.yaml"))
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	testVariableName := "TEST_TEMPLATE_VARIABLE"
+	testVariableValue := "test-template"
+
+	envs := []fn.Env{
+		{
+			Name:  &testVariableName,
+			Value: &testVariableValue,
+		},
+	}
+
+	f, err := fn.NewFunction(root)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if !reflect.DeepEqual(f.BuildEnvs, envs) {
+		if diff := cmp.Diff(f.BuildEnvs, envs); diff != "" {
+			t.Fatalf("Unexpected difference between template's manifest.yaml buildEnvs and Function BuildEnvs (-want, +got): %v", diff)
+		}
+	}
+}
+
+// TestRepositoryManifestBuildEnvs ensures that BuildEnvs specified in a
+// repository's manifest are included in the final Function.
+func TestRepositoryManifestBuildEnvs(t *testing.T) {
+	// create test directory
+	root := "testdata/testRepositoryManifestBuildEnvs"
+	defer Using(t, root)()
+
+	// Client whose internal templates will be used.
+	client := fn.New(
+		fn.WithRegistry(TestRegistry),
+		fn.WithRepositories("testdata/repositories"))
+
+	// write out a template
+	err := client.Create(fn.Function{
+		Root:     root,
+		Runtime:  "customRuntime",
+		Template: "customLanguagePackRepo/customTemplate",
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// Assert file exists as expected
+	_, err = os.Stat(filepath.Join(root, "func.yaml"))
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	testVariableName := "TEST_REPO_VARIABLE"
+	testVariableValue := "test-repo"
+
+	envs := []fn.Env{
+		{
+			Name:  &testVariableName,
+			Value: &testVariableValue,
+		},
+	}
+
+	f, err := fn.NewFunction(root)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if !reflect.DeepEqual(f.BuildEnvs, envs) {
+		if diff := cmp.Diff(f.BuildEnvs, envs); diff != "" {
+			t.Fatalf("Unexpected difference between repository's manifest.yaml buildEnvs and Function BuildEnvs (-want, +got): %v", diff)
+		}
+
+	}
+}

testdata/repositories/customLanguagePackRepo/manifest.yaml

@@ -13,3 +13,7 @@ healthEndpoints:
 # Repo-wide setting for buildpacks
 buildpacks:
   - repoBuildpack
+
+buildEnvs:
+  - name: "TEST_REPO_VARIABLE"
+    value: "test-repo"

testdata/repositories/customLanguagePackRepo/templates/manifestedRuntime/manifest.yaml

@@ -7,4 +7,6 @@ healthEndpoints:
 buildpacks:
   - runtimeBuildpack
 
-
+buildEnvs:
+  - name: "TEST_RUNTIME_VARIABLE"
+    value: "test-runtime"

testdata/repositories/customLanguagePackRepo/templates/manifestedRuntime/manifestedTemplate/manifest.yaml

@@ -6,4 +6,6 @@ healthEndpoints:
 buildpacks:
   - templateBuildpack
 
-
+buildEnvs:
+  - name: "TEST_TEMPLATE_VARIABLE"
+    value: "test-template"