Skip to content

Commit

Permalink
Merge pull request #2583 from openshift-cherrypick-robot/cherry-pick-…
Browse files Browse the repository at this point in the history
…2581-to-release_1.2.47

[release_1.2.47] OCM-12119 | fix: Env var regression with create/network
  • Loading branch information
hunterkepley authored Oct 28, 2024
2 parents 250113f + 005e6da commit bee4f53
Show file tree
Hide file tree
Showing 4 changed files with 67 additions and 7 deletions.
3 changes: 2 additions & 1 deletion cmd/create/network/cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,8 @@ func NetworkRunner(userOptions *opts.NetworkUserOptions) rosa.CommandRunner {
var err error
templateCommand := "rosa-quickstart-default-vpc"
options := NewNetworkOptions()
options.args = userOptions
userOptions.CleanTemplateDir()
options.Bind(userOptions)

defer r.Cleanup()

Expand Down
4 changes: 4 additions & 0 deletions cmd/create/network/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@ type Options struct {
args *opts.NetworkUserOptions
}

func (o *Options) Bind(userOptions *opts.NetworkUserOptions) {
o.args = userOptions
}

func NewNetworkUserOptions() *opts.NetworkUserOptions {
return &opts.NetworkUserOptions{
Params: []string{},
Expand Down
30 changes: 24 additions & 6 deletions pkg/options/network/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import (
"github.com/spf13/cobra"

"github.com/openshift/rosa/pkg/constants"
"github.com/openshift/rosa/pkg/helper"
"github.com/openshift/rosa/pkg/reporter"
)

Expand Down Expand Up @@ -38,8 +37,11 @@ func NewNetworkUserOptions() *NetworkUserOptions {
options := &NetworkUserOptions{}

// Set template directory from environment variable or use default
templateDir := os.Getenv(constants.OcmTemplateDir)
if helper.HandleEscapedEmptyString(templateDir) != "" {
templateDir, isSet := os.LookupEnv(constants.OcmTemplateDir)
if isSet {
if templateDir == "\"\"" {
templateDir = ""
}
options.TemplateDir = templateDir
} else {
options.TemplateDir = defaultTemplateDir
Expand All @@ -48,6 +50,13 @@ func NewNetworkUserOptions() *NetworkUserOptions {
return options
}

func (n *NetworkUserOptions) CleanTemplateDir() {
// Clean up trailing '/' to work with filepath logic later
if len(n.TemplateDir) > 0 && n.TemplateDir[len(n.TemplateDir)-1] == '/' {
n.TemplateDir = n.TemplateDir[:len(n.TemplateDir)-1]
}
}

func NewNetworkOptions() *NetworkOptions {
return &NetworkOptions{
reporter: reporter.CreateReporter(),
Expand All @@ -72,9 +81,18 @@ func BuildNetworkCommandWithOptions() (*cobra.Command, *NetworkUserOptions) {
}

flags := cmd.Flags()
flags.StringVar(&options.TemplateDir, "template-dir", defaultTemplateDir, "Use a specific template directory,"+
" overriding the OCM_TEMPLATE_DIR environment variable.")
flags.StringArrayVar(&options.Params, "param", []string{}, "List of parameters")
flags.StringVar(
&options.TemplateDir,
"template-dir",
options.TemplateDir,
"Use a specific template directory, overriding the OCM_TEMPLATE_DIR environment variable.",
)
flags.StringArrayVar(
&options.Params,
"param",
[]string{},
"List of parameters",
)

return cmd, options
}
37 changes: 37 additions & 0 deletions pkg/options/network/create_test.go
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
package network

import (
"os"

. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
"github.com/spf13/cobra"

"github.com/openshift/rosa/pkg/constants"
"github.com/openshift/rosa/pkg/reporter"
)

Expand Down Expand Up @@ -32,6 +35,40 @@ var _ = Describe("BuildMachinePoolCreateCommandWithOptions", func() {
})
})

Context("NewNetworkUserOptions", func() {
It("should create default network options with env var", func() {
testDir := "test/1"
Expect(os.Setenv(constants.OcmTemplateDir, testDir)).ToNot(HaveOccurred())
options := NewNetworkUserOptions()
options.CleanTemplateDir()
Expect(options.TemplateDir).To(Equal(testDir))
})
It("should create default network options with env var which has trailing '/' and remove it", func() {
testDir := "/test/1/"
finalTestDir := "/test/1"
Expect(os.Setenv(constants.OcmTemplateDir, testDir)).ToNot(HaveOccurred())
options := NewNetworkUserOptions()
options.CleanTemplateDir()
Expect(options.TemplateDir).To(Equal(finalTestDir))
})
It("should create default network options with env var which has trailing '/' and remove it", func() {
testDir := "/"
finalTestDir := ""
Expect(os.Setenv(constants.OcmTemplateDir, testDir)).ToNot(HaveOccurred())
options := NewNetworkUserOptions()
options.CleanTemplateDir()
Expect(options.TemplateDir).To(Equal(finalTestDir))
})
It("should create default network options with empty dir and not fail", func() {
testDir := ""
finalTestDir := ""
Expect(os.Setenv(constants.OcmTemplateDir, testDir)).ToNot(HaveOccurred())
options := NewNetworkUserOptions()
options.CleanTemplateDir()
Expect(options.TemplateDir).To(Equal(finalTestDir))
})
})

Context("NetworkOptions.Network", func() {
It("should return the args field", func() {
options := NewNetworkOptions()
Expand Down

0 comments on commit bee4f53

Please sign in to comment.