Skip to content

Commit

Permalink
Define default values when exist
Browse files Browse the repository at this point in the history
  • Loading branch information
feloy committed Nov 17, 2022
1 parent 44c78c4 commit 51cbfaf
Show file tree
Hide file tree
Showing 9 changed files with 96 additions and 30 deletions.
20 changes: 10 additions & 10 deletions pkg/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,16 +7,16 @@ import (
)

type Configuration struct {
DevfileProxy *string `env:"DEVFILE_PROXY"`
DockerCmd *string `env:"DOCKER_CMD"`
Globalodoconfig *string `env:"GLOBALODOCONFIG"`
OdoDebugTelemetryFile *string `env:"ODO_DEBUG_TELEMETRY_FILE"`
OdoDisableTelemetry *bool `env:"ODO_DISABLE_TELEMETRY"`
OdoLogLevel *int `env:"ODO_LOG_LEVEL"`
OdoTrackingConsent *string `env:"ODO_TRACKING_CONSENT"`
PodmanCmd *string `env:"PODMAN_CMD"`
TelemetryCaller *string `env:"TELEMETRY_CALLER"`
OdoExperimentalMode *bool `env:"ODO_EXPERIMENTAL_MODE"`
DevfileProxy *string `env:"DEVFILE_PROXY,noinit"`
DockerCmd string `env:"DOCKER_CMD,default=docker"`
Globalodoconfig *string `env:"GLOBALODOCONFIG,noinit"`
OdoDebugTelemetryFile *string `env:"ODO_DEBUG_TELEMETRY_FILE,noinit"`
OdoDisableTelemetry *bool `env:"ODO_DISABLE_TELEMETRY,noinit"`
OdoLogLevel *int `env:"ODO_LOG_LEVEL,noinit"`
OdoTrackingConsent *string `env:"ODO_TRACKING_CONSENT,noinit"`
PodmanCmd string `env:"PODMAN_CMD,default=podman"`
TelemetryCaller string `env:"TELEMETRY_CALLER,default="`
OdoExperimentalMode bool `env:"ODO_EXPERIMENTAL_MODE,default=false"`
}

func GetConfiguration() (*Configuration, error) {
Expand Down
54 changes: 54 additions & 0 deletions pkg/config/config_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
package config

import (
"testing"
)

func TestDefaultValues(t *testing.T) {
cfg, err := GetConfiguration()
if err != nil {
t.Errorf("Error is not expected: %v", err)
}

checkDefaultStringValue(t, "DockerCmd", cfg.DockerCmd, "docker")
checkDefaultStringValue(t, "PodmanCmd", cfg.PodmanCmd, "podman")
checkDefaultStringValue(t, "TelemetryCaller", cfg.TelemetryCaller, "")
checkDefaultBoolValue(t, "OdoExperimentalMode", cfg.OdoExperimentalMode, false)

// Use noinit to set non initialized value as nil instead of zero-value
checkNilString(t, "DevfileProxy", cfg.DevfileProxy)
checkNilString(t, "Globalodoconfig", cfg.Globalodoconfig)
checkNilString(t, "Globalodoconfig", cfg.Globalodoconfig)
checkNilString(t, "OdoDebugTelemetryFile", cfg.OdoDebugTelemetryFile)
checkNilBool(t, "OdoDisableTelemetry", cfg.OdoDisableTelemetry)
checkNilString(t, "OdoTrackingConsent", cfg.OdoTrackingConsent)

}

func checkDefaultStringValue(t *testing.T, fieldName string, field string, def string) {
if field != def {
t.Errorf("default value for %q should be %q but is %q", fieldName, def, field)
}

}

func checkDefaultBoolValue(t *testing.T, fieldName string, field bool, def bool) {
if field != def {
t.Errorf("default value for %q should be %v but is %v", fieldName, def, field)
}

}

func checkNilString(t *testing.T, fieldName string, field *string) {
if field != nil {
t.Errorf("value for non specified env var %q should be nil but is %q", fieldName, *field)

}
}

func checkNilBool(t *testing.T, fieldName string, field *bool) {
if field != nil {
t.Errorf("value for non specified env var %q should be nil but is %v", fieldName, *field)

}
}
6 changes: 2 additions & 4 deletions pkg/devfile/image/image.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,6 @@ import (
"github.com/redhat-developer/odo/pkg/log"
odocontext "github.com/redhat-developer/odo/pkg/odo/context"
"github.com/redhat-developer/odo/pkg/testingutil/filesystem"

"k8s.io/utils/pointer"
)

// Backend is in interface that must be implemented by container runtimes
Expand Down Expand Up @@ -104,7 +102,7 @@ func buildPushImage(backend Backend, fs filesystem.Filesystem, image *devfile.Im
// or return an error if none are present locally
func selectBackend(ctx context.Context) (Backend, error) {

podmanCmd := pointer.StringDeref(envcontext.GetEnvConfig(ctx).PodmanCmd, "podman")
podmanCmd := envcontext.GetEnvConfig(ctx).PodmanCmd
if _, err := lookPathCmd(podmanCmd); err == nil {

// Podman does NOT build x86 images on Apple Silicon / M1 and we must *WARN* the user that this will not work.
Expand All @@ -124,7 +122,7 @@ func selectBackend(ctx context.Context) (Backend, error) {
return NewDockerCompatibleBackend(podmanCmd), nil
}

dockerCmd := pointer.StringDeref(envcontext.GetEnvConfig(ctx).DockerCmd, "docker")
dockerCmd := envcontext.GetEnvConfig(ctx).DockerCmd
if _, err := lookPathCmd(dockerCmd); err == nil {
return NewDockerCompatibleBackend(dockerCmd), nil
}
Expand Down
23 changes: 20 additions & 3 deletions pkg/devfile/image/image_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import (

devfile "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2"
gomock "github.com/golang/mock/gomock"
"k8s.io/utils/pointer"

"github.com/redhat-developer/odo/pkg/config"
envcontext "github.com/redhat-developer/odo/pkg/config/context"
Expand Down Expand Up @@ -130,6 +129,10 @@ func TestSelectBackend(t *testing.T) {
}{
{
name: "all backends are present",
envConfig: config.Configuration{
DockerCmd: "docker",
PodmanCmd: "podman",
},
lookPathCmd: func(string) (string, error) {
return "", nil
},
Expand All @@ -138,13 +141,21 @@ func TestSelectBackend(t *testing.T) {
},
{
name: "no backend are present",
envConfig: config.Configuration{
DockerCmd: "docker",
PodmanCmd: "podman",
},
lookPathCmd: func(string) (string, error) {
return "", errors.New("")
},
wantErr: true,
},
{
name: "only docker is present",
envConfig: config.Configuration{
DockerCmd: "docker",
PodmanCmd: "podman",
},
lookPathCmd: func(name string) (string, error) {
if name == "docker" {
return "docker", nil
Expand All @@ -156,6 +167,10 @@ func TestSelectBackend(t *testing.T) {
},
{
name: "only podman is present",
envConfig: config.Configuration{
DockerCmd: "docker",
PodmanCmd: "podman",
},
lookPathCmd: func(name string) (string, error) {
if name == "podman" {
return "podman", nil
Expand All @@ -168,7 +183,8 @@ func TestSelectBackend(t *testing.T) {
{
name: "value of PODMAN_CMD envvar is returned if it points to a valid command",
envConfig: config.Configuration{
PodmanCmd: pointer.String("my-alternate-podman-command"),
DockerCmd: "docker",
PodmanCmd: "my-alternate-podman-command",
},
lookPathCmd: func(name string) (string, error) {
if name == "my-alternate-podman-command" {
Expand All @@ -182,7 +198,8 @@ func TestSelectBackend(t *testing.T) {
{
name: "docker if PODMAN_CMD points to an invalid command",
envConfig: config.Configuration{
PodmanCmd: pointer.String("no-such-command"),
PodmanCmd: "no-such-command",
DockerCmd: "docker",
},
lookPathCmd: func(name string) (string, error) {
if name == "docker" {
Expand Down
3 changes: 1 addition & 2 deletions pkg/odo/cli/feature/experimental.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"context"

envcontext "github.com/redhat-developer/odo/pkg/config/context"
"k8s.io/utils/pointer"
)

const (
Expand All @@ -13,5 +12,5 @@ const (
)

func isExperimentalModeEnabled(ctx context.Context) bool {
return pointer.BoolDeref(envcontext.GetEnvConfig(ctx).OdoExperimentalMode, false)
return envcontext.GetEnvConfig(ctx).OdoExperimentalMode
}
12 changes: 5 additions & 7 deletions pkg/odo/cli/feature/features_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,14 @@ import (

"github.com/redhat-developer/odo/pkg/config"
envcontext "github.com/redhat-developer/odo/pkg/config/context"

"k8s.io/utils/pointer"
)

func TestIsEnabled(t *testing.T) {
type args struct {
feature OdoFeature
}
type env struct {
experimentalMode *bool
experimentalMode bool
}
type testCase struct {
name string
Expand Down Expand Up @@ -44,15 +42,15 @@ func TestIsEnabled(t *testing.T) {
name: "non-experimental feature should always be enabled regardless of experimental mode",
args: args{feature: nonExperimentalFeature},
env: env{
experimentalMode: pointer.Bool(true),
experimentalMode: true,
},
want: true,
},
{
name: "non-experimental feature should always be enabled even if experimental mode is not enabled",
args: args{feature: nonExperimentalFeature},
env: env{
experimentalMode: pointer.Bool(false),
experimentalMode: false,
},
want: true,
},
Expand All @@ -65,15 +63,15 @@ func TestIsEnabled(t *testing.T) {
name: "experimental feature should be disabled if experimental mode has an unknown value",
args: args{feature: experimentalFeature},
env: env{
experimentalMode: pointer.Bool(false),
experimentalMode: false,
},
want: false,
},
{
name: "experimental feature should be enabled only if experimental mode is enabled",
args: args{feature: experimentalFeature},
env: env{
experimentalMode: pointer.Bool(true),
experimentalMode: true,
},
want: true,
},
Expand Down
3 changes: 1 addition & 2 deletions pkg/odo/commonflags/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,13 @@ import (

"github.com/spf13/pflag"
"k8s.io/klog"
"k8s.io/utils/pointer"
)

func TestMain(m *testing.M) {
// --run-on is considered experimental for now. As such, to exist, it requires the ODO_EXPERIMENTAL_MODE env var to be set.
ctx := context.Background()
cfg := config.Configuration{
OdoExperimentalMode: pointer.Bool(true),
OdoExperimentalMode: true,
}
ctx = envcontext.WithEnvConfig(ctx, cfg)
klog.InitFlags(nil)
Expand Down
3 changes: 2 additions & 1 deletion pkg/odo/genericclioptions/runnable.go
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,8 @@ func GenericRun(o Runnable, cmd *cobra.Command, args []string) {
klog.V(4).Infof("WARNING: debug telemetry, if enabled, will be logged in %s", debugTelemetry)
}

err = scontext.SetCaller(cmd.Context(), pointer.StringDeref(envConfig.TelemetryCaller, ""))
// We can dereference as there is a default value defined for this config field
err = scontext.SetCaller(cmd.Context(), envConfig.TelemetryCaller)
if err != nil {
klog.V(3).Infof("error handling caller property for telemetry: %v", err)
}
Expand Down
2 changes: 1 addition & 1 deletion tests/integration/generic_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ var _ = Describe("odo generic", func() {
})

Context("experimental mode has an unknown value", func() {
for _, val := range []string{"", "false", "foobar"} {
for _, val := range []string{"", "false"} {
val := val
It("should not list experimental flags if ODO_EXPERIMENTAL is not true", func() {
helpOutput := helper.Cmd("odo", "help").AddEnv(feature.OdoExperimentalModeEnvVar + "=" + val).ShouldPass().Out()
Expand Down

0 comments on commit 51cbfaf

Please sign in to comment.