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
| File | Description |
|---|---|
cmd/{apply,diff,pipeline_apply,pipeline_diff}.go |
Adds the relative-path option to pipeline commands. |
cmd/{compose_apply,compose_diff}.go |
Propagates the option through compose execution. |
cmd/{env,flags,root}.go |
Defines the environment variable, flag, and shared state. |
pkg/core/config/{main,defaults,options,schemavalidate_registry}.go |
Loads, validates, merges, and applies manifest path options. |
pkg/core/config/{main,relativepath}_test.go |
Tests configuration precedence and base selection. |
pkg/core/engine/{autodiscovery,configuration,manifest_validate,options}.go |
Carries manifest options through loading and autodiscovery. |
pkg/core/pipeline/{main,scmdirectory}.go |
Computes and distributes each manifest’s base directory. |
pkg/core/pipeline/{condition,source,target}/main.go |
Constructs resolvers for pipeline resources. |
pkg/core/pipeline/resource/main.go |
Extends the resource interface with a resolver. |
pkg/core/pipeline/main_test.go |
Updates pipeline configuration tests. |
pkg/plugins/utils/{filepath,resolver}.go |
Replaces legacy path helpers with the shared resolver. |
pkg/plugins/utils/resolver_test.go |
Covers resolution and containment behavior. |
pkg/plugins/utils/dasel/{main,read}.go |
Adds idempotent path resolution to structured files. |
pkg/plugins/utils/dasel/read_containment_test.go |
Tests local paths, containment, and idempotency. |
pkg/plugins/utils/toolversions/{main,read}.go |
Adds idempotent .tool-versions path resolution. |
pkg/plugins/utils/toolversions/read_containment_test.go |
Updates containment coverage. |
pkg/plugins/autodiscovery/cargo/utils.go |
Uses the resolver-aware TOML reader. |
pkg/plugins/resources/containment/containment_test.go |
Adds cross-resource containment regression tests. |
pkg/plugins/resources/{file,yaml}/*.go |
Centralizes file and glob path resolution. |
pkg/plugins/resources/{xml,json,toml,csv,toolversions}/*.go |
Migrates structured-file resources to resolvers. |
pkg/plugins/resources/{hcl,dockerfile,systemd,bazelmod}/*.go |
Resolves and contains file-based resource paths. |
pkg/plugins/resources/terraform/{lock,provider}/*.go |
Resolves Terraform files through the shared resolver. |
pkg/plugins/resources/go/gomod/*.go |
Resolves go.mod paths consistently. |
pkg/plugins/resources/helm/*.go |
Resolves chart, metadata, and local repository locations. |
pkg/plugins/resources/shell/*.go |
Resolves shell working directories from the selected base. |
pkg/plugins/resources/{gitbranch,gitcommit,gittag}/*.go |
Resolves local repository paths. |
pkg/plugins/resources/cargopackage/*.go |
Resolves filesystem registry roots. |
pkg/plugins/resources/{awsami,bazelregistry,dockerdigest,dockerimage,githubrelease,jenkins,maven,npm,pypi,temurin,updateclihttp}/*.go |
Updates resource interfaces for resolver propagation. |
pkg/plugins/resources/go/{language,module}/*.go |
Updates Go resource interfaces and tests. |
pkg/plugins/resources/gitea/{branch,release,tag}/*.go |
Updates Gitea resource interfaces and tests. |
pkg/plugins/resources/gitlab/{branch,release,tag}/*.go |
Updates GitLab resource interfaces and tests. |
pkg/plugins/resources/stash/{branch,release,tag}/*.go |
Updates Stash resource interfaces. |
e2e/updatecli.d/success.d/relativepaths/{manifest,workingdirectory}.yaml |
Exercises both selectable path bases. |
e2e/updatecli.d/success.d/relativepaths/data.txt |
Supplies manifest-relative fixture data. |
💡 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>
|
Important Review skippedToo many files! This PR contains 297 files, which is 147 over the limit of 150. To get a review, reduce the PR to 150 files or fewer by splitting it into smaller PRs or changing its base branch. Upgrade to a paid plan to raise the limit. This review couldn't start because sufficient usage credits or metered capacity aren't available. Add credits or update usage-based reviews in the billing tab, then retry. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (297)
You can disable this status message by setting the Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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>
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