From 7b83101d5f93d1b9f29bfe988ed6313681764d3e Mon Sep 17 00:00:00 2001 From: Rajat Chopra Date: Tue, 22 Jan 2019 20:24:51 -0500 Subject: [PATCH 1/2] pkg/asset/installconfig: Add a cloud credentials checker asset A cloud creds checker asset just checks upon the credentials. The 'cluster' target can then depend on it and, thus, cloud creds will be checked again before the cluster is finally created. The reason for a double check is that a cluster can be created through an imported install-config and the credentials may be missing then. If the cluster were to be created in one go (i.e. no install-config generation as a sub-step), then the asset creation graph ensures that they are checked upon only once. (Or rather asked about the creds only once.) --- pkg/asset/cluster/cluster.go | 4 ++ pkg/asset/installconfig/aws/aws.go | 6 +- pkg/asset/installconfig/aws/basedomain.go | 2 +- pkg/asset/installconfig/platformcredscheck.go | 55 +++++++++++++++++++ pkg/asset/machines/master.go | 4 ++ pkg/asset/machines/worker.go | 4 ++ 6 files changed, 72 insertions(+), 3 deletions(-) create mode 100644 pkg/asset/installconfig/platformcredscheck.go diff --git a/pkg/asset/cluster/cluster.go b/pkg/asset/cluster/cluster.go index 6ebf997a74b..d88d5d76ca9 100644 --- a/pkg/asset/cluster/cluster.go +++ b/pkg/asset/cluster/cluster.go @@ -38,6 +38,10 @@ func (c *Cluster) Dependencies() []asset.Asset { return []asset.Asset{ &installconfig.ClusterID{}, &installconfig.InstallConfig{}, + // PlatformCredsCheck just checks the creds (and asks, if needed) + // We do not actually use it in this asset directly, hence + // it is put in the dependencies but not fetched in Generate + &installconfig.PlatformCredsCheck{}, &TerraformVariables{}, &password.KubeadminPassword{}, } diff --git a/pkg/asset/installconfig/aws/aws.go b/pkg/asset/installconfig/aws/aws.go index 5036cb3dc2a..bc09a15d1be 100644 --- a/pkg/asset/installconfig/aws/aws.go +++ b/pkg/asset/installconfig/aws/aws.go @@ -38,7 +38,7 @@ func Platform() (*aws.Platform, error) { panic(fmt.Sprintf("installer bug: invalid default AWS region %q", defaultRegion)) } - ssn, err := getSession() + ssn, err := GetSession() if err != nil { return nil, err } @@ -85,7 +85,9 @@ func Platform() (*aws.Platform, error) { }, nil } -func getSession() (*session.Session, error) { +// GetSession returns an AWS session by checking credentials +// and, if no creds are found, asks for them and stores them on disk in a config file +func GetSession() (*session.Session, error) { ssn := session.Must(session.NewSessionWithOptions(session.Options{ SharedConfigState: session.SharedConfigEnable, })) diff --git a/pkg/asset/installconfig/aws/basedomain.go b/pkg/asset/installconfig/aws/basedomain.go index 848a2fd43ff..52fcb03e174 100644 --- a/pkg/asset/installconfig/aws/basedomain.go +++ b/pkg/asset/installconfig/aws/basedomain.go @@ -22,7 +22,7 @@ func IsForbidden(err error) bool { // GetBaseDomain returns a base domain chosen from among the account's // public routes. func GetBaseDomain() (string, error) { - session, err := getSession() + session, err := GetSession() if err != nil { return "", err } diff --git a/pkg/asset/installconfig/platformcredscheck.go b/pkg/asset/installconfig/platformcredscheck.go new file mode 100644 index 00000000000..671ae141974 --- /dev/null +++ b/pkg/asset/installconfig/platformcredscheck.go @@ -0,0 +1,55 @@ +package installconfig + +import ( + "fmt" + + "github.com/gophercloud/utils/openstack/clientconfig" + "github.com/openshift/installer/pkg/asset" + awsconfig "github.com/openshift/installer/pkg/asset/installconfig/aws" + "github.com/openshift/installer/pkg/types/aws" + "github.com/openshift/installer/pkg/types/libvirt" + "github.com/openshift/installer/pkg/types/none" + "github.com/openshift/installer/pkg/types/openstack" +) + +// PlatformCredsCheck is an asset that checks the platform credentials, asks for them or errors out if invalid +// the cluster. +type PlatformCredsCheck struct { +} + +var _ asset.Asset = (*PlatformCredsCheck)(nil) + +// Dependencies returns the dependencies for PlatformCredsCheck +func (a *PlatformCredsCheck) Dependencies() []asset.Asset { + return []asset.Asset{ + &InstallConfig{}, + } +} + +// Generate queries for input from the user. +func (a *PlatformCredsCheck) Generate(dependencies asset.Parents) error { + ic := &InstallConfig{} + dependencies.Get(ic) + + var err error + platform := ic.Config.Platform.Name() + switch platform { + case aws.Name: + _, err = awsconfig.GetSession() + case libvirt.Name: + case none.Name: + case openstack.Name: + opts := new(clientconfig.ClientOpts) + opts.Cloud = ic.Config.Platform.OpenStack.Cloud + _, err = clientconfig.GetCloudFromYAML(opts) + default: + err = fmt.Errorf("unknown platform type %q", platform) + } + + return err +} + +// Name returns the human-friendly name of the asset. +func (a *PlatformCredsCheck) Name() string { + return "Platform Credentials Check" +} diff --git a/pkg/asset/machines/master.go b/pkg/asset/machines/master.go index 953d7001a99..387476401cb 100644 --- a/pkg/asset/machines/master.go +++ b/pkg/asset/machines/master.go @@ -41,6 +41,10 @@ func (m *Master) Name() string { func (m *Master) Dependencies() []asset.Asset { return []asset.Asset{ &installconfig.ClusterID{}, + // PlatformCredsCheck just checks the creds (and asks, if needed) + // We do not actually use it in this asset directly, hence + // it is put in the dependencies but not fetched in Generate + &installconfig.PlatformCredsCheck{}, &installconfig.InstallConfig{}, new(rhcos.Image), &machine.Master{}, diff --git a/pkg/asset/machines/worker.go b/pkg/asset/machines/worker.go index 7b67d085989..d653a7d63d7 100644 --- a/pkg/asset/machines/worker.go +++ b/pkg/asset/machines/worker.go @@ -71,6 +71,10 @@ func (w *Worker) Name() string { func (w *Worker) Dependencies() []asset.Asset { return []asset.Asset{ &installconfig.ClusterID{}, + // PlatformCredsCheck just checks the creds (and asks, if needed) + // We do not actually use it in this asset directly, hence + // it is put in the dependencies but not fetched in Generate + &installconfig.PlatformCredsCheck{}, &installconfig.InstallConfig{}, new(rhcos.Image), &machine.Worker{}, From 727123c46f26f99a5b555993eaf0556d9a9fd284 Mon Sep 17 00:00:00 2001 From: Rajat Chopra Date: Fri, 25 Jan 2019 17:59:55 -0500 Subject: [PATCH 2/2] pkg/asset/machines/aws: Reuse GetSession function for ec2Client Reuse removes code duplication as well as ensuring there is one central way of creating the sessions. --- pkg/asset/machines/aws/zones.go | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/pkg/asset/machines/aws/zones.go b/pkg/asset/machines/aws/zones.go index e0100993641..50788f115f6 100644 --- a/pkg/asset/machines/aws/zones.go +++ b/pkg/asset/machines/aws/zones.go @@ -4,13 +4,16 @@ import ( "fmt" "github.com/aws/aws-sdk-go/aws" - "github.com/aws/aws-sdk-go/aws/session" "github.com/aws/aws-sdk-go/service/ec2" + awsutil "github.com/openshift/installer/pkg/asset/installconfig/aws" ) // AvailabilityZones retrieves a list of availability zones for the given region. func AvailabilityZones(region string) ([]string, error) { - ec2Client := ec2Client(region) + ec2Client, err := ec2Client(region) + if err != nil { + return nil, err + } zones, err := fetchAvailabilityZones(ec2Client, region) if err != nil { return nil, fmt.Errorf("cannot fetch availability zones: %v", err) @@ -18,14 +21,14 @@ func AvailabilityZones(region string) ([]string, error) { return zones, nil } -func ec2Client(region string) *ec2.EC2 { - ssn := session.Must(session.NewSessionWithOptions(session.Options{ - SharedConfigState: session.SharedConfigEnable, - Config: aws.Config{ - Region: aws.String(region), - }, - })) - return ec2.New(ssn) +func ec2Client(region string) (*ec2.EC2, error) { + ssn, err := awsutil.GetSession() + if err != nil { + return nil, err + } + + client := ec2.New(ssn, aws.NewConfig().WithRegion(region)) + return client, nil } func fetchAvailabilityZones(client *ec2.EC2, region string) ([]string, error) {