Bug 1813949: ignore local env variables when we create a service client

pkg/asset/installconfig/openstack/openstack.go

@@ -2,7 +2,6 @@
 package openstack
 
 import (
-	"os"
 	"sort"
 	"strings"
 
@@ -46,11 +45,6 @@ func Platform() (*openstack.Platform, error) {
 		return nil, errors.Wrap(err, "failed UserInput")
 	}
 
-	// We should unset OS_CLOUD env variable here, because the real cloud name was defined
-	// on the previous step. OS_CLOUD has more priority, so the value from "cloud" variable
-	// will be ignored if OS_CLOUD contains something.
-	os.Unsetenv("OS_CLOUD")
-
 	networkNames, err := getExternalNetworkNames(cloud)
 	if err != nil {
 		return nil, err

pkg/asset/installconfig/openstack/session.go

@@ -2,9 +2,9 @@
 package openstack
 
 import (
-	"os"
 	"sync"
 
+	openstackdefaults "github.com/openshift/installer/pkg/types/openstack/defaults"
 	"github.com/pkg/errors"
 
 	"github.com/ghodss/yaml"
@@ -21,12 +21,8 @@ type Session struct {
 
 // GetSession returns an OpenStack session for a given cloud name in clouds.yaml.
 func GetSession(cloudName string) (*Session, error) {
-	opts := defaultClientOpts(cloudName)
-
-	// We should unset OS_CLOUD env variable here, because the real cloud name was
-	// defined on the previous step. OS_CLOUD has more priority, so the value from
-	// "opts" variable will be ignored if OS_CLOUD contains something.
-	os.Unsetenv("OS_CLOUD")
+	opts := openstackdefaults.DefaultClientOpts(cloudName)
+	opts.YAMLOpts = new(yamlLoadOpts)
 
 	cloudConfig, err := clientconfig.GetCloudFromYAML(opts)
 	if err != nil {
@@ -37,13 +33,6 @@ func GetSession(cloudName string) (*Session, error) {
 	}, nil
 }
 
-func defaultClientOpts(cloudName string) *clientconfig.ClientOpts {
-	opts := new(clientconfig.ClientOpts)
-	opts.Cloud = cloudName
-	opts.YAMLOpts = new(yamlLoadOpts)
-	return opts
-}
-
 type yamlLoadOpts struct{}
 
 func (opts yamlLoadOpts) LoadCloudsYAML() (map[string]clientconfig.Cloud, error) {

pkg/asset/installconfig/openstack/validation/cloudinfo.go

@@ -2,7 +2,6 @@ package validation
 
 import (
 	"net/url"
-	"os"
 	"strings"
 
 	"github.com/gophercloud/gophercloud"
@@ -24,6 +23,7 @@ import (
 
 	"github.com/openshift/installer/pkg/quota"
 	"github.com/openshift/installer/pkg/types"
+	openstackdefaults "github.com/openshift/installer/pkg/types/openstack/defaults"
 )
 
 // CloudInfo caches data fetched from the user's openstack cloud
@@ -68,12 +68,7 @@ func GetCloudInfo(ic *types.InstallConfig) (*CloudInfo, error) {
 		Flavors: map[string]Flavor{},
 	}
 
-	opts := &clientconfig.ClientOpts{Cloud: ic.OpenStack.Cloud}
-
-	// We should unset OS_CLOUD env variable here, because the real cloud name was
-	// defined on the previous step. OS_CLOUD has more priority, so the value from
-	// "opts" variable will be ignored if OS_CLOUD contains something.
-	os.Unsetenv("OS_CLOUD")
+	opts := openstackdefaults.DefaultClientOpts(ic.OpenStack.Cloud)
 
 	ci.clients.networkClient, err = clientconfig.NewServiceClient("network", opts)
 	if err != nil {

pkg/asset/installconfig/openstack/validvaluesfetcher.go

@@ -9,6 +9,8 @@ import (
 	"github.com/gophercloud/gophercloud/openstack/networking/v2/networks"
 	"github.com/gophercloud/utils/openstack/clientconfig"
 	networkutils "github.com/gophercloud/utils/openstack/networking/v2/networks"
+
+	openstackdefaults "github.com/openshift/installer/pkg/types/openstack/defaults"
 )
 
 // getCloudNames gets the valid cloud names. These are read from clouds.yaml.
@@ -28,9 +30,7 @@ func getCloudNames() ([]string, error) {
 // getExternalNetworkNames interrogates OpenStack to get the external network
 // names.
 func getExternalNetworkNames(cloud string) ([]string, error) {
-	conn, err := clientconfig.NewServiceClient("network", &clientconfig.ClientOpts{
-		Cloud: cloud,
-	})
+	conn, err := clientconfig.NewServiceClient("network", openstackdefaults.DefaultClientOpts(cloud))
 	if err != nil {
 		return nil, err
 	}
@@ -61,9 +61,7 @@ func getExternalNetworkNames(cloud string) ([]string, error) {
 
 // getFlavorNames gets a list of valid flavor names.
 func getFlavorNames(cloud string) ([]string, error) {
-	conn, err := clientconfig.NewServiceClient("compute", &clientconfig.ClientOpts{
-		Cloud: cloud,
-	})
+	conn, err := clientconfig.NewServiceClient("compute", openstackdefaults.DefaultClientOpts(cloud))
 	if err != nil {
 		return nil, err
 	}
@@ -91,9 +89,7 @@ func getFlavorNames(cloud string) ([]string, error) {
 	return flavorNames, nil
 }
 func getFloatingIPNames(cloud string, floatingNetworkName string) ([]string, error) {
-	conn, err := clientconfig.NewServiceClient("network", &clientconfig.ClientOpts{
-		Cloud: cloud,
-	})
+	conn, err := clientconfig.NewServiceClient("network", openstackdefaults.DefaultClientOpts(cloud))
 	if err != nil {
 		return nil, err
 	}

pkg/asset/machines/openstack/machines.go

@@ -15,6 +15,7 @@ import (
 
 	"github.com/openshift/installer/pkg/types"
 	"github.com/openshift/installer/pkg/types/openstack"
+	openstackdefaults "github.com/openshift/installer/pkg/types/openstack/defaults"
 )
 
 const (
@@ -40,7 +41,7 @@ func Machines(clusterID string, config *types.InstallConfig, pool *types.Machine
 
 	mpool := pool.Platform.OpenStack
 	platform := config.Platform.OpenStack
-	trunkSupport, err := checkNetworkExtensionAvailability(platform.Cloud, "trunk", nil)
+	trunkSupport, err := checkNetworkExtensionAvailability(platform.Cloud, "trunk")
 	if err != nil {
 		return nil, err
 	}
@@ -175,13 +176,8 @@ func generateProvider(clusterID string, platform *openstack.Platform, mpool *ope
 	return &spec, nil
 }
 
-func checkNetworkExtensionAvailability(cloud, alias string, opts *clientconfig.ClientOpts) (bool, error) {
-	if opts == nil {
-		opts = &clientconfig.ClientOpts{}
-	}
-	opts.Cloud = cloud
-
-	conn, err := clientconfig.NewServiceClient("network", opts)
+func checkNetworkExtensionAvailability(cloud, alias string) (bool, error) {
+	conn, err := clientconfig.NewServiceClient("network", openstackdefaults.DefaultClientOpts(cloud))
 	if err != nil {
 		return false, err
 	}

pkg/asset/machines/openstack/machinesets.go

@@ -4,7 +4,6 @@ package openstack
 import (
 	"fmt"
 
-	"github.com/gophercloud/utils/openstack/clientconfig"
 	clusterapi "github.com/openshift/machine-api-operator/pkg/apis/machine/v1beta1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
@@ -14,7 +13,7 @@ import (
 )
 
 // MachineSets returns a list of machinesets for a machinepool.
-func MachineSets(clusterID string, config *types.InstallConfig, pool *types.MachinePool, osImage, role, userDataSecret string, clientOpts *clientconfig.ClientOpts) ([]*clusterapi.MachineSet, error) {
+func MachineSets(clusterID string, config *types.InstallConfig, pool *types.MachinePool, osImage, role, userDataSecret string) ([]*clusterapi.MachineSet, error) {
 	if configPlatform := config.Platform.Name(); configPlatform != openstack.Name {
 		return nil, fmt.Errorf("non-OpenStack configuration: %q", configPlatform)
 	}
@@ -23,7 +22,7 @@ func MachineSets(clusterID string, config *types.InstallConfig, pool *types.Mach
 	}
 	platform := config.Platform.OpenStack
 	mpool := pool.Platform.OpenStack
-	trunkSupport, err := checkNetworkExtensionAvailability(platform.Cloud, "trunk", clientOpts)
+	trunkSupport, err := checkNetworkExtensionAvailability(platform.Cloud, "trunk")
 	if err != nil {
 		return nil, err
 	}

pkg/asset/machines/worker.go

@@ -8,7 +8,6 @@ import (
 	"strings"
 
 	"github.com/ghodss/yaml"
-	openstackclientconfig "github.com/gophercloud/utils/openstack/clientconfig"
 	baremetalapi "github.com/metal3-io/cluster-api-provider-baremetal/pkg/apis"
 	baremetalprovider "github.com/metal3-io/cluster-api-provider-baremetal/pkg/apis/baremetal/v1alpha1"
 	gcpapi "github.com/openshift/cluster-api-provider-gcp/pkg/apis"
@@ -364,7 +363,7 @@ func (w *Worker) Generate(dependencies asset.Parents) error {
 
 			imageName, _ := rhcosutils.GenerateOpenStackImageName(string(*rhcosImage), clusterID.InfraID)
 
-			sets, err := openstack.MachineSets(clusterID.InfraID, ic, &pool, imageName, "worker", "worker-user-data", &openstackclientconfig.ClientOpts{})
+			sets, err := openstack.MachineSets(clusterID.InfraID, ic, &pool, imageName, "worker", "worker-user-data")
 			if err != nil {
 				return errors.Wrap(err, "failed to create worker machine objects")
 			}

pkg/destroy/openstack/glance.go

@@ -8,6 +8,8 @@ import (
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
 
+	openstackdefaults "github.com/openshift/installer/pkg/types/openstack/defaults"
+
 	"k8s.io/apimachinery/pkg/util/wait"
 )
 
@@ -29,11 +31,7 @@ func DeleteGlanceImage(name string, cloud string) error {
 }
 
 func deleteGlanceImage(name string, cloud string) (bool, error) {
-	opts := clientconfig.ClientOpts{
-		Cloud: cloud,
-	}
-
-	conn, err := clientconfig.NewServiceClient("image", &opts)
+	conn, err := clientconfig.NewServiceClient("image", openstackdefaults.DefaultClientOpts(cloud))
 	if err != nil {
 		logrus.Warningf("There was an error during the image removal: %v", err)
 		return false, nil

pkg/destroy/openstack/openstack.go

@@ -1,12 +1,12 @@
 package openstack
 
 import (
-	"os"
 	"strings"
 	"time"
 
 	"github.com/openshift/installer/pkg/destroy/providers"
 	"github.com/openshift/installer/pkg/types"
+	openstackdefaults "github.com/openshift/installer/pkg/types/openstack/defaults"
 
 	"github.com/gophercloud/gophercloud"
 	"github.com/gophercloud/gophercloud/openstack/blockstorage/v3/volumes"
@@ -101,14 +101,7 @@ func (o *ClusterUninstaller) Run() error {
 	}
 	returnChannel := make(chan string)
 
-	opts := &clientconfig.ClientOpts{
-		Cloud: o.Cloud,
-	}
-
-	// We should unset OS_CLOUD env variable here, because the real cloud name was
-	// defined on the previous step. OS_CLOUD has more priority, so the value from
-	// "opts" variable will be ignored if OS_CLOUD contains something.
-	os.Unsetenv("OS_CLOUD")
+	opts := openstackdefaults.DefaultClientOpts(o.Cloud)
 
 	// launch goroutines
 	for name, function := range deleteFuncs {

pkg/tfvars/openstack/bootstrap_ignition.go

@@ -16,18 +16,16 @@ import (
 	"github.com/vincent-petithory/dataurl"
 
 	"github.com/openshift/installer/pkg/asset/ignition"
+	openstackdefaults "github.com/openshift/installer/pkg/types/openstack/defaults"
 )
 
 // Starting from OpenShift 4.4 we store bootstrap Ignition configs in Glance.
 
 // uploadBootstrapConfig uploads the bootstrap Ignition config in Glance and returns its location
 func uploadBootstrapConfig(cloud string, bootstrapIgn string, clusterID string) (string, error) {
 	logrus.Debugln("Creating a Glance image for your bootstrap ignition config...")
-	opts := clientconfig.ClientOpts{
-		Cloud: cloud,
-	}
 
-	conn, err := clientconfig.NewServiceClient("image", &opts)
+	conn, err := clientconfig.NewServiceClient("image", openstackdefaults.DefaultClientOpts(cloud))
 	if err != nil {
 		return "", err
 	}
@@ -179,11 +177,7 @@ func generateIgnitionShim(userCA string, clusterID string, bootstrapConfigURL st
 
 // getAuthToken fetches valid OpenStack authentication token ID
 func getAuthToken(cloud string) (string, error) {
-	opts := &clientconfig.ClientOpts{
-		Cloud: cloud,
-	}
-
-	conn, err := clientconfig.NewServiceClient("identity", opts)
+	conn, err := clientconfig.NewServiceClient("identity", openstackdefaults.DefaultClientOpts(cloud))
 	if err != nil {
 		return "", err
 	}

pkg/tfvars/openstack/openstack.go

@@ -15,6 +15,7 @@ import (
 	"github.com/openshift/installer/pkg/rhcos"
 	"github.com/openshift/installer/pkg/tfvars/internal/cache"
 	types_openstack "github.com/openshift/installer/pkg/types/openstack"
+	openstackdefaults "github.com/openshift/installer/pkg/types/openstack/defaults"
 	"github.com/pkg/errors"
 
 	"sigs.k8s.io/cluster-api-provider-openstack/pkg/apis/openstackproviderconfig/v1alpha1"
@@ -197,11 +198,7 @@ func getGlancePublicURL(serviceCatalog *tokens.ServiceCatalog) (string, error) {
 
 // getServiceCatalog fetches OpenStack service catalog with service endpoints
 func getServiceCatalog(cloud string) (*tokens.ServiceCatalog, error) {
-	opts := &clientconfig.ClientOpts{
-		Cloud: cloud,
-	}
-
-	conn, err := clientconfig.NewServiceClient("identity", opts)
+	conn, err := clientconfig.NewServiceClient("identity", openstackdefaults.DefaultClientOpts(cloud))
 	if err != nil {
 		return nil, err
 	}
@@ -222,11 +219,7 @@ func getServiceCatalog(cloud string) (*tokens.ServiceCatalog, error) {
 
 // getNetworkFromSubnet looks up a subnet in openstack and returns the ID of the network it's a part of
 func getNetworkFromSubnet(cloud string, subnetID string) (string, error) {
-	opts := &clientconfig.ClientOpts{
-		Cloud: cloud,
-	}
-
-	networkClient, err := clientconfig.NewServiceClient("network", opts)
+	networkClient, err := clientconfig.NewServiceClient("network", openstackdefaults.DefaultClientOpts(cloud))
 	if err != nil {
 		return "", err
 	}

pkg/tfvars/openstack/rhcos_image.go

@@ -13,6 +13,8 @@ import (
 	"github.com/gophercloud/utils/openstack/clientconfig"
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
+
+	openstackdefaults "github.com/openshift/installer/pkg/types/openstack/defaults"
 )
 
 // uploadBaseImage creates a new image in Glance and uploads the RHCOS image there
@@ -25,11 +27,7 @@ func uploadBaseImage(cloud string, localFilePath string, imageName string, clust
 	}
 	defer f.Close()
 
-	opts := clientconfig.ClientOpts{
-		Cloud: cloud,
-	}
-
-	conn, err := clientconfig.NewServiceClient("image", &opts)
+	conn, err := clientconfig.NewServiceClient("image", openstackdefaults.DefaultClientOpts(cloud))
 	if err != nil {
 		return err
 	}
@@ -125,11 +123,7 @@ func isImageImportSupported(cloud string) (bool, error) {
 	// https://docs.openstack.org/api-ref/image/v2/?expanded=#image-service-info-discovery
 	logrus.Debugln("Checking if the image import mechanism is supported")
 
-	opts := clientconfig.ClientOpts{
-		Cloud: cloud,
-	}
-
-	conn, err := clientconfig.NewServiceClient("image", &opts)
+	conn, err := clientconfig.NewServiceClient("image", openstackdefaults.DefaultClientOpts(cloud))
 	if err != nil {
 		return false, err
 	}

pkg/types/openstack/defaults/clientopts.go

@@ -0,0 +1,16 @@
+package defaults
+
+import (
+	"github.com/gophercloud/utils/openstack/clientconfig"
+)
+
+// DefaultClientOpts generates default client opts based on cloud name
+func DefaultClientOpts(cloudName string) *clientconfig.ClientOpts {
+	opts := new(clientconfig.ClientOpts)
+	opts.Cloud = cloudName
+	// We explicitly disable reading auth data from env variables by setting an invalid EnvPrefix.
+	// By doing this, we make sure that the data from clouds.yaml is enough to authenticate.
+	// For more information: https://github.com/gophercloud/utils/blob/8677e053dcf1f05d0fa0a616094aace04690eb94/openstack/clientconfig/requests.go#L508
+	opts.EnvPrefix = "NO_ENV_VARIABLES_"
+	return opts
+}