Merge pull request openshift#711 from staebler/improve_input_validation

asset/installconfig: improve validation of user inputs

CONTRIBUTING.md

@@ -44,6 +44,7 @@ For contributors who want to work up pull requests, the workflow is roughly:
     hack/tf-fmt.sh -list -check
     hack/tf-lint.sh
     hack/yaml-lint.sh
+    hack/go-test.sh
     ```
 7. Submit a pull request to the original repository.
 8. The [repo](OWNERS) [owners](OWNERS_ALIASES) will respond to your issue promptly, following [the ususal Prow workflow][prow-review].
@@ -86,6 +87,21 @@ second line is always blank, and other lines should be wrapped at 80 characters.
 This allows the message to be easier to read on GitHub as well as in various
 git tools.
 
+## Generating Test Mocks
+
+Some unit tests use mocks that are generated with gomock. If the underlying interface for a mock changes, then the mock will need to be regenerated. Use the following to regenerate all of the mocks under the pkg package.
+
+```
+go generate ./pkg/...
+```
+
+This requires gomock and mockgen. These can be installed by running the following.
+
+```
+go get -u github.com/golang/mock/gomock
+go get -u github.com/golang/mock/mockgen
+```
+
 [golang-style]: https://github.com/golang/go/wiki/CodeReviewComments
 [disclosure]: https://coreos.com/security/disclosure/
 [new-issue]: https://github.com/openshift/installer/issues/new

hack/go-test.sh

@@ -0,0 +1,13 @@
+#!/bin/sh
+# Example:  ./hack/go-test.sh
+
+if [ "$IS_CONTAINER" != "" ]; then
+  go test ./cmd/... ./data/... ./pkg/... "${@}"
+else
+  podman run --rm \
+    --env IS_CONTAINER=TRUE \
+    --volume "${PWD}:/go/src/github.com/openshift/installer:z" \
+    --workdir /go/src/github.com/openshift/installer \
+    docker.io/openshift/origin-release:golang-1.10 \
+    ./hack/go-test.sh "${@}"
+fi

pkg/asset/filefetcher.go

@@ -6,6 +6,8 @@ import (
 	"sort"
 )
 
+//go:generate mockgen -source=./filefetcher.go -destination=./mock/filefetcher_generated.go -package=mock
+
 // FileFetcher fetches the asset files from disk.
 type FileFetcher interface {
 	// FetchByName returns the file with the given name.

pkg/asset/ignition/machine/master_test.go

@@ -1,7 +1,6 @@
 package machine
 
 import (
-	"net"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -24,12 +23,7 @@ func TestMasterGenerate(t *testing.T) {
 			},
 			BaseDomain: "test-domain",
 			Networking: types.Networking{
-				ServiceCIDR: ipnet.IPNet{
-					IPNet: func(s string) net.IPNet {
-						_, cidr, _ := net.ParseCIDR(s)
-						return *cidr
-					}("10.0.1.0/24"),
-				},
+				ServiceCIDR: *ipnet.MustParseCIDR("10.0.1.0/24"),
 			},
 			Platform: types.Platform{
 				AWS: &aws.Platform{

pkg/asset/ignition/machine/worker_test.go

@@ -1,7 +1,6 @@
 package machine
 
 import (
-	"net"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -19,12 +18,7 @@ func TestWorkerGenerate(t *testing.T) {
 	installConfig := &installconfig.InstallConfig{
 		Config: &types.InstallConfig{
 			Networking: types.Networking{
-				ServiceCIDR: ipnet.IPNet{
-					IPNet: func(s string) net.IPNet {
-						_, cidr, _ := net.ParseCIDR(s)
-						return *cidr
-					}("10.0.1.0/24"),
-				},
+				ServiceCIDR: *ipnet.MustParseCIDR("10.0.1.0/24"),
 			},
 			Platform: types.Platform{
 				AWS: &aws.Platform{

pkg/asset/installconfig/aws/aws.go

@@ -16,41 +16,19 @@ import (
 	"github.com/sirupsen/logrus"
 	survey "gopkg.in/AlecAivazis/survey.v1"
 
+	"github.com/openshift/installer/pkg/ipnet"
 	"github.com/openshift/installer/pkg/types/aws"
 )
 
-const (
-	defaultVPCCIDR = "10.0.0.0/16"
-)
-
 var (
-	validAWSRegions = map[string]string{
-		"ap-northeast-1": "Tokyo",
-		"ap-northeast-2": "Seoul",
-		"ap-northeast-3": "Osaka-Local",
-		"ap-south-1":     "Mumbai",
-		"ap-southeast-1": "Singapore",
-		"ap-southeast-2": "Sydney",
-		"ca-central-1":   "Central",
-		"cn-north-1":     "Beijing",
-		"cn-northwest-1": "Ningxia",
-		"eu-central-1":   "Frankfurt",
-		"eu-west-1":      "Ireland",
-		"eu-west-2":      "London",
-		"eu-west-3":      "Paris",
-		"sa-east-1":      "São Paulo",
-		"us-east-1":      "N. Virginia",
-		"us-east-2":      "Ohio",
-		"us-west-1":      "N. California",
-		"us-west-2":      "Oregon",
-	}
+	defaultVPCCIDR = ipnet.MustParseCIDR("10.0.0.0/16")
 )
 
 // Platform collects AWS-specific configuration.
 func Platform() (*aws.Platform, error) {
-	longRegions := make([]string, 0, len(validAWSRegions))
-	shortRegions := make([]string, 0, len(validAWSRegions))
-	for id, location := range validAWSRegions {
+	longRegions := make([]string, 0, len(aws.ValidRegions))
+	shortRegions := make([]string, 0, len(aws.ValidRegions))
+	for id, location := range aws.ValidRegions {
 		longRegions = append(longRegions, fmt.Sprintf("%s (%s)", id, location))
 		shortRegions = append(shortRegions, id)
 	}
@@ -59,7 +37,7 @@ func Platform() (*aws.Platform, error) {
 	})
 
 	defaultRegion := "us-east-1"
-	_, ok := validAWSRegions[defaultRegion]
+	_, ok := aws.ValidRegions[defaultRegion]
 	if !ok {
 		panic(fmt.Sprintf("installer bug: invalid default AWS region %q", defaultRegion))
 	}
@@ -81,7 +59,7 @@ func Platform() (*aws.Platform, error) {
 
 	defaultRegionPointer := ssn.Config.Region
 	if defaultRegionPointer != nil && *defaultRegionPointer != "" {
-		_, ok := validAWSRegions[*defaultRegionPointer]
+		_, ok := aws.ValidRegions[*defaultRegionPointer]
 		if ok {
 			defaultRegion = *defaultRegionPointer
 		} else {
@@ -98,7 +76,7 @@ func Platform() (*aws.Platform, error) {
 			Prompt: &survey.Select{
 				Message: "Region",
 				Help:    "The AWS region to be used for installation.",
-				Default: fmt.Sprintf("%s (%s)", defaultRegion, validAWSRegions[defaultRegion]),
+				Default: fmt.Sprintf("%s (%s)", defaultRegion, aws.ValidRegions[defaultRegion]),
 				Options: longRegions,
 			},
 			Validate: survey.ComposeValidators(survey.Required, func(ans interface{}) error {

pkg/asset/installconfig/installconfig.go

@@ -12,6 +12,8 @@ import (
 	"github.com/openshift/installer/pkg/asset"
 	"github.com/openshift/installer/pkg/ipnet"
 	"github.com/openshift/installer/pkg/types"
+	openstackvalidation "github.com/openshift/installer/pkg/types/openstack/validation"
+	"github.com/openshift/installer/pkg/types/validation"
 )
 
 const (
@@ -156,6 +158,10 @@ func (a *InstallConfig) Load(f asset.FileFetcher) (found bool, err error) {
 		return false, errors.Wrapf(err, "failed to unmarshal")
 	}
 
+	if err := validation.ValidateInstallConfig(config, openstackvalidation.NewValidValuesFetcher()).ToAggregate(); err != nil {
+		return false, errors.Wrapf(err, "invalid %q file", installConfigFilename)
+	}
+
 	a.File, a.Config = file, config
 	return true, nil
 }