From 597195bab81aa5f69540f8524924529b22611c99 Mon Sep 17 00:00:00 2001 From: salaboy Date: Mon, 15 Nov 2021 13:35:55 +0000 Subject: [PATCH] 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 | 25 ++- repository_test.go | 18 +++ template.go | 3 + templates.go | 5 +- templates_test.go | 150 ++++++++++++++++++ .../customLanguagePackRepo/manifest.yaml | 4 + .../templates/manifestedRuntime/manifest.yaml | 4 +- .../manifestedTemplate/manifest.yaml | 4 +- 8 files changed, 202 insertions(+), 11 deletions(-) diff --git a/repository.go b/repository.go index d9500a91c..3939c28b6 100644 --- a/repository.go +++ b/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 diff --git a/repository_test.go b/repository_test.go index e5a55d37d..febcef7ad 100644 --- a/repository_test.go +++ b/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") } + } diff --git a/template.go b/template.go index 1b5a14d3d..d7cd48113 100644 --- a/template.go +++ b/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 diff --git a/templates.go b/templates.go index 08a038d90..37d4f2eab 100644 --- a/templates.go +++ b/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 } diff --git a/templates_test.go b/templates_test.go index 5ab1a0a13..1f8be1b94 100644 --- a/templates_test.go +++ b/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) + } + + } +} diff --git a/testdata/repositories/customLanguagePackRepo/manifest.yaml b/testdata/repositories/customLanguagePackRepo/manifest.yaml index 791d3ae9f..75951a602 100644 --- a/testdata/repositories/customLanguagePackRepo/manifest.yaml +++ b/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" diff --git a/testdata/repositories/customLanguagePackRepo/templates/manifestedRuntime/manifest.yaml b/testdata/repositories/customLanguagePackRepo/templates/manifestedRuntime/manifest.yaml index 9f37ed0ed..03dbe52bb 100644 --- a/testdata/repositories/customLanguagePackRepo/templates/manifestedRuntime/manifest.yaml +++ b/testdata/repositories/customLanguagePackRepo/templates/manifestedRuntime/manifest.yaml @@ -7,4 +7,6 @@ healthEndpoints: buildpacks: - runtimeBuildpack - +buildEnvs: + - name: "TEST_RUNTIME_VARIABLE" + value: "test-runtime" diff --git a/testdata/repositories/customLanguagePackRepo/templates/manifestedRuntime/manifestedTemplate/manifest.yaml b/testdata/repositories/customLanguagePackRepo/templates/manifestedRuntime/manifestedTemplate/manifest.yaml index 28b3f5bb7..800425229 100644 --- a/testdata/repositories/customLanguagePackRepo/templates/manifestedRuntime/manifestedTemplate/manifest.yaml +++ b/testdata/repositories/customLanguagePackRepo/templates/manifestedRuntime/manifestedTemplate/manifest.yaml @@ -6,4 +6,6 @@ healthEndpoints: buildpacks: - templateBuildpack - +buildEnvs: + - name: "TEST_TEMPLATE_VARIABLE" + value: "test-template"