{{ message }}
Repository navigation
feat: support relative path, relative to Updatecli manifest files. - #10150
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Adds opt-in manifest-relative path resolution while preserving working-directory defaults and SCM containment.
Changes:
- Introduces manifest/CLI path-base configuration and shared resolver plumbing.
- Migrates resource implementations to centralized path resolution.
- Adds containment, resolver, configuration, and end-to-end coverage.
Reviewed changes
Copilot reviewed 276 out of 276 changed files in this pull request and generated 2 comments.
Show a summary per file
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Signed-off-by: Olblak <me@olblak.com>
Signed-off-by: Olblak <me@olblak.com>
Signed-off-by: Olblak <me@olblak.com>
Signed-off-by: Olblak <me@olblak.com>
Signed-off-by: Olblak <me@olblak.com>
Signed-off-by: Olblak <me@olblak.com>
Fix the regressions found while reviewing the relative paths work: - keep the auto guessed "local" scm on its own clone instead of the user's working copy - read helm Chart.yaml from the resolved chart directory without an scm - resolve each scm "directory" once, in Init, Update and autodiscovery, including the typed specs of the githubsearch, gitlabsearch and azuredevopssearch scms - keep accepting absolute and parent paths for the resources that never enforced the scm boundary, and read absolute dockerfile source and helm file:// paths under the checkout - resolve the git resources "path" from the manifest directory, and open the process working directory when they name no scm, url or path - create git branches in the resolved repository directory - add the base directory to the source cache key - register --relative-paths on prepare, show and manifest validate - document what "relativepaths: manifest" changes Signed-off-by: Olblak <me@olblak.com>
olblak
changed the title
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.
Fix #8300
Relative paths in a manifest resolve against the directory updatecli was started from, never against the manifest that declared them. A manifest in updatecli.d/nodejs.yaml saying file: package.json means "package.json next to wherever the user happened to cd", which makes manifests non-relocatable: they cannot be moved, vendored into another repository, or pulled as an OCI policy without rewriting every path.
This pull request adds an opt-in way to say "resolve my paths against my own directory", the rule updatecli compose already applies to its own file (relativePathToFile in pkg/core/compose/file.go):
It can also be set for a whole run with --relative-paths manifest or UPDATECLI_RELATIVE_PATHS=manifest; a manifest declaring the key wins over the command line. The default is unchanged, and is meant to stay that way: the dominant layout is updatecli.d/*.yaml referencing paths from the repository root, so flipping it would break nearly every existing manifest.
Only resources without an scmid are affected. A resource attached to an scm still resolves against the scm working directory, exactly as before.
Why this needed a refactor first
There was no single place that resolved a path. Each of the 43 resource kinds computed its own base directory and joined it itself, in five different ways:
On top of that, source.Run passed os.Getwd() when there was no scm, so six source implementations undid it with a copy-pasted block whose own comment asked for this fix:
The root cause: SanitizeFilePathWithWorkingDirectory conflated two different ideas into one workingDir argument — where relative paths resolve from and the boundary they must stay inside. Separating them is what makes manifest-relative resolution possible without weakening the containment fix:
// pkg/plugins/utils/resolver.go
type Resolver struct {
BaseDir string // where relative paths resolve from ("" = process working directory)
Boundary string // where they must stay inside ("" = no boundary)
}
Boundary is only ever set from an scm, so absolute paths keep being rejected inside a checkout and keep being accepted in local runs. Resolve is for files Updatecli reads or writes; Join is for locations it only needs in order to find something (a git repository, a chart directory, shell.workdir), which legitimately sit outside a checkout.
The Resolver is built once in pkg/core/pipeline and handed to every resource, which deleted the os.Getwd() branch and the sentinel hack along with it. SanitizeFilePathWithWorkingDirectory and JoinFilePathWithWorkingDirectoryPath are gone.
Bug fixed along the way
kind: json, toml, csv and toolversions currently fail on an absolute spec.file in a local run, reproducible on the released binary:
Test
To test this pull request, you can run the following commands:
Additional Information
Checklist
Evidence that the default path did not move:
Tradeoff
Two behavior changes, both deliberate.
hcl, terraform/lock, terraform/provider, gomod, dockerfile, bazelmod and systemd used the uncontained join and could read or write outside an scm checkout through an absolute path. They are contained now, consistent with the other file based kinds. This is a tightening: a manifest using an absolute spec.file together with an scmid will now fail where it used to work. The GHSA fix clearly intended to cover these and simply missed them.
Conversely, json, toml, csv and toolversions become more permissive in local runs, which restores the behavior they had before the GHSA fix.
A large, mostly mechanical diff. The Resource interface gained a parameter, so all 43 kinds are touched (about two thirds are a signature change only). The alternative — resolving paths at manifest load time from struct tags — kept the diff small but fought with spec.searchpattern globs, helm's chart-relative file, and manifest show printing absolute paths. Paying once for explicit plumbing seemed better than a second implicit mechanism alongside the existing one.
options is opt-in forever, in practice. Because updatecli.d/*.yaml referencing repository-root paths is the dominant layout, the default cannot flip on a minor release. Revisiting it belongs to a major version.
Potential improvement