Rearrange the `crossplane` subcommands #4832

negz · 2023-10-17T22:56:53Z

Description of your changes

Here's what it looks like:

$ go run cmd/crank/main.go --help
Usage: crossplane <command>

A command line tool for interacting with Crossplane.

Commands:
  xpkg login        Login to the default package registry (xpkg.upbound.io).
  xpkg logout       Logout of the default package registry (xpkg.upbound.io).
  xpkg build        Build a package, by default from the current directory.
  xpkg push         Push a package, by default to xpkg.upbound.io.
  xpkg install      Install a package.
  xpkg update       Update an installed package.
  beta xpkg init    Initialize a package from a template.
  beta render       Render a claim or XR locally.

Flags:
  -h, --help       Show context-sensitive help.
  -v, --version    Print version and quit.
      --verbose    Print verbose logging statements.

Run "crossplane <command> --help" for more information on a command.

Notable changes in this PR:

Renamed marketplace login to xpkg login
Renamed xpkg build --controller flag to --embed-runtime-image.
Everything uses xpkg.upbound.io as the default registry
Store xpkg.upbound.io auth config in ~/.crossplane to avoid conflicts with ~/.up.
The kind of package to install (e.g. provider, function, etc) is now an argument to xpkg install, not a subcommand.
You can use xpkg init to initialize a provider now too (or really anything - for now it's a tiny wrapper on git clone).

I have:

Read and followed Crossplane's contribution process.
~~Added or updated unit tests.~~
~~Added or updated e2e tests, if necessary. (check with reviewers/maintainers if you're unsure whether E2E tests are necessary for the change).~~
~~Run make reviewable to ensure this PR is ready for review.~~
~~Added backport release-x.y labels to auto-backport this PR, if necessary.~~
~~Opened a PR updating the docs, if necessary.~~

negz · 2023-10-17T23:02:03Z

Edit: updated the PR to use compact.

I personally found the tree view pretty ugly/hard to read with the new layout so I reverted that. Not sure if that will be controversial.

Some other options available to us are to skip expanding subcommands by default, which looks like:

Usage: crossplane <command>

A command line tool for interacting with Crossplane.

Commands:
  xpkg <command>
    Crossplane package management.

  beta <command>
    Beta features. WARN: May be changed or deprecated in a future release

Flags:
  -h, --help       Show context-sensitive help.
  -v, --version    Print version and quit.
      --verbose    Print verbose logging statements.

Run "crossplane <command> --help" for more information on a command.

Or to get compact help, which looks like:

$ go run cmd/crank/main.go --help
Usage: crossplane <command>

A command line tool for interacting with Crossplane.

Commands:
  xpkg login                    Login to the default package registry (xpkg.upbound.io).
  xpkg logout                   Logout of the default package registry (xpkg.upbound.io).
  xpkg build                    Build a package, by default from the current directory.
  xpkg push                     Push a package, by default to xpkg.upbound.io.
  xpkg install configuration    Install a Configuration package.
  xpkg install provider         Install a Provider package.
  xpkg update configuration     Update a Configuration package.
  xpkg update provider          Update a Provider package.
  beta xpkg init                Initialize a package from a template.
  beta xpkg install function    Install a Function package.
  beta xpkg update function     Update a Function package.
  beta render                   Render a claim or XR locally.

Flags:
  -h, --help       Show context-sensitive help.
  -v, --version    Print version and quit.
      --verbose    Print verbose logging statements.

jbw976 · 2023-10-17T23:13:32Z

I like the compact view the best! I agree that the tree view is now too big and unwieldy and it will only get worse with more commands that we add. I do like having subcommands expanded so the user knows everything they can do up front without having to spelunk manually down through all the commands and get help for all of them.

Compact help seems to be a good balance of all that for now - we can always revisit if that gets too large in the future.

jbw976 · 2023-10-17T23:15:10Z

Make update and install take the kind of package as an argument, not a subcommand.

So this means that these commands would go away, right?

beta xpkg install function
beta xpkg update function

jbw976

Thanks for taking this clean-up effort on @negz!! took a quick pass through now with a few comments, but no major objections to anything here 🙇

cmd/crank/main.go

cmd/crank/beta/xpkg/xpkg.go

internal/xpkg/upbound/context.go

negz · 2023-10-18T04:42:25Z

cmd/crank/beta/xpkg/init.go

 	}
+	return b.String()


FYI this renders like this:

remote: $ go run cmd/crank/main.go beta xpkg init --help Usage: crossplane beta xpkg init <name> <template> Initialize a package from a template. Crossplane initializes a package by using git to clone a template from a repository. The template can be either a git repository URL, or a well-known template name. Crossplane supports the following well-known-template names: - provider-minimal (https://github.com/crossplane/provider-template) - provider-upjet (https://github.com/upbound/upjet-provider-template) - function-go (https://github.com/crossplane/function-template-go) Arguments: <name> Name of the package to initialize. <template> Template to initialize the package from. Flags: -h, --help Show context-sensitive help. -v, --version Print version and quit. --verbose Print verbose logging statements. -d, --directory="." Path of the directory to initialize.

phisco

A few feedbacks:

I preferred keeping the beta/alpha commands where they would be without the beta/alpha prefix, as that would mean promoting them would be a 1 line change instead of moving all the related files.
I like the direction of --embed-runtime-image, I think it conveys better how the image is being used by the build subcommands, however embed suggests it's a Boolean, I'd rather have it as --embedded-runtime-image probably.
I guess you switched the tree view off because it wasn't playing nicely with having a single top subcommands, is this the case? I prefer the tree view, but I also found the default visualization not to work so well in this case

plumbis

The commands were modified in this PR but we should update cmd/crank/beta/render/cmd.go

type Cmd struct {
	CompositeResource string `arg:"" type:"existingfile" help:"A composite resource (XR) YAML file to render."`
	Composition       string `arg:"" type:"existingfile" help:"A Composition YAML file to use."`
	Functions         string `arg:"" help:"A stream or directory of YAML files containing the Composition Functions to use."`

	ContextValues map[string]string `placeholder:"KEY=JSON-VALUE;..." help:"Semicolon-separated JSON encoded context variables to pass to the Function pipeline. Takes precedence over --context-files."`
	ContextFiles  map[string]string `placeholder:"KEY=FILENAME;..." help:"Semicolon-separated JSON encoded context variables to pass to the Function pipeline."`

	IncludeResults    bool     `short:"r" default:"true" help:"Include the Function's Results output."`
	ObservedResources []string `short:"o" help:"A stream or directory of YAML files mocking the observed state of composed resources."`

	Timeout time.Duration `help:"Amount of time to wait for the function to complete." default:"1m"`
}

cmd/crank/xpkg/xpkg.go

cmd/crank/beta/beta.go

cmd/crank/xpkg/build.go

cmd/crank/xpkg/install.go

plumbis · 2023-10-18T16:08:49Z

cmd/crank/xpkg/push.go

+	Tag     string `arg:"" help:"Tag of the package to be pushed. Must be a valid OCI image tag. Unqualified tags will be pushed to xpkg.upbound.io."`
 	Package string `short:"f" help:"Path to package. If not specified and only one package exists in current directory it will be used."`


Suggested change

Tag string `arg:"" help:"Tag of the package to be pushed. Must be a valid OCI image tag. Unqualified tags will be pushed to xpkg.upbound.io."`

Package string `short:"f" help:"Path to package. If not specified and only one package exists in current directory it will be used."`

Tag string `arg:"" help:"Package filename and version tag to use in the container registry."`

Package string `short:"f" help:"Path to the Crossplane xpkg file." default:"."`

What do you mean by "Unqualified tags"?

cmd/crank/xpkg/update.go

plumbis · 2023-10-18T16:10:01Z

cmd/crank/xpkg/update.go

+	Ref  string `arg:"" help:"The package's OCI image reference (e.g. tag)."`
+	Name string `arg:""  optional:"" help:"Name of the package to update. Will be derived from the ref if omitted."`


Suggested change

Ref string `arg:"" help:"The package's OCI image reference (e.g. tag)."`

Name string `arg:"" optional:"" help:"Name of the package to update. Will be derived from the ref if omitted."`

Ref string `arg:"" help:"Package filename and version tag in the container registry."`

Name string `arg:"" optional:"" help:"Name of the package inside Crossplane. Derived from the package metadata name by default."`

cmd/crank/beta/xpkg/init.go

Ditto logout. Signed-off-by: Nic Cope <nicc@rk0n.org>

This isn't where Crossplane pulls from by default, but we think we'd like it to be someday. It's the only Crossplane-aware OCI registry we're aware of, in that packages can show up at marketplace.upbound.io decorated with docs, examples, etc. Signed-off-by: Nic Cope <nicc@rk0n.org>

I think this might have been done to avoid shadowing, but I'm not getting any linter complaints after renaming back to config. Signed-off-by: Nic Cope <nicc@rk0n.org>