Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

enh: target cmd - only register target flags with higher specificty #206

Merged
merged 3 commits into from
Jan 20, 2023
Merged

enh: target cmd - only register target flags with higher specificty #206

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
af74c33
enh: target cmd - only register target flags with higher specificty
sven-petersen Jan 3, 2023
461fd9e
expose TargetFlags on factory
sven-petersen Jan 19, 2023
6edafd4
PR feedback
sven-petersen Jan 20, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
expose TargetFlags on factory
  • Loading branch information
@sven-petersen
sven-petersen committed Jan 19, 2023
commit 461fd9eca60b7a11777bd4ffb98c83b4232705da
8 changes: 8 additions & 0 deletions internal/fake/factory.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ type Factory struct {
Config *config.Config
ClientProviderImpl target.ClientProvider
TargetProviderImpl target.TargetProvider
TargetFlagsImpl target.TargetFlags

// Override the clock implementation. Will use a real clock if not set.
ClockImpl util.Clock
Expand Down Expand Up @@ -58,11 +59,14 @@ func NewFakeFactory(cfg *config.Config, clock util.Clock, clientProvider target.
clock = &util.RealClock{}
}

targetFlags := target.NewTargetFlags("", "", "", "", false)

return &Factory{
Config: cfg,
ClockImpl: clock,
ClientProviderImpl: clientProvider,
TargetProviderImpl: targetProvider,
TargetFlagsImpl: targetFlags,
}
}

Expand Down Expand Up @@ -95,3 +99,7 @@ func (f *Factory) Clock() util.Clock {
func (f *Factory) PublicIPs(ctx context.Context) ([]string, error) {
return []string{"192.0.2.42", "2001:db8::8a2e:370:7334"}, nil
}

func (f *Factory) TargetFlags() target.TargetFlags {
return f.TargetFlagsImpl
}
19 changes: 16 additions & 3 deletions internal/util/factory.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,9 @@ type Factory interface {
// returned slice can contain IPv6, IPv4 or both, in no particular
// order.
PublicIPs(context.Context) ([]string, error)
// TargetFlags returns the TargetFlags to which the cobra flags are bound allowing the user to
// override the target configuration stored on the filesystem.
TargetFlags() target.TargetFlags
}

// FactoryImpl implements util.Factory interface.
Expand All @@ -63,13 +66,19 @@ type FactoryImpl struct {
// if empty.
ConfigFile string

// TargetFlags can be used to completely override the target configuration
// targetFlags can be used to completely override the target configuration
// stored on the filesystem via a CLI flags.
TargetFlags target.TargetFlags
targetFlags target.TargetFlags
}

var _ Factory = &FactoryImpl{}

func NewFactory() *FactoryImpl {
return &FactoryImpl{
targetFlags: target.NewTargetFlags("", "", "", "", false),
}
}
holgerkoser marked this conversation as resolved.
Show resolved Hide resolved

func (f *FactoryImpl) Context() context.Context {
return context.Background()
}
Expand All @@ -92,7 +101,7 @@ func (f *FactoryImpl) Manager() (target.Manager, error) {
return nil, fmt.Errorf("failed to create session directory: %w", err)
}

targetProvider := target.NewTargetProvider(filepath.Join(sessionDirectory, "target.yaml"), f.TargetFlags)
targetProvider := target.NewTargetProvider(filepath.Join(sessionDirectory, "target.yaml"), f.targetFlags)
clientProvider := target.NewClientProvider()

return target.NewManager(cfg, targetProvider, clientProvider, sessionDirectory)
Expand Down Expand Up @@ -125,6 +134,10 @@ func (f *FactoryImpl) PublicIPs(ctx context.Context) ([]string, error) {
return addresses, nil
}

func (f *FactoryImpl) TargetFlags() target.TargetFlags {
return f.targetFlags
}

func callIPify(ctx context.Context, domain string) (*net.IP, error) {
req, err := http.NewRequest("GET", fmt.Sprintf("https://%s/", domain), nil)
if err != nil {
Expand Down
17 changes: 15 additions & 2 deletions internal/util/mocks/mock_factory.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 2 additions & 5 deletions pkg/cmd/cmd.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ import (
cmdsshpatch "github.com/gardener/gardenctl-v2/pkg/cmd/sshpatch"
cmdtarget "github.com/gardener/gardenctl-v2/pkg/cmd/target"
cmdversion "github.com/gardener/gardenctl-v2/pkg/cmd/version"
"github.com/gardener/gardenctl-v2/pkg/target"
)

const (
Expand All @@ -52,12 +51,10 @@ func Execute() {

// NewDefaultGardenctlCommand creates the `gardenctl` command with defaults.
func NewDefaultGardenctlCommand() *cobra.Command {
factory := util.FactoryImpl{
TargetFlags: target.NewTargetFlags("", "", "", "", false),
}
factory := util.NewFactory()
ioStreams := util.NewIOStreams()

return NewGardenctlCommand(&factory, ioStreams)
return NewGardenctlCommand(factory, ioStreams)
}

// NewGardenctlCommand creates the `gardenctl` command.
Expand Down
16 changes: 5 additions & 11 deletions pkg/cmd/cmd_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,19 +26,16 @@ var _ = Describe("Gardenctl command", func() {
)

var (
gardenName string
streams util.IOStreams
out *util.SafeBytesBuffer
targetFlags target.TargetFlags
gardenName string
streams util.IOStreams
out *util.SafeBytesBuffer
)

BeforeEach(func() {
gardenName = cfg.Gardens[0].Name

streams, _, out, _ = util.NewTestIOStreams()

targetFlags = target.NewTargetFlags("", "", "", "", false)

targetProvider := target.NewTargetProvider(filepath.Join(sessionDir, "target.yaml"), nil)
Expect(targetProvider.Write(target.NewTarget(gardenName, projectName, "", shootName))).To(Succeed())
})
Expand All @@ -47,10 +44,7 @@ var _ = Describe("Gardenctl command", func() {
var factory *util.FactoryImpl

BeforeEach(func() {
factory = &util.FactoryImpl{
TargetFlags: targetFlags,
ConfigFile: configFile,
}
factory = util.NewFactory()
})

Context("when running the completion command", func() {
Expand All @@ -74,7 +68,7 @@ var _ = Describe("Gardenctl command", func() {

// check target flags
tf := manager.TargetFlags()
Expect(tf).To(BeIdenticalTo(factory.TargetFlags))
Expect(tf).To(BeIdenticalTo(factory.TargetFlags()))

// check current target values
current, err := manager.CurrentTarget()
Expand Down
11 changes: 4 additions & 7 deletions pkg/cmd/target/control-plane.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ package target

import (
"github.com/spf13/cobra"
utilruntime "k8s.io/apimachinery/pkg/util/runtime"

"github.com/gardener/gardenctl-v2/internal/util"
"github.com/gardener/gardenctl-v2/pkg/cmd/base"
Expand Down Expand Up @@ -38,12 +37,10 @@ gardenctl target control-plane --garden my-garden --project my-project --shoot m

o.AddFlags(cmd.Flags())

manager, err := f.Manager()
utilruntime.Must(err)
manager.TargetFlags().AddGardenFlag(cmd.Flags())
manager.TargetFlags().AddProjectFlag(cmd.Flags())
manager.TargetFlags().AddShootFlag(cmd.Flags())
manager.TargetFlags().AddSeedFlag(cmd.Flags())
f.TargetFlags().AddGardenFlag(cmd.Flags())
f.TargetFlags().AddProjectFlag(cmd.Flags())
f.TargetFlags().AddShootFlag(cmd.Flags())
f.TargetFlags().AddSeedFlag(cmd.Flags())
flags.RegisterCompletionFuncsForTargetFlags(cmd, f, ioStreams, cmd.Flags())

return cmd
Expand Down
5 changes: 1 addition & 4 deletions pkg/cmd/target/project.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ package target

import (
"github.com/spf13/cobra"
utilruntime "k8s.io/apimachinery/pkg/util/runtime"

"github.com/gardener/gardenctl-v2/internal/util"
"github.com/gardener/gardenctl-v2/pkg/cmd/base"
Expand Down Expand Up @@ -38,9 +37,7 @@ gardenctl target project my-project --garden my-garden`,

o.AddFlags(cmd.Flags())

manager, err := f.Manager()
utilruntime.Must(err)
manager.TargetFlags().AddGardenFlag(cmd.Flags())
f.TargetFlags().AddGardenFlag(cmd.Flags())
flags.RegisterCompletionFuncsForTargetFlags(cmd, f, ioStreams, cmd.Flags())

return cmd
Expand Down
5 changes: 1 addition & 4 deletions pkg/cmd/target/seed.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ package target

import (
"github.com/spf13/cobra"
utilruntime "k8s.io/apimachinery/pkg/util/runtime"

"github.com/gardener/gardenctl-v2/internal/util"
"github.com/gardener/gardenctl-v2/pkg/cmd/base"
Expand Down Expand Up @@ -38,9 +37,7 @@ gardenctl target seed my-seed --garden my-garden`,

o.AddFlags(cmd.Flags())

manager, err := f.Manager()
utilruntime.Must(err)
manager.TargetFlags().AddGardenFlag(cmd.Flags())
f.TargetFlags().AddGardenFlag(cmd.Flags())
flags.RegisterCompletionFuncsForTargetFlags(cmd, f, ioStreams, cmd.Flags())

return cmd
Expand Down
9 changes: 3 additions & 6 deletions pkg/cmd/target/shoot.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ package target

import (
"github.com/spf13/cobra"
utilruntime "k8s.io/apimachinery/pkg/util/runtime"

"github.com/gardener/gardenctl-v2/internal/util"
"github.com/gardener/gardenctl-v2/pkg/cmd/base"
Expand Down Expand Up @@ -38,11 +37,9 @@ gardenctl target shoot my-shoot --garden my-garden --project my-project`,

o.AddFlags(cmd.Flags())

manager, err := f.Manager()
utilruntime.Must(err)
manager.TargetFlags().AddGardenFlag(cmd.Flags())
manager.TargetFlags().AddProjectFlag(cmd.Flags())
manager.TargetFlags().AddSeedFlag(cmd.Flags())
f.TargetFlags().AddGardenFlag(cmd.Flags())
f.TargetFlags().AddProjectFlag(cmd.Flags())
f.TargetFlags().AddSeedFlag(cmd.Flags())
flags.RegisterCompletionFuncsForTargetFlags(cmd, f, ioStreams, cmd.Flags())

return cmd
Expand Down
5 changes: 1 addition & 4 deletions pkg/cmd/target/target.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ import (

"github.com/fatih/color"
"github.com/spf13/cobra"
utilruntime "k8s.io/apimachinery/pkg/util/runtime"

"github.com/gardener/gardenctl-v2/internal/util"
"github.com/gardener/gardenctl-v2/pkg/ac"
Expand Down Expand Up @@ -52,9 +51,7 @@ gardenctl target value/that/matches/pattern --control-plane`,

o.AddFlags(cmd.Flags())

manager, err := f.Manager()
utilruntime.Must(err)
manager.TargetFlags().AddFlags(cmd.Flags())
f.TargetFlags().AddFlags(cmd.Flags())
flags.RegisterCompletionFuncsForTargetFlags(cmd, f, ioStreams, cmd.Flags())

return cmd
Expand Down
10 changes: 2 additions & 8 deletions pkg/flags/target_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ var _ = Describe("Target flags", func() {
gardenName2 string
streams util.IOStreams
errOut *util.SafeBytesBuffer
targetFlags target.TargetFlags
)

BeforeEach(func() {
Expand All @@ -52,8 +51,6 @@ var _ = Describe("Target flags", func() {

streams, _, _, errOut = util.NewTestIOStreams()

targetFlags = target.NewTargetFlags("", "", "", "", false)

targetProvider := target.NewTargetProvider(filepath.Join(sessionDir, "target.yaml"), nil)
Expect(targetProvider.Write(target.NewTarget(gardenName1, projectName, "", shootName))).To(Succeed())
})
Expand Down Expand Up @@ -263,10 +260,7 @@ var _ = Describe("Target flags", func() {
var factory *util.FactoryImpl

BeforeEach(func() {
factory = &util.FactoryImpl{
TargetFlags: targetFlags,
ConfigFile: configFile,
}
factory = util.NewFactory()
holgerkoser marked this conversation as resolved.
Show resolved Hide resolved
})

Context("when wrapping completion functions", func() {
Expand Down Expand Up @@ -327,7 +321,7 @@ var _ = Describe("Target flags", func() {

// check target flags values
tf := manager.TargetFlags()
Expect(tf).To(BeIdenticalTo(factory.TargetFlags))
Expect(tf).To(BeIdenticalTo(factory.TargetFlags()))
Expect(tf.GardenName()).To(BeEmpty())
Expect(tf.ProjectName()).To(BeEmpty())
Expect(tf.SeedName()).To(BeEmpty())
Expand Down