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

remove unused 'path' after loading config #13

Merged
merged 3 commits into from
Mar 21, 2024
Merged

remove unused 'path' after loading config #13

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d65a594
remove unused 'path' after loading config
xcoulon Mar 19, 2024
cce892e
Merge branch 'master' into drop_clusterconfig_path
xcoulon Mar 20, 2024
3741a10
Merge branch 'master' into drop_clusterconfig_path
xcoulon Mar 20, 2024
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
24 changes: 11 additions & 13 deletions pkg/configuration/configuration.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,34 +25,34 @@
}

// Load reads in config file and ENV variables if set.
func Load(term ioutils.Terminal) (SandboxUserConfig, string, error) {
func Load(term ioutils.Terminal) (SandboxUserConfig, error) {
path := ConfigFileFlag
if path == "" {
// Find home directory.
home, err := homedir.Dir()
if err != nil {
return SandboxUserConfig{}, "", errs.Wrap(err, "unable to read home directory")
return SandboxUserConfig{}, errs.Wrap(err, "unable to read home directory")

Check warning on line 34 in pkg/configuration/configuration.go

View check run for this annotation

Codecov / codecov/patch

pkg/configuration/configuration.go#L34 

Added line #L34 was not covered by tests
}
path = filepath.Join(home, ".ksctl.yaml")

if _, err := os.Stat(path); err != nil && os.IsNotExist(err) {
if _, err := os.Stat(filepath.Join(home, ".sandbox.yaml")); err != nil && !os.IsNotExist(err) {
return SandboxUserConfig{}, "", err
return SandboxUserConfig{}, err

Check warning on line 40 in pkg/configuration/configuration.go

View check run for this annotation

Codecov / codecov/patch

pkg/configuration/configuration.go#L40 

Added line #L40 was not covered by tests
} else if err == nil {
path = filepath.Join(home, ".sandbox.yaml")
term.Println("The default location of ~/.sandbox.yaml file is deprecated. Rename it to ~/.ksctl.yaml")
}
} else if err != nil {
return SandboxUserConfig{}, "", err
return SandboxUserConfig{}, err

Check warning on line 46 in pkg/configuration/configuration.go

View check run for this annotation

Codecov / codecov/patch

pkg/configuration/configuration.go#L46 

Added line #L46 was not covered by tests
}
}

info, err := os.Stat(path)
if err != nil {
return SandboxUserConfig{}, "", errs.Wrapf(err, "unable to read the file '%s'", path)
return SandboxUserConfig{}, errs.Wrapf(err, "unable to read the file '%s'", path)
}
if info.IsDir() {
return SandboxUserConfig{}, "", fmt.Errorf("the '%s' is not file but a directory", path)
return SandboxUserConfig{}, fmt.Errorf("the '%s' is not file but a directory", path)
}

if Verbose {
Expand All @@ -61,13 +61,13 @@

bytes, err := os.ReadFile(path)
if err != nil {
return SandboxUserConfig{}, "", err
return SandboxUserConfig{}, err

Check warning on line 64 in pkg/configuration/configuration.go

View check run for this annotation

Codecov / codecov/patch

pkg/configuration/configuration.go#L64 

Added line #L64 was not covered by tests
}
sandboxUserConfig := SandboxUserConfig{}
if err := yaml.Unmarshal(bytes, &sandboxUserConfig); err != nil {
return SandboxUserConfig{}, "", err
return SandboxUserConfig{}, err

Check warning on line 68 in pkg/configuration/configuration.go

View check run for this annotation

Codecov / codecov/patch

pkg/configuration/configuration.go#L68 

Added line #L68 was not covered by tests
}
return sandboxUserConfig, path, nil
return sandboxUserConfig, nil
}

const HostName = "host"
Expand Down Expand Up @@ -110,7 +110,7 @@

// LoadClusterAccessDefinition loads ClusterAccessDefinition object from the config file and checks that all required parameters are set
func LoadClusterAccessDefinition(term ioutils.Terminal, clusterName string) (ClusterAccessDefinition, error) {
sandboxUserConfig, _, err := Load(term)
sandboxUserConfig, err := Load(term)

Check warning on line 113 in pkg/configuration/configuration.go

View check run for this annotation

Codecov / codecov/patch

pkg/configuration/configuration.go#L113 

Added line #L113 was not covered by tests
if err != nil {
return ClusterAccessDefinition{}, err
}
Expand Down Expand Up @@ -156,13 +156,12 @@
ClusterName string
Token string
SandboxNamespace string
PathToConfigFile string
}

// LoadClusterConfig loads ClusterConfig object from the config file and checks that all required parameters are set
// as well as the token for the given name
func LoadClusterConfig(term ioutils.Terminal, clusterName string) (ClusterConfig, error) {
sandboxUserConfig, path, err := Load(term)
sandboxUserConfig, err := Load(term)
if err != nil {
return ClusterConfig{}, err
}
Expand Down Expand Up @@ -196,7 +195,6 @@
ClusterName: clusterName,
Token: clusterDef.Token,
SandboxNamespace: sandboxNamespace,
PathToConfigFile: path,
}, nil
}

Expand Down
43 changes: 4 additions & 39 deletions pkg/configuration/configuration_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,6 @@ func TestLoadClusterConfig(t *testing.T) {
assert.Equal(t, "cool-server.com", cfg.ServerName)
assert.Len(t, cfg.AllClusterNames, 1)
assert.Contains(t, cfg.AllClusterNames, utils.CamelCaseToKebabCase(clusterName))
assert.True(t, strings.HasPrefix(cfg.PathToConfigFile, os.TempDir()))
assert.Contains(t, term.Output(), fmt.Sprintf("Using config file: '%s'", configuration.ConfigFileFlag))
assert.Contains(t, term.Output(), fmt.Sprintf("Using '%s' configuration for '%s' cluster running at '%s' and in namespace '%s'",
cfg.ClusterName, cfg.ServerName, cfg.ServerAPI, cfg.SandboxNamespace))
Expand Down Expand Up @@ -238,46 +237,13 @@ func TestLoad(t *testing.T) {
configuration.Verbose = true

// when
sandboxUserConfig, path, err := configuration.Load(term)
sandboxUserConfig, err := configuration.Load(term)

// then
require.NoError(t, err)
expectedConfig := NewSandboxUserConfig(Host(), Member())
assert.Equal(t, expectedConfig, sandboxUserConfig)
assert.Contains(t, term.Output(), "Using config file")
assert.True(t, strings.HasPrefix(path, os.TempDir()))

t.Run("reload again the same", func(t *testing.T) {
// given
term := NewFakeTerminal()

// when
sandboxUserConfig, theSamePath, err := configuration.Load(term)

// then
require.NoError(t, err)
assert.Equal(t, expectedConfig, sandboxUserConfig)
assert.Contains(t, term.Output(), "Using config file")
assert.True(t, strings.HasPrefix(path, os.TempDir()))
assert.Equal(t, path, theSamePath)
})

t.Run("load a new one", func(t *testing.T) {
// given
term := NewFakeTerminal()
SetFileConfig(t, Host(), Member())

// when
sandboxUserConfig, newPath, err := configuration.Load(term)

// then
require.NoError(t, err)
expectedConfig := NewSandboxUserConfig(Host(), Member())
assert.Equal(t, expectedConfig, sandboxUserConfig)
assert.Contains(t, term.Output(), "Using config file")
assert.True(t, strings.HasPrefix(path, os.TempDir()))
assert.NotEqual(t, path, newPath)
})
})

t.Run("without verbose messages", func(t *testing.T) {
Expand All @@ -287,14 +253,13 @@ func TestLoad(t *testing.T) {
configuration.Verbose = false

// when
sandboxUserConfig, path, err := configuration.Load(term)
sandboxUserConfig, err := configuration.Load(term)

// then
require.NoError(t, err)
expectedConfig := NewSandboxUserConfig(Host(), Member())
assert.Equal(t, expectedConfig, sandboxUserConfig)
assert.NotContains(t, term.Output(), "Using config file")
assert.True(t, strings.HasPrefix(path, os.TempDir()))
})

}
Expand All @@ -306,7 +271,7 @@ func TestLoadFails(t *testing.T) {
configuration.ConfigFileFlag = "/tmp/should-not-exist.yaml"

// when
_, _, err := configuration.Load(term)
_, err := configuration.Load(term)

// then
require.Error(t, err)
Expand All @@ -318,7 +283,7 @@ func TestLoadFails(t *testing.T) {
configuration.ConfigFileFlag = os.TempDir()

// when
_, _, err := configuration.Load(term)
_, err := configuration.Load(term)

// then
require.Error(t, err)
Expand Down
Loading