Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OpenStack: add clusterOSImage validations #3964

Merged
merged 2 commits into from Oct 15, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
43 changes: 41 additions & 2 deletions pkg/asset/installconfig/openstack/validation/cloudinfo.go
@@ -1,17 +1,20 @@
package validation

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

"github.com/gophercloud/gophercloud"
"github.com/gophercloud/gophercloud/openstack/compute/v2/extensions/availabilityzones"
"github.com/gophercloud/gophercloud/openstack/compute/v2/flavors"
"github.com/gophercloud/gophercloud/openstack/imageservice/v2/images"
"github.com/gophercloud/gophercloud/openstack/networking/v2/extensions/layer3/floatingips"
"github.com/gophercloud/gophercloud/openstack/networking/v2/networks"
"github.com/gophercloud/gophercloud/openstack/networking/v2/subnets"
"github.com/gophercloud/utils/openstack/clientconfig"
flavorutils "github.com/gophercloud/utils/openstack/compute/v2/flavors"
imageutils "github.com/gophercloud/utils/openstack/imageservice/v2/images"
networkutils "github.com/gophercloud/utils/openstack/networking/v2/networks"
"github.com/pkg/errors"

Expand All @@ -20,11 +23,12 @@ import (

// CloudInfo caches data fetched from the user's openstack cloud
type CloudInfo struct {
APIFIP *floatingips.FloatingIP
ExternalNetwork *networks.Network
Flavors map[string]Flavor
MachinesSubnet *subnets.Subnet
APIFIP *floatingips.FloatingIP
IngressFIP *floatingips.FloatingIP
MachinesSubnet *subnets.Subnet
OSImage *images.Image
Zones []string

clients *clients
Expand All @@ -33,6 +37,7 @@ type CloudInfo struct {
type clients struct {
networkClient *gophercloud.ServiceClient
computeClient *gophercloud.ServiceClient
imageClient *gophercloud.ServiceClient
}

// Flavor embeds information from the Gophercloud Flavor struct and adds
Expand Down Expand Up @@ -67,6 +72,11 @@ func GetCloudInfo(ic *types.InstallConfig) (*CloudInfo, error) {
return nil, errors.Wrap(err, "failed to create a compute client")
}

ci.clients.imageClient, err = clientconfig.NewServiceClient("image", opts)
if err != nil {
return nil, errors.Wrap(err, "failed to create an image client")
}

err = ci.collectInfo(ic)
if err != nil {
return nil, errors.Wrap(err, "failed to generate OpenStack cloud info")
Expand All @@ -88,6 +98,17 @@ func (ci *CloudInfo) collectInfo(ic *types.InstallConfig) error {
return errors.Wrap(err, "failed to fetch platform flavor info")
}

// Fetch the image info if the user provided a Glance image name
imagePtr := ic.OpenStack.ClusterOSImage
if imagePtr != "" {
if _, err := url.ParseRequestURI(imagePtr); err != nil {
ci.OSImage, err = ci.getImage(imagePtr)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this err here is not handled

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh... my bad :( thanks for noticing!

if err != nil {
return err
}
}
}

if ic.ControlPlane != nil && ic.ControlPlane.Platform.OpenStack != nil {
crtlPlaneFlavor := ic.ControlPlane.Platform.OpenStack.FlavorName
if crtlPlaneFlavor != "" {
Expand Down Expand Up @@ -228,6 +249,24 @@ func (ci *CloudInfo) getFloatingIP(fip string) (*floatingips.FloatingIP, error)
return nil, nil
}

func (ci *CloudInfo) getImage(imageName string) (*images.Image, error) {
imageID, err := imageutils.IDFromName(ci.clients.imageClient, imageName)
if err != nil {
var gerr *gophercloud.ErrResourceNotFound
if errors.As(err, &gerr) {
return nil, nil
}
return nil, err
}

image, err := images.Get(ci.clients.imageClient, imageID).Extract()
if err != nil {
return nil, err
}

return image, nil
}

func (ci *CloudInfo) getZones() ([]string, error) {
zones := []string{}
allPages, err := availabilityzones.List(ci.clients.computeClient).AllPages()
Expand Down
35 changes: 35 additions & 0 deletions pkg/asset/installconfig/openstack/validation/platform.go
Expand Up @@ -3,9 +3,11 @@ package validation
import (
"errors"
"fmt"
"net/url"

"k8s.io/apimachinery/pkg/util/validation/field"

"github.com/gophercloud/gophercloud/openstack/imageservice/v2/images"
"github.com/openshift/installer/pkg/types"
"github.com/openshift/installer/pkg/types/openstack"
)
Expand All @@ -27,6 +29,9 @@ func ValidatePlatform(p *openstack.Platform, n *types.Networking, ci *CloudInfo)
// validate floating ips
allErrs = append(allErrs, validateFloatingIPs(p, ci, fldPath)...)

// validate custom cluster os image
allErrs = append(allErrs, validateClusterOSImage(p, ci, fldPath)...)

if p.DefaultMachinePlatform != nil {
allErrs = append(allErrs, ValidateMachinePool(p.DefaultMachinePlatform, ci, true, fldPath.Child("defaultMachinePlatform"))...)
}
Expand Down Expand Up @@ -127,3 +132,33 @@ func validateFloatingIPs(p *openstack.Platform, ci *CloudInfo, fldPath *field.Pa
}
return allErrs
}

// validateExternalNetwork validates the user's input for the clusterOSImage and returns a list of all validation errors
func validateClusterOSImage(p *openstack.Platform, ci *CloudInfo, fldPath *field.Path) (allErrs field.ErrorList) {
if p.ClusterOSImage == "" {
return
}

// For URLs we support only 'http(s)' and 'file' schemes
if uri, err := url.ParseRequestURI(p.ClusterOSImage); err == nil {
switch uri.Scheme {
case "http", "https", "file":
default:
allErrs = append(allErrs, field.Invalid(fldPath.Child("clusterOSImage"), p.ClusterOSImage, fmt.Sprintf("URL scheme should be either http(s) or file but it is '%v'", uri.Scheme)))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Go has fmt directive %q which will quote the string

}
return
}

// Image should exist in OpenStack Glance
if ci.OSImage == nil {
allErrs = append(allErrs, field.NotFound(fldPath.Child("clusterOSImage"), p.ClusterOSImage))
return allErrs
}

// Image should have "active" status
if ci.OSImage.Status != images.ImageStatusActive {
allErrs = append(allErrs, field.Invalid(fldPath.Child("clusterOSImage"), p.ClusterOSImage, fmt.Sprintf("OS image must be active but its status is '%s'", ci.OSImage.Status)))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Go has fmt directive %q which will quote the string

}

return allErrs
}
110 changes: 110 additions & 0 deletions pkg/asset/installconfig/openstack/validation/platform_test.go
Expand Up @@ -4,6 +4,7 @@ import (
"testing"

"github.com/gophercloud/gophercloud/openstack/compute/v2/flavors"
"github.com/gophercloud/gophercloud/openstack/imageservice/v2/images"
"github.com/gophercloud/gophercloud/openstack/networking/v2/extensions/layer3/floatingips"
"github.com/gophercloud/gophercloud/openstack/networking/v2/networks"
"github.com/openshift/installer/pkg/types"
Expand Down Expand Up @@ -249,3 +250,112 @@ func TestOpenStackPlatformValidation(t *testing.T) {
})
}
}

func TestClusterOSImage(t *testing.T) {
cases := []struct {
name string
platform *openstack.Platform
cloudInfo *CloudInfo
networking *types.Networking
expectedErrMsg string // NOTE: this is a REGEXP
}{
{
name: "no image provided",
platform: validPlatform(),
cloudInfo: validPlatformCloudInfo(),
networking: validNetworking(),
expectedErrMsg: "",
},
{
name: "HTTP address instead of the image name",
platform: func() *openstack.Platform {
p := validPlatform()
p.ClusterOSImage = "http://example.com/myrhcos.iso"
return p
}(),
cloudInfo: validPlatformCloudInfo(),
networking: validNetworking(),
expectedErrMsg: "",
},
{
name: "file location instead of the image name",
platform: func() *openstack.Platform {
p := validPlatform()
p.ClusterOSImage = "file:///home/user/myrhcos.iso"
return p
}(),
cloudInfo: validPlatformCloudInfo(),
networking: validNetworking(),
expectedErrMsg: "",
},
{
name: "valid image",
platform: func() *openstack.Platform {
p := validPlatform()
p.ClusterOSImage = "my-rhcos"
return p
}(),
cloudInfo: func() *CloudInfo {
ci := validPlatformCloudInfo()
ci.OSImage = &images.Image{
Name: "my-rhcos",
Status: images.ImageStatusActive,
}
return ci
}(),
networking: validNetworking(),
expectedErrMsg: "",
},
{
name: "image with invalid status",
platform: func() *openstack.Platform {
p := validPlatform()
p.ClusterOSImage = "my-rhcos"
return p
}(),
cloudInfo: func() *CloudInfo {
ci := validPlatformCloudInfo()
ci.OSImage = &images.Image{
Name: "my-rhcos",
Status: images.ImageStatusSaving,
}
return ci
}(),
networking: validNetworking(),
expectedErrMsg: "platform.openstack.clusterOSImage: Invalid value: \"my-rhcos\": OS image must be active but its status is 'saving'",
},
{
name: "image not found",
platform: func() *openstack.Platform {
p := validPlatform()
p.ClusterOSImage = "my-rhcos"
return p
}(),
cloudInfo: validPlatformCloudInfo(),
networking: validNetworking(),
expectedErrMsg: "platform.openstack.clusterOSImage: Not found: \"my-rhcos\"",
},
{
name: "Unsupported image URL scheme",
platform: func() *openstack.Platform {
p := validPlatform()
p.ClusterOSImage = "s3://mybucket/myrhcos.iso"
return p
}(),
cloudInfo: validPlatformCloudInfo(),
networking: validNetworking(),
expectedErrMsg: "platform.openstack.clusterOSImage: Invalid value: \"s3://mybucket/myrhcos.iso\": URL scheme should be either http\\(s\\) or file but it is 's3'",
},
}

for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
aggregatedErrors := ValidatePlatform(tc.platform, tc.networking, tc.cloudInfo).ToAggregate()
if tc.expectedErrMsg != "" {
assert.Regexp(t, tc.expectedErrMsg, aggregatedErrors)
} else {
assert.NoError(t, aggregatedErrors)
}
})
}
}

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions vendor/modules.txt
Expand Up @@ -664,6 +664,7 @@ github.com/gophercloud/utils/openstack/baremetal/v1/nodes
github.com/gophercloud/utils/openstack/clientconfig
github.com/gophercloud/utils/openstack/compute/v2/flavors
github.com/gophercloud/utils/openstack/compute/v2/images
github.com/gophercloud/utils/openstack/imageservice/v2/images
github.com/gophercloud/utils/openstack/networking/v2/networks
github.com/gophercloud/utils/terraform/auth
# github.com/h2non/filetype v1.0.12
Expand Down