Skip to content

Commit

Permalink
Bug 1813949: ignore local env variables when we create a service client
Browse files Browse the repository at this point in the history
This commit explicitly disables 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.

After this change we don't have to unset OS_CLOUD env variable explicitly
anymore.

Ref https://issues.redhat.com/browse/OSASINFRA-2152
  • Loading branch information
Fedosin committed Jan 11, 2021
1 parent 69f0bbc commit 05453ef
Show file tree
Hide file tree
Showing 13 changed files with 48 additions and 92 deletions.
6 changes: 0 additions & 6 deletions pkg/asset/installconfig/openstack/openstack.go
Expand Up @@ -2,7 +2,6 @@
package openstack

import (
"os"
"sort"
"strings"

Expand Down Expand Up @@ -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
Expand Down
17 changes: 3 additions & 14 deletions pkg/asset/installconfig/openstack/session.go
Expand Up @@ -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"
Expand All @@ -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 {
Expand All @@ -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) {
Expand Down
9 changes: 2 additions & 7 deletions pkg/asset/installconfig/openstack/validation/cloudinfo.go
Expand Up @@ -2,7 +2,6 @@ package validation

import (
"net/url"
"os"
"strings"

"github.com/gophercloud/gophercloud"
Expand All @@ -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
Expand Down Expand Up @@ -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 {
Expand Down
14 changes: 5 additions & 9 deletions pkg/asset/installconfig/openstack/validvaluesfetcher.go
Expand Up @@ -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.
Expand All @@ -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
}
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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
}
Expand Down
12 changes: 4 additions & 8 deletions pkg/asset/machines/openstack/machines.go
Expand Up @@ -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 (
Expand All @@ -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
}
Expand Down Expand Up @@ -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
}
Expand Down
5 changes: 2 additions & 3 deletions pkg/asset/machines/openstack/machinesets.go
Expand Up @@ -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"
Expand All @@ -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)
}
Expand All @@ -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
}
Expand Down
3 changes: 1 addition & 2 deletions pkg/asset/machines/worker.go
Expand Up @@ -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"
Expand Down Expand Up @@ -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")
}
Expand Down
8 changes: 3 additions & 5 deletions pkg/destroy/openstack/glance.go
Expand Up @@ -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"
)

Expand All @@ -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
Expand Down
11 changes: 2 additions & 9 deletions 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"
Expand Down Expand Up @@ -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 {
Expand Down
12 changes: 3 additions & 9 deletions pkg/tfvars/openstack/bootstrap_ignition.go
Expand Up @@ -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
}
Expand Down Expand Up @@ -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
}
Expand Down
13 changes: 3 additions & 10 deletions pkg/tfvars/openstack/openstack.go
Expand Up @@ -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"
Expand Down Expand Up @@ -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
}
Expand All @@ -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
}
Expand Down
14 changes: 4 additions & 10 deletions pkg/tfvars/openstack/rhcos_image.go
Expand Up @@ -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
Expand All @@ -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
}
Expand Down Expand Up @@ -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
}
Expand Down
16 changes: 16 additions & 0 deletions 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
}

0 comments on commit 05453ef

Please sign in to comment.