From 641f54dc1ad5e74b79081d0b8c6f838c44b2d613 Mon Sep 17 00:00:00 2001 From: Pedro Almeida Date: Tue, 1 Sep 2026 17:17:02 +0100 Subject: [PATCH] feat: configure CI workflow builder through WorkflowConfig and func.yaml - Add Builder field to WorkflowConfig so the builder flows through the established config pattern alongside all other workflow settings - Generate() resolves builder with priority: WorkflowConfig.Builder > f.Build.Builder (func.yaml) > runtime default; g.cfg is never mutated - Validate explicit builder values against the known set; unknown values produce a clear error instead of a broken workflow - Validate that builder "host" is incompatible with remote builds --- pkg/ci/github/common.go | 27 +++++++++- pkg/ci/github/generator.go | 13 +++-- pkg/ci/github/generator_test.go | 94 +++++++++++++++++++++++++++++++++ pkg/ci/github/printer.go | 2 +- pkg/ci/github/workflow.go | 5 +- 5 files changed, 132 insertions(+), 9 deletions(-) diff --git a/pkg/ci/github/common.go b/pkg/ci/github/common.go index 5fd105c86c..c094e1806e 100644 --- a/pkg/ci/github/common.go +++ b/pkg/ci/github/common.go @@ -1,8 +1,31 @@ package github -import "fmt" +import ( + "fmt" + "slices" -func determineBuilder(runtime string, remote bool) (string, error) { + "knative.dev/func/pkg/builders" +) + +func determineBuilder(runtime, builder string, remote bool) (string, error) { + defaultBuilder, err := runtimeDefaultBuilder(runtime, remote) + if err != nil { + return "", err + } + if builder == "" { + return defaultBuilder, nil + } + all := builders.All() + if !slices.Contains(all, builder) { + return "", builders.ErrUnknownBuilder{Name: builder, Known: all} + } + if remote && builder == builders.Host { + return "", fmt.Errorf("builder %q is incompatible with remote builds", builders.Host) + } + return builder, nil +} + +func runtimeDefaultBuilder(runtime string, remote bool) (string, error) { switch runtime { case "go": if remote { diff --git a/pkg/ci/github/generator.go b/pkg/ci/github/generator.go index 6d5ed696d2..fe2ba1b0e8 100644 --- a/pkg/ci/github/generator.go +++ b/pkg/ci/github/generator.go @@ -78,23 +78,28 @@ func (g *workflowGenerator) Generate(ctx context.Context, f fn.Function) error { return fmt.Errorf("function root path can not be empty") } - githubWorkflow, err := newGitHubWorkflow(g.cfg, f.Runtime, g.messageWriter) + cfg := g.cfg + if cfg.Builder == "" { + cfg.Builder = f.Build.Builder + } + + githubWorkflow, err := newGitHubWorkflow(cfg, f.Runtime, g.messageWriter) if err != nil { return err } - if err := githubWorkflow.Export(g.cfg.fnGitHubWorkflowFilepath(f.Root), g.workflowWriter, g.cfg.Force, g.messageWriter); err != nil { + if err := githubWorkflow.Export(cfg.fnGitHubWorkflowFilepath(f.Root), g.workflowWriter, cfg.Force, g.messageWriter); err != nil { return err } if g.verbose { // best-effort user message; errors are non-critical - _ = PrintConfiguration(g.cfg, f.Runtime, g.messageWriter) + _ = PrintConfiguration(cfg, f.Runtime, g.messageWriter) return nil } // best-effort user message; errors are non-critical - _ = PrintPostExportMessage(g.cfg, g.messageWriter) + _ = PrintPostExportMessage(cfg, g.messageWriter) return nil } diff --git a/pkg/ci/github/generator_test.go b/pkg/ci/github/generator_test.go index 6f8902a96a..97d0568dee 100644 --- a/pkg/ci/github/generator_test.go +++ b/pkg/ci/github/generator_test.go @@ -9,6 +9,7 @@ import ( "gotest.tools/v3/assert" "gotest.tools/v3/assert/cmp" + "knative.dev/func/pkg/builders" "knative.dev/func/pkg/ci/github" fn "knative.dev/func/pkg/functions" ) @@ -518,6 +519,74 @@ func TestCIGenerator_BuilderForRuntime(t *testing.T) { } } +func TestCIGenerator_BuilderConfiguredInFuncYaml(t *testing.T) { + testCases := []struct { + name string + runtime string + builder string + }{ + { + name: "node function with s2i builder configured", + runtime: "node", + builder: "s2i", + }, + { + name: "python function with pack builder configured", + runtime: "python", + builder: "pack", + }, + { + name: "quarkus function with s2i builder configured", + runtime: "quarkus", + builder: "s2i", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + // GIVEN + opts := defaultOpts() + opts.goFn.Runtime = tc.runtime + opts.goFn.Build.Builder = tc.builder + + // WHEN + result := runGenerateWorkflow(t, opts) + + // THEN + assert.NilError(t, result.executeErr) + assert.Assert(t, strings.Contains(result.gwYamlString, "FUNC_BUILDER: "+tc.builder)) + }) + } +} + +func TestCIGenerator_WorkflowConfigBuilderTakesPriorityOverFuncYaml(t *testing.T) { + // GIVEN + opts := defaultOpts() + opts.goFn.Runtime = "node" + opts.goFn.Build.Builder = "s2i" + opts.cfg.Builder = "pack" + + // WHEN + result := runGenerateWorkflow(t, opts) + + // THEN + assert.NilError(t, result.executeErr) + assert.Assert(t, strings.Contains(result.gwYamlString, "FUNC_BUILDER: "+opts.cfg.Builder)) +} + +func TestCIGenerator_HostBuilderWithRemoteErrors(t *testing.T) { + // GIVEN + opts := defaultOpts() + opts.cfg.Builder = builders.Host + opts.cfg.RemoteBuild = true + + // WHEN + result := runGenerateWorkflow(t, opts) + + // THEN + assert.Error(t, result.executeErr, `builder "host" is incompatible with remote builds`) +} + func TestCIGenerator_BuilderForRuntimeError(t *testing.T) { // GIVEN opts := defaultOpts() @@ -530,6 +599,31 @@ func TestCIGenerator_BuilderForRuntimeError(t *testing.T) { assert.Error(t, result.executeErr, "no builder support for runtime: zig") } +func TestCIGenerator_UnsupportedRuntimeErrorsEvenWithExplicitBuilder(t *testing.T) { + // GIVEN + opts := defaultOpts() + opts.goFn.Runtime = "zig" + opts.goFn.Build.Builder = "pack" + + // WHEN + result := runGenerateWorkflow(t, opts) + + // THEN + assert.Error(t, result.executeErr, "no builder support for runtime: zig") +} + +func TestCIGenerator_InvalidBuilderErrors(t *testing.T) { + // GIVEN + opts := defaultOpts() + opts.goFn.Build.Builder = "pakc" + + // WHEN + result := runGenerateWorkflow(t, opts) + + // THEN + assert.Error(t, result.executeErr, `"pakc" is not a known builder. Available builders are "host", "pack" and "s2i"`) +} + // --------------------- // END: Broad Unit Tests diff --git a/pkg/ci/github/printer.go b/pkg/ci/github/printer.go index 962801d373..d9c4e0c07c 100644 --- a/pkg/ci/github/printer.go +++ b/pkg/ci/github/printer.go @@ -50,7 +50,7 @@ Create the following Secret on github.com: %s ) func PrintConfiguration(cfg WorkflowConfig, runtime string, w io.Writer) error { - builder, err := determineBuilder(runtime, cfg.RemoteBuild) + builder, err := determineBuilder(runtime, cfg.Builder, cfg.RemoteBuild) if err != nil { return err } diff --git a/pkg/ci/github/workflow.go b/pkg/ci/github/workflow.go index e4eae9d90d..39b4204664 100644 --- a/pkg/ci/github/workflow.go +++ b/pkg/ci/github/workflow.go @@ -46,7 +46,8 @@ type WorkflowConfig struct { RegistryUserVar, RegistryPassSecret, RegistryUrlVar, - FuncCliVersion string + FuncCliVersion, + Builder string RegistryLogin, SelfHostedRunner, RemoteBuild, @@ -241,7 +242,7 @@ func createFuncDeployStep(opts WorkflowConfig, runtime string, steps []step) ([] deployFuncStep := newStep("Deploy function"). withEnv("FUNC_VERBOSE", "true") - builder, err := determineBuilder(runtime, opts.RemoteBuild) + builder, err := determineBuilder(runtime, opts.Builder, opts.RemoteBuild) if err != nil { return nil, err }