From 49cae21846218877b5a068cefda8693288bb5768 Mon Sep 17 00:00:00 2001 From: Bulat Shakirzyanov <83289+avalanche123@users.noreply.github.com> Date: Mon, 1 Apr 2024 17:25:54 -0400 Subject: [PATCH 1/4] allow detaching space without a cluster --- cmd/up/space/attach.go | 4 +- cmd/up/space/detach.go | 110 +++++++++++++++++++++++++++++++++++------ 2 files changed, 96 insertions(+), 18 deletions(-) diff --git a/cmd/up/space/attach.go b/cmd/up/space/attach.go index 37ea943f..bccf8bef 100644 --- a/cmd/up/space/attach.go +++ b/cmd/up/space/attach.go @@ -148,7 +148,7 @@ func (c *attachCmd) Run(ctx context.Context, mgr *helm.Installer, kClient *kuber } }() return undo.Do(func(u undo.Undoer) error { - a, err := c.getAccount(ctx, upCtx, ac) + a, err := getAccount(ctx, upCtx, ac) if err != nil { return err } @@ -352,7 +352,7 @@ func (c *attachCmd) deleteSpace(ctx context.Context, p pterm.TextPrinter, a *acc return nil } -func (c *attachCmd) getAccount(ctx context.Context, upCtx *upbound.Context, ac *accounts.Client) (*accounts.AccountResponse, error) { +func getAccount(ctx context.Context, upCtx *upbound.Context, ac *accounts.Client) (*accounts.AccountResponse, error) { a, err := ac.Get(ctx, upCtx.Account) if err != nil { return nil, errors.Wrapf(err, "failed to get Account %q", upCtx.Account) diff --git a/cmd/up/space/detach.go b/cmd/up/space/detach.go index 4d0da040..149b2cc7 100644 --- a/cmd/up/space/detach.go +++ b/cmd/up/space/detach.go @@ -31,6 +31,8 @@ import ( "k8s.io/client-go/kubernetes" sdkerrs "github.com/upbound/up-sdk-go/errors" + "github.com/upbound/up-sdk-go/service/accounts" + "github.com/upbound/up-sdk-go/service/organizations" "github.com/upbound/up-sdk-go/service/robots" "github.com/upbound/up-sdk-go/service/spaces" "github.com/upbound/up/internal/install/helm" @@ -41,6 +43,8 @@ import ( type detachCmd struct { Upbound upbound.Flags `embed:""` Kube upbound.KubeFlags `embed:""` + + Space string `arg:"" optional:"" help:"Name of the Upbound Space. If name is not a supplied, it will be determined from the connection info in the space."` } func (c *detachCmd) AfterApply(kongCtx *kong.Context) error { @@ -49,8 +53,12 @@ func (c *detachCmd) AfterApply(kongCtx *kong.Context) error { return err } + needsKube := true if err := c.Kube.AfterApply(); err != nil { - return err + if c.Space == "" { + return errors.Wrap(err, "failed to get kube config") + } + needsKube = false } // NOTE(tnthornton) we currently only have support for stylized output. @@ -68,12 +76,20 @@ func (c *detachCmd) AfterApply(kongCtx *kong.Context) error { kongCtx.Bind(upCtx) kongCtx.Bind(robots.NewClient(cfg)) kongCtx.Bind(spaces.NewClient(cfg)) + kongCtx.Bind(accounts.NewClient(cfg)) + kongCtx.Bind(organizations.NewClient(cfg)) + // bind nils as k8s client and helm manager + if !needsKube { + kongCtx.Bind((*kubernetes.Clientset)(nil)) + kongCtx.Bind((*helm.Installer)(nil)) + return nil + } kubeconfig := c.Kube.GetConfig() kClient, err := kubernetes.NewForConfig(kubeconfig) if err != nil { - return err + return errors.Wrap(err, "failed to create kube client") } kongCtx.Bind(kClient) @@ -96,8 +112,12 @@ func (c *detachCmd) AfterApply(kongCtx *kong.Context) error { } // Run executes the detach command. -func (c *detachCmd) Run(ctx context.Context, kClient *kubernetes.Clientset, mgr *helm.Installer, sc *spaces.Client, rc *robots.Client) (rErr error) { - detachSpinner, err := upterm.CheckmarkSuccessSpinner.Start("Disconnecting Space from Upbound Console...") +func (c *detachCmd) Run(ctx context.Context, upCtx *upbound.Context, ac *accounts.Client, oc *organizations.Client, kClient *kubernetes.Clientset, mgr *helm.Installer, sc *spaces.Client, rc *robots.Client) (rErr error) { + msg := "Disconnecting Space from Upbound Console..." + if c.Space != "" { + msg = fmt.Sprintf("Disconnecting Space %q from Upbound Console...", c.Space) + } + detachSpinner, err := upterm.CheckmarkSuccessSpinner.Start(msg) if err != nil { return err } @@ -106,21 +126,68 @@ func (c *detachCmd) Run(ctx context.Context, kClient *kubernetes.Clientset, mgr detachSpinner.Fail(rErr) } }() - if err := c.deleteResources(ctx, detachSpinner.InfoPrinter, kClient, rc, sc); err != nil { + if err := c.detachSpace(ctx, detachSpinner.InfoPrinter, upCtx, ac, oc, kClient, mgr, rc, sc); err != nil { return err } - detachSpinner.InfoPrinter.Println("Uninstalling connect agent...") - if err := mgr.Uninstall(); err != nil && !errors.Is(err, driver.ErrReleaseNotFound) { - return errors.Wrapf(err, `failed to uninstall Chart "%s/%s"`, agentNs, agentChart) + msg = "Space has been successfully disconnected from Upbound Console" + if c.Space != "" { + msg = fmt.Sprintf("Space %q has been successfully disconnected from Upbound Console", c.Space) } - if err := deleteTokenSecret(ctx, detachSpinner.InfoPrinter, kClient, agentNs, agentSecret); err != nil && !kerrors.IsNotFound(err) { - return err + detachSpinner.Success(msg) + return nil +} + +func (c *detachCmd) detachSpace(ctx context.Context, p pterm.TextPrinter, upCtx *upbound.Context, ac *accounts.Client, oc *organizations.Client, kClient *kubernetes.Clientset, mgr *helm.Installer, rc *robots.Client, sc *spaces.Client) error { + if kClient == nil { + p.Printfln("Not connected to a Space cluster, deleting API resources only...") + a, err := getAccount(ctx, upCtx, ac) + if err != nil { + return err + } + if err := c.deleteRobot(ctx, p, oc, rc, a); err != nil { + return err + } + if err := c.deleteSpace(ctx, p, sc, a); err != nil { + return err + } + return nil + } + return c.deleteResources(ctx, p, kClient, mgr, rc, sc) +} + +func (c *detachCmd) deleteSpace(ctx context.Context, p pterm.TextPrinter, sc *spaces.Client, ar *accounts.AccountResponse) error { + p.Printf(`Deleting Space "%s/%s"`, ar.Organization.Name, c.Space) + if err := sc.Delete(ctx, ar.Organization.Name, c.Space, nil); err != nil && !kerrors.IsNotFound(err) { + return errors.Wrapf(err, `failed to delete Space "%s/%s"`, ar.Organization.Name, c.Space) } - detachSpinner.Success("Space has been successfully disconnected from Upbound Console") + p.Printfln(`Space "%s/%s" deleted`, ar.Organization.Name, c.Space) + // replace space with full name for display purposes + c.Space = fmt.Sprintf("%s/%s", ar.Organization.Name, c.Space) return nil } -func (c *detachCmd) deleteResources(ctx context.Context, p pterm.TextPrinter, kClient *kubernetes.Clientset, rc *robots.Client, sc *spaces.Client) error { +func (c *detachCmd) deleteRobot(ctx context.Context, p pterm.TextPrinter, oc *organizations.Client, rc *robots.Client, ar *accounts.AccountResponse) error { + p.Printf("Looking for robot token for Space %q", c.Space) + rr, err := oc.ListRobots(ctx, ar.Organization.ID) + if err != nil { + return errors.Wrap(err, "failed to list Robots") + } + for _, r := range rr { + if r.Name != c.Space { + continue + } + p.Printfln(`Deleting Robot "%s/%s"`, ar.Organization.Name, c.Space) + if err := rc.Delete(ctx, r.ID); err != nil && !sdkerrs.IsNotFound(err) { + return errors.Wrapf(err, `failed to delete Robot "%s/%s"`, ar.Organization.Name, c.Space) + } + p.Printfln(`Robot "%s/%s" deleted`, ar.Organization.Name, c.Space) + return nil + } + p.Printf("No robot token for Space %q, skipping...", c.Space) + return nil +} + +func (c *detachCmd) deleteResources(ctx context.Context, p pterm.TextPrinter, kClient *kubernetes.Clientset, mgr *helm.Installer, rc *robots.Client, sc *spaces.Client) error { cm, err := getConnectConfigmap(ctx, kClient, agentNs, connConfMap) if kerrors.IsNotFound(err) { return nil @@ -129,15 +196,22 @@ func (c *detachCmd) deleteResources(ctx context.Context, p pterm.TextPrinter, kC return errors.Wrapf(err, `failed to get ConfigMap "%s/%s"`, agentNs, agentSecret) } p.Printfln(`ConfigMap "%s/%s" found, deleting resources in Upbound Console...`, agentNs, agentSecret) - if err := c.deleteAgentRobot(ctx, p, kClient, rc, &cm); err != nil { + if err := c.deleteGeneratedSpace(ctx, p, kClient, sc, &cm); err != nil { return err } - if err := c.deleteGeneratedSpace(ctx, p, kClient, sc, &cm); err != nil { + if err := c.deleteAgentRobot(ctx, p, kClient, rc, &cm); err != nil { return err } if err := deleteConnectConfigmap(ctx, p, kClient, agentNs, connConfMap); err != nil { return err } + p.Println("Uninstalling connect agent...") + if err := mgr.Uninstall(); err != nil && !errors.Is(err, driver.ErrReleaseNotFound) { + return errors.Wrapf(err, `failed to uninstall Chart "%s/%s"`, agentNs, agentChart) + } + if err := deleteTokenSecret(ctx, p, kClient, agentNs, agentSecret); err != nil && !kerrors.IsNotFound(err) { + return err + } return nil } @@ -175,10 +249,14 @@ func (c *detachCmd) deleteGeneratedSpace(ctx context.Context, p pterm.TextPrinte if len(parts) != 2 || parts[0] == "" || parts[1] == "" { return fmt.Errorf("invalid space %q", v) } - p.Printfln("Deleting Space %q", v) ns, name := parts[0], parts[1] + if c.Space != "" && c.Space != name { + return fmt.Errorf("connected Space %q does not match specified name %q", name, c.Space) + } + c.Space = name + p.Printfln("Deleting Space %q", name) if err := sc.Delete(ctx, ns, name, nil); err != nil && !kerrors.IsNotFound(err) { - return errors.Wrapf(err, `failed to delete Space "%s/%s"`, ns, name) + return errors.Wrapf(err, `failed to delete Space %q`, name) } delete(cm.Data, keySpace) cm, err := kClient.CoreV1().ConfigMaps(agentNs).Update(ctx, cm, metav1.UpdateOptions{}) From fac7ffc86d29818c5bf5718c361d7468cc3caefd Mon Sep 17 00:00:00 2001 From: Bulat Shakirzyanov <83289+avalanche123@users.noreply.github.com> Date: Tue, 2 Apr 2024 12:27:15 -0400 Subject: [PATCH 2/4] add confirmations for attach/detach conditions --- cmd/up/space/attach.go | 44 ++++++++++++++++++++++++++++++------------ cmd/up/space/detach.go | 19 ++++++++++++------ 2 files changed, 45 insertions(+), 18 deletions(-) diff --git a/cmd/up/space/attach.go b/cmd/up/space/attach.go index bccf8bef..21b2a4c0 100644 --- a/cmd/up/space/attach.go +++ b/cmd/up/space/attach.go @@ -158,7 +158,7 @@ func (c *attachCmd) Run(ctx context.Context, mgr *helm.Installer, kClient *kuber return err } - if err := c.prepareSpace(ctx, attachSpinner.InfoPrinter, kClient, a, sc, u, &cc); err != nil { + if err := c.prepareSpace(ctx, attachSpinner, kClient, a, sc, u, &cc); err != nil { return err } attachSpinner.UpdateText(fmt.Sprintf("Connecting Space %q to Upbound Console...", cc.Data[keySpace])) @@ -167,7 +167,7 @@ func (c *attachCmd) Run(ctx context.Context, mgr *helm.Installer, kClient *kuber return err } - attachSpinner.UpdateText("Installing agent...") + attachSpinner.UpdateText("Installing Upbound agent...") if err := c.createNamespace(ctx, attachSpinner.InfoPrinter, kClient, agentNs, u); err != nil { return err } @@ -278,7 +278,7 @@ func (c *attachCmd) prepareToken(ctx context.Context, p pterm.TextPrinter, kClie return nil } -func (c *attachCmd) prepareSpace(ctx context.Context, p pterm.TextPrinter, kClient *kubernetes.Clientset, a *accounts.AccountResponse, sc *spaces.Client, u undo.Undoer, cmr **corev1.ConfigMap) error { //nolint:gocyclo +func (c *attachCmd) prepareSpace(ctx context.Context, attachSpinner *pterm.SpinnerPrinter, kClient *kubernetes.Clientset, a *accounts.AccountResponse, sc *spaces.Client, u undo.Undoer, cmr **corev1.ConfigMap) error { //nolint:gocyclo cm := *cmr space := &upboundv1alpha1.Space{ ObjectMeta: metav1.ObjectMeta{ @@ -297,14 +297,14 @@ func (c *attachCmd) prepareSpace(ctx context.Context, p pterm.TextPrinter, kClie } ns, name := parts[0], parts[1] if space.Namespace == ns && (space.Name == "" || space.Name == name) { - p.Printfln("Using Space %q", v) + attachSpinner.InfoPrinter.Printfln("Using Space %q", v) u.Undo(func() error { - return c.deleteSpace(ctx, p, a, sc) + return c.deleteSpace(ctx, attachSpinner.InfoPrinter, a, sc) }) c.Space = name return nil } - p.Printfln("Not using Space %q", v) + attachSpinner.InfoPrinter.Printfln("Not using Space %q", v) delete(cm.Data, keySpace) var err error cm, err = kClient.CoreV1().ConfigMaps(agentNs).Update(ctx, cm, metav1.UpdateOptions{}) @@ -313,7 +313,7 @@ func (c *attachCmd) prepareSpace(ctx context.Context, p pterm.TextPrinter, kClie } *cmr = cm } - name, err := c.createSpace(ctx, p, kClient, a, space, sc, u, cmr) + name, err := c.createSpace(ctx, attachSpinner, kClient, a, space, sc, u, cmr) if err != nil { return err } @@ -321,20 +321,38 @@ func (c *attachCmd) prepareSpace(ctx context.Context, p pterm.TextPrinter, kClie return nil } -func (c *attachCmd) createSpace(ctx context.Context, p pterm.TextPrinter, kClient *kubernetes.Clientset, a *accounts.AccountResponse, space *upboundv1alpha1.Space, sc *spaces.Client, u undo.Undoer, cmr **corev1.ConfigMap) (string, error) { +func (c *attachCmd) createSpace(ctx context.Context, attachSpinner *pterm.SpinnerPrinter, kClient *kubernetes.Clientset, a *accounts.AccountResponse, space *upboundv1alpha1.Space, sc *spaces.Client, u undo.Undoer, cmr **corev1.ConfigMap) (string, error) { cm := *cmr - p.Printfln("Creating a new Space in Upbound Console in organization %q...", a.Organization.Name) + attachSpinner.InfoPrinter.Printfln("Creating a new Space in Upbound Console in organization %q...", a.Organization.Name) space, err := sc.Create(ctx, a.Organization.Name, space, nil) if err != nil { if kerrors.IsAlreadyExists(err) && c.Space != "" { + attachSpinner.UpdateText("Continue? (Y/n)") + if err := warnAndConfirm( + `Space "%s/%s" already exists. Would you like to overwrite it?`+"\n\n"+ + " If the other Space cluster still exists, it will continue to have the Upbound agent running and you will need to delete manually.\n", + a.Organization.Name, c.Space, + ); err != nil { + return "", err + } + attachSpinner.UpdateText(fmt.Sprintf("Connecting Space %q to Upbound Console...", c.Space)) + if cm.Data[keySpace] == path.Join(a.Organization.Name, c.Space) { + return c.Space, nil + } + cm.Data[keySpace] = path.Join(a.Organization.Name, c.Space) + cm, err = kClient.CoreV1().ConfigMaps(agentNs).Update(ctx, cm, metav1.UpdateOptions{}) + if err != nil { + return "", errors.Wrapf(err, `failed to update ConfigMap "%s/%s"`, agentNs, connConfMap) + } + *cmr = cm return c.Space, nil } return "", errors.Wrapf(err, errCreateSpace) } u.Undo(func() error { - return c.deleteSpace(ctx, p, a, sc) + return c.deleteSpace(ctx, attachSpinner.InfoPrinter, a, sc) }) - p.Printfln(`Space "%s/%s" created`, a.Organization.Name, space.Name) + attachSpinner.InfoPrinter.Printfln(`Space "%s/%s" created`, a.Organization.Name, space.Name) cm.Data[keySpace] = path.Join(a.Organization.Name, space.Name) cm, err = kClient.CoreV1().ConfigMaps(agentNs).Update(ctx, cm, metav1.UpdateOptions{}) if err != nil { @@ -440,6 +458,7 @@ func deleteConnectConfigmap(ctx context.Context, p pterm.TextPrinter, kClient *k } func (c *attachCmd) createTokenSecret(ctx context.Context, p pterm.TextPrinter, kClient *kubernetes.Clientset, ns, name string, u undo.Undoer) error { + p.Printfln(`Creating Secret "%s/%s"`, ns, name) _, err := kClient.CoreV1().Secrets(ns).Create(ctx, &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ Name: name, @@ -459,6 +478,7 @@ func (c *attachCmd) createTokenSecret(ctx context.Context, p pterm.TextPrinter, if !kerrors.IsAlreadyExists(err) { return errors.Wrapf(err, `failed to create Secret "%s/%s"`, ns, name) } + p.Printfln(`Secret "%s/%s" exists, updating...`, ns, name) // secret already exists, replace it s, err := kClient.CoreV1().Secrets(ns).Get(ctx, name, metav1.GetOptions{}) if err != nil { @@ -505,7 +525,7 @@ func (c *attachCmd) createRobot(ctx context.Context, p pterm.TextPrinter, kClien if r.Name != c.Space { continue } - p.Printfln(`Reusing existing Robot "%s/%s"`, ar.Organization.Name, c.Space) + p.Printfln(`Robot "%s/%s" exists`, ar.Organization.Name, c.Space) // delete generated robot at clean up u.Undo(func() error { return c.deleteRobot(ctx, p, ar, rc, r.ID) diff --git a/cmd/up/space/detach.go b/cmd/up/space/detach.go index 149b2cc7..c9b443cb 100644 --- a/cmd/up/space/detach.go +++ b/cmd/up/space/detach.go @@ -126,7 +126,7 @@ func (c *detachCmd) Run(ctx context.Context, upCtx *upbound.Context, ac *account detachSpinner.Fail(rErr) } }() - if err := c.detachSpace(ctx, detachSpinner.InfoPrinter, upCtx, ac, oc, kClient, mgr, rc, sc); err != nil { + if err := c.detachSpace(ctx, detachSpinner, upCtx, ac, oc, kClient, mgr, rc, sc); err != nil { return err } msg = "Space has been successfully disconnected from Upbound Console" @@ -137,22 +137,29 @@ func (c *detachCmd) Run(ctx context.Context, upCtx *upbound.Context, ac *account return nil } -func (c *detachCmd) detachSpace(ctx context.Context, p pterm.TextPrinter, upCtx *upbound.Context, ac *accounts.Client, oc *organizations.Client, kClient *kubernetes.Clientset, mgr *helm.Installer, rc *robots.Client, sc *spaces.Client) error { +func (c *detachCmd) detachSpace(ctx context.Context, detachSpinner *pterm.SpinnerPrinter, upCtx *upbound.Context, ac *accounts.Client, oc *organizations.Client, kClient *kubernetes.Clientset, mgr *helm.Installer, rc *robots.Client, sc *spaces.Client) error { if kClient == nil { - p.Printfln("Not connected to a Space cluster, deleting API resources only...") + detachSpinner.UpdateText("Continue? (Y/n)") + if err := warnAndConfirm( + "Not connected to a Space cluster, would you like to only remove the Space from the Upbound Console?\n\n" + + " If the Space cluster still exists, it will continue to have the Upbound agent running and you will need to delete manually.\n", + ); err != nil { + return err + } + detachSpinner.UpdateText(fmt.Sprintf("Disconnecting Space %q from Upbound Console...", c.Space)) a, err := getAccount(ctx, upCtx, ac) if err != nil { return err } - if err := c.deleteRobot(ctx, p, oc, rc, a); err != nil { + if err := c.deleteRobot(ctx, detachSpinner.InfoPrinter, oc, rc, a); err != nil { return err } - if err := c.deleteSpace(ctx, p, sc, a); err != nil { + if err := c.deleteSpace(ctx, detachSpinner.InfoPrinter, sc, a); err != nil { return err } return nil } - return c.deleteResources(ctx, p, kClient, mgr, rc, sc) + return c.deleteResources(ctx, detachSpinner.InfoPrinter, kClient, mgr, rc, sc) } func (c *detachCmd) deleteSpace(ctx context.Context, p pterm.TextPrinter, sc *spaces.Client, ar *accounts.AccountResponse) error { From 00b2f7690ffef57cf74ff08e60f5f4e12aa3c4cf Mon Sep 17 00:00:00 2001 From: Bulat Shakirzyanov <83289+avalanche123@users.noreply.github.com> Date: Tue, 2 Apr 2024 14:11:15 -0400 Subject: [PATCH 3/4] Apply suggestions from code review Co-authored-by: Nicholas Thomson --- cmd/up/space/attach.go | 2 +- cmd/up/space/detach.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/cmd/up/space/attach.go b/cmd/up/space/attach.go index 21b2a4c0..2cd7cd50 100644 --- a/cmd/up/space/attach.go +++ b/cmd/up/space/attach.go @@ -330,7 +330,7 @@ func (c *attachCmd) createSpace(ctx context.Context, attachSpinner *pterm.Spinne attachSpinner.UpdateText("Continue? (Y/n)") if err := warnAndConfirm( `Space "%s/%s" already exists. Would you like to overwrite it?`+"\n\n"+ - " If the other Space cluster still exists, it will continue to have the Upbound agent running and you will need to delete manually.\n", + " If the other Space cluster still exists, the Upbound agent will be left running and you will need to delete it manually.\n", a.Organization.Name, c.Space, ); err != nil { return "", err diff --git a/cmd/up/space/detach.go b/cmd/up/space/detach.go index c9b443cb..b8a7168e 100644 --- a/cmd/up/space/detach.go +++ b/cmd/up/space/detach.go @@ -142,7 +142,7 @@ func (c *detachCmd) detachSpace(ctx context.Context, detachSpinner *pterm.Spinne detachSpinner.UpdateText("Continue? (Y/n)") if err := warnAndConfirm( "Not connected to a Space cluster, would you like to only remove the Space from the Upbound Console?\n\n" + - " If the Space cluster still exists, it will continue to have the Upbound agent running and you will need to delete manually.\n", + " If the other Space cluster still exists, the Upbound agent will be left running and you will need to delete it manually.\n", ); err != nil { return err } From fef4e27d98507de2c5979e84a8aace82f8a7077f Mon Sep 17 00:00:00 2001 From: Bulat Shakirzyanov <83289+avalanche123@users.noreply.github.com> Date: Tue, 2 Apr 2024 15:16:52 -0400 Subject: [PATCH 4/4] fix lint --- cmd/up/space/detach.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/up/space/detach.go b/cmd/up/space/detach.go index b8a7168e..68725b02 100644 --- a/cmd/up/space/detach.go +++ b/cmd/up/space/detach.go @@ -142,7 +142,7 @@ func (c *detachCmd) detachSpace(ctx context.Context, detachSpinner *pterm.Spinne detachSpinner.UpdateText("Continue? (Y/n)") if err := warnAndConfirm( "Not connected to a Space cluster, would you like to only remove the Space from the Upbound Console?\n\n" + - " If the other Space cluster still exists, the Upbound agent will be left running and you will need to delete it manually.\n", + " If the other Space cluster still exists, the Upbound agent will be left running and you will need to delete it manually.\n", ); err != nil { return err }