Repository navigation
Add repository option to crossplane project init - #105
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe ChangesProject initialization repository configuration
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 6✅ Passed checks (6 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cmd/crossplane/project/init.go`:
- Around line 77-78: Normalize and validate the repository base (c.Repository)
before composing spec.repository: ensure c.Repository is non-empty and trim any
trailing slashes or whitespace, then join it with c.Name using a safe join
(e.g., path.Join or by appending "/" then name) instead of raw "%s/%s"
concatenation so you never produce double slashes; update the code that formats
spec.repository (the spot using c.Repository and c.Name) to use the normalized
value and return or surface a validation error if repository is empty/invalid.
- Line 42: The Directory field in the project init command is currently declared
as a positional arg (arg:"") but also has a short flag, so --directory/-d won't
parse; change Directory in cmd/crossplane/project/init.go from a positional to a
real flag by removing arg:"" and replacing it with a name tag and the existing
short/help/type tags (e.g. name:"directory" short:"d" help:"Directory to
initialize..." type:"path"), and if you instead prefer a second positional arg
update the command help/examples in cmd/crossplane/project/help/init.md to show
the positional usage; locate the Directory field in the relevant struct to apply
the change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: cb516f46-3c47-48a4-a6a1-4696dba3d528
📒 Files selected for processing (1)
cmd/crossplane/project/init.go
Make the directory name truly optional. Signed-off-by: Bob Haddleton <bob.haddleton@nokia.com>
Signed-off-by: Bob Haddleton <bob.haddleton@nokia.com>
There was a problem hiding this comment.
♻️ Duplicate comments (1)
cmd/crossplane/project/init.go (1)
65-68:⚠️ Potential issue | 🟠 Major | ⚡ Quick winWhitespace-only repository values can pass validation and generate invalid
spec.repository.Could you confirm whether
--repository " "should be rejected? On Line 65, only trailing/is trimmed; on Line 66 the emptiness check can be bypassed by whitespace, which then propagates to Line 81 and fails later in downstream repository parsing.Suggested minimal fix
- repo := strings.TrimRight(c.Repository, "/") + repo := strings.TrimRight(strings.TrimSpace(c.Repository), "/") if repo == "" { return errors.New("repository cannot be empty; set --repository to an OCI repository prefix like 'xpkg.crossplane.io/my-org'") }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cmd/crossplane/project/init.go` around lines 65 - 68, The current validation trims only trailing slashes and allows all-whitespace repository values to pass; update the handling of c.Repository by applying strings.TrimSpace (and still TrimRight for slashes if needed) to produce repo, then validate repo == "" after trimming so that inputs like "--repository \" \"" are rejected; ensure the rest of the function (where repo is used to set spec.repository) uses this trimmed repo variable (referencing c.Repository, repo) so downstream parsing no longer sees whitespace-only values.
🧹 Nitpick comments (1)
cmd/crossplane/project/init.go (1)
50-83: ⚡ Quick winPlease add focused tests for repository normalization and optional directory behavior.
Thanks for the feature update — would you add table-driven coverage for: default repository, trailing slash trimming, whitespace-only rejection, and omitted directory defaulting to project name? This should lock in the intended CLI UX and prevent regressions.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cmd/crossplane/project/init.go` around lines 50 - 83, Add table-driven unit tests for initCmd.Run to validate repository normalization and directory defaulting: create test cases for (1) default repository value (empty repo should error with the same message as currently returned for repo == ""), (2) repository with trailing slash gets normalized (strings.TrimRight) and appears in generated content, (3) whitespace-only repository is rejected, and (4) omitted Directory results in c.Directory being set to c.Name and used to create projectFileName content; instantiate initCmd with appropriate fields, call Run with a no-op SpinnerPrinter or wrap the inner logic (refer to initCmd.Run, c.Repository, c.Directory, checkTargetDirectory, and projectFileName) and assert expected errors or generated file/content behavior for each table row.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@cmd/crossplane/project/init.go`:
- Around line 65-68: The current validation trims only trailing slashes and
allows all-whitespace repository values to pass; update the handling of
c.Repository by applying strings.TrimSpace (and still TrimRight for slashes if
needed) to produce repo, then validate repo == "" after trimming so that inputs
like "--repository \" \"" are rejected; ensure the rest of the function (where
repo is used to set spec.repository) uses this trimmed repo variable
(referencing c.Repository, repo) so downstream parsing no longer sees
whitespace-only values.
---
Nitpick comments:
In `@cmd/crossplane/project/init.go`:
- Around line 50-83: Add table-driven unit tests for initCmd.Run to validate
repository normalization and directory defaulting: create test cases for (1)
default repository value (empty repo should error with the same message as
currently returned for repo == ""), (2) repository with trailing slash gets
normalized (strings.TrimRight) and appears in generated content, (3)
whitespace-only repository is rejected, and (4) omitted Directory results in
c.Directory being set to c.Name and used to create projectFileName content;
instantiate initCmd with appropriate fields, call Run with a no-op
SpinnerPrinter or wrap the inner logic (refer to initCmd.Run, c.Repository,
c.Directory, checkTargetDirectory, and projectFileName) and assert expected
errors or generated file/content behavior for each table row.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 93e17c21-4531-4195-950c-bdee283d8b13
📒 Files selected for processing (1)
cmd/crossplane/project/init.go
Signed-off-by: Bob Haddleton <bob.haddleton@nokia.com>
|
Docs PR crossplane/docs#1108 |
| repository: %s/%s | ||
| `, c.Name, repo, c.Name) |
There was a problem hiding this comment.
I'd prefer to just use repo when it's specified, rather than appending the project name. OCI defines two things: registries and repositories; the in-between of a partially-specified repository isn't really a concept.
There was a problem hiding this comment.
Don't we really have three parts - the registry, the org and the repository? Maybe the option should be registryPath or similar to include both the registry and the org? I'd like to be able to default the repository name to the project name rather than have to specify it again in the registry path.
There was a problem hiding this comment.
The first component of the repository being an organization name is just a convention, not specified by OCI. That said, it is a very common convention - every public registry I can think of follows it (GHCR, Docker Hub, Upbound marketplace, etc.).
Agreed it's a nice UX to not require repeating the project name. I think making the argument --registry (not technically correct, but less surprising to append to it) or --repository-prefix would make sense. That would allow for a --repository flag to provide the full path as well, if users want the repository name to not match the project name.
|
|
||
| repo := strings.TrimRight(strings.TrimSpace(c.Repository), "/") | ||
| if repo == "" { | ||
| return errors.New("repository cannot be empty; set --repository to an OCI repository prefix like 'xpkg.crossplane.io/my-org'") |
There was a problem hiding this comment.
Might be worth validating that repo is a valid, fully-qualified repository by calling name.NewRepository.
Signed-off-by: Bob Haddleton <bob.haddleton@nokia.com>
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cmd/crossplane/project/init.go`:
- Around line 74-77: The error message in the repository creation block uses
internal wording "cannot build function reference" which is confusing in the
project init command context. Replace this error message with user-facing
wording that clearly indicates the problem is with invalid registry or
repository parameters provided via the --registry and --repository command
flags. Keep the same error wrapping structure but update the message text to be
more actionable and appropriate for the project init command path.
- Around line 61-63: In the Repository field initialization block, the TrimSpace
function is only used for the emptiness check, but the untrimmed value is still
assigned to c.Repository when empty. Persist the trimmed value by assigning the
result of strings.TrimSpace(c.Repository) back to c.Repository before the
fallback check, so that any subsequent validation or usage of the Repository
field at line 74 or elsewhere operates on the normalized value without
accidental leading or trailing whitespace.
- Around line 70-72: The error message returned when the registry path is empty
references the incorrect flag name. In the if block that checks if rp == "",
update the error message to reference the actual flag name `--registry` instead
of `--registryPath` to match the flag definition on line 44 and provide users
with accurate guidance for fixing the issue.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: cf7f8fff-566c-4f03-8e2e-bd789ffec85c
📒 Files selected for processing (1)
cmd/crossplane/project/init.go
| if strings.TrimSpace(c.Repository) == "" { | ||
| c.Repository = c.Name | ||
| } |
There was a problem hiding this comment.
Normalize Repository after trimming, before fallback and validation.
Right now TrimSpace is only used for the emptiness check, but Line 74 still consumes the untrimmed value. Could we persist the trimmed value so accidental leading/trailing spaces don’t cause avoidable parse failures?
Suggested patch
- if strings.TrimSpace(c.Repository) == "" {
+ c.Repository = strings.TrimSpace(c.Repository)
+ if c.Repository == "" {
c.Repository = c.Name
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@cmd/crossplane/project/init.go` around lines 61 - 63, In the Repository field
initialization block, the TrimSpace function is only used for the emptiness
check, but the untrimmed value is still assigned to c.Repository when empty.
Persist the trimmed value by assigning the result of
strings.TrimSpace(c.Repository) back to c.Repository before the fallback check,
so that any subsequent validation or usage of the Repository field at line 74 or
elsewhere operates on the normalized value without accidental leading or
trailing whitespace.
Signed-off-by: Bob Haddleton <bob.haddleton@nokia.com>
|
I'm going to remove the backport label, since this isn't strictly a bug fix and we're going to release CLI v2.4 right away anyway. |
crossplane 2.4.0 Created-by: HarmonybrewBot Commit-by: HarmonybrewBot Merged-by: HarmonybrewBot Description: Created by `brew bump` --- Created with `brew bump-formula-pr`.<details> <summary>release notes</summary> <pre>The `v2.4.0` release is the first Crossplane CLI release that does not correspond to a Crossplane core release. It is compatible with the currently supported minor versions of Crossplane core: v2.3, v2.2, v2.1, and v1.20. It includes a number of incremental features and improvements along with many bug fixes. ## 🚨 Installation Note Since this release does not correspond to a Crossplane core release, it will not be uploaded to [releases.crossplane.io](https://releases.crossplane.io). Its artifacts are available only from [cli.crossplane.io](https://cli.crossplane.io). The `install.sh` script will install it from the correct location. ## 🎉 Highlights * **New Commands** * `crossplane xr generate` converts a claim to a composite resource, as Crossplane would do internally when a claim is applied to a cluster. #13 * `crossplane xr patch` applies defaults from an XRD to an input XR, as the apiserver would do when an XR is applied to a cluster. #61 * `crossplane xpkg get-crds` downloads package dependencies based on a `crossplane.yaml` file or package resource and writes their CRDs to files. #26 * **Command Updates** * `crossplane project init` now allows a repository to be specified. #105 * `crossplane resource validate` now has YAML and JSON output formats. #66 * `crossplane xpkg push` now allows OCI manifest annotations to be specified with the `--oci-annotation` flag. #11 * `crossplane composition render` now supports repeating the `--required-resources` and `--extra-resources` flags. #107 * Commands that interact with a cluster (e.g., `crossplane resource trace` and `crossplane xpkg install`) now support impersonation with the `--as`, `--as-group`, and `--as-uid` flags. #114 * **Developer Experience Updates** * Projects can now include pre-built functions specified in the `crossplane-project.yaml` file, allowing functions to be built using any language, tool, or framework. #24 * Projects can now specify a subset of languages for which schemas should be generated in the `crossplane-project.yaml` file. #24 ## What's Changed * Update dependencies and renovate for v2.3.0 release (main) by @adamwg in crossplane/cli#19 * ci: Remove image promotion steps by @adamwg in crossplane/cli#22 * build: Use the shared crossplane cachix cache in nix.sh by @adamwg in crossplane/cli#17 * Add the LICENSE file by @adamwg in crossplane/cli#27 * chore(deps): update module github.com/containerd/containerd to v1.7.32 [security] (main) by @crossplane-renovate[bot] in crossplane/cli#34 * chore(deps): update module github.com/sigstore/cosign/v2 to v2.6.2 [security] (main) by @crossplane-renovate[bot] in crossplane/cli#35 * chore(deps): update module golang.org/x/crypto to v0.52.0 [security] (main) by @crossplane-renovate[bot] in crossplane/cli#36 * Improve generation of reference docs by @adamwg in crossplane/cli#30 * chore(deps): update module golang.org/x/net to v0.55.0 [security] (main) by @crossplane-renovate[bot] in crossplane/cli#37 * chore(deps): update actions/create-github-app-token digest to fee1f7d (main) by @crossplane-renovate[bot] in crossplane/cli#52 * Update CONTRIBUTING.md and add install instructions to README.md by @adamwg in crossplane/cli#49 * Add the install.sh script and upload it to S3 by @adamwg in crossplane/cli#54 * docs: Enclose the CLI version number in backticks by @adamwg in crossplane/cli#55 * fix(docs): compile the bqRE once at package level by @tampakrap in crossplane/cli#57 * chore(deps): update actions/stale digest to eb5cf3a (main) by @crossplane-renovate[bot] in crossplane/cli#58 * feat(xr): Convert a Claim to XR via `crossplane xr generate` by @tampakrap in crossplane/cli#13 * chore(deps): pin dependencies (main) by @crossplane-renovate[bot] in crossplane/cli#51 * chore(deps): update cachix/install-nix-action digest to 8aa0397 (main) by @crossplane-renovate[bot] in crossplane/cli#60 * fix(deps): update module github.com/go-git/go-git/v5 to v5.19.1 [security] (main) by @crossplane-renovate[bot] in crossplane/cli#40 * render: Sync render.proto with c/c and wire XRD through by @jcogilvie in crossplane/cli#74 * Fix Python schema generation for fields named int or bool by @negz in crossplane/cli#64 * chore(deps): update actions/checkout digest to df4cb1c (main) by @crossplane-renovate[bot] in crossplane/cli#76 * chore(deps): update codecov/codecov-action digest to 75cd116 (main) by @crossplane-renovate[bot] in crossplane/cli#77 * chore(deps): update github/codeql-action digest to 8aad20d (main) by @crossplane-renovate[bot] in crossplane/cli#80 * chore(deps): update mheap/require-checklist-action digest to 9c8100a (main) by @crossplane-renovate[bot] in crossplane/cli#81 * Support pre-built function runtimes and per-language schema generation by @negz in crossplane/cli#24 * render: Clean up unused code and duplicate consts by @adamwg in crossplane/cli#83 * render: Replace condition timestamps and sort resources to stabilize output by @adamwg in crossplane/cli#82 * feat(render): capture stderr and handle pipeline-fatal exit code in both engines by @jcogilvie in crossplane/cli#91 * chore(deps): update actions/checkout action to v6.0.3 (main) by @crossplane-renovate[bot] in crossplane/cli#84 * chore(deps): update vale-cli/vale-action action to v2.1.2 (main) by @crossplane-renovate[bot] in crossplane/cli#85 * feat(xr): Introduce `xr patch --xrd` by @tampakrap in crossplane/cli#61 * Update the CLI docs path by @adamwg in crossplane/cli#99 * fix(xr): add missing import, caused by merge of outdated base tree by @tampakrap in crossplane/cli#103 * ci: Update the CLI reference documentation on every merge by @adamwg in crossplane/cli#106 * feat(validate): structured results and --output json|yaml flag by @jcogilvie in crossplane/cli#66 * function: Add missing dependencies to the Go function template by @adamwg in crossplane/cli#109 * xrd: Opportunistically infer integer types when generating from an XR by @adamwg in crossplane/cli#108 * feat: add xpkg get-crds subcommand by @fernandezcuesta in crossplane/cli#26 * Drop the scale subresource before building OpenAPI for schema generation by @negz in crossplane/cli#119 * ci: Re-use exising docs PRs and sign off on commits by @adamwg in crossplane/cli#120 * ci: Make cli-docs-bot the author of docs update commits by @adamwg in crossplane/cli#121 * xrd: Handle v2 XRDs in `crossplane xrd convert` by @adamwg in crossplane/cli#122 * fix(render): do not overwrite function docker network if set, start crossplane-container in same network by @nkzk in crossplane/cli#65 * chore(deps): update codecov/codecov-action digest to 0fb7174 (main) by @crossplane-renovate[bot] in crossplane/cli#101 * chore(deps): update korthout/backport-action action to v4.5.2 (main) by @crossplane-renovate[bot] in crossplane/cli#102 * Expose host-native CLI binary as the default flake package by @negz in crossplane/cli#126 * chore(deps): pin dependencies (main) by @crossplane-renovate[bot] in crossplane/cli#128 * chore(deps): update renovatebot/github-action action to v46.1.15 (main) by @crossplane-renovate[bot] in crossplane/cli#129 * Decompress function runtime tarballs once when loading by @negz in crossplane/cli#127 * fix: loaded XRD must honor the composite schema by @fernandezcuesta in crossplane/cli#123 * schemas: Prime oapi-codegen's global state to make capitalization consistent by @adamwg in crossplane/cli#130 * Add repository option to `crossplane project init` by @bobh66 in crossplane/cli#105 * feat(xpkg): add --annotation flag to xpkg build and xpkg push by @chaitanyapantheor in crossplane/cli#11 * fix(render): support repeating --required-resources and --extra-resources by @chaitanyapantheor in crossplane/cli#107 * fix(deps): update module github.com/alecthomas/kong to v1.15.0 (main) by @crossplane-renovate[bot] in crossplane/cli#133 * chore(deps): update module github.com/containerd/containerd to v1.7.33 [security] (main) by @crossplane-renovate[bot] in crossplane/cli#132 * fix(deps): update module github.com/oapi-codegen/oapi-codegen/v2 to v2.7.1 (main) by @crossplane-renovate[bot] in crossplane/cli#88 * fix(deps): update module github.com/kubernetes-sigs/kro to v0.9.2 (main) by @crossplane-renovate[bot] in crossplane/cli#87 * fix(deps): update module github.com/google/go-containerregistry to v0.21.7 (main) by @crossplane-renovate[bot] in https://github.com/crossplane/ See merge request: Harmonybrew/homebrew-core!12309
Description of your changes
Added the
--repositoryoption tocrossplane project initto override the defaultexample.com/my-orgvalues.Made the directory name truly optional.
I have:
./nix.sh flake checkto ensure this PR is ready for review.- [ ] Added or updated unit tests.backport release-x.ylabels to auto-backport this PR.Need help with this checklist? See the cheat sheet.