diff --git a/cmd/deploy.go b/cmd/deploy.go index e5a1a8af29..eaba56427d 100644 --- a/cmd/deploy.go +++ b/cmd/deploy.go @@ -320,12 +320,6 @@ func runDeploy(cmd *cobra.Command, newClient ClientFactory) (err error) { // Deploy if cfg.Remote { - // Write func.yaml before the pipeline uploads sources to the PVC, - // so that the on-cluster deploy step sees the latest config - // (e.g. --image-pull-secret, --service-account, --deployer). - if err = f.Write(); err != nil { - return - } var url string // Invoke a remote build/push/deploy pipeline // Returned is the function with fields like Registry, f.Deploy.Image & diff --git a/cmd/deploy_test.go b/cmd/deploy_test.go index 540cb4e15f..f753f4659b 100644 --- a/cmd/deploy_test.go +++ b/cmd/deploy_test.go @@ -1912,9 +1912,9 @@ func TestDeploy_ImagePullSecretFromEnv(t *testing.T) { } } -// TestDeploy_ImagePullSecretRemote ensures that when deploying remotely, -// func.yaml is written to disk before the pipeline starts, so the on-cluster -// deploy step picks up the --image-pull-secret value. +// TestDeploy_ImagePullSecretRemote: a remote deploy hands the effective config +// (here --image-pull-secret) to the pipeline and persists func.yaml only after +// success — never mutating the working tree on a mere attempt. func TestDeploy_ImagePullSecretRemote(t *testing.T) { root := FromTempDirectory(t) @@ -1926,14 +1926,17 @@ func TestDeploy_ImagePullSecretRemote(t *testing.T) { pipelinesProvider := mock.NewPipelinesProvider() pipelinesProvider.RunFn = func(f fn.Function) (string, fn.Function, error) { - // Inside the pipeline Run, func.yaml on disk should already - // have the image pull secret written. + // The function handed to the pipeline carries the effective config... + if f.Deploy.ImagePullSecret != "my-remote-secret" { + t.Fatalf("expected effective config to have imagePullSecret 'my-remote-secret', got '%v'", f.Deploy.ImagePullSecret) + } + // ...while the working tree has not been mutated by a mere attempt to deploy diskFn, err := fn.NewFunction(root) if err != nil { t.Fatalf("failed to load func.yaml during pipeline Run: %v", err) } - if diskFn.Deploy.ImagePullSecret != "my-remote-secret" { - t.Fatalf("expected func.yaml on disk to have imagePullSecret 'my-remote-secret', got '%v'", diskFn.Deploy.ImagePullSecret) + if diskFn.Deploy.ImagePullSecret != "" { + t.Fatalf("func.yaml was written to disk before deployment success; got imagePullSecret '%v'", diskFn.Deploy.ImagePullSecret) } f.Deploy.Namespace = "default" if f.Deploy.Image, err = f.ImageName(); err != nil { @@ -1954,6 +1957,15 @@ func TestDeploy_ImagePullSecretRemote(t *testing.T) { if !pipelinesProvider.RunInvoked { t.Fatal("expected pipeline Run to be invoked") } + + // After the successful deploy, the configuration is persisted as usual. + diskFn, err := fn.NewFunction(root) + if err != nil { + t.Fatal(err) + } + if diskFn.Deploy.ImagePullSecret != "my-remote-secret" { + t.Fatalf("expected func.yaml to have imagePullSecret persisted after success, got '%v'", diskFn.Deploy.ImagePullSecret) + } } // Test_ValidateBuilder tests that the builder validation accepts the diff --git a/pkg/functions/function.go b/pkg/functions/function.go index 86c96321ba..315c45751e 100644 --- a/pkg/functions/function.go +++ b/pkg/functions/function.go @@ -451,6 +451,26 @@ func nameFromPath(path string) string { */ } +// MarshalFuncYaml serializes the function in the exact form Write persists to +// func.yaml: the schema header followed by the YAML document. Used by Write +// and by direct-upload remote build which use this in-memory serialization +// of func.yaml instead of the on-disk one (which can be outdated via flag +// overrides, namespace/cluster resolution...). +// +// Unlike Write, this does not call Validate: the upload path must serialize +// unconditionally. Caller is responsible for validation => f.Validate() +func (f Function) MarshalFuncYaml() ([]byte, error) { + bb, err := yaml.Marshal(&f) + if err != nil { + return nil, err + } + schemaURI := funcYamlSchemaURI() + header := fmt.Sprintf(`# $schema: %s +# yaml-language-server: $schema=%s +`, schemaURI, schemaURI) + return append([]byte(header), bb...), nil +} + // Write Function struct (metadata) to Disk at f.Root func (f Function) Write() (err error) { // Skip writing (and dirtying the work tree) if there were no modifications. @@ -466,30 +486,12 @@ func (f Function) Write() (err error) { // Write var bb []byte - if bb, err = yaml.Marshal(&f); err != nil { + if bb, err = f.MarshalFuncYaml(); err != nil { return } // TODO: open existing file for writing, such that existing permissions // are preserved? - rwFile, err := os.OpenFile(filepath.Join(f.Root, FunctionFile), os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0644) - if err != nil { - return err - } - defer rwFile.Close() - - schemaURI := funcYamlSchemaURI() - - // Write schema header - schemaHeader := fmt.Sprintf(`# $schema: %s -# yaml-language-server: $schema=%s -`, schemaURI, schemaURI) - - if _, err = rwFile.WriteString(schemaHeader); err != nil { - return err - } - - // Write function data - if _, err = rwFile.Write(bb); err != nil { + if err = os.WriteFile(filepath.Join(f.Root, FunctionFile), bb, 0644); err != nil { return err } diff --git a/pkg/pipelines/tekton/pipelines_provider.go b/pkg/pipelines/tekton/pipelines_provider.go index 6d13178c30..bc60656d9a 100644 --- a/pkg/pipelines/tekton/pipelines_provider.go +++ b/pkg/pipelines/tekton/pipelines_provider.go @@ -172,6 +172,12 @@ func (pp *PipelinesProvider) Run(ctx context.Context, f fn.Function) (string, fn if f.Build.Git.URL == "" { // Use direct upload to PVC if Git is not set up. + + // The uploaded func.yaml is synthesized from this in-memory f (see + // sourcesAsTarStream) so we validate it first + if err = f.Validate(); err != nil { + return "", f, fmt.Errorf("function is invalid: %w", err) + } content := sourcesAsTarStream(f) defer content.Close() err = k8s.UploadToVolume(ctx, content, getPipelinePvcName(f), namespace) @@ -315,6 +321,30 @@ func sourcesAsTarStream(f fn.Function) *io.PipeReader { _ = pw.CloseWithError(fmt.Errorf("error while creating tar stream from sources: %w", err)) } + // The uploaded func.yaml is synthesized from the in-memory 'f' + fy, err := f.MarshalFuncYaml() + if err != nil { + _ = pw.CloseWithError(fmt.Errorf("error serializing function config for upload: %w", err)) + return + } + err = tw.WriteHeader(&tar.Header{ + Typeflag: tar.TypeReg, + Name: "source/" + fn.FunctionFile, + Mode: 0644, + Size: int64(len(fy)), + Uid: nobodyID, + Gid: nobodyID, + Uname: "nobody", + Gname: "nobody", + }) + if err == nil { + _, err = tw.Write(fy) + } + if err != nil { + _ = pw.CloseWithError(fmt.Errorf("error writing function config to tar stream: %w", err)) + return + } + err = filepath.Walk(f.Root, func(p string, fi fs.FileInfo, err error) error { if err != nil { return fmt.Errorf("error traversing function directory: %w", err) @@ -329,6 +359,12 @@ func sourcesAsTarStream(f fn.Function) *io.PipeReader { return nil } + // func.yaml was already written into the stream above, from the + // in-memory function; skip the on-disk copy. + if relp == fn.FunctionFile { + return nil + } + if ignored(relp) { if fi.IsDir() { return filepath.SkipDir diff --git a/pkg/pipelines/tekton/pipelines_provider_test.go b/pkg/pipelines/tekton/pipelines_provider_test.go index 158bc55930..542e2964e5 100644 --- a/pkg/pipelines/tekton/pipelines_provider_test.go +++ b/pkg/pipelines/tekton/pipelines_provider_test.go @@ -201,3 +201,77 @@ func Test_createPipelinePersistentVolumeClaim(t *testing.T) { }) } } + +// TestSourcesAsTarStream_InjectsEffectiveConfig: the uploaded func.yaml is the +// serialized in-memory (effective) config, not the on-disk copy, and the +// on-disk file is left untouched. +func TestSourcesAsTarStream_InjectsEffectiveConfig(t *testing.T) { + root := t.TempDir() + diskYaml := "specVersion: 0.36.0\nname: my-fn\nruntime: go\ncreated: 2026-01-01T00:00:00Z\n" + if err := os.WriteFile(filepath.Join(root, "func.yaml"), []byte(diskYaml), 0644); err != nil { + t.Fatal(err) + } + + f, err := fn.NewFunction(root) + if err != nil { + t.Fatal(err) + } + // Effective, not-yet-disk-persisted configuration, as set by deploy flags. + f.Namespace = "effective-ns" + f.Deploy.ServiceAccountName = "effective-sa" + + rc := sourcesAsTarStream(f) + t.Cleanup(func() { _ = rc.Close() }) + + var uploaded []byte + entries := 0 + tr := tar.NewReader(rc) + for { + hdr, err := tr.Next() + if err != nil { + if errors.Is(err, io.EOF) { + break + } + t.Fatal(err) + } + if hdr.Name == "source/func.yaml" { + entries++ + if uploaded, err = io.ReadAll(tr); err != nil { + t.Fatal(err) + } + } + } + if entries != 1 { + t.Fatalf("expected exactly one func.yaml entry in the stream, got %d", entries) + } + + want, err := f.MarshalFuncYaml() + if err != nil { + t.Fatal(err) + } + if string(uploaded) != string(want) { + t.Errorf("uploaded func.yaml != f.MarshalFuncYaml()\n--- uploaded ---\n%s\n--- want ---\n%s", uploaded, want) + } + + extract := t.TempDir() + if err := os.WriteFile(filepath.Join(extract, "func.yaml"), uploaded, 0644); err != nil { + t.Fatal(err) + } + got, err := fn.NewFunction(extract) + if err != nil { + t.Fatalf("uploaded func.yaml does not parse: %v", err) + } + if got.Namespace != "effective-ns" || got.Deploy.ServiceAccountName != "effective-sa" { + t.Errorf("parsed uploaded func.yaml lost effective values: ns=%q sa=%q", + got.Namespace, got.Deploy.ServiceAccountName) + } + + // The on-disk func.yaml must remain untouched. + disk, err := os.ReadFile(filepath.Join(root, "func.yaml")) + if err != nil { + t.Fatal(err) + } + if string(disk) != diskYaml { + t.Errorf("on-disk func.yaml was mutated by the upload:\n%s", disk) + } +}