pipelines: ship effective config inside the source upload#3919
Open
gauron99 wants to merge 1 commit into
Open
Conversation
…eploy func.yaml write Remote (direct upload) builds previously wrote func.yaml to disk before the pipeline uploaded sources, so the on-cluster deploy step would see deploy-time flags (--image-pull-secret, --service-account, --deployer, the target namespace, ...). That was a workaround with two real costs: a mere deployment attempt mutated the user's working tree before success, and for git-flavor builds it never worked at all (the pipeline checks out the remote repository; a local write cannot reach it). Instead, synthesize the uploaded func.yaml from the in-memory, fully configured function: sourcesAsTarStream now serializes the effective configuration into the stream (via the new fn.MarshalFuncYaml, shared with Function.Write) and skips the on-disk copy. The on-cluster deploy step reads the same file it always did -- it just now reflects the actual request. The working tree is persisted only after the deployment succeeds, exactly like local deploys. Git-flavor builds are intentionally unchanged: they deploy the committed configuration, which is the GitOps contract.
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: gauron99 The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
knative-prow
Bot
added
approved
gauron99
requested review from
lkingland and
matejvasek
and removed request for
dsimansk and
jrangelramos
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes remote (direct-upload) deploy behavior so the func.yaml included in the uploaded sources reflects the effective in-memory function configuration (including deploy-time flag overrides), while avoiding any pre-deploy mutation of the working tree.
Changes:
- Validate the in-memory
fn.Functionbefore direct-upload and serialize its effective config directly into the uploaded tar stream assource/func.yaml. - Add
Function.MarshalFuncYaml()and reuse it fromFunction.Write()so both disk persistence and upload serialization share the same formatting (schema header + YAML). - Remove the pre-pipeline
func.yamlwrite from remote deploys and adjust/add tests to ensure the working tree is only written after successful deploy.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
pkg/pipelines/tekton/pipelines_provider.go |
Validates f before direct-upload and injects a synthesized func.yaml into the upload tar stream while skipping the on-disk func.yaml. |
pkg/pipelines/tekton/pipelines_provider_test.go |
Adds coverage ensuring exactly one func.yaml is uploaded, matches MarshalFuncYaml(), parses correctly, and does not mutate the on-disk file. |
pkg/functions/function.go |
Introduces MarshalFuncYaml() and refactors Write() to use it, centralizing func.yaml serialization. |
cmd/deploy.go |
Removes the pre-remote-deploy f.Write() so remote deploy attempts don’t dirty the working tree. |
cmd/deploy_test.go |
Updates remote deploy test expectations: effective config is passed to pipeline, disk is unchanged until success, then persisted after success. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+456
to
+457
| // 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 |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3919 +/- ##
=======================================
Coverage 54.00% 54.01%
=======================================
Files 200 200
Lines 23687 23706 +19
=======================================
+ Hits 12793 12804 +11
- Misses 9659 9666 +7
- Partials 1235 1236 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Remote (direct upload) builds wrote
func.yamlto disk before uploading sources, sothe on-cluster deploy step would pick up deploy-time flags.
Instead, synthesize the uploaded
func.yamlfrom the in-memory function:sourcesAsTarStreamserializes the effective config into the stream (via the newfn.MarshalFuncYaml, shared withFunction.Write) and skips the on-disk copyChanges
func.yamlis written only after success, like local deploys.func.yamlwrite with in-memory synthesis into the source upload.fn.MarshalFuncYaml, shared byFunction.Writeand the upload path./kind bug