address review comments

pkg/cliplugins/bind/plugin/bind.go

@@ -105,9 +105,9 @@ func (b *BindOptions) Run(ctx context.Context) error {
  apiBindingName = apiExportName
  }
 
- _, currentClusterName, err := pluginhelpers.ParseClusterURL(config.Host)
+ currentClusterName, err := pluginhelpers.GetCurrentClusterName(config.Host)
  if err != nil {
- return fmt.Errorf("current URL %q does not point to cluster workspace", config.Host)
+ return err
  }
 
  binding := &apisv1alpha1.APIBinding{

pkg/cliplugins/claims/cmd/cmd.go

@@ -37,10 +37,10 @@ var (
  %[1]s claims get apibinding
 
  # Edit the permission claims' status related to a specific APIBinding with an interactive prompt.
- %[1]s claims get apibinding cert-manager
+ %[1]s claims edit apibinding cert-manager
 
  # Edit the permission claims' status for all APIBindings in current workspace with an interactive prompt.
- %[1]s claims get apibinding
+ %[1]s claims edit apibinding
  `
 )
 
@@ -105,7 +105,7 @@ func New(streams genericclioptions.IOStreams) *cobra.Command {
  apibindingEditOpts := plugin.NewEditAPIBindingOptions(streams)
  apibindingEditCmd := &cobra.Command{
  Use: "apibinding <apibinding_name>",
- Short: "Edit claims related to apibinding",
+ Short: "Edit open permission claims",
  SilenceUsage: true,
  RunE: func(cmd *cobra.Command, args []string) error {
  if err := apibindingEditOpts.Complete(args); err != nil {

pkg/cliplugins/claims/plugin/edit.go

@@ -80,18 +80,19 @@ func (e *EditAPIBindingOptions) Run(ctx context.Context) error {
  return err
  }
 
- _, currentClusterName, err := pluginhelpers.ParseClusterURL(cfg.Host)
+ currentClusterName, err := pluginhelpers.GetCurrentClusterName(cfg.Host)
  if err != nil {
- return fmt.Errorf("current URL %q does not point to cluster workspace", cfg.Host)
+ return err
  }
 
  kcpClusterClient, err := newKCPClusterClient(e.ClientConfig)
  if err != nil {
  return fmt.Errorf("error while creating kcp client %w", err)
  }
 
- // The status of an apibinding has a set of `exportClaims`. The subset which is present in exportClaims but not in spec.AcceptableClaims, are the open claims.
- // The command will list those open claims and update `spec.AcceptableClaims` with the status based on the input of the user.
+ // The status of an APIBinding has a set of PermissionClaims declared by the bound APIExport. The spec of an APIBinding has the subset of those PermissionClaims that the user has already taken action on.
+ // The remaining PermissionClaims are open to be accepted or rejected.
+ // The command will list those open claims and update `spec.PermissionClaims` with the status based on the input of the user.
 
  apibindings := []apiv1alpha1.APIBinding{}
  // List permission claims for all bindings in current workspace.
@@ -100,7 +101,7 @@ func (e *EditAPIBindingOptions) Run(ctx context.Context) error {
  if err != nil {
  return fmt.Errorf("error listing apibindings in %q workspace: %w", currentClusterName, err)
  }
- apibindings = append(apibindings, bindings.Items...)
+ apibindings = bindings.Items
  } else {
  binding, err := kcpClusterClient.Cluster(currentClusterName).ApisV1alpha1().APIBindings().Get(ctx, e.APIBindingName, metav1.GetOptions{})
  if err != nil {
@@ -135,7 +136,7 @@ func (e *EditAPIBindingOptions) Run(ctx context.Context) error {
  // List open claims and update `spec.AcceptableClaims` with the status based on the input of the user.
  for _, openClaim := range openClaims {
  // print the prompt and infer the user input.
- action, err := getRequiredInput(e.In, e.Out, apibinding.Name, openClaim.Group, openClaim.Resource)
+ action, err := getRequiredInput(e.In, e.Out, apibinding.Name, openClaim)
  if err != nil {
  allErrors = append(allErrors, err)
  }
@@ -151,7 +152,7 @@ func (e *EditAPIBindingOptions) Run(ctx context.Context) error {
 }
 
 // getOpenClaims gets the claims which are in a set of `exportClaims` but not in `spec.AcceptableClaims` of the apibinding.
-func getOpenClaims(exportClaims []apiv1alpha1.PermissionClaim, aceptableClaims []apiv1alpha1.AcceptablePermissionClaim) []apiv1alpha1.PermissionClaim {
+func getOpenClaims(exportClaims []apiv1alpha1.PermissionClaim, acceptableClaims []apiv1alpha1.AcceptablePermissionClaim) []apiv1alpha1.PermissionClaim {
  openPermissionClaims := []apiv1alpha1.PermissionClaim{}
 
  // Find the subset which is in exportClaims but not in AcceptableClaims.
@@ -161,7 +162,7 @@ func getOpenClaims(exportClaims []apiv1alpha1.PermissionClaim, aceptableClaims [
  }
 
  acceptableClaimsMap := map[apiv1alpha1.GroupResource]apiv1alpha1.PermissionClaim{}
- for _, a := range aceptableClaims {
+ for _, a := range acceptableClaims {
  acceptableClaimsMap[a.GroupResource] = a.PermissionClaim
  }
 

pkg/cliplugins/claims/plugin/get.go

@@ -85,9 +85,9 @@ func (g *GetAPIBindingOptions) Run(ctx context.Context) error {
  return err
  }
 
- _, currentClusterName, err := pluginhelpers.ParseClusterURL(cfg.Host)
+ currentClusterName, err := pluginhelpers.GetCurrentClusterName(cfg.Host)
  if err != nil {
- return fmt.Errorf("current URL %q does not point to cluster workspace", cfg.Host)
+ return err
  }
 
  kcpClusterClient, err := newKCPClusterClient(g.ClientConfig)

pkg/cliplugins/claims/plugin/util.go

@@ -22,6 +22,8 @@ import (
  "io"
  "log"
  "strings"
+
+ apiv1alpha1 "github.com/kcp-dev/kcp/pkg/apis/apis/v1alpha1"
 )
 
 // ClaimAction captures the user's preference of a specific action
@@ -47,11 +49,11 @@ func printMessage(bindingName, claimGroup, claimResource string) string {
  return fmt.Sprintf("Accept permission claim for Resource: %s (APIBinding: %s) > ", claimResource, bindingName)
 }
 
-func getRequiredInput(rd io.Reader, wr io.Writer, bindingName, claimGroup, claimResource string) (ClaimAction, error) {
+func getRequiredInput(rd io.Reader, wr io.Writer, bindingName string, claim apiv1alpha1.PermissionClaim) (ClaimAction, error) {
  reader := bufio.NewReader(rd)
 
  for {
- _, err := fmt.Fprint(wr, printMessage(bindingName, claimGroup, claimResource))
+ _, err := fmt.Fprint(wr, printMessage(bindingName, claim.Group, claim.Resource))
  if err != nil {
  return -1, err
  }
@@ -60,7 +62,7 @@ func getRequiredInput(rd io.Reader, wr io.Writer, bindingName, claimGroup, claim
  if value != "" {
  return inferText(value, wr)
  }
- _, err = fmt.Fprintf(wr, "Input is required. Enter `skip/s` instead. ")
+ _, err = fmt.Fprintf(wr, "Input is required. Enter `(s)kip` instead. ")
  if err != nil {
  return -1, err
  }

pkg/cliplugins/helpers/helpers.go

@@ -51,3 +51,11 @@ func ParseClusterURL(host string) (*url.URL, logicalcluster.Name, error) {
 
  return &ret, clusterName, nil
 }
+
+func GetCurrentClusterName(host string) (logicalcluster.Name, error) {
+ _, currentClusterName, err := ParseClusterURL(host)
+ if err != nil {
+ return logicalcluster.Name{}, fmt.Errorf("current URL %q does not point to cluster workspace", host)
+ }
+ return currentClusterName, nil
+}

pkg/cliplugins/workspace/plugin/kubeconfig.go

@@ -492,9 +492,9 @@ func (o *CreateWorkspaceOptions) Run(ctx context.Context) error {
  if err != nil {
  return err
  }
- _, currentClusterName, err := pluginhelpers.ParseClusterURL(config.Host)
+ currentClusterName, err := pluginhelpers.GetCurrentClusterName(config.Host)
  if err != nil {
- return fmt.Errorf("current URL %q does not point to cluster workspace", config.Host)
+ return err
  }
 
  if o.IgnoreExisting && o.Type != "" && !logicalcluster.New(o.Type).HasPrefix(tenancyv1alpha1.RootCluster) {
@@ -690,9 +690,9 @@ func (o *CreateContextOptions) Run(ctx context.Context) error {
  if !ok {
  return fmt.Errorf("current cluster %q is not found in kubeconfig", currentContext.Cluster)
  }
- _, currentClusterName, err := pluginhelpers.ParseClusterURL(currentCluster.Server)
+ currentClusterName, err := pluginhelpers.GetCurrentClusterName(currentCluster.Server)
  if err != nil {
- return fmt.Errorf("current URL %q does not point to cluster workspace", currentCluster.Server)
+ return err
  }
 
  if o.Name == "" {
@@ -788,9 +788,10 @@ func (o *TreeOptions) Run(ctx context.Context) error {
  if err != nil {
  return err
  }
- _, currentClusterName, err := pluginhelpers.ParseClusterURL(config.Host)
+
+ currentClusterName, err := pluginhelpers.GetCurrentClusterName(config.Host)
  if err != nil {
- return fmt.Errorf("current config context URL %q does not point to workspace", config.Host)
+ return err
  }
 
  tree := treeprint.New()
@@ -820,9 +821,9 @@ func (o *TreeOptions) populateBranch(ctx context.Context, tree treeprint.Tree, n
  }
 
  for _, workspace := range results.Items {
- _, currentClusterName, err := pluginhelpers.ParseClusterURL(workspace.Status.URL)
+ currentClusterName, err := pluginhelpers.GetCurrentClusterName(workspace.Status.URL)
  if err != nil {
- return fmt.Errorf("current config context URL %q does not point to workspace", workspace.Status.URL)
+ return err
  }
  err = o.populateBranch(ctx, b, currentClusterName)
  if err != nil {