Skip to content

Commit

Permalink
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
refactor(cli): avoid in-lining of command logic
Browse files Browse the repository at this point in the history
Signed-off-by: Hidde Beydals <hidde@hhh.computer>
hiddeco committed Feb 29, 2024
1 parent ffd814f commit baa7901
Showing 25 changed files with 1,366 additions and 1,048 deletions.
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
@@ -34,7 +34,6 @@ require (
github.com/withfig/autocomplete-tools/integrations/cobra v1.2.1
go.uber.org/ratelimit v0.3.0
golang.org/x/crypto v0.19.0
golang.org/x/exp v0.0.0-20230807204917-050eac23e9de
golang.org/x/net v0.21.0
golang.org/x/oauth2 v0.17.0
golang.org/x/sync v0.6.0
@@ -54,6 +53,7 @@ require (
require (
github.com/distribution/reference v0.5.0 // indirect
github.com/matttproud/golang_protobuf_extensions/v2 v2.0.0 // indirect
golang.org/x/exp v0.0.0-20230807204917-050eac23e9de // indirect
gopkg.in/evanphx/json-patch.v5 v5.6.0 // indirect
sigs.k8s.io/kustomize/api v0.16.0 // indirect
sigs.k8s.io/kustomize/kyaml v0.16.0 // indirect
199 changes: 105 additions & 94 deletions internal/cli/cmd/apply/apply.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package apply

import (
"context"
goerrors "errors"
"fmt"

@@ -22,10 +23,39 @@ import (

type applyOptions struct {
*option.Option
Config config.CLIConfig

Filenames []string
}

func NewCommand(cfg config.CLIConfig, opt *option.Option) *cobra.Command {
cmdOpts := &applyOptions{
Option: opt,
Config: cfg,

Check warning on line 34 in internal/cli/cmd/apply/apply.go

Codecov / codecov/patch

internal/cli/cmd/apply/apply.go#L32-L34

Added lines #L32 - L34 were not covered by tests
}

cmd := &cobra.Command{
Use: "apply [--project=project] -f (FILENAME)",
Short: "Apply a resource from a file or from stdin",
Example: `
# Apply a stage using the data in stage.yaml
kargo apply -f stage.yaml
`,
RunE: func(cmd *cobra.Command, _ []string) error {
if err := cmdOpts.validate(); err != nil {
return err

Check warning on line 46 in internal/cli/cmd/apply/apply.go

Codecov / codecov/patch

internal/cli/cmd/apply/apply.go#L44-L46

Added lines #L44 - L46 were not covered by tests
}

return cmdOpts.run(cmd.Context())

Check warning on line 49 in internal/cli/cmd/apply/apply.go

Codecov / codecov/patch

internal/cli/cmd/apply/apply.go#L49

Added line #L49 was not covered by tests
},
}

// Register the option flags on the command.
cmdOpts.addFlags(cmd)

Check warning on line 54 in internal/cli/cmd/apply/apply.go

Codecov / codecov/patch

internal/cli/cmd/apply/apply.go#L54

Added line #L54 was not covered by tests

return cmd

Check warning on line 56 in internal/cli/cmd/apply/apply.go

Codecov / codecov/patch

internal/cli/cmd/apply/apply.go#L56

Added line #L56 was not covered by tests
}

// addFlags adds the flags for the apply options to the provided command.
func (o *applyOptions) addFlags(cmd *cobra.Command) {
o.PrintFlags.AddFlags(cmd)

Check warning on line 61 in internal/cli/cmd/apply/apply.go

Codecov / codecov/patch

internal/cli/cmd/apply/apply.go#L60-L61

Added lines #L60 - L61 were not covered by tests
@@ -60,106 +90,87 @@ func (o *applyOptions) validate() error {
return nil

Check warning on line 90 in internal/cli/cmd/apply/apply.go

Codecov / codecov/patch

internal/cli/cmd/apply/apply.go#L90

Added line #L90 was not covered by tests
}

func NewCommand(cfg config.CLIConfig, opt *option.Option) *cobra.Command {
cmdOpts := &applyOptions{Option: opt}

cmd := &cobra.Command{
Use: "apply [--project=project] -f (FILENAME)",
Short: "Apply a resource from a file or from stdin",
Example: `
# Apply a stage using the data in stage.yaml
kargo apply -f stage.yaml
`,
RunE: func(cmd *cobra.Command, args []string) error {
ctx := cmd.Context()

if err := cmdOpts.validate(); err != nil {
return err
}
// run performs the apply operation using the provided options.
func (o *applyOptions) run(ctx context.Context) error {
rawManifest, err := yaml.Read(o.Filenames)
if err != nil {
return errors.Wrap(err, "read manifests")

Check warning on line 97 in internal/cli/cmd/apply/apply.go

Codecov / codecov/patch

internal/cli/cmd/apply/apply.go#L94-L97

Added lines #L94 - L97 were not covered by tests
}

rawManifest, err := yaml.Read(cmdOpts.Filenames)
if err != nil {
return errors.Wrap(err, "read manifests")
}
var printer printers.ResourcePrinter
if ptr.Deref(o.PrintFlags.OutputFormat, "") != "" {
printer, err = o.PrintFlags.ToPrinter()
if err != nil {
return errors.Wrap(err, "new printer")

Check warning on line 104 in internal/cli/cmd/apply/apply.go

Codecov / codecov/patch

internal/cli/cmd/apply/apply.go#L100-L104

Added lines #L100 - L104 were not covered by tests
}
}

var printer printers.ResourcePrinter
if ptr.Deref(cmdOpts.PrintFlags.OutputFormat, "") != "" {
printer, err = cmdOpts.PrintFlags.ToPrinter()
if err != nil {
return errors.Wrap(err, "new printer")
}
}
kargoSvcCli, err := client.GetClientFromConfig(ctx, o.Config, o.Option)
if err != nil {
return errors.Wrap(err, "get client from config")

Check warning on line 110 in internal/cli/cmd/apply/apply.go

Codecov / codecov/patch

internal/cli/cmd/apply/apply.go#L108-L110

Added lines #L108 - L110 were not covered by tests
}

kargoSvcCli, err := client.GetClientFromConfig(ctx, cfg, cmdOpts.Option)
if err != nil {
return errors.Wrap(err, "get client from config")
}
// TODO: Current implementation of apply is not the same as `kubectl` does.
// It actually "replaces" resource with the given file.
// We should provide the same implementation as `kubectl` does.
resp, err := kargoSvcCli.CreateOrUpdateResource(ctx,
connect.NewRequest(&kargosvcapi.CreateOrUpdateResourceRequest{
Manifest: rawManifest,
}))
if err != nil {
return errors.Wrap(err, "apply resource")

Check warning on line 121 in internal/cli/cmd/apply/apply.go

Codecov / codecov/patch

internal/cli/cmd/apply/apply.go#L116-L121

Added lines #L116 - L121 were not covered by tests
}

// TODO: Current implementation of apply is not the same as `kubectl` does.
// It actually "replaces" resource with the given file.
// We should provide the same implementation as `kubectl` does.
resp, err := kargoSvcCli.CreateOrUpdateResource(ctx,
connect.NewRequest(&kargosvcapi.CreateOrUpdateResourceRequest{
Manifest: rawManifest,
}))
if err != nil {
return errors.Wrap(err, "apply resource")
}
resCap := len(resp.Msg.GetResults())
createdRes := make([]*kargosvcapi.CreateOrUpdateResourceResult_CreatedResourceManifest, 0, resCap)
updatedRes := make([]*kargosvcapi.CreateOrUpdateResourceResult_UpdatedResourceManifest, 0, resCap)
errs := make([]error, 0, resCap)
for _, r := range resp.Msg.GetResults() {
switch typedRes := r.GetResult().(type) {
case *kargosvcapi.CreateOrUpdateResourceResult_CreatedResourceManifest:
createdRes = append(createdRes, typedRes)
case *kargosvcapi.CreateOrUpdateResourceResult_UpdatedResourceManifest:
updatedRes = append(updatedRes, typedRes)
case *kargosvcapi.CreateOrUpdateResourceResult_Error:
errs = append(errs, errors.New(typedRes.Error))

Check warning on line 135 in internal/cli/cmd/apply/apply.go

Codecov / codecov/patch

internal/cli/cmd/apply/apply.go#L124-L135

Added lines #L124 - L135 were not covered by tests
}
}

resCap := len(resp.Msg.GetResults())
createdRes := make([]*kargosvcapi.CreateOrUpdateResourceResult_CreatedResourceManifest, 0, resCap)
updatedRes := make([]*kargosvcapi.CreateOrUpdateResourceResult_UpdatedResourceManifest, 0, resCap)
errs := make([]error, 0, resCap)
for _, r := range resp.Msg.GetResults() {
switch typedRes := r.GetResult().(type) {
case *kargosvcapi.CreateOrUpdateResourceResult_CreatedResourceManifest:
createdRes = append(createdRes, typedRes)
case *kargosvcapi.CreateOrUpdateResourceResult_UpdatedResourceManifest:
updatedRes = append(updatedRes, typedRes)
case *kargosvcapi.CreateOrUpdateResourceResult_Error:
errs = append(errs, errors.New(typedRes.Error))
}
}
for _, r := range createdRes {
var obj unstructured.Unstructured
if err := sigyaml.Unmarshal(r.CreatedResourceManifest, &obj); err != nil {
fmt.Fprintf(cmdOpts.IOStreams.ErrOut, "%s",
errors.Wrap(err, "Error: unmarshal created manifest"))
continue
}
if printer == nil {
name := types.NamespacedName{
Namespace: obj.GetNamespace(),
Name: obj.GetName(),
}.String()
fmt.Fprintf(cmdOpts.IOStreams.Out, "%s Created: %q\n", obj.GetKind(), name)
continue
}
_ = printer.PrintObj(&obj, cmdOpts.IOStreams.Out)
}
for _, r := range updatedRes {
var obj unstructured.Unstructured
if err := sigyaml.Unmarshal(r.UpdatedResourceManifest, &obj); err != nil {
fmt.Fprintf(cmdOpts.IOStreams.ErrOut, "%s",
errors.Wrap(err, "Error: unmarshal updated manifest"))
continue
}
if printer == nil {
name := types.NamespacedName{
Namespace: obj.GetNamespace(),
Name: obj.GetName(),
}.String()
fmt.Fprintf(cmdOpts.IOStreams.Out, "%s Updated: %q\n", obj.GetKind(), name)
continue
}
_ = printer.PrintObj(&obj, cmdOpts.IOStreams.Out)
}
return goerrors.Join(errs...)
},
for _, res := range createdRes {
var obj unstructured.Unstructured
if err := sigyaml.Unmarshal(res.CreatedResourceManifest, &obj); err != nil {
fmt.Fprintf(o.IOStreams.ErrOut, "%s",
errors.Wrap(err, "Error: unmarshal created manifest"))
continue

Check warning on line 144 in internal/cli/cmd/apply/apply.go

Codecov / codecov/patch

internal/cli/cmd/apply/apply.go#L139-L144

Added lines #L139 - L144 were not covered by tests
}
if printer == nil {
name := types.NamespacedName{
Namespace: obj.GetNamespace(),
Name: obj.GetName(),
}.String()
fmt.Fprintf(o.IOStreams.Out, "%s Created: %q\n", obj.GetKind(), name)
continue

Check warning on line 152 in internal/cli/cmd/apply/apply.go

Codecov / codecov/patch

internal/cli/cmd/apply/apply.go#L146-L152

Added lines #L146 - L152 were not covered by tests
}
_ = printer.PrintObj(&obj, o.IOStreams.Out)

Check warning on line 154 in internal/cli/cmd/apply/apply.go

Codecov / codecov/patch

internal/cli/cmd/apply/apply.go#L154

Added line #L154 was not covered by tests
}

// Register the option flags on the command.
cmdOpts.addFlags(cmd)
for _, res := range updatedRes {
var obj unstructured.Unstructured
if err := sigyaml.Unmarshal(res.UpdatedResourceManifest, &obj); err != nil {
fmt.Fprintf(o.IOStreams.ErrOut, "%s",
errors.Wrap(err, "Error: unmarshal updated manifest"))
continue

Check warning on line 162 in internal/cli/cmd/apply/apply.go

Codecov / codecov/patch

internal/cli/cmd/apply/apply.go#L157-L162

Added lines #L157 - L162 were not covered by tests
}
if printer == nil {
name := types.NamespacedName{
Namespace: obj.GetNamespace(),
Name: obj.GetName(),
}.String()
fmt.Fprintf(o.IOStreams.Out, "%s Updated: %q\n", obj.GetKind(), name)
continue

Check warning on line 170 in internal/cli/cmd/apply/apply.go

Codecov / codecov/patch

internal/cli/cmd/apply/apply.go#L164-L170

Added lines #L164 - L170 were not covered by tests
}
_ = printer.PrintObj(&obj, o.IOStreams.Out)

Check warning on line 172 in internal/cli/cmd/apply/apply.go

Codecov / codecov/patch

internal/cli/cmd/apply/apply.go#L172

Added line #L172 was not covered by tests
}

return cmd
return goerrors.Join(errs...)

Check warning on line 175 in internal/cli/cmd/apply/apply.go

Codecov / codecov/patch

internal/cli/cmd/apply/apply.go#L175

Added line #L175 was not covered by tests
}
82 changes: 45 additions & 37 deletions internal/cli/cmd/approve/approve.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package approve

import (
"context"
goerrors "errors"

"connectrpc.com/connect"
@@ -15,11 +16,37 @@ import (

type approvalOptions struct {
*option.Option
Config config.CLIConfig

Freight string
Stage string
}

func NewCommand(cfg config.CLIConfig, opt *option.Option) *cobra.Command {
cmdOpts := &approvalOptions{
Option: opt,
Config: cfg,

Check warning on line 28 in internal/cli/cmd/approve/approve.go

Codecov / codecov/patch

internal/cli/cmd/approve/approve.go#L26-L28

Added lines #L26 - L28 were not covered by tests
}

cmd := &cobra.Command{
Use: "approve --project=project --freight=freight --stage=stage",
Short: "Manually approve freight for promotion to a stage",
Example: "kargo approve --project=project --freight=abc1234 --stage=qa",
RunE: func(cmd *cobra.Command, _ []string) error {
if err := cmdOpts.validate(); err != nil {
return err

Check warning on line 37 in internal/cli/cmd/approve/approve.go

Codecov / codecov/patch

internal/cli/cmd/approve/approve.go#L36-L37

Added lines #L36 - L37 were not covered by tests
}

return cmdOpts.run(cmd.Context())

Check warning on line 40 in internal/cli/cmd/approve/approve.go

Codecov / codecov/patch

internal/cli/cmd/approve/approve.go#L40

Added line #L40 was not covered by tests
},
}

// Register the option flags on the command.
cmdOpts.addFlags(cmd)

Check warning on line 45 in internal/cli/cmd/approve/approve.go

Codecov / codecov/patch

internal/cli/cmd/approve/approve.go#L45

Added line #L45 was not covered by tests

return cmd
}

// addFlags adds the flags for the approval options to the provided command.
func (o *approvalOptions) addFlags(cmd *cobra.Command) {

Check warning on line 51 in internal/cli/cmd/approve/approve.go

Codecov / codecov/patch

internal/cli/cmd/approve/approve.go#L51

Added line #L51 was not covered by tests
// TODO: Factor out server flags to a higher level (root?) as they are
@@ -62,43 +89,24 @@ func (o *approvalOptions) validate() error {
return goerrors.Join(errs...)

Check warning on line 89 in internal/cli/cmd/approve/approve.go

Codecov / codecov/patch

internal/cli/cmd/approve/approve.go#L89

Added line #L89 was not covered by tests
}

func NewCommand(cfg config.CLIConfig, opt *option.Option) *cobra.Command {
cmdOpts := &approvalOptions{Option: opt}

cmd := &cobra.Command{
Use: "approve --project=project --freight=freight --stage=stage",
Short: "Manually approve freight for promotion to a stage",
Example: "kargo approve --project=project --freight=abc1234 --stage=qa",
RunE: func(cmd *cobra.Command, _ []string) error {
ctx := cmd.Context()

if err := cmdOpts.validate(); err != nil {
return err
}

kargoSvcCli, err := client.GetClientFromConfig(ctx, cfg, cmdOpts.Option)
if err != nil {
return errors.Wrap(err, "get client from config")
}

if _, err = kargoSvcCli.ApproveFreight(
ctx,
connect.NewRequest(
&v1alpha1.ApproveFreightRequest{
Project: cmdOpts.Project,
Id: cmdOpts.Freight,
Stage: cmdOpts.Stage,
},
),
); err != nil {
return errors.Wrap(err, "approve freight")
}
return nil
},
// run performs the approval of a freight based on the options.
func (o *approvalOptions) run(ctx context.Context) error {
kargoSvcCli, err := client.GetClientFromConfig(ctx, o.Config, o.Option)
if err != nil {
return errors.Wrap(err, "get client from config")

Check warning on line 96 in internal/cli/cmd/approve/approve.go

Codecov / codecov/patch

internal/cli/cmd/approve/approve.go#L93-L96

Added lines #L93 - L96 were not covered by tests
}

// Register the option flags on the command.
cmdOpts.addFlags(cmd)

return cmd
if _, err = kargoSvcCli.ApproveFreight(
ctx,
connect.NewRequest(
&v1alpha1.ApproveFreightRequest{
Project: o.Project,
Id: o.Freight,
Stage: o.Stage,
},
),
); err != nil {
return errors.Wrap(err, "approve freight")

Check warning on line 109 in internal/cli/cmd/approve/approve.go

Codecov / codecov/patch

internal/cli/cmd/approve/approve.go#L99-L109

Added lines #L99 - L109 were not covered by tests
}
return nil

Check warning on line 111 in internal/cli/cmd/approve/approve.go

Codecov / codecov/patch

internal/cli/cmd/approve/approve.go#L111

Added line #L111 was not covered by tests
}
3 changes: 2 additions & 1 deletion internal/cli/cmd/config/config.go
Original file line number Diff line number Diff line change
@@ -12,7 +12,8 @@ func NewCommand(cfg config.CLIConfig) *cobra.Command {
Short: "Manage Kargo CLI configuration",
}

// Subcommands
// Register subcommands.
cmd.AddCommand(newSetProjectCommand(cfg))

return cmd
}
Loading

0 comments on commit baa7901

Please sign in to comment.