airship/airshipctl
Code Issues Proposed changes

Merge "Change logic of config set-context cmd"

Browse Source
This commit is contained in:
Zuul 2021-03-31 02:11:41 +00:00 committed by Gerrit Code Review
commit bba25054b0
12 changed files with 226 additions and 222 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

78
cmd/config/set_context.go
View File

@ -15,9 +15,8 @@ limitations under the License.
package config
import (
"fmt"
"github.com/spf13/cobra"
"github.com/spf13/pflag"
"opendev.org/airship/airshipctl/pkg/config"
)
@ -37,6 +36,10 @@ airshipctl config set-context \
--current \
--manifest=exampleManifest
`
setContextManifestFlag = "manifest"
setContextManagementConfigFlag = "management-config"
setContextCurrentFlag = "current"
)
// NewSetContextCommand creates a command for creating and modifying contexts
@ -49,51 +52,60 @@ func NewSetContextCommand(cfgFactory config.Factory) *cobra.Command {
Long: setContextLong[1:],
Example: setContextExample,
Args: cobra.MaximumNArgs(1),
RunE: func(cmd *cobra.Command, args []string) error {
nFlags := cmd.Flags().NFlag()
if len(args) == 1 {
// context name is made optional with --current flag added
o.Name = args[0]
}
if o.Name != "" && nFlags == 0 {
fmt.Fprintf(cmd.OutOrStdout(), "Context %q not modified. No new options provided.\n", o.Name)
return nil
RunE: setContextRunE(cfgFactory, o),
}
cfg, err := cfgFactory()
if err != nil {
return err
}
modified, err := config.RunSetContext(o, cfg, true)
if err != nil {
return err
}
if modified {
fmt.Fprintf(cmd.OutOrStdout(), "Context %q modified.\n", o.Name)
} else {
fmt.Fprintf(cmd.OutOrStdout(), "Context %q created.\n", o.Name)
}
return nil
},
}
addSetContextFlags(o, cmd)
addSetContextFlags(cmd, o)
return cmd
}
func addSetContextFlags(o *config.ContextOptions, cmd *cobra.Command) {
func addSetContextFlags(cmd *cobra.Command, o *config.ContextOptions) {
flags := cmd.Flags()
flags.StringVar(
&o.Manifest,
"manifest",
setContextManifestFlag,
"",
"set the manifest for the specified context")
flags.StringVar(
&o.ManagementConfiguration,
setContextManagementConfigFlag,
"",
"set the management config for the specified context")
flags.BoolVar(
&o.Current,
"current",
setContextCurrentFlag,
false,
"update the current context")
}
func setContextRunE(cfgFactory config.Factory, o *config.ContextOptions) func(cmd *cobra.Command, args []string) error {
return func(cmd *cobra.Command, args []string) error {
ctxName := ""
if len(args) == 1 {
ctxName = args[0]
}
// Go through all the flags that have been set
var opts []config.ContextOption
fn := func(flag *pflag.Flag) {
switch flag.Name {
case setContextManifestFlag:
opts = append(opts, config.SetContextManifest(o.Manifest))
case setContextManagementConfigFlag:
opts = append(opts, config.SetContextManagementConfig(o.ManagementConfiguration))
}
}
cmd.Flags().Visit(fn)
options := &config.RunSetContextOptions{
CfgFactory: cfgFactory,
CtxName: ctxName,
Current: o.Current,
Writer: cmd.OutOrStdout(),
}
return options.RunSetContext(opts...)
}
}

92
cmd/config/set_context_test.go
View File

@ -15,109 +15,21 @@ limitations under the License.
package config_test
import (
"fmt"
"strings"
"testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
cmd "opendev.org/airship/airshipctl/cmd/config"
"opendev.org/airship/airshipctl/pkg/config"
"opendev.org/airship/airshipctl/cmd/config"
"opendev.org/airship/airshipctl/testutil"
)
const (
defaultManifest = "edge_cloud"
)
type setContextTest struct {
givenConfig *config.Config
cmdTest *testutil.CmdTest
contextName string
manifest string
}
func TestConfigSetContext(t *testing.T) {
cmdTests := []*testutil.CmdTest{
{
Name: "config-cmd-set-context-with-help",
CmdLine: "--help",
Cmd: cmd.NewSetContextCommand(nil),
},
{
Name: "config-cmd-set-context-no-flags",
CmdLine: "context",
Cmd: cmd.NewSetContextCommand(nil),
},
{
Name: "config-cmd-set-context-too-many-args",
CmdLine: "arg1 arg2",
Cmd: cmd.NewSetContextCommand(nil),
Error: fmt.Errorf("accepts at most %d arg(s), received %d", 1, 2),
Cmd: config.NewSetContextCommand(nil),
},
}
for _, tt := range cmdTests {
testutil.RunTest(t, tt)
}
}
func TestSetContext(t *testing.T) {
given, cleanupGiven := testutil.InitConfig(t)
defer cleanupGiven(t)
tests := []struct {
testName string
contextName string
flags []string
givenConfig *config.Config
manifest string
}{
{
testName: "set-context",
contextName: "dummycontext",
flags: []string{
"--manifest=" + defaultManifest,
},
givenConfig: given,
manifest: defaultManifest,
},
}
for _, tt := range tests {
tt := tt
cmd := &testutil.CmdTest{
Name: tt.testName,
CmdLine: fmt.Sprintf("%s %s", tt.contextName, strings.Join(tt.flags, " ")),
}
test := setContextTest{
givenConfig: tt.givenConfig,
cmdTest: cmd,
contextName: tt.contextName,
manifest: tt.manifest,
}
test.run(t)
}
}
func (test setContextTest) run(t *testing.T) {
// Get the Environment
settings := func() (*config.Config, error) {
return test.givenConfig, nil
}
test.cmdTest.Cmd = cmd.NewSetContextCommand(settings)
testutil.RunTest(t, test.cmdTest)
afterRunConf := test.givenConfig
// Find the Context Created or Modified
afterRunContext, err := afterRunConf.GetContext(test.contextName)
require.NoError(t, err)
require.NotNil(t, afterRunContext)
if test.manifest != "" {
assert.EqualValues(t, afterRunContext.Manifest, test.manifest)
}
}

1
cmd/config/testdata/TestConfigSetContextGoldenOutput/config-cmd-set-context-no-flags.golden vendored
View File

@ -1 +0,0 @@
Context "context" not modified. No new options provided.

21
cmd/config/testdata/TestConfigSetContextGoldenOutput/config-cmd-set-context-too-many-args.golden vendored
View File

@ -1,21 +0,0 @@
Error: accepts at most 1 arg(s), received 2
Usage:
set-context NAME [flags]
Examples:
# Create a new context named "exampleContext"
airshipctl config set-context exampleContext \
--manifest=exampleManifest \
# Update the manifest of the current-context
airshipctl config set-context \
--current \
--manifest=exampleManifest
Flags:
--current update the current context
-h, --help help for set-context
--manifest string set the manifest for the specified context

1
cmd/config/testdata/TestConfigSetContextGoldenOutput/config-cmd-set-context-with-help.golden vendored
View File

@ -18,4 +18,5 @@ airshipctl config set-context \
Flags:
--current update the current context
-h, --help help for set-context
--management-config string set the management config for the specified context
--manifest string set the manifest for the specified context

1
cmd/config/testdata/TestSetContextGoldenOutput/set-context.golden vendored
View File

@ -1 +0,0 @@
Context "dummycontext" created.

1
docs/source/cli/airshipctl_config_set-context.md
View File

@ -31,6 +31,7 @@ airshipctl config set-context \
```
--current update the current context
-h, --help help for set-context
--management-config string set the management config for the specified context
--manifest string set the manifest for the specified context
```

1
go.mod
View File

@ -30,6 +30,7 @@ require (
github.com/opencontainers/image-spec v1.0.1 // indirect
github.com/pkg/errors v0.9.1
github.com/spf13/cobra v1.0.0
github.com/spf13/pflag v1.0.5
github.com/stretchr/testify v1.6.1
golang.org/x/net v0.0.0-20201021035429-f5854403a974 // indirect
golang.org/x/sys v0.0.0-20210119212857-b64e53b001e4 // indirect

22
pkg/config/config.go
View File

@ -173,6 +173,11 @@ func (c *Config) EnsureComplete() error {
return nil
}
// SetFs allows to set custom filesystem used in Config object. Required for unit tests
func (c *Config) SetFs(fsys fs.FileSystem) {
c.fileSystem = fsys
}
// PersistConfig updates the airshipctl config file to match
// the current Config object.
// If file did not previously exist, the file will be created.
@ -278,24 +283,21 @@ func (c *Config) GetManagementConfiguration(name string) (*ManagementConfigurati
// AddContext creates a new context and returns the instance of
// newly created context
func (c *Config) AddContext(theContext *ContextOptions) *Context {
func (c *Config) AddContext(ctxName string, opts ...ContextOption) *Context {
// Create the new Airship config context
nContext := NewContext()
c.Contexts[theContext.Name] = nContext
c.Contexts[ctxName] = nContext
// Ok , I have initialized structs for the Context information
// We can use Modify to populate the correct information
c.ModifyContext(nContext, theContext)
c.ModifyContext(nContext, opts...)
return nContext
}
// ModifyContext updates Context object with given given context options
func (c *Config) ModifyContext(context *Context, theContext *ContextOptions) {
if theContext.ManagementConfiguration != "" {
context.ManagementConfiguration = theContext.ManagementConfiguration
}
if theContext.Manifest != "" {
context.Manifest = theContext.Manifest
// ModifyContext updates Context object with given context options
func (c *Config) ModifyContext(context *Context, opts ...ContextOption) {
for _, o := range opts {
o(context)
}
}

90
pkg/config/config_helper.go
View File

@ -15,58 +15,72 @@ limitations under the License.
package config
import (
"errors"
"fmt"
"io"
)
// ContextOption is a function that allows to modify context object
type ContextOption func(ctx *Context)
// SetContextManifest sets manifest in context object
func SetContextManifest(manifest string) ContextOption {
return func(ctx *Context) {
ctx.Manifest = manifest
}
}
// SetContextManagementConfig sets management config in context object
func SetContextManagementConfig(managementConfig string) ContextOption {
return func(ctx *Context) {
ctx.ManagementConfiguration = managementConfig
}
}
// RunSetContextOptions are options required to create/modify airshipctl context
type RunSetContextOptions struct {
CfgFactory Factory
CtxName string
Current bool
Writer io.Writer
}
// RunSetContext validates the given command line options and invokes AddContext/ModifyContext
func RunSetContext(o *ContextOptions, airconfig *Config, writeToStorage bool) (bool, error) {
modified := false
err := o.Validate()
func (o *RunSetContextOptions) RunSetContext(opts ...ContextOption) error {
cfg, err := o.CfgFactory()
if err != nil {
return modified, err
return err
}
if o.Current {
if airconfig.CurrentContext == "" {
return modified, ErrMissingCurrentContext{}
}
// when --current flag is passed, use current context
o.Name = airconfig.CurrentContext
o.CtxName = cfg.CurrentContext
}
context, err := airconfig.GetContext(o.Name)
if o.CtxName == "" {
return ErrEmptyContextName{}
}
infoMsg := fmt.Sprintf("context with name %s", o.CtxName)
context, err := cfg.GetContext(o.CtxName)
if err != nil {
var cerr ErrMissingConfig
if !errors.As(err, &cerr) {
// An error occurred, but it wasn't a "missing" config error.
return modified, err
}
if o.CurrentContext {
return modified, ErrMissingConfig{}
}
// context didn't exist, create it
// ignoring the returned added context
airconfig.AddContext(o)
} else {
// Found the desired Current Context
// Lets update it and be done.
if o.CurrentContext {
airconfig.CurrentContext = o.Name
cfg.AddContext(o.CtxName, opts...)
infoMsg = fmt.Sprintf("%s created\n", infoMsg)
} else {
// Context exists, lets update
airconfig.ModifyContext(context, o)
}
modified = true
}
// Update configuration file just in time persistence approach
if writeToStorage {
if err := airconfig.PersistConfig(true); err != nil {
// Error that it didnt persist the changes
return modified, ErrConfigFailed{}
}
cfg.ModifyContext(context, opts...)
infoMsg = fmt.Sprintf("%s modified\n", infoMsg)
}
return modified, nil
// Verify we didn't break anything
if err = cfg.EnsureComplete(); err != nil {
return err
}
if _, err := o.Writer.Write([]byte(infoMsg)); err != nil {
return err
}
// Update configuration file just in time persistence approach
return cfg.PersistConfig(true)
}
// RunUseContext validates the given context name and updates it as current context

117
pkg/config/config_helper_test.go
View File

@ -15,34 +15,117 @@ limitations under the License.
package config_test
import (
"bytes"
"os"
"testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
kustfs "sigs.k8s.io/kustomize/api/filesys"
"opendev.org/airship/airshipctl/pkg/config"
"opendev.org/airship/airshipctl/testutil"
testfs "opendev.org/airship/airshipctl/testutil/fs"
)
func prepareConfig() func() (*config.Config, error) {
return func() (*config.Config, error) {
cfg := testutil.DummyConfig()
cfg.SetLoadedConfigPath("test")
cfg.SetFs(testfs.MockFileSystem{
FileSystem: kustfs.MakeFsInMemory(),
MockChmod: func(s string, mode os.FileMode) error {
return nil
},
MockDir: func(s string) string {
return "."
},
})
return cfg, nil
}
}
func TestRunSetContext(t *testing.T) {
t.Run("testAddContext", func(t *testing.T) {
conf := testutil.DummyConfig()
dummyContextOptions := testutil.DummyContextOptions()
dummyContextOptions.Name = "second_context"
ioBuffer := bytes.NewBuffer(nil)
tests := []struct {
name string
options config.RunSetContextOptions
ctxopts []config.ContextOption
expectedOut string
err error
}{
{
name: "create new context",
options: config.RunSetContextOptions{
CfgFactory: prepareConfig(),
CtxName: "new_context",
Current: false,
Writer: ioBuffer,
},
ctxopts: []config.ContextOption{config.SetContextManifest("dummy_manifest"),
config.SetContextManagementConfig("dummy_management_config")},
err: nil,
expectedOut: "context with name new_context created\n",
},
{
name: "modify current context",
options: config.RunSetContextOptions{
CfgFactory: prepareConfig(),
CtxName: "",
Current: true,
Writer: ioBuffer,
},
ctxopts: []config.ContextOption{config.SetContextManagementConfig("")},
err: nil,
expectedOut: "context with name dummy_context modified\n",
},
{
name: "bad config",
options: config.RunSetContextOptions{
CfgFactory: func() (*config.Config, error) {
return nil, config.ErrMissingConfig{What: "bad config"}
},
},
err: config.ErrMissingConfig{What: "bad config"},
},
{
name: "no context name provided",
options: config.RunSetContextOptions{
CfgFactory: prepareConfig(),
CtxName: "",
Current: false,
},
err: config.ErrEmptyContextName{},
},
{
name: "setup invalid manifest",
options: config.RunSetContextOptions{
CfgFactory: prepareConfig(),
CtxName: "",
Current: true,
Writer: ioBuffer,
},
ctxopts: []config.ContextOption{config.SetContextManifest("invalid_manifest")},
err: config.ErrMissingConfig{What: "Current Context (dummy_context) does not identify a defined Manifest"},
},
}
modified, err := config.RunSetContext(dummyContextOptions, conf, false)
assert.NoError(t, err)
assert.False(t, modified)
assert.Contains(t, conf.Contexts, "second_context")
})
t.Run("testModifyContext", func(t *testing.T) {
conf := testutil.DummyConfig()
dummyContextOptions := testutil.DummyContextOptions()
modified, err := config.RunSetContext(dummyContextOptions, conf, false)
assert.NoError(t, err)
assert.True(t, modified)
for _, tt := range tests {
tt := tt
t.Run(tt.name, func(t *testing.T) {
ioBuffer.Reset()
err := tt.options.RunSetContext(tt.ctxopts...)
if tt.err != nil {
require.Error(t, err)
require.Equal(t, tt.err, err)
} else {
require.NoError(t, err)
}
if tt.expectedOut != "" {
require.Equal(t, tt.expectedOut, ioBuffer.String())
}
})
}
}
func TestRunUseContext(t *testing.T) {

7
pkg/config/config_test.go
View File

@ -257,7 +257,8 @@ func TestAddContext(t *testing.T) {
defer cleanup(t)
co := testutil.DummyContextOptions()
context := conf.AddContext(co)
context := conf.AddContext(co.Name, config.SetContextManifest(co.Manifest),
config.SetContextManagementConfig(co.ManagementConfiguration))
assert.EqualValues(t, conf.Contexts[co.Name], context)
}
@ -266,10 +267,10 @@ func TestModifyContext(t *testing.T) {
defer cleanup(t)
co := testutil.DummyContextOptions()
context := conf.AddContext(co)
context := conf.AddContext(co.Name, config.SetContextManifest(co.Manifest))
co.Manifest += stringDelta
conf.ModifyContext(context, co)
conf.ModifyContext(context, config.SetContextManifest(co.Manifest))
assert.EqualValues(t, conf.Contexts[co.Name].Manifest, co.Manifest)
assert.EqualValues(t, conf.Contexts[co.Name], context)
}